From 6f8574eef3aae5cee65365ce0e7f7847e7838865 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Sat, 9 May 2026 02:40:35 -0700 Subject: [PATCH] fix(reasoning): synthesize reasoning_text alongside non-reasoning provider_blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GoogleProvider attaches raw tool_call dicts as ``provider_blocks`` on the finish chunk for ``thought_signature`` round-trip (``_google.py:_iter_stream``). When the same turn streamed Gemini's ``reasoning_content`` as ``reasoning_delta`` chunks, the prior synthesizer bailed out the moment ``provider_blocks`` was non-empty — so the captured reasoning was visible live but lost on page reload. Replace the early-return-if-non-empty check with a reasoning-bearing type test (``thinking`` / ``redacted_thinking`` / ``reasoning`` / ``reasoning_text``). When none of those types appear, append the synthetic ``reasoning_text`` block to the existing list rather than replacing it — preserving Google's tool-call fidelity blocks. Also addresses two doc-accuracy review findings: - ``LLMProvider.extract_reasoning_text`` docstring no longer claims OpenAI Chat / Responses are unwired (Phase 3+4 shipped extractors). - Add the method to the Protocol methods table in ``docs/architecture.md`` (was missing alongside the class diagram). --- docs/architecture.md | 1 + tests/test_session_synth_reasoning_block.py | 44 +++++++++++++++ turnstone/core/providers/_protocol.py | 24 +++++++-- turnstone/core/session.py | 59 ++++++++++++++++----- 4 files changed, 110 insertions(+), 18 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 86ac1ac0..eb7ffee7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -629,6 +629,7 @@ LLMProvider (protocol) | `get_capabilities()` | Per-model flags (`ModelCapabilities`) | | `convert_tools()` | Translate OpenAI tool schemas to provider format | | `retryable_error_names` | Exception class names that trigger retry | +| `extract_reasoning_text()` | Walk stored `provider_blocks`, return concatenated reasoning text for UI rehydration (per-provider block-type knowledge: Anthropic `thinking`, OpenAI Responses `reasoning`, OpenAI Chat synthetic `reasoning_text`) | **Normalized data types:** diff --git a/tests/test_session_synth_reasoning_block.py b/tests/test_session_synth_reasoning_block.py index 7345dbac..5f5b1083 100644 --- a/tests/test_session_synth_reasoning_block.py +++ b/tests/test_session_synth_reasoning_block.py @@ -94,6 +94,50 @@ class TestMaybeSynthReasoningBlock: assert out[0]["text"] == "text" assert "source" not in out[0] + def test_synth_appends_when_provider_blocks_are_non_reasoning(self) -> None: + # GoogleProvider attaches raw tool_call dicts as provider_blocks + # on the finish chunk (for thought_signature round-trip). When + # the same turn streamed reasoning_delta (Gemini's reasoning_ + # content extra), the synthesizer must APPEND the synthetic + # reasoning block rather than skip synthesis — otherwise the + # reasoning text is shown live but lost on page reload. + session = _make_session() + existing = [ + { + "id": "call_1", + "type": "function", + "function": {"name": "search", "arguments": "{}"}, + "thought_signature": "sig123", + } + ] + out = session._maybe_synth_reasoning_block(existing, ["I should search"]) + assert len(out) == 2 + assert out[0] is existing[0] # tool_call fidelity block survives intact + assert out[1]["type"] == "reasoning_text" + assert out[1]["text"] == "I should search" + + def test_no_synth_when_openai_responses_reasoning_already_present(self) -> None: + # OpenAI Responses native reasoning item — synth must NOT fire + # even though provider_blocks contains ALSO non-reasoning items + # (e.g. message blocks). The reasoning-bearing block satisfies + # the persistence contract on its own. + session = _make_session() + existing = [ + {"type": "reasoning", "summary": [{"text": "openai reasoning"}]}, + {"type": "message", "role": "assistant", "content": "answer"}, + ] + out = session._maybe_synth_reasoning_block(existing, ["live reasoning text"]) + assert out is existing + + def test_no_synth_when_non_reasoning_blocks_but_reasoning_parts_empty(self) -> None: + # Google tool_calls with no reasoning streamed — return as-is. + session = _make_session() + existing = [ + {"id": "call_1", "type": "function", "function": {"name": "f", "arguments": "{}"}} + ] + out = session._maybe_synth_reasoning_block(existing, []) + assert out is existing + class TestSyntheticBlockShapeContract: """The synthetic block shape MUST stay outside Anthropic's valid diff --git a/turnstone/core/providers/_protocol.py b/turnstone/core/providers/_protocol.py index b3ca4459..34d60320 100644 --- a/turnstone/core/providers/_protocol.py +++ b/turnstone/core/providers/_protocol.py @@ -256,10 +256,24 @@ class LLMProvider(Protocol): ) -> str: """Return concatenated reasoning text from stored ``provider_blocks``. - Providers without a first-class reasoning shape (OpenAI Chat, - Google) or that haven't been wired yet (OpenAI Responses pre- - Phase-3) return ``""``. AnthropicProvider walks - ``type=="thinking"`` blocks and returns the concatenated - ``thinking`` text, capped at an operator-friendly size. + Each provider walks the block types it owns: + + * ``AnthropicProvider`` — ``thinking`` blocks (concatenated + ``thinking`` text). + * ``OpenAIResponsesProvider`` — ``reasoning`` items + (concatenated ``summary`` + ``content`` text). + * ``OpenAIChatCompletionsProvider`` — synthetic + ``reasoning_text`` blocks stamped by + ``ChatSession._maybe_synth_reasoning_block`` for vLLM / + llama.cpp / Gemini-OpenAI-compat reasoning capture. + * ``GoogleProvider`` — inherits the OpenAI Chat extractor + (Gemini's ``/v1beta/openai/`` reasoning surfaces as + synthetic ``reasoning_text`` blocks too). + + All providers return the joined text capped at + :data:`MAX_REASONING_DISPLAY_CHARS` for UI rendering; full + bytes remain in ``provider_data`` for replay. Returns ``""`` + when the input list contains no recognised reasoning-bearing + blocks for the implementing provider. """ ... diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 74434aff..ab7c5a55 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -579,6 +579,21 @@ def _render_template(content: str, context: dict[str, str]) -> str: return _TEMPLATE_VAR_RE.sub(_replace, content) +# Block types that carry reasoning content across providers. Used by +# ``ChatSession._maybe_synth_reasoning_block`` to decide whether +# captured ``reasoning_parts`` need a synthetic ``reasoning_text`` +# block: if any of these types already appear in ``provider_blocks``, +# native lane handles persistence and synthesis is a no-op. +# - ``thinking`` / ``redacted_thinking`` — Anthropic native +# - ``reasoning`` — OpenAI Responses native +# - ``reasoning_text`` — synthetic (path-3 capture; included so +# re-running this code path against an already-synthesized list is +# idempotent). +_REASONING_BEARING_BLOCK_TYPES: frozenset[str] = frozenset( + {"thinking", "redacted_thinking", "reasoning", "reasoning_text"} +) + + # --------------------------------------------------------------------------- # SessionUI protocol — the contract every frontend must implement # --------------------------------------------------------------------------- @@ -1135,20 +1150,32 @@ class ChatSession: reasoning_parts: list[str], ) -> list[dict[str, Any]]: """Stamp captured ``reasoning_parts`` as a synthetic ``reasoning_text`` - block when no native ``provider_blocks`` were emitted. + block when no reasoning-bearing block already appears in + ``provider_blocks``. Anthropic emits native ``thinking`` blocks; OpenAI Responses emits native ``reasoning`` items via ``output_item.done``. - Both populate ``provider_blocks`` directly during streaming - and need no synthesis here. + Both populate ``provider_blocks`` with reasoning-bearing + shapes during streaming and need no synthesis here. - OpenAI Chat Completions (with vLLM ``--reasoning-parser``, - llama.cpp ``reasoning_format``, or Gemini's ``/v1beta/openai/`` - endpoint if it surfaces ``reasoning_content``) streams - reasoning as ``reasoning_delta`` chunks but never produces a - provider_blocks item. Without this synthesis the captured - text would be dropped at the end of the stream — visible live, - invisible on page reload. + OpenAI Chat Completions (vLLM ``--reasoning-parser``, llama.cpp + ``reasoning_format``, Gemini's ``/v1beta/openai/`` endpoint + when it surfaces ``reasoning_content``) streams reasoning as + ``reasoning_delta`` chunks but never emits a reasoning-bearing + provider block. Without this synthesis the captured text would + be dropped at the end of the stream — visible live, invisible + on page reload. + + Crucially, GoogleProvider attaches raw tool_call dicts as + ``provider_blocks`` on the finish chunk for ``thought_signature`` + round-trip (``_google.py:_iter_stream``). An earlier version + bailed out whenever ``provider_blocks`` was non-empty, which + silently lost reasoning text on Google + reasoning_delta turns. + The fix tests for reasoning-bearing block types specifically + (see ``_REASONING_BEARING_BLOCK_TYPES``) and APPENDS the + synthetic block to the existing list rather than replacing it + — preserving Google's tool-call fidelity blocks alongside the + new synthetic reasoning entry. The synthetic block uses ``type="reasoning_text"`` (NOT ``"thinking"``) so it falls through Phase 2's @@ -1165,11 +1192,15 @@ class ChatSession: reasoning back into a vllm round-trip) — not consumed today; the field is informational metadata, not dead code. """ - if provider_blocks: - return provider_blocks text = "".join(reasoning_parts) if not text.strip(): return provider_blocks + # Native reasoning already present — Anthropic / OpenAI + # Responses path. No synth needed; return reference unchanged + # so the existing identity contract holds. + for b in provider_blocks: + if isinstance(b, dict) and b.get("type") in _REASONING_BEARING_BLOCK_TYPES: + return provider_blocks block: dict[str, Any] = { "type": "reasoning_text", "text": text, @@ -1177,7 +1208,9 @@ class ChatSession: server_type = self._resolve_server_type() if server_type: block["source"] = server_type - return [block] + # Append rather than replace so non-reasoning fidelity blocks + # (e.g. Google tool_calls with thought_signature) survive. + return [*provider_blocks, block] def _resolve_replay_reasoning_to_model(self, alias: str | None = None) -> bool: """Read ``ModelConfig.replay_reasoning_to_model`` for an alias.