diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e1648b2..1c313673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,17 @@ Earlier stable lines (`stable/1.6`, `stable/1.5`) are frozen. ### Added +- **`server_parses_reasoning` model capability.** Declare it on a model + definition whose backend segregates reasoning into its own channel (a + vLLM launched with a reasoning parser, a commercial provider): the + inline think-tag scan turns off on every lane — interactive and + drained alike — so content is trusted verbatim and prose that merely + quotes a tag can no longer be misrouted into the reasoning lane, and + the utility lanes stop suppressing reasoning they'd otherwise pin off. + Default off for local lanes, preserving the passthrough-server + behavior; the built-in capability tables declare it for every real + commercial endpoint (known models and table-miss defaults alike), + which also removes the quoted-tag false positive from those lanes. - **Per-model Entra gateway authentication.** Model definitions can bind either a caller-delegated OBO token (`entra_obo`) or a shared app-identity token (`entra_app`) through the provider SDK credential surface. Mints reuse the @@ -229,7 +240,42 @@ Earlier stable lines (`stable/1.6`, `stable/1.5`) are frozen. sub-agents, optimizer, eval) — and web-fetch extraction deliberately never will, since it runs on parallel tool threads where registering one would clobber the main stream's. - +- **Unmarked chain-of-thought no longer leaks into titles, summaries, or + web-fetch tool results (#940).** Some serving setups emit reasoning + inline with no tags and no `reasoning_content` at all — nothing any + parser can segregate. The bounded-artifact lanes (title, compaction, + web-fetch extraction) now ask the model for no reasoning instead: + the model definition's declared thinking toggle is pinned off for that + call — the same suppression transcription already used — and the + reasoning-effort channels (the relayed session knob, the definition's + default, the graded template key) are withheld with it, since an + effort value beside a pinned-off toggle re-requests the reasoning the + pin declined. A no-op on backends that segregate reasoning + server-side. Title generation additionally stopped trusting line + position: it takes the last line that reads as a title (within the + word cap and ending in a word character, so explanation sentences, + sign-offs, and reasoning headings lose in any script) rather than the + first non-empty line, which unmarked reasoning turned into titles + like "Thinking Process:". +- **A think tag split across a reasoning delta now reassembles.** The + non-streaming drain closes content runs at interleaving signals; a + partial-tag tail is carried across reasoning-delta boundaries (a + reasoning delta cannot terminate a tag) so the tag is consumed instead + of its halves passing through as visible content. Tool-call boundaries + still flush — no tag spans a tool call. +- **Streaming consumers follow the ACTIVE model's capabilities.** The + interactive tag-scan posture and the drain's scan gate now read the + capabilities of the lane that owns the stream being consumed (fallback + walks included) instead of the session's primary alias. +- **Notification bodies no longer fuse multi-block answers.** `Turn.text` + joins text blocks with a newline; a final assistant turn stored as + multiple text blocks previously concatenated the last word of one + block to the first word of the next in completion notifications and + every other flattened read. +- **String-typed boolean capability overrides coerce instead of + truthiness-flipping.** A hand-edited `"false"`/`"0"` in a model + definition's capabilities JSON now means false; unrecognized values + drop the key and keep the field's default. - **Inline ``/`` blocks no longer leak into drained results (#965, #940).** On servers without a reasoning parser (parserless vLLM/llama.cpp, LM Studio, bare gateways), reasoning diff --git a/tests/_reasoning_dialect.py b/tests/_reasoning_dialect.py index 8711b1ed..47090ba1 100644 --- a/tests/_reasoning_dialect.py +++ b/tests/_reasoning_dialect.py @@ -159,6 +159,10 @@ CASES: tuple[DialectCase, ...] = ( # OPEN tag in legitimate prose misroutes the remainder — the same # false positive the interactive splitter has carried in the # field. This pin makes any future fix a conscious change. + # Scope note: R2 applies only where the scan runs — a backend + # declaring ``server_parses_reasoning`` turns the scan off and + # this utterance passes through byte-identical (pinned in + # test_scan_tags_off_returns_every_utterance_byte_identical). id="literal_open_tag_false_positive_r2", utterance="The `` tag opens a block.", content="The `", diff --git a/tests/test_drain_stream.py b/tests/test_drain_stream.py index 454f3d8a..95542bcd 100644 --- a/tests/test_drain_stream.py +++ b/tests/test_drain_stream.py @@ -652,6 +652,60 @@ def test_inter_run_paragraph_separator_survives_reasoning_boundary(): assert result.reasoning == "server-parsed\n\nplan" +def test_tag_split_across_reasoning_boundary_reassembles(): + """A reasoning delta cannot terminate a tag: a think tag the server + split across one must reassemble — a partial-tag TAIL is carried into + the next run (``partial_tag_tail``) instead of the halves passing + through as visible content.""" + result = drain_stream( + iter( + [ + StreamChunk(content_delta="Hello ` tag opens a block.\n" + assert result.reasoning == "server-parsed" + + def test_inter_run_separator_survives_tool_boundary(): result = drain_stream( iter( diff --git a/tests/test_judge.py b/tests/test_judge.py index 0a52a60d..890e97b5 100644 --- a/tests/test_judge.py +++ b/tests/test_judge.py @@ -12,6 +12,7 @@ from unittest.mock import MagicMock from tests._session_helpers import as_stream from tests._session_helpers import mock_completion_result as _mock_result from turnstone.core.judge import IntentJudge, IntentVerdict, JudgeConfig, evaluate_heuristic +from turnstone.core.providers._protocol import ModelCapabilities from turnstone.core.trajectory import Role # --------------------------------------------------------------------------- @@ -28,10 +29,13 @@ def _make_mock_provider( """Create a mock LLM provider that returns a fixed response.""" provider = MagicMock() provider.provider_name = "openai" - caps = MagicMock() - caps.context_window = 100_000 - caps.max_output_tokens = 4096 - provider.get_capabilities.return_value = caps + # A REAL ModelCapabilities, never a MagicMock: every attribute of a + # mock is truthy, so any boolean capability the code consults (the + # drain's ``server_parses_reasoning`` scan gate, and whatever field + # lands next) would silently flip behavior for the whole suite. + provider.get_capabilities.return_value = ModelCapabilities( + context_window=100_000, max_output_tokens=4096 + ) if side_effect: provider.create_streaming.side_effect = side_effect @@ -70,7 +74,9 @@ def _make_judge( session_provider=provider, session_client=client, session_model="test-model", - session_capabilities=MagicMock(context_window=100_000), + # Real caps for the same reason as in ``_make_mock_provider`` — + # the judge PREFERS session_capabilities over the provider's. + session_capabilities=ModelCapabilities(context_window=100_000), ) @@ -365,10 +371,9 @@ class TestMultiTurnToolUse: """Provider requests read_file, then returns verdict.""" provider = MagicMock() provider.provider_name = "openai" - caps = MagicMock() - caps.context_window = 100_000 - caps.max_output_tokens = 4096 - provider.get_capabilities.return_value = caps + provider.get_capabilities.return_value = ModelCapabilities( + context_window=100_000, max_output_tokens=4096 + ) provider.convert_tools.side_effect = lambda tools, **kw: tools # Turn 1: tool call @@ -405,10 +410,9 @@ class TestMultiTurnToolUse: """Provider keeps requesting tools — stops at _JUDGE_MAX_TURNS.""" provider = MagicMock() provider.provider_name = "openai" - caps = MagicMock() - caps.context_window = 100_000 - caps.max_output_tokens = 4096 - provider.get_capabilities.return_value = caps + provider.get_capabilities.return_value = ModelCapabilities( + context_window=100_000, max_output_tokens=4096 + ) provider.convert_tools.side_effect = lambda tools, **kw: tools # Every turn returns a tool call @@ -937,7 +941,7 @@ class TestModelAliasResolution: session_provider=_make_mock_provider(), session_client=MagicMock(base_url="https://s/v1", api_key="s"), session_model="session-model", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) # Merged at construction: overrides applied, untouched fields survive. @@ -975,7 +979,7 @@ class TestModelAliasResolution: session_provider=_make_mock_provider(), session_client=MagicMock(base_url="https://s/v1", api_key="s"), session_model="session-model", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) assert registry.get_config.call_count == 0 @@ -1086,7 +1090,7 @@ class TestModelAliasResolution: session_provider=_make_mock_provider(), session_client=MagicMock(base_url="http://s", api_key="s"), session_model="session-model", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) assert judge._judge_context_window == 100_000 # session window, not 0 @@ -1114,7 +1118,7 @@ class TestModelAliasResolution: session_provider=session_provider, session_client=session_client, session_model="session-default-model", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) @@ -1142,7 +1146,7 @@ class TestModelAliasResolution: session_provider=_make_mock_provider(), session_client=MagicMock(base_url="https://s/v1", api_key="s"), session_model="session-model", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) diff --git a/tests/test_model_turn.py b/tests/test_model_turn.py index a0bed2d3..b18a6d6e 100644 --- a/tests/test_model_turn.py +++ b/tests/test_model_turn.py @@ -866,3 +866,28 @@ def test_synth_bail_is_silent_and_leaks_nothing( ) assert blocks == [{"type": "thinking", "thinking": "native"}] assert secret_reasoning not in caplog.text + + +def test_capability_bool_overrides_coerced() -> None: + """The capabilities dict is hand-edited JSON: a string "false" is + truthy, and left raw it would flip every downstream truthiness read + (a ``server_parses_reasoning: "false"`` typo silently turning the + inline tag scan off is #940 reopened by punctuation). Recognized + spellings coerce, ints pass through ``bool()``, and an unrecognized + value drops the key so the field keeps its default.""" + from turnstone.core.model_turn import apply_capability_overrides + + base = ModelCapabilities() + off = apply_capability_overrides(base, {"server_parses_reasoning": "false"}) + assert off.server_parses_reasoning is False + on = apply_capability_overrides(base, {"server_parses_reasoning": "true"}) + assert on.server_parses_reasoning is True + coerced = apply_capability_overrides(base, {"supports_vision": 1, "supports_tools": 0}) + assert coerced.supports_vision is True + assert coerced.supports_tools is False + # Unrecognized string: key dropped, default kept; non-bool fields untouched. + kept = apply_capability_overrides( + base, {"server_parses_reasoning": "maybe", "thinking_mode": "manual"} + ) + assert kept.server_parses_reasoning is False + assert kept.thinking_mode == "manual" diff --git a/tests/test_notify_completion.py b/tests/test_notify_completion.py index 5a8eeb06..84aa72c3 100644 --- a/tests/test_notify_completion.py +++ b/tests/test_notify_completion.py @@ -29,10 +29,9 @@ from turnstone.console.server import ( ) from turnstone.core.auth import AuthResult from turnstone.core.storage._sqlite import SQLiteBackend -from turnstone.core.trajectory import turns_from_dicts +from turnstone.core.trajectory import final_assistant_text, turns_from_dicts from turnstone.server import ( _deliver_notification, - _extract_last_assistant_content, _fire_notify_targets, _validate_notify_targets, ) @@ -208,22 +207,25 @@ class TestValidateNotifyTargets: # --------------------------------------------------------------------------- -class TestExtractLastAssistantContent: +class TestNotifyFinalSayRead: + """The notify hook reads ``trajectory.final_assistant_text`` directly — + these pin the read's semantics over the notify path's turn shapes.""" + def test_string_content(self): - session = MagicMock() - session.messages = turns_from_dicts( + turns = turns_from_dicts( [ {"role": "user", "content": "hello"}, {"role": "assistant", "content": "world"}, ] ) - assert _extract_last_assistant_content(session) == "world" + assert final_assistant_text(turns) == "world" def test_structured_content(self): # Multi-block text flattens via the canonical Turn.text projection - # (the shared final-say read), not a notify-private join. - session = MagicMock() - session.messages = turns_from_dicts( + # (the shared final-say read), not a notify-private join — with a + # newline separator, so the delivered notification never fuses + # the last word of one block to the first word of the next. + turns = turns_from_dicts( [ { "role": "assistant", @@ -234,39 +236,33 @@ class TestExtractLastAssistantContent: }, ] ) - assert _extract_last_assistant_content(session) == "part onepart two" + assert final_assistant_text(turns) == "part one\npart two" def test_whitespace_only_final_say_reports_empty(self): # A whitespace-only final say is empty — the notify fallback fires # instead of sending raw whitespace. - session = MagicMock() - session.messages = turns_from_dicts([{"role": "assistant", "content": " \n"}]) - assert _extract_last_assistant_content(session) == "" + turns = turns_from_dicts([{"role": "assistant", "content": " \n"}]) + assert final_assistant_text(turns) == "" def test_empty_messages(self): - session = MagicMock() - session.messages = [] - assert _extract_last_assistant_content(session) == "" + assert final_assistant_text([]) == "" def test_no_assistant_messages(self): - session = MagicMock() - session.messages = turns_from_dicts([{"role": "user", "content": "hello"}]) - assert _extract_last_assistant_content(session) == "" + turns = turns_from_dicts([{"role": "user", "content": "hello"}]) + assert final_assistant_text(turns) == "" def test_picks_last_assistant(self): - session = MagicMock() - session.messages = turns_from_dicts( + turns = turns_from_dicts( [ {"role": "assistant", "content": "first"}, {"role": "user", "content": "question"}, {"role": "assistant", "content": "second"}, ] ) - assert _extract_last_assistant_content(session) == "second" + assert final_assistant_text(turns) == "second" def test_skips_non_text_blocks(self): - session = MagicMock() - session.messages = turns_from_dicts( + turns = turns_from_dicts( [ { "role": "assistant", @@ -277,7 +273,7 @@ class TestExtractLastAssistantContent: }, ] ) - assert _extract_last_assistant_content(session) == "result" + assert final_assistant_text(turns) == "result" # --------------------------------------------------------------------------- diff --git a/tests/test_output_guard_judge.py b/tests/test_output_guard_judge.py index 8c08c6a1..ef668a35 100644 --- a/tests/test_output_guard_judge.py +++ b/tests/test_output_guard_judge.py @@ -26,10 +26,12 @@ def _make_provider( """Build a mock LLMProvider whose create_streaming returns the given content.""" provider = MagicMock() provider.provider_name = "openai" - # The judge reads context_window at construction for its oversize guard. - caps = MagicMock() - caps.context_window = 200_000 - provider.get_capabilities = MagicMock(return_value=caps) + # The judge reads context_window at construction for its oversize + # guard. A REAL ModelCapabilities, never a MagicMock: every mock + # attribute is truthy, so any boolean capability the code consults + # (the drain's ``server_parses_reasoning`` scan gate, and whatever + # field lands next) would silently flip behavior for the suite. + provider.get_capabilities = MagicMock(return_value=ModelCapabilities(context_window=200_000)) def _create_streaming(**_kwargs: Any) -> Any: if delay: @@ -130,7 +132,7 @@ class TestCapabilityThreading: session_provider=_make_provider(), session_client=client, session_model="m", - session_capabilities=MagicMock(context_window=100_000), + session_capabilities=ModelCapabilities(context_window=100_000), model_registry=registry, ) judge._create_client = lambda: client # type: ignore[method-assign] @@ -358,7 +360,9 @@ class TestOversizeGuard: local model and would leave the guard blind to overflow.""" provider = _make_provider(content='{"risk_level": "none", "flags": []}') # provider caps report the fictitious 200k; the guard must ignore it. - provider.get_capabilities = MagicMock(return_value=MagicMock(context_window=200_000)) + provider.get_capabilities = MagicMock( + return_value=ModelCapabilities(context_window=200_000) + ) judge = OutputGuardJudge( config=JudgeConfig(output_guard_llm=True), # no output_guard_model session_provider=provider, @@ -366,7 +370,7 @@ class TestOversizeGuard: session_model="test-model", # The session's real window rides in the resolved caps the caller # passes; the guard must key off it, not provider.get_capabilities(). - session_capabilities=MagicMock(context_window=40_000), + session_capabilities=ModelCapabilities(context_window=40_000), ) assert judge._judge_context_window == 40_000 @@ -393,7 +397,7 @@ class TestOversizeGuard: session_client=MagicMock(base_url="http://s", api_key="s"), session_model="m", model_registry=registry, - session_capabilities=MagicMock(context_window=64_000), + session_capabilities=ModelCapabilities(context_window=64_000), ) assert alias_judge._judge_context_window == 64_000 diff --git a/tests/test_provider_anthropic_compat.py b/tests/test_provider_anthropic_compat.py index 9146b758..6a690f68 100644 --- a/tests/test_provider_anthropic_compat.py +++ b/tests/test_provider_anthropic_compat.py @@ -313,6 +313,19 @@ class TestCompatReasoningControl: "foo": 1, } + def test_utility_pin_survives_adaptive_injection(self) -> None: + """The exact pair ``lane_without_thinking`` relies on: an adaptive + model's injection always sends ``true``, but the utility lanes' + pinned ``false`` is already present in extra_params and existing + keys win — the pin reaches the wire.""" + caps = dataclasses.replace(self._MANUAL_CAPS, thinking_mode="adaptive") + kwargs = self._stream_kwargs( + caps, + "high", + extra_params={"chat_template_kwargs": {"enable_thinking": False}}, + ) + assert kwargs["extra_body"]["chat_template_kwargs"]["enable_thinking"] is False + def test_caller_extra_params_not_mutated(self) -> None: """The session's extra_params dict must never be written through.""" extra = {"chat_template_kwargs": {"foo": 1}} diff --git a/tests/test_providers.py b/tests/test_providers.py index b6c604e5..330298e5 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -6267,3 +6267,31 @@ def test_sanitize_keeps_empty_content_assistant_turn_with_tool_calls(): assert out[1]["content"] == "" assert out[1]["tool_calls"][0]["id"] == "c1" assert out[2]["tool_call_id"] == "c1" + + +def test_commercial_lanes_declare_server_parses_reasoning(): + """Finding-of-record for the scan-off capability: every REAL commercial + lane segregates reasoning natively (thinking blocks / reasoning items / + ``reasoning_content``), so its static caps declare the flag — known + models and table-miss defaults alike — while the local compat lanes + keep the passthrough default the tag scan exists for.""" + from turnstone.core.providers import create_provider + + for name, model in [ + ("anthropic", "claude-opus-5"), + ("anthropic", "claude-unknown-future"), + ("openai", "gpt-5.4"), + ("openai", "some-unknown-model"), + ("google", "gemini-3-pro"), + ("xai", "grok-4"), + ("xai", "grok-unknown"), + ]: + caps = create_provider(name).get_capabilities(model) + assert caps.server_parses_reasoning is True, (name, model) + for name, model in [ + ("anthropic-compatible", "qwen3.6-27b"), + ("openai-compatible", "qwen3.6-27b"), + ("openai-compatible", "gpt-5.4-my-finetune"), + ]: + caps = create_provider(name).get_capabilities(model) + assert caps.server_parses_reasoning is False, (name, model) diff --git a/tests/test_session.py b/tests/test_session.py index 5eddd41f..2085a9a1 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -1859,16 +1859,17 @@ class TestTitleRetry: def test_title_strips_reasoning_variants(self, tmp_db): """Reasoning reaches ``content`` in several shapes the title pass must survive: an opener-absent ``…`` (templates that pre-inject the - opening tag), a paired ```` block, and a trailing - explanation after the title (only the first non-empty line is kept). + opening tag), a paired ```` block, trailing prose after the + title (an explanation sentence, a short sign-off, a parenthetical — + each rejected by the word cap or the ends-alphanumeric check, so the + end-first scan still lands on the title), an over-cap padded answer + (kept via the last-line fallback rather than replaced by a + reasoning fragment from higher up), and a CJK title whose trailing + explanation whitespace-counts as one word but ends in terminal + punctuation. - The last two cases pin the BOTH-VOCABULARY shape in either order. - The peel walks the close-tag vocabularies in sequence, which is - equivalent to one cut after whichever close occurs last: the - remainder of the first cut begins after the last ````, so a - ```` still found in it is necessarily the later tag. - Title text after the last stray close always survives; only - reasoning between the tags is dropped.""" + Two cases pin the BOTH-VOCABULARY peel shape in either order — the + cut lands after whichever close tag occurs LAST.""" from turnstone.core.providers._protocol import ModelCapabilities cases = [ @@ -1878,6 +1879,13 @@ class TestTitleRetry: "Cluster Health Digest", ), ("Auth Layer Refactor\n\nThis title captures the request well.", "Auth Layer Refactor"), + ("Fix Login Bug\n\nHope this helps!", "Fix Login Bug"), + ("Alembic Migration Fix\n\n(3 words)", "Alembic Migration Fix"), + ( + "Hmm, let me reconsider.\n\nAlembic Async Migration Failure Debugging Session", + "Alembic Async Migration Failure Debugging Session", + ), + ("数据库迁移问题\n\n这个标题很好地概括了用户的请求。", "数据库迁移问题"), ( "weighingstill weighing\n\nRendezvous Routing", "Rendezvous Routing", @@ -1905,9 +1913,77 @@ class TestTitleRetry: session._generate_title() assert captured.get("title") == expected, (content, captured) + def test_title_from_unmarked_reasoning_takes_the_answer(self, tmp_db): + """A server can leave reasoning inline and entirely UNMARKED — no open + tag, no close tag, no ``reasoning_content`` — so there is nothing for + the seam to segregate and nothing for the lane to peel. Measured on + the dev vLLM (qwen3.6-27b, 20 sampled responses): the chain-of-thought + opens with a ``Thinking Process:`` heading, which BECAME the title. + + The answer is last and honors the prompt's word cap; the reasoning + lines around it do not.""" + from turnstone.core.providers._protocol import ModelCapabilities + + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = mock_completion_result() + # Condensed from a captured qwen3.6-27b streamed response. + result.content = ( + "Thinking Process:\n" + "1. **Analyze the Request:** The user wants a title of at most 3 words.\n" + "2. **Brainstorm:** Alembic Migration Failure, Migration Debugging.\n" + "6. **Final Output Generation:** Alembic Migration Fix\n" + "\n\n" + "Alembic Migration Fix" + ) + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_streaming.return_value = as_stream(result) + + captured: dict[str, str] = {} + with patch( + "turnstone.core.session.update_workstream_title", + side_effect=lambda ws_id, title: captured.update(title=title), + ): + session._generate_title() + + assert captured["title"] == "Alembic Migration Fix" + + def test_title_peel_off_when_backend_segregates(self, tmp_db): + """On a backend that segregates reasoning (``server_parses_reasoning``) + a close tag in content IS quoted prose — the title lane's cosmetic + peel is off there, like the seam's scan, so a title that mentions + the tag survives intact.""" + from turnstone.core.providers._protocol import ModelCapabilities + + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = mock_completion_result() + result.content = "Fixing Leak" + session._provider = MagicMock() + session._provider.provider_name = "openai-compatible" + session._provider.get_capabilities.return_value = ModelCapabilities( + server_parses_reasoning=True + ) + session._provider.create_streaming.return_value = as_stream(result) + + captured: dict[str, str] = {} + with patch( + "turnstone.core.session.update_workstream_title", + side_effect=lambda ws_id, title: captured.update(title=title), + ): + session._generate_title() + + assert captured["title"] == "Fixing Leak" + def test_title_truncates_to_max_chars(self, tmp_db): """The ``[:_TITLE_MAX_CHARS]`` slice is the only length guard now that - the persist-time ``title[:80]`` is gone — a long title is bounded.""" + the persist-time ``title[:80]`` is gone — a long title is bounded. + + No line here honors the word cap, so the scan falls back to the last + non-empty line rather than yielding nothing.""" from turnstone.core.providers._protocol import ModelCapabilities from turnstone.core.session import _TITLE_MAX_CHARS @@ -7518,6 +7594,104 @@ def test_utility_completion_defers_temperature_to_session(): assert kw2["temperature"] == 0.9 # explicit override still honored +def test_utility_completion_asks_a_passthrough_backend_for_no_reasoning(): + """#940: a server that does not segregate reasoning leaves it in + ``content``, and when it arrives UNMARKED the seam cannot lift it out — + the chain-of-thought becomes the artifact (the web-fetch tool result, + then every following turn's context). The bounded-artifact lanes + therefore ask for none through EVERY channel: the alias's OWN declared + toggle pinned off (over any operator ``server_compat`` flag, surviving + the provider's adaptive-``true`` injection), the model definition's + default-effort rung cleared, and the caller's relayed effort knob + zeroed — an effort value beside a pinned-off toggle re-requests the + reasoning the pin declined.""" + from turnstone.core.providers._protocol import CompletionResult, ModelCapabilities + + session = _make_session() + session._provider = MagicMock() + session._provider.provider_name = "openai-compatible" + session._provider.get_capabilities.return_value = ModelCapabilities( + thinking_mode="adaptive", + thinking_param="enable_thinking", + default_reasoning_effort="high", + ) + session._provider.create_streaming.return_value = as_stream(CompletionResult(content="x")) + + # The web-fetch relay shape: an explicit caller effort rides in. + session._utility_completion([Turn.user("hi")], reasoning_effort="high") + _, kw = session._provider.create_streaming.call_args + assert kw["extra_params"]["chat_template_kwargs"] == {"enable_thinking": False} + # Neither the caller rung nor the definition's default survives. + assert kw["reasoning_effort"] is None + + +def test_utility_completion_suppresses_effort_on_toggle_less_passthrough(): + """A passthrough box with NO template toggle (thinking_mode="none", + effort-passthrough) has no off switch — but the effort channel alone is + a reasoning request, so the utility lanes omit it entirely rather than + asking a non-segregating box for more chain-of-thought.""" + from turnstone.core.providers._protocol import CompletionResult, ModelCapabilities + + session = _make_session() + session._provider = MagicMock() + session._provider.provider_name = "openai-compatible" + session._provider.get_capabilities.return_value = ModelCapabilities( + thinking_mode="none", + effort_passthrough=True, + default_reasoning_effort="high", + ) + session._provider.create_streaming.return_value = as_stream(CompletionResult(content="x")) + + session._utility_completion([Turn.user("hi")], reasoning_effort="high") + _, kw = session._provider.create_streaming.call_args + assert kw["reasoning_effort"] is None + # No toggle declared → no guessed key. + assert (kw["extra_params"] or {}).get("chat_template_kwargs") is None + + +def test_utility_completion_keeps_reasoning_when_the_backend_segregates_it(): + """The pin is remediation for a lane that cannot separate reasoning from + the artifact. A backend that puts reasoning in its own channel has no + such problem, so nothing is suppressed — reasoning there costs the + artifact nothing, and silencing a model the operator chose for its + reasoning would be the harness overriding them for no gain.""" + from turnstone.core.providers._protocol import CompletionResult, ModelCapabilities + + session = _make_session() + session._provider = MagicMock() + session._provider.provider_name = "openai-compatible" + session._provider.get_capabilities.return_value = ModelCapabilities( + thinking_mode="adaptive", + thinking_param="enable_thinking", + server_parses_reasoning=True, + ) + session._provider.create_streaming.return_value = as_stream(CompletionResult(content="x")) + + session._utility_completion([Turn.user("hi")], reasoning_effort="high") + _, kw = session._provider.create_streaming.call_args + assert (kw["extra_params"] or {}).get("chat_template_kwargs") is None + # The relayed effort knob stands — the operator chose a reasoning + # model whose reasoning costs the artifact nothing. + assert kw["reasoning_effort"] == "high" + + +def test_utility_completion_never_guesses_a_toggle_key(): + """A model that declares no thinking toggle keeps its template default: + the pin sends the alias's declared key or nothing at all. Inventing one + would flip a lever the operator never wired.""" + from turnstone.core.providers._protocol import CompletionResult, ModelCapabilities + + session = _make_session() + session._provider = MagicMock() + session._provider.provider_name = "openai-compatible" + session._provider.get_capabilities.return_value = ModelCapabilities(thinking_mode="none") + session._provider.create_streaming.return_value = as_stream(CompletionResult(content="x")) + + session._utility_completion([Turn.user("hi")]) + _, kw = session._provider.create_streaming.call_args + assert (kw["extra_params"] or {}).get("chat_template_kwargs") is None + + 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 diff --git a/tests/test_think_tag_split.py b/tests/test_think_tag_split.py index 27f8fbb0..a5bf53ca 100644 --- a/tests/test_think_tag_split.py +++ b/tests/test_think_tag_split.py @@ -199,6 +199,53 @@ def test_one_shot_passthrough_byte_identity(case): assert content == case.utterance +@pytest.mark.parametrize("case", DIALECT_CASES, ids=[c.id for c in DIALECT_CASES]) +def test_scan_tags_off_returns_every_utterance_byte_identical(case): + """``server_parses_reasoning`` backends put reasoning in their own + channel, so content carries none — the scan is turned OFF and EVERY + catalog utterance passes through untouched, including the ones the + scan would otherwise consume. This is what buys back residual R2: + prose that merely QUOTES a tag can no longer be misrouted.""" + content, reasoning = split_inline_reasoning(case.utterance, scan_tags=False) + assert content == case.utterance + assert reasoning == "" + + +def test_session_consumer_scan_follows_server_parses_reasoning(): + """The interactive consumer wires ``scan_tags`` from the SAME capability + the drain seam reads (``server_parses_reasoning``), so the two lanes + cannot disagree. With the flag declared, streamed tag text reaches the + UI verbatim as content — it is prose on such a backend, not a + boundary.""" + from turnstone.core.providers._protocol import ModelCapabilities + + session = make_session() + session._cached_capabilities = ModelCapabilities(server_parses_reasoning=True) + ui = _TokenRecorderUI() + session.ui = ui + msg = session._stream_attempt(iter([_c("quotedanswer"), _FINISH])) + assert msg["content"] == "quotedanswer" + assert all(kind == "content" for kind, _ in ui.tokens) + + +def test_scan_tags_off_holds_no_carry_and_honors_out_of_band_state(): + """With the scan off there is nothing to resolve, so nothing is held: + every span emits immediately at the current state. The state machine + stays live — the consumer still writes ``in_think`` for the + provider-parsed reasoning transitions, which is the whole point on a + backend that segregates.""" + events = [] + splitter = ThinkTagSplitter( + lambda text, is_reasoning: events.append((text, is_reasoning)), scan_tags=False + ) + splitter.feed("abc") + assert events == [("abc", False)] + assert splitter.pending == "" + splitter.in_think = True + splitter.feed("still content-lane text") + assert events[-1] == ("still content-lane text", True) + + @pytest.mark.parametrize("case", DIALECT_CASES, ids=[c.id for c in DIALECT_CASES]) def test_one_shot_equivalent_to_streaming_over_random_chunkings(case): """One-shot ≡ the streaming class fed the same utterance in arbitrary diff --git a/tests/test_trajectory_model.py b/tests/test_trajectory_model.py index 31309e4f..8b7f8f46 100644 --- a/tests/test_trajectory_model.py +++ b/tests/test_trajectory_model.py @@ -24,8 +24,11 @@ def test_text_joins_only_textblocks() -> None: Role.USER, (TextBlock("look at "), AttachmentRef("sha-1", "image"), TextBlock("this")), ) - # Attachment blocks contribute nothing to the FTS/text projection. - assert turn.text == "look at this" + # Attachment blocks contribute nothing to the FTS/text projection, and + # adjacent text blocks join with a NEWLINE — they are distinct spans, + # and a bare concatenation fused words across block boundaries in + # every flattened read (notification bodies, final_assistant_text). + assert turn.text == "look at \nthis" def test_user_helper() -> None: diff --git a/turnstone/core/audio.py b/turnstone/core/audio.py index d96c4508..7c10a85a 100644 --- a/turnstone/core/audio.py +++ b/turnstone/core/audio.py @@ -21,6 +21,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any from turnstone.core.log import get_logger +from turnstone.core.providers import thinking_off_template_kwargs from turnstone.core.server_compat import merge_server_compat if TYPE_CHECKING: @@ -260,17 +261,21 @@ def _omni_chat_extra_body(cfg: Any) -> dict[str, Any]: The STT path calls the raw client, so it bypasses the provider's request shaping. Reuse ``merge_server_compat`` to forward any operator-stored - ``server_compat["extra_body"]``, then force **thinking OFF** via the model's - own ``thinking_param``: transcription needs no reasoning, and leaving it on - multiplies latency ~10x and (on some chat templates) empties the content. - The override is applied last so it wins over any operator thinking flag. + ``server_compat["extra_body"]``, then force **thinking OFF** via + :func:`thinking_off_template_kwargs` — THE shared spelling of "this lane + needs no reasoning", also used by the drained utility completions: + transcription needs no reasoning, and leaving it on multiplies latency + ~10x and (on some chat templates) empties the content. The override is + applied last so it wins over any operator thinking flag. """ server_compat = getattr(cfg, "server_compat", None) extra = merge_server_compat(None, server_compat) if isinstance(server_compat, dict) else {} caps = getattr(cfg, "capabilities", None) or {} - thinking_param = caps.get("thinking_param") - if thinking_param and caps.get("thinking_mode") in ("manual", "adaptive"): - extra.setdefault("chat_template_kwargs", {})[thinking_param] = False + off = thinking_off_template_kwargs( + str(caps.get("thinking_mode") or ""), str(caps.get("thinking_param") or "") + ) + if off: + extra.setdefault("chat_template_kwargs", {}).update(off) return extra diff --git a/turnstone/core/model_turn.py b/turnstone/core/model_turn.py index a24993b7..d8d44055 100644 --- a/turnstone/core/model_turn.py +++ b/turnstone/core/model_turn.py @@ -63,6 +63,7 @@ from turnstone.core.lowering import ( from turnstone.core.providers._protocol import ( drain_stream, has_reasoning_bearing_block, + thinking_off_template_kwargs, ) from turnstone.core.storage._utils import ( _CLIENT_TOOL_CALL_BLOCK_TYPES, @@ -146,6 +147,19 @@ def resolve_capabilities( return caps +_CAPABILITY_BOOL_STRINGS = { + "true": True, + "yes": True, + "on": True, + "1": True, + "false": False, + "no": False, + "off": False, + "0": False, + "": False, +} + + def apply_capability_overrides(caps: ModelCapabilities, overrides_raw: Any) -> ModelCapabilities: """Field-filtered merge of an operator ``capabilities`` dict onto *caps*. @@ -154,15 +168,48 @@ def apply_capability_overrides(caps: ModelCapabilities, overrides_raw: Any) -> M overrides dict use this directly instead of faking a ModelConfig. Unknown keys are ignored (the registry accepts free-form dicts) and a non-dict value degrades to "no overrides". + + Values landing on BOOL-defaulted fields are coerced: the capabilities + dict is hand-edited JSON, and a string ``"false"`` is truthy — left + raw it would silently FLIP every downstream truthiness read (a + ``server_parses_reasoning: "false"`` typo turning the inline tag scan + off is the #940 leak reopened by punctuation). Recognized spellings + map to their boolean, ints pass through ``bool()`` (0/1 rows), and an + unrecognized value drops the key — the field keeps its default, the + same degrade-not-crash posture as the non-dict case. """ if isinstance(overrides_raw, dict) and overrides_raw: - names = {f.name for f in fields(type(caps))} - overrides = {k: v for k, v in overrides_raw.items() if k in names} + by_name = {f.name: f for f in fields(type(caps))} + overrides: dict[str, Any] = {} + for key, value in overrides_raw.items(): + fld = by_name.get(key) + if fld is None: + continue + # bool check FIRST — bool is an int subclass, so the int arm + # below would otherwise claim real booleans. + if isinstance(fld.default, bool) and not isinstance(value, bool): + if isinstance(value, int): + value = bool(value) + elif isinstance(value, str) and value.strip().lower() in _CAPABILITY_BOOL_STRINGS: + value = _CAPABILITY_BOOL_STRINGS[value.strip().lower()] + else: + continue + overrides[key] = value if overrides: caps = replace(caps, **overrides) return caps +# Providers whose request shape carries an ``extra_body`` dict, so +# operator ``server_compat`` pins and template-kwarg reasoning levers +# reach the wire through it. Real Anthropic and Google keep their own +# param paths inside their providers and must never be handed one. THE +# membership test — shared by :func:`provider_extra_params` and the +# callers that layer their own pins onto a resolved lane, so the two +# cannot disagree about which lanes accept extra_body at all. +EXTRA_BODY_PROVIDERS: tuple[str, ...] = ("openai", "openai-compatible", "anthropic-compatible") + + def provider_extra_params( provider: LLMProvider, registry: ModelRegistry | None, @@ -180,7 +227,7 @@ def provider_extra_params( """ from turnstone.core.server_compat import merge_server_compat - if provider.provider_name not in ("openai", "openai-compatible", "anthropic-compatible"): + if provider.provider_name not in EXTRA_BODY_PROVIDERS: return None if cfg is ...: cfg = _get_config_or_none(registry, alias) @@ -385,6 +432,82 @@ class ModelLane: backend_auth_resolver: Callable[[str], str | None] | None = None +def lane_thinking_suppressed(lane: ModelLane) -> bool: + """True when *lane* gets the bounded-artifact no-reasoning posture. + + THE gate for :func:`lane_without_thinking` — exposed so a caller that + relays its own effort knob (``_utility_completion``'s web-fetch + relay) can zero the caller rung under exactly the same condition the + lane rungs are zeroed under, instead of re-deriving it. False when + the backend segregates reasoning (``server_parses_reasoning`` — + reasoning then costs the artifact nothing and the operator's knobs + stand), when the lane carries no resolved capabilities, or on + providers that take no ``extra_body`` (real Anthropic/Google shape + their own thinking params). + """ + caps = lane.capabilities + return ( + caps is not None + and not caps.server_parses_reasoning + and lane.provider.provider_name in EXTRA_BODY_PROVIDERS + ) + + +def lane_without_thinking(lane: ModelLane) -> ModelLane: + """*lane* with every reasoning request it owns turned OFF. + + For the bounded-artifact lanes (the session's ``_utility_completion``: + title, compaction, web-fetch extraction): a server that does not + segregate reasoning leaves it in ``content``, and when it arrives + UNMARKED — no tags, no ``reasoning_content`` — the drain seam cannot + lift it out, so it lands in the artifact (#940). Asking for no + reasoning is the only lever that survives that, so when + :func:`lane_thinking_suppressed` holds, EVERY channel this lane + controls goes silent: + + - the declared template toggle is pinned ``False`` + (:func:`thinking_off_template_kwargs` — the alias's OWN key, never + a guessed one), layered into the already-resolved ``extra_params`` + (no second config fetch) over any operator ``server_compat`` + thinking flag — that flag speaks for the answering lane, and these + calls are not it. The provider's own injection only + ``setdefault``s, so the pin also beats the ``adaptive`` branch's + unconditional ``true``; + - the lane's operator effort rung and the model definition's + ``default_reasoning_effort`` rung are cleared, so ``model_turn`` + resolves NO effective effort and neither the flat + ``reasoning_effort`` param (effort-passthrough boxes) nor the + graded template ``effort_param`` is emitted — an effort value + beside a pinned-off toggle re-requests the reasoning the pin just + declined, and on toggle-less templates the effort key alone is a + reasoning request. Where a box reasons unconditionally (no toggle, + no effort semantics), omitting the knobs at least never asks for + MORE — the remediation there is server-side + (``server_parses_reasoning`` once a parser is configured). + + Callers relaying a caller-rung effort must zero it under the same + predicate — see :func:`lane_thinking_suppressed`. + """ + if not lane_thinking_suppressed(lane): + return lane + caps = lane.capabilities + assert caps is not None # lane_thinking_suppressed guarantees it + extra = lane.extra_params + off = thinking_off_template_kwargs(caps.thinking_mode, caps.thinking_param) + if off: + extra = dict(lane.extra_params or {}) + raw_ctk = extra.get("chat_template_kwargs") + ctk = dict(raw_ctk) if isinstance(raw_ctk, dict) else {} + ctk.update(off) + extra["chat_template_kwargs"] = ctk + return replace( + lane, + extra_params=extra, + reasoning_effort=None, + capabilities=replace(caps, default_reasoning_effort=""), + ) + + def resolve_lane( provider: LLMProvider, client: Any, @@ -844,7 +967,14 @@ def model_turn( resolve_attachments=resolve_attachments, ) try: - result = drain_stream(chunks) + result = drain_stream( + chunks, + # A lane with no declared capabilities keeps the + # passthrough-server default: scan. + scan_inline_reasoning=not ( + lane.capabilities.server_parses_reasoning if lane.capabilities else False + ), + ) break except Exception as exc: attempt += 1 diff --git a/turnstone/core/providers/__init__.py b/turnstone/core/providers/__init__.py index d4ff36bb..d3d8bf8e 100644 --- a/turnstone/core/providers/__init__.py +++ b/turnstone/core/providers/__init__.py @@ -19,6 +19,7 @@ from turnstone.core.providers._protocol import ( accumulate_tool_call_delta, drain_stream, merge_usage, + thinking_off_template_kwargs, transport_guarded, ) from turnstone.core.providers._xai import XAI_DEFAULT_BASE_URL, XAIProvider @@ -42,6 +43,7 @@ __all__ = [ "list_known_models", "lookup_model_capabilities", "merge_usage", + "thinking_off_template_kwargs", "transport_guarded", ] diff --git a/turnstone/core/providers/_anthropic.py b/turnstone/core/providers/_anthropic.py index 70c1f8d4..ff1a3b9d 100644 --- a/turnstone/core/providers/_anthropic.py +++ b/turnstone/core/providers/_anthropic.py @@ -9,6 +9,7 @@ from __future__ import annotations import json import logging import sys +from dataclasses import replace from typing import TYPE_CHECKING, Any from turnstone.core.attachments import safe_attachment_label @@ -303,6 +304,19 @@ _ANTHROPIC_CAPABILITIES: dict[str, ModelCapabilities] = { ), } +# The commercial endpoint segregates reasoning natively (``thinking`` / +# ``redacted_thinking`` blocks) — content never carries inline think +# tags, so the inline tag scan is off for every entry, known or +# defaulted, as ONE rule applied to the whole table (a per-entry flag +# would be forgotten on the next model row). ``_ANTHROPIC_COMPAT_DEFAULT`` +# is deliberately NOT covered: local /v1/messages checkpoints are exactly +# the passthrough dialect the scan exists for. +_ANTHROPIC_DEFAULT = replace(_ANTHROPIC_DEFAULT, server_parses_reasoning=True) +_ANTHROPIC_CAPABILITIES = { + name: replace(caps, server_parses_reasoning=True) + for name, caps in _ANTHROPIC_CAPABILITIES.items() +} + def _map_reasoning_to_effort( reasoning_effort: str | None, diff --git a/turnstone/core/providers/_google.py b/turnstone/core/providers/_google.py index 519c9cf3..576b5a7d 100644 --- a/turnstone/core/providers/_google.py +++ b/turnstone/core/providers/_google.py @@ -46,6 +46,10 @@ _GOOGLE_DEFAULT = ModelCapabilities( max_output_tokens=65_536, supports_temperature=True, supports_vision=True, + # The commercial endpoint segregates reasoning natively (thought + # parts / ``reasoning_content`` on the compat surface) — content + # never carries inline think tags, so the inline tag scan is off. + server_parses_reasoning=True, # Gemini's OpenAI-compat endpoint accepts max_tokens (not # max_completion_tokens which is OpenAI Responses-specific). token_param="max_tokens", diff --git a/turnstone/core/providers/_openai_common.py b/turnstone/core/providers/_openai_common.py index 1c1a112a..29cce582 100644 --- a/turnstone/core/providers/_openai_common.py +++ b/turnstone/core/providers/_openai_common.py @@ -8,6 +8,7 @@ formatting, and message sanitisation live here so both from __future__ import annotations import uuid +from dataclasses import replace from typing import Any import structlog @@ -170,8 +171,19 @@ OPENAI_CAPABILITIES: dict[str, ModelCapabilities] = { ), } +# The commercial endpoint segregates reasoning natively (Responses +# reasoning items / ``reasoning_content``) — content never carries inline +# think tags, so the inline tag scan is off for every entry, known or +# defaulted, as ONE rule applied to the whole table (a per-entry flag +# would be forgotten on the next model row). The compat default below is +# deliberately NOT covered: local checkpoints are exactly the +# passthrough dialect the scan exists for. +OPENAI_CAPABILITIES = { + name: replace(caps, server_parses_reasoning=True) for name, caps in OPENAI_CAPABILITIES.items() +} + # Default for unknown models on the commercial lane. -OPENAI_DEFAULT = ModelCapabilities() +OPENAI_DEFAULT = ModelCapabilities(server_parses_reasoning=True) # The ``openai-compatible`` lane (either API surface) never consults the # commercial table above: a local server serves whatever the operator diff --git a/turnstone/core/providers/_protocol.py b/turnstone/core/providers/_protocol.py index 3e3ea84e..2ef563d7 100644 --- a/turnstone/core/providers/_protocol.py +++ b/turnstone/core/providers/_protocol.py @@ -10,7 +10,11 @@ from __future__ import annotations from dataclasses import dataclass, field, replace from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable -from turnstone.core.streaming_text import split_inline_reasoning, strip_blank_edge_lines +from turnstone.core.streaming_text import ( + partial_tag_tail, + split_inline_reasoning, + strip_blank_edge_lines, +) if TYPE_CHECKING: from collections.abc import Callable, Iterator @@ -247,9 +251,16 @@ def transport_guarded(chunks: Iterator[StreamChunk]) -> Iterator[StreamChunk]: yield sc -def drain_stream(chunks: Iterator[StreamChunk]) -> CompletionResult: +def drain_stream( + chunks: Iterator[StreamChunk], *, scan_inline_reasoning: bool = True +) -> CompletionResult: """Drain a ``create_streaming`` iterator into a ``CompletionResult``. + *scan_inline_reasoning* ``False`` (``capabilities.server_parses_reasoning`` + — the backend puts reasoning in its own channel) skips the inline + split: there is none to find, and the scan could only misroute prose + that quotes a tag. + The ONE non-streaming transport: single-shot callers (``model_turn``) sample through the provider's streaming entry and accumulate here, so the streaming and non-streaming lanes cannot drift apart per adapter. @@ -316,11 +327,24 @@ def drain_stream(chunks: Iterator[StreamChunk]) -> CompletionResult: usage: UsageInfo | None = None finish_reason: str | None = None provider_blocks: list[dict[str, Any]] = [] + tag_carry = "" - def _close_segment() -> None: - if segment_parts: - content_segments.append("".join(segment_parts)) - segment_parts.clear() + def _close_segment(*, carry_tail: bool = False) -> None: + # *carry_tail* (the reasoning_delta boundary): hold back a + # possible partial tag for the NEXT run — a reasoning delta + # cannot terminate a tag, so a tag the server split across it + # must reassemble ('……'), or the halves + # would pass through as visible content. Tool boundaries close + # WITHOUT carry: no tag spans a tool call (the interactive + # consumer's flush-at-tool-boundary rule). + nonlocal tag_carry + seg = tag_carry + "".join(segment_parts) + segment_parts.clear() + tag_carry = partial_tag_tail(seg) if carry_tail else "" + if tag_carry: + seg = seg[: -len(tag_carry)] + if seg: + content_segments.append(seg) for sc in transport_guarded(chunks): # Content accumulates in RUNS bounded by interleaving signals @@ -338,7 +362,7 @@ def drain_stream(chunks: Iterator[StreamChunk]) -> CompletionResult: # run exactly as the interactive consumer emits it. if sc.reasoning_delta: reasoning_parts.append(sc.reasoning_delta) - _close_segment() + _close_segment(carry_tail=True) if sc.content_delta: segment_parts.append(sc.content_delta) if sc.tool_call_deltas: @@ -369,7 +393,9 @@ def drain_stream(chunks: Iterator[StreamChunk]) -> CompletionResult: # distinguish tag residue from a genuine paragraph separator the # model emitted just before an interleaving signal — trimming each # run's edges fused sentences across the separator-less join. - split_segments = [split_inline_reasoning(seg) for seg in content_segments] + split_segments = [ + split_inline_reasoning(seg, scan_tags=scan_inline_reasoning) for seg in content_segments + ] content = "".join(c for c, _ in split_segments) extracted = "".join(r for _, r in split_segments) # The splitter can only REMOVE characters, so a shrunken total is the @@ -429,6 +455,16 @@ class ModelCapabilities: # Ignored when thinking_mode is "none" or by providers that handle # thinking natively (real Anthropic). thinking_param: str = "enable_thinking" + # The backend segregates model reasoning into its OWN channel + # (``reasoning_content`` deltas, native reasoning blocks) instead of + # leaving it in the content stream — a vLLM launched with a reasoning + # parser, a commercial provider. True turns the inline tag scan OFF + # everywhere (drain seam and interactive consumer alike): content is + # trusted verbatim, so prose that merely QUOTES a tag can no longer be + # misrouted, and the lanes that need no reasoning stop suppressing it + # (segregated reasoning costs the caller nothing). Default False is + # the passthrough-server fallback this whole dialect exists for. + server_parses_reasoning: bool = False # For local-server lanes (openai-compatible, anthropic-compatible): # the chat_template_kwargs key that carries a graded reasoning-effort # value, for templates that have one (e.g. "reasoning_effort" for @@ -690,6 +726,32 @@ def reasoning_template_kwargs( return updates +def thinking_off_template_kwargs(thinking_mode: str, thinking_param: str) -> dict[str, Any]: + """``chat_template_kwargs`` that turn the template's thinking toggle OFF. + + THE spelling of "this lane needs no reasoning", shared by every lane + that asks the model for a bounded artifact rather than a considered + answer: omni transcription (:func:`audio._omni_chat_extra_body`) and + the drained utility completions (title, compaction, web-fetch + extraction). Those lanes pay for reasoning twice — latency, and a + chain-of-thought that lands in the artifact whenever the server does + not segregate it (#940: the leaked reasoning then rides every + following turn as tool-result context). + + Only the DECLARED toggle is sent — the alias's own + ``thinking_param``, and only at a ``thinking_mode`` that has a toggle + at all. A model that declares none keeps its template default: this + is not a licence to guess a key. Note ``adaptive`` deliberately + always sends ``true`` through :func:`reasoning_template_kwargs` (the + knob may not force-disable a self-regulating model), so a lane that + genuinely needs silence must pin the key itself — that pin wins, + since the merge only ``setdefault``s. + """ + if thinking_param and thinking_mode in ("manual", "adaptive"): + return {thinking_param: False} + return {} + + def merge_reasoning_template_kwargs( caps: ModelCapabilities, reasoning_effort: str | None, diff --git a/turnstone/core/providers/_xai.py b/turnstone/core/providers/_xai.py index 876fa6e3..d4e23887 100644 --- a/turnstone/core/providers/_xai.py +++ b/turnstone/core/providers/_xai.py @@ -40,6 +40,7 @@ reasoning-replay behaviour. from __future__ import annotations +from dataclasses import replace from typing import Any from turnstone.core.providers._openai_common import resolve_server_side_tools @@ -128,8 +129,19 @@ _GROK_DEFAULT = ModelCapabilities( max_output_tokens=64_000, supports_web_search=True, server_side_tools=("web_search",), + # The commercial endpoint segregates reasoning natively + # (``reasoning_content``) — content never carries inline think tags, + # so the inline tag scan is off; the table transform below applies + # the same rule to every known entry. + server_parses_reasoning=True, ) +# ONE rule for the whole table — a per-entry flag would be forgotten on +# the next model row (see the default's comment). +GROK_CAPABILITIES = { + name: replace(caps, server_parses_reasoning=True) for name, caps in GROK_CAPABILITIES.items() +} + def lookup_grok_capabilities(model: str) -> ModelCapabilities: """Find capabilities for *model* by longest prefix match.""" diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 51aab631..6161d14a 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -142,6 +142,8 @@ from turnstone.core.model_turn import ( ModelTurnResult, ensure_tool_call_ids, finalize_provider_blocks, + lane_thinking_suppressed, + lane_without_thinking, maybe_attach_vllm_chat_reasoning, model_turn, provider_extra_params, @@ -960,17 +962,27 @@ _SPEC_ARGUMENTS_LITERAL_RE = re.compile(r"\$ARGUMENTS\b(?!\[)") # assignment scheme unless the operator set one), so the budget must fit a # full thinking pass at the MODEL'S OWN default — the prompt's hard word cap # keeps the visible answer trivially cheap, and ``_TITLE_MAX_TOKENS`` carries -# the rest. Recover the title from ``content`` — IR-clean at the drain -# seam (``split_inline_reasoning``; servers that leave reasoning inline -# rather than in ``reasoning_content`` are segregated there, this lane -# holds no strip of its own): take the first non-empty line (a model that -# appends an explanation shouldn't fold prose into the title), then peel a -# ``Title:`` label and wrapping markdown/quote decoration. Internal -# punctuation is preserved so ``.NET``, ``CI/CD``, ``v1.6.0`` survive. +# the rest. Recover the title from ``content`` — TAGGED reasoning is +# segregated at the drain seam (``split_inline_reasoning``) and this lane +# holds no strip of its own, but a server can also leave reasoning inline +# and UNMARKED — no tags, no ``reasoning_content`` — which no seam may +# reclassify. So the pick is the prompt's own contract rather than a +# position: the last line that reads as a title (word cap + ends in a +# word character — see the pick loop), then peel a ``Title:`` label and +# wrapping markdown/quote decoration. Internal punctuation is preserved +# so ``.NET``, ``CI/CD``, ``v1.6.0`` survive. _TITLE_MAX_TOKENS = 8192 # Match the manual-rename (alias) cap so generated and hand-set titles share # one length bound. _TITLE_MAX_CHARS = 80 +# The prompt's hard rule is 3 words; the slack absorbs sloppy compliance +# while still rejecting reasoning prose — the shape that has to lose is a +# sentence ("This title captures the request well." — 6), never a title a +# model padded by a word or two. Whitespace word counts are meaningless +# in unspaced scripts (a CJK sentence is one token), so the cap always +# pairs with the pick loop's ends-alphanumeric check, which rejects prose +# by its terminal punctuation instead. +_TITLE_MAX_WORDS = 5 _TITLE_LABEL_RE = re.compile(r"(?i)^\s*title\s*[:\-—]\s*") # Wrapping decoration peeled off both ends of a generated title. _TITLE_WRAP_CHARS = "*`\"' " @@ -1951,13 +1963,17 @@ class ChatSession: self._background_shells = BackgroundShellRegistry(on_exit=self._on_background_shell_exit) self._cancelled_partial_msg: dict[str, Any] | None = None # Creation-time HANDOFF REGISTER: _try_stream stamps the provider - # that owns the stream it is about to return (fallback walk - # included), and _stream_response copies it into a frame local - # immediately after each create returns. Nothing else reads it — - # a late read would race a superseding generation's creation — and - # it is deliberately never cleared (stale values are unreachable - # by construction). + # AND resolved capabilities that own the stream it is about to + # return (fallback walk included), and _stream_response copies + # them into frame locals immediately after each create returns. + # Nothing else reads them — a late read would race a superseding + # generation's creation — and they are deliberately never cleared + # (stale values are unreachable by construction). The caps ride + # beside the provider so the consumer's tag-scan posture + # (``server_parses_reasoning``) follows the ACTIVE lane, never + # the primary's, exactly like the retry gate's retryable set. self._active_stream_provider: LLMProvider | None = None + self._active_stream_caps: ModelCapabilities | None = None self._pending_retry: str | None = None # True when a fatal exception's text has been persisted to # workstream_config["last_error"] for the coord's inspect/wait @@ -3998,18 +4014,55 @@ class ChatSession: # with no open is indistinguishable from quoted prose, and # reclassifying would let quoted text destroy real answers) — # for a 3-word display string the cheap cosmetic call goes the - # other way, so peel through the LAST stray close tag here. - # This is title formatting like ``_TITLE_WRAP_CHARS``, not - # reasoning segregation. See ``_TITLE_*``. - for _close in ThinkTagSplitter.CLOSE_TAGS: - _pos = raw.rfind(_close) - if _pos != -1: - raw = raw[_pos + len(_close) :] - # First non-empty line, with a ``Title:`` label and wrapping - # markdown/quote decoration peeled (internal punctuation kept). - line = next((ln for ln in raw.splitlines() if ln.strip()), "") - line = _TITLE_LABEL_RE.sub("", line.strip(_TITLE_WRAP_CHARS)) - title = line.strip(_TITLE_WRAP_CHARS)[:_TITLE_MAX_CHARS] + # other way, so cut through the LAST stray close tag of either + # vocabulary here. This is title formatting like + # ``_TITLE_WRAP_CHARS``, not reasoning segregation — and like + # the seam's scan it is OFF on a backend that segregates + # (``server_parses_reasoning``): there a close tag in content + # IS quoted prose, and cutting would eat a title that mentions + # it. See ``_TITLE_*``. + if not self._get_capabilities().server_parses_reasoning: + _cut = max( + (raw.rfind(_t) + len(_t) for _t in ThinkTagSplitter.CLOSE_TAGS if _t in raw), + default=0, + ) + raw = raw[_cut:] + # Then the line, with a ``Title:`` label and wrapping markdown/ + # quote decoration peeled (internal punctuation kept). Scanning + # from the END applies the same law as the peel above: where a + # lane's reasoning shares the content field, the ANSWER comes + # last. A tag is not always there to peel — a server can leave + # reasoning inline and entirely UNMARKED: no open tag, no close + # tag, and no ``reasoning_content`` either (measured on the dev + # vLLM, which prefaces its chain-of-thought with a heading like + # ``Thinking Process:`` — that heading then BECAME the title). + # Nothing downstream can segregate that, and nothing should + # try: unmarked prose is exactly what the seam must pass + # through. So the pick is a contract check, not a position — + # the first line from the end that reads AS a title: + # * within the word cap — the prose rejector for spaced + # scripts ("This title captures the request well." is six + # words); and + # * ending in a letter/digit — the sentence/heading/sign-off + # rejector ("Hope that helps!", "(3 words)", "Thinking + # Process:", "Hmm, let me reconsider.") that also carries + # unspaced scripts, where whitespace word counts are + # meaningless but prose still ends in terminal punctuation + # (``…请求。``) while a title ends in a word character. + # Else the last non-empty line (a model that answered in one + # long or padded line still gets titled, bounded by + # ``_TITLE_MAX_CHARS`` — a padded answer beats promoting a + # reasoning fragment from higher up). + title = "" + for _ln in reversed(raw.splitlines()): + _cand = _TITLE_LABEL_RE.sub("", _ln.strip(_TITLE_WRAP_CHARS)) + _cand = _cand.strip(_TITLE_WRAP_CHARS) + if not _cand: + continue + title = title or _cand[:_TITLE_MAX_CHARS] + if len(_cand.split()) <= _TITLE_MAX_WORDS and _cand[-1].isalnum(): + title = _cand[:_TITLE_MAX_CHARS] + break if title and self._ws_id == ws_id: log.info("ws.title.updating", ws_id=ws_id[:8], title=title) update_workstream_title(ws_id, title) @@ -5608,6 +5661,25 @@ class ChatSession: for every caller of this funnel — present and future — and no caller may add a private strip. + Segregation only reaches reasoning the model MARKED, though, and a + passthrough server can emit it as unmarked prose — no tags, no + ``reasoning_content`` — which nothing downstream may reclassify + (#940: that prose became the web-fetch tool result, and tool + results ride every following turn). So when the backend does not + segregate reasoning itself, this funnel asks for none through + EVERY channel: the alias's declared thinking toggle is pinned off + and the lane/definition effort rungs cleared + (:func:`lane_without_thinking`), and the caller's relayed effort + knob is zeroed under the same predicate + (:func:`lane_thinking_suppressed`) — an effort value beside a + pinned-off toggle re-requests the reasoning the pin declined. + This is the same call omni transcription makes for the same + reason. Every caller here wants a bounded artifact — a title, a + summary, an extracted answer — not a considered one, and the + whole posture is skipped on a backend that segregates + (``server_parses_reasoning``), where reasoning costs the artifact + nothing and the operator's knobs stand. + ``max_tokens`` is clamped to the model's advertised output limit so small models don't error. @@ -5626,7 +5698,9 @@ class ChatSession: generously for exactly that reason. Callers relaying the session's user-facing effort knob (web-fetch extraction) pass it explicitly. extra_params resolve inside the lane from the same single config - fetch as the rest. + fetch as the rest, and the thinking pin is layered onto that + resolved dict rather than resolved separately — one config + generation, as ``resolve_lane`` intends. """ caps = self._get_capabilities() clamped = min(max_tokens, caps.max_output_tokens) if caps.max_output_tokens else max_tokens @@ -5640,12 +5714,14 @@ class ChatSession: config_store=self._config_store, backend_auth_resolver=self._model_backend_auth_token, ) + suppress_reasoning = lane_thinking_suppressed(lane) + lane = lane_without_thinking(lane) result = model_turn( lane, turns, max_tokens=clamped, temperature=self.temperature if temperature is None else temperature, - reasoning_effort=reasoning_effort, + reasoning_effort=None if suppress_reasoning else reasoning_effort, # The abort seam (default None): compaction passes a fresh # per-attempt _CancelRef so a user Stop closes the in-flight # summary HTTP stream instead of waiting it out. Title-gen @@ -6020,12 +6096,14 @@ class ChatSession: for attempt in range(self._MAX_RETRIES + 1): self._check_cancelled() self._cancel_ref.clear() # discard stale handle from prior attempt - # The provider about to own the live stream — read by the - # mid-stream retry gate (retryable-set membership) and the - # fatal formatter's label. Written here so the fallback walk - # (which routes through this method with provider=fb_provider) - # is covered by construction. + # The provider (and its resolved caps) about to own the live + # stream — read by the mid-stream retry gate (retryable-set + # membership), the fatal formatter's label, and the consumer's + # tag-scan posture. Written here so the fallback walk (which + # routes through this method with provider=fb_provider) is + # covered by construction. self._active_stream_provider = prov + self._active_stream_caps = resolved_caps try: return prov.create_streaming( client=client, @@ -7890,12 +7968,17 @@ class ChatSession: # FRAME-LOCAL copy of the creation-time handoff register, taken # immediately after the create returns: the retry gate must judge a # death by the provider that owns THIS stream (a fallback's set can - # differ), and reading the shared register later would race a - # superseding generation's own creation. + # differ), the consumer must scan tags by THIS stream's caps (a + # fallback's ``server_parses_reasoning`` can differ), and reading + # the shared register later would race a superseding generation's + # own creation. live_provider = self._active_stream_provider or self._provider + live_caps = self._active_stream_caps or self._get_capabilities() while True: try: - result = self._stream_attempt(transport_guarded(stream), my_generation) + result = self._stream_attempt( + transport_guarded(stream), my_generation, caps=live_caps + ) # The fold this turn was ACTUALLY created from rides the # returned message (popped by send() at calibration, before # commit — the message-dict underscore lane, like @@ -8030,10 +8113,11 @@ class ChatSession: msgs = self._prepare_wire_messages(self._full_messages()) try: stream = self._create_stream_with_retry(msgs) - # Refresh the frame-local gate identity: the + # Refresh the frame-local gate identities: the # re-create may have walked to a different - # provider (fallback, rebind). + # provider (fallback, rebind) with different caps. live_provider = self._active_stream_provider or self._provider + live_caps = self._active_stream_caps or self._get_capabilities() except Exception as recreate_exc: if _is_ctx_overflow(recreate_exc): # Deterministic — surface as ITSELF so send()'s @@ -8061,7 +8145,11 @@ class ChatSession: raise def _stream_attempt( - self, stream: Iterator[StreamChunk], my_generation: int = 0 + self, + stream: Iterator[StreamChunk], + my_generation: int = 0, + *, + caps: ModelCapabilities | None = None, ) -> dict[str, Any]: """Consume ONE streaming attempt, dispatching tokens to the UI live. @@ -8075,8 +8163,13 @@ class ChatSession: appending to self.messages. Single-pass by contract: the acquisition + mid-stream-retry wrapper is :meth:`_stream_response`, which hands this method a ``transport_guarded`` iterator per - attempt. + attempt — along with *caps*, the ACTIVE lane's capabilities from + the creation-time handoff register, so the tag-scan posture + follows the stream actually being consumed (a fallback's + ``server_parses_reasoning`` can differ from the primary's). + ``None`` (direct callers, tests) resolves the primary's. """ + caps = caps or self._get_capabilities() # Reset so this API call captures fresh usage — prevents stale # completion_tokens from a prior tool-chain iteration leaking # through the max() accumulator. _assistant_pending_tokens is the @@ -8110,8 +8203,16 @@ class ChatSession: self.ui.on_content_token(text) # Owns the partial-tag carry buffer and the in-think state; - # dispatch stays here in _flush_text. - splitter = ThinkTagSplitter(_flush_text) + # dispatch stays here in _flush_text. The tag scan follows the + # SAME capability the drain seam reads + # (``server_parses_reasoning``) so the interactive and drained + # lanes cannot disagree about whether a backend's content may + # contain inline reasoning — read from the ACTIVE lane's caps + # (the *caps* parameter), never the primary's. + splitter = ThinkTagSplitter( + _flush_text, + scan_tags=not caps.server_parses_reasoning, + ) def _stop_spinner_once() -> None: """Stop the spinner on first real content. Call is idempotent.""" @@ -19172,9 +19273,7 @@ class ChatSession: max_tokens=min(self.max_tokens, self.context_window // 4), reasoning_effort=self.reasoning_effort, ) - answer = result.content or "" - if not answer.strip(): - answer = "Error: extraction returned no answer" + answer = _non_blank_or(result.content, "Error: extraction returned no answer") except Exception as e: answer = f"Extraction failed (page was fetched but summarization errored): {e}" diff --git a/turnstone/core/streaming_text.py b/turnstone/core/streaming_text.py index 9a5254f2..e109acb6 100644 --- a/turnstone/core/streaming_text.py +++ b/turnstone/core/streaming_text.py @@ -36,6 +36,14 @@ class ThinkTagSplitter: Tag selection: at each step the EARLIEST occurrence wins among the tag variants for the current state (open tags outside a block, close tags inside). + + *scan_tags* ``False`` turns the tag scan OFF for backends that + segregate reasoning themselves (``capabilities.server_parses_reasoning`` + — a vLLM launched with a reasoning parser, a commercial provider): + spans pass straight through at the current state, so prose that merely + QUOTES a tag can no longer be misrouted, and no carry is held. The + state machine stays live either way — :attr:`in_think` still mirrors + the out-of-band transitions its consumer writes. """ OPEN_TAGS: tuple[str, ...] = ("", "") @@ -43,8 +51,9 @@ class ThinkTagSplitter: ALL_TAGS: tuple[str, ...] = OPEN_TAGS + CLOSE_TAGS MAX_TAG_LEN = max(len(t) for t in ALL_TAGS) - def __init__(self, emit: Callable[[str, bool], None]) -> None: + def __init__(self, emit: Callable[[str, bool], None], *, scan_tags: bool = True) -> None: self._emit = emit + self._scan_tags = scan_tags self.pending = "" self.in_think = False @@ -64,6 +73,12 @@ class ThinkTagSplitter: self.pending = "" def _drain(self) -> None: + if not self._scan_tags: + # Nothing to resolve, so nothing to hold: a tag-free contract + # makes every span immediately safe — the same emit + # ``flush_pending`` performs at a stream boundary. + self.flush_pending() + return while self.pending: tags = self.CLOSE_TAGS if self.in_think else self.OPEN_TAGS best_idx, best_tag = None, None @@ -90,16 +105,43 @@ class ThinkTagSplitter: break -def split_inline_reasoning(text: str) -> tuple[str, str]: +def partial_tag_tail(text: str) -> str: + """The longest suffix of *text* that could still grow into a tag. + + Tag-vocabulary knowledge for boundary handling: a consumer that must + finalize a span at an interleaving signal (``drain_stream`` closing a + content run at a ``reasoning_delta``) uses this to hold back ONLY a + possible partial tag for the next span — reassembling a tag the + server split across the signal — while everything decided emits with + the span it arrived in. Returns ``""`` when no suffix is a proper + prefix of any tag (a complete tag is not a partial one). + """ + limit = min(len(text), ThinkTagSplitter.MAX_TAG_LEN - 1) + for size in range(limit, 0, -1): + suffix = text[-size:] + if any(tag.startswith(suffix) for tag in ThinkTagSplitter.ALL_TAGS): + return suffix + return "" + + +def split_inline_reasoning(text: str, *, scan_tags: bool = True) -> tuple[str, str]: """Split a complete drained text into ``(content, reasoning)``. + *scan_tags* ``False`` (the backend segregates reasoning itself — + ``capabilities.server_parses_reasoning``) returns the text unsplit: + there is no inline reasoning to find, and scanning could only + misroute prose that quotes a tag. + The one-shot form of :class:`ThinkTagSplitter` for non-streaming consumers (``drain_stream``): a plain feed-and-flush of ONE content run — the interactive lane's per-run rule, no more. The caller owns run boundaries (``drain_stream`` closes a run when tool-call deltas or provider-parsed reasoning interleave, mirroring the interactive consumer's flush-and-reset at those signals); a partial tag never - spans an interleaving signal. Balanced + spans a TOOL boundary, and across a reasoning delta the caller + carries a possible partial-tag tail into the next run + (:func:`partial_tag_tail`) so a tag the server split there still + reassembles. Balanced blocks land in the reasoning lane; an unterminated open sends the tail to reasoning; an orphan CLOSE tag (no prior open) stays in content untouched. That last case is deliberate: a close tag whose @@ -120,7 +162,7 @@ def split_inline_reasoning(text: str) -> tuple[str, str]: survive). With no tag present anywhere the input returns byte-identical (fast path), so tag-free lanes cannot drift. """ - if not any(tag in text for tag in ThinkTagSplitter.ALL_TAGS): + if not scan_tags or not any(tag in text for tag in ThinkTagSplitter.ALL_TAGS): return text, "" content_parts: list[str] = [] diff --git a/turnstone/core/trajectory.py b/turnstone/core/trajectory.py index e85dd3a1..951b2817 100644 --- a/turnstone/core/trajectory.py +++ b/turnstone/core/trajectory.py @@ -146,9 +146,14 @@ class Turn: def text(self) -> str: """The turn's text content — the FTS projection and the str fast-path. - Joins the text of every :class:`TextBlock`; non-text blocks (attachments) + Joins the text of every non-empty :class:`TextBlock` with a + newline — adjacent blocks are distinct spans (an assistant's text + around a tool use, a user's text beside an attachment), and a + bare concatenation fused the last word of one to the first word + of the next in every downstream read (FTS tokens, notification + bodies, ``final_assistant_text``). Non-text blocks (attachments) contribute nothing (you cannot full-text-search an image).""" - return "".join(b.text for b in self.content if isinstance(b, TextBlock)) + return "\n".join(t for t in (b.text for b in self.content if isinstance(b, TextBlock)) if t) @property def effect_status(self) -> EffectStatus | None: diff --git a/turnstone/server.py b/turnstone/server.py index 6ec10f44..ebf48c18 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -2224,16 +2224,6 @@ def _validate_notify_targets(raw: Any) -> tuple[str, str]: return json.dumps(normalized), "" -def _extract_last_assistant_content(session: Any) -> str: - """Return the text of the session's final assistant say. - - THE final-say read (``trajectory.final_assistant_text``): no - walk-back, whitespace-only says report empty — so the notify - fallback fires instead of sending raw whitespace. - """ - return final_assistant_text(session.messages) - - def _fire_notify_targets(ws: Any, content: str) -> None: """Send completion notifications to all configured targets.""" if not ws.notify_targets: @@ -2816,7 +2806,10 @@ async def _interactive_create_post_install( # empty-content "(Task completed)" fallback, not "Failed:" — # is deferred to #865. try: - last_content = _extract_last_assistant_content(session) + # THE final-say read: no walk-back, whitespace-only + # says report empty — so the notify fallback fires + # instead of sending raw whitespace. + last_content = final_assistant_text(session.messages) _fire_notify_targets(ws, last_content) except Exception: log.warning("notify_completion.hook_error", ws_id=ws.id, exc_info=True)