From df0d904d878a1bcff7d5408f44e4eb56eb30fce4 Mon Sep 17 00:00:00 2001 From: nesquena <[email protected]> Date: Sun, 3 May 2026 16:34:25 +0000 Subject: [PATCH] fix(streaming): pass agent.reasoning_effort into WebUI agents (salvages #1531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spliced from #1531 by @Asunfly: take Change-1 only (the actual bug fix + cache signature inclusion) and skip Change-2 (auxiliary title-route extra_body change) which is a separate scope concern. ## What Two surgical fixes in api/streaming.py: 1. Line 1820 — `_cfg.cfg.get(...)` → `_cfg.get(...)`. `get_config()` returns a plain dict (not a wrapper exposing `.cfg`). The buggy line raised AttributeError that the surrounding try/except swallowed, so `_reasoning_config` was always None regardless of what `/reasoning ` had been set to. Verified locally — `api/streaming.py:1959` already correctly used `_cfg.get(...)` in the same function, so the same `_cfg` was being read two different ways in one file. 2. Line 1888 — added `_reasoning_config or {}` to `_sig_blob`. Without this, switching effort mid-session would fail to take effect because the per-session agent cache key would still match the old entry. Mirrors how `resolved_provider` / `resolved_base_url` already participate in the signature. ## Why splice instead of merge #1531 directly @Asunfly force-pushed a Change-2 onto #1531 after the original review that removes `extra_body={"reasoning": {"enabled": False}}` from `generate_title_raw_via_aux` (the auxiliary title-generation route). That intent is reasonable (let operator-configured `extra_body.reasoning` flow through to the title route) but it touches a different surface and deserves its own PR. The narrow concern is operators who selected a reasoning-capable auxiliary title model without explicitly setting `reasoning.enabled=False` in the task config — pre-Change-2 the WebUI defended against accidental reasoning on the title hot path; post-Change-2 those configs would reason on every new conversation`s title, with cost and latency implications. ## What is NOT in this PR - The `generate_title_raw_via_aux` extra_body refactor (Change-2 from #1531). - The `test_does_not_override_configured_reasoning_extra_body` test (guards Change-2). Asunfly can re-open that as its own focused PR. ## Tests Two new R17b/R17c regression assertions in tests/test_regressions.py: - `test_streaming_reads_reasoning_effort_from_config_dict` — static-source guard: `_cfg.cfg` must not return to streaming.py - `test_streaming_agent_cache_signature_includes_reasoning_config` — catches removal of `_reasoning_config` from `_sig_blob` ## Closes - Closes #1531 (the Change-1 portion ships here; Asunfly can re-open Change-2 as a separate PR if desired) Co-authored-by: Asunfly <[email protected]> --- api/streaming.py | 3 ++- tests/test_regressions.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/api/streaming.py b/api/streaming.py index bb208a41..648df143 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1817,7 +1817,7 @@ def _run_agent_streaming( # the key is absent or invalid, pass None → agent uses its default. try: from api.config import parse_reasoning_effort as _parse_reff - _effort_cfg = _cfg.cfg.get('agent', {}) if isinstance(_cfg.cfg, dict) else {} + _effort_cfg = _cfg.get('agent', {}) if isinstance(_cfg, dict) else {} _effort_raw = _effort_cfg.get('reasoning_effort') if isinstance(_effort_cfg, dict) else None _reasoning_config = _parse_reff(_effort_raw) except Exception: @@ -1885,6 +1885,7 @@ def _run_agent_streaming( _max_tokens_cfg or '', _fallback_resolved or {}, sorted(_toolsets) if _toolsets else [], + _reasoning_config or {}, ], sort_keys=True) _agent_sig = _hashlib.sha256(_sig_blob.encode()).hexdigest()[:16] diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 906799b4..126a1e6a 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -608,6 +608,38 @@ def test_streaming_bridge_accepts_current_tool_progress_callback_signature(clean "streaming.py must emit live tool completion SSE events" +def test_streaming_reads_reasoning_effort_from_config_dict(cleanup_test_sessions): + """R17b: WebUI must read agent.reasoning_effort from the dict returned by get_config(). + + `get_config()` returns a plain dict (not a wrapper exposing `.cfg`). The + pre-fix line `_cfg.cfg.get('agent', {})` raised AttributeError that the + surrounding try/except swallowed, so `_reasoning_config` was always None + regardless of what `/reasoning ` had been set to. This static + source assertion pins the fix because the runtime symptom is silent. + """ + src = (REPO_ROOT / "api/streaming.py").read_text() + assert "_cfg.cfg" not in src, \ + "get_config() returns a dict; accessing _cfg.cfg drops reasoning_config to None" + assert "_cfg.get('agent', {})" in src or '_cfg.get("agent", {})' in src, \ + "streaming.py must read agent.reasoning_effort via the config dict" + + +def test_streaming_agent_cache_signature_includes_reasoning_config(cleanup_test_sessions): + """R17c: changing reasoning effort mid-session must rebuild the cached per-session agent. + + Without `_reasoning_config` participating in `_sig_blob`, the cache key + matches the old entry and the operator's `/reasoning xhigh` change has + no effect on the live session. + """ + src = (REPO_ROOT / "api/streaming.py").read_text() + start = src.find("_sig_blob = _json.dumps") + end = src.find("_agent_sig", start) + assert start >= 0 and end > start, "agent cache signature block not found" + sig_block = src[start:end] + assert "_reasoning_config" in sig_block, \ + "agent cache signature must include reasoning_config so xhigh/medium changes take effect" + + def test_messages_js_supports_live_reasoning_and_tool_completion(cleanup_test_sessions): """R18: messages.js must render live reasoning and react to tool completion events. Without these handlers, the operator only sees generic Thinking… or nothing