diff --git a/src/loop/core.ts b/src/loop/core.ts index a8e873b..a355dbc 100644 --- a/src/loop/core.ts +++ b/src/loop/core.ts @@ -24,6 +24,31 @@ import type { WireProtocol } from "../util.js"; export const MAX_LOOP_ROUNDS = 10; +/** #156: identity of a round's failed compress call(s): the requested refs + * plus the failure text with volatile numbers collapsed, so "Summary too + * short (28 chars…)" and "(31 chars…)" count as the same failure. Two + * consecutive rounds with the same signature mean the model re-sent the + * same bad request and cannot self-correct — re-requesting again just + * burns upstream calls (field logs showed all 10 rounds wasted). */ +function compressFailureSignature(proxyResults: { name: string; result: string; arguments: string }[]): string | null { + const parts: string[] = []; + for (const pr of proxyResults) { + if (pr.name !== "compress" || !pr.result.includes("FAILED")) continue; + let args: unknown; + try { + args = pr.arguments.length > 0 ? JSON.parse(pr.arguments) : {}; + } catch { + args = {}; + } + const ranges = parseCompressInput(args); + const rangeKey = ranges.length > 0 + ? ranges.map((r) => `${r.startRef}..${r.endRef}`).join("+") + : pr.arguments.replace(/\s+/g, ""); + parts.push(`${rangeKey} :: ${pr.result.replace(/\d+/g, "#").replace(/\s+/g, " ")}`); + } + return parts.length > 0 ? parts.sort().join(" | ") : null; +} + export interface LoopCtx { core: CompressionCore; config: Config; @@ -187,6 +212,9 @@ export async function* runCompressLoop( let activeClearTimer: (() => void) | null = null; let currentUpstream = upstream; const coreMessages: CoreMessage[] = [...ctx.messages]; + // #156: signature of the previous round's failed compress call(s), used + // to short-circuit loops where the model repeats the identical failure. + let prevCompressFailureSig: string | null = null; try { for (let round = 1; round <= MAX_LOOP_ROUNDS; round++) { @@ -370,6 +398,20 @@ export async function* runCompressLoop( yield adapter.emitToolCall(tc); } + // #156: identical compress failures in consecutive rounds mean + // the model cannot self-correct (small models were observed + // burning all MAX_LOOP_ROUNDS on the same range + same + // validation error). Complete gracefully after the second + // identical failure instead of re-requesting a third time. + const failureSig = compressFailureSignature(proxyResults); + if (failureSig !== null && failureSig === prevCompressFailureSig) { + ctx.log(`[acp-loop] round ${round}: identical compress failure twice; completing instead of re-requesting`); + loggerLog("warn", `[acp-loop] compress loop short-circuited (identical failure twice): ${failureSig}`); + yield adapter.emitCompletion({ finishReason: "length", usage }); + return; + } + prevCompressFailureSig = failureSig; + // Re-request so the model receives the proxy-tool result and can // continue (standard function-calling continuation: the proxy acts as // the client, executes compress/decompress/acp_status/search, then diff --git a/src/stream.ts b/src/stream.ts index 770722c..05e3b2d 100644 --- a/src/stream.ts +++ b/src/stream.ts @@ -216,7 +216,7 @@ function executeAnthropicProxyTool(toolName: string, args: Record, ctx: RewriteCtx): string { if (ranges.length === 0) { ctx.log("[acp-proxy: compress call had no valid ranges; nothing compressed.]"); - return "[Compression FAILED: no valid ranges parsed from the tool call. Check your startId/endId parameters.]"; + return "[Compression FAILED: no valid ranges parsed from the tool call. Each range must be an object with string startId/endId refs (e.g. m00001) and a non-empty summary; run acp_status to list currently valid refs.]"; } ctx.log(`[acp-proxy: compress requested ${ranges.length} range(s): ${ranges.map((r) => `${r.startRef}–${r.endRef}`).join(", ")}]`); ctx.log(`[acp-proxy: ctx has ${ctx.messages.length} message(s), state has ${ctx.session.state.messageRefs?.byRef?.size ?? "?"} ref(s) mapped]`); diff --git a/tests/compress-shortcircuit.test.ts b/tests/compress-shortcircuit.test.ts new file mode 100644 index 0000000..087bb6c --- /dev/null +++ b/tests/compress-shortcircuit.test.ts @@ -0,0 +1,143 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import type { CompressionCore, Config, CoreMessage } from "acp-kernel"; +import { createCore, createInitialState, defaultConfig } from "acp-kernel"; +import type { Session } from "../src/session.ts"; +import { runCompressLoop, createResponsesAdapter } from "../src/loop/index.ts"; +import { buildCompressSystemPrompt } from "../src/compress-tool.ts"; + +// #156: a small model (2B, Codex+Responses) retried the SAME failed compress +// range with the SAME validation error for all 10 loop rounds — every retry a +// real upstream call. The loop now short-circuits after two consecutive +// IDENTICAL failures (same refs + same error class) and completes gracefully. + +interface CtxFixture { + core: CompressionCore; + config: Config; + messages: CoreMessage[]; + session: Session; + log: (m: string) => void; + proxyUrl?: string; + textProtocol?: boolean; +} + +function makeCtx(messages: CoreMessage[] = []): CtxFixture { + return { + core: createCore(), + config: defaultConfig(200000), + messages, + session: { + id: "compress-shortcircuit-test", + meta: {}, + stats: { requests: 0, tokensSaved: 0, inputTokens: 0, cachedTokens: 0, outputTokens: 0, cacheSamples: 0, lastInputTokens: 0, contextTokens: 0 }, + metadata: {}, + state: createInitialState(), + createdAt: Date.now(), + lastSeen: Date.now(), + blockContents: new Map(), + inFlight: 0, + persisted: false, + }, + log: () => {}, + }; +} + +function sse(type: string, data: unknown): string { + return `event: ${type}\ndata: ${JSON.stringify(data)}\n\n`; +} + +function fcEvents(outputIndex: number, callId: string, name: string, args: string): string { + return [ + sse("response.output_item.added", { item: { type: "function_call", id: `fc_${callId}`, call_id: callId, name }, output_index: outputIndex }), + sse("response.function_call_arguments.delta", { item_id: `fc_${callId}`, delta: args }), + sse("response.output_item.done", { item: { type: "function_call", id: `fc_${callId}`, call_id: callId, name, arguments: args }, output_index: outputIndex }), + ].join(""); +} + +const COMPLETED = sse("response.completed", { response: { id: "resp_done", status: "completed", output: [] } }); +const SYS_PROMPT = buildCompressSystemPrompt(); + +/** A round whose only call is a compress that deterministically FAILS: + * makeCtx() has no ref map, so the kernel resolves 0 blocks for any refs. */ +function failingCompressRound(callId: string, startId: string, endId: string): string { + return [ + sse("response.created", { response: { id: `resp_${callId}`, status: "in_progress" } }), + fcEvents(0, callId, "compress", JSON.stringify({ content: [{ startId, endId, summary: "s" }] })), + COMPLETED, + ].join(""); +} + +/** Mock the loop's re-request fetch: each call plays back streams[i] in order. */ +function reFetchScript(streams: string[]): { calls: () => number; restore: () => void } { + let n = 0; + const orig = globalThis.fetch; + globalThis.fetch = (async () => { + const body = streams[Math.min(n, streams.length - 1)]; + n++; + return new Response(body, { status: 200, headers: { "content-type": "text/event-stream" } }); + }) as typeof fetch; + return { calls: () => n, restore: () => { globalThis.fetch = orig; } }; +} + +async function drain(stream: ReadableStream, ctx: CtxFixture): Promise { + const chunks: Buffer[] = []; + const gen = runCompressLoop( + stream, + ctx, + { model: "gpt-4o", input: [], stream: true }, + { url: "http://mock", headers: {} }, + createResponsesAdapter(), + SYS_PROMPT, + ); + for await (const chunk of gen) chunks.push(chunk); + return Buffer.concat(chunks).toString("utf8"); +} + +test("#156: identical compress failure twice → short-circuit, no third re-request", async () => { + // Round 1 (initial stream) fails on m00001..m00002; re-fetch round 2 fails + // IDENTICALLY (same refs, same error — numbers may differ, e.g. char counts). + const round2 = failingCompressRound("refetch_1", "m00001", "m00002").replace("resp_refetch_1", "resp_refetch_2"); + const probe = reFetchScript([round2]); + try { + const out = await drain(new Response(failingCompressRound("round1", "m00001", "m00002"), { status: 200 }).body!, makeCtx()); + assert.equal(probe.calls(), 1, "exactly ONE re-request: the second identical failure completes the loop"); + assert.ok(out.includes("[ACP]"), "failure markers still surfaced to the client"); + assert.match(out, /event: response\.completed/, "graceful completion, not an error"); + } finally { + probe.restore(); + } +}); + +test("#156: DIFFERENT failure signature → loop keeps going (no premature short-circuit)", async () => { + // Round 1 fails on m00001..m00002, round 2 fails on m00003..m00004 (the + // model varied its range — still trying), round 3 completes cleanly. + const probe = reFetchScript([ + failingCompressRound("refetch_1", "m00003", "m00004"), + sse("response.created", { response: { id: "resp_clean", status: "in_progress" } }) + COMPLETED, + ]); + try { + const out = await drain(new Response(failingCompressRound("round1", "m00001", "m00002"), { status: 200 }).body!, makeCtx()); + assert.equal(probe.calls(), 2, "two re-requests: distinct failures keep the loop alive until a clean round"); + assert.match(out, /event: response\.completed/, "graceful completion"); + } finally { + probe.restore(); + } +}); + +test("#156: 0-ranges failure carries actionable guidance (acp_status pointer)", async () => { + const round1 = [ + sse("response.created", { response: { id: "resp_1", status: "in_progress" } }), + // content is a non-JSON string → parseCompressInput yields 0 ranges + fcEvents(0, "call_g", "compress", JSON.stringify({ content: "not json" })), + COMPLETED, + ].join(""); + const probe = reFetchScript([sse("response.created", { response: { id: "resp_clean", status: "in_progress" } }) + COMPLETED]); + try { + const out = await drain(new Response(round1, { status: 200 }).body!, makeCtx()); + assert.ok(out.includes("no valid ranges"), "0-ranges failure surfaced"); + assert.ok(out.includes("acp_status"), "failure tells the model how to recover (run acp_status)"); + assert.match(out, /event: response\.completed/, "graceful completion"); + } finally { + probe.restore(); + } +}); diff --git a/tests/fix-stream.test.ts b/tests/fix-stream.test.ts index 3b37886..51280a4 100644 --- a/tests/fix-stream.test.ts +++ b/tests/fix-stream.test.ts @@ -49,10 +49,14 @@ function fcEvents(outputIndex: number, callId: string, name: string, args: strin const COMPLETED = sse("response.completed", { response: { id: "resp_done", status: "completed", output: [] } }); -function mutatingRound(): string { +/** A round whose compress call deterministically FAILS (no ref map in + * makeCtx()). `round` varies the requested refs so consecutive rounds have + * distinct failure signatures — the #156 identical-failure short-circuit + * must not fire in these tests. */ +function mutatingRound(round = 1): string { return [ sse("response.created", { response: { id: "resp_1", status: "in_progress" } }), - fcEvents(0, "call_c", "compress", JSON.stringify({ content: [{ startId: "m00001", endId: "m00002", summary: "s" }] })), + fcEvents(0, "call_c", "compress", JSON.stringify({ content: [{ startId: `m${String(round).padStart(5, "0")}`, endId: `m${String(round + 1).padStart(5, "0")}`, summary: "s" }] })), COMPLETED, ].join(""); } @@ -110,9 +114,12 @@ test("client-abort: signal aborted during a re-request stops further re-requests }); test("client-abort control: without a signal, mutating rounds keep re-requesting up to the loop limit", async () => { + // #156 note: the fetch mock varies the failing range each round so the + // identical-failure short-circuit stays out of the picture — what stops + // this loop must be the round limit, not a signal and not short-circuit. let fetchCalls = 0; const orig = globalThis.fetch; - globalThis.fetch = (() => { fetchCalls++; return new Response(mutatingRound(), { status: 200 }); }) as typeof fetch; + globalThis.fetch = (() => { fetchCalls++; return new Response(mutatingRound(fetchCalls + 1), { status: 200 }); }) as typeof fetch; try { await drainSig(new Response(mutatingRound(), { status: 200 }).body!, makeCtx(), undefined); assert.ok(fetchCalls > 1, `control path re-requested multiple times (fetchCalls=${fetchCalls}); abort is what stops it`);