From 6dad6a746987ea65bcf360b9881a11afcb39244c Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 4 Aug 2026 19:22:45 +0200 Subject: [PATCH] test: normalize legacy view replay output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e1a2b0c-e763-4417-80e3-d78fcce79925 --- test/harness/replayingCapiProxy.test.ts | 269 +++++++++++++++++++++++- test/harness/replayingCapiProxy.ts | 213 +++++++++++++++---- 2 files changed, 445 insertions(+), 37 deletions(-) diff --git a/test/harness/replayingCapiProxy.test.ts b/test/harness/replayingCapiProxy.test.ts index c5747a3067..df9d66b375 100644 --- a/test/harness/replayingCapiProxy.test.ts +++ b/test/harness/replayingCapiProxy.test.ts @@ -11,7 +11,7 @@ import type { } from "openai/resources/chat/completions"; import os from "os"; import path from "path"; -import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import yaml from "yaml"; import { NormalizedData, @@ -724,6 +724,134 @@ Always include PINEAPPLE_COCONUT_42. }); } + async function replayToolResult({ + storedContent, + requestContent, + toolName = "view", + viewRange, + }: { + storedContent?: string; + requestContent?: string; + toolName?: string; + viewRange?: [number, number]; + }): Promise<{ status: number; body: string }> { + const storedArguments = JSON.stringify({ + path: `${workingDirPlaceholder}/test.txt`, + ...(viewRange ? { view_range: viewRange } : {}), + }); + const requestArguments = JSON.stringify({ + path: `${workDir}/test.txt`, + ...(viewRange ? { view_range: viewRange } : {}), + }); + const cachePath = path.join(tempDir, "cache.yaml"); + const cacheContent = yaml.stringify({ + models: ["test-model"], + conversations: [ + { + messages: [ + { role: "system", content: "${system}" }, + { role: "user", content: "Read file" }, + { + role: "assistant", + tool_calls: [ + { + id: "toolcall_0", + type: "function", + function: { + name: toolName, + arguments: storedArguments, + }, + }, + ], + }, + { + role: "tool", + tool_call_id: "toolcall_0", + ...(storedContent === undefined + ? {} + : { content: storedContent }), + }, + { role: "assistant", content: "Done" }, + ], + }, + ], + } satisfies NormalizedData); + await writeFile(cachePath, cacheContent); + + const proxy = new ReplayingCapiProxy( + "http://localhost:9999", + cachePath, + workDir, + ); + const proxyUrl = await proxy.start(); + + try { + return await makeRequest(proxyUrl, "/chat/completions", { + body: { + model: "test-model", + messages: [ + { role: "system", content: "System prompt" }, + { role: "user", content: "Read file" }, + { + role: "assistant", + tool_calls: [ + { + id: "runtime-call-id", + type: "function", + function: { + name: toolName, + arguments: requestArguments, + }, + }, + ], + }, + { + role: "tool", + tool_call_id: "runtime-call-id", + ...(requestContent === undefined + ? {} + : { content: requestContent }), + }, + ], + }, + }); + } finally { + await proxy.stop(); + } + } + + async function expectToolResultMismatch( + options: Parameters[0], + ) { + const previousGitHubActions = process.env.GITHUB_ACTIONS; + const stderrWrite = vi + .spyOn(process.stderr, "write") + .mockImplementation(() => true); + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + process.env.GITHUB_ACTIONS = "true"; + try { + expect((await replayToolResult(options)).status).toBe(500); + expect(stderrWrite).toHaveBeenCalledWith( + expect.stringContaining( + "No cached response found for POST /chat/completions.", + ), + ); + expect(stderrWrite).toHaveBeenCalledWith( + expect.stringContaining("mismatch at message 3"), + ); + } finally { + stderrWrite.mockRestore(); + consoleError.mockRestore(); + if (previousGitHubActions === undefined) { + delete process.env.GITHUB_ACTIONS; + } else { + process.env.GITHUB_ACTIONS = previousGitHubActions; + } + } + } + test("returns cached response when request matches prefix", async () => { const cachePath = path.join(tempDir, "cache.yaml"); const cacheContent = yaml.stringify({ @@ -902,6 +1030,145 @@ Always include PINEAPPLE_COCONUT_42. } }); + const truncationNotice = + "[Output truncated. Use view_range=[4, ...] to continue reading.]"; + const viewResultCases: Array<{ + description: string; + numberedContent: string; + unnumberedContent?: string; + viewRange?: [number, number]; + }> = [ + { + description: "ordinary content", + numberedContent: "1. alpha\n2. beta", + unnumberedContent: "alpha\nbeta", + }, + { + description: "intrinsically numbered file content", + numberedContent: "1. 1. first\n2. 2. second", + unnumberedContent: "1. first\n2. second", + }, + { + description: "JSON content", + numberedContent: '1. {\n2. "b": 2,\n3. "a": 1\n4. }', + unnumberedContent: '{"a":1,"b":2}', + }, + { + description: "view_range offset", + numberedContent: "2. line2\n3. line3\n4. line4", + unnumberedContent: "line2\nline3\nline4", + viewRange: [2, 4], + }, + { + description: "trailing empty line", + numberedContent: "1. alpha\n2. beta\n3.", + unnumberedContent: "alpha\nbeta", + }, + { + description: "blank and spaces before a truncation notice", + numberedContent: `1. alpha\n2. \n3. \n \t\n${truncationNotice}`, + unnumberedContent: `alpha\n \n\n \t\n${truncationNotice}`, + }, + { + description: "empty result", + numberedContent: "1.", + unnumberedContent: undefined, + }, + ]; + + test.each( + viewResultCases.flatMap( + ({ description, numberedContent, unnumberedContent, viewRange }) => [ + { + description: `${description}, numbered snapshot`, + storedContent: numberedContent, + requestContent: unnumberedContent, + viewRange, + }, + { + description: `${description}, numbered request`, + storedContent: unnumberedContent, + requestContent: numberedContent, + viewRange, + }, + ], + ), + )( + "matches equivalent view results with $description", + async ({ storedContent, requestContent, viewRange }) => { + const response = await replayToolResult({ + storedContent, + requestContent, + viewRange, + }); + expect(response.status).toBe(200); + expect( + (JSON.parse(response.body) as ChatCompletion).choices[0].message + .content, + ).toBe("Done"); + }, + ); + + test("preserves exact matches with multiple numbering layers", async () => { + const response = await replayToolResult({ + storedContent: "1. 1. alpha", + requestContent: "1. 1. alpha", + }); + expect(response.status).toBe(200); + }); + + test("does not remove multiple numbering layers to find a match", async () => { + await expectToolResultMismatch({ + storedContent: "1. 1. alpha", + requestContent: "alpha", + }); + }); + + test("preserves numbered results from non-view tools", async () => { + const requestBody = JSON.stringify({ + messages: [ + { role: "user", content: "List items" }, + { + role: "assistant", + tool_calls: [ + { + id: "tc1", + type: "function", + function: { name: "list_items", arguments: "{}" }, + }, + ], + }, + { + role: "tool", + tool_call_id: "tc1", + content: "1. first\n2. second", + }, + ], + }); + const responseBody = JSON.stringify({ + choices: [{ message: { role: "assistant", content: "Done" } }], + }); + + const outputPath = await createProxy([ + { url: "/chat/completions", requestBody, responseBody }, + ]); + + const result = await readYamlOutput(outputPath); + expect( + result.conversations[0].messages.find( + (message) => message.role === "tool", + )?.content, + ).toBe("1. first\n2. second"); + }); + + test("does not apply view compatibility to non-view tool results", async () => { + await expectToolResultMismatch({ + toolName: "list_items", + storedContent: "1. first\n2. second", + requestContent: "first\nsecond", + }); + }); + test("matches available-tools results after the built-in tool set changes", async () => { const cachePath = path.join(tempDir, "cache.yaml"); // Legacy snapshot recorded before write_agent was a built-in tool: the diff --git a/test/harness/replayingCapiProxy.ts b/test/harness/replayingCapiProxy.ts index 47ebda9f7b..97f45ae2ba 100644 --- a/test/harness/replayingCapiProxy.ts +++ b/test/harness/replayingCapiProxy.ts @@ -652,6 +652,154 @@ async function writeCapturesToDisk( } } +function normalizedMessageSequencesEqual( + requestMessages: NormalizedMessage[], + savedMessages: NormalizedMessage[], +): boolean { + return ( + requestMessages.length === savedMessages.length && + requestMessages.every((_, index) => + normalizedMessagesEqual(requestMessages, savedMessages, index), + ) + ); +} + +function normalizedMessagesEqual( + requestMessages: NormalizedMessage[], + savedMessages: NormalizedMessage[], + index: number, +): boolean { + const requestMessage = requestMessages[index]; + const savedMessage = savedMessages[index]; + if (JSON.stringify(requestMessage) === JSON.stringify(savedMessage)) { + return true; + } + + if ( + requestMessage.role !== "tool" || + savedMessage.role !== "tool" || + !requestMessage.tool_call_id || + requestMessage.tool_call_id !== savedMessage.tool_call_id + ) { + return false; + } + + const { content: requestContent, ...requestRest } = requestMessage; + const { content: savedContent, ...savedRest } = savedMessage; + if (JSON.stringify(requestRest) !== JSON.stringify(savedRest)) { + return false; + } + + const requestToolCall = findPrecedingToolCall( + requestMessages, + index, + requestMessage.tool_call_id, + ); + const savedToolCall = findPrecedingToolCall( + savedMessages, + index, + savedMessage.tool_call_id, + ); + if ( + requestToolCall?.function?.name !== "view" || + savedToolCall?.function?.name !== "view" + ) { + return false; + } + + if (typeof requestContent === "string") { + const strippedRequest = stripLegacyViewNumbering( + requestContent, + requestToolCall.function.arguments, + ); + if (strippedRequest !== null && strippedRequest === savedContent) { + return true; + } + } + + if (typeof savedContent === "string") { + const strippedSaved = stripLegacyViewNumbering( + savedContent, + savedToolCall.function.arguments, + ); + if (strippedSaved !== null && strippedSaved === requestContent) { + return true; + } + } + + return false; +} + +function findPrecedingToolCall( + messages: NormalizedMessage[], + messageIndex: number, + toolCallId: string, +): NormalizedToolCall | undefined { + for (let index = messageIndex - 1; index >= 0; index--) { + const toolCall = messages[index].tool_calls?.find( + (candidate) => candidate.id === toolCallId, + ); + if (toolCall) { + return toolCall; + } + } + return undefined; +} + +/** + * Legacy compatibility for CLI `view` results recorded before `N. ` prefixes + * were removed. Strips one layer only when every content line is consecutively numbered. + */ +function stripLegacyViewNumbering( + content: string, + argumentsJson: string, +): string | undefined | null { + let firstLineNumber = 1; + try { + const args = JSON.parse(argumentsJson) as { view_range?: unknown }; + const rangeStart = Array.isArray(args.view_range) + ? args.view_range[0] + : undefined; + if (typeof rangeStart === "number" && Number.isInteger(rangeStart)) { + firstLineNumber = rangeStart; + } + } catch { + return null; + } + + const lines = content.split("\n"); + const truncationNoticeIndex = lines.findIndex((line) => + line.trimStart().startsWith("[Output truncated."), + ); + let numberedLineCount = + truncationNoticeIndex >= 0 ? truncationNoticeIndex : lines.length; + while ( + truncationNoticeIndex >= 0 && + numberedLineCount > 0 && + lines[numberedLineCount - 1].trim() === "" + ) { + numberedLineCount--; + } + if (numberedLineCount === 0) { + return null; + } + + const strippedLines = [...lines]; + for (let index = 0; index < numberedLineCount; index++) { + const prefix = `${firstLineNumber + index}.`; + const line = strippedLines[index]; + if (line === prefix) { + strippedLines[index] = ""; + } else if (line.startsWith(`${prefix} `)) { + strippedLines[index] = line.slice(prefix.length + 1); + } else { + return null; + } + } + + return normalizeToolMessageContent(strippedLines.join("\n")) || undefined; +} + /** * Produces a human-readable explanation of why no stored conversation matched * a given request. For each stored conversation it reports the first reason @@ -687,7 +835,7 @@ function diagnoseMatchFailure( // Find the first message that doesn't match let mismatchIndex = -1; for (let i = 0; i < requestMessages.length; i++) { - if (JSON.stringify(requestMessages[i]) !== JSON.stringify(saved[i])) { + if (!normalizedMessagesEqual(requestMessages, saved, i)) { mismatchIndex = i; break; } @@ -827,12 +975,7 @@ async function findSavedChatCompletionError( if (error.model && error.model !== requestModel) { continue; } - if ( - requestMessages.length === error.messages.length && - requestMessages.every( - (msg, i) => JSON.stringify(msg) === JSON.stringify(error.messages[i]), - ) - ) { + if (normalizedMessageSequencesEqual(requestMessages, error.messages)) { return error; } } @@ -857,11 +1000,7 @@ async function isRequestOnlySnapshot( for (const conversation of storedData.conversations) { if ( - requestMessages.length === conversation.messages.length && - requestMessages.every( - (msg, i) => - JSON.stringify(msg) === JSON.stringify(conversation.messages[i]), - ) + normalizedMessageSequencesEqual(requestMessages, conversation.messages) ) { return true; } @@ -1201,27 +1340,7 @@ function transformOpenAIRequestMessage( } content = parts.join("\n") || undefined; } else if (m.role === "tool" && typeof m.content === "string") { - // If it's a JSON tool call result, normalize the whitespace and property ordering. - // For successful tool results wrapped in {resultType, textResultForLlm}, unwrap to - // just the inner value so snapshots stay stable across envelope format changes. - try { - const parsed = JSON.parse(m.content); - if ( - parsed && - typeof parsed === "object" && - parsed.resultType === "success" && - "textResultForLlm" in parsed - ) { - content = - typeof parsed.textResultForLlm === "string" - ? parsed.textResultForLlm - : JSON.stringify(sortJsonKeys(parsed.textResultForLlm)); - } else { - content = JSON.stringify(sortJsonKeys(parsed)); - } - } catch { - content = m.content.trim(); - } + content = normalizeToolMessageContent(m.content); } else if (typeof m.content === "string") { content = m.content; } @@ -1237,6 +1356,28 @@ function transformOpenAIRequestMessage( return msg; } +function normalizeToolMessageContent(content: string): string { + // If it's a JSON tool call result, normalize the whitespace and property ordering. + // For successful tool results wrapped in {resultType, textResultForLlm}, unwrap to + // just the inner value so snapshots stay stable across envelope format changes. + try { + const parsed = JSON.parse(content); + if ( + parsed && + typeof parsed === "object" && + parsed.resultType === "success" && + "textResultForLlm" in parsed + ) { + return typeof parsed.textResultForLlm === "string" + ? parsed.textResultForLlm + : JSON.stringify(sortJsonKeys(parsed.textResultForLlm)); + } + return JSON.stringify(sortJsonKeys(parsed)); + } catch { + return content.trim(); + } +} + function normalizeUserMessage(content: string): string { return normalizeSkillContextFrontmatter(content) .replace( @@ -1550,9 +1691,9 @@ function findAssistantIndexAfterPrefix( } for (let i = 0; i < requestMessages.length; i++) { - const reqMsg = JSON.stringify(requestMessages[i]); - const savedMsg = JSON.stringify(savedMessages[i]); - if (reqMsg !== savedMsg) { + if (!normalizedMessagesEqual(requestMessages, savedMessages, i)) { + const reqMsg = JSON.stringify(requestMessages[i]); + const savedMsg = JSON.stringify(savedMessages[i]); log(`mismatch at index ${i}:`); log(` REQ: ${reqMsg.substring(0, 1000)}`); log(` SAVED: ${savedMsg.substring(0, 1000)}`);