fix: short-circuit identical compress failures (#156)

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

src/stream.ts

@@ -216,7 +216,7 @@ function executeAnthropicProxyTool(toolName: string, args: Record<string, unknow
 export function applyRanges(ranges: ReturnType<typeof parseCompressInput>, 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]`);

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<Uint8Array>, ctx: CtxFixture): Promise<string> {
+    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();
+    }
+});

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`);