fix(llm): preserve provider reasoning/thinking across assistant turns - #1070
Merged
Conversation
Contributor
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 9 selected item(s). |
lizhengfeng101
force-pushed
the
fix/reasoning-replay-805
branch
from
August 27, 2026 07:06
c00e650 to
6a988d1
Compare
Assistant tool-call history was rebuilt from Content()/ToolCalls() alone, so provider reasoning state was dropped on replay across all three supported protocols: - OpenAI-compatible Chat Completions: reasoning_content was parsed but never sent back on the next request. - Anthropic Messages: thinking/redacted_thinking blocks were flattened to a display string, losing the signature required to replay them. - OpenAI Responses: reasoning items (encrypted_content) were dropped entirely from the next request's input. Adds Message.Native (NativeTurn), an opaque per-protocol replay payload built by reusing each SDK's own response->request converter (anthropic.Message.ToParam(), each Responses output item's ToParam()) so the wire format is never hand-rolled. Each adapter type-asserts its own payload before reuse and falls back to the existing reconstruction otherwise, so cross-provider or native-less turns are unaffected. Also: - Adds Message.ReasoningContent, a plain-text projection used by the compression summarizer so a reasoning-only turn isn't rendered blank. - Fixes NativeTurn.EstimatedTokens() double-counting visible content already counted via ExtractText(), which could trigger compression earlier than necessary. - Guards the Responses adapter against carrying a dangling reasoning item with no accompanying message/function_call into Native. - Persists reasoning_content in the session JSONL transcript and surfaces it in the session viewer, for audit visibility into no-visible-answer turns. - Makes Anthropic extended thinking (extra_body.thinking) safe to enable via the existing provider config: dropped per-request when it would conflict with a forced tool_choice or exceed that call's max_tokens, rather than failing the whole request. - Removes GroupingTask's hardcoded 4096 max_tokens cap in favor of the template's own completion limit, which was the actual trigger for the max_tokens/budget_tokens conflict above. Refs #805
- Compare Anthropic extended thinking's budget_tokens against the effective max_tokens (falling back to the same 8192 default buildAnthropicParams uses) instead of the raw, possibly-zero ChatRequest.MaxTokens, so the guard doesn't get skipped when the caller leaves MaxTokens unset. - Preserve a degenerate turn in history when it has reasoning text but neither visible content nor a replayable Native payload (e.g. an openai-responses reasoning item with no accompanying message/function_call), instead of dropping all trace of it. - Unify token accounting between CountMessagesTokens and computeActiveZoneSize via a shared messageTokens() helper, so reasoning-heavy conversations don't get judged compressible by one and still-over-budget by the other. - Only write reasoning_content to the session JSONL transcript when non-empty, matching the existing pattern for comments/error/ sourceSessionId.
Session JSONL carried a human-readable ReasoningContent projection but dropped the opaque replay state (llm.Message.Native) that holds an Anthropic thinking block's signature or an OpenAI Responses reasoning item's encrypted_content — the fields a full session export/training reflow actually needs. - ResponseRecord gains a Native field; SetResponse populates it from resp.Native() and persists it verbatim as native_payload on the llm_response JSONL record. - copyMessagesForJSON now includes tool_calls (previously dropped) and native_payload per assistant message, so llm_request records are a complete, replayable projection of what was actually sent. - copyMessages (the in-memory deep copy backing TaskRecord.RequestMessages) now also carries Native/ReasoningContent, closing a gap where its own "deep copy" contract silently dropped both fields. Nothing parses or reshapes the payload on the way to disk — it is written exactly as the provider adapter would replay it.
lizhengfeng101
force-pushed
the
fix/reasoning-replay-805
branch
from
August 27, 2026 13:30
997369b to
6d25d0b
Compare
- Remove redundant 'what' comments and Go-basics explanations across the NativeTurn/reasoning replay implementation - Keep only non-obvious 'why' comments (external API constraints, aliasing hazards, fallback semantics) - Add 6 direct unit tests for ChatResponse.VisibleContent() covering: no choices, nil content, empty string, normal text, think-tag stripping, and whitespace-only (key difference from Content()) Net: -185 lines of comments, +66 lines of tests.
lizhengfeng101
changed the title
This was referenced Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #805. Assistant tool-call history was rebuilt from
Content()/ToolCalls()alone, so provider reasoning state was silently dropped when replaying a turn on the next request — across all three supported protocols:reasoning_contentwas parsed from the response but never sent back on the next request.thinking/redacted_thinkingblocks were flattened into a display string, losing thesignaturerequired to replay them (Anthropic rejects a modified/missing signature with a 400).reasoningoutput items (encrypted_content) were dropped entirely from the next request'sinput.Approach
Adds
Message.Native(NativeTurn), an opaque per-protocol replay payload built by reusing each SDK's own response→request converter (anthropic.Message.ToParam(), each Responses output item'sToParam()) instead of hand-rolling a serialization format. Each adapter type-asserts its own payload type before reuse and falls back to the existing reconstruction otherwise — a cross-provider turn or a turn built without a native payload is unaffected.Also included
Message.ReasoningContent: a plain-text projection (separate from the opaqueNativepayload) used by the compression summarizer, so a reasoning-only turn no longer renders as a blank message in the summarization prompt.NativeTurn.EstimatedTokens()double-counting visible content already counted viaExtractText(), which could trigger compression earlier than necessary.reasoningitem (no accompanyingmessage/function_call) intoNative, which risked a 400 on replay.reasoning_contentis now persisted in the session JSONL transcript and surfaced in the session viewer, for audit visibility into turns with no visible answer.extra_body.thinking) safe to enable via the existing provider config: it's dropped for a given request when it would conflict with a forcedtool_choiceor exceed that call'smax_tokens, instead of failing the request outright.GroupingTask's hardcodedmax_tokens: 4096cap in favor of the template's own completion limit — this was the actual trigger for themax_tokens/budget_tokensconflict above once thinking was enabled.Test plan
go build ./...go vet ./...go test ./...internal/llm/native_turn_test.go) asserting the replayed request body contains the original reasoning payload byte-for-byteextra_body.thinking): reasoning + tool_use turns replay cleanly across dozens of rounds with zero errors