From f1fecb82326b6bb22ea24b4fb29604c947350cad Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Thu, 6 Aug 2026 02:59:46 -0500 Subject: [PATCH] fix(explore): fund the guard from room that exists, and cut the epilogue first (CG-31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections found by measuring the first cut of the guard against the 6-repo suite. The first version held back the FULL sum of the reservations below a file. On django that took 2,319 chars off a file the agent receives and handed them to a section the hard ceiling then threw away — the guard's own failure mode, one layer down. tokio lost 1,298 the same way. 1. `owedPayableBelow` — hold back only the prefix of what is owed below that the response can still PAY, in rank order. A promise the ceiling cannot reach is not a claim on this file's bytes. 2. The final truncation now spends the EPILOGUE before it spends a rendered file section. It used to cut at the last section header, dropping that section AND the trailing notes; dropping the notes alone is almost always enough. A section is source the agent otherwise has to Read; the epilogue is a pointer list and two reminders, and the note that replaces it carries the "explore these names" instruction forward. Also count `flow.text` in `totalChars`. It is prepended to `lines` to make the final output, so the render loop always spent against a ceiling it was ~2K under on symbol-bag queries. Deterministic, same clean-rebuilt indexes, both builds (baseline = CG-30 tip): repo base source new source files django 20,033 20,791 5 trunc -> 6 excalidraw 18,776 20,204 7 trunc -> 8 okhttp 15,628 19,034 4 trunc -> 5 tokio 20,340 21,521 4 trunc -> 5 gin 10,776 10,776 4 -> 4 (byte-identical) alamofire 11,662 11,662 2 -> 2 (byte-identical) No repo delivers less; four stop truncating. `funded` in the diagnostic now reports the render CEILING the guard allows, which is what every render path is actually bounded by. Co-Authored-By: Claude Opus 5 --- __tests__/explore-displacement-guard.test.ts | 15 ++- src/mcp/explore-diagnostics.ts | 22 ++-- src/mcp/tools.ts | 116 ++++++++++++++----- 3 files changed, 110 insertions(+), 43 deletions(-) diff --git a/__tests__/explore-displacement-guard.test.ts b/__tests__/explore-displacement-guard.test.ts index aebf792..716bc97 100644 --- a/__tests__/explore-displacement-guard.test.ts +++ b/__tests__/explore-displacement-guard.test.ts @@ -151,9 +151,11 @@ describe('CG-31 — the cluster path holds back what is still owed below it', () // shape that makes the bounded overshoot fire at all. const source = fs.readFileSync(path.join(testDir, GIANT), 'utf-8'); expect(source.length).toBeGreaterThan((rec.spendable ?? 0) * 2); - // And the guard actually bit — a vacuous pass here would hide a regression. + // And the guard actually bit — a vacuous pass here would hide a + // regression. Measured against the bounded overshoot a cluster's top + // member may otherwise take (1.5x, CG-30), which is what it refused. expect(rec.funded).not.toBeNull(); - expect(rec.funded!).toBeLessThan(rec.spendable!); + expect(rec.funded!).toBeLessThan(Math.round(rec.spendable! * 1.5)); }); }); @@ -203,6 +205,15 @@ describe('CG-31 — the cluster path holds back what is still owed below it', () } }); + it('nothing is lost to the hard ceiling — the epilogue is cut before a section', () => { + // A section thrown away by the final truncation is the same starvation + // arriving after the guard has done its work: the bytes were held back + // for that file and then nobody received them. + for (const probe of [spread, precise]) { + expect(probe.report.files.filter((f) => f.render === 'dropped')).toEqual([]); + } + }); + it('keeps the response inside the hard ceiling', () => { for (const probe of [spread, precise]) { expect(probe.report.envelope.chars).toBeLessThanOrEqual(probe.report.budget.hardCeiling); diff --git a/src/mcp/explore-diagnostics.ts b/src/mcp/explore-diagnostics.ts index 758442a..110f56c 100644 --- a/src/mcp/explore-diagnostics.ts +++ b/src/mcp/explore-diagnostics.ts @@ -104,12 +104,14 @@ interface FileRecord extends ExploreCandidateMeta { */ spendable: number | null; /** - * The DISPLACEMENT-GUARDED bound (CG-31): how much this file may render + * The DISPLACEMENT-GUARDED ceiling (CG-31): the most this file may render * without spending a reservation still owed to a file the loop has not - * reached. `spendable` is what the file was promised, this is what is - * actually still there to pay it with — when it sits below `spendable`, the - * difference is the overshoot the guard refused, and the files below this one - * in the table are the reason. `null` until the render loop reaches the file. + * reached AND can still pay. `spendable` is what the file was promised, this + * is what is actually still there to pay it with — every render path is + * bounded by it, so `emittedChars` above it is a bug. Sits ABOVE `spendable` + * when the room is there (the bounded overshoot a big cluster member may + * take) and BELOW it when the files underneath need the bytes. `null` until + * the render loop reaches the file. */ funded: number | null; render?: ExploreRenderMode; @@ -161,7 +163,7 @@ export interface ExploreDiagnosticFile extends ExploreCandidateMeta { allowance: number | null; /** Reservation + inherited slack — the bound the render paths actually use. */ spendable: number | null; - /** Same bound after holding back what is still owed to unreached files. */ + /** Render ceiling after holding back what is still owed to unreached files. */ funded: number | null; render: ExploreRenderMode | null; skipped: ExploreSkipReason | null; @@ -753,10 +755,10 @@ export function renderTable(report: ExploreDiagnosticReport): string { if (f.spendable !== null && f.allowance !== null && f.spendable !== f.allowance) { out.push(` spendable: ${num(f.spendable)} (reservation + inherited slack)`); } - // Only when the displacement guard actually bit: the gap is what this file - // was refused so the files below it could still be paid. - if (f.funded !== null && f.spendable !== null && f.funded < f.spendable) { - out.push(` funded: ${num(f.funded)} (capped — ${num(f.spendable - f.funded)} held back for files not yet rendered)`); + // Only when the displacement guard actually bit: the gap is the overshoot + // this file was refused so the files below it could still be paid. + if (f.funded !== null && f.spendable !== null && f.funded < Math.round(f.spendable * 1.5)) { + out.push(` funded: ${num(f.funded)} (held to this so the files below keep their reservations)`); } if (f.dedupSavedChars > 0) { const spans = f.dedupCovered.slice(0, 6).map(([a, b]) => (a === b ? `${a}` : `${a}-${b}`)).join(','); diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 869e2eb..1c499a3 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -3996,7 +3996,15 @@ export class ToolHandler { // whichever section happened to land last. Kept in sync with `hardCeiling` // below; the margin covers the drift epilogue and the trailing notes. const renderCeiling = Math.min(Math.round(budget.maxOutputChars * 1.5), 25000) - 600; - let totalChars = lines.join('\n').length; + // `flow.text` is PART of the response — it is prepended to `lines` to make + // the final output — so the render loop has to spend against it, and it + // never did. Counting it is what makes `renderCeiling` the ceiling it + // claims to be: without it the loop believed it had room for a trailing + // section the final truncation then threw away whole, and (CG-31) the + // displacement guard dutifully held bytes back to pay for that section — + // taking them off a file the agent DOES receive and handing them to one it + // never sees. + let totalChars = flow.text.length + lines.join('\n').length; let filesIncluded = 0; // Paths we actually render source for below. Drives the curated header count // (#1046) — it must reflect what we show, not the raw candidate gather. @@ -4048,11 +4056,6 @@ export class ToolHandler { // and no file is ever cut BELOW the reservation it was promised. let reservedSoFar = 0; let sourceSpent = 0; - // How many admitted files the loop has already drawn a reservation for. - // Pairs with `reservedSoFar` to say how many reservations are still owed - // BELOW the current file — the render-space overhead of those pending - // sections has to be held back too, not just their source (CG-31). - let admittedSoFar = 0; // Funding line for the whole-file BUY rule: the response's SOURCE may reach // everything the allocator promised plus one bounded overshoot, and no more. // Measured against the promise rather than `renderCeiling` on purpose — the @@ -4060,12 +4063,42 @@ export class ToolHandler { // what, so funding a buy from it just moves the shortfall to whichever file // the loop reaches last. See WHOLE_FILE_BUY_OVERSHOOT_FRACTION. const reservedTotal = [...allocation.allowances.values()].reduce((sum, n) => sum + n, 0); - const admittedTotal = allocation.allowances.size; const sourceCeiling = reservedTotal + Math.round( budget.maxOutputChars * EXPLORE_ALLOCATION.WHOLE_FILE_BUY_OVERSHOOT_FRACTION, ); + /** + * How much of what is still owed BELOW `fileIndex` the response can actually + * still PAY, in render-space chars (CG-31). + * + * Not the same as the sum of those reservations. The allocator splits the + * envelope; the render loop spends against a ceiling that also has to hold + * the response's own prose, so on a saturated response the promises are + * OVER-SUBSCRIBED and the tail is going to be dropped whatever happens + * above it. Bytes held back for a file that then gets dropped are bytes + * nobody ever receives — measured on django, holding the full owed sum cost + * the rank-#1 file 2,126 chars and handed them to a rank-#6 section the + * hard ceiling threw away. So walk the remaining files in RANK order and + * hold back only the prefix that fits `budgetLeft`; the first one that does + * not fit ends it, because everything after it is further out of reach. + * + * Conservative and self-correcting: it assumes each file below spends its + * whole reservation, and when they do not, the carry-forward hands the + * difference to whoever comes next anyway. + */ + const owedPayableBelow = (fileIndex: number, budgetLeft: number): number => { + let held = 0; + for (let j = fileIndex + 1; j < sortedFiles.length; j++) { + const r = allocation.allowances.get(sortedFiles[j]![0]); + if (r === undefined) continue; + const need = r + EXPLORE_ALLOCATION.FILE_OVERHEAD; + if (held + need > budgetLeft) break; + held += need; + } + return held; + }; - for (const [filePath, group] of sortedFiles) { + for (let fileIndex = 0; fileIndex < sortedFiles.length; fileIndex++) { + const [filePath, group] = sortedFiles[fileIndex]!; if (filesIncluded >= maxFiles) { if (diag) for (const [fp] of sortedFiles) diag.recordSkip(fp, 'max-files'); break; @@ -4099,39 +4132,35 @@ export class ToolHandler { Math.max(reserved, Math.round(budget.maxOutputChars * EXPLORE_ALLOCATION.MAX_SHARE)), ); reservedSoFar += reserved; - admittedSoFar++; diag?.recordSpendable(filePath, allowance); // DISPLACEMENT GUARD, in render space (CG-31). `allowance` says what this // file MAY spend; it does not say the bytes are still there to spend. The // hard ceiling is shared with every file the loop has not reached yet, and // their reservations are promises the allocator already made — so what is - // left before the ceiling is not all ours: `owedRenderBelow` of it is - // spoken for. Subtracting it is the same inequality the whole-file BUY arm - // enforces with `owedBelow` (see below), moved into the units the cluster - // path actually spends in — source PLUS the per-section overhead each - // pending file will charge. + // left before the ceiling is not all ours. Holding that back is the same + // inequality the whole-file BUY arm enforces with `owedBelow` (see below), + // moved into the units the cluster path actually spends in: source PLUS + // the per-section overhead each pending file will charge. + // + // Held back only where it can be PAID — see `owedPayableBelow`. A promise + // the ceiling cannot reach is not a claim on this file's bytes; honouring + // it anyway just moves source from a file the agent gets to one it does + // not. // // Floored at this file's OWN reservation, never below: a kept promise is // not a displacement, and cutting a file under what it earned is the - // failure this whole allocation layer exists to prevent. When the - // reservations genuinely cannot all fit under the ceiling (the response - // preamble is charged to the same ceiling but not to the allocator's - // envelope), the floor means the shortfall lands on the LAST file rather - // than being taken out of the top one — same as before this guard. + // failure this whole allocation layer exists to prevent. // // Slack still reaches the file: a file above that under-spends leaves // `totalChars` lower, which raises `headroom` one-for-one, so the // carry-forward the `allowance` line grants is exactly the carry-forward // this bound funds. - const owedBelow = Math.max(0, reservedTotal - reservedSoFar); - const owedRenderBelow = owedBelow - + EXPLORE_ALLOCATION.FILE_OVERHEAD * Math.max(0, admittedTotal - admittedSoFar); const headroom = Math.max(0, renderCeiling - totalChars - EXPLORE_ALLOCATION.FILE_OVERHEAD); const fundedHeadroom = Math.max( Math.min(reserved, headroom), - headroom - owedRenderBelow, + headroom - owedPayableBelow(fileIndex, Math.max(0, headroom - reserved)), ); - diag?.recordFunded(filePath, Math.min(allowance, fundedHeadroom)); + diag?.recordFunded(filePath, fundedHeadroom); const absPath = validatePathWithinRoot(projectRoot, filePath); if (!absPath || !existsSync(absPath)) { diag?.recordSkip(filePath, 'unreadable'); @@ -4478,10 +4507,10 @@ export class ToolHandler { // rather than a size cap — a buy that fits the line only by spending a // lower-ranked file's reservation is the trade that dropped // `payslip_builder.go`, and it is refused here. Self-limiting: each buy - // grows `sourceSpent`, so the pool cannot be spent twice. (`owedBelow` is - // computed once at the top of the iteration — the cluster path below - // enforces the same inequality in render space; see `fundedHeadroom`.) - // + // grows `sourceSpent`, so the pool cannot be spent twice. The cluster path + // below enforces the same inequality in render space — see + // `fundedHeadroom` / `owedPayableBelow` (CG-31). + const owedBelow = Math.max(0, reservedTotal - reservedSoFar); // Third condition on the BUY arm only: it must also FIT. A whole render // that overruns `renderCeiling` is skipped ENTIRELY a few lines below (the // branch refuses to slice a file mid-method), so attempting a buy that @@ -5158,6 +5187,15 @@ export class ToolHandler { } } + // Everything pushed from here on is EPILOGUE — meta-text about the response + // rather than the response. Marked so the hard-ceiling cut at the end can + // spend it before it spends a rendered file section (CG-31): a section is + // source the agent otherwise has to Read, the epilogue is a pointer list and + // two reminders. Lines already in `lines` are only MUTATED below (the + // verbatim header, the summary sentinel), never re-ordered, so the index + // stays valid. + const epilogueStart = lines.length; + // The back-reference convention, stated once where the verbatim guarantee is // (#1474 does the same for drift). Without it a pointer reads as an // apology for missing source rather than as an index into source the agent @@ -5268,9 +5306,25 @@ export class ToolHandler { const hardCeiling = Math.min(Math.round(budget.maxOutputChars * 1.5), 25000); let finalText: string; - if (output.length > hardCeiling) { - // Cut at a FILE-SECTION boundary (the last ``**` `` file header before the - // ceiling) so we drop whole trailing file-sections rather than slicing + // The epilogue costs less than a file section, so it is cut FIRST (CG-31). + // Dropping a trailing section throws away source the render loop had already + // set that file's reservation aside for — the exact starvation the + // displacement guard exists to prevent, arriving after the guard has done + // its work. The epilogue is a pointer list and two reminders; its own + // "explore these names" instruction survives in the note below. + const epilogueOnlyCut = epilogueStart < lines.length + ? flow.text + lines.slice(0, epilogueStart).join('\n') + : null; + const EPILOGUE_CUT_NOTE = '\n\n> (Trailing notes omitted for size. The source above is complete and verbatim — treat it as already Read. For anything this call did not cover, run another codegraph_explore with the specific names rather than reading those files.)'; + + if (output.length > hardCeiling + && epilogueOnlyCut !== null + && epilogueOnlyCut.length + EPILOGUE_CUT_NOTE.length <= hardCeiling) { + finalText = epilogueOnlyCut + EPILOGUE_CUT_NOTE; + } else if (output.length > hardCeiling) { + // Still over with the epilogue gone: cut at a FILE-SECTION boundary (the + // last ``**` `` file header before the ceiling) so we drop whole trailing + // file-sections rather than slicing // through a method body — a half-rendered method just forces the Read this // tool exists to prevent. Fall back to a line boundary only if no section // header sits in the back half (degenerate single-giant-section case).