diff --git a/docs/api-reference.md b/docs/api-reference.md index d1d7efa2..c5da6cc3 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -281,6 +281,7 @@ Each message in the `messages` array has: | `role` | string | `"user"`, `"assistant"`, or `"tool"` | | `content` | string or null | Text content of the message | | `tool_calls` | array or null | Present only on assistant messages with calls | +| `reasoning` | string (optional) | Concatenated reasoning / chain-of-thought text on assistant turns whose `provider_data` carried reasoning-bearing blocks (Anthropic `thinking`, OpenAI Responses `reasoning`, or synthetic `reasoning_text` from local-model servers). Present only when the active model's `surface_persisted_reasoning` flag is True. | Each entry in `tool_calls`: diff --git a/docs/architecture.md b/docs/architecture.md index 9723cecc..86ac1ac0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -634,9 +634,9 @@ LLMProvider (protocol) | Type | Fields | |------|--------| -| `StreamChunk` | `content_delta`, `reasoning_delta`, `tool_call_deltas`, `info_delta`, `usage`, `finish_reason` | -| `CompletionResult` | `content`, `tool_calls`, `finish_reason`, `usage` | -| `ModelCapabilities` | `context_window`, `max_output_tokens`, `supports_temperature`, `token_param`, `thinking_mode`, `supports_effort`, `supports_web_search`, `supports_tool_search`, `supports_vision` | +| `StreamChunk` | `content_delta`, `reasoning_delta`, `tool_call_deltas`, `info_delta`, `usage`, `finish_reason`, `provider_blocks` | +| `CompletionResult` | `content`, `tool_calls`, `finish_reason`, `usage`, `provider_blocks` | +| `ModelCapabilities` | `context_window`, `max_output_tokens`, `supports_temperature`, `token_param`, `thinking_mode`, `supports_effort`, `supports_web_search`, `supports_tool_search`, `supports_vision`, `supports_reasoning_replay` | | `UsageInfo` | `prompt_tokens`, `completion_tokens`, `total_tokens`, `cache_creation_tokens`, `cache_read_tokens` | **OpenAIProvider** (`_openai.py`): passes messages through unchanged (they are @@ -724,6 +724,35 @@ and `"openai-compatible"`. `max_tokens`, and `reasoning_effort` to override the global defaults from ConfigStore. When unset (`NULL`), the global default is used. +**Per-model reasoning persistence:** Two booleans on `model_definitions` +(migration 052) control how reasoning text round-trips: + +* `surface_persisted_reasoning` (default `True`) — gates whether stored + reasoning text is surfaced on `/history` payloads for UI rehydration. + **Storage of reasoning bytes happens regardless of this flag** — they + ride in `provider_data` independently. Phase-1 admin UI label "Surface + persisted reasoning." +* `replay_reasoning_to_model` (default `False`) — gates whether stored + reasoning blocks are sent back to the provider on subsequent turns. + Capability-gated: `ModelCapabilities.supports_reasoning_replay` must + also be `True` for the wire path to actually replay (canonical OpenAI + gpt-5*/o-series and Anthropic Claude entries set it; unknown / local- + server models default to `False`). + +Three reasoning paths are recognised: + +| Path | Provider | Capture | Persist | Replay | +|------|----------|---------|---------|--------| +| 1 | Anthropic Messages API | `thinking_delta` | `provider_blocks` (`type="thinking"`) | Verbatim via `_provider_content` | +| 2 | OpenAI Responses (gpt-5*, o-series) | `response.reasoning_text.delta` events | `provider_blocks` (`type="reasoning"`) — only when `include=["reasoning.encrypted_content"]` | `ResponseReasoningItemParam` input items | +| 3 | OpenAI Chat Completions (vLLM, llama.cpp, Gemini-compat) | `delta.reasoning_content` Pydantic extras | Synthetic `{type: "reasoning_text", text, source}` block stamped at end-of-stream | None — no API surface for replay on Chat Completions | + +Cross-provider safety is enforced by `ANTHROPIC_VALID_BLOCK_TYPES` (a +shape filter in `_anthropic.py:_convert_messages`): foreign blocks +(OpenAI `reasoning`, synthetic `reasoning_text`) fall through to the +text+tool_calls rebuild path rather than reaching Anthropic's input +boundary as malformed content. + ```toml [models.local] base_url = "http://localhost:8000/v1" diff --git a/docs/diagrams/03-core-engine-classes.puml b/docs/diagrams/03-core-engine-classes.puml index b026da23..fdba3a6b 100644 --- a/docs/diagrams/03-core-engine-classes.puml +++ b/docs/diagrams/03-core-engine-classes.puml @@ -69,9 +69,10 @@ class "NullUI" as NullUI { interface "LLMProvider" as LLMProvider <> { + provider_name: str {property} + get_capabilities(model) → ModelCapabilities - + create_streaming(client, model, messages, ...) → Iterator[StreamChunk] - + create_completion(client, model, messages, ...) → CompletionResult + + create_streaming(client, model, messages, ..., replay_reasoning_to_model) → Iterator[StreamChunk] + + create_completion(client, model, messages, ..., replay_reasoning_to_model) → CompletionResult + convert_tools(tools) → list[dict] + + extract_reasoning_text(provider_blocks) → str + retryable_error_names: frozenset[str] {property} -- core/providers/_protocol.py @@ -126,6 +127,7 @@ class "ModelCapabilities" as ModelCaps <> { + supports_web_search: bool + supports_tool_search: bool + supports_vision: bool + + supports_reasoning_replay: bool } ' ChatSession diff --git a/docs/diagrams/png/03-core-engine-classes.png b/docs/diagrams/png/03-core-engine-classes.png index b6d07b97..7baa410c 100644 --- a/docs/diagrams/png/03-core-engine-classes.png +++ b/docs/diagrams/png/03-core-engine-classes.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:25b5448bbb7da8ddafe4f65c6c5e6cbcaa9cb9f31746ca46d3a2241bc47b1956 -size 259687 +oid sha256:9857db23fe3c4316d492073aac69c7e7558b1abe3b95ad7756d4a5933bd0ece7 +size 620214 diff --git a/docs/settings.md b/docs/settings.md index bc3a063a..ad36742c 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -59,6 +59,21 @@ from ConfigStore. Model names and context windows are now configured per-model in the Models tab. A startup warning is logged if these keys appear in `config.toml`. +### Reasoning persistence (per-model) + +Two boolean flags on `model_definitions` (migration 052) control how +reasoning text round-trips per model: + +| Flag | Default | Effect | +|------|---------|--------| +| `surface_persisted_reasoning` | `True` | Surface stored reasoning text on `/history` payloads so a page reload re-renders the reasoning bubble. **Storage of reasoning bytes is independent of this flag** — they ride in `provider_data` regardless. | +| `replay_reasoning_to_model` | `False` | Send stored reasoning blocks back to the provider on subsequent turns. Capability-gated: only takes effect when the model's `ModelCapabilities.supports_reasoning_replay` is also `True`. Set on canonical OpenAI gpt-5*/o-series and Anthropic Claude entries; unknown / local-server models default to `False` so an operator who flips the flag on a model whose API doesn't understand reasoning replay silently no-ops rather than 400-ing. | + +Edit both via the admin Models tab. See the architecture doc for the +provider-side mechanics (Anthropic `thinking`, OpenAI Responses +`reasoning` + `include=["reasoning.encrypted_content"]`, synthetic +`reasoning_text` for Chat Completions / vLLM / llama.cpp / Gemini-compat). + ### Plan / task agent overrides `plan_agent` and `task_agent` sub-sessions resolve independently from the diff --git a/sdk/typescript/src/events.ts b/sdk/typescript/src/events.ts index b8332435..d344f2ee 100644 --- a/sdk/typescript/src/events.ts +++ b/sdk/typescript/src/events.ts @@ -22,8 +22,10 @@ export interface HistoryEvent { * - `reminders`: metacognitive nudge bubbles (user/tool channels) * - `advisories`: extracted `UserInterjection` payloads on tool turns * - `reasoning`: concatenated reasoning text for assistant turns whose - * `provider_data` carried thinking blocks (Anthropic today). Present - * only when the active model's `persist_reasoning` flag is true. + * `provider_data` carried reasoning-bearing blocks (Anthropic + * `thinking`, OpenAI Responses `reasoning`, or synthetic + * `reasoning_text` from path-3 servers). Present only when the + * active model's `surface_persisted_reasoning` flag is true. */ messages: Array>; } diff --git a/tests/test_history_decoration.py b/tests/test_history_decoration.py index 116ab83b..6bcb9799 100644 --- a/tests/test_history_decoration.py +++ b/tests/test_history_decoration.py @@ -617,6 +617,36 @@ class TestExtractReasoningForHistory: assert messages[0]["reasoning"] == "synth thought" assert "_provider_content" not in messages[0] + def test_dispatcher_scans_past_unrecognized_first_blocks(self) -> None: + # Regression for Copilot finding: dispatcher used to inspect + # only provider_content[0]['type']. OpenAI Responses captures + # EVERY output_item.done event into provider_blocks (not just + # reasoning), so a hypothetical [message, reasoning, ...] + # ordering would have silently dropped the reasoning. Now + # walks the list for the first recognised reasoning-bearing + # type and dispatches the whole list to that provider. + from turnstone.core.history_decoration import extract_reasoning_for_history + + messages = [ + { + "role": "assistant", + "content": "answer", + "_provider_content": [ + # First block is a non-reasoning OpenAI Responses item. + {"type": "message", "role": "assistant", "content": "answer"}, + # Reasoning sits later in the list. + { + "type": "reasoning", + "id": "r_1", + "summary": [{"type": "summary_text", "text": "deferred"}], + }, + ], + } + ] + extract_reasoning_for_history(messages, surface_persisted_reasoning_flag=True) + assert messages[0]["reasoning"] == "deferred" + assert "_provider_content" not in messages[0] + def test_first_block_redacted_thinking_dispatches_to_anthropic(self) -> None: # Anthropic's extended-thinking API documents that # ``redacted_thinking`` blocks (sealed by the safety system) diff --git a/turnstone/core/history_decoration.py b/turnstone/core/history_decoration.py index d2fbd531..73b3eb8c 100644 --- a/turnstone/core/history_decoration.py +++ b/turnstone/core/history_decoration.py @@ -339,9 +339,28 @@ _BLOCK_TYPE_PROVIDER_FACTORY: dict[str, Callable[[], LLMProvider]] = { def extract_reasoning_text_from_provider_content(provider_content: Any) -> str: - """Dispatch reasoning extraction by first-block ``type`` field. + """Dispatch reasoning extraction by scanning for a recognised block type. - Returns ``""`` for empty / missing / non-list / unknown-type input. + Walks ``provider_content`` looking for the first block whose + ``type`` is in :data:`_BLOCK_TYPE_PROVIDER_FACTORY`, then dispatches + the WHOLE list to that provider's ``extract_reasoning_text``. Each + provider's extractor already filters internally by its own block + type (Anthropic walks ``thinking``, OpenAI Responses walks + ``reasoning``, OpenAI Chat walks ``reasoning_text``), so passing + the full list is correct — interleaved foreign blocks are ignored. + + Returns ``""`` for empty / missing / non-list input or when no + recognised reasoning-bearing block type appears anywhere in the + list. + + Why scan instead of just inspecting ``provider_content[0]``: the + OpenAI Responses streaming layer captures EVERY ``output_item.done`` + event into ``provider_blocks`` (``_openai_responses.py:415-420``), + not just reasoning items. In practice the order is usually + ``[reasoning, message, ...]`` but the API doesn't guarantee that — + a hypothetical ``[message, reasoning]`` ordering would silently + drop the reasoning under an index-only check. Same robustness + point for Anthropic's hypothetical mixed-order outputs. Pure transform — safe from any thread. Both history surfaces (interactive ``_build_history`` and lifted ``make_history_handler``) @@ -350,16 +369,16 @@ def extract_reasoning_text_from_provider_content(provider_content: Any) -> str: """ if not isinstance(provider_content, list) or not provider_content: return "" - first_block = provider_content[0] - if not isinstance(first_block, dict): - return "" - block_type = first_block.get("type") - if not isinstance(block_type, str): - return "" - factory = _BLOCK_TYPE_PROVIDER_FACTORY.get(block_type) - if factory is None: - return "" - return factory().extract_reasoning_text(provider_content) + for block in provider_content: + if not isinstance(block, dict): + continue + block_type = block.get("type") + if not isinstance(block_type, str): + continue + factory = _BLOCK_TYPE_PROVIDER_FACTORY.get(block_type) + if factory is not None: + return factory().extract_reasoning_text(provider_content) + return "" def extract_reasoning_for_history( diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 2e6168fe..c78fd573 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -1118,7 +1118,15 @@ class ChatSession: if isinstance(sc, dict): return str(sc.get("server_type") or "") except Exception: - pass + # Best-effort lookup — synth-block source tagging is + # informational, never load-bearing. Log at debug so a + # repeated registry-lookup failure during a session shows + # up under DEBUG triage but doesn't spam normal logs. + log.debug( + "_resolve_server_type lookup failed for alias=%s; defaulting to empty", + target_alias, + exc_info=True, + ) return "" def _maybe_synth_reasoning_block( diff --git a/turnstone/core/storage/migrations/versions/052_model_reasoning_persistence.py b/turnstone/core/storage/migrations/versions/052_model_reasoning_persistence.py index a4bc68b7..da6fee1a 100644 --- a/turnstone/core/storage/migrations/versions/052_model_reasoning_persistence.py +++ b/turnstone/core/storage/migrations/versions/052_model_reasoning_persistence.py @@ -8,9 +8,10 @@ Adds two boolean (integer-coded) operator knobs: page refresh re-renders the reasoning bubble. **Storage of the reasoning bytes happens regardless of this flag** — it only controls the extract-and-include step in ``_build_history`` / - ``decorate_history_messages``. The earlier name ``surface_persisted_reasoning`` - was renamed because it implied a storage-control switch; this flag - is purely about UI rehydration. + ``decorate_history_messages``. The pre-rename column was + ``persist_reasoning``; the rename to ``surface_persisted_reasoning`` + happened in the review-fix wave because the original name implied a + storage-control switch when the flag is purely about UI rehydration. * ``replay_reasoning_to_model`` (default ``0``) — when true, the wire-build path keeps reasoning blocks in the outgoing ``_provider_content`` lane on subsequent provider calls. False is the