chat-ui
108b97bc - Preserve reasoning and replay tool history across turns (#2470)

Commit
6 days ago
Preserve reasoning and replay tool history across turns (#2470) * Preserve reasoning and replay tool history for multi-turn tool calling Inside the MCP loop, keep <think> content as reasoning_content on the assistant tool-call message instead of dropping it: preserved-thinking models (Kimi K2/K3 family) condition each tool round on their prior reasoning and degrade when it is stripped. Also omit the content field entirely when nothing visible remains, since some OpenAI-compatible backends reject empty text next to tool_calls with a 400. Across turns, the MCP flow now opts into replayToolHistory in prepareMessagesWithFiles: past assistant turns are expanded from their persisted tool updates into assistant/tool message pairs (grouped back into rounds, update uuid as tool_call_id, outputs capped at 8k chars), with reasoning re-attached as reasoning_content instead of inline <think> text. Previously history was flattened to plain {role, content}, so models saw no evidence of tools they had just used and would deny having them. The plain completion path is unchanged. Verified with scripts/reasoning-replay-harness.ts, which sends the same tool-using conversation in the old flat shape and both new shapes to the first 10 router models: no model that accepts the old shape rejects the new ones (2 runs, 0 regressions). * Measure streaming TTFT and throughput in the replay harness Requests now stream like prod and repeat per model/scenario (sequential reps, models in parallel), reporting median time-to-first-token and chunk-approximated tokens/sec alongside the acceptance verdict. * Harden tool history replay: provider-safe ids, honest interrupts, size budget Derive nine-character alphanumeric tool_call_ids from the persisted update uuids, since Mistral-family chat templates reject other shapes. Replay calls that have no persisted Result or Error as an explicit interruption error instead of fabricating an empty successful output, which an aborted run can otherwise leave behind. Cap the cumulative expanded replay at 100k chars spent newest-first, with older turns falling back to the flat shape, so long tool-heavy histories cannot outgrow a context window they previously fit. Document that replayed arguments are best-effort, as only top-level primitive params are persisted by design. * Extend reasoning_content replay to the plain completion flow The tool-less chat_completions path now re-attaches persisted reasoning (inline think blocks and message.reasoning) as reasoning_content on past assistant turns, so preserved-thinking models keep their chain in ordinary conversations too. Tool replay stays off there since the path never declares tools. Per review: the cross-turn reasoning echo is gated on the model's supportsReasoning flag in both flows (matching how reasoning_effort is forwarded), so strict non-reasoning backends never see the nonstandard field, and attachReasoning payloads now spend the same newest-first 100k replay budget as tool history, falling back to the flat shape when exhausted. The in-loop echo remains ungated because it only fires when the model emitted reasoning in that same turn. The harness gains tool-less scenarios (flat vs reasoning_content): across the first 10 router models, no model that accepts the flat shape rejects the reasoning shape. * Clarify why replayed reasoning attaches to the final message Only the last loop iteration's text is persisted, so the recovered reasoning belongs with the final answer; per-round reasoning is never stored and the live in-loop echo covers that case instead. * Make replay budget degradation monotonic When the newest turn alone exceeded the 100k replay budget it fell back to flat without consuming the budget, so an older, smaller turn could still expand: rich tool history for a stale turn while the turn being continued from was plain prose, inverting the newest-first invariant. Once any turn falls back to flat, every older turn now does too, with a regression test where the newest turn alone exceeds the budget. * Attribute replayed reasoning to its tool round; honor user reasoning override Review follow-ups on the replay shape: The FinalAnswer handler merges the pre-tool stream into content when tools ran, so recovered think blocks can belong to earlier tool rounds, not just the final answer. The live loop now persists each round's reasoning on the round's first Call update (optional field, absent on old messages), and replay re-attaches it to that round's assistant tool-call message, deduping it out of the final message's blocks. Old conversations keep the previous behavior of attaching recovered reasoning to the final message. The cross-turn reasoning gate previously read only the static supportsReasoning flag, while reasoning_effort forwarding also honors the per-user reasoningOverrides setting, which can force-enable a model on self-hosted installs or force-disable a flagged one. The override is now threaded through both flows and wins in both directions, with the capability flag as fallback. * Accept reasoning_text as a third incoming reasoning field Some OpenAI-compatible providers stream reasoning under reasoning_text rather than reasoning or reasoning_content. Read it in the MCP loop, in both stream adapters of the plain flow, and in the harness. * Flag reasoning support for DeepSeek-V4-Flash and Qwen3.6 models Vendor research across the top router models: DeepSeek documents a hard 400 when reasoning_content is not passed back on tool-call turns (Flash was the only Required-verdict model missing the flag; Pro already had it), and the Qwen3.6 generation is trained for preserve_thinking with documented accuracy loss in tool flows when reasoning is omitted. gemma-4 stays unflagged deliberately: Google requires stripping thoughts across completed turns. * Teach sync-models that supportsReasoning also gates reasoning replay The flag now controls the cross-turn reasoning_content echo in addition to the thinking-effort dropdown, so the skill's flagging decision needs the vendor's preserved-thinking guidance: flag models whose vendors require or recommend passing reasoning back (Kimi, MiniMax, DeepSeek V4, GLM, Qwen3.6), and never flag models whose vendors require stripping historical thoughts (Gemma family), even when they accept an effort knob. * Fix reasoning byte fidelity, preamble attribution, and budget-fallback leak Four review follow-ups on the replay shape: Reasoning was trimmed before being echoed and persisted, both in the live loop (runMcpFlow) and in cross-turn replay (splitReasoning). Vendors documenting preserved thinking can require the payload sent back unmodified or use it for cache matching, so trimming must only decide whether a value counts as empty, never change what gets echoed. Whitespace-only parts are still filtered out; surviving parts keep their exact bytes. Visible preamble text streamed before a round's tool calls (e.g. "Let me check that.") was being moved onto the final answer on replay, after the tool results, since only reasoning had a per-round slot to reclaim it from. The live loop now persists each round's preamble on the round's first Call update (optional content field, mirroring the existing reasoning field), and replay re-attaches it to that round's message, deduping it out of the final message the same way reasoning already was. The replay-budget fallback used the raw message.content when a turn didn't fit, which defeats the point for models whose vendor requires historical thoughts stripped (e.g. Gemma): they'd get raw <think> text back regardless of budget, on top of never losing anything for preserved-thinking models either. The fallback is now the <think>-stripped shape in both the tool-replay and reasoning-only paths. Also tightened scripts/reasoning-replay-harness.ts: a scenario counted as compatible if any single repetition succeeded, silently rounding up a real half-failure rate. It now requires every repetition to pass and reports partial failures as FLAKY(n/total) instead of OK. 6 new unit tests (16 total in prepareFiles.spec.ts, up from 10); full server/SSR suite: 518/519 pass, the one failure is the pre-existing CORS test unrelated to this branch. * Fix preamble duplication on replay and whitespace loss in reasoning stream The pre-tool preamble was persisted untrimmed while replay compares it against trim-normalized visible text, so a preamble starting with newlines (the common case after a think block) failed the dedup match and replayed twice. Visible text is now trimmed on both sides; reasoning stays byte-exact. Whitespace-only reasoning deltas were dropped from the think merge even mid-block, losing paragraph breaks from the echoed trace. They are now appended whenever a think block is already open; non-blank text is still required to open one, so stray whitespace cannot create empty blocks. * Strip Gemma-flow think leak, gate cross-producer reasoning, persist raw tool args External review (discussed with Codex) confirmed the design and flagged concrete gaps. This addresses the five agreed pre-merge items, then three more found by /review on the resulting diff, then one final read-boundary hardening Codex caught on a second pass. Five agreed items: - Plain flow (endpointOai.ts) previously fell through to raw message.content when attachReasoning was false, leaking inline <think> text to models like Gemma whose vendor requires historical thoughts stripped. Now always routes through splitReasoning first. - Cross-turn reasoning_content is now gated on message.routerMetadata.model matching the current turn's resolved model: under the "omni" router alias, per-message routing can mix producers within one conversation with no user action, and reasoning is conditioned on its own producer. Tool call/result replay stays unconditional; only reasoning_content is gated. Applies in both the MCP flow (candidateModelId) and the plain flow (model.id, already resolved by the time endpointOai.ts runs, whether invoked directly or as a router candidate). - Original provider tool-call id and raw JSON arguments string are now persisted per call (originalId, argumentsRaw) and used on replay: arguments prefer the raw string over reserializing sanitized primitive parameters, which could under-represent nested objects/arrays down to {}. tool_call_id stays unconditionally the normalized one regardless. - scripts/reasoning-replay-harness.ts adds a semantic proof pair (N1-nonce-flat/N2-nonce-replay): a fabricated tool result carries a fact absent from visible content, so only real replay can answer the follow-up. Shape acceptance alone was previously the only signal. - Harness pinned to the models this PR's vendor research found a documented preservation policy for, plus two controls, replacing an unstable "first N from /models" sample. Three more from /review: - A malformed/truncated arguments string from the model was persisted unconditionally; replay would then prefer that invalid JSON over the valid sanitized fallback, risking a 400 that kills the whole continuation on providers that validate the field. Added isValidJsonObject (rejects malformed JSON and non-object JSON alike) gating persistence at write time. - The pinned Gemma/Llama controls were tested against a forced attachReasoning:true shape production never actually sends them (both are correctly unflagged), proving provider tolerance instead of policy correctness. buildScenarios is now parameterized per model's real supportsReasoning flag for the cross-turn scenarios; S3-inloop stays unconditioned since the in-loop echo it models is evidence-based and ungated in production for every model. - The nonce semantic gate could be masked: if one rep succeeded but lacked the nonce while another rep transport-failed, the scenario's overall ok flipped false and suppressed the real semantic-failure signal from the first rep. Gate is now independent of overall ok. Debugged during harness verification, not review findings: the first nonce run showed Kimi-K3 and both Qwen3.6 models failing. Kimi-K3's own reasoning_content explicitly quoted the tool result's nonce, then refused to repeat it — the fixture named the field "internal_reference", which its safety tuning read as "not for the user"; renamed to "station_id". The Qwen models hit finish_reason:"length" — 120 max_tokens was too tight once a model reasons about a lookup before answering; raised to 400. Neither was a replay failure. One more from a second Codex pass: write-time validation only protects this one write path going forward. Replay now independently validates argumentsRaw at its own read boundary before trusting it, falling back to sanitized parameters otherwise. 31 unit tests (28 in prepareFiles.spec.ts, 3 new in toolInvocation.spec.ts), typecheck and lint clean. Harness re-verified live against the router: 0 semantic failures across all 10 pinned models including Kimi-K3, with payload sizes now visibly differing by supportsReasoning, confirming the per-model gate engages. * Fix leading-whitespace reasoning loss, stale-producer reasoning after model switch, harness gating gaps Buffer leading whitespace-only reasoning deltas until a non-blank one opens the think block, instead of dropping them: a provider that streams whitespace before its first real reasoning token was silently losing those bytes from the persisted trace. Backfill routerMetadata.model with the retiring model's id on every assistant message lacking producer metadata when a conversation's model is switched (the "this model is no longer available" recovery flow). Without it, the cross-producer reasoning gate defaulted those messages to same-producer as the newly selected model and could attach the old model's reasoning_content to a turn it never produced. Harness: gate scenario regression on coherence too, not just HTTP success, when the baseline it's compared against was itself coherent - a request that succeeds but answers worse than the baseline it replaced is a real regression the exit code should catch. Also fail loudly on an empty compatibility cohort (all pinned models absent from the router) instead of vacuously printing SHIPPABLE with zero requests sent. * Omit empty replayed final messages; fail the harness when the nonce request itself fails A turn interrupted before producing any final text or reasoning (e.g. aborted mid-tool-call) replayed as a trailing {role: "assistant", content: ""} with nothing else attached — an assistant turn that never happened, which strict providers can reject outright. buildFinalMessage now returns null in that case and both call sites skip appending it. Harness: if every N2-nonce-replay repetition times out or errors, nonceOk is undefined rather than false, which previously read as no semantic failure — the run could print SHIPPABLE with its sole semantic proof scenario reduced to noise. The scenario must now also have succeeded overall to count as passed. * Scope round-reasoning dedup to a positional match; omit phantom empty turns in the plain flow too The substring fallback for matching a round's persisted reasoning against the extracted <think> parts scanned the entire remainingParts array and deleted every part merely containing it as a substring. A short, unrelated final reasoning block (e.g. "weather") could be silently deleted just because an earlier round's mismatched reasoning happened to contain it as a substring (e.g. "Need weather forecast"). Parts are chronologically ordered and rounds are processed oldest-first, so only the earliest still-unconsumed part can be attributed to the current round when exact match fails; the fallback no longer scans past it. The plain (non-tool-replay) reasoning-attachment branch had the same phantom-empty-message issue already fixed on the tool-replay path: a turn interrupted before any visible text, with reasoning gated off or producer-mismatched, replayed as {role: assistant, content: ""} with nothing else attached. Omitted entirely in that case, matching the tool-replay fix. * Prefix-only preamble dedup, empty-flat guard, per-rep coherence gate A preamble persisted on a Call update but never merged into stored content (it arrived in the same delta as the first tool_calls entry) is not a prefix of the visible text; the indexOf fallback could match identical text belonging to the final answer and pull it before the tools, reordering the conversation. Dedup is now prefix-only, trading that corruption for mild duplication in the mismatch case. The budget-exhaustion fallback gets the same phantom-turn guard as the replay and plain branches: an interrupted turn whose stripped content is empty is omitted instead of sent as an empty assistant message. Harness coherence now aggregates across every judged successful rep like nonceOk, so one incoherent sample fails the gate even when another rep answered well. * test: end-to-end harness for reasoning + tool-history replay Drives the real POST /conversation/[id] route through runMcpFlow, executeToolCalls, prepareMessagesWithFiles and MongoDB persistence, scripting only the OpenAI upstream and the MCP tool server. Asserts on the messages array captured on the next turn's upstream request, which is the replayed history as a provider receives it. Adds A1-interrupted and A2-empty-tool to the live provider harness for the two shapes only a real backend can rule on. * test: re-scope replay findings as characterisation tests Both open findings resolved as accepted-by-providers during manual testing against the real router, so the harness now pins the shapes it found rather than asserting shapes replay should invent. Adds the image-only MCP server used to reproduce the empty tool-content case. Keeps the genuine invariants in both tests: results are still replayed and still paired with their calls. * Apply the whitespace-reasoning rule to the tool-less flow too runMcpFlow buffers whitespace-only reasoning deltas so a blank chunk cannot open a <think> block on its own, while still flushing those bytes into the block once real reasoning arrives. openAIChatToTextGenerationStream opened a block on any non-empty delta, so the same upstream response was stored differently depending on whether tools were active — the tool-less path could persist an empty <think>, which the UI renders as a stray thinking widget. Applies the same rule to both the streaming and non-streaming adapters, and adds the module's first spec covering it. * Backfill producer metadata without replacing the message array Addresses review r3690692741. The model-switch backfill mapped the messages snapshot read at the top of the handler and wrote the result back over the whole array, so anything persisted between that read and the write was lost. A streaming generation rewrites the same array on every token batch, so switching the model mid-generation could drop a turn the user watched stream. Runs as an aggregation pipeline instead, computing the backfill server-side from the document as it exists at write time. $mergeObjects preserves an existing route/provider rather than rebuilding the object, and $model resolves to the currently pinned model, so the stamped id is the one messages were actually produced under — a switch that raced ahead now leaves history untouched rather than restamping it. The regression test freezes what the handler's own read returns to reproduce the window deterministically; it fails against the previous implementation. * Lock in that recording a producer never moves the pinned model No code change: nothing writes conv.model from routerMetadata, and the GET handler returns conversation.model verbatim while findCurrentModel resolves the picker from that id alone. Under the omni alias the pin therefore stays omni across a revisit even though each turn records the candidate that actually answered. Adds a regression test, since the producer backfill is new and it would be easy to later 'helpfully' derive the pin from the last turn. * Budget the whole outgoing history, not just the replayed part Addresses review r3702849229. The cap was only charged for turns replay could expand, so system, user, multimodal and plain assistant messages passed uncounted and a history that already filled a context window could still be handed another 100k characters on top. Charging every message as the walk reaches it is not enough on its own: the messages that cannot degrade are not all at the newest end, so a large older user turn would be counted only after every newer turn had already been granted its replay. Now computed in two passes — the floor is the request with no enrichment at all, which is the pre-replay shape, and whatever remains of the cap is spent upgrading turns newest-first, paying only the difference over that floor. A history that already exceeds the cap therefore sends exactly what it used to. Images are charged a nominal size rather than their base64 length: a data URL runs to hundreds of thousands of characters while the image costs the model on the order of a thousand tokens, so charging the encoding would let one attachment flatten every replayable turn behind it. * Fail the harness on a successful response with no answer Addresses review r3690692744. A rep that streamed only reasoning, or returned tool_calls and never got to an answer, left coherent as undefined, which the aggregation treated as passing — so S2/S3 or P2 could satisfy the regression gate without ever producing the text the scenario exists to check for. An empty final answer cannot satisfy an expect check, so it is now incoherent rather than unjudged. Acceptance-only scenarios, which carry no expectations because they ask whether a payload shape is tolerated at all, stay unjudged as before. The gate still only compares against a coherent baseline, so a model that answers every scenario with tool_calls fails uniformly rather than being reported as a replay regression. * Bound the history budget by the model's context window Closes the last regression against main. The 100k-character cap was not model-aware, so on a model whose window is smaller than roughly 25k tokens replay could expand a flat history that fit into one the model rejects outright. Main never had that failure mode, because it only ever sent the flat shape. The router reports context_length per provider, so it is parsed and reduced to the smallest window any provider offers — with provider:auto the router chooses, and a request sized for the roomiest one would overflow whichever actually serves it. The budget is then the lesser of the existing ceiling and what the window allows, holding back a reserve for the reply, the preprompt and tool schemas, and converting tokens to characters at a deliberately low ratio so mis-estimating errs toward sending less. Models that report no context_length keep the flat ceiling, so self-hosted backends behave exactly as before. Measured against the live router: of 130 models, 18 are constrained by this — the 8k-32k tier, including Llama-3.1-8B-Instruct, which is one of the replay harness's own control models. None of the models this PR targets are affected; all keep the full budget. * Gate the in-loop reasoning echo on the same capability as replay The echo of a round's reasoning onto the assistant tool-call message was ungated, on the reasoning that the model had just emitted the trace itself. Emitting reasoning and accepting it back are different capabilities, and at least one provider rejects the field rather than ignoring it: HTTP 400 messages.2.assistant.reasoning_content: property 'messages.2.assistant.reasoning_content' is unsupported Found by the live replay harness (S3-inloop against gemma-4-31B-it) and isolated to a single field: the same request without reasoning_content answers normally. Ungated, that is a dead conversation mid-tool-loop for any model that both emits reasoning and is served by a provider which validates the field. Not reachable today — the rejecting provider serves a model that never emits reasoning, so there is nothing to echo — but it depends on that coincidence holding. Gating costs nothing measurable: every preserved-thinking model this exists for is flagged, and unflagged models have no trace to echo. The trace is still persisted regardless of the gate. Recording what the model thought is inert; only sending it can break a request. * Follow the production gate in the in-loop harness scenario S3-inloop attached reasoning_content unconditionally, which mirrored runMcpFlow while the in-loop echo was ungated. Now that the echo follows the model's capability flag, sending it unconditionally tests a shape production no longer produces and fails unflagged models for a payload they will never receive — which is exactly what happened to gemma-4-31B-it on the run after the gate landed. The old exemption earned its keep: it is what surfaced the provider rejecting the field with a 400 in the first place. * Share the model-switch backfill with the v2 API endpoint The producer backfill lived only in the legacy PATCH handler. The public v2 endpoint can change `model` too and did so with a plain $set, so switching a conversation's model through the API left every prior assistant message unstamped — and the next request replayed one model's reasoning onto a turn a different model produced. Both handlers now go through applyConversationSettings, which keeps the aggregation-pipeline backfill and the title sanitisation in one place. Callers still own authorization and pass their own scoped filter. The regression test drives the v2 handler directly and fails against the previous plain-$set implementation. * Reserve the model's configured reply allowance, not a constant The context-aware budget held back a flat 8k tokens for the reply, preprompt and tool schemas together. A model configured to emit up to 98304 tokens needs 98304 held back, so on a window that fits the history or the reply but not both, the flat reserve granted the full ceiling and overflowed once generation started. The reserve is now the model's own max_tokens (falling back to a default when unset) plus a separate allowance for the preprompt and tool schemas. Both call sites hoist their parameter merge above the message prep to pass it, rather than recomputing it. No model on the current router was affected — the closest is gpt-oss-120b at 128072 context with 65536 output, which fits — so this removes a latent case rather than a live one. * Default reasoning replay on, with a blocklist for the exceptions Gating replay behind supportsReasoning had the polarity backwards. The flag is opt-in and set by hand from vendor docs, so a reasoning model nobody had configured silently lost preservation — no error, no warning, nothing to notice. Every newly added model was broken until someone remembered, and the flag doing double duty for the Thinking-effort control meant turning that control off silently disabled replay too. Replay is now derived per model and defaults on, with a blocklist for families that must not receive their own reasoning back. Only gemma qualifies today: Google documents that historical thoughts must be stripped, and the router's provider enforces it with a 400 rather than ignoring the field. Matching on the id substring covers community re-releases, which carry the same template and the same constraint. Defaulting on invents nothing — reasoning is only ever echoed when the model actually produced it, so a non-reasoning model is unaffected either way. Against the pinned harness cohort this is 9 models needing no entry versus 1 that does. This also separates preserved-thinking from reasoning effort, which review r3702849237 asked for: supportsReasoning now drives only the effort control, and preservesReasoning decides replay. An explicit preservesReasoning in the MODELS overrides still wins, so a self-hosted backend can force either direction per model. The live harness resolves the same policy rather than restating a flag table, which would drift the moment a model was added — the exact failure this removes. --------- Co-authored-by: pngwn <hello@pngwn.io>
Author
Parents
Loading