mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(reasoning): synthesize reasoning_text alongside non-reasoning provider_blocks
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).
This commit is contained in:
@@ -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:**
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
"""
|
||||
...
|
||||
|
||||
+46
-13
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user