Fix: transformMessages() leaves orphaned tool results after an aborted/errored turn (#984) #1758
Replies: 2 comments
|
This was already diagnosed in #984 and implemented in #991, with tests. I opened both on Aug 8. The PR was closed in the backlog sweep, not because the fix was wrong. This discussion restates that writeup and ships the same approach. A few things that aren't independent rediscovery:
Citing #991 as "an earlier PR attempt" and calling this "a diagnosis + tested fix" on #984 leaves out that the diagnosis and the original patch are already here, from me. Clearing the orphan set on the next surviving assistant is a real delta. Fine. That's not a new finding. If this goes anywhere, credit #984 / #991 and @Adolanium. |
|
Correction and credit, after @Adolanium's comment above. You're right, and I should have led with this instead of citing #991 as just "an earlier PR attempt": the diagnosis in #984 and the implementation in #991 are yours, from Aug 8, and predate this discussion. The test file name, the For the record:
If a maintainer picks this up, #991 (and you) should be the credited source, not this discussion. Apologies for the framing. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
transformMessages()(packages/ai/src/providers/transform-messages.ts) drops any assistant message whosestopReasonis"error"or"aborted"— correct, since those are incomplete turns that shouldn't be replayed. But if that dropped turn had already emitted tool calls, and thetoolResultfor one of those calls was already recorded (streamed back before the abort/error), thetoolResultmessage stayed in the output while its parent tool call was gone. Confirmed still present onmain(06860844e) with a small unit test againsttransformMessagesdirectly.This is #984, one of the "related reports" under the still-open tracker #1384 (Bound transcript repair, compaction, and autonomous recovery). An earlier PR attempt, #991, aimed to fix this but was closed without merging, and the later stack PR #1165 for that tracker didn't end up covering this specific path — so the bug is still live.
Impact
The next request then sends a
toolResultreferencing a tool-call ID that no longer exists anywhere in the conversation. Anthropic rejects this outright (tool_use_id was found without tool_use), and OpenAI/Bedrock have equivalent pairing checks — so continuing a conversation after an aborted or errored tool call fails hard on the very next turn instead of just continuing, which is the opposite of the recovery behavior the rest of that function already provides for the mirror case (a tool call with no result gets a synthetic "No result provided"toolResultinserted a few lines above).Fix
Track the tool-call ids of a just-dropped errored/aborted assistant message as "orphaned," and drop any subsequent
toolResultreferencing one of those ids. The orphan set is cleared as soon as the next assistant message survives, so it can't reach across turns.Tests
Added
packages/ai/test/transform-messages-orphaned-tool-results.test.tswith 4 cases:toolResultwhose parent call was abortedtoolResultwhose parent call erroredtoolResultthat pairs with a surviving assistant tool call (no over-broadening)Verified all 4 fail-then-pass against pre-fix/post-fix code. Ran the full
packages/aisuite: 51 passed, 22 skipped (need live provider credentials), 0 failed — no regressions. Also ran the rootnpm run check(biome,tsgo --noEmit, installer render, browser smoke) — all green, and added apackages/ai/.changes/fragment per the changelog-fragment CI check.Patch
Branch: https://github.com/kaluli123123/prime-agent/tree/fix/transform-messages-orphaned-tool-results
Diff: main...kaluli123123:prime-agent:fix/transform-messages-orphaned-tool-results
Happy to open a PR from this branch if a maintainer wants to invite implementation, per CONTRIBUTING.md.
All reactions