fix(runtime): persist one assistant message per AI SDK step, replay signed thinking in tool turns - #633
Merged
Merged
Conversation
Emit a per-step AssistantMessage (text + thinking + signature) at each finish-step boundary instead of one accumulated message at turn end, so the ledger records the text-tool timeline at step granularity and each step's Anthropic thinking signature stays paired with its own reasoning text. Each step rotates a fresh assistant messageId; tool calls stamp their step id onto tool_start (new ToolStartEvent.stepId -> RuntimeEventRefs.stepId) so model replay can regroup a step's reasoning with its tool calls. Pure-tool steps write no placeholder; the step-cap grace notice and abort partials flush per step. Also count steps on the v6 finish-step chunk (the old step-finish check was dead) and document start-step/finish-step as no-ops in the adapter.
Narrow the tool-turn signed-thinking replay skip to legacy per-turn history (tool calls with no step id); per-step turns whose tool calls carry refs.stepId now regroup into one provider assistant message (reasoning leads, then text, then tool calls, then tool results) — the Anthropic-valid shape, so signed thinking from tool-calling turns replays instead of being dropped. Read-model projectThinking pairs each step's reasoning to its own assistant row by step message id instead of attaching to the last same-turn assistant row. Tests: reverse the tool-turn thinking case to the per-step (merged) shape, keep a legacy (unpaired) suppression regression, add a multi-step backend integration test and a two-assistant read-model pairing test.
materialize concatenates a turn's per-step AssistantMessages (text and thinking) in step order, keeping the first step id as the anchor and advancing durationMs to the turn's final assistant message. The desktop Markdown export joins all step texts. The renderer text_delta slot resets on a messageId change so a new step's answer starts a fresh bubble instead of appending onto the draining prior step.
Assert handleStreamChunk emits no events and leaves the text/thinking callbacks untouched for start-step / finish-step chunks — the backend owns step accounting.
…ive replay The empty text_complete a thinking/tool-only step emits (so the read-model gets an assistant row for the step's reasoning) was classified unsupported_content in the replay plan — a blocking diagnostic that degraded the WHOLE ledger to stored-message projection. With interleaved thinking this is the most common step shape, so any such step poisoned replay for the rest of the session. Skip empty model text benignly (new empty_text_skipped diagnostic, non-blocking) and pair a step's parked reasoning with its tool calls by stepId at flush time in the materializer, so the no-text step still replays as one assistant message [reasoning, tool-call…] with no empty text block. Locked by a new replay test for the thinking + tool, no-text per-step shape.
…lay materializer An orphan tool_result (call sliced away or ledger corrupt) is dropped on purpose: a standalone tool message with no preceding tool_use is an Anthropic 400. The plan flags these as unmatched_tool_result; do not restore the old item-by-item orphan emission.
…rror The catch block only flushed the current step's streamed text/thinking on the abort (user_stop) exit; a provider failure or watchdog timeout dropped it. Hoist the flush ahead of both branches so partialOutputRetained reflects what the user actually saw on every terminal exit. Regression: mock stream emits a text delta then errors; the step's AssistantMessage persists with the streamed partial and the turn still closes as an error.
…g diagnostic The materializer already drops an orphan tool result on its own (a standalone tool message with no preceding tool_use is an Anthropic 400), but listing unmatched_tool_result in hasBlockingReplayDiagnostics meant one orphan degraded the ENTIRE ledger to stored-message projection — the drop path was unreachable. Unblock it: paired calls/results and signed reasoning replay provider-native, the orphan is dropped. Rewrote the test that locked the old fallback and added an end-to-end orphan + paired-history case.
…dary If a dependency version emits the older step-finish name, matching only finish-step would silently degrade back to one AssistantMessage per turn. Duplicate boundaries are safe: the second flush no-ops on cleared accumulators and an extra id rotation only discards an unused id.
…arkdown cast model-history: document why paired/unpaired tool-turn classification is turn-granular (one turn = one backend build; formats cannot mix within a turn) and that pairedToolTurnIds exists for sliced-view safe degradation, not dead state. conversation-markdown: replace the bare text cast with discriminated union narrowing via flatMap; no behavior change.
On the catch-all path (stream closed without a trailing finish-step) the current step id is already taken: the catch-all flush just used it for a thinking-only last step's AssistantMessage (reuse duplicated a ledger id), and a pure-tool last step's tool_starts carry it as stepId (replay would adopt the grace text as that step's closer). Always newId() — a rotated-but-unused id is discardable by design. Regression drives the backend through a patched startStream (streamText always synthesizes trailing boundaries): real tool execute in step 1, thinking-only step 2, abrupt close; asserts the grace id collides with no assistant row and no tool_start.stepId, and the ledger has no duplicate ids. Verified red against the old code.
sunheyi6
added a commit
to sunheyi6/maka-agent
that referenced
this pull request
Jul 8, 2026
…rializer PR apache#629 regressed the per-step persistence and RuntimeEvent replay materializer introduced by apache#633. Restore flushStep/currentStepMessageId (one AssistantMessage per AI SDK step, signed thinking merged with its tool call, orphan tool_results dropped) while keeping the guidance continuation pass and standing-steer prepareStep.
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
1 participant
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
The runtime accumulated an entire turn's assistant text/thinking into one
AssistantMessageflushed at turn end, losing the text↔tool timeline and overwriting per-step Anthropic thinking signatures. This PR flushes oneAssistantMessageper AI SDK step (its own text + thinking + signature), rotates the assistantmessageIdat each step boundary, stampsstepIdontotool_startevents, and rewrites replay materialization to merge each step's signed reasoning + text + tool calls into one provider assistant message — the shape Anthropic interleaved thinking requires.Why
The single-message-per-turn ledger had three concrete defects: (1)
text → tool → textorder was unrecoverable from storage; (2) multi-step turns kept only the last step's thinking signature, mismatched against the whole turn's concatenated thinking; (3) because of (2),model-history.tsskipped signed-thinking replay entirely for any tool-calling turn (signed_thinking_in_tool_turn_skipped), so the model lost its reasoning chain on every tool turn. Per-step persistence fixes all three at the source and unblocks the upcoming streaming-UI rework (step timeline = trow grouping).Scope
Changed: runtime (per-step flush, step-boundary handling incl.
finish-step/step-finishfallback, replay regrouping bystepId, read-model pairing by message id, error-path partial flush,unmatched_tool_resultno longer replay-blocking), core (ToolStartEvent.stepId,RuntimeEventRefs.stepId), ui/desktop neutrality (turn text/thinking concatenation, streaming slot reset on messageId change, Markdown export walks all steps).Not included: UI streaming rework (thinking block, tool trow, fade-in — next PR);
ai-sdk-backend.tsmodule split (tracked separately); per-steptoken_usage(stays turn-level).Verification
@maka/runtime977 pass /@maka/desktop2211 pass /@maka/storage+@maka/headlessgreen /npm run typecheckclean. New tests: multi-step flush +tool_start.stepId, per-step vs legacy replay, thinking-only no-text step (locks theempty_text_skippedpath that previously poisoned whole-ledger replay), orphantool_resultprovider-native drop, provider-error partial flush, two-assistant read-model pairing, multi-step turn concatenation.User-facing impact
Sessions recorded by this build replay signed thinking through tool turns (previously dropped). Partial output now survives provider errors, not just user aborts. Legacy sessions degrade to the old safe behavior. No schema migration: one turn simply may contain multiple assistant rows, which old readers already tolerate.
Reviewer notes
Reviewed pre-PR by Codex (xhigh) and DeepSeek v4 Pro; all accepted findings are in the last four commits. Highest-risk area:
materializeRuntimeReplayPlanregrouping (Anthropic 400 on mistakes) — the pairing is order-independent viarefs.stepIdbecause tool calls persist before their step's assistant row (execute()fires mid-step; buffering appends would deadlock the permission flow). Rollback: single squash revert restores per-turn behavior; new-format ledgers then degrade to the legacy skip, not corruption.Checklist