mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(streaming): add Slack streaming=false default to match Discord
PR #37303 added per-platform streaming defaults and the commit message explicitly called out "Discord/Slack/etc. only have edit-based streaming (repeated editMessage), which flickers and is noticeably jankier" — but only discord.streaming=false was shipped. Slack uses the same edit-based streaming mechanism and has the same flicker problem, yet it was left to follow the global switch (default true when streaming is enabled). Add "slack": {"streaming": False} to DEFAULT_CONFIG["display"]["platforms"] alongside the Discord default. The same deep-merge semantics apply: a user who explicitly sets display.platforms.slack.streaming: true keeps their value unchanged. The dashboard schema gains a slack.streaming toggle automatically since it is generated from DEFAULT_CONFIG. Update test_per_platform_streaming_defaults.py to cover slack in all existing assertions and rename the resolver test to reflect both platforms.
This commit is contained in:
parent
2946805299
commit
805c22c836
2 changed files with 23 additions and 14 deletions
|
|
@ -1976,9 +1976,9 @@ DEFAULT_CONFIG = {
|
|||
# per platform:
|
||||
# - Telegram has native animated draft streaming (sendMessageDraft),
|
||||
# which is smooth, so streaming is on by default there.
|
||||
# - Discord/Slack/etc. only have edit-based streaming (repeated
|
||||
# - Discord and Slack only have edit-based streaming (repeated
|
||||
# editMessage), which flickers and is noticeably jankier, so
|
||||
# streaming is off by default there.
|
||||
# streaming is off by default for both.
|
||||
# These are gap-fillers: a user who explicitly sets, e.g.,
|
||||
# display.platforms.discord.streaming: true keeps their value
|
||||
# (config deep-merge has user values win over defaults). The global
|
||||
|
|
@ -1987,6 +1987,7 @@ DEFAULT_CONFIG = {
|
|||
"platforms": {
|
||||
"telegram": {"streaming": True},
|
||||
"discord": {"streaming": False},
|
||||
"slack": {"streaming": False},
|
||||
},
|
||||
# Gateway runtime-metadata footer appended to the FINAL message of a turn
|
||||
# (disabled by default to keep replies minimal). When enabled, renders
|
||||
|
|
|
|||
|
|
@ -1,11 +1,11 @@
|
|||
"""Per-platform streaming defaults + dashboard exposure.
|
||||
|
||||
Streaming is smooth on Telegram (native sendMessageDraft) but flickers on
|
||||
edit-only platforms like Discord. The shipped defaults encode that:
|
||||
display.platforms.telegram.streaming=true, .discord.streaming=false. 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.
|
||||
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
|
||||
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.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -16,11 +16,12 @@ def test_default_per_platform_streaming_flags():
|
|||
plats = DEFAULT_CONFIG["display"]["platforms"]
|
||||
assert plats["telegram"]["streaming"] is True
|
||||
assert plats["discord"]["streaming"] is False
|
||||
assert plats["slack"]["streaming"] is False
|
||||
|
||||
|
||||
def test_resolver_telegram_on_discord_off_when_global_enabled():
|
||||
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 not — matching the platforms' actual streaming quality."""
|
||||
and Discord/Slack not — matching the platforms' actual streaming quality."""
|
||||
from hermes_cli.config import DEFAULT_CONFIG
|
||||
from gateway.display_config import resolve_display_setting
|
||||
|
||||
|
|
@ -34,18 +35,23 @@ def test_resolver_telegram_on_discord_off_when_global_enabled():
|
|||
|
||||
assert streams("telegram") is True
|
||||
assert streams("discord") is False
|
||||
# A platform with no default entry follows the global switch.
|
||||
assert streams("slack") is True
|
||||
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 streaming keeps their value — the
|
||||
default false must not clobber it (config deep-merge: user wins)."""
|
||||
"""A user who explicitly enables Discord or Slack streaming keeps their value
|
||||
— the default false must not clobber it (config deep-merge: user wins)."""
|
||||
from hermes_cli.config import DEFAULT_CONFIG, _deep_merge
|
||||
|
||||
user = {"display": {"platforms": {"discord": {"streaming": True}}}}
|
||||
user = {"display": {"platforms": {
|
||||
"discord": {"streaming": True},
|
||||
"slack": {"streaming": True},
|
||||
}}}
|
||||
merged = _deep_merge(dict(DEFAULT_CONFIG), user)
|
||||
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
|
||||
|
||||
|
|
@ -59,7 +65,9 @@ def test_dashboard_schema_exposes_per_platform_streaming():
|
|||
|
||||
assert "display.platforms.telegram.streaming" in CONFIG_SCHEMA
|
||||
assert "display.platforms.discord.streaming" in CONFIG_SCHEMA
|
||||
assert "display.platforms.slack.streaming" in CONFIG_SCHEMA
|
||||
assert CONFIG_SCHEMA["display.platforms.discord.streaming"]["type"] == "boolean"
|
||||
assert CONFIG_SCHEMA["display.platforms.slack.streaming"]["type"] == "boolean"
|
||||
# Global streaming controls are exposed too.
|
||||
assert "streaming.enabled" in CONFIG_SCHEMA
|
||||
assert "streaming.transport" in CONFIG_SCHEMA
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue