From d5ddc95e9fff27c9e9ed2bcc57205abb0521bd34 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Wed, 8 Jul 2026 00:25:07 -0700 Subject: [PATCH] fix(web_fetch): inherit model settings for the extraction completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The URL-extraction call hard-coded max_tokens=8192 and rode the "low" reasoning default, which broke local-inference models whose registry entry advertises a tighter output limit or a different reasoning config. Inherit the session/registry max_tokens and reasoning_effort instead (temperature already was) — the same knobs the main turn uses. max_tokens is capped to context_window // 4, the ~25% output slice Phase 2 already reserves, matching the main turn's response reserve (_remaining_token_budget), so a large operator budget can't push prompt + output past a small context window on strict runtimes. --- tests/test_session.py | 67 +++++++++++++++++++++++++++++++++++++++ turnstone/core/session.py | 18 ++++++++--- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/tests/test_session.py b/tests/test_session.py index d8236a2f..f47c0862 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -7033,6 +7033,73 @@ def test_utility_completion_defers_temperature_to_session(): assert kw2["temperature"] == 0.9 # explicit override still honored +def test_web_fetch_extraction_inherits_session_max_tokens_and_effort(): + """web_fetch's extraction call must inherit the session/registry max_tokens + and reasoning_effort rather than forcing constants. Hard-coding + max_tokens=8192 / reasoning_effort="low" broke local-inference models whose + registry entry advertises a tighter output limit or a reasoning config the + forced values fought — this lane now behaves like the main turn.""" + from unittest.mock import patch + + from turnstone.core.providers._protocol import CompletionResult + + session = _make_session(max_tokens=512, reasoning_effort="high") + + resp = MagicMock() + resp.raise_for_status.return_value = None + resp.headers = {"content-type": "text/plain"} + resp.text = "The page body that holds the answer." + + with ( + patch("turnstone.core.session.fetch_with_ssrf_guard", return_value=resp), + patch.object( + session, + "_utility_completion", + return_value=CompletionResult(content="Extracted answer."), + ) as uc, + ): + call_id, answer = session._exec_web_fetch({"call_id": "c1", "url": "https://example.com/"}) + + assert call_id == "c1" + assert answer == "Extracted answer." + _, kw = uc.call_args + # 512 < context_window // 4 (8192), so the tighter session value passes + # through unclamped — inheritance, not the old hard-coded 8192. + assert kw["max_tokens"] == 512 + assert kw["reasoning_effort"] == "high" # session value, not the old "low" + + +def test_web_fetch_extraction_caps_max_tokens_to_window_reserve(): + """The extraction request is capped to the ~25% window slice Phase 2 + reserves (``context_window // 4``), matching the main turn's response + reserve — so a large operator ``max_tokens`` on a small-context local + model can't push prompt + output past the window.""" + from unittest.mock import patch + + from turnstone.core.providers._protocol import CompletionResult + + # context_window=8192 -> reserve 2048; the session budget is far larger. + session = _make_session(max_tokens=16384, context_window=8192) + + resp = MagicMock() + resp.raise_for_status.return_value = None + resp.headers = {"content-type": "text/plain"} + resp.text = "The page body that holds the answer." + + with ( + patch("turnstone.core.session.fetch_with_ssrf_guard", return_value=resp), + patch.object( + session, + "_utility_completion", + return_value=CompletionResult(content="Extracted answer."), + ) as uc, + ): + session._exec_web_fetch({"call_id": "c1", "url": "https://example.com/"}) + + _, kw = uc.call_args + assert kw["max_tokens"] == 2048 # context_window // 4, not the 16384 session value + + def test_record_aux_usage_skips_when_usage_missing(): """A provider that reports no usage object must not emit a phantom zero-token row.""" diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 06bef381..2d36ece8 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -16195,10 +16195,17 @@ class ChatSession: text = text[:max_content] + f"\n\n... [{len(text) - max_content} chars truncated] ...\n" # Phase 3: summarization API call. - # Use a generous max_tokens so thinking models don't starve the - # visible answer, and pass reasoning_effort="low" to avoid wasting - # budget on deep reasoning for a simple extraction task. Temperature - # is left to the session/registry default rather than overridden here. + # Inherit the operator's per-model settings — reasoning_effort and + # (via the default) temperature from the session/registry — rather + # than forcing constants here: hard-coding reasoning_effort="low" and + # a fixed max_tokens kept breaking local-inference models whose + # registry entry advertises a different reasoning config or a tighter + # output limit. max_tokens is the session budget, but capped to the + # ~25% window slice Phase 2 reserved above — the same bound the main + # turn puts on its response reserve (``_remaining_token_budget``). + # That honors a tighter registry max_tokens while keeping prompt + + # output from overflowing a small context window on strict runtimes + # (the old fixed 8192 was exactly this reserve for the 32k default). try: result = self._utility_completion( [ @@ -16221,7 +16228,8 @@ class ChatSession: ), }, ], - max_tokens=8192, + max_tokens=min(self.max_tokens, self.context_window // 4), + reasoning_effort=self.reasoning_effort, ) answer = result.content or "" if not answer: