Skip to content

fix: keep hasToolResult record/replay symmetric + Anthropic tool_result+text scoping#319

Merged
jpr5 merged 4 commits into
mainfrom
fix/hastoolresult-record-replay-symmetry
Jul 20, 2026
Merged

fix: keep hasToolResult record/replay symmetric + Anthropic tool_result+text scoping#319
jpr5 merged 4 commits into
mainfrom
fix/hastoolresult-record-replay-symmetry

Conversation

@jpr5

@jpr5 jpr5 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Problem

PR #316 (released as v1.37.3) scoped the replay-side hasToolResult check to the current turn (messages after the last user message) but left two paths inconsistent, so v1.37.3 can silently produce fixtures that never match their own recorded request.

Root cause

1. Record/replay split-brain. The matcher (src/router.ts) was changed to a turn-scoped check, but the recorder (src/recorder.ts, buildFixtureMatch) still STAMPED hasToolResult with a whole-conversation predicate messages.some(m => m.role === "tool"). Both operate on the same normalized ChatCompletionRequest.messages, so for a genuinely-recorded turn-2 leg-1 request [user, assistant, tool, user] (a fresh question whose history still carries turn-1's tool result):

  • Recorder writes hasToolResult: true (whole conversation has a tool).
  • Matcher on replay computes false (nothing after the last user) → true !== false → the fixture can never match its own recorded request. The comment at recorder.ts even claimed the value was "matcher-aware" — it no longer was.

2. Anthropic tool_result + text ordering. Anthropic bundles a leg-2 tool_result and accompanying user text into ONE user message. Normalization emitted [..., tool, user(text)], which is byte-identical to a genuine leg-1 turn, so the turn-scoped check saw no tool after the last user and misclassified a real leg-2 turn as false. Because the two shapes are indistinguishable after normalization, this can only be fixed where the source-message grouping is still known — in normalization, not in the shared helper.

Fix

  • Extracted one shared currentTurnHasToolResult(messages) helper in src/router.ts (where the other message-shape utilities live and which recorder.ts already imports) and used it on both the record side and the match side. Sharing a single predicate makes the two sides structurally incapable of drifting. Replaced the false "matcher-aware" comment.
  • Fixed claudeToCompletionRequest to emit the accompanying text user message before the tool message(s), so a real Anthropic leg-2-with-text turn normalizes to [..., user(text), tool] and is classified true, while a genuine leg-1 turn (separate messages) stays false. The common tool_result-only follow-up is unchanged.

Red-green proof

Both defects are proven on the real HTTP record→replay surface (spin up a mock upstream, record through the real recorder proxy to disk, replay the recorded fixtures through the real server/matcher). Source fixes were git stash-ed to observe RED, then restored for GREEN — tests unchanged between runs.

RED (source fixes reverted)

FAIL  hasToolResult record→replay symmetry > replays a genuinely-recorded turn-2 leg-1 request against its own recorded fixture
AssertionError: expected 503 to be 200 // Object.is equality
- Expected  200
+ Received  503

FAIL  hasToolResult record→replay symmetry > replays an Anthropic leg-2 turn that bundles tool_result WITH accompanying text
AssertionError: expected 503 to be 200 // Object.is equality
- Expected  200
+ Received  503

FAIL  claudeToCompletionRequest > handles tool_result with text blocks alongside in same user message
AssertionError: expected 'tool' to be 'user' // Object.is equality
Expected: "user"
Received: "tool"

(503 = strict-mode "No fixture matched": the recorded fixture could not match its own request.)

GREEN (source fixes restored, tests unchanged)

✓ src/__tests__/hastoolresult-record-replay-symmetry.test.ts (2 tests)
✓ src/__tests__/router.test.ts (144 tests)
✓ src/__tests__/messages.test.ts (73 tests)
✓ src/__tests__/thinking-invariants.test.ts (47 tests)
Test Files  4 passed (4)
     Tests  266 passed (266)

Both the OpenAI split-brain (Finding 1) and the Anthropic tool_result+text case (Finding 2) are covered by the real-path HTTP record→replay test, so both got a genuine RED→GREEN on the actual failure surface (not tests-against-fakes).

Tests added

  • src/__tests__/hastoolresult-record-replay-symmetry.test.ts — full HTTP record→replay for (a) an OpenAI turn-2 leg-1 request and (b) an Anthropic tool_result+text leg-2 turn; both would 503 pre-fix.
  • router.test.ts — direct currentTurnHasToolResult coverage (leg-2 true; new-turn turn-scoped false; leading-system-message; lastUserIdx === -1 no-user whole-conversation fallback), plus matcher composition of the scoping with explicit turnIndex and sequenceIndex gates, and a system-message-led new-turn case.
  • messages.test.ts — the two existing tool_result+text tests that encoded the old (buggy) tool-before-text order were corrected to the fixed order.

Full suite

pnpm format:check, pnpm lint, pnpm build all clean. pnpm test: 4683 passed, 1 pre-existing wall-clock flake (timing-replay.test.ts > replaySpeed 2.0 halves the replay duration, expected 209 to be less than 200) that passes in isolation and is unrelated to this change.

Notes

No version bump and no changeset (repo has no .changeset/; releases are cut by a separate release PR). This completes the incomplete fix from #316.

🤖 Generated with Claude Code

…ol_result+text scoping

PR #316 scoped the replay-side hasToolResult check to the current turn
(messages after the last user message) but left the record side and one
Anthropic normalization case inconsistent, so v1.37.3 can silently produce
fixtures that never match their own recorded request.

- Extract one shared currentTurnHasToolResult(messages) helper in router.ts and
  use it on BOTH the record side (recorder.ts buildFixtureMatch, which previously
  stamped a whole-conversation messages.some(role==="tool")) and the match side
  (router.ts matchFixtureDiagnostic). A genuinely-recorded turn-2 leg-1 request
  was stamped true (whole-conversation) but matched false (turn-scoped), so its
  fixture could never replay. Sharing one predicate makes drift impossible. Also
  replaces the now-false "matcher-aware" comment in recorder.ts.

- Fix Anthropic tool_result-with-text normalization: a user message bundling a
  tool_result WITH text normalized to [..., tool, user(text)], byte-identical to
  a genuine leg-1 turn, so a real leg-2 turn was misclassified false. Emit the
  accompanying text BEFORE the tool message so the tool trails the last user
  message and the turn is correctly classified true. This is the only place the
  source-message grouping is known, so it cannot be fixed in the shared helper.

Proven red->green on the real HTTP record->replay surface (see PR body).
@pkg-pr-new

pkg-pr-new Bot commented Jul 20, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@copilotkit/aimock@319

commit: e192e5b

jpr5 added 3 commits July 20, 2026 10:22
The symmetry test hand-rolled a fixture reader that pushed raw
FixtureFileEntry values (from the parsed FixtureFile) where createServer
expects Fixture — a genuine TS2345 (FixtureFileTextResponse.content can be
object/array, not just string). aimock's CI never runs tsc and tsconfig
excludes src/__tests__, so the defect was invisible under vitest/esbuild.

Replace the hand-rolled reader with the project's own loadFixturesFromDir,
which runs entryToFixture to normalize each FixtureFileResponse into a proper
FixtureResponse and returns Fixture[]. No behavior change; the two record→replay
tests stay green. tsc --noEmit is now clean for this file.
…esult

Record/replay symmetry only holds if both buildFixtureMatch (set side) and
matchFixtureDiagnostic (check side) feed this helper the SAME normalized
(post-requestTransform) messages. Document that invariant so a future caller
passing raw/untransformed messages can't silently reopen the multi-turn bug.
No logic change.
- CHANGELOG: add [1.37.4] (record/replay symmetry via shared
  currentTurnHasToolResult helper; Anthropic tool_result+text ordering) and
  the previously-undocumented [1.37.3] (behavior change: hasToolResult scoped
  to the current turn, with a migration note).
- skills/write-fixtures/SKILL.md (mirrored by the .claude/commands symlink):
  reword the hasToolResult match-table row and the "Best For" row to current-turn
  scoping, and add a caveat to the tool-result predicate gotcha that a
  whole-conversation role==="tool" check diverges from hasToolResult in
  multi-turn flows.
- src/types.ts: add TSDoc on both hasToolResult fields describing the
  current-turn scoping and the no-user-message whole-conversation fallback.

Docs only; no behavior change.
@jpr5
jpr5 marked this pull request as ready for review July 20, 2026 18:41
@jpr5
jpr5 merged commit 922afee into main Jul 20, 2026
23 checks passed
@jpr5
jpr5 deleted the fix/hastoolresult-record-replay-symmetry branch July 20, 2026 18:42
@jpr5 jpr5 mentioned this pull request Jul 20, 2026
jpr5 added a commit that referenced this pull request Jul 20, 2026
Release v1.37.4. Bumps `package.json` to publish the fix from #319 to
npm + ghcr.

## What ships
- **#319** — `hasToolResult` record/replay symmetry via shared
`currentTurnHasToolResult` helper (recorder previously stamped
whole-conversation while the matcher read current-turn, so
genuinely-recorded multi-turn fixtures could never match), plus
Anthropic `tool_result`+text ordering fix. Completes the incomplete #316
/ v1.37.3 fix.
- CHANGELOG entries for 1.37.4 and the retroactive 1.37.3 (which shipped
undocumented) already landed in #319.

Consumer: unblocks CopilotKit showcase multi-turn recording once bumped
to 1.37.4.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

1 participant