Skip to content

fix(core): keep reasoning provider metadata on errored turns for valid Anthropic replay - #44054

Open
BigBrown10 wants to merge 1 commit into
anomalyco:devfrom
BigBrown10:fix/errored-turn-anthropic-replay
Open

fix(core): keep reasoning provider metadata on errored turns for valid Anthropic replay#44054
BigBrown10 wants to merge 1 commit into
anomalyco:devfrom
BigBrown10:fix/errored-turn-anthropic-replay

Conversation

@BigBrown10

Copy link
Copy Markdown

Issue for this PR

Closes #38620

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Replaying an errored assistant turn dropped reasoning providerMetadata while still replaying that turn's tool_use parts. With extended thinking enabled, Anthropic requires every tool_use block to be preceded by its signed/redacted thinking block, so any interrupt mid-step poisoned the rest of the session: the next request replayed orphaned tool_use without its thinking block and got a 400.

The fix splits the guard in to-llm-message.ts. Reasoning continuation metadata is now reused whenever the message stays on the same model (sameModel), regardless of message.error, because both the signature and redactedData Anthropic needs live in that one field. Tool execution metadata still requires message.error === undefined, since metadata from failed executions should not be trusted. That restores the thinking-before-tool_use invariant without changing what gets replayed.

The existing test "drops provider-native continuation metadata from failed assistant turns" pinned the broken shape, so I renamed it and updated its expectation deliberately (the issue notes the old guard was itself deliberate).

How did you verify your code works?

  • Updated/added tests in packages/core/test/session-runner-message.test.ts: same-model failed turns keep openai metadata; anthropic signature preserved on a failed turn; redactedData preserved on a failed turn; cross-model stripping still drops metadata.
  • bun test in packages/core: 8/8 pass in session-runner-message.test.ts; full package suite is 1083 pass / 7 fail, and those 7 (npm-config, modelsdev, cross-spawn) also fail on a clean dev checkout, so they are unrelated environment failures.
  • tsgo --noEmit passes.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  1. packages/core/src/session/runner/to-llm-message.ts:88 — The sameModel gate disappeared entirely for reasoning parts: providerMetadata: item.providerMetadata is now unconditional, so a model-switch retry (e.g. fallback model) replays foreign anthropic.signature /openai.itemId metadata. Why it matters: signatures/item IDs are model-scoped, so the retried request can be rejected with a 400 — recreating the exact failure class (V2: errored-message replay can 400 with Anthropic thinking + tool use #38620) this PR fixes, just via the fallback path. Suggestion: keep a dedicated gate for reasoning, e.g. providerMetadata: sameModel ? item.providerMetadata : undefined (error-agnostic, per the new intent), while leaving reuseToolMetadata as-is.

  2. packages/core/test/session-runner-message.test.ts:403 — There is no test pinning reasoning-metadata behavior across a model switch; the existing "drops provider-native continuation metadata after a model switch" test only guards tool-call metadata. Why it matters: item 1's behavior is currently unspecified and could flip silently. Suggestion: add a case asserting reasoning metadata is dropped (or kept, once decided) after a provider/model change.

  3. packages/core/src/session/runner/to-llm-message.ts:74 — The new comment states metadata "must be reused whenever the turn stays on the same model," but the code reuses it even when the turn does not stay on the same model. Comment/code mismatch will mislead the next reader. Suggestion: align wording with actual behavior (or fix behavior per item 1).

  4. packages/core/src/session/runner/to-llm-message.ts:87-92 (nit) — The three parallel reuseToolMetadata ? … : undefined ternaries could collapse into one toolMeta = (m) => (reuseToolMetadata ? m : undefined) helper for readability.

— automated review (ox-alpha, round2)

@BigBrown10

Copy link
Copy Markdown
Author

Thanks for the automated review. Items 1 and 3 are based on a misreading of the diff, item 2 is already covered, and I'm declining the nit to keep the diff minimal.

Items 1 & 3 (sameModel gate "disappeared"): The sameModel ternary still wraps the entire reasoning return; only the inner reuseProviderMetadata ? was removed:

if (item.type === "reasoning")
  return sameModel
    ? [{ type: "reasoning", text: item.text, providerMetadata: item.providerMetadata }]
    : item.text.length > 0
      ? [{ type: "text", text: item.text }]
      : []

On a model-switch/fallback retry (sameModel === false) the reasoning part becomes a plain text part or is dropped entirely — there is no code path where foreign metadata reaches the wire, so the suggested providerMetadata: sameModel ? ... : undefined would be behaviorally identical to what's already committed. The comment matches the code.

Item 2 (model-switch coverage): Already pinned by the existing test drops provider-native continuation metadata after a model switch, which builds a reasoning part carrying anthropic: { signature: "sig_old" } on the old model and asserts the replayed content is exactly { type: "text", text: "Visible thought" } — i.e. no reasoning part, no providerMetadata survives the switch.

Item 4 (helper nit): Declining for now; it touches three call sites to save one expression each in a PR that's intentionally minimal while competing with several open approaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V2: errored-message replay can 400 with Anthropic thinking + tool use

2 participants