mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(gateway): normalize YAML boolean streaming mode and keep enabled a mode-only alias
Address PR #62873 review: - Bare YAML `mode: off`/`on` parse to Python False/True (YAML 1.1). Stringifying False yielded "false" (not "off"), so `mode: off` wrongly enabled streaming. Add _normalize_transport_token() to map booleans to canonical off/auto tokens, mirroring the normalization documented in gateway/display_config.py. - Only the `mode` alias infers `enabled`; a bare `transport` no longer enables streaming, preserving `streaming.enabled` as the documented master switch (website/docs/user-guide/configuration.md). - Update tests to the corrected contract and add YAML-boolean coverage plus loader-level regressions for unquoted `mode: off` and nested mode enable.
This commit is contained in:
parent
62cdb3e1be
commit
4cbceae9f4
2 changed files with 90 additions and 12 deletions
|
|
@ -74,6 +74,23 @@ def _env_multiplex_profiles_override() -> "bool | None":
|
|||
return None
|
||||
|
||||
|
||||
def _normalize_transport_token(value: Any) -> str:
|
||||
"""Normalize a streaming transport/mode value to a canonical token.
|
||||
|
||||
Handles the YAML 1.1 boolean quirk where bare ``on`` / ``off`` parse to
|
||||
Python ``True`` / ``False`` (see ``gateway/display_config.py`` ``_normalise``).
|
||||
Without this, ``mode: off`` arrives as boolean ``False`` and stringifying it
|
||||
yields ``"false"`` instead of the advertised ``"off"``, so streaming would be
|
||||
enabled instead of disabled. Booleans map to ``"auto"`` (True) / ``"off"``
|
||||
(False); anything else is lower-cased, defaulting to ``"auto"``.
|
||||
"""
|
||||
if value is None:
|
||||
return "auto"
|
||||
if isinstance(value, bool):
|
||||
return "auto" if value else "off"
|
||||
return str(value).strip().lower() or "auto"
|
||||
|
||||
|
||||
def _coerce_float(value: Any, default: float) -> float:
|
||||
"""Coerce numeric config values, falling back on malformed input."""
|
||||
if value is None:
|
||||
|
|
@ -727,19 +744,25 @@ class StreamingConfig:
|
|||
# a surprising footgun: the whole reply buffers and sends at once.
|
||||
# ``mode: off`` disables streaming; an explicit ``enabled`` key always
|
||||
# wins so callers can force either state.
|
||||
#
|
||||
# ``transport`` alone does NOT imply ``enabled``: ``streaming.enabled``
|
||||
# is the documented master switch (see website/docs/user-guide/
|
||||
# configuration.md), so a bare ``transport`` only selects HOW to stream
|
||||
# once streaming is on. Only the ``mode`` alias flips ``enabled``.
|
||||
raw_transport = data.get("transport")
|
||||
raw_mode = data.get("mode")
|
||||
transport = raw_transport if raw_transport is not None else raw_mode
|
||||
if transport is None:
|
||||
transport = "auto"
|
||||
transport = str(transport).strip().lower() or "auto"
|
||||
# Normalize both through the same helper so YAML's bare ``off``/``on``
|
||||
# (parsed as bool False/True) become canonical tokens rather than
|
||||
# ``"false"``/``"true"``.
|
||||
picked = raw_transport if raw_transport is not None else raw_mode
|
||||
transport = _normalize_transport_token(picked)
|
||||
|
||||
if "enabled" in data:
|
||||
enabled = _coerce_bool(data.get("enabled"), False)
|
||||
elif raw_mode is not None or raw_transport is not None:
|
||||
# A mode/transport was given without an explicit enabled flag:
|
||||
# infer enabled from the transport ("off" = disabled, else on).
|
||||
enabled = transport != "off"
|
||||
elif raw_mode is not None:
|
||||
# The ``mode`` alias (and only ``mode``) infers enabled:
|
||||
# ``off`` disables, anything else enables.
|
||||
enabled = _normalize_transport_token(raw_mode) != "off"
|
||||
else:
|
||||
enabled = False
|
||||
|
||||
|
|
|
|||
|
|
@ -90,12 +90,17 @@ class TestStreamingModeAlias:
|
|||
# transport still resolves from mode
|
||||
assert sc.transport == "auto"
|
||||
|
||||
def test_transport_takes_precedence_over_mode(self):
|
||||
def test_transport_selects_transport_but_mode_controls_enabled(self):
|
||||
"""``transport`` picks HOW to stream; ``mode`` is the enable alias.
|
||||
|
||||
With ``mode: off`` the master switch is off even though ``transport``
|
||||
selects the draft path — ``enabled`` is inferred from ``mode`` only.
|
||||
"""
|
||||
from gateway.config import StreamingConfig
|
||||
|
||||
sc = StreamingConfig.from_dict({"mode": "off", "transport": "draft"})
|
||||
assert sc.transport == "draft"
|
||||
assert sc.enabled is True
|
||||
assert sc.enabled is False
|
||||
|
||||
def test_empty_block_stays_disabled(self):
|
||||
from gateway.config import StreamingConfig
|
||||
|
|
@ -103,9 +108,59 @@ class TestStreamingModeAlias:
|
|||
sc = StreamingConfig.from_dict({})
|
||||
assert sc.enabled is False
|
||||
|
||||
def test_transport_only_enables(self):
|
||||
def test_transport_only_does_not_enable(self):
|
||||
"""``streaming.enabled`` is the documented master switch, so a bare
|
||||
``transport`` must not flip streaming on by itself."""
|
||||
from gateway.config import StreamingConfig
|
||||
|
||||
sc = StreamingConfig.from_dict({"transport": "edit"})
|
||||
assert sc.enabled is True
|
||||
assert sc.enabled is False
|
||||
# transport is still recorded so it takes effect once streaming is on.
|
||||
assert sc.transport == "edit"
|
||||
|
||||
|
||||
class TestStreamingYamlBooleanQuirk:
|
||||
"""YAML 1.1 parses bare ``off``/``on`` as booleans; ``mode``/``transport``
|
||||
must normalize those back to canonical string tokens.
|
||||
|
||||
Regression for the review on PR #62873: bare ``mode: off`` arrived as
|
||||
Python ``False`` and stringified to ``"false"``, which is not ``"off"``,
|
||||
so streaming was enabled instead of honoring the advertised disable.
|
||||
"""
|
||||
|
||||
def test_mode_bare_off_boolean_disables(self):
|
||||
from gateway.config import StreamingConfig
|
||||
|
||||
# yaml.safe_load("off") -> False
|
||||
sc = StreamingConfig.from_dict({"mode": False})
|
||||
assert sc.enabled is False
|
||||
assert sc.transport == "off"
|
||||
|
||||
def test_mode_bare_on_boolean_enables(self):
|
||||
from gateway.config import StreamingConfig
|
||||
|
||||
# yaml.safe_load("on") -> True
|
||||
sc = StreamingConfig.from_dict({"mode": True})
|
||||
assert sc.enabled is True
|
||||
assert sc.transport == "auto"
|
||||
|
||||
def test_transport_bare_off_boolean_normalizes(self):
|
||||
from gateway.config import StreamingConfig
|
||||
|
||||
# transport alone never enables, but the token must still normalize.
|
||||
sc = StreamingConfig.from_dict({"transport": False})
|
||||
assert sc.enabled is False
|
||||
assert sc.transport == "off"
|
||||
|
||||
def test_loader_normalizes_bare_yaml_off(self):
|
||||
"""End-to-end through load_gateway_config(): unquoted ``mode: off``
|
||||
(a YAML boolean) must keep streaming disabled."""
|
||||
cfg = _load_with_yaml_dict({"streaming": {"mode": False}})
|
||||
assert cfg.streaming.enabled is False
|
||||
assert cfg.streaming.transport == "off"
|
||||
|
||||
def test_loader_nested_mode_enables(self):
|
||||
"""Nested gateway.streaming.mode alias enables through the loader."""
|
||||
cfg = _load_with_yaml_dict({"gateway": {"streaming": {"mode": "edit"}}})
|
||||
assert cfg.streaming.enabled is True
|
||||
assert cfg.streaming.transport == "edit"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue