diff --git a/CHANGELOG.md b/CHANGELOG.md index 45bafb8d..cef29ae9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph callers`, `codegraph callees` and `codegraph impact` (CLI and MCP) now report missing names with did-you-mean suggestions instead of another symbol's results, and exact matches with no callers stay empty; thanks @uvmplus. (#1473, #1481) - When `codegraph_explore` trims a large file, the gaps in its source now name the symbols that were left out, using only room the file's budget has left, so the names never push out the code itself. (#1711) - `codegraph_explore` no longer reports "runtime dispatch" through a base class that doesn't declare the method, so a common name shared by many unrelated subclasses no longer opens an answer with a list of thousands of implementations. +- When you name several functions in one file and unrelated code sits between them, `codegraph_explore` now returns the functions you named before the code around them, instead of spending the file's room on that code and leaving one of the functions you asked for with no source at all. #### MCP / indexing diff --git a/__tests__/explore-isolated-named-cluster.test.ts b/__tests__/explore-isolated-named-cluster.test.ts new file mode 100644 index 00000000..6981bfe0 --- /dev/null +++ b/__tests__/explore-isolated-named-cluster.test.ts @@ -0,0 +1,267 @@ +/** + * Regression gate: a cluster holding only a named function must not lose its + * file's budget to the incidental members of a cluster ranked above it. + * + * `handleExplore` ranks a file's clusters spine first, then by max importance, + * then by DENSITY — summed member importance over span. Density counts every + * member, so a named function (importance 10) with unrelated helpers merged + * around it scores the helpers' definitions (1) and their edge lines (2) too, + * and outranks a cluster holding the same kind of named function alone. Taken + * whole, the higher clusters then spent the reservation on the helpers, and the + * isolated named function below them rendered nothing: the query named five + * functions and got two to four of them back, depending on the build. + * + * The fixture is that shape and nothing else: one file, five independent + * exported functions the query names, three unrelated `headerSlotN` helpers + * between each pair, and ~45 more helpers after them. Three controls pin that the + * gate measures the ordering and not the budget: + * + * - the same five functions CONTIGUOUS all render whole under the same + * reservation, so the budget holds the named bodies; + * - their rendered cost is well under the reservation, and the file is far + * over it, so something has to be dropped and it need not be a named body; + * - a query naming only `renderView` still renders the helpers merged around + * it: with nothing named below a cluster, its incidental context stays. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import CodeGraph from '../src/index'; +import { ToolHandler } from '../src/mcp/tools'; +import type { ExploreDiagnosticReport } from '../src/mcp/explore-diagnostics'; + +const TARGET = 'lib/response.ts'; +const NAMED = ['sendBody', 'sendJson', 'renderView', 'redirectTo', 'sendFileStream']; +const QUERY = NAMED.join(' '); +/** Statements per named body: 26 lines with the signature and closing brace. */ +const BODY_STATEMENTS = 24; +const HELPERS_BETWEEN = 3; +const HELPERS_AFTER = 45; + +/** + * `lib/response.ts`. The helpers are the filler: never named, never called by a + * named function, and wide enough that the named functions plus the helpers + * merged around them overrun the file's reservation both at the 9,100-char + * `MAX_SHARE` valve and at the ~12,800 a file the query named gets when the + * valve is lifted for it — so the gate stays red without the fix either way. + */ +function responseSource(interleave: boolean): string { + const out: string[] = [ + 'export interface Res {', + ' body: string[];', + ' setHeader(name: string, value: string): void;', + '}', + '', + ]; + let slot = 0; + const helper = (): void => { + slot++; + out.push(`export function headerSlot${slot}(res: Res, value: string): void {`); + for (let k = 0; k < 6; k++) { + out.push(` res.setHeader('x-slot-${slot}-${k}', \`max-age=31536000; includeSubDomains; preload; ` + + `report-uri=https://reports.example.test/csp/slot-${slot}/${k}?format=json&v=\${value}\`);`); + } + out.push('}', ''); + }; + NAMED.forEach((name, i) => { + out.push(`export function ${name}(res: Res, payload: string): void {`); + for (let k = 0; k < BODY_STATEMENTS; k++) out.push(` res.body.push('${name}:${k}:' + payload);`); + out.push('}', ''); + if (interleave && i < NAMED.length - 1) for (let h = 0; h < HELPERS_BETWEEN; h++) helper(); + }); + for (let h = 0; h < HELPERS_AFTER; h++) helper(); + return out.join('\n'); +} + +interface Run { + dir: string; + cg: CodeGraph; + response: string; + report: ExploreDiagnosticReport; +} + +async function runExplore(interleave: boolean, query: string): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-isolated-named-')); + fs.mkdirSync(path.join(dir, 'lib')); + fs.writeFileSync(path.join(dir, TARGET), responseSource(interleave)); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"isolated-named","version":"1.0.0"}\n'); + const cg = CodeGraph.initSync(dir); + await cg.indexAll(); + + const sidecar = path.join(dir, 'explore-diag.jsonl'); + const previous = process.env.CODEGRAPH_EXPLORE_DEBUG; + process.env.CODEGRAPH_EXPLORE_DEBUG = sidecar; + let response: string; + try { + response = (await new ToolHandler(cg).execute('codegraph_explore', { query })) + .content?.[0]?.text ?? ''; + } finally { + if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEBUG; + else process.env.CODEGRAPH_EXPLORE_DEBUG = previous; + } + const written = fs.readFileSync(sidecar, 'utf-8').trim().split('\n').filter(Boolean); + return { dir, cg, response, report: JSON.parse(written[written.length - 1]!) }; +} + +function teardown(run: Run | undefined): void { + if (!run) return; + run.cg.destroy(); + if (fs.existsSync(run.dir)) fs.rmSync(run.dir, { recursive: true, force: true }); +} + +/** Every `\t` line number the response sent. The fixture has one file. */ +function renderedLines(response: string): Set { + const out = new Set(); + for (const m of response.matchAll(/^(\d+)\t/gm)) out.add(Number(m[1])); + return out; +} + +function defOf(run: Run, name: string) { + const node = run.cg.getNodesInFile(TARGET).find((n) => n.name === name && n.kind === 'function'); + expect(node, `${name} is not indexed in ${TARGET}`).toBeDefined(); + return node!; +} + +/** Lines of `name`'s body the response carries, out of its full span. */ +function coverage(run: Run, name: string): { got: number; of: number } { + const node = defOf(run, name); + const lines = renderedLines(run.response); + let got = 0; + for (let ln = node.startLine; ln <= node.endLine; ln++) if (lines.has(ln)) got++; + return { got, of: node.endLine - node.startLine + 1 }; +} + +function targetFile(run: Run): ExploreDiagnosticReport['files'][number] { + const file = run.report.files.find((f) => f.path === TARGET); + expect(file, `${TARGET} is not among the ranked candidates`).toBeDefined(); + return file!; +} + +describe('an isolated named cluster keeps its share of the file budget', () => { + let run: Run; + let contiguous: Run; + + beforeAll(async () => { + run = await runExplore(true, QUERY); + contiguous = await runExplore(false, QUERY); + }, 120_000); + + afterAll(() => { + teardown(run); + teardown(contiguous); + }); + + describe('fixture shape — if this rots, the gate below means nothing', () => { + it('puts unrelated helpers between every pair of named functions', () => { + const helpers = run.cg.getNodesInFile(TARGET) + .filter((n) => n.kind === 'function' && n.name.startsWith('headerSlot')); + for (let i = 0; i + 1 < NAMED.length; i++) { + const above = defOf(run, NAMED[i]!); + const below = defOf(run, NAMED[i + 1]!); + const between = helpers.filter((h) => h.startLine > above.endLine && h.endLine < below.startLine); + expect(between.length, `${NAMED[i]} → ${NAMED[i + 1]}`).toBe(HELPERS_BETWEEN); + } + }); + + it('names functions that do not call each other, so no call path ranks one first', () => { + const ids = new Set(NAMED.map((name) => defOf(run, name).id)); + for (const name of NAMED) { + const reaches = run.cg.getCallees(defOf(run, name).id).filter(({ node }) => ids.has(node.id)); + expect(reaches, name).toHaveLength(0); + } + }); + + it('clusters a file far bigger than its reservation', () => { + const file = targetFile(run); + expect(file.render).toBe('clusters'); + expect(file.clipped).toBe(true); + const size = fs.readFileSync(path.join(run.dir, TARGET), 'utf-8').length; + expect(size).toBeGreaterThan(3 * file.allowance!); + }); + + it('holds the named bodies well inside that reservation', () => { + // What the five bodies cost rendered: their lines, the three lines of + // context padding on each side, and the `\t` line-number prefix. + const lines = fs.readFileSync(path.join(run.dir, TARGET), 'utf-8').split('\n'); + let cost = 0; + for (const name of NAMED) { + const node = defOf(run, name); + for (let ln = node.startLine - 3; ln <= node.endLine + 3; ln++) { + cost += (lines[ln - 1] ?? '').length + String(ln).length + 2; + } + } + expect(cost).toBeLessThan(0.9 * targetFile(run).allowance!); + }); + + it('renders every named body whole when the same functions sit together', () => { + expect(targetFile(contiguous).allowance).toBe(targetFile(run).allowance); + for (const name of NAMED) { + const { got, of } = coverage(contiguous, name); + expect(got, `${name} (contiguous)`).toBe(of); + } + }); + }); + + describe('the gate', () => { + it('renders every named body whole with helpers between them', () => { + for (const name of NAMED) { + const { got, of } = coverage(run, name); + expect(got, `${name}: ${got}/${of} lines`).toBe(of); + } + }); + + it('keeps the response inside the hard ceiling', () => { + expect(run.report.envelope.chars).toBeLessThanOrEqual(run.report.budget.hardCeiling); + }); + }); +}); + +/** + * The same rule inside one cluster. A cluster taken after the first is bounded + * by its room exactly, and its member shrink estimated on raw source, which runs + * well under the rendered size: it kept the helpers merged above `renderView`, + * overran the room, and the window back to it cut in source order — through the + * last lines of `renderView`, the body the query named. + */ +describe('a named body in a later cluster is not trimmed for the helpers above it', () => { + let run: Run; + + beforeAll(async () => { + run = await runExplore(true, 'renderView redirectTo'); + }, 120_000); + + afterAll(() => teardown(run)); + + it('renders both named bodies whole', () => { + for (const name of ['renderView', 'redirectTo']) { + const { got, of } = coverage(run, name); + expect(got, `${name}: ${got}/${of} lines`).toBe(of); + } + }); + + it('keeps the response inside the hard ceiling', () => { + expect(run.report.envelope.chars).toBeLessThanOrEqual(run.report.budget.hardCeiling); + }); +}); + +describe('a named cluster with nothing named below it keeps its incidental context', () => { + let run: Run; + + beforeAll(async () => { + run = await runExplore(true, 'renderView'); + }, 120_000); + + afterAll(() => teardown(run)); + + it('renders the helper merged in above renderView, not just renderView', () => { + const view = coverage(run, 'renderView'); + expect(view.got).toBe(view.of); + const target = defOf(run, 'renderView'); + const above = run.cg.getNodesInFile(TARGET) + .filter((n) => n.kind === 'function' && n.name.startsWith('headerSlot') && n.endLine < target.startLine) + .sort((a, b) => b.endLine - a.endLine)[0]!; + const { got, of } = coverage(run, above.name); + expect(got, `${above.name}: ${got}/${of} lines`).toBe(of); + }); +}); diff --git a/docs/design/explore-budget-allocation.md b/docs/design/explore-budget-allocation.md index ee28f1da..d77d6dd8 100644 --- a/docs/design/explore-budget-allocation.md +++ b/docs/design/explore-budget-allocation.md @@ -664,3 +664,173 @@ Full record: [`../benchmarks/explore-allocation-ab-1500.md`](../benchmarks/explo CG-15 session and this one — the same magnitude as the gap. Agent wall-clock on this repo is noise-dominated at this sample size, which is the known shape (host-model thinking dominates, not tool latency). + +## Named members before incidental ones (2026-09-28) + +A query naming five independent functions in one file got two to four of them back. Repro, small +tier: `lib/response.ts` holds `sendBody`, `sendJson`, `renderView`, `redirectTo` and +`sendFileStream`, ~36 lines each, with three unrelated `headerSlotN(res, value)` helpers between +each pair and ~45 more after. Query: the five names. On `main`, `sendBody` rendered 0 of 36 lines +and `sendFileStream` 4 while helper bodies rendered in full. The same five functions written +contiguously all rendered. + +### Two places the helpers were paid first + +**Between clusters.** `rankedClusters` orders clusters of equal max importance by density, summed +member importance over span. Density counts every member, so the helpers merged around a named +function (a definition at 1, each `setHeader` edge line at 2) raised the density of its cluster, +and a cluster holding a named function alone ranked below it: 0.39 against 0.95 and 0.84 in the +repro. Taken whole, the two filler-heavy clusters spent the file's 9,100-char reservation and the +isolated `sendBody` got nothing. Reordering does not fix this. Rank `sendBody` first and the +victim moves to whichever named cluster is now last, because each cluster above it still pays for +its helpers before the one below gets a turn. + +**Inside one cluster, at its ceiling.** Every render is bounded by a ceiling, and for a cluster +taken after the first, and for every re-render the ceiling fit makes, that ceiling is its room +exactly. The member shrink estimates on raw source, which runs well under the rendered size +(padding and line numbers). Incidental members were kept on that estimate, the render overran the +ceiling, and the window back to it cut in source order, through the tail of a named body. That is +gin's `handleHTTPRequest` at 61 of 71 lines once the first fix had given it room, and, with the +named-concentration branch merged, vscode's `_remoteCall` at 23 of 36 when that branch's regrow +re-rendered `rpcProtocol.ts`. When the 12-line minimum +windows overran the room instead, the loop dropped the whole cluster, named members and all, and +the room went to clusters with nothing named in them. + +### The rule + +A member is **protected** if the query named it or it is an entry point (importance >= 9), or it +sits on the rendered call path. Everything else in a cluster is incidental. + +- Before selection, `protectedCoreCost` prices each cluster's protected members rendered on their + own, after the session history. `owedFrom` sums that cost over every cluster ranked below each + position. +- A cluster's incidental members may only use what is left of its room once the clusters below it + are paid their protected cost. Its own protected members are chosen first, against the same cap + as before, and are never held back. So when named bodies themselves do not all fit, the + trade-off between them is still the one rank makes. +- In any cluster holding a protected member, incidental members are also never kept past the + render's ceiling, in rendered size. That is `renderCluster`'s own rule, so it covers selection + and every re-render alike: the render fits without being windowed, and nothing named is cut to + make space for its neighbours. For the first cluster the ceiling is the 1.5× spine bound, so + the rule only bites near the point where the render would be windowed anyway (a little before + it, since each incidental member's padded cost is an over-estimate). +- A cluster with nothing protected in it renders byte-for-byte as before, and so does one that + fits its cap with room left for everything owed below it. A lone named function keeps the + helpers merged around it (the test's control). + +The incidental cap is measured against `fileBudget` even for a spine cluster: the clusters below +it that are not on the spine are held to `fileBudget`, so room a spine cluster's incidental +members take past it is room they never get. + +**Rejected: ranking by named density.** Dividing only protected members' importance by span, in +the named tier, stops filler from raising a cluster's rank. Measured on top of the hold-back +(before the ceiling guard was added), it changed 4 of 47 responses, left complete named +bodies where they were (218), and put tokio-pin over the 25K inline cap (25,007). Rank is left +alone. + +### Measured effect + +Deterministic, 47 fixed queries over the 7 README repos plus express: the README questions, the +suite's prose questions, precise symbol bags, pinned-file flows and named-concentration stress +queries (the set the unmerged named-concentration work was measured on). Baseline `main` +`e63fe2ec`, same index for both builds. Named coverage means every definition of every queried +name in every rendered file. + +| | `main` | this change | +|---|---|---| +| named bodies complete | 198 | **234** (17 responses up, **0 down**) | +| named lines delivered | 6,912 | **7,631** (+10%) | +| byte-identical responses | — | 17 of 47 | +| responses over the 25K inline cap | 5 | **1** | +| responses truncated by the final cut | 2 | 3 (see below) | + +No definition that rendered whole on `main` renders partially, except one 5-line overload: tokio's +`park.rs` `park` (5 → 2), in a response that completes three other named bodies while the 46-line +`park` beside it grows (11 → 20). Otherwise what gets smaller is the un-named part of a whole-class container +too big to keep as one member, whose body had been arriving as incidental members: Alamofire's +`Session` (133 → 103 of 1,301 lines), vscode's `RPCProtocol` (194 → 129) beside a now-complete +`MessageIO` class, `ChannelClient` (109 → 63) where the file's incidental lines are now chosen by +importance rather than by source order. No named method inside those classes loses a line. + +| query | complete named bodies | named lines | +|---|---|---| +| vscode `ipc.net.ts` pinned flow | 3 → 8 | 139 → 331 | +| alamofire `Request.swift` pinned | 2 → 6 | 18 → 90 | +| tokio `pb-worker2` | 7 → 10 | 104 → 168 | +| tokio `worker.rs` pinned | 6 → 9 | 182 → 278 | +| gin `gin.go ServeHTTP handleHTTPRequest addRoute` | 2 → 4 | 138 → 223 | +| django symbol bag (`execute_sql` ×2) | 5 → 7 | 201 → 261 | +| excalidraw symbol bag (`_renderStaticScene`) | 3 → 3 | 32 → 79 | + +**The one new truncation is django-bag, and it is not this rule's.** The named file now spends +157 chars less than its reservation, the carry-forward moves that down the rank order, and the +rank-3 file's funding rises from 963 to 1,777 because `owedPayableBelow` stops holding a partial +share for the last file once that share falls under `MIN_CHARS` + overhead. A 32-char shift +upstream crossed that threshold. The last file, a 151-char skeleton, is dropped, and the pointer +list after it is cut by the final trim because the epilogue is never paid for in the render loop. +The named-concentration branch reserves the epilogue; with both, the same query is not truncated +and renders as many files as that branch does alone (below). + +### On the named-concentration branch + +`claude/inspiring-grothendieck-8a7b3e` (`fd8eab23`, unmerged) was cut before #2057, and its own +tip is the wrong base to measure against: there `sectionText` still measures gaps with their names +while the window to a ceiling measures them bare, so a bound render that "fits" overruns its room +and the cluster is dropped. Merged onto `main` (one conflict: keep the branch's `buildSection` and +`main`'s bare-gap `sectionText`), then this change on top (four hunks in `shrinkCluster`, +`renderCluster` and the selection loop, all mechanical), the same 47 queries: + +| | `main` + named concentration | + this change | +|---|---|---| +| named bodies complete | 235 | **279** (19 responses up, 1 down) | +| named lines delivered | 9,330 | **9,965** (+7%) | +| responses over the 25K inline cap | 0 | 0 | +| responses truncated by the final cut | 0 | 0 | + +The response that goes down is tokio's pinned stress query, the same 5-line `park` overload as on +`main`. That branch's own tests (`explore-merged-spine-cluster`, `explore-named-file-valve`) pass +with this change merged in. The two hold-backs apply together to a merged spine cluster: that +branch's limits the whole first cluster to leave the other clusters' top members room, and this +one limits only incidental members, of any cluster. On vscode's `rpcProtocol.ts` that moves room +from `RPCProtocol`'s un-named methods to the named `MessageBuffer` class (66 → 187 of 187 lines, +`RPCProtocol` 383 → 220), with every named method of `RPCProtocol` still whole. + +On that branch a lone named file's reservation rises to 12,800, which absorbs the filler of the +original repro, so the hermetic fixture uses wider helper lines. It is red there without this +change (`sendJson` 0 of 26 lines). + +### Coverage + +`__tests__/explore-isolated-named-cluster.test.ts`, one generated file in the repro's shape +(26-line bodies, three wide helpers between each pair). Its shape block checks that helpers sit +between every pair, that no named function calls another (so no call path ranks one), that the +file is more than three times its reservation, that the named bodies cost under 90% of it, and +that the same functions written contiguously all render whole under the same reservation. + +| Mutation | Red | +|---|---| +| `main`'s `tools.ts` | five-name gate: `sendBody` 0/26 | +| `main` + named concentration, without this change | five-name gate: `sendJson` 0/26 | +| No hold-back for clusters below (ceiling guard kept) | five-name gate: `sendBody` 21/26 | +| No ceiling guard on incidental members (hold-back kept) | `renderView redirectTo`: `renderView` 22/26 | +| Incidental members always dropped | control: the helper above a lone `renderView` | +| Protected members not sorted first in the guarded shrink | none — and all 47 sweep responses are byte-identical | + +The last lever is kept on reasoning alone. Named members already sort first by importance, so the +sort only matters for a call-path member below importance 9 that shares a guarded cluster with a +higher-importance incidental one. There it keeps the incidental member from claiming the cap +first, and it keeps the size the incidental test starts from exact: that is the rendered cost of +the protected set only when every protected member has been weighed before the first incidental +one. + +With the repro's exact geometry (36-line bodies), five padded bodies come to more than the 9,100 +reservation. This change renders four whole and `sendBody` at 29 of 36, the lowest-ranked named +cluster windowed to what is left. That is the budget, not the defect: with the named-concentration +branch as well, all five render whole. + +### Found, not fixed + +- **`owedPayableBelow`'s partial hold has a cliff at `MIN_CHARS` + overhead.** Just above it, the + last admitted file is held its partial share. Just below it, it is held nothing, and the file + above takes ~800 chars the last file needed. Any upstream shift of a few dozen chars can flip it + (django-bag above). diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 8e9c8138..850e1644 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -5648,6 +5648,48 @@ export class ToolHandler { }]; }; + /** Rendered cost of one source line, line numbering included. */ + const lineCost = (ln: number): number => + (fileLines[ln - 1] ?? '').length + 1 + (withLineNumbers ? String(ln).length + 1 : 0); + /** Rendered cost of one member on its own, context padding included. */ + const paddedCost = (r: ExploreRange): number => { + let chars = 0; + const hi = Math.min(fileLines.length, r.end + contextPadding); + for (let ln = Math.max(1, r.start - contextPadding); ln <= hi; ln++) chars += lineCost(ln); + return chars; + }; + /** + * A member the query asked for: one it named or that is an entry point + * (importance >= 9), or one on the rendered call path. The rest of a + * cluster — glue, connected nodes, edge lines, peripheral declarations — + * is incidental context that happens to sit within `gapThreshold` of it. + */ + const isProtectedMember = (r: ExploreRange): boolean => r.importance >= 9 || r.spine; + /** Whole member ranges, re-merged in source order and rendered. */ + const partsOfMembers = (members: ReadonlyArray): SectionPart[] => { + // Re-merge in source order so adjacent survivors read as one block + // rather than a stutter of one-symbol fragments. + const sorted = [...members].sort((a, b) => a.start - b.start); + const merged: Array<{ start: number; end: number }> = []; + for (const r of sorted) { + const last = merged[merged.length - 1]; + if (last && r.start <= last.end + gapThreshold) last.end = Math.max(last.end, r.end); + else merged.push({ start: r.start, end: r.end }); + } + return merged.flatMap((m) => buildSection(m)); + }; + /** + * What this cluster's protected members cost to render on their own, after + * the session history — the room a cluster ranked ABOVE it must leave for + * them. 0 for a cluster with nothing the query asked for. + */ + const protectedCoreCost = (c: ExploreCluster): number => { + const core = c.members.filter(isProtectedMember); + if (core.length === 0) return 0; + const parts = core.length === c.members.length ? buildSection(c) : partsOfMembers(core); + return sectionText(dedupeSpans(parts).parts).length + GAP_MARKER.length; + }; + /** * Shrink an oversize cluster to the highest-importance symbols inside it * that fit `cap`, rendered in source order with gap markers (CG-12). @@ -5676,14 +5718,30 @@ export class ToolHandler { * decide WHICH members survive: that is the ceiling trim's job, and CG-38 is * why that trim now protects the named spans instead of cutting in source * order. See `docs/benchmarks/explore-tail-render-cg38.md`. + * + * `incidentalCap` is set for a cluster holding a member the query asked for + * (see `renderCluster` and `protectedCoreCost`). Protected members are then + * chosen first, exactly as above; every other member must also fit + * `incidentalCap`, which is measured on the rendered text (padding, line + * numbers) because it is a bound — the ceiling past which the render is + * windowed, or bytes owed to a cluster below — not an estimate. */ - const shrinkCluster = (c: ExploreCluster, cap: number): SectionPart[] | null => { + const shrinkCluster = ( + c: ExploreCluster, cap: number, incidentalCap?: number, + ): SectionPart[] | null => { if (c.members.length < 2) return null; + const guarded = incidentalCap !== undefined; const byImportance = [...c.members].sort((a, b) => - b.importance - a.importance || (a.end - a.start) - (b.end - b.start) || a.start - b.start); + (guarded ? Number(isProtectedMember(b)) - Number(isProtectedMember(a)) : 0) + || b.importance - a.importance || (a.end - a.start) - (b.end - b.start) || a.start - b.start); const sizeOf = (r: ExploreRange) => fileLines.slice(r.start - 1, r.end).join('\n').length; const keep: ExploreRange[] = []; let kept = 0; + // Rendered size of what is kept so far, once the first incidental member + // is weighed. Every protected member sorts ahead of it, so this is the + // exact cost of the protected set; each incidental member then adds its + // own padded span, an over-estimate that errs toward the cluster below. + let rendered: number | undefined; for (const r of byImportance) { const sz = sizeOf(r) + GAP_MARKER.length; // Always keep the most important range, even if it alone is oversize — @@ -5691,20 +5749,17 @@ export class ToolHandler { // far it may overshoot is bounded by the caller's ceiling (CG-30), which // windows a runaway member instead of dropping it. if (keep.length > 0 && kept + sz > cap) continue; + if (guarded && keep.length > 0 && !isProtectedMember(r)) { + rendered ??= sectionText(dedupeSpans(partsOfMembers(keep)).parts).length; + const cost = paddedCost(r) + GAP_MARKER.length; + if (rendered + cost > incidentalCap) continue; + rendered += cost; + } keep.push(r); kept += sz; } if (keep.length === c.members.length) return null; - // Re-merge the kept ranges in source order so adjacent survivors read as - // one block rather than a stutter of one-symbol fragments. - keep.sort((a, b) => a.start - b.start); - const merged: Array<{ start: number; end: number }> = []; - for (const r of keep) { - const last = merged[merged.length - 1]; - if (last && r.start <= last.end + gapThreshold) last.end = Math.max(last.end, r.end); - else merged.push({ start: r.start, end: r.end }); - } - return merged.flatMap((m) => buildSection(m)); + return partsOfMembers(keep); }; /** @@ -5721,9 +5776,6 @@ export class ToolHandler { * the call path IS the answer. */ const MIN_WINDOW_LINES = 12; - /** Rendered cost of one source line, line numbering included. */ - const lineCost = (ln: number): number => - (fileLines[ln - 1] ?? '').length + 1 + (withLineNumbers ? String(ln).length + 1 : 0); /** * Longest prefix of `r` that fits `room`. `minLines` is the never-empty * floor — it may overrun `room`, so it is only ever asked for when nothing @@ -5891,8 +5943,25 @@ export class ToolHandler { * never touched. */ ceiling: number = Infinity, + /** + * What this cluster's INCIDENTAL members may render into, when a cluster + * ranked below it holds a member the query asked for — see the selection + * loop. + */ + incidentalCap?: number, ): { parts: SectionPart[]; covered: ExploreLineRange[]; shrunk: boolean } => { const base = dedupeSpans(buildSection(c)); + // In a cluster holding a member the query asked for, incidental members + // are never kept past `ceiling` either. The member shrink estimates on raw + // source, which runs well under the rendered size, so it kept helpers the + // render then had no room for, and the window back to the ceiling cut in + // source order — through the tail of a named body (gin's + // `handleHTTPRequest` lost 10 of 71 lines to the helpers above it), or, + // when its minimum windows overran a later cluster's room, costing that + // cluster its place. Every re-render into an exact room is the same case. + const guard = Number.isFinite(ceiling) && c.members.some(isProtectedMember) + ? Math.min(incidentalCap ?? Infinity, ceiling) + : incidentalCap; const bound = ( r: { parts: SectionPart[]; covered: ExploreLineRange[]; shrunk: boolean }, ) => { @@ -5902,10 +5971,10 @@ export class ToolHandler { const parts = windowToCeiling(r.parts, ceiling, focusLinesOf(c)); return { parts, covered: r.covered, shrunk: true }; }; - if (sectionText(base.parts).length <= cap) { + if (sectionText(base.parts).length <= Math.min(cap, guard ?? Infinity)) { return { parts: base.parts, covered: base.covered, shrunk: false }; } - const shrunk = shrinkCluster(c, cap); + const shrunk = shrinkCluster(c, cap, guard); if (shrunk === null) { return bound({ parts: base.parts, covered: base.covered, shrunk: false }); } @@ -5969,7 +6038,32 @@ export class ToolHandler { const renderedClusters = new Map>(); let anyClusterShrunk = false; let projectedChars = 0; - for (const rc of rankedClusters) { + // What the protected members of every cluster ranked BELOW position p cost + // to render. Rank orders clusters, not members: among clusters of equal + // max importance it prefers density, and density counts every member, so + // a named function with unrelated helpers merged around it outranks a + // cluster holding that same kind of function alone — the helpers' edge + // lines raise its density. Taken whole, the first cluster then spent the + // file's budget on the helpers and the isolated named function rendered + // nothing. So a cluster's incidental members may only use what is left + // once the named members below it are paid for. Its own protected members + // are never held back, so the trade-off between two named symbols is the + // one rank already made. + const owedFrom = new Array(rankedClusters.length + 1).fill(0); + for (let p = rankedClusters.length - 1; p >= 0; p--) { + owedFrom[p] = owedFrom[p + 1]! + protectedCoreCost(rankedClusters[p]!.c); + } + // Measured against `fileBudget` even for a spine cluster: the clusters + // below it that are not on the spine are held to `fileBudget`, so room a + // spine cluster's incidental members take past it is room they never get. + const incidentalCapFor = (p: number, room: number): number | undefined => { + const owedBelow = owedFrom[p + 1]!; + if (owedBelow === 0) return undefined; + const spent = projectedChars + (chosenIndices.size > 0 ? GAP_MARKER.length : 0); + return Math.min(room, fileBudget - spent) - owedBelow; + }; + for (let p = 0; p < rankedClusters.length; p++) { + const rc = rankedClusters[p]!; // The top-ranked cluster is always taken — an empty file section sends the // agent to Read, negating the savings. But "always taken" is not "taken at // any size": when it overruns the reservation it is SHRUNK to the @@ -5987,7 +6081,7 @@ export class ToolHandler { // this holds it to it rather than letting the member rule walk past it. const ceiling = Math.max(cap, SPINE_CEILING); if (first) { - const section = renderCluster(rc.c, cap, ceiling); + const section = renderCluster(rc.c, cap, ceiling, incidentalCapFor(p, cap)); renderedClusters.set(rc.idx, section); anyClusterShrunk = anyClusterShrunk || section.shrunk; chosenIndices.add(rc.idx); @@ -6012,7 +6106,7 @@ export class ToolHandler { // fragments the next call's dedup then has to shred around. const room = cap - projectedChars - GAP_MARKER.length; if (room < EXPLORE_ALLOCATION.MIN_CHARS) continue; - const section = renderCluster(rc.c, room, room); + const section = renderCluster(rc.c, room, room, incidentalCapFor(p, room)); const text = sectionText(section.parts); if (text.length === 0) continue; // The never-empty floors inside the windowing may overrun `room` (a