fix(gateway): resolve progress mode and intent from the same source
This commit is contained in:
committed by
Teknium
parent
bc125d59d5
commit
dab7eebf47
+32
-10
@@ -84,22 +84,44 @@ def resolve_display_setting(user_config: dict, platform_key: str, setting: str,
|
||||
``platform_key`` is the platform config key (``"telegram"``; see ``_platform_config_key`` in
|
||||
gateway/run.py). Returns *fallback* when nothing is configured.
|
||||
"""
|
||||
display_cfg = user_config.get("display") or {}
|
||||
plat_overrides = (display_cfg.get("platforms") or {}).get(platform_key)
|
||||
if isinstance(plat_overrides, dict) and plat_overrides.get(setting) is not None:
|
||||
return _normalise(setting, plat_overrides[setting])
|
||||
if setting == "tool_progress": # legacy display.tool_progress_overrides.<platform>
|
||||
legacy = display_cfg.get("tool_progress_overrides")
|
||||
if isinstance(legacy, dict) and legacy.get(platform_key) is not None:
|
||||
return _normalise(setting, legacy[platform_key])
|
||||
if setting != "streaming" and display_cfg.get(setting) is not None: # display.streaming is CLI-only
|
||||
return _normalise(setting, display_cfg[setting])
|
||||
configured = _configured_display_value(user_config, platform_key, setting)
|
||||
if configured is not None:
|
||||
return _normalise(setting, configured)
|
||||
val = _PLATFORM_DEFAULTS.get(platform_key, {}).get(setting)
|
||||
if val is None:
|
||||
val = _GLOBAL_DEFAULTS.get(setting)
|
||||
return fallback if val is None else val
|
||||
|
||||
|
||||
def _configured_display_value(user_config: dict, platform_key: str, setting: str) -> Any:
|
||||
"""First non-None operator value, without introducing tier defaults."""
|
||||
display_cfg = user_config.get("display") or {}
|
||||
plat_overrides = (display_cfg.get("platforms") or {}).get(platform_key)
|
||||
if isinstance(plat_overrides, dict) and plat_overrides.get(setting) is not None:
|
||||
return plat_overrides[setting]
|
||||
if setting == "tool_progress":
|
||||
legacy = display_cfg.get("tool_progress_overrides")
|
||||
if isinstance(legacy, dict) and legacy.get(platform_key) is not None:
|
||||
return legacy[platform_key]
|
||||
if setting != "streaming": # display.streaming is CLI-only
|
||||
return display_cfg.get(setting)
|
||||
return None
|
||||
|
||||
|
||||
def resolve_tool_progress(user_config: dict, platform_key: str, env_mode: str | None = None) -> tuple[str, bool]:
|
||||
"""Return (mode, explicit intent) from the same winning source.
|
||||
|
||||
Non-None YAML wins over the legacy env bridge. Null inherits through to env,
|
||||
then tier defaults. A tier's off is not an operator request to disable cards.
|
||||
"""
|
||||
configured = _configured_display_value(user_config, platform_key, "tool_progress")
|
||||
if configured is not None:
|
||||
return _normalise("tool_progress", configured), True
|
||||
if env_mode:
|
||||
return _normalise("tool_progress", env_mode), True
|
||||
return resolve_display_setting(user_config, platform_key, "tool_progress"), False
|
||||
|
||||
|
||||
# --- Normalisation of YAML quirks (bare ``off`` → False in YAML 1.1, etc.) ---
|
||||
|
||||
_TRUTHY = {"true", "1", "yes", "on"}
|
||||
|
||||
+4
-23
@@ -2710,11 +2710,6 @@ class GatewayTurnMixin:
|
||||
platform_key = _platform_config_key(source.platform)
|
||||
enabled_toolsets, disabled_toolsets = self._resolve_turn_toolsets(user_config, source, platform_key)
|
||||
adapter = self._adapter_for_source(source)
|
||||
# display.platforms.<platform>.<key> → display.<key> → built-in platform defaults.
|
||||
_display_cfg = user_config.get("display", {})
|
||||
if not isinstance(_display_cfg, dict):
|
||||
_display_cfg = {}
|
||||
|
||||
# Tool preview length (0 = no limit) and friendly tool labels (default on), per-platform.
|
||||
for _setter, _setting, _default, _cast in (
|
||||
("set_tool_preview_max_len", "tool_preview_length", 0, lambda v: int(v) if v else 0),
|
||||
@@ -2725,24 +2720,10 @@ class GatewayTurnMixin:
|
||||
_val = resolve_display_setting(user_config, platform_key, _setting, _default)
|
||||
getattr(_agent_display, _setter)(_cast(_val))
|
||||
|
||||
# Tool progress mode; HERMES_TOOL_PROGRESS_MODE wins only when the config never set it.
|
||||
_resolved_tp = resolve_display_setting(user_config, platform_key, "tool_progress")
|
||||
_env_tp = os.getenv("HERMES_TOOL_PROGRESS_MODE")
|
||||
_platform_cfg = (_display_cfg.get("platforms") or {}).get(platform_key) or {}
|
||||
_legacy_tp_overrides = _display_cfg.get("tool_progress_overrides") or {}
|
||||
_tool_progress_configured = "tool_progress" in _display_cfg or any(
|
||||
isinstance(cfg, dict) and key in cfg
|
||||
for cfg, key in ((_platform_cfg, "tool_progress"), (_legacy_tp_overrides, platform_key))
|
||||
)
|
||||
progress_mode = _env_tp if _env_tp and not _tool_progress_configured else (_resolved_tp or _env_tp or "all")
|
||||
# Operator intent vs tier default: True when a human WROTE a mode (config or env). A ``null``
|
||||
# value is inheritance, not intent — the resolver skips None (display_config.py), so a bare
|
||||
# key with no value must not read as "explicitly off".
|
||||
_tool_progress_explicit = bool(_env_tp) or any(
|
||||
isinstance(cfg, dict) and cfg.get(key) is not None
|
||||
for cfg, key in (
|
||||
(_display_cfg, "tool_progress"), (_platform_cfg, "tool_progress"), (_legacy_tp_overrides, platform_key),
|
||||
)
|
||||
# Resolve the mode and its provenance together: null inherits, tier off is not intent.
|
||||
from gateway.display_config import resolve_tool_progress
|
||||
progress_mode, _tool_progress_explicit = resolve_tool_progress(
|
||||
user_config, platform_key, os.getenv("HERMES_TOOL_PROGRESS_MODE"),
|
||||
)
|
||||
# "accumulate" (edit one bubble) or "separate" (one msg per tool)
|
||||
progress_grouping = resolve_display_setting(user_config, platform_key, "tool_progress_grouping") or "accumulate"
|
||||
|
||||
@@ -5,6 +5,25 @@
|
||||
# Resolver: resolution order
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestToolProgressProvenance:
|
||||
def test_winning_source_controls_mode_and_intent(self):
|
||||
from gateway.display_config import resolve_tool_progress
|
||||
|
||||
cases = [
|
||||
({}, None, ("off", False)),
|
||||
({}, "all", ("all", True)),
|
||||
({"tool_progress": None}, "all", ("all", True)),
|
||||
({"platforms": {"slack": {"tool_progress": None}}}, "off", ("off", True)),
|
||||
({"tool_progress_overrides": {"slack": None}}, "new", ("new", True)),
|
||||
({"tool_progress": False}, "all", ("off", True)),
|
||||
({"tool_progress": "all", "platforms": {"slack": {"tool_progress": None}}}, "off", ("all", True)),
|
||||
({"tool_progress": "off", "tool_progress_overrides": {"slack": "new"}}, "all", ("new", True)),
|
||||
({"tool_progress_overrides": {"slack": "off"}, "platforms": {"slack": {"tool_progress": "all"}}}, None, ("all", True)),
|
||||
]
|
||||
for display, env, expected in cases:
|
||||
assert resolve_tool_progress({"display": display}, "slack", env) == expected
|
||||
|
||||
|
||||
class TestResolveDisplaySetting:
|
||||
"""resolve_display_setting() resolves with correct priority."""
|
||||
|
||||
|
||||
@@ -1187,8 +1187,12 @@ async def test_slack_operator_tool_progress_off_disables_task_cards(monkeypatch,
|
||||
],
|
||||
ids=["platform-null", "global-null", "legacy-null", "platform-null-over-global-all"],
|
||||
)
|
||||
async def test_slack_null_tool_progress_is_inheritance_not_explicit_off(monkeypatch, tmp_path, display_cfg):
|
||||
@pytest.mark.parametrize("env_mode", [None, "all", "new"])
|
||||
async def test_slack_null_tool_progress_is_inheritance_not_explicit_off(monkeypatch, tmp_path, display_cfg, env_mode):
|
||||
# A bare key with ``null`` inherits (the resolver skips None); it is not an operator saying "off".
|
||||
monkeypatch.delenv("HERMES_TOOL_PROGRESS_MODE", raising=False)
|
||||
if env_mode is not None:
|
||||
monkeypatch.setenv("HERMES_TOOL_PROGRESS_MODE", env_mode)
|
||||
adapter, result = await _run_with_agent(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
|
||||
Reference in New Issue
Block a user