Skip to content

Commit 92ab017

Browse files
committed
fix: keep hasToolResult record and replay symmetric, fix Anthropic tool_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).
1 parent 50bc447 commit 92ab017

6 files changed

Lines changed: 443 additions & 38 deletions

File tree

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
import { describe, it, expect, afterEach } from "vitest";
2+
import * as http from "node:http";
3+
import * as fs from "node:fs";
4+
import * as os from "node:os";
5+
import * as path from "node:path";
6+
import type { Fixture, FixtureFile } from "../types.js";
7+
import { createServer, type ServerInstance } from "../server.js";
8+
9+
// ---------------------------------------------------------------------------
10+
// HTTP helper
11+
// ---------------------------------------------------------------------------
12+
13+
function post(
14+
url: string,
15+
body: unknown,
16+
headers?: Record<string, string>,
17+
): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> {
18+
return new Promise((resolve, reject) => {
19+
const data = JSON.stringify(body);
20+
const parsed = new URL(url);
21+
const req = http.request(
22+
{
23+
hostname: parsed.hostname,
24+
port: parsed.port,
25+
path: parsed.pathname,
26+
method: "POST",
27+
headers: {
28+
"Content-Type": "application/json",
29+
"Content-Length": Buffer.byteLength(data),
30+
...headers,
31+
},
32+
},
33+
(res) => {
34+
const chunks: Buffer[] = [];
35+
res.on("data", (c: Buffer) => chunks.push(c));
36+
res.on("end", () =>
37+
resolve({
38+
status: res.statusCode ?? 0,
39+
headers: res.headers,
40+
body: Buffer.concat(chunks).toString(),
41+
}),
42+
);
43+
},
44+
);
45+
req.on("error", reject);
46+
req.write(data);
47+
req.end();
48+
});
49+
}
50+
51+
// ---------------------------------------------------------------------------
52+
// Server lifecycle
53+
// ---------------------------------------------------------------------------
54+
55+
let upstream: ServerInstance | undefined;
56+
let recorder: ServerInstance | undefined;
57+
let replay: ServerInstance | undefined;
58+
let tmpDir: string | undefined;
59+
60+
afterEach(async () => {
61+
for (const s of [replay, recorder, upstream]) {
62+
if (s) await new Promise<void>((resolve) => s.server.close(() => resolve()));
63+
}
64+
replay = recorder = upstream = undefined;
65+
if (tmpDir) {
66+
fs.rmSync(tmpDir, { recursive: true, force: true });
67+
tmpDir = undefined;
68+
}
69+
});
70+
71+
/** Read every recorded fixture out of a record-mode fixture directory. */
72+
function readRecordedFixtures(fixturePath: string): Fixture[] {
73+
const out: Fixture[] = [];
74+
const walk = (dir: string) => {
75+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
76+
const full = path.join(dir, entry.name);
77+
if (entry.isDirectory()) walk(full);
78+
else if (entry.name.endsWith(".json")) {
79+
const parsed = JSON.parse(fs.readFileSync(full, "utf-8")) as FixtureFile;
80+
out.push(...(parsed.fixtures ?? []));
81+
}
82+
}
83+
};
84+
walk(fixturePath);
85+
return out;
86+
}
87+
88+
// ---------------------------------------------------------------------------
89+
// Finding 1 — recorder/matcher record→replay symmetry (OpenAI shape)
90+
// ---------------------------------------------------------------------------
91+
92+
describe("hasToolResult record→replay symmetry", () => {
93+
it("replays a genuinely-recorded turn-2 leg-1 request against its own recorded fixture", async () => {
94+
// Turn-2 leg-1: a FRESH user question whose history still carries turn-1's
95+
// completed tool result. Shape: [user, assistant(tool_call), tool, user].
96+
// Current-turn hasToolResult is FALSE (nothing after the last user message),
97+
// but the whole conversation DOES contain a tool message. Pre-fix the
98+
// recorder stamped `true` (whole-conversation) while the matcher checked
99+
// `false` (turn-scoped) → the fixture could never match its own request.
100+
const turn2Leg1 = {
101+
model: "gpt-4",
102+
messages: [
103+
{ role: "user", content: "first question" },
104+
{
105+
role: "assistant",
106+
content: null,
107+
tool_calls: [
108+
{
109+
id: "call_1",
110+
type: "function",
111+
function: { name: "lookup", arguments: "{}" },
112+
},
113+
],
114+
},
115+
{ role: "tool", content: "tool output", tool_call_id: "call_1" },
116+
{ role: "user", content: "second question" },
117+
],
118+
};
119+
120+
// 1. Upstream mock returns an answer for the second question.
121+
upstream = await createServer(
122+
[
123+
{
124+
match: { userMessage: "second question" },
125+
response: { content: "Answer to the second question." },
126+
},
127+
],
128+
{ port: 0, logLevel: "silent" },
129+
);
130+
131+
// 2. Recorder proxies the turn-2 leg-1 request to upstream and records it.
132+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-sym-"));
133+
recorder = await createServer([], {
134+
port: 0,
135+
logLevel: "silent",
136+
record: { providers: { openai: upstream.url }, fixturePath: tmpDir },
137+
});
138+
const recResp = await post(`${recorder.url}/v1/chat/completions`, turn2Leg1, {
139+
"x-test-id": "symmetry",
140+
});
141+
expect(recResp.status).toBe(200);
142+
143+
// 3. Replay the SAME request against the just-recorded fixtures.
144+
const fixtures = readRecordedFixtures(tmpDir);
145+
expect(fixtures.length).toBeGreaterThan(0);
146+
replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true });
147+
const replayResp = await post(`${replay.url}/v1/chat/completions`, turn2Leg1);
148+
149+
// Pre-fix: 503 "No fixture matched" (recorded true vs matched false).
150+
// Post-fix: matches and serves the recorded answer.
151+
expect(replayResp.status).toBe(200);
152+
expect(replayResp.body).toContain("Answer to the second question.");
153+
});
154+
155+
it("replays an Anthropic leg-2 turn that bundles tool_result WITH accompanying text", async () => {
156+
// Anthropic packs a leg-2 tool_result and accompanying user text into ONE
157+
// user message. Normalization must emit `[..., user(text), tool]` so the tool
158+
// trails the last user message and the turn is classified as carrying a tool
159+
// result (`true`). Pre-fix it emitted `[..., tool, user(text)]`, byte-identical
160+
// to a fresh leg-1 turn, so the recorded fixture (whole-conversation `true`)
161+
// could never match the turn-scoped `false` the matcher computed on replay.
162+
const leg2WithText = {
163+
model: "claude-3-5-sonnet-20241022",
164+
max_tokens: 1024,
165+
messages: [
166+
{ role: "user", content: "initial question" },
167+
{
168+
role: "assistant",
169+
content: [{ type: "tool_use", id: "toolu_1", name: "lookup", input: {} }],
170+
},
171+
{
172+
role: "user",
173+
content: [
174+
{ type: "tool_result", tool_use_id: "toolu_1", content: "tool data" },
175+
{ type: "text", text: "please summarize the tool result" },
176+
],
177+
},
178+
],
179+
};
180+
181+
// Upstream matches on the accompanying text (the last user message).
182+
upstream = await createServer(
183+
[
184+
{
185+
match: { userMessage: "please summarize the tool result" },
186+
response: { content: "Recorded narration answer." },
187+
},
188+
],
189+
{ port: 0, logLevel: "silent" },
190+
);
191+
192+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-anthropic-"));
193+
recorder = await createServer([], {
194+
port: 0,
195+
logLevel: "silent",
196+
record: { providers: { anthropic: upstream.url }, fixturePath: tmpDir },
197+
});
198+
const recResp = await post(`${recorder.url}/v1/messages`, leg2WithText, {
199+
"x-test-id": "anthropic-leg2-text",
200+
});
201+
expect(recResp.status).toBe(200);
202+
203+
const fixtures = readRecordedFixtures(tmpDir);
204+
expect(fixtures.length).toBeGreaterThan(0);
205+
// The recorded fixture must carry the turn-scoped classification (true).
206+
expect(fixtures.some((f) => f.match.hasToolResult === true)).toBe(true);
207+
208+
replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true });
209+
const replayResp = await post(`${replay.url}/v1/messages`, leg2WithText);
210+
211+
expect(replayResp.status).toBe(200);
212+
expect(replayResp.body).toContain("Recorded narration answer.");
213+
});
214+
});

src/__tests__/messages.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -854,12 +854,16 @@ describe("claudeToCompletionRequest (fallback branches)", () => {
854854
},
855855
],
856856
});
857-
// Should produce tool message + user message
857+
// Should produce user message (accompanying text) THEN tool message. The
858+
// text is emitted FIRST so the tool message trails the last user message and
859+
// the current-turn hasToolResult predicate classifies this leg-2 turn as
860+
// carrying a tool result (`true`). Emitting the tool first would make this
861+
// byte-identical to a genuine leg-1 turn and misclassify it `false`.
858862
expect(result.messages).toHaveLength(2);
859-
expect(result.messages[0].role).toBe("tool");
860-
expect(result.messages[0].content).toBe("result data");
861-
expect(result.messages[1].role).toBe("user");
862-
expect(result.messages[1].content).toBe("follow up question");
863+
expect(result.messages[0].role).toBe("user");
864+
expect(result.messages[0].content).toBe("follow up question");
865+
expect(result.messages[1].role).toBe("tool");
866+
expect(result.messages[1].content).toBe("result data");
863867
});
864868

865869
it("handles text content blocks with missing text (text ?? '')", () => {
@@ -1086,8 +1090,12 @@ describe("claudeToCompletionRequest (fallback branches)", () => {
10861090
},
10871091
],
10881092
});
1089-
expect(result.messages[1].role).toBe("user");
1090-
expect(result.messages[1].content).toBe("");
1093+
// Accompanying text is emitted BEFORE the tool message (see the ordering
1094+
// rationale in claudeToCompletionRequest); a missing text field → "".
1095+
expect(result.messages[0].role).toBe("user");
1096+
expect(result.messages[0].content).toBe("");
1097+
expect(result.messages[1].role).toBe("tool");
1098+
expect(result.messages[1].content).toBe("result");
10911099
});
10921100

10931101
it("handles system content blocks with text ?? '' in filter/map", () => {

0 commit comments

Comments
 (0)