Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions gateway/display_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,18 @@

_PLATFORM_DEFAULTS: dict[str, dict[str, Any]] = {
# Tier 1 — full edit support, personal/team use
# Telegram is usually a mobile inbox: keep tool_progress quiet and skip
# the verbose busy-ack iteration counter, but DO surface real mid-turn
# assistant commentary (interim_assistant_messages) and DO send periodic
# heartbeats (long_running_notifications) so the user has signal between
# turn start and final answer. Otherwise it looks like "typing..." for
# 30 minutes with nothing happening. Opt in to verbose iteration detail
# via display.platforms.telegram.busy_ack_detail / tool_progress.
# Telegram is usually a durable mobile inbox. Bot-authored progress and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reverses the explicit maintainer decision in 0325e18f (#33187): Telegram should retain real mid-turn commentary because suppressing it left users with only a typing indicator during long turns. The existing per-platform setting already lets users opt out.

# interim assistant/commentary fragments stay in chat history and can look

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main addresses durable-chat noise without disabling the liveness signal: gateway/run.py:19358-19436 edits a single heartbeat message in place. Please preserve this default unless maintainers choose to reverse the #33187 product decision.

# like leaked internal state or the wrong identity when mirrored through
# userbot/Business tooling. Default to final-answer-first; users can opt in
# explicitly per platform.
"telegram": {
**_TIER_HIGH,
"streaming": False,
"tool_progress": "off",
"interim_assistant_messages": False,
"long_running_notifications": False,
"cleanup_progress": True,
"busy_ack_detail": False,
},
# Discord has a native "subtext" primitive (-# small grey text) that reads
Expand Down
7 changes: 6 additions & 1 deletion gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -20150,7 +20150,12 @@ def voice_ack_callback(call_id, tool_name, args):
)
_cleanup_adapter = self._adapter_for_source(source) if _cleanup_progress else None
if _cleanup_adapter is not None and (
type(_cleanup_adapter).delete_message is BasePlatformAdapter.delete_message
getattr(
type(_cleanup_adapter),
"delete_message",
BasePlatformAdapter.delete_message,
)
is BasePlatformAdapter.delete_message
):
# Adapter doesn't support deletion — silently disable.
_cleanup_progress = False
Expand Down
20 changes: 15 additions & 5 deletions hermes_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2010,10 +2010,14 @@ def _ensure_hermes_home_managed(home: Path):
# display settings that override the global value for that platform
# only. A setting left unset here falls through to the global default.
#
# Shipped defaults encode the streaming experience that works best
# per platform:
# - Telegram has native animated draft streaming (sendMessageDraft),
# which is smooth, so streaming is on by default there.
# Shipped defaults encode the streaming/progress experience that works
# best per platform:
# - Telegram is a durable mobile inbox and may be mirrored through
# userbot/Business tooling. Keep persistent progress/interim chatter
# off by default; otherwise internal fragments or tool bubbles
# remain in chat history and can look misattributed.
# - Telegram draft streaming can be enabled explicitly where desired;
# final answers still send normally when streaming/progress are off.
# - Discord and Slack only have edit-based streaming (repeated
# editMessage), which flickers and is noticeably jankier, so
# streaming is off by default for both.
Expand All @@ -2023,7 +2027,13 @@ def _ensure_hermes_home_managed(home: Path):
# streaming.enabled master switch still gates everything — these
# per-platform flags only take effect once streaming is enabled.
"platforms": {
"telegram": {"streaming": True},
"telegram": {
"streaming": False,
"tool_progress": "off",
"interim_assistant_messages": False,
"long_running_notifications": False,
"cleanup_progress": True,
},
"discord": {"streaming": False},
"slack": {"streaming": False},
},
Expand Down
48 changes: 23 additions & 25 deletions tests/gateway/test_display_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,13 @@ class TestPlatformDefaults:
"""Built-in defaults reflect platform capability tiers."""

def test_high_tier_platforms(self):
"""Discord defaults to 'all'; Telegram defaults quiet for mobile."""
"""Discord defaults to 'all'; Telegram defaults final-answer-first."""
from gateway.display_config import resolve_display_setting

# Telegram: tier_high transport, but quiet mobile default.
assert resolve_display_setting({}, "telegram", "tool_progress") == "off"
assert resolve_display_setting({}, "telegram", "interim_assistant_messages") is False
assert resolve_display_setting({}, "telegram", "long_running_notifications") is False
# Discord: pure tier_high.
assert resolve_display_setting({}, "discord", "tool_progress") == "all"

Expand Down Expand Up @@ -302,23 +304,19 @@ def test_low_tier_streaming_defaults_to_false(self):
assert resolve_display_setting({}, "signal", "streaming") is False
assert resolve_display_setting({}, "email", "streaming") is False

def test_high_tier_streaming_defaults_to_none(self):
"""High-tier platforms default streaming to None (follow global)."""
def test_telegram_raw_config_defaults_to_final_answer_first(self):
"""Raw gateway config must keep Telegram final-answer-first."""
from gateway.display_config import resolve_display_setting

assert resolve_display_setting({}, "telegram", "streaming") is None
assert resolve_display_setting({}, "telegram", "streaming") is False
assert resolve_display_setting({}, "telegram", "cleanup_progress") is True

def test_telegram_mobile_chatter_defaults(self):
"""Telegram keeps real mid-turn signal (interim commentary + heartbeats)
but skips the verbose busy-ack iteration counter by default."""
"""Telegram avoids persistent interim chatter by default."""
from gateway.display_config import resolve_display_setting

# Real model voice — keep on. Without this, Telegram users see
# "typing..." for the entire turn duration with no feedback.
assert resolve_display_setting({}, "telegram", "interim_assistant_messages") is True
# Periodic "Working — N min" heartbeat — keep on. Otherwise long
# turns appear completely silent.
assert resolve_display_setting({}, "telegram", "long_running_notifications") is True
assert resolve_display_setting({}, "telegram", "interim_assistant_messages") is False
assert resolve_display_setting({}, "telegram", "long_running_notifications") is False
# Verbose iteration counter in busy-ack and heartbeat — off by
# default on Telegram (mobile chat is cramped enough without
# "iteration 21/60" debug detail).
Expand All @@ -337,23 +335,22 @@ def test_slack_workspace_chatter_defaults(self):
assert resolve_display_setting({}, "slack", "busy_ack_detail") is False

def test_telegram_mobile_chatter_can_opt_in(self):
"""Per-platform config can re-enable Telegram busy-ack detail
and re-disable the kept-on defaults."""
"""Explicit per-platform config can opt Telegram into chatter."""
from gateway.display_config import resolve_display_setting

config = {
"display": {
"platforms": {
"telegram": {
"interim_assistant_messages": False,
"long_running_notifications": False,
"interim_assistant_messages": True,
"long_running_notifications": True,
"busy_ack_detail": "on",
}
}
}
}
assert resolve_display_setting(config, "telegram", "interim_assistant_messages") is False
assert resolve_display_setting(config, "telegram", "long_running_notifications") is False
assert resolve_display_setting(config, "telegram", "interim_assistant_messages") is True
assert resolve_display_setting(config, "telegram", "long_running_notifications") is True
assert resolve_display_setting(config, "telegram", "busy_ack_detail") is True


Expand Down Expand Up @@ -430,8 +427,8 @@ def test_none_means_follow_global(self):
from gateway.display_config import resolve_display_setting

config = {}
# Telegram has no streaming override in defaults → None
result = resolve_display_setting(config, "telegram", "streaming")
# Discord has no built-in streaming override → None.
result = resolve_display_setting(config, "discord", "streaming")
assert result is None # caller should check global StreamingConfig

def test_global_display_streaming_is_cli_only(self):
Expand All @@ -440,7 +437,7 @@ def test_global_display_streaming_is_cli_only(self):

for value in (True, False):
config = {"display": {"streaming": value}}
assert resolve_display_setting(config, "telegram", "streaming") is None
assert resolve_display_setting(config, "telegram", "streaming") is False
assert resolve_display_setting(config, "discord", "streaming") is None

def test_explicit_false_disables(self):
Expand Down Expand Up @@ -471,14 +468,15 @@ def test_explicit_true_enables(self):
# ---------------------------------------------------------------------------

class TestCleanupProgress:
"""``cleanup_progress`` is off by default and resolvable per-platform."""
"""``cleanup_progress`` defaults per platform and remains configurable."""

def test_default_off_for_all_platforms(self):
"""No config set → cleanup_progress resolves to False everywhere."""
def test_telegram_defaults_cleanup_on_other_platforms_off(self):
"""Telegram cleans temporary progress; other platforms preserve it."""
from gateway.display_config import resolve_display_setting

for plat in ("telegram", "discord", "slack", "email"):
for plat in ("discord", "slack", "email"):
assert resolve_display_setting({}, plat, "cleanup_progress") is False
assert resolve_display_setting({}, "telegram", "cleanup_progress") is True

def test_global_true_applies_to_all_platforms(self):
"""display.cleanup_progress=true opts in globally."""
Expand Down
37 changes: 23 additions & 14 deletions tests/gateway/test_per_platform_streaming_defaults.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
"""Per-platform streaming defaults + dashboard exposure.

Streaming is smooth on Telegram (native sendMessageDraft) but flickers on
edit-only platforms like Discord and Slack (repeated editMessage). The shipped
defaults encode that: display.platforms.telegram.streaming=true,
.discord.streaming=false, .slack.streaming=false. These are gap-fillers (user
Telegram is a durable mobile inbox, so shipped defaults keep persistent
streaming/progress/interim chatter off unless explicitly enabled. Discord and
Slack edit-based streaming also defaults off. These are gap-fillers (user
values win via deep-merge) and, because the dashboard schema is generated from
DEFAULT_CONFIG, they automatically appear as editable toggles in the web UI.
DEFAULT_CONFIG, they automatically appear as editable controls in the web UI.
"""

from __future__ import annotations
Expand All @@ -14,14 +13,19 @@
def test_default_per_platform_streaming_flags():
from hermes_cli.config import DEFAULT_CONFIG
plats = DEFAULT_CONFIG["display"]["platforms"]
assert plats["telegram"]["streaming"] is True
assert plats["telegram"] == {
"streaming": False,
"tool_progress": "off",
"interim_assistant_messages": False,
"long_running_notifications": False,
"cleanup_progress": True,
}
assert plats["discord"]["streaming"] is False
assert plats["slack"]["streaming"] is False


def test_resolver_telegram_on_discord_and_slack_off_when_global_enabled():
"""With global streaming on, the per-platform defaults make Telegram stream
and Discord/Slack not — matching the platforms' actual streaming quality."""
def test_resolver_telegram_discord_and_slack_off_when_global_enabled():
"""Per-platform safety defaults beat the enabled global streaming switch."""
from hermes_cli.config import DEFAULT_CONFIG
from gateway.display_config import resolve_display_setting

Expand All @@ -33,27 +37,32 @@ def streams(plat):
# global enabled; None override = follow global (True)
return True if ov is None else bool(ov)

assert streams("telegram") is True
assert streams("telegram") is False
assert streams("discord") is False
assert streams("slack") is False
# A platform with no default entry still follows the global switch.
assert streams("matrix") is True


def test_user_override_wins_over_default():
"""A user who explicitly enables Discord or Slack streaming keeps their value
— the default false must not clobber it (config deep-merge: user wins)."""
"""Explicit per-platform values win without clobbering sibling defaults."""
from hermes_cli.config import DEFAULT_CONFIG, _deep_merge

user = {"display": {"platforms": {
"telegram": {
"streaming": True,
"tool_progress": "all",
"interim_assistant_messages": True,
"long_running_notifications": True,
"cleanup_progress": False,
},
"discord": {"streaming": True},
"slack": {"streaming": True},
}}}
merged = _deep_merge(dict(DEFAULT_CONFIG), user)
assert merged["display"]["platforms"]["telegram"] == user["display"]["platforms"]["telegram"]
assert merged["display"]["platforms"]["discord"]["streaming"] is True
assert merged["display"]["platforms"]["slack"]["streaming"] is True
# Partial override must not wipe the sibling telegram default.
assert merged["display"]["platforms"]["telegram"]["streaming"] is True


def test_dashboard_schema_exposes_per_platform_streaming():
Expand Down
62 changes: 48 additions & 14 deletions tests/gateway/test_run_cleanup_progress.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"""Tests for opt-in cleanup of temporary progress bubbles.
"""Tests for cleanup of temporary progress bubbles.

When ``display.platforms.<plat>.cleanup_progress: true`` is set for a
platform whose adapter supports message deletion (e.g. Telegram), the
Telegram enables cleanup by default. Other platforms can opt in with
``display.platforms.<plat>.cleanup_progress: true``, and Telegram can opt out
with an explicit ``false``. For adapters that support message deletion, the
tool-progress bubble, "⏳ Working — N min" heartbeats, and status-callback
messages sent during a run are deleted after the final response is
delivered.
messages sent during a run are deleted after the final response is delivered.

Failed runs skip cleanup so the bubbles remain as breadcrumbs.
Adapters without ``delete_message`` silently no-op.
Expand Down Expand Up @@ -170,7 +170,7 @@ def _install_fakes(
monkeypatch,
agent_cls,
*,
cleanup_on: bool,
cleanup_on: bool | None,
cleanup_platform: Platform = Platform.TELEGRAM,
):
"""Wire up the module stubs every _run_agent test needs."""
Expand All @@ -190,13 +190,17 @@ def _install_fakes(

# Wire the per-platform cleanup_progress flag via the config loader the
# gateway actually reads (``_load_gateway_config`` returns user config).
cfg = {
"display": {
"platforms": {
cleanup_platform.value: {"cleanup_progress": True},
cfg = (
{}
if cleanup_on is None
else {
"display": {
"platforms": {
cleanup_platform.value: {"cleanup_progress": cleanup_on},
}
}
}
} if cleanup_on else {}
)
monkeypatch.setattr(gateway_run, "_load_gateway_config", lambda: cfg)
return gateway_run

Expand All @@ -207,9 +211,8 @@ def _install_fakes(


@pytest.mark.asyncio
async def test_cleanup_off_by_default_leaves_bubbles(monkeypatch, tmp_path):
"""Without ``cleanup_progress: true``, firing whatever callback is
registered never reaches delete_message."""
async def test_explicit_telegram_cleanup_opt_out_leaves_bubbles(monkeypatch, tmp_path):
"""An explicit Telegram ``cleanup_progress: false`` preserves bubbles."""
adapter = CleanupCaptureAdapter()
runner = _make_runner(adapter)
gateway_run = _install_fakes(monkeypatch, ProgressAgent, cleanup_on=False)
Expand Down Expand Up @@ -239,6 +242,37 @@ async def test_cleanup_off_by_default_leaves_bubbles(monkeypatch, tmp_path):
assert adapter.deleted == []


@pytest.mark.asyncio
async def test_telegram_raw_default_deletes_progress_bubbles(monkeypatch, tmp_path):
"""Raw Telegram config enables cleanup without a user override."""
adapter = CleanupCaptureAdapter()
runner = _make_runner(adapter)
gateway_run = _install_fakes(monkeypatch, ProgressAgent, cleanup_on=None)
monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path)

source = SessionSource(platform=Platform.TELEGRAM, chat_id="-1001")
session_key = "agent:main:telegram:group:-1001"

result = await runner._run_agent(
message="hello",
context_prompt="",
history=[],
source=source,
session_id="sess-raw-default",
session_key=session_key,
)

assert result["final_response"] == "done"
cb = adapter.pop_post_delivery_callback(session_key)
assert callable(cb)
await _fire_post_delivery_cb(cb)
for _ in range(20):
await asyncio.sleep(0.01)
if adapter.deleted:
break
assert len(adapter.deleted) >= 1, f"deleted={adapter.deleted} sent={adapter.sent}"


@pytest.mark.asyncio
async def test_messaging_agent_forwards_checkpoint_config(monkeypatch, tmp_path):
"""Writable gateway agents must receive the configured checkpoint limits."""
Expand Down
12 changes: 10 additions & 2 deletions tests/gateway/test_run_progress_topics.py
Original file line number Diff line number Diff line change
Expand Up @@ -1028,12 +1028,16 @@ async def test_run_agent_surfaces_real_interim_commentary(monkeypatch, tmp_path)


@pytest.mark.asyncio
async def test_run_agent_surfaces_interim_commentary_by_default(monkeypatch, tmp_path):
async def test_run_agent_surfaces_interim_commentary_by_default_on_discord(monkeypatch, tmp_path):
adapter, result = await _run_with_agent(
monkeypatch,
tmp_path,
CommentaryAgent,
session_id="sess-commentary-default-on",
platform=Platform.DISCORD,
chat_id="discord-channel-1",
chat_type="channel",
thread_id="",
)

assert any(call["content"] == "I'll inspect the repo first." for call in adapter.sent)
Expand Down Expand Up @@ -1079,7 +1083,11 @@ async def test_run_agent_streaming_does_not_enable_completed_interim_commentary(
CommentaryAgent,
session_id="sess-commentary-streaming",
config_data={
"display": {"tool_progress": "off", "interim_assistant_messages": False},
"display": {
"tool_progress": "off",
"interim_assistant_messages": False,
"platforms": {"telegram": {"streaming": True}},
},
"streaming": {"enabled": True},
},
)
Expand Down
Loading