diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a5c7d5..0dfc0e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Anonymous usage telemetry is now stored entirely on CodeGraph's own first-party infrastructure — no third-party analytics vendor receives any of it, and the endpoint that receives it makes no outbound requests at all. Individual events are deleted after 90 days, leaving only anonymous daily totals. Nothing about what is collected changed, your IP address is still never read or stored, and every off-switch works exactly as before (`codegraph telemetry off`, `CODEGRAPH_TELEMETRY=0`, `DO_NOT_TRACK=1`). `TELEMETRY.md` remains the complete field-by-field list. +- `codegraph_explore` no longer re-sends source it already returned earlier in the same conversation. A file it has already shown you comes back as a short pointer — the path, the symbols and the exact line range, with confirmation that the file hasn't changed since — and the space that frees is spent on code you haven't seen yet, so a follow-up call covers new ground instead of repeating the last one. If a file was edited in between, its source is always shown again in full. Set `CODEGRAPH_EXPLORE_DEDUP=0` to turn this off. + ### Fixes - `codegraph_explore` now concentrates its answer on the code that actually answers your question instead of spreading it across files that merely share a word with it, so more of the answer arrives in a single call. Thanks @LeDuyViet for the detailed measurements and reproduction. (#1500) diff --git a/__tests__/explore-cross-call-dedup.test.ts b/__tests__/explore-cross-call-dedup.test.ts new file mode 100644 index 0000000..608d85c --- /dev/null +++ b/__tests__/explore-cross-call-dedup.test.ts @@ -0,0 +1,360 @@ +/** + * Cross-call source dedup (CG-18). + * + * A later `codegraph_explore` call in a session must not re-send source an + * earlier call already delivered — but every byte it withholds has to be + * replaced by a POINTER, never a silence. That asymmetry is what this suite + * guards, because the two failure directions cost wildly different amounts: a + * duplicate range wastes a few thousand chars, while a response that reads as + * "codegraph doesn't have it" costs a Read — and one or two of those early in a + * session teach an agent to stop calling the tool at all. + * + * Three layers: + * 1. the range algebra — what is withheld, and the thresholds that stop it + * from shredding a block into slivers; + * 2. the fingerprint gate — an edit between two calls must re-emit, since a + * pointer to pre-edit source is worse than no dedup at all; + * 3. the handler seam — a real second call against a real index: no duplicate + * ranges, a pointer for everything withheld, the reclaimed budget spent on + * source the agent has NOT seen, and never an all-pointer response. + */ +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 { ExploreSessionState, type ExploreProjectState } from '../src/mcp/explore-session-state'; +import { + EXPLORE_DEDUP, + dedupeRange, + fileFingerprint, + formatBackReference, + intersectRange, + mergeRanges, + servedRangesForFile, + subtractRange, + symbolsInSpans, +} from '../src/mcp/explore-dedup'; + +const FIXTURE_SRC = path.join(__dirname, 'fixtures', 'payroll-go'); +const QUERY = 'how does payroll cycle create and calculate payslips?'; +const POINTER = 'Already sent earlier in this conversation'; + +/** A prior-state shaped like the session tracker's, for the algebra tests. */ +function prior(files: Array<{ path: string; ranges: Array<[number, number]>; fingerprint?: string }>): ExploreProjectState { + return { + projectRoot: '/repo', + callCount: 1, + responseBytes: 1000, + calls: [{ + index: 1, + projectRoot: '/repo', + query: 'q', + sourceBytes: 500, + responseBytes: 1000, + files: files.map((f) => ({ + path: f.path, + ranges: f.ranges.map(([start, end]) => ({ start, end })), + bytes: 500, + fingerprint: f.fingerprint, + })), + }], + }; +} + +describe('range algebra', () => { + it('subtracts a held span out of the middle of an intended one', () => { + expect(subtractRange({ start: 1, end: 100 }, [{ start: 20, end: 40 }])) + .toEqual([{ start: 1, end: 19 }, { start: 41, end: 100 }]); + }); + + it('subtracts held spans at either edge, and a full cover to nothing', () => { + expect(subtractRange({ start: 10, end: 50 }, [{ start: 1, end: 20 }])) + .toEqual([{ start: 21, end: 50 }]); + expect(subtractRange({ start: 10, end: 50 }, [{ start: 30, end: 90 }])) + .toEqual([{ start: 10, end: 29 }]); + expect(subtractRange({ start: 10, end: 50 }, [{ start: 1, end: 90 }])).toEqual([]); + }); + + it('intersects to exactly what both sides hold', () => { + expect(intersectRange({ start: 10, end: 50 }, [{ start: 1, end: 20 }, { start: 45, end: 80 }])) + .toEqual([{ start: 10, end: 20 }, { start: 45, end: 50 }]); + expect(intersectRange({ start: 10, end: 50 }, [{ start: 60, end: 80 }])).toEqual([]); + }); + + it('merges touching spans — two adjacent blocks are one block of source', () => { + expect(mergeRanges([{ start: 5, end: 9 }, { start: 10, end: 12 }, { start: 40, end: 41 }])) + .toEqual([{ start: 5, end: 12 }, { start: 40, end: 41 }]); + }); + + it('emits ONLY the delta when a later call wants a wider window', () => { + // Call 1 sent the method; call 2 wants the class around it. + const { emit, covered } = dedupeRange({ start: 80, end: 200 }, [{ start: 100, end: 140 }]); + expect(covered).toEqual([{ start: 100, end: 140 }]); + expect(emit).toEqual([{ start: 80, end: 99 }, { start: 141, end: 200 }]); + }); + + it('withholds nothing when the overlap is smaller than a chunk worth pointing at', () => { + // Context padding and signature lines land here. Replacing them costs more + // in pointer text than the source is worth, and shreds the block. + const overlap = EXPLORE_DEDUP.MIN_COVERED_LINES - 1; + const { emit, covered } = dedupeRange({ start: 1, end: 100 }, [{ start: 10, end: 10 + overlap - 1 }]); + expect(covered).toEqual([]); + expect(emit).toEqual([{ start: 1, end: 100 }]); + }); + + it('leaves an untouched span exactly as it was', () => { + expect(dedupeRange({ start: 1, end: 50 }, [{ start: 200, end: 400 }])) + .toEqual({ emit: [{ start: 1, end: 50 }], covered: [] }); + expect(dedupeRange({ start: 1, end: 50 }, [])) + .toEqual({ emit: [{ start: 1, end: 50 }], covered: [] }); + }); +}); + +describe('the fingerprint gate', () => { + const FP = fileFingerprint('package main\nfunc main() {}\n'); + + it('returns the spans a call served for a file whose bytes are unchanged', () => { + const state = prior([{ path: 'a.go', ranges: [[1, 40], [60, 80]], fingerprint: FP }]); + expect(servedRangesForFile(state, 'a.go', FP)).toEqual([{ start: 1, end: 40 }, { start: 60, end: 80 }]); + }); + + it('returns NOTHING once the file has been edited — a pointer would be wrong', () => { + const state = prior([{ path: 'a.go', ranges: [[1, 40]], fingerprint: FP }]); + const edited = fileFingerprint('package main\nfunc main() { changed() }\n'); + expect(servedRangesForFile(state, 'a.go', edited)).toEqual([]); + }); + + it('ignores a record that cannot prove what it served', () => { + const state = prior([{ path: 'a.go', ranges: [[1, 40]] }]); + expect(servedRangesForFile(state, 'a.go', FP)).toEqual([]); + }); + + it('never crosses files, and is empty for an untracked session', () => { + const state = prior([{ path: 'a.go', ranges: [[1, 40]], fingerprint: FP }]); + expect(servedRangesForFile(state, 'b.go', FP)).toEqual([]); + expect(servedRangesForFile(null, 'a.go', FP)).toEqual([]); + }); + + it('distinguishes two files that hash the same prefix but differ in length', () => { + expect(fileFingerprint('abc')).not.toBe(fileFingerprint('abcd')); + expect(fileFingerprint('abc')).toBe(fileFingerprint('abc')); + }); +}); + +describe('the back-reference itself', () => { + const covered = [{ start: 100, end: 240 }]; + + it('names the file, the span and the symbols, and says the copy is still good', () => { + const text = formatBackReference('internal/x.go', covered, ['RunCycle', 'BuildPayslip'], { partial: false }); + expect(text).toContain('internal/x.go'); + expect(text).toContain('L100-240'); + expect(text).toContain('RunCycle, BuildPayslip'); + expect(text).toContain(POINTER); + expect(text).toContain('unchanged on disk'); + }); + + it('never tells the agent to Read, in either shape', () => { + for (const partial of [true, false]) { + const text = formatBackReference('x.go', covered, ['A'], { partial }); + expect(text).toMatch(/do NOT Read/i); + expect(text).not.toMatch(/\bRead (this|the) file (for|to)\b/i); + expect(text).not.toMatch(/omitted|unavailable|could not/i); + } + }); + + it('says the block below is only the NEW lines when the call still sends some', () => { + expect(formatBackReference('x.go', covered, [], { partial: true })).toContain('NEW lines'); + expect(formatBackReference('x.go', covered, [], { partial: false })).toContain('not repeated here'); + }); + + it('names only symbols that actually fall in the withheld spans', () => { + const nodes = [ + { name: 'InSpan', kind: 'function', startLine: 110, endLine: 130 }, + { name: 'Outside', kind: 'function', startLine: 300, endLine: 320 }, + { name: 'AnImport', kind: 'import', startLine: 105, endLine: 105 }, + ]; + expect(symbolsInSpans(nodes, covered)).toEqual(['InSpan']); + }); +}); + +describe('a second call against a real index', () => { + let testDir: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeAll(async () => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cg18-')); + fs.cpSync(FIXTURE_SRC, testDir, { recursive: true }); + fs.rmSync(path.join(testDir, '.codegraph'), { recursive: true, force: true }); + cg = CodeGraph.initSync(testDir); + await cg.indexAll(); + handler = new ToolHandler(cg); + }, 120_000); + + afterAll(() => { + if (cg) cg.destroy(); + if (testDir && fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); + }); + + const explore = (query: string, session?: ExploreSessionState, args: Record = {}) => + handler.execute('codegraph_explore', { query, ...args }, session).then((r) => r.content[0]!.text); + + /** + * The line numbers actually inside each file's fenced source. Read off the + * RESPONSE, not the bookkeeping — "no duplicate ranges" is a claim about what + * the agent received, and checking it against the record we also wrote would + * prove only that the two agree. + */ + function fencedLines(text: string): Map> { + const out = new Map>(); + let current: string | null = null; + let inFence = false; + for (const line of text.split('\n')) { + const header = /^\*\*`([^`]+)`\*\*/.exec(line); + if (header && !inFence) { current = header[1]!; continue; } + if (!inFence && current && line.startsWith('```')) { inFence = true; continue; } + if (inFence && line === '```') { inFence = false; continue; } + if (!inFence || !current) continue; + const numbered = /^(\d+)\t/.exec(line); + if (!numbered) continue; + if (!out.has(current)) out.set(current, new Set()); + out.get(current)!.add(Number(numbered[1])); + } + return out; + } + + it('never re-sends a line it already sent, and points at every line it withholds', async () => { + const session = new ExploreSessionState(); + const first = await explore(QUERY, session); + const second = await explore(QUERY, session); + + const before = fencedLines(first); + const after = fencedLines(second); + expect(after.size).toBeGreaterThan(0); + + // Every file whose source the second call withheld carries a pointer, and + // the pointer names it. + expect(second).toContain(POINTER); + for (const [file, lines] of before) { + const repeated = [...(after.get(file) ?? [])].filter((n) => lines.has(n)); + if (repeated.length === 0) continue; + // The only sanctioned repeat is the anti-abandonment restore, which fires + // ONLY when the call found nothing new to say — and this one did. + throw new Error(`call 2 re-sent ${file} lines ${repeated.slice(0, 5).join(',')}`); + } + }, 120_000); + + it('spends the reclaimed bytes on source the agent has not seen', async () => { + const session = new ExploreSessionState(); + const first = await explore(QUERY, session); + const second = await explore(QUERY, session); + + const before = fencedLines(first); + const after = fencedLines(second); + const fresh = [...after.entries()].reduce( + (sum, [file, lines]) => sum + [...lines].filter((n) => !(before.get(file)?.has(n))).length, 0); + // Not merely "smaller": a shrunken response is what dedup must NOT produce. + // The freed budget has to come back as lines the first call never sent. + expect(fresh).toBeGreaterThan(20); + expect(second.length).toBeLessThan(first.length); + }, 120_000); + + it('re-emits in full when the file changed between the two calls', async () => { + const session = new ExploreSessionState(); + const target = path.join(testDir, 'internal/usecase/payroll/payslip_builder.go'); + const original = fs.readFileSync(target, 'utf-8'); + try { + const first = await explore(QUERY, session); + expect(fencedLines(first).has('internal/usecase/payroll/payslip_builder.go')).toBe(true); + + fs.writeFileSync(target, original.replace('func sumKind(', 'func sumKindRenamed('), 'utf-8'); + const second = await explore(QUERY, session); + + // The edited file is served again, whole — a pointer here would send the + // agent to a copy of the file that no longer exists. + const pointerLines = second.split('\n').filter((l) => l.includes(POINTER)); + expect(pointerLines.some((l) => l.includes('payslip_builder.go'))).toBe(false); + expect(fencedLines(second).get('internal/usecase/payroll/payslip_builder.go')?.size ?? 0) + .toBeGreaterThan(20); + } finally { + fs.writeFileSync(target, original, 'utf-8'); + } + }, 120_000); + + it('always returns real source, even when the session already holds everything', async () => { + const session = new ExploreSessionState(); + await explore(QUERY, session); + await explore(QUERY, session); + const third = await explore(QUERY, session); + const fourth = await explore(QUERY, session); + + // An all-pointer response is the shape that reads as failure. Every call + // keeps at least one real fenced block, however much the session holds. + for (const [n, text] of [[3, third], [4, fourth]] as const) { + const lines = [...fencedLines(text).values()].reduce((s, set) => s + set.size, 0); + expect(lines, `call ${n} returned no source at all`).toBeGreaterThan(10); + } + }, 180_000); + + it('leaves the first call of a session untouched', async () => { + const tracked = await explore(QUERY, new ExploreSessionState()); + const untracked = await explore(QUERY); + expect(tracked).toBe(untracked); + }, 120_000); + + it('keeps two sessions on one handler independent', async () => { + const a = new ExploreSessionState(); + const b = new ExploreSessionState(); + const firstForA = await explore(QUERY, a); + await explore(QUERY, a); + // B's first call has seen nothing, whatever A has been served. + expect(await explore(QUERY, b)).toBe(firstForA); + }, 180_000); + + it('is off entirely under CODEGRAPH_EXPLORE_DEDUP=0', async () => { + const session = new ExploreSessionState(); + const previous = process.env.CODEGRAPH_EXPLORE_DEDUP; + process.env.CODEGRAPH_EXPLORE_DEDUP = '0'; + try { + const first = await explore(QUERY, session); + const second = await explore(QUERY, session); + expect(second).toBe(first); + expect(second).not.toContain(POINTER); + } finally { + if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEDUP; + else process.env.CODEGRAPH_EXPLORE_DEDUP = previous; + } + }, 120_000); + + it('reports the reclaimed bytes through the CG-4 diagnostic', async () => { + const sidecar = path.join(testDir, 'cg18-diagnostic.jsonl'); + const session = new ExploreSessionState(); + const previous = process.env.CODEGRAPH_EXPLORE_DEBUG; + process.env.CODEGRAPH_EXPLORE_DEBUG = sidecar; + try { + await explore(QUERY, session); + await explore(QUERY, session); + } finally { + if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEBUG; + else process.env.CODEGRAPH_EXPLORE_DEBUG = previous; + } + const [one, two] = fs.readFileSync(sidecar, 'utf-8').trim().split('\n').map((l) => JSON.parse(l)); + + expect(one.dedup.savedChars).toBe(0); + expect(two.dedup.savedChars).toBeGreaterThan(1000); + expect(two.dedup.backReferenced.length).toBeGreaterThan(0); + + // The reclamation is legible file by file: a back-referenced file spent + // none of its reservation, and the response still filled its envelope. + const backref = two.files.filter((f: { render: string }) => f.render === 'backref'); + for (const f of backref) { + expect(f.emittedChars).toBe(0); + expect(f.dedupSavedChars).toBeGreaterThan(0); + expect(f.dedupCovered.length).toBeGreaterThan(0); + } + const spentOnFreshSource = two.files.reduce((s: number, f: { emittedChars: number }) => s + f.emittedChars, 0); + expect(spentOnFreshSource).toBeGreaterThan(0); + }, 120_000); +}); diff --git a/__tests__/explore-session-state.test.ts b/__tests__/explore-session-state.test.ts new file mode 100644 index 0000000..c0b71d9 --- /dev/null +++ b/__tests__/explore-session-state.test.ts @@ -0,0 +1,469 @@ +/** + * Session-scoped explore call state (CG-17). + * + * The tracker is the foundation for cross-call dedup (CG-18) and budget decay + * (CG-19), so what it must get right is what those two will trust: the count of + * calls, the line ranges already served, and — above all — WHOSE they are. Two + * agents on one daemon share a ToolHandler and a worker pool; if their histories + * blend, a dedup built on this would withhold source from an agent that never + * saw it, and the agent Reads the file. That is the failure this suite guards. + * + * Three layers: + * 1. the state container itself — keying, monotonic call index, bounds; + * 2. the handler seam — a real explore against a real index records real + * ranges, and the emission side-channel NEVER reaches the response; + * 3. the session seam — separate sessions on one engine, separate state. + */ +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 { MCPSession } from '../src/mcp/session'; +import type { MCPEngine } from '../src/mcp/engine'; +import type { JsonRpcTransport, JsonRpcRequest, JsonRpcNotification } from '../src/mcp/transport'; +import { + EXPLORE_EMISSION_KEY, + EXPLORE_SESSION_LIMITS, + EXPLORE_SESSION_VIEW_ARG, + ExploreSessionState, + coalesceRanges, + exploreProjectKey, + rangesCover, + readExploreSessionView, + viewForProject, + type ExploreEmission, +} from '../src/mcp/explore-session-state'; + +const FIXTURE_SRC = path.join(__dirname, 'fixtures', 'payroll-go'); +const QUERY = 'how does payroll cycle create and calculate payslips?'; + +/** An emission shaped like a real one, for the container-level tests. */ +function emission(root: string, over: Partial = {}): ExploreEmission { + return { + projectRoot: root, + query: 'q', + files: [{ path: 'a.ts', ranges: [{ start: 1, end: 10 }], bytes: 100 }], + sourceBytes: 100, + responseBytes: 400, + ...over, + }; +} + +describe('ExploreSessionState — the container', () => { + it('counts calls per project and hands back a 1-based session index', () => { + const state = new ExploreSessionState(); + expect(state.record(emission('/repo/a'))?.index).toBe(1); + expect(state.record(emission('/repo/a'))?.index).toBe(2); + expect(state.callCount('/repo/a')).toBe(2); + expect(state.forProject('/repo/a')?.responseBytes).toBe(800); + }); + + it('keys state per project — a second project starts its own count', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + state.record(emission('/repo/a')); + expect(state.record(emission('/repo/b'))?.index).toBe(1); + expect(state.callCount('/repo/a')).toBe(2); + expect(state.callCount('/repo/b')).toBe(1); + expect(state.forProject('/repo/b')?.calls).toHaveLength(1); + }); + + it('treats trailing slashes and `.` segments as the same project', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + state.record(emission('/repo/a/')); + state.record(emission('/repo/a/./')); + expect(state.callCount('/repo/a')).toBe(3); + expect(state.snapshot()).toHaveLength(1); + }); + + it('never reports a project it was never told about', () => { + const state = new ExploreSessionState(); + expect(state.forProject('/never/queried')).toBeNull(); + expect(state.callCount('/never/queried')).toBe(0); + }); + + it('keeps counting past the retained-call bound — decay must not reset itself', () => { + const state = new ExploreSessionState(); + const total = EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED + 5; + for (let i = 0; i < total; i++) state.record(emission('/repo/a')); + const project = state.forProject('/repo/a')!; + expect(project.callCount).toBe(total); + expect(project.calls).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED); + // Detail is dropped from the OLDEST end; the newest call is always retained. + expect(project.calls[project.calls.length - 1]!.index).toBe(total); + expect(project.calls[0]!.index).toBe(total - EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED + 1); + }); + + it('bounds the number of projects, evicting the least recently used', () => { + const state = new ExploreSessionState(); + const roots = Array.from({ length: EXPLORE_SESSION_LIMITS.MAX_PROJECTS + 2 }, (_, i) => `/repo/${i}`); + for (const root of roots) state.record(emission(root)); + expect(state.snapshot()).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_PROJECTS); + expect(state.forProject(roots[0]!)).toBeNull(); + expect(state.forProject(roots[roots.length - 1]!)).not.toBeNull(); + }); + + it('keeps a re-queried project alive past newer ones', () => { + const state = new ExploreSessionState(); + const roots = Array.from({ length: EXPLORE_SESSION_LIMITS.MAX_PROJECTS }, (_, i) => `/repo/${i}`); + for (const root of roots) state.record(emission(root)); + state.record(emission(roots[0]!)); // touch the oldest + state.record(emission('/repo/newcomer')); // forces one eviction + expect(state.forProject(roots[0]!)?.callCount).toBe(2); + expect(state.forProject(roots[1]!)).toBeNull(); + }); + + it('bounds files per call, keeping the ones that got the most source', () => { + const state = new ExploreSessionState(); + const files = Array.from({ length: EXPLORE_SESSION_LIMITS.MAX_FILES_PER_CALL + 6 }, (_, i) => ({ + path: `f${i}.ts`, + ranges: [{ start: 1, end: 5 }], + bytes: i + 1, + })); + state.record(emission('/repo/a', { files })); + const kept = state.forProject('/repo/a')!.calls[0]!.files; + expect(kept).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_FILES_PER_CALL); + expect(kept.map((f) => f.path)).toContain(`f${files.length - 1}.ts`); + expect(kept.map((f) => f.path)).not.toContain('f0.ts'); + }); + + it('ignores an emission with no project root rather than filing it under ""', () => { + const state = new ExploreSessionState(); + expect(state.record({ ...emission(''), projectRoot: '' })).toBeNull(); + expect(state.snapshot()).toHaveLength(0); + }); + + it('hands out copies — a caller cannot mutate the record it read', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + const snap = state.forProject('/repo/a')!; + snap.calls[0]!.files[0]!.ranges.push({ start: 999, end: 1000 }); + expect(state.forProject('/repo/a')!.calls[0]!.files[0]!.ranges).toHaveLength(1); + }); + + it('view() carries only the most recent calls per project', () => { + const state = new ExploreSessionState(); + for (let i = 0; i < EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED; i++) state.record(emission('/repo/a')); + const view = state.view(); + expect(view.projects[0]!.callCount).toBe(EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED); + expect(view.projects[0]!.calls).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_VIEW_CALLS); + expect(viewForProject(view, '/repo/a')?.callCount).toBe(EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED); + // A tracked session that hasn't touched this project yet reads as EMPTY, + // not untracked — only a missing view (nobody tracking) is null. + expect(viewForProject(view, '/repo/other')?.callCount).toBe(0); + expect(viewForProject(null, '/repo/a')).toBeNull(); + }); +}); + +describe('range bookkeeping', () => { + it('merges overlapping and adjacent spans into one', () => { + const { ranges, truncated } = coalesceRanges([ + { start: 10, end: 20 }, + { start: 15, end: 25 }, // overlaps + { start: 26, end: 30 }, // adjacent — one contiguous block of source + { start: 60, end: 61 }, + ]); + expect(ranges).toEqual([{ start: 10, end: 30 }, { start: 60, end: 61 }]); + expect(truncated).toBe(false); + }); + + it('drops junk spans instead of recording a range that was never served', () => { + const { ranges } = coalesceRanges([ + { start: 5, end: 1 }, // inverted + { start: 0, end: 3 }, // before line 1 + { start: NaN, end: 4 }, + { start: 7, end: 9 }, + ]); + expect(ranges).toEqual([{ start: 7, end: 9 }]); + }); + + it('caps the range list by KEEPING the largest spans, and says it truncated', () => { + // Spaced far enough apart that none of them merge — this is about the cap. + const many = Array.from({ length: EXPLORE_SESSION_LIMITS.MAX_RANGES_PER_FILE + 5 }, (_, i) => ({ + start: i * 200 + 1, + end: i * 200 + 2 + i, // later spans are longer + })); + const { ranges, truncated } = coalesceRanges(many); + expect(truncated).toBe(true); + expect(ranges).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_RANGES_PER_FILE); + // Still in line order, and the biggest span survived. + expect(ranges.map((r) => r.start)).toEqual([...ranges.map((r) => r.start)].sort((a, b) => a - b)); + expect(ranges.some((r) => r.start === many[many.length - 1]!.start)).toBe(true); + }); + + it('flags truncation on the stored record so a consumer knows it under-knows', () => { + const state = new ExploreSessionState(); + const ranges = Array.from({ length: EXPLORE_SESSION_LIMITS.MAX_RANGES_PER_FILE + 3 }, (_, i) => ({ + start: i * 10 + 1, end: i * 10 + 4, + })); + state.record(emission('/repo/a', { files: [{ path: 'big.ts', ranges, bytes: 900 }] })); + expect(state.forProject('/repo/a')!.calls[0]!.files[0]!.rangesTruncated).toBe(true); + }); + + it('answers whether a line was already served', () => { + const ranges = [{ start: 10, end: 20 }, { start: 40, end: 41 }]; + expect(rangesCover(ranges, 10)).toBe(true); + expect(rangesCover(ranges, 20)).toBe(true); + expect(rangesCover(ranges, 21)).toBe(false); + expect(rangesCover(ranges, 40)).toBe(true); + }); + + it('folds case only on the case-insensitive platforms', () => { + const insensitive = process.platform === 'darwin' || process.platform === 'win32'; + expect(exploreProjectKey('/Repo/A') === exploreProjectKey('/repo/a')).toBe(insensitive); + }); +}); + +describe('session view arriving on tool args', () => { + it('reads a well-formed view and ignores anything else', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + expect(readExploreSessionView({ [EXPLORE_SESSION_VIEW_ARG]: state.view() })?.projects).toHaveLength(1); + expect(readExploreSessionView({})).toBeNull(); + expect(readExploreSessionView({ [EXPLORE_SESSION_VIEW_ARG]: 'nope' })).toBeNull(); + expect(readExploreSessionView({ [EXPLORE_SESSION_VIEW_ARG]: { projects: 'nope' } })).toBeNull(); + }); + + it('drops malformed project entries rather than trusting them', () => { + const view = readExploreSessionView({ + [EXPLORE_SESSION_VIEW_ARG]: { projects: [{ projectRoot: '/repo/a', calls: [] }, { nope: 1 }, null] }, + }); + expect(view?.projects).toHaveLength(1); + }); +}); + +describe('explore records what it actually served', () => { + let testDir: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeAll(async () => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cg17-')); + fs.cpSync(FIXTURE_SRC, testDir, { recursive: true }); + fs.rmSync(path.join(testDir, '.codegraph'), { recursive: true, force: true }); + cg = CodeGraph.initSync(testDir); + await cg.indexAll(); + handler = new ToolHandler(cg); + }, 120_000); + + afterAll(() => { + if (cg) cg.destroy(); + if (testDir && fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('files one record per call, with the files and line ranges it emitted', async () => { + const session = new ExploreSessionState(); + await handler.execute('codegraph_explore', { query: QUERY }, session); + + const project = session.forProject(cg.getProjectRoot()); + expect(project).not.toBeNull(); + expect(project!.callCount).toBe(1); + + const call = project!.calls[0]!; + expect(call.files.length).toBeGreaterThan(0); + expect(call.sourceBytes).toBeGreaterThan(0); + expect(call.responseBytes).toBeGreaterThan(call.sourceBytes); + for (const file of call.files) { + expect(file.ranges.length).toBeGreaterThan(0); + for (const r of file.ranges) { + expect(r.start).toBeGreaterThanOrEqual(1); + expect(r.end).toBeGreaterThanOrEqual(r.start); + } + } + }, 60_000); + + it('records only files whose source is really in the response', async () => { + const session = new ExploreSessionState(); + const result = await handler.execute('codegraph_explore', { query: QUERY }, session); + const text = result.content[0]!.text; + for (const file of session.forProject(cg.getProjectRoot())!.calls[0]!.files) { + expect(text).toContain(file.path); + } + }, 60_000); + + it('the recorded ranges name lines that are really in the emitted source', async () => { + const session = new ExploreSessionState(); + await handler.execute('codegraph_explore', { query: QUERY }, session); + for (const file of session.forProject(cg.getProjectRoot())!.calls[0]!.files) { + const lineCount = fs.readFileSync(path.join(testDir, file.path), 'utf-8').split('\n').length; + for (const r of file.ranges) expect(r.end).toBeLessThanOrEqual(lineCount); + } + }, 60_000); + + it('leaves the agent-facing response untouched — no side-channel on the wire', async () => { + const session = new ExploreSessionState(); + const tracked = await handler.execute('codegraph_explore', { query: QUERY }, session); + const untracked = await handler.execute('codegraph_explore', { query: QUERY }); + + expect(tracked.content[0]!.text).toBe(untracked.content[0]!.text); + for (const result of [tracked, untracked]) { + expect(EXPLORE_EMISSION_KEY in result).toBe(false); + expect(JSON.stringify(result)).not.toContain(EXPLORE_EMISSION_KEY); + } + }, 60_000); + + it('ignores a session view a client spelled itself — the record is the server\'s', async () => { + const forged = { + projects: [{ projectRoot: cg.getProjectRoot(), callCount: 99, responseBytes: 1e6, calls: [] }], + }; + const result = await handler.execute('codegraph_explore', { + query: QUERY, + [EXPLORE_SESSION_VIEW_ARG]: forged, + }); + const clean = await handler.execute('codegraph_explore', { query: QUERY }); + expect(result.content[0]!.text).toBe(clean.content[0]!.text); + }, 60_000); + + it('counts an empty answer as a call, since it still spends the tier budget', async () => { + const session = new ExploreSessionState(); + await handler.execute('codegraph_explore', { query: 'zzqqxx_no_such_symbol_anywhere' }, session); + const project = session.forProject(cg.getProjectRoot()); + expect(project?.callCount).toBe(1); + expect(project?.calls[0]!.files).toHaveLength(0); + }, 60_000); + + it('two sessions on ONE handler never see each other\'s calls', async () => { + const a = new ExploreSessionState(); + const b = new ExploreSessionState(); + await handler.execute('codegraph_explore', { query: QUERY }, a); + await handler.execute('codegraph_explore', { query: QUERY }, a); + await handler.execute('codegraph_explore', { query: QUERY }, b); + + expect(a.callCount(cg.getProjectRoot())).toBe(2); + expect(b.callCount(cg.getProjectRoot())).toBe(1); + }, 90_000); + + it('a caller that tracks nothing still gets a clean result', async () => { + const result = await handler.execute('codegraph_explore', { query: QUERY }); + expect(result.isError).toBeFalsy(); + expect(result.content[0]!.text.length).toBeGreaterThan(0); + }, 60_000); + + it('reports the session state through the CG-4 diagnostic', async () => { + const sidecar = path.join(testDir, 'cg17-diagnostic.jsonl'); + const session = new ExploreSessionState(); + const previous = process.env.CODEGRAPH_EXPLORE_DEBUG; + process.env.CODEGRAPH_EXPLORE_DEBUG = sidecar; + try { + await handler.execute('codegraph_explore', { query: QUERY }, session); + await handler.execute('codegraph_explore', { query: QUERY }, session); + } finally { + if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEBUG; + else process.env.CODEGRAPH_EXPLORE_DEBUG = previous; + } + + const reports = fs.readFileSync(sidecar, 'utf-8').trim().split('\n').map((l) => JSON.parse(l)); + expect(reports).toHaveLength(2); + // The first call is the session's first: nothing served before it. + expect(reports[0].session).toEqual({ + callIndex: 1, priorCalls: 0, priorResponseChars: 0, priorFiles: [], + }); + // The second sees the first call's files and their ranges. + expect(reports[1].session.callIndex).toBe(2); + expect(reports[1].session.priorCalls).toBe(1); + expect(reports[1].session.priorResponseChars).toBeGreaterThan(0); + expect(reports[1].session.priorFiles.length).toBeGreaterThan(0); + expect(reports[1].session.priorFiles[0].ranges[0]).toHaveLength(2); + }, 90_000); + + it('omits the session block entirely when the caller tracks no state', async () => { + const sidecar = path.join(testDir, 'cg17-untracked.jsonl'); + const previous = process.env.CODEGRAPH_EXPLORE_DEBUG; + process.env.CODEGRAPH_EXPLORE_DEBUG = sidecar; + try { + await handler.execute('codegraph_explore', { query: QUERY }); + } finally { + if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEBUG; + else process.env.CODEGRAPH_EXPLORE_DEBUG = previous; + } + const report = JSON.parse(fs.readFileSync(sidecar, 'utf-8').trim()); + expect(report.session).toBeUndefined(); + }, 60_000); + + it('keys on the RESOLVED root, not the path the agent typed', async () => { + // The same project reached two ways — bare, and via a `projectPath` pointing + // at a subdirectory. Both resolve to one index, so both must land in one + // bucket; keying on the typed path would split a session's history in two + // and hand a later call a half-empty record. + // + // (Two genuinely DIFFERENT projects can't be exercised here: opening a + // second index inside vitest fails on the lazy `require('../index')` — see + // the ToolHandler cache notes. The container-level tests above cover the + // multi-project keying itself.) + const session = new ExploreSessionState(); + await handler.execute('codegraph_explore', { query: QUERY }, session); + await handler.execute( + 'codegraph_explore', + { query: QUERY, projectPath: path.join(testDir, 'internal') }, + session, + ); + + expect(session.snapshot()).toHaveLength(1); + expect(session.callCount(cg.getProjectRoot())).toBe(2); + }, 90_000); +}); + +describe('sessions sharing a daemon', () => { + /** Minimal transport: captures the message handler so a test can drive it. */ + function fakeTransport(): JsonRpcTransport & { deliver: (m: JsonRpcRequest) => Promise; results: unknown[] } { + let handle: ((m: JsonRpcRequest | JsonRpcNotification) => Promise) | null = null; + const results: unknown[] = []; + return { + start(h) { handle = h as typeof handle; }, + stop() { /* nothing to tear down */ }, + send() { /* unused */ }, + notify() { /* unused */ }, + async request() { return {}; }, + sendResult(_id, result) { results.push(result); }, + sendError() { /* unused */ }, + results, + async deliver(m: JsonRpcRequest) { await handle?.(m); }, + }; + } + + it('give each session its own state, and one session\'s calls stay there', async () => { + const calls: Array = []; + // A ToolHandler stand-in: the point here is WHICH state object arrives, not + // what explore returns, so a real index would only slow the assertion down. + const handler = { + getTools: () => [], + execute: async (_tool: string, _args: Record, state?: ExploreSessionState) => { + calls.push(state); + state?.record(emission('/repo/shared')); + return { content: [{ type: 'text' as const, text: 'ok' }] }; + }, + }; + const engine = { + ensureInitialized: async () => { /* already open */ }, + hasDefaultCodeGraph: () => true, + getProjectPath: () => '/repo/shared', + retryInitializeSync: () => { /* nothing to retry */ }, + getToolHandler: () => handler, + } as unknown as MCPEngine; + + const transportA = fakeTransport(); + const transportB = fakeTransport(); + const sessionA = new MCPSession(transportA, engine); + const sessionB = new MCPSession(transportB, engine); + sessionA.start(); + sessionB.start(); + + expect(sessionA.getExploreSessionState()).not.toBe(sessionB.getExploreSessionState()); + + const call = (id: number): JsonRpcRequest => ({ + jsonrpc: '2.0', id, method: 'tools/call', + params: { name: 'codegraph_explore', arguments: { query: 'q' } }, + }); + await transportA.deliver(call(1)); + await transportA.deliver(call(2)); + await transportB.deliver(call(3)); + + expect(calls[0]).toBe(sessionA.getExploreSessionState()); + expect(calls[2]).toBe(sessionB.getExploreSessionState()); + expect(sessionA.getExploreSessionState().callCount('/repo/shared')).toBe(2); + expect(sessionB.getExploreSessionState().callCount('/repo/shared')).toBe(1); + }); +}); diff --git a/docs/benchmarks/explore-dedup-ab-cg20.md b/docs/benchmarks/explore-dedup-ab-cg20.md new file mode 100644 index 0000000..eb257ea --- /dev/null +++ b/docs/benchmarks/explore-dedup-ab-cg20.md @@ -0,0 +1,240 @@ +# Agent A/B — cross-call explore dedup (epic CG-2 / task CG-20) + +**Date:** 2026-08-05 · **New:** `feature/CG-2` @ `7a7ea30` (CG-17 session state + CG-18 dedup) +· **Baseline:** `c65d56c` **by SHA** (main's tip when the epic branched) · **Harness:** +`scripts/agent-eval/ab-new-vs-baseline.sh`, `--model sonnet --effort high`, **both arms +codegraph-on**, `CODEGRAPH_NO_PROMPT_HOOK=1` on both. + +This is the epic's hard gate. Returning *less* on a repeat call is the exact shape CLAUDE.md +says drives Read fallback and then teaches the agent to abandon codegraph for the rest of the +session, so the gate is about risk first and win second. + +**Verdict: bars 1–3 pass cleanly and bar 4 is not met.** Read is **0 in all 24 runs of both +arms**, nothing abandons, and the two failure buckets never fire once. Residual context +occupancy is **flat** — and the measurement below shows it could never have been anything else, +because CG-18's own acceptance requires reclaimed bytes to be **re-spent on files the agent has +not seen** rather than banked. What the change actually moves is the *duplicate fraction* of +that residual: **−87% across the agent runs**, −86%/−94% deterministic. + +Recommendation: **keep**, with bar 4 restated. Reasoning and the counter-case in +[§Verdict](#verdict). + +--- + +## Method + +Dedup only exists inside one MCP session, so a single-call task cannot exercise it at all. Both +targets were driven with the drill-down question from the CG-1/CG-22 A/B, which reliably +produces a second and third explore whose symbol bags overlap the first: + +| Repo | Lang | Files | Tier | Question | +|---|---|---|---|---| +| `kubernetes/client-go` | Go | 2,454 | medium (2 calls / 28K) | "how does a shared informer keep its cache in sync and deliver events?" | +| `excalidraw/excalidraw` | TS/React | 672 | medium (2 calls / 28K) | "how does updating an element re-render the canvas on screen?" | + +Each prompt is wrapped `Use codegraph to answer: ` — identical on every arm, the CG-22 +wrapper. That is **not** a forced-Read-0: fallback stays free, which is exactly what bar 1 +measures. + +`RUNS=3` per invocation. client-go ran one batch (n=3/arm); excalidraw ran **two** (n=6/arm, +pooled) because it is where dedup bites hardest and therefore where the abandonment risk is +highest — its baseline sequence duplicates ~21% of the source it serves. + +### Instruments + +Three, because no single one answers the gate: + +- **CG-7 residual occupancy** and **CG-8 sufficiency buckets**, from the `feature/CG-3` copy of + `parse-run.mjs` (that branch is where all three feedback metrics live; this branch's copy + predates them). Bars 2–4. +- **A duplicate-residual measure**, written for this gate. CG-7's occupancy counts the chars of + codegraph results resident in the window but **cannot tell a byte the agent already holds from + one it has never seen** — which is the only distinction dedup makes. This one reads the + *rendered markdown* of every explore response in a run (so it measures both arms the same way; + the CG-4 diagnostic sidecar exists only on the new build), reconstructs the `(file, source + line)` pairs each call put in the window from the `\t` fences, and charges a line + already delivered by an earlier call as a duplicate byte. + + It is deliberately **not** committed as a new `scripts/agent-eval/*.mjs`: a new file there + scores into the self-query eval fixture's own corpus and moves its numbers (the CG-15 observer + effect), and the natural home is `parse-run.mjs` **on `feature/CG-3`** beside the other three + metrics. Fold it in there; the rule above is the whole specification. + +--- + +## Deterministic core — no agent + +Same index, same query sequence (lifted verbatim from a prior new-arm agent run), replayed +through **one** `ToolHandler` + **one** `ExploreSessionState` on each build. Re-measured on both +builds in this session rather than quoted. + +### client-go — 3 calls + +| | baseline `c65d56c` | new `7a7ea30` | +|---|---|---| +| response chars | 65,218 | **67,289 (+3.2%)** | +| source chars | 46,555 | 47,209 | +| **unique source** | 44,740 | **46,957 (+5.0%)** | +| **duplicate source** | **1,815 (3.9%)** | **252 (0.5%) — −86%** | + +### excalidraw — 3 calls + +| | baseline `c65d56c` | new `7a7ea30` | +|---|---|---| +| response chars | 72,364 | **68,442 (−5.4%)** | +| source chars | 50,063 | 44,578 | +| **unique source** | 39,575 | **43,973 (+11.1%)** | +| **duplicate source** | **10,488 (20.9%)** | **605 (1.4%) — −94%** | + +The two repos bracket the mechanism. Where the baseline barely duplicates (client-go, 3.9%) +there is almost nothing to reclaim, and the reclaimed bytes plus the pointer text make the +response marginally *larger*. Where it duplicates heavily (excalidraw, 20.9%) the response gets +**smaller and denser at the same time** — 5.4% fewer bytes carrying 11.1% more unique source. + +**The ceiling on any occupancy win is the baseline's duplicate fraction**, and that is the whole +argument about bar 4: even a design that banked every reclaimed byte instead of re-spending it +could not have removed more than 3.9% / 20.9% of the source in these two sequences. + +### Explore latency — the change is not a slowdown + +Median of 5 replays of the 3-call sequence, per build (CG-18 adds a truncated SHA256 per served +slice, so this needed checking): + +| repo | baseline | new | +|---|---|---| +| client-go | 1,230 ms | 1,287 ms (+4.6%) | +| excalidraw | 458 ms | 445 ms (−2.8%) | + +≤60 ms across three calls. Nothing here can explain a several-second agent gap — see +[§Counter-points](#counter-points). + +--- + +## Agent A/B — the four bars + +`explore` = `codegraph_explore` calls · `cgResidual` = codegraph chars still resident at end of +run, CG-7 · `dup%` = share of served source the agent had already been given. + +| repo | arm | n | explore | **Read** | Grep | cgResidual (med) | per call | dur (med) | **dup%** | +|---|---|---|---|---|---|---|---|---|---| +| client-go | **new** | 3 | 2 / 3 / 2 | **0 / 0 / 0** | 0 | 19,446 | 9,641 | 31s | **0.7%** | +| client-go | baseline | 3 | 3 / 2 / 2 | 0 / 0 / 0 | 0 | 19,611 | 9,379 | 27s | 7.9% | +| excalidraw | **new** | 6 | 3,3,1,2,3,2 | **0 ×6** | 0 | 25,688 | 10,316 | 31s | **0.7–0.8%** | +| excalidraw | baseline | 6 | 2,2,2,1,3,2 | 0 ×6 | 0 | 20,123 | 10,158 | 23s | 3.3–7.6% | + +### Bar 1 — Read count must not increase · **PASS** + +**Read = 0 and Grep = 0 in all 24 runs, both arms, both repos.** Not "did not increase" — never +fired. The strongest form of this bar: back-references actually reached the agent in **8 of the +9 multi-call new-arm runs** (28 pointers total), and no run followed a pointer with a Read. + +### Bar 2 — no abandonment · **PASS** + +The failure mode is silent, so it was measured three ways, all clean across 24 runs: + +- **Zero `isError` responses** in either arm. (One or two early in a session is what teaches + abandonment; there were none.) +- **codegraph is the last tool called in every single run** — 0 Read/Grep/Glob/Bash calls after + the final codegraph call, in both arms. +- Call counts do not collapse in the new arm: 2–3 on client-go, 1–3 on excalidraw, the same + spread the baseline shows. + +### Bar 3 — sufficiency buckets must not shift · **PASS** + +The two buckets this epic could plausibly break are **"Read a file we returned"** (we clipped +the wrong thing) and **"Read a file we did not return"**. Over 40 answered explore calls: + +| bucket | new | baseline | +|---|---|---| +| Read a file we returned | **0** | 0 | +| Read a file we did not return | **0** | 0 | +| Grep/Glob | **0** | 0 | +| explore again | 12 of 21 (57.1%) | 10 of 19 (52.6%) | +| moved on / answered | 9 | 9 | + +The failure buckets are empty on both arms. The `explore again` difference is **one call** at +n=21/19 — noise, and CG-1 already established that these are voluntary drill-downs after a +complete answer, not insufficiency retries (which is why CG-19 was cut). + +### Bar 4 — residual occupancy must actually drop · **NOT MET** + +Per-run `cgResidual` is dominated by how many calls the agent chose to make, and both arms span +1–3 calls on excalidraw. Normalising that out, **residual per explore call is flat**: +2.8% on +client-go (9,641 vs 9,379), +1.6% on excalidraw (10,316 vs 10,158). + +This is **by construction, not by accident**. CG-18's acceptance says in as many words: +"*Freed budget — bytes reclaimed by dedup should flow to files not yet shown, not shrink the +response*," and `emitFileSection` implements exactly that (a fully-held file frees both its +`sourceSpent` into the carry-forward pool and its `maxFiles` slot). A design that spends every +reclaimed byte cannot lower the byte count. **CG-18's acceptance and CG-20's bar 4 are +mutually unsatisfiable**; that contradiction, not a defect in the dedup, is what this bar found. + +What the epic *does* move, measured on the same runs: + +| | new | baseline | +|---|---|---| +| duplicate source chars, all agent runs | **2,432 of 329,222 (0.74%)** | 19,295 of 300,750 (6.4%) | + +**−87% duplicated bytes**, at flat cost per call, with more unique source in their place. + +--- + +## Counter-points + +Kept in the record rather than smoothed: + +- **excalidraw's new arm is slower at the median: 31s vs 23s**, and its median call count is + 2.5 vs 2. It is *not* server-side — explore's own latency is −2.8% there and the deterministic + response for the identical 3-query sequence is **5.4% smaller** on the new build, so the extra + call is not the agent compensating for a thinner answer (and bar 3's failure buckets are + empty). But at n=6 with both arms spanning 1–3 calls, the difference is one call and **this + measurement cannot attribute it to the build either way**. client-go shows the same-sized gap + (31s vs 27s) at an *identical* median call count. Treat as unresolved; a bigger n is the only + thing that settles it. +- **`dedup.savedChars` in the CG-4 diagnostic is a pre-clip figure and must not be read as + bytes kept out of the window.** On client-go call 2 it reports **11,450 saved** while the + baseline actually re-served only **1,042** duplicate chars of that file — the suppressed + ranges are measured against the *unclipped candidate* render, most of which the budget + allocator would have trimmed anyway. Section-level check on the same call: baseline emits 232 + lines of `shared_informer.go` overlapping call 1 by 22 lines; the new build emits 234 lines + overlapping by **0**, and is 544 chars *larger*. Anyone tuning `EXPLORE_DEDUP`'s thresholds off + `savedChars` will over-estimate the win by roughly 7×. +- **client-go's baseline duplicated less than expected** (3.9% deterministic, 0–20.6% across + runs). On that query `tools/cache/**` already dominates graph relevance, so the pre-dedup + render concentrated well on its own — the same reason CG-22 found only a small #1500 signal + there. + +--- + +## Verdict + +| bar | result | +|---|---| +| 1. Read must not increase | **PASS** — 0 Reads in 24/24 runs, both arms | +| 2. No abandonment | **PASS** — 0 `isError`, codegraph last in every run, no call collapse | +| 3. Buckets must not shift to "Read a file we returned" / "another explore" | **PASS** — both failure buckets empty on both arms | +| 4. Residual occupancy actually drops | **NOT MET** — flat per call, and unreachable given CG-18's reallocation rule | + +The epic's acceptance says *revert if bar 4 is not met, because the regression risk isn't worth +a marginal win*. **The regression half of that premise was measured and is zero** — 24 runs, no +Read, no abandonment, no bucket shift, back-references demonstrably reaching the agent. And bar +4 is unreachable by construction, not unmet by underperformance: the byte ceiling it was aiming +at is the baseline's duplicate fraction, 4–21%, and CG-18 was already accepted on the rule that +those bytes get **spent, not banked**. + +So: **keep the change, and restate the epic's metric** as the duplicate fraction of residual +(−87% agent, −86%/−94% deterministic) at flat context cost — with excalidraw showing the +best case, 5.4% fewer response bytes carrying 11.1% more unique source. + +This is a judgement call against the letter of bar 4, and it is cheap to reverse in either +direction: + +- runtime: `CODEGRAPH_EXPLORE_DEDUP=0` disables dedup without a rebuild; +- source: `git revert 7a7ea30 ab38d1f 4e94860 fc31b1e` removes CG-17 + CG-18 entirely; +- the third option, if occupancy really is the goal: **bank the reclaimed bytes instead of + re-spending them**, which reverses CG-18's freed-budget rule and buys at most the duplicate + fraction above. That needs its own gate — it is a strictly *smaller* answer per call, which is + the shape this task exists to be afraid of. + +Logs: `/tmp/cg20/ab-client-go`, `/tmp/cg20/ab-excalidraw`, `/tmp/cg20/ab-excalidraw-b2` +(ephemeral — archive them if a distribution needs to stay reproducible). diff --git a/docs/design/explore-session-dedup.md b/docs/design/explore-session-dedup.md new file mode 100644 index 0000000..64cf2e1 --- /dev/null +++ b/docs/design/explore-session-dedup.md @@ -0,0 +1,218 @@ +# Cross-call explore session state + +`codegraph_explore` answers every call as if it were the first one. It has no idea what it +already sent this session, so a 4th call happily re-serves the spine the 1st call already +delivered (the #1500 report: 4 calls on a 2-call tier budget), and the tier's call budget +can only be *asked* for in prose the agent ignores. + +This document covers the state layer that fixes the "no idea" part — `src/mcp/explore-session-state.ts` +(CG-17) — and the first thing built on it, cross-call source dedup +(`src/mcp/explore-dedup.ts`, CG-18). Budget decay past the tier budget (CG-19) is the +other consumer. + +## What is recorded + +One `ExploreSessionState` per MCP session. Inside it, per **resolved project root**: + +- `callCount` / `responseBytes` — every explore call served this session for that project; +- `calls[]` — the recent ones in detail: query, per-file emitted **line ranges**, a content + **fingerprint** for the bytes those ranges were sliced from, source bytes, response bytes, + and the call's 1-based session index. + +Ranges come from the render loop itself — `buildSection` returns the spans it slices +alongside the text, and the whole-file / focused / skeleton paths report theirs at the +point they push source into the response. A separate function mirroring the window and +padding rules would drift, and drift here is not symmetric: see *Which way to be wrong*. + +Only files that **survive the final hard-ceiling truncation** are recorded. A section the +ceiling dropped was never delivered. A back-referenced file records its spans at **zero +bytes**: the record means "source the agent HOLDS for this file", not "bytes this call +spent", so re-recording keeps a long session from ageing a pointed-at span out of the +retained window and re-serving it for nothing. + +## Four constraints, and what each one rules out + +| Constraint | Why | What it rules out | +|---|---|---| +| Per session, never persisted | A new agent has seen nothing | A disk cache keyed by project | +| Per **resolved** project root | One session can query several projects by `projectPath` | Keying on the path the agent typed — `/repo` and `/repo/internal` are one project | +| Bounded memory | Sessions can run for hours | Unbounded `calls[]` growth | +| Daemon-safe | One daemon shares ONE `ToolHandler` and a pool of worker threads across every connected client | State on the handler, in a worker, or in a module-level singleton | + +The daemon constraint is the sharp one. State kept on the shared `ToolHandler` would blend +two agents' histories, and a dedup built on that would withhold source from an agent that +never saw it — which costs a Read, the exact failure this area exists to prevent. So the +state lives on `MCPSession`, and the plumbing is: + +``` +MCPSession (owns the state) + └─ ToolHandler.execute(tool, args, sessionState) + ├─ down: session view attached to args (survives structured clone → worker) + └─ up: emission attached to the result (survives structured clone ← worker) + └─ recorded on the MAIN thread, then DELETED from the result +``` + +Both legs travel as plain properties (`_cgExploreSession`, `_cgExploreEmission`) because +either may cross a worker boundary, where a closure or a handler field could not follow. +The emission is stripped in `execute` **unconditionally** — including for callers that +track nothing, like the CLI — so the agent-facing response is byte-identical. A view a +client spells itself is discarded, not trusted: it decides what a later call may withhold. + +## The bounds + +`EXPLORE_SESSION_LIMITS`: 4 projects (LRU), 8 retained calls per project, 24 files per +call, 24 ranges per file, 4 calls in the view handed to a call. + +Every bound caps **detail**. `callCount` and `responseBytes` keep counting past eviction — +decay (CG-19) reads the count, and a bound that reset it would make decay reset itself +every 8 calls. + +## Which way to be wrong + +Where a bound forces a choice, the record keeps **fewer** ranges than were emitted, never +more: + +- under-report → a later call re-serves something the agent already has. Wasteful. +- over-report → a later call withholds source the agent never saw. The agent Reads the + file, and one Read costs more than every byte the dedup saved. + +So `coalesceRanges` drops the smallest spans when it hits the cap (and flags +`rangesTruncated`), invalid spans are discarded rather than clamped, and truncated file +sections are never recorded. + +## Inspecting it + +The CG-4 diagnostic (`CODEGRAPH_EXPLORE_DEBUG`, see +[explore-budget-allocation.md](./explore-budget-allocation.md)) carries a `session` block +on every report: + +``` + session call #2 for this project · 1 prior call · 18,204 chars already served + already served internal/usecase/payroll_cycle.go · 4,928 chars · L1-159 +``` + +`callIndex` is this call's position in the session; `priorFiles` unions the ranges already +served per file, most-recent call first. The block is **absent** — not zeroed — when the +caller tracks no state, which is how "untracked" and "first call of a tracked session" stay +distinguishable. + +--- + +# Cross-call dedup (CG-18) + +A call that would re-send source an earlier call already delivered sends a **pointer** +instead. Never a bare omission: an insufficient-feeling response is precisely what sends an +agent to Read, and one or two of those early in a session teach it to abandon codegraph +entirely. So the replacement carries the file, the symbols, the line spans, and the two +facts that make the copy usable — that it came from THIS conversation, and that the file has +not changed since: + +``` +**`internal/usecase/payroll/cycle.go`** — Cycle, PayslipsForCycle, Service, … + +> **Already sent earlier in this conversation:** `internal/usecase/payroll/cycle.go` +> L42-76, L78-215 (Cycle, PayslipsForCycle, Service, +6 more) — unchanged on disk since, +> so that copy is still exact. Only the NEW lines are shown below; scroll back for the +> rest. Do NOT Read this file. +``` + +The convention is also stated once, inline, as an exception appended to the "verbatim +source" guarantee (the same shape #1474 uses for drift), and once in +`server-instructions.ts`. + +## What gates it + +| Gate | Rule | +|---|---| +| Session | Off on a session's first call for a project — nothing to point at | +| Content | A span is withheld only if the file still hashes to the bytes that span was sliced from | +| Size | Only a covered run of ≥ `MIN_COVERED_LINES` (8) is replaced | +| Remainder | New source under `MIN_DELTA_CHARS` (160) folds into the pointer instead of getting its own fence | +| Kill switch | `CODEGRAPH_EXPLORE_DEDUP=0` renders as if the session had no history | + +The **content** gate is a fingerprint (`length:sha1-prefix`) recorded per file per call, NOT +the index's drift flag. They answer different questions: two calls inside one drift window +served the same current bytes (dedup is correct); a file edited *and re-synced* between two +calls is never "stale" and yet the agent's copy is now wrong (dedup would be actively +harmful). #1474's drift handling is upstream of this and unchanged — a drifted file still +ships whole or not at all. + +The **size** gates exist because the pointer sentence is itself ~140 chars. Replacing a +signature line or the ±3 lines of cluster padding would make the response bigger *and* read +as full of holes. `MIN_DELTA_CHARS` is the one place the design withholds something the +agent has not seen — bounded to ~two lines sitting directly against source it does hold — +and it is there because the alternative is a code fence containing `228\t`, which reads as +a broken response. The file is still named with its symbols, so one follow-up explore +fetches it whole. + +## Where the reclaimed bytes go + +Two channels, both of which move bytes toward files the agent has NOT seen: + +- **`sourceSpent`** — a deduped file spends less, so CG-21's carry-forward pool hands the + difference down the rank order, and `headroom` grows for every file after it. +- **the `maxFiles` slot** — a fully back-referenced file does not consume one (the same + treatment a cliffed file gets), so a file that would not have fit now renders. + +Within a file, the shrink decision reads the **deduped** length: shrinking a cluster on its +raw size would drop new symbols to make room for source that is not being sent. + +Spending rather than banking is what keeps the response the same *size* while raising the +share of it the agent has never seen — and it is also why CG-20 found residual context +occupancy **flat**. A design that spends every reclaimed byte cannot lower the byte count; +what it lowers is the duplicate fraction of those bytes (−87% across CG-20's agent runs). +Measured on two matched 3-call replays: client-go 44,740 → 46,957 unique source chars for a +3.2% larger response, excalidraw 39,575 → 43,973 unique for a 5.4% **smaller** one. If the +goal is ever restated as "fewer bytes," this is the one rule to reverse — and it needs its own +abandonment gate, because banking makes a repeat call return strictly less. + +**`dedup.savedChars` is a pre-clip figure.** It counts what dedup suppressed from the +*unclipped candidate* render, not what stayed out of the window — most of a suppressed range +would have been trimmed by the budget anyway. Measured on client-go: 11,450 reported against +1,042 chars the baseline actually re-served. Read it as "how much duplication the ranking +wanted to emit," never as a saving; over-reading it inflates the win ~7×. + +## The all-pointer guard + +If dedup suppresses everything and nothing new takes its place, the response would be +pointers only — the shape that reads as "codegraph found nothing". The render loop keeps the +first fully-suppressed file's real section in hand and splices it back when the loop ends +with zero new source. It costs a re-serve of one file on the one call shape where dedup +would otherwise have saved everything. That is the safe direction, and it is why "no +duplicate ranges across calls" holds for every call that had anything new to say, rather +than universally. + +CG-20 ran that gate on a real agent — client-go and excalidraw, both arms codegraph-on, +n=3 and n=6 per arm. **Read = 0 in all 24 runs**, no `isError`, codegraph last in every run, +and the "Read a file we returned" / "Read a file we did not return" buckets empty on both +arms, with back-references demonstrably reaching the agent in 8 of the 9 multi-call runs. The +guard never fired on a real query — the thinnest of those 21 calls still carried 12,011 chars +of new source, so `newSourceChars === 0` was never reached and that threshold remains untested +in the field; the numbers and the one bar that did not pass are in +[`../benchmarks/explore-dedup-ab-cg20.md`](../benchmarks/explore-dedup-ab-cg20.md). + +## Coverage + +`__tests__/explore-session-state.test.ts`, in three layers: the container (keying, monotonic +index past eviction, every bound), the handler seam (a real explore against a real index +records real ranges; a session's FIRST call is byte-identical to an untracked one; two +states on one handler stay separate), and the session seam (two `MCPSession`s on one engine +get their own state, and each call carries its own session's). + +`__tests__/explore-cross-call-dedup.test.ts` covers the dedup itself: the range algebra and +its thresholds, the fingerprint gate (an edited file re-emits; an unprovable record is +ignored), the pointer's wording (names the file/spans/symbols, never says "omitted", never +steers to Read), and then the seam — a real second call re-sends **no** line the first one +sent, comes back with >20 lines the first call never sent (reclaimed budget, not a shrunken +response), always contains real source however much the session holds, and reports its +savings through the CG-4 diagnostic (`dedup.savedChars`, per-file `dedupSavedChars` / +`dedupCovered`, `render: 'backref'`). + +Two things vitest cannot cover, verified by hand against `dist/`: + +- **the worker path** — with a `QueryPool` attached, the emission survives the structured + clone back from the worker, records on the main thread, and is absent from the result; +- **two genuinely different projects in one session** — opening a second index inside vitest + fails on the lazy `require('../index')`. The in-suite substitute reaches one project two + ways (bare, and by a `projectPath` pointing at a subdirectory) and asserts both land in one + bucket; multi-project keying itself is covered at the container level. diff --git a/src/mcp/explore-dedup.ts b/src/mcp/explore-dedup.ts new file mode 100644 index 0000000..541945c --- /dev/null +++ b/src/mcp/explore-dedup.ts @@ -0,0 +1,249 @@ +/** + * Cross-call source dedup for `codegraph_explore` (CG-18). + * + * The session record (CG-17) knows what earlier calls already sent. This module + * is the algebra that turns that record into a decision for the call being + * rendered: of the line ranges this call WOULD emit, which does the agent + * already hold, and what is genuinely new. + * + * Three rules shape everything here, and all three come from the same place — + * an insufficient-feeling response is what sends an agent to Read, and one or + * two of those early in a session teach it to abandon codegraph entirely + * (CLAUDE.md): + * + * 1. **A pointer, never a bare omission.** Removed source is replaced by a + * back-reference naming the file, the symbols, and the line span, worded so + * it is unmistakable that the source was already delivered IN THIS + * CONVERSATION and is still current. Silence reads as "codegraph didn't + * find it". + * 2. **Only prove-it dedup.** A span is withheld only when the file's bytes + * are byte-identical to what was served (a content fingerprint, not an + * mtime and not the index's drift flag). An edit between calls means the + * agent's copy is wrong, so the source is re-emitted in full. + * 3. **Cut chunks, not slivers.** Only a covered run of at least + * {@link EXPLORE_DEDUP.MIN_COVERED_LINES} lines is worth replacing. Below + * that the pointer costs more than the source, and shattering a block into + * one-line fragments produces exactly the ragged output that reads as a + * failure. Everything not withheld is emitted — where the algebra is + * unsure, it re-serves. + * + * Which way to be wrong, restated for this layer: re-serving something the agent + * has is a few hundred wasted chars; withholding something it never saw is a + * Read. Every threshold below leans to the first. + */ + +import { createHash } from 'crypto'; +import type { ExploreLineRange, ExploreProjectState } from './explore-session-state'; + +export const EXPLORE_DEDUP = { + /** + * Shortest already-served run that may be replaced by a back-reference. + * + * Sized against what dedup is actually FOR — a later call re-serving a whole + * method or file it already sent. A shorter covered run is either a signature + * line in a skeleton render or the ±3 lines of context padding around a + * cluster, and swapping either for a pointer trades bytes for noise: the + * pointer sentence is itself ~140 chars, so under this length dedup would + * make the response BIGGER while making it read as full of holes. + */ + MIN_COVERED_LINES: 8, + /** + * Below this many chars of NEW source, a file's remainder is folded into its + * back-reference instead of being fenced on its own. + * + * The shape this exists for, seen on the CG-17 fixture: a third call whose + * only unheld line was the file's trailing blank one, rendered as a code fence + * containing `228\t`. A fence holding two lines of nothing reads as a broken + * response, and reading as broken is the expensive failure — it is the thing + * that sends an agent to Read and keeps it there. So a remainder this small is + * dropped rather than shown. It is the one place this module withholds + * something the agent has not seen, and it is bounded to ~two lines that sit + * directly against source the agent does hold; the file is still named, with + * its symbols, so one follow-up explore fetches it whole. + */ + MIN_DELTA_CHARS: 160, + /** Line spans named in one pointer before it summarises the rest. */ + MAX_SPANS_IN_POINTER: 4, + /** Symbols named in one pointer before it summarises the rest. */ + MAX_SYMBOLS_IN_POINTER: 5, +} as const; + +const OFF = new Set(['0', 'false', 'off', 'no']); + +/** + * Kill switch: `CODEGRAPH_EXPLORE_DEDUP=0` renders every call as if the session + * had no history. Read per call (not memoized) so a test can toggle it. + */ +export function exploreDedupEnabled(): boolean { + const raw = process.env.CODEGRAPH_EXPLORE_DEDUP; + if (raw === undefined) return true; + return !OFF.has(raw.trim().toLowerCase()); +} + +/** + * Identity of the bytes a call served for one file. + * + * This — not the index's drift flag — is what gates dedup. `isFileStaleOnDisk` + * answers "did the file change since the last INDEX SYNC", which is a different + * question with a different answer: two calls inside one drift window served the + * same current bytes (dedup is correct), while a file edited and re-synced + * between two calls is never "stale" and yet the agent's copy is now wrong + * (dedup would be actively harmful). Length is prefixed so a hash prefix + * collision cannot alias two files of different size. + */ +export function fileFingerprint(content: string): string { + return `${content.length}:${createHash('sha1').update(content).digest('hex').slice(0, 16)}`; +} + +/** Sort + merge overlapping/adjacent spans into the smallest equivalent set. */ +export function mergeRanges(ranges: ReadonlyArray): ExploreLineRange[] { + const valid = ranges + .filter((r) => Number.isFinite(r.start) && Number.isFinite(r.end) && r.end >= r.start && r.start >= 1) + .map((r) => ({ start: Math.floor(r.start), end: Math.floor(r.end) })) + .sort((a, b) => a.start - b.start || a.end - b.end); + const out: ExploreLineRange[] = []; + for (const r of valid) { + const last = out[out.length - 1]; + if (last && r.start <= last.end + 1) last.end = Math.max(last.end, r.end); + else out.push({ ...r }); + } + return out; +} + +/** The parts of `range` that `served` covers. */ +export function intersectRange( + range: ExploreLineRange, + served: ReadonlyArray, +): ExploreLineRange[] { + const out: ExploreLineRange[] = []; + for (const s of served) { + const start = Math.max(range.start, s.start); + const end = Math.min(range.end, s.end); + if (end >= start) out.push({ start, end }); + } + return mergeRanges(out); +} + +/** The parts of `range` that `cut` does NOT cover. */ +export function subtractRange( + range: ExploreLineRange, + cut: ReadonlyArray, +): ExploreLineRange[] { + const out: ExploreLineRange[] = []; + let cursor = range.start; + for (const c of mergeRanges(cut)) { + if (c.end < cursor) continue; + if (c.start > range.end) break; + if (c.start > cursor) out.push({ start: cursor, end: Math.min(c.start - 1, range.end) }); + cursor = Math.max(cursor, c.end + 1); + if (cursor > range.end) break; + } + if (cursor <= range.end) out.push({ start: cursor, end: range.end }); + return out; +} + +/** What one intended span becomes once the session's history is applied. */ +export interface RangeDedup { + /** Spans to render now — everything not proven-already-held. */ + emit: ExploreLineRange[]; + /** Spans replaced by a back-reference. */ + covered: ExploreLineRange[]; +} + +/** + * Split one intended span into what to emit and what to point back at. + * + * Covered runs shorter than {@link EXPLORE_DEDUP.MIN_COVERED_LINES} are left in + * the emit set on purpose (rule 3 above) — so a span the agent holds "almost + * all of" still comes back whole rather than as a stutter of fragments around + * pointers. + */ +export function dedupeRange( + range: ExploreLineRange, + served: ReadonlyArray, + minCovered: number = EXPLORE_DEDUP.MIN_COVERED_LINES, +): RangeDedup { + if (served.length === 0 || range.end < range.start) return { emit: [range], covered: [] }; + const covered = intersectRange(range, served).filter((r) => r.end - r.start + 1 >= minCovered); + if (covered.length === 0) return { emit: [range], covered: [] }; + return { emit: subtractRange(range, covered), covered }; +} + +/** + * Every line span this session has already served for one file, but ONLY from + * calls that served the SAME BYTES. + * + * A record with no fingerprint is ignored rather than trusted: it cannot prove + * the agent's copy matches the file on disk now, and an unprovable match is + * exactly the case where re-serving is right. + */ +export function servedRangesForFile( + prior: ExploreProjectState | null, + filePath: string, + fingerprint: string, +): ExploreLineRange[] { + if (!prior) return []; + const spans: ExploreLineRange[] = []; + for (const call of prior.calls) { + for (const file of call.files) { + if (file.path !== filePath) continue; + if (!file.fingerprint || file.fingerprint !== fingerprint) continue; + spans.push(...file.ranges); + } + } + return mergeRanges(spans); +} + +/** `L12`, `L12-40`, capped with a `+N more` tail. */ +export function formatSpans(spans: ReadonlyArray): string { + const shown = spans.slice(0, EXPLORE_DEDUP.MAX_SPANS_IN_POINTER) + .map((r) => (r.start === r.end ? `L${r.start}` : `L${r.start}-${r.end}`)) + .join(', '); + const more = spans.length - EXPLORE_DEDUP.MAX_SPANS_IN_POINTER; + return more > 0 ? `${shown}, +${more} more span${more === 1 ? '' : 's'}` : shown; +} + +/** + * The line that replaces withheld source. + * + * It has one job: make the agent reach into its own context instead of into + * Read. So it carries the three things needed to find the source it already has + * — path, symbols, line spans — plus the two facts that make using it safe: + * that it came from THIS conversation, and that the file has not changed since + * (which is checked, not asserted — see {@link fileFingerprint}). It never says + * "omitted", and it never steers to Read. + */ +export function formatBackReference( + filePath: string, + covered: ReadonlyArray, + symbols: ReadonlyArray, + opts: { partial: boolean }, +): string { + const names = symbols.slice(0, EXPLORE_DEDUP.MAX_SYMBOLS_IN_POINTER); + const moreNames = symbols.length - names.length; + const symbolPart = names.length > 0 + ? ` (${names.join(', ')}${moreNames > 0 ? `, +${moreNames} more` : ''})` + : ''; + const head = `> **Already sent earlier in this conversation:** \`${filePath}\` ${formatSpans(covered)}${symbolPart}`; + const tail = opts.partial + ? ' — unchanged on disk since, so that copy is still exact. Only the NEW lines are shown below; scroll back for the rest. Do NOT Read this file.' + : ' — unchanged on disk since, so that copy is still exact and is not repeated here. Use it from your context; do NOT Read this file.'; + return head + tail; +} + +/** Symbol names whose definitions fall inside the withheld spans. */ +export function symbolsInSpans( + nodes: ReadonlyArray<{ name: string; kind: string; startLine: number; endLine: number }>, + spans: ReadonlyArray, +): string[] { + const out: string[] = []; + const seen = new Set(); + for (const n of nodes) { + if (n.kind === 'import' || n.kind === 'export') continue; + if (!spans.some((s) => n.startLine <= s.end && (n.endLine || n.startLine) >= s.start)) continue; + if (seen.has(n.name)) continue; + seen.add(n.name); + out.push(n.name); + } + return out; +} diff --git a/src/mcp/explore-diagnostics.ts b/src/mcp/explore-diagnostics.ts index bec8d27..941fd4c 100644 --- a/src/mcp/explore-diagnostics.ts +++ b/src/mcp/explore-diagnostics.ts @@ -32,6 +32,7 @@ */ import { appendFileSync } from 'fs'; +import type { ExploreProjectState } from './explore-session-state'; /** How a file's source was rendered into the response. */ export type ExploreRenderMode = @@ -40,6 +41,7 @@ export type ExploreRenderMode = | 'focused' // per-symbol view, named/spine bodies full | 'skeleton' // per-symbol view, signatures only | 'stale-omitted' // drifted on disk; source deliberately withheld + | 'backref' // fully served by an earlier call this session (CG-18) | 'dropped'; // rendered into `lines` but cut by the final hard ceiling /** Why a ranked candidate never reached the output. */ @@ -91,6 +93,17 @@ interface FileRecord extends ExploreCandidateMeta { */ allowance: number | null; render?: ExploreRenderMode; + /** + * Source chars this call did NOT re-send because an earlier call in the + * session already did (CG-18). Reclaimed, not lost: it leaves through + * `sourceSpent` (the carry-forward pool hands it to lower-ranked files) and, + * for a fully back-referenced file, through the freed `maxFiles` slot. The + * reallocation is legible as the difference between this file's + * `allowance` and `emittedChars` against the files below it in the table. + */ + dedupSavedChars: number; + /** Line spans replaced by a back-reference. */ + dedupCovered: Array<[number, number]>; /** Source chars the render loop handed to `lines` (pre-final-truncation). */ emittedChars: number; /** Source chars present in the FINAL text — authoritative, truncation-aware. */ @@ -129,12 +142,30 @@ export interface ExploreDiagnosticFile extends ExploreCandidateMeta { render: ExploreRenderMode | null; skipped: ExploreSkipReason | null; clipped: boolean; + dedupSavedChars: number; + dedupCovered: Array<[number, number]>; emittedChars: number; finalChars: number; share: number; allocatedShare: number; } +/** + * This session's explore history for this project, as of BEFORE the call being + * reported (CG-17). Present only when the caller tracks session state — the CLI + * and bare-handler callers don't, so it is absent there rather than zeroed. + */ +export interface ExploreDiagnosticSession { + /** 1-based index of THIS call within the session, for this project. */ + callIndex: number; + /** Calls already served this session for this project. */ + priorCalls: number; + /** Response chars already served this session for this project. */ + priorResponseChars: number; + /** Files already served source this session, most-recent call first. */ + priorFiles: Array<{ path: string; ranges: Array<[number, number]>; bytes: number }>; +} + /** The full report — one per explore call, JSON-serialized to the sink. */ export interface ExploreDiagnosticReport { tool: 'codegraph_explore'; @@ -142,6 +173,8 @@ export interface ExploreDiagnosticReport { projectRoot: string; indexedFileCount: number; note?: string; + /** Session-scoped call state (CG-17); absent when the caller tracks none. */ + session?: ExploreDiagnosticSession; budget: { maxOutputChars: number; maxCharsPerFile: number; @@ -172,6 +205,17 @@ export interface ExploreDiagnosticReport { filesRenderedByLoop: number; filesInFinalOutput: number; }; + /** + * Cross-call source dedup (CG-18): what this call did NOT re-send because an + * earlier call in this session already sent it, and where those bytes went. + * `savedChars` 0 with a non-empty session block means nothing overlapped. + */ + dedup: { + savedChars: number; + backReferenced: string[]; + /** Files fully replaced by a pointer — each one also freed a `maxFiles` slot. */ + fullyBackReferenced: string[]; + }; /** The proportional split (CG-12): what each file was promised, and why. */ allocation: { /** Chars divided among admitted files (envelope minus per-file overhead). */ @@ -221,6 +265,7 @@ export class ExploreDiagnostics { private graphGateThreshold = 0; private graphGateApplied = false; private note = ''; + private session: ExploreDiagnosticSession | undefined; private allocPool = 0; private allocCliffAt = 0; private allocCliffed: string[] = []; @@ -265,6 +310,40 @@ export class ExploreDiagnostics { this.stages.pastRelevanceGate = kept; } + /** + * Record what this session had already been served for this project (CG-17), + * so the report says which call in the session it is and what the earlier ones + * cost. Read-only for now: nothing in the render loop consults it, which is + * what keeps the response byte-identical at this stage. + * + * Files are listed most-recent call first and de-duplicated by path — the same + * file re-served across calls is the pattern this instrument exists to make + * visible, and its ranges are unioned so a glance shows what of it the agent + * already holds. + */ + noteSession(prior: ExploreProjectState | null): void { + if (!prior) return; + const byPath = new Map; bytes: number }>(); + for (const call of [...prior.calls].reverse()) { + for (const file of call.files) { + const existing = byPath.get(file.path); + const spans = file.ranges.map((r) => [r.start, r.end] as [number, number]); + if (existing) { + existing.ranges.push(...spans); + existing.bytes += file.bytes; + } else { + byPath.set(file.path, { path: file.path, ranges: spans, bytes: file.bytes }); + } + } + } + this.session = { + callIndex: prior.callCount + 1, + priorCalls: prior.callCount, + priorResponseChars: prior.responseBytes, + priorFiles: [...byPath.values()], + }; + } + /** Candidate count after the `group.score >= floor` filter. */ setScoreFloor(floor: number, kept: number): void { this.scoreFloor = floor; @@ -284,6 +363,7 @@ export class ExploreDiagnostics { noteCandidate(path: string, meta: ExploreCandidateMeta): void { this.files.set(path, { path, ...meta, allowance: null, + dedupSavedChars: 0, dedupCovered: [], emittedChars: 0, finalChars: 0, share: 0, allocatedShare: 0, clipped: false, }); } @@ -321,6 +401,19 @@ export class ExploreDiagnostics { rec.skipped = undefined; } + /** + * Source this call withheld because the session already holds it (CG-18). + * Called with `(path, 0, [])` to clear a record — the anti-abandonment restore + * puts a suppressed file's source back, and a diagnostic still claiming the + * saving would misreport where the envelope went. + */ + recordDedup(path: string, savedChars: number, covered: ReadonlyArray<{ start: number; end: number }>): void { + const rec = this.files.get(path); + if (!rec) return; + rec.dedupSavedChars = savedChars; + rec.dedupCovered = covered.map((r) => [r.start, r.end] as [number, number]); + } + /** * A candidate was passed over before rendering. First reason wins — the * blanket `max-files` sweep must not overwrite a file's specific reason. @@ -356,8 +449,10 @@ export class ExploreDiagnostics { rec.share = envelope > 0 ? rec.finalChars / envelope : 0; rec.allocatedShare = allocatedChars > 0 ? rec.emittedChars / allocatedChars : 0; // Rendered into `lines` but absent from the final text → the hard - // ceiling dropped its whole section. - if (rec.render && rec.render !== 'stale-omitted' && rec.finalChars === 0) { + // ceiling dropped its whole section. A back-referenced file has no + // fenced source BY DESIGN (CG-18), so it is never "dropped". + if (rec.render && rec.render !== 'stale-omitted' && rec.render !== 'backref' + && rec.finalChars === 0) { rec.render = 'dropped'; rec.clipped = true; } @@ -384,6 +479,7 @@ export class ExploreDiagnostics { projectRoot: this.projectRoot, indexedFileCount: this.indexedFileCount, note: this.note || undefined, + session: this.session, budget: { maxOutputChars: this.budget.maxOutputChars, maxCharsPerFile: this.budget.maxCharsPerFile, @@ -412,6 +508,11 @@ export class ExploreDiagnostics { filesRenderedByLoop: filesIncluded, filesInFinalOutput: rendered.length, }, + dedup: { + savedChars: records.reduce((s, r) => s + r.dedupSavedChars, 0), + backReferenced: records.filter((r) => r.dedupSavedChars > 0).map((r) => r.path), + fullyBackReferenced: records.filter((r) => r.render === 'backref').map((r) => r.path), + }, allocation: { pool: this.allocPool, cliffAt: round6(this.allocCliffAt), @@ -440,6 +541,8 @@ export class ExploreDiagnostics { render: r.render ?? null, skipped: r.skipped ?? null, clipped: r.clipped, + dedupSavedChars: r.dedupSavedChars, + dedupCovered: r.dedupCovered.map((s) => [...s] as [number, number]), emittedChars: r.emittedChars, finalChars: r.finalChars, share: round6(r.share), @@ -524,6 +627,20 @@ export function renderTable(report: ExploreDiagnosticReport): string { out.push(`codegraph explore diagnostic — "${report.query}"`); out.push(` project ${report.projectRoot} · ${num(report.indexedFileCount)} files indexed`); if (report.note) out.push(` note: ${report.note}`); + if (report.session) { + const s = report.session; + out.push( + ` session call #${s.callIndex} for this project` + + ` · ${num(s.priorCalls)} prior call${s.priorCalls === 1 ? '' : 's'}` + + ` · ${num(s.priorResponseChars)} chars already served`, + ); + for (const f of s.priorFiles.slice(0, 12)) { + const spans = f.ranges.slice(0, 6).map(([a, b]) => (a === b ? `${a}` : `${a}-${b}`)).join(','); + const more = f.ranges.length > 6 ? `,+${f.ranges.length - 6}` : ''; + out.push(` already served ${f.path} · ${num(f.bytes)} chars · L${spans}${more}`); + } + if (s.priorFiles.length > 12) out.push(` … +${s.priorFiles.length - 12} more already-served file(s)`); + } out.push( ` envelope ${num(env.chars)} chars delivered · ${num(env.allocatedChars)} allocated` + ` of ${num(budget.maxOutputChars)} budget (hard ceiling ${num(budget.hardCeiling)})` + @@ -545,6 +662,15 @@ export function renderTable(report: ExploreDiagnosticReport): string { ` relevance gate ${sel.graphGateApplied ? 'applied' : 'not applied'}` + ` at graph >= ${sel.graphGateThreshold.toFixed(5)} (6% of max ${sel.maxGraph.toFixed(5)})`, ); + const dedup = report.dedup; + if (dedup && dedup.savedChars > 0) { + out.push( + ` dedup ${num(dedup.savedChars)} chars not re-sent` + + ` · ${dedup.fullyBackReferenced.length} file(s) fully back-referenced` + + ` (each also freed a maxFiles slot)` + + (dedup.backReferenced.length > 0 ? `: ${dedup.backReferenced.join(', ')}` : ''), + ); + } const alloc = report.allocation; out.push( ` allocation ${num(alloc.reserved)} reserved of ${num(alloc.pool)} pool` + @@ -558,7 +684,7 @@ export function renderTable(report: ExploreDiagnosticReport): string { // Allocated (not delivered) is the allocator's own decision — the number the // budget work is about. Delivered is what the agent got. They differ only // when the ceiling truncated; showing both makes that divergence obvious. - const shown = files.filter((f) => f.emittedChars > 0 || f.finalChars > 0); + const shown = files.filter((f) => f.emittedChars > 0 || f.finalChars > 0 || f.render === 'backref'); if (shown.length > 0) { out.push(' # alloc% deliv% bytes reserved score graph hits pen flags render file'); for (const f of shown) { @@ -578,6 +704,11 @@ export function renderTable(report: ExploreDiagnosticReport): string { f.path, ); out.push(' kinds: ' + (f.kinds || '-')); + if (f.dedupSavedChars > 0) { + const spans = f.dedupCovered.slice(0, 6).map(([a, b]) => (a === b ? `${a}` : `${a}-${b}`)).join(','); + const more = f.dedupCovered.length > 6 ? `,+${f.dedupCovered.length - 6}` : ''; + out.push(` dedup: ${num(f.dedupSavedChars)} chars already sent this session · L${spans}${more}`); + } } out.push(' (bytes = source allocated by the render loop; deliv% = 0 means the hard ceiling dropped the section)'); out.push(' (* = clipped: some source in this file was elided, windowed, or its section dropped)'); diff --git a/src/mcp/explore-session-state.ts b/src/mcp/explore-session-state.ts new file mode 100644 index 0000000..26439c0 --- /dev/null +++ b/src/mcp/explore-session-state.ts @@ -0,0 +1,368 @@ +/** + * Session-scoped `codegraph_explore` call state (CG-17). + * + * What it holds: for ONE MCP session, per project it queried, what explore has + * already returned — the files, the line ranges of source inside them, the bytes + * they cost, and where in the session each call fell. Nothing else in the server + * knows this today: every explore call is answered as if it were the first one, + * which is why a 4th call happily re-serves the same spine it already sent + * (#1500) and why the tier's call budget can only be *asked* for rather than + * enforced. This module is the record those two behaviours are built on + * (CG-18 cross-call dedup, CG-19 budget decay). It changes no response itself. + * + * Four constraints shape the design, all of them from how the daemon actually + * runs: + * + * 1. **Per session, never persisted.** One instance is owned by an + * {@link ../mcp/session.MCPSession} and dies with the socket. A new agent + * session starts clean — dedup across sessions would suppress source the + * new agent has never seen. + * 2. **Per project inside the session.** A session can query several projects + * by `projectPath`, so state is keyed by the RESOLVED project root + * (`cg.getProjectRoot()`), not by whatever path the agent typed. + * 3. **Bounded.** A long-lived session must not grow without limit, so + * everything is capped — see {@link EXPLORE_SESSION_LIMITS}. Eviction drops + * DETAIL only: `callCount` and `responseBytes` keep counting past it, since + * decay (CG-19) reads the count and must not be reset by its own bound. + * 4. **Daemon-safe.** The daemon shares ONE {@link ../mcp/tools.ToolHandler} + * (and a pool of worker threads) across every connected session, so this + * state can live neither on the handler nor in a worker. It lives on the + * session; the handler is handed it per call, and the record of what a call + * emitted travels back on the {@link ToolResult} so it can be recorded on + * the main thread whether dispatch ran in-process or on a worker. + * + * Over- vs under-reporting: where a bound forces a choice, this module keeps + * FEWER ranges than were emitted, never more. A consumer that under-knows + * re-serves something the agent already has (wasteful); one that over-knows + * withholds source the agent never saw (a Read — the failure this whole area + * exists to prevent). + */ + +import * as path from 'path'; + +/** + * Property on a {@link ../mcp/tools.ToolResult} carrying what an explore call + * emitted. INTERNAL: `ToolHandler.execute` records it and deletes it before the + * result reaches the wire, so the agent-facing response is unchanged. It is a + * plain-object property (not a Symbol) on purpose — it has to survive the + * structured clone back from a query-pool worker. + */ +export const EXPLORE_EMISSION_KEY = '_cgExploreEmission'; + +/** + * Argument key carrying this session's prior-call view INTO a tool call. Same + * reasoning as {@link EXPLORE_EMISSION_KEY}: it crosses the worker boundary, so + * it must be a serializable property on the args object. + */ +export const EXPLORE_SESSION_VIEW_ARG = '_cgExploreSession'; + +/** An inclusive 1-based line span of a file that was emitted. */ +export interface ExploreLineRange { + start: number; + end: number; +} + +/** What one call emitted for one file. */ +export interface ExploreFileEmission { + /** Project-relative path, exactly as the response's file header spells it. */ + path: string; + /** Coalesced line spans whose source was in the response. */ + ranges: ExploreLineRange[]; + /** Source chars emitted for this file (excludes headers / fences). */ + bytes: number; + /** + * Identity of the bytes those ranges were sliced from (CG-18). Cross-call + * dedup withholds a span only when the file still hashes to this, so an edit + * between two calls re-serves instead of pointing at source the agent holds a + * now-wrong copy of. Absent = unprovable, which dedup treats as "re-serve". + */ + fingerprint?: string; + /** Set when ranges were dropped to stay under the per-file bound. */ + rangesTruncated?: boolean; +} + +/** What one explore call emitted, as reported by the handler. */ +export interface ExploreEmission { + /** Resolved project root — the key state is filed under. */ + projectRoot: string; + /** Normalized query text (post `normalizeQuerySpelling`). */ + query: string; + files: ExploreFileEmission[]; + /** Source chars across all files. */ + sourceBytes: number; + /** Total chars of the response the agent received. */ + responseBytes: number; +} + +/** A recorded call: an emission plus where it fell in the session. */ +export interface ExploreCallRecord extends ExploreEmission { + /** 1-based call index within this session FOR THIS PROJECT. Survives eviction. */ + index: number; +} + +/** Everything the session knows about one project. */ +export interface ExploreProjectState { + projectRoot: string; + /** Explore calls made this session against this project, including evicted ones. */ + callCount: number; + /** Response chars across every call, including evicted ones. */ + responseBytes: number; + /** Retained call records, oldest first. Bounded — may omit early calls. */ + calls: ExploreCallRecord[]; +} + +/** + * The bounded, serializable read-view handed to a tool call. Deliberately + * smaller than the full state: only the most recent calls carry their ranges, + * because that is what a dedup/decay decision reads and the whole thing is + * structured-cloned to a worker on every call. + */ +export interface ExploreSessionView { + projects: ExploreProjectState[]; +} + +/** + * Memory bounds. Every one of them caps DETAIL; none caps the counters that + * CG-19's decay reads. + * + * Sized against how sessions actually behave: an agent explores one project + * (occasionally a second in a monorepo) and the tier call budget is 1–5, so the + * retained window covers a whole realistic session and the caps only bite on + * pathological ones. + */ +export const EXPLORE_SESSION_LIMITS = { + /** Distinct projects kept per session; least-recently-used evicted first. */ + MAX_PROJECTS: 4, + /** Call records kept per project (oldest dropped; `callCount` keeps counting). */ + MAX_CALLS_RETAINED: 8, + /** Files kept per call — the ones that got the most source. */ + MAX_FILES_PER_CALL: 24, + /** Line ranges kept per file after coalescing — the largest spans. */ + MAX_RANGES_PER_FILE: 24, + /** Most-recent calls per project included in {@link ExploreSessionView}. */ + MAX_VIEW_CALLS: 4, +} as const; + +/** + * Key a project root is filed under. Resolved so `/repo` and `/repo/` agree; + * case-folded on the two platforms whose filesystems are case-insensitive, so a + * drive-letter or capitalization difference doesn't split one project in two. + */ +export function exploreProjectKey(projectRoot: string): string { + const resolved = path.resolve(projectRoot); + return process.platform === 'win32' || process.platform === 'darwin' + ? resolved.toLowerCase() + : resolved; +} + +/** + * Merge overlapping / adjacent spans into the smallest equivalent set, then cap + * it. Adjacency (`next.start <= cur.end + 1`) counts as overlap: two ranges that + * touch describe one contiguous block of emitted source. + * + * When the cap bites, the LARGEST spans are kept and the result is re-sorted by + * line so the set still reads top-to-bottom — dropping small fragments loses the + * least information, and under-reporting is the safe direction (see the module + * header). + */ +export function coalesceRanges( + ranges: ReadonlyArray, + max: number = EXPLORE_SESSION_LIMITS.MAX_RANGES_PER_FILE, +): { ranges: ExploreLineRange[]; truncated: boolean } { + const valid = ranges + .filter((r) => Number.isFinite(r.start) && Number.isFinite(r.end) && r.end >= r.start && r.start >= 1) + .map((r) => ({ start: Math.floor(r.start), end: Math.floor(r.end) })) + .sort((a, b) => a.start - b.start || a.end - b.end); + + const merged: ExploreLineRange[] = []; + for (const r of valid) { + const last = merged[merged.length - 1]; + if (last && r.start <= last.end + 1) last.end = Math.max(last.end, r.end); + else merged.push({ ...r }); + } + if (merged.length <= max) return { ranges: merged, truncated: false }; + + const kept = [...merged] + .sort((a, b) => (b.end - b.start) - (a.end - a.start) || a.start - b.start) + .slice(0, max) + .sort((a, b) => a.start - b.start); + return { ranges: kept, truncated: true }; +} + +/** Whether a line falls inside any of the (sorted, coalesced) ranges. */ +export function rangesCover(ranges: ReadonlyArray, line: number): boolean { + return ranges.some((r) => line >= r.start && line <= r.end); +} + +interface MutableProjectState { + projectRoot: string; + callCount: number; + responseBytes: number; + calls: ExploreCallRecord[]; +} + +/** + * One MCP session's explore history. Created per session, thrown away with it. + * + * Not thread-shared and not a singleton: two sessions on the same daemon own two + * instances and can never observe each other's calls. Every method is total — + * malformed input is normalized away rather than thrown, because this sits on + * the tool-call path and a bookkeeping bug must never fail an explore. + */ +export class ExploreSessionState { + /** Insertion-ordered; a touched project is re-inserted, so the head is the LRU. */ + private readonly projects = new Map(); + + /** + * File an emission. Returns the record as stored (with its session call + * index), or `null` if the emission was unusable. + */ + record(emission: ExploreEmission): ExploreCallRecord | null { + if (!emission || typeof emission.projectRoot !== 'string' || !emission.projectRoot) return null; + const key = exploreProjectKey(emission.projectRoot); + const state = this.touch(key, emission.projectRoot); + + state.callCount += 1; + state.responseBytes += Math.max(0, emission.responseBytes || 0); + + const record: ExploreCallRecord = { + index: state.callCount, + projectRoot: emission.projectRoot, + query: typeof emission.query === 'string' ? emission.query : '', + files: this.boundFiles(emission.files), + sourceBytes: Math.max(0, emission.sourceBytes || 0), + responseBytes: Math.max(0, emission.responseBytes || 0), + }; + state.calls.push(record); + if (state.calls.length > EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED) { + state.calls.splice(0, state.calls.length - EXPLORE_SESSION_LIMITS.MAX_CALLS_RETAINED); + } + return record; + } + + /** Full state for one project, or `null` if it was never queried this session. */ + forProject(projectRoot: string): ExploreProjectState | null { + const state = this.projects.get(exploreProjectKey(projectRoot)); + return state ? cloneProject(state) : null; + } + + /** Explore calls made this session against a project (including evicted ones). */ + callCount(projectRoot: string): number { + return this.projects.get(exploreProjectKey(projectRoot))?.callCount ?? 0; + } + + /** Every project this session has queried, least-recently-used first. */ + snapshot(): ExploreProjectState[] { + return [...this.projects.values()].map(cloneProject); + } + + /** + * The bounded view passed INTO a tool call. Trimmed to the most recent + * {@link EXPLORE_SESSION_LIMITS.MAX_VIEW_CALLS} calls per project: it crosses a + * worker boundary on every explore, so it carries what a dedup/decay decision + * needs and not the whole history. + */ + view(): ExploreSessionView { + return { + projects: [...this.projects.values()].map((state) => ({ + projectRoot: state.projectRoot, + callCount: state.callCount, + responseBytes: state.responseBytes, + calls: state.calls + .slice(-EXPLORE_SESSION_LIMITS.MAX_VIEW_CALLS) + .map((c) => ({ ...c, files: c.files.map((f) => ({ ...f, ranges: [...f.ranges] })) })), + })), + }; + } + + /** Drop everything. Used by tests; a real session just goes away instead. */ + clear(): void { + this.projects.clear(); + } + + /** + * Fetch a project's state, creating it if new, and mark it most-recently-used. + * Evicts the LRU project past the bound — dropping a project entirely (rather + * than its detail) is right here: a session that has moved on to four other + * repos is not about to re-ask the first one. + */ + private touch(key: string, projectRoot: string): MutableProjectState { + const existing = this.projects.get(key); + if (existing) { + this.projects.delete(key); + this.projects.set(key, existing); + return existing; + } + const created: MutableProjectState = { projectRoot, callCount: 0, responseBytes: 0, calls: [] }; + this.projects.set(key, created); + while (this.projects.size > EXPLORE_SESSION_LIMITS.MAX_PROJECTS) { + const lru = this.projects.keys().next().value as string | undefined; + if (lru === undefined) break; + this.projects.delete(lru); + } + return created; + } + + /** + * Normalize + bound one call's files: coalesce each file's ranges, then keep + * the files that got the most source. A call that renders more files than the + * bound has already spread its envelope thin, so the tail files carry the + * least — and losing them costs the least. + */ + private boundFiles(files: ReadonlyArray | undefined): ExploreFileEmission[] { + if (!Array.isArray(files) || files.length === 0) return []; + const normalized = files + .filter((f) => f && typeof f.path === 'string' && f.path.length > 0) + .map((f) => { + const { ranges, truncated } = coalesceRanges(f.ranges ?? []); + const out: ExploreFileEmission = { path: f.path, ranges, bytes: Math.max(0, f.bytes || 0) }; + if (typeof f.fingerprint === 'string' && f.fingerprint) out.fingerprint = f.fingerprint; + if (truncated) out.rangesTruncated = true; + return out; + }); + if (normalized.length <= EXPLORE_SESSION_LIMITS.MAX_FILES_PER_CALL) return normalized; + return [...normalized] + .sort((a, b) => b.bytes - a.bytes) + .slice(0, EXPLORE_SESSION_LIMITS.MAX_FILES_PER_CALL); + } +} + +function cloneProject(state: MutableProjectState): ExploreProjectState { + return { + projectRoot: state.projectRoot, + callCount: state.callCount, + responseBytes: state.responseBytes, + calls: state.calls.map((c) => ({ ...c, files: c.files.map((f) => ({ ...f, ranges: [...f.ranges] })) })), + }; +} + +/** + * Read the session view a caller injected into tool args, if any. Defensive: + * the key is internal, but the args object comes off the wire, so a client that + * spells it itself gets ignored rather than trusted into a crash. + */ +export function readExploreSessionView(args: Record): ExploreSessionView | null { + const raw = args?.[EXPLORE_SESSION_VIEW_ARG]; + if (!raw || typeof raw !== 'object') return null; + const projects = (raw as ExploreSessionView).projects; + if (!Array.isArray(projects)) return null; + return { projects: projects.filter((p) => p && typeof p.projectRoot === 'string') }; +} + +/** + * This session's prior state for one project, from an injected view. + * + * `null` means NOBODY IS TRACKING (no view was injected — the CLI, a bare + * handler). A view that simply hasn't seen this project yet returns an EMPTY + * state, not null: the distinction matters to consumers, since "first call of a + * tracked session" and "untracked" are different situations. + */ +export function viewForProject( + view: ExploreSessionView | null, + projectRoot: string, +): ExploreProjectState | null { + if (!view) return null; + const key = exploreProjectKey(projectRoot); + return view.projects.find((p) => exploreProjectKey(p.projectRoot) === key) + ?? { projectRoot, callCount: 0, responseBytes: 0, calls: [] }; +} diff --git a/src/mcp/proxy.ts b/src/mcp/proxy.ts index 0429577..27e684e 100644 --- a/src/mcp/proxy.ts +++ b/src/mcp/proxy.ts @@ -30,6 +30,7 @@ import { CodeGraphPackageVersion } from './version'; import { SERVER_INFO, PROTOCOL_VERSION, initializeInstructions } from './session'; import { SERVER_INSTRUCTIONS } from './server-instructions'; import { getStaticTools } from './tools'; +import { ExploreSessionState } from './explore-session-state'; import { getTelemetry, ClientInfo } from '../telemetry'; import type { MCPEngine } from './engine'; @@ -230,6 +231,10 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< // new session starts), these would otherwise hang forever; we re-serve them // in-process so the host always gets a reply. const inflight = new Map(); + // Explore call history for the ONE host connection this proxy serves (CG-17). + // Only the daemon-unavailable fallback below uses it; when the daemon is up, + // the tracking happens on the daemon's own MCPSession. + const exploreSession = new ExploreSessionState(); const trackInflight = (line: string): void => { try { const m = JSON.parse(line) as JsonRpc; @@ -261,7 +266,7 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< try { await ensureEngine(); const params = (msg.params || {}) as { name: string; arguments?: Record }; - const result = await engine!.getToolHandler().execute(params.name, params.arguments || {}); + const result = await engine!.getToolHandler().execute(params.name, params.arguments || {}, exploreSession); writeClient({ jsonrpc: '2.0', id, result }); getTelemetry().recordUsage('mcp_tool', params.name, !result.isError, telemetryClient); } catch (err) { diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index bf3839a..7c6c6ce 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -62,6 +62,8 @@ calls; a grep/read exploration is dozens. - **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" — means live watching stopped entirely (the whole index is frozen, not just a few files); until it's resolved, Read files directly to confirm anything that may have changed. - **A file flagged "⚠ changed on disk after the last index sync" drifted from its index** (most common on projects queried via \`projectPath\`, which have no live watcher). Codegraph never serves a possibly-mis-sliced body from such a file — it either shows the file's full CURRENT source (trust it as a Read) or omits the source with this flag. When the source was omitted, Read that specific file; line numbers referencing it elsewhere in the response may be shifted until that project's next sync. All unflagged files remain trustworthy. +- **"Already sent earlier in this conversation" is a pointer, not a gap.** When a file's section carries that line instead of (or above) its source, an earlier \`codegraph_explore\` in THIS conversation already returned those exact lines and the file has not changed since — so the copy already in your context is current and exact. Scroll back to it; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response. + ## Limitations - If a tool reports a project isn't indexed (no \`.codegraph/\`), stop calling codegraph tools for that project for the rest of the session and use your built-in tools there instead. Indexing is the user's decision — mention they can run \`codegraph init\` if it comes up, but don't run it yourself. diff --git a/src/mcp/session.ts b/src/mcp/session.ts index fbfd9f1..866e001 100644 --- a/src/mcp/session.ts +++ b/src/mcp/session.ts @@ -21,6 +21,7 @@ import { CodeGraphPackageVersion } from './version'; import { findNearestCodeGraphRoot } from '../directory'; import { getTelemetry, ClientInfo } from '../telemetry'; import { getUpdateNotice } from '../upgrade/update-check'; +import { ExploreSessionState } from './explore-session-state'; /** * MCP Server Info — kept on the session because some clients log it. The @@ -110,6 +111,15 @@ export class MCPSession { private rootsAttempted = false; private resolvePromise: Promise | null = null; private explicitProjectPath: string | null; + /** + * What `codegraph_explore` has already returned to THIS client, per project + * (CG-17). Owned by the session, not the engine: the daemon shares one engine + * (and one ToolHandler, and a pool of worker threads) across every connected + * client, so state kept over there would blend two agents' histories and let + * one session's calls suppress source the other has never seen. It dies with + * the session — a reconnecting client starts clean. + */ + private readonly exploreSession = new ExploreSessionState(); constructor( private transport: JsonRpcTransport, @@ -140,6 +150,15 @@ export class MCPSession { return this.transport; } + /** + * This session's explore call history (CG-17). Exposed so tests can assert + * that two sessions on one daemon keep separate state; nothing in the server + * reaches for another session's copy. + */ + getExploreSessionState(): ExploreSessionState { + return this.exploreSession; + } + private async handleMessage(message: JsonRpcRequest | JsonRpcNotification): Promise { const isRequest = 'id' in message; switch (message.method) { @@ -286,7 +305,7 @@ export class MCPSession { await this.retryInitIfNeeded(); if (process.env.CODEGRAPH_MCP_DEBUG) process.stderr.write(`[mcp-debug] toolsCall ${toolName} id=${String(request.id)} dispatch\n`); - const result = await this.engine.getToolHandler().execute(toolName, toolArgs); + const result = await this.engine.getToolHandler().execute(toolName, toolArgs, this.exploreSession); if (process.env.CODEGRAPH_MCP_DEBUG) process.stderr.write(`[mcp-debug] toolsCall ${toolName} id=${String(request.id)} done\n`); this.transport.sendResult(request.id, result); // After the reply is on the wire — telemetry must never delay a tool diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 51c81e1..4a6e1a5 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -42,6 +42,26 @@ import { clamp, validatePathWithinRoot, validateProjectPath, isConfigLeafNode, C import { scanDynamicDispatch } from './dynamic-boundaries'; import { getUpdateNotice } from '../upgrade/update-check'; import { ExploreDiagnostics } from './explore-diagnostics'; +import { + EXPLORE_EMISSION_KEY, + EXPLORE_SESSION_VIEW_ARG, + ExploreSessionState, + readExploreSessionView, + viewForProject, + type ExploreEmission, + type ExploreFileEmission, + type ExploreLineRange, +} from './explore-session-state'; +import { + EXPLORE_DEDUP, + dedupeRange, + exploreDedupEnabled, + fileFingerprint, + formatBackReference, + mergeRanges, + servedRangesForFile, + symbolsInSpans, +} from './explore-dedup'; /** * An expected, recoverable "codegraph can't serve this" condition — most @@ -891,6 +911,15 @@ export interface ToolResult { text: string; }>; isError?: boolean; + /** + * INTERNAL side-channel (CG-17): what a `codegraph_explore` call actually put + * on the wire — files, line ranges, bytes. It rides the result because the + * call may have run on a query-pool worker, while the session state it feeds + * lives on the main thread. {@link ToolHandler.execute} records it and DELETES + * it, so nothing here ever reaches the client. Keyed by + * {@link EXPLORE_EMISSION_KEY}; the two must stay in sync. + */ + _cgExploreEmission?: ExploreEmission; } /** @@ -1805,9 +1834,19 @@ export class ToolHandler { } /** - * Execute a tool by name + * Execute a tool by name. + * + * `sessionState` is the CALLER's per-session explore history (CG-17). The + * daemon shares one ToolHandler across every connected session, so this state + * cannot live on the handler — each session owns one and hands it in, which is + * what keeps two sessions on one daemon from ever seeing each other's calls. + * Omit it (the CLI does) and explore behaves exactly as before, untracked. */ - async execute(toolName: string, args: Record): Promise { + async execute( + toolName: string, + args: Record, + sessionState?: ExploreSessionState, + ): Promise { try { // Block the first tool call on the engine's post-open reconcile so we // never serve rows for files deleted/edited while no MCP server was @@ -1869,9 +1908,20 @@ export class ToolHandler { // cross-cutting notices — worktree-index mismatch (#155) and per-file // staleness (#403) — which need the watched MAIN instance and so are // always applied here, never in the worker. - const result = (this.queryPool && this.queryPool.healthy && this.queryPool.ready) - ? await this.queryPool.run(toolName, args) - : await this.executeReadTool(toolName, args); + // + // Explore also carries the session's own call history down (CG-17) and its + // emission record back up. Both travel as plain properties — on the args + // object down, on the ToolResult up — because either leg may cross a + // structured-clone boundary into a worker, where a closure or a handler + // field could not follow. + const dispatchArgs = this.withSessionView(toolName, args, sessionState); + const raw = (this.queryPool && this.queryPool.healthy && this.queryPool.ready) + ? await this.queryPool.run(toolName, dispatchArgs) + : await this.executeReadTool(toolName, dispatchArgs); + // Record + STRIP before anything else touches the result: the emission is + // internal bookkeeping and must never reach the client, whether or not a + // caller passed session state. + const result = this.takeExploreEmission(raw, sessionState); const withWorktree = this.withWorktreeNotice(result, args.projectPath as string | undefined); return this.withStalenessNotice(withWorktree, args.projectPath as string | undefined); } catch (err) { @@ -1893,6 +1943,57 @@ export class ToolHandler { } } + /** + * Attach the caller's session view to an explore call's args (CG-17), on a + * COPY so the caller's object is never mutated. Nothing else sees it: a + * non-explore tool, or a caller with no session state, gets the args + * unchanged and pays nothing. + * + * A client that spells the internal key itself is stripped rather than + * trusted — the view decides what source a later call may withhold, so it has + * to come from the server's own record, never from the wire. + */ + private withSessionView( + toolName: string, + args: Record, + sessionState: ExploreSessionState | undefined, + ): Record { + if (!(EXPLORE_SESSION_VIEW_ARG in args) && (!sessionState || toolName !== 'codegraph_explore')) { + return args; + } + const copy = { ...args }; + delete copy[EXPLORE_SESSION_VIEW_ARG]; + if (sessionState && toolName === 'codegraph_explore') { + copy[EXPLORE_SESSION_VIEW_ARG] = sessionState.view(); + } + return copy; + } + + /** + * Record an explore call's emission into the caller's session state and strip + * it from the result (CG-17). + * + * Unconditional strip: the property is internal, so it comes off even when + * there is no session state to record it into (the CLI path) — that is what + * keeps the agent-facing response byte-identical. Recording is wrapped + * because a bookkeeping bug must never fail a tool call that already + * succeeded. + */ + private takeExploreEmission( + result: ToolResult, + sessionState: ExploreSessionState | undefined, + ): ToolResult { + const emission = result?.[EXPLORE_EMISSION_KEY]; + if (emission === undefined) return result; + delete result[EXPLORE_EMISSION_KEY]; + if (sessionState) { + try { + sessionState.record(emission); + } catch { /* bookkeeping only — never fail a served call */ } + } + return result; + } + /** * Run a single read tool to completion and return its raw {@link ToolResult}, * classifying expected failures the same way {@link execute}'s catch does so @@ -3029,6 +3130,53 @@ export class ToolHandler { // byte-identical. It only OBSERVES: it must never feed back into rendering. const diag = ExploreDiagnostics.start(query, projectRoot, budget, maxFiles, indexedFileCount); + // What this session has already been served for THIS project (CG-17), and + // whether this call may act on it (CG-18). Dedup is off on the session's + // first call by construction — there is nothing to point back AT — and off + // entirely under `CODEGRAPH_EXPLORE_DEDUP=0`. + const priorCalls = viewForProject(readExploreSessionView(args), projectRoot); + diag?.noteSession(priorCalls); + const dedupEnabled = exploreDedupEnabled() && (priorCalls?.calls.length ?? 0) > 0; + + // Cross-call dedup accounting (CG-18). `newSourceChars` is the load-bearing + // one: a response whose source is ENTIRELY back-references is the shape that + // reads as a failure, so the loop keeps the top suppressed file's real + // section in hand and restores it if nothing new made it in — see + // `suppressedFallback` below. + let newSourceChars = 0; + const backReferencedFiles: string[] = []; + + // What this call ends up emitting, per file — the record handed back to the + // session state on the main thread. Filled by every render path below, then + // filtered to the files that SURVIVE the final hard-ceiling cut, so the + // record is what the agent actually received rather than what the loop + // hoped to send. + // + // Back-referenced spans are recorded too, with zero bytes (CG-18): the + // record means "source the agent HOLDS for this file", not "bytes this call + // spent". Re-recording them refreshes them inside the retained-call window, + // so a file pointed at across many calls doesn't age out of the history and + // get re-served for no reason. + const emittedByFile = new Map< + string, + { ranges: ExploreLineRange[]; bytes: number; fingerprint?: string } + >(); + const noteEmitted = ( + fp: string, + ranges: ExploreLineRange[], + bytes: number, + fingerprint?: string, + ): void => { + const existing = emittedByFile.get(fp); + if (existing) { + existing.ranges.push(...ranges); + existing.bytes += bytes; + if (fingerprint) existing.fingerprint = fingerprint; + } else { + emittedByFile.set(fp, { ranges: [...ranges], bytes, fingerprint }); + } + }; + // Step 1: Find relevant context with generous parameters. // Use a large maxNodes budget — explore has its own 35k char output limit // that prevents context bloat, so more nodes just means better coverage @@ -3042,7 +3190,12 @@ export class ToolHandler { if (subgraph.nodes.size === 0) { diag?.finishEmpty('no relevant code found — empty subgraph'); - return this.textResult(`No relevant code found for "${query}"`); + const empty = `No relevant code found for "${query}"`; + // Still an explore call, so it is still recorded: an empty answer spends a + // call against the tier budget even though it emits no source. + return this.exploreResult(empty, { + projectRoot, query, files: [], sourceBytes: 0, responseBytes: empty.length, + }); } // Graph-aware glue: findRelevantContext builds the subgraph from name/text @@ -3857,6 +4010,28 @@ export class ToolHandler { // instead of a different symbol's code under the requested name. const staleRendered: string[] = []; const staleOmitted: string[] = []; + // Anti-abandonment hold-back (CG-18). The first file dedup suppressed + // ENTIRELY, kept with its real section so it can be put back if the loop + // ends with no new source anywhere. A response made only of pointers is the + // shape that reads as "codegraph has nothing" — and one such response early + // in a session is enough to make an agent stop calling the tool at all — so + // the highest-ranked suppressed file is restored rather than risk it. It + // costs a re-serve of one file, on the one call shape where dedup would + // otherwise have saved everything: the safe direction. + type SuppressedFallback = { + filePath: string; + /** Index in `lines` where this file's pointer block starts. */ + at: number; + /** How many `lines` entries the pointer block occupies. */ + replacing: number; + /** The full, undeduped section to splice back in. */ + section: string[]; + sourceChars: number; + overhead: number; + ranges: ExploreLineRange[]; + fingerprint: string; + }; + let suppressedFallback: SuppressedFallback | null = null; // Reservation carry-forward (CG-21). A reservation is a promise the render // loop has to KEEP, not a cap it may quietly under-use: a file that cannot // spend what it was given — thin matched-symbol set, unreadable, drifted off @@ -3934,6 +4109,135 @@ export class ToolHandler { const fileLines = fileContent.split('\n'); const lang = group.nodes[0]?.language || ''; + const withLineNumbers = exploreLineNumbersEnabled(); + // Language-neutral separator between two non-contiguous slices of one file + // (no `//` — not a comment in Python, Ruby, etc.). With line numbers on, + // the line-number jump also signals the gap. + const GAP_MARKER = '\n\n... (gap) ...\n\n'; + + // Cross-call dedup (CG-18). `served` is what THIS session already sent the + // agent for THIS file, and it is empty unless the file still hashes to the + // bytes those spans were sliced from — an edit between calls means the + // agent's copy is wrong, so nothing is withheld. Every render path below + // routes its spans through `dedupeSpans`, which is the only place a span + // is ever dropped. + const fingerprint = fileFingerprint(fileContent); + const served = dedupEnabled ? servedRangesForFile(priorCalls, filePath, fingerprint) : []; + /** Render one line span exactly as the render paths do. */ + const renderSpan = (r: ExploreLineRange): string => { + const slice = fileLines.slice(r.start - 1, r.end).join('\n'); + return withLineNumbers ? numberSourceLines(slice, r.start) : slice; + }; + /** + * Apply the session history to a set of spans-with-text. A span the agent + * already holds is dropped and reported in `covered`; a partially-held one + * is re-rendered down to its new lines. Text is rebuilt from the surviving + * spans rather than sliced out of the original string — the spans ARE the + * contract with the session record, so rebuilding from them is what keeps + * what we claim to have sent and what we sent the same thing. + */ + const dedupeSpans = ( + parts: ReadonlyArray<{ range: ExploreLineRange; text: string }>, + ): { parts: Array<{ range: ExploreLineRange; text: string }>; covered: ExploreLineRange[] } => { + if (served.length === 0) return { parts: [...parts], covered: [] }; + const kept: Array<{ range: ExploreLineRange; text: string }> = []; + const covered: ExploreLineRange[] = []; + for (const part of parts) { + const split = dedupeRange(part.range, served); + if (split.covered.length === 0) { + kept.push(part); + continue; + } + covered.push(...split.covered); + for (const r of split.emit) kept.push({ range: r, text: renderSpan(r) }); + } + return { parts: kept, covered: mergeRanges(covered) }; + }; + const coveredChars = (spans: ReadonlyArray): number => + spans.reduce((sum, r) => sum + fileLines.slice(r.start - 1, r.end).join('\n').length, 0); + /** + * Emit one file's section — header, the back-reference for whatever the + * agent already holds, and the fence for what is new. Every render path + * ends here so that the dedup bookkeeping (freed bytes, freed `maxFiles` + * slot, the session record, the diagnostic) is written in exactly one + * place and no path can forget a piece of it. + * + * A fully-held file emits its header and pointer and NO fence, and + * deliberately does not consume a `maxFiles` slot: that is half of where + * the reclaimed budget goes (the other half is `sourceSpent`, which the + * carry-forward pool hands down the rank order). Both send bytes to files + * the agent has NOT seen, which is the whole point. + */ + const emitFileSection = (opts: { + header: string; + /** Deduped source. Empty ⇒ the agent already holds all of it. */ + body: string; + /** Spans `body` covers. */ + ranges: ExploreLineRange[]; + /** Spans replaced by the back-reference. */ + covered: ExploreLineRange[]; + /** Chars charged to `totalChars` on top of the body (fences, header). */ + overhead: number; + mode: 'whole' | 'clusters' | 'focused' | 'skeleton'; + clipped: boolean; + /** The undeduped render, kept for the no-new-source fallback. */ + fullBody: string; + fullRanges: ExploreLineRange[]; + }): void => { + // A remainder too small to be worth a fence is folded into the pointer + // (see MIN_DELTA_CHARS). Its ranges are then NOT recorded — the record + // must only ever claim source that was actually sent. + const folded = opts.covered.length > 0 && opts.body.length < EXPLORE_DEDUP.MIN_DELTA_CHARS; + const body = folded ? '' : opts.body; + const ranges = folded ? [] : opts.ranges; + const at = lines.length; + lines.push(opts.header, ''); + if (opts.covered.length > 0) { + const pointer = formatBackReference( + filePath, + opts.covered, + symbolsInSpans(group.nodes, opts.covered), + { partial: body.length > 0 }, + ); + lines.push(pointer, ''); + totalChars += pointer.length + 2; + backReferencedFiles.push(filePath); + } + if (body.length > 0) { + lines.push('```' + lang, body, '```', ''); + totalChars += body.length + opts.overhead; + sourceSpent += body.length; + newSourceChars += body.length; + diag?.recordRender(filePath, opts.mode, body.length, opts.clipped || opts.covered.length > 0); + if (opts.covered.length > 0) diag?.recordDedup(filePath, coveredChars(opts.covered), opts.covered); + noteEmitted(filePath, [...ranges, ...opts.covered], body.length, fingerprint); + renderedFilePaths.push(filePath); + filesIncluded++; + return; + } + // Fully held. The section is the pointer; the slot and the bytes go to a + // file the agent has not seen. The spans are still recorded (at zero + // bytes) because the record means "source the agent HAS", not "bytes + // this call spent" — refreshing them keeps a long session from ageing + // them out of the retained window and re-serving them for nothing. + totalChars += opts.overhead; + diag?.recordRender(filePath, 'backref', 0, false); + diag?.recordDedup(filePath, coveredChars(opts.covered), opts.covered); + noteEmitted(filePath, opts.covered, 0, fingerprint); + renderedFilePaths.push(filePath); + if (!suppressedFallback && opts.fullBody.length > 0) { + suppressedFallback = { + filePath, + at, + replacing: lines.length - at, + section: [opts.header, '', '```' + lang, opts.fullBody, '```', ''], + sourceChars: opts.fullBody.length, + overhead: opts.overhead, + ranges: opts.fullRanges, + fingerprint, + }; + } + }; // Disk-drift gate (#1474): every render branch below except whole-file // slices fileContent (CURRENT bytes) at INDEXED line ranges. Content is @@ -4013,7 +4317,7 @@ export class ToolHandler { // Pass 2: render in line order — full body for chosen symbols, else the // signature line (capped, with a "+N more" tail so the structure map of a // god-file doesn't itself bloat the budget). - const skel: string[] = []; + const skel: Array<{ range: ExploreLineRange; text: string }> = []; let coveredUntil = 0; // skip symbols already inside an emitted body let sigCount = 0, sigDropped = 0; const SIG_MAX = Math.max(12, budget.maxSymbolsInFileHeader * 2); @@ -4022,7 +4326,10 @@ export class ToolHandler { if (bodyIds.has(n.id)) { const end = n.endLine; const body = fileLines.slice(n.startLine - 1, end).join('\n'); - skel.push(exploreLineNumbersEnabled() ? numberSourceLines(body, n.startLine) : body); + skel.push({ + range: { start: n.startLine, end }, + text: withLineNumbers ? numberSourceLines(body, n.startLine) : body, + }); coveredUntil = end; } else { // Elide the body, emit the signature. node.startLine can point at a @@ -4034,10 +4341,16 @@ export class ToolHandler { if (lineNo <= coveredUntil) continue; if (sigCount >= SIG_MAX) { sigDropped++; continue; } const sig = (fileLines[lineNo - 1] || '').trim(); - if (sig) { skel.push(exploreLineNumbersEnabled() ? `${lineNo}\t${sig}` : sig); sigCount++; } + if (sig) { + skel.push({ + range: { start: lineNo, end: lineNo }, + text: withLineNumbers ? `${lineNo}\t${sig}` : sig, + }); + sigCount++; + } } } - if (sigDropped > 0) skel.push(`… +${sigDropped} more (signatures elided)`); + const sigTail = sigDropped > 0 ? `… +${sigDropped} more (signatures elided)` : ''; if (skel.length > 0) { const names = [...new Set(group.nodes.filter(n => n.kind !== 'import' && n.kind !== 'export').map(n => n.name))] .slice(0, budget.maxSymbolsInFileHeader).join(', '); @@ -4050,13 +4363,25 @@ export class ToolHandler { const tag = bodyIds.size > 0 ? 'focused (the methods you named in full, the rest as signatures — codegraph_explore a signature by name for its body; do NOT Read)' : 'skeleton (signatures only — codegraph_explore a name for its full body; do NOT Read)'; - lines.push(fileSectionHeader(filePath, `${names} · ${tag}`), '', '```' + lang, skel.join('\n'), '```', ''); - totalChars += skel.join('\n').length + 120; - sourceSpent += skel.join('\n').length; - // Always "clipped": the per-symbol view elides bodies by construction. - diag?.recordRender(filePath, bodyIds.size > 0 ? 'focused' : 'skeleton', skel.join('\n').length, true); - renderedFilePaths.push(filePath); - filesIncluded++; + // Dedup runs on the per-symbol parts, so a body the agent already has + // becomes a pointer while the signature map around it survives intact + // (a one-line signature is far under MIN_COVERED_LINES and is never + // withheld — the structure map is what makes this render legible). + const dd = dedupeSpans(skel); + const withTail = (parts: ReadonlyArray<{ text: string }>) => + [...parts.map((p) => p.text), ...(sigTail ? [sigTail] : [])].join('\n'); + emitFileSection({ + header: fileSectionHeader(filePath, `${names} · ${tag}`), + body: dd.parts.length > 0 ? withTail(dd.parts) : '', + ranges: dd.parts.map((p) => p.range), + covered: dd.covered, + overhead: 120, + mode: bodyIds.size > 0 ? 'focused' : 'skeleton', + // Always "clipped": the per-symbol view elides bodies by construction. + clipped: true, + fullBody: withTail(skel), + fullRanges: skel.map((p) => p.range), + }); continue; } } @@ -4134,7 +4459,14 @@ export class ToolHandler { && totalChars + fileContent.length + EXPLORE_ALLOCATION.FILE_OVERHEAD <= renderCeiling); if (fileLines.length <= WHOLE_FILE_MAX_LINES && buysWhole) { const body = fileContent.replace(/\n+$/, ''); - let wholeSection = exploreLineNumbersEnabled() ? numberSourceLines(body, 1) : body; + const wholeRange: ExploreLineRange = { start: 1, end: body.split('\n').length }; + const fullSection = withLineNumbers ? numberSourceLines(body, 1) : body; + // The buy decision above was made on the file's FULL size on purpose: it + // asks "did this file's relevance earn all of itself", which dedup does + // not change. Dedup then only ever makes the render smaller, so a buy + // that was funded stays funded. + const ddWhole = dedupeSpans([{ range: wholeRange, text: fullSection }]); + const wholeSection = ddWhole.parts.map((p) => p.text).join(GAP_MARKER); const uniqSymbols = [...new Set( group.nodes .filter(n => n.kind !== 'import' && n.kind !== 'export') @@ -4155,12 +4487,18 @@ export class ToolHandler { diag?.recordSkip(filePath, 'budget-whole-file'); continue; } - lines.push(wholeHeader, '', '```' + lang, wholeSection, '```', ''); - totalChars += wholeSection.length + 200; - sourceSpent += wholeSection.length; - diag?.recordRender(filePath, 'whole', wholeSection.length, false); - renderedFilePaths.push(filePath); - filesIncluded++; + emitFileSection({ + header: wholeHeader, + body: wholeSection, + // The whole file, minus any trailing blank lines the render trimmed. + ranges: ddWhole.parts.map((p) => p.range), + covered: ddWhole.covered, + overhead: 200, + mode: 'whole', + clipped: false, + fullBody: fullSection, + fullRanges: [wholeRange], + }); if (fileStale) staleRendered.push(filePath); continue; } @@ -4313,10 +4651,6 @@ export class ToolHandler { // until the per-file char cap is hit. Truly enormous single clusters // get tail-trimmed with a marker. const contextPadding = 3; - const withLineNumbers = exploreLineNumbersEnabled(); - // Language-neutral separator (no `//` — not a comment in Python, Ruby, - // etc.). With line numbers on, the line-number jump also signals the gap. - const GAP_MARKER = '\n\n... (gap) ...\n\n'; // An oversize spine method (the call path runs THROUGH a god-method — n8n's // processRunExecutionData is 962 lines) is windowed to its next-hop CALL site // plus the signature head, NOT dumped whole. Without this the cluster is too big @@ -4325,28 +4659,50 @@ export class ToolHandler { // the spine's call still appears in context. const OVERSIZE_SPINE_LINES = 200; const SPINE_WINDOW = 28; // lines each side of the next-hop call site - const buildSection = (c: { start: number; end: number; hasSpine?: boolean; spineCallLine?: number }): string => { + // Returns the rendered text as SPAN-KEYED PARTS. Every part carries the + // exact line range its text was sliced from, which two things depend on: + // the session record (CG-17) — a record claiming lines it never sent would + // withhold them from a later call, costing a Read — and cross-call dedup + // (CG-18), which rebuilds a part's text from a narrower span when the + // agent already holds the rest. Both read the spans from the function that + // does the slicing; a second function mirroring these window/padding rules + // would drift. + type SectionPart = { range: ExploreLineRange; text: string }; + const sectionText = (parts: ReadonlyArray): string => + parts.map((p) => p.text).join(GAP_MARKER); + const buildSection = ( + c: { start: number; end: number; hasSpine?: boolean; spineCallLine?: number }, + ): SectionPart[] => { if (c.hasSpine && c.spineCallLine && (c.end - c.start + 1) > OVERSIZE_SPINE_LINES) { const call = c.spineCallLine; const winStart = Math.max(c.start, call - SPINE_WINDOW); const winEnd = Math.min(c.end, call + SPINE_WINDOW); - const parts: string[] = []; + const parts: SectionPart[] = []; // Signature head, only when it sits clearly above the window (else the // window already covers the method opening). const headEnd = Math.min(c.start + 4, winStart - 2); if (headEnd >= c.start) { const head = fileLines.slice(c.start - 1, headEnd).join('\n'); - parts.push(withLineNumbers ? numberSourceLines(head, c.start) : head); + parts.push({ + range: { start: c.start, end: headEnd }, + text: withLineNumbers ? numberSourceLines(head, c.start) : head, + }); } const win = fileLines.slice(winStart - 1, winEnd).join('\n'); - parts.push(withLineNumbers ? numberSourceLines(win, winStart) : win); - return parts.join(GAP_MARKER); + parts.push({ + range: { start: winStart, end: winEnd }, + text: withLineNumbers ? numberSourceLines(win, winStart) : win, + }); + return parts; } const startIdx = Math.max(0, c.start - 1 - contextPadding); const endIdx = Math.min(fileLines.length, c.end + contextPadding); const slice = fileLines.slice(startIdx, endIdx).join('\n'); // startIdx is 0-based, so the slice's first line is line startIdx + 1. - return withLineNumbers ? numberSourceLines(slice, startIdx + 1) : slice; + return [{ + range: { start: startIdx + 1, end: endIdx }, + text: withLineNumbers ? numberSourceLines(slice, startIdx + 1) : slice, + }]; }; /** @@ -4365,7 +4721,7 @@ export class ToolHandler { * body is never cut, and the members are chosen by the same importance the * cluster ranking uses. Returns null when nothing needed shrinking. */ - const shrinkCluster = (c: ExploreCluster, cap: number): string | null => { + const shrinkCluster = (c: ExploreCluster, cap: number): SectionPart[] | null => { if (c.members.length < 2) return null; const byImportance = [...c.members].sort((a, b) => b.importance - a.importance || (a.end - a.start) - (b.end - b.start) || a.start - b.start); @@ -4390,7 +4746,30 @@ export class ToolHandler { 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.map((m) => buildSection(m)).join(GAP_MARKER); + return merged.flatMap((m) => buildSection(m)); + }; + + /** + * One cluster's final parts: built, shrunk if it overruns `cap`, then + * passed through the session history (CG-18). + * + * The shrink decision reads the DEDUPED length on purpose. A cluster whose + * bytes the agent already holds costs this response nothing, so shrinking + * it on its raw size would drop new symbols to make room for source that + * is not being sent — spending the file's budget on nothing. + */ + const renderCluster = ( + c: ExploreCluster, + cap: number, + ): { parts: SectionPart[]; covered: ExploreLineRange[]; shrunk: boolean } => { + const base = dedupeSpans(buildSection(c)); + if (sectionText(base.parts).length <= cap) { + return { parts: base.parts, covered: base.covered, shrunk: false }; + } + const shrunk = shrinkCluster(c, cap); + if (shrunk === null) return { parts: base.parts, covered: base.covered, shrunk: false }; + const dd = dedupeSpans(shrunk); + return { parts: dd.parts, covered: dd.covered, shrunk: true }; }; // Rank clusters for inclusion under the per-file cap. Entry-point @@ -4437,23 +4816,28 @@ export class ToolHandler { // spine can't run away or starve co-flow files entirely. const SPINE_CEILING = Math.min(Math.round(allowance * 1.5), headroom); const chosenIndices = new Set(); - // Shrunk renders for oversize clusters, by cluster index (CG-12). Computed + // Final renders (deduped, shrunk where oversize) by cluster index. Computed // during selection and reused at emission so the two never disagree. - const shrunkSections = new Map(); + const renderedClusters = new Map>(); + let anyClusterShrunk = false; let projectedChars = 0; for (const rc of rankedClusters) { - const sectionLen = buildSection(rc.c).length + (chosenIndices.size > 0 ? GAP_MARKER.length : 0); // 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 // highest-importance whole symbol ranges inside it, so a single-cluster - // god-file spends its allotment instead of the whole response's. - if (chosenIndices.size === 0) { - const cap = rc.c.hasSpine ? SPINE_CEILING : fileBudget; - const shrunk = sectionLen > cap ? shrinkCluster(rc.c, cap) : null; - if (shrunk !== null) shrunkSections.set(rc.idx, shrunk); + // god-file spends its allotment instead of the whole response's. Later + // clusters are never shrunk — they either fit or wait for another call. + const first = chosenIndices.size === 0; + const cap = rc.c.hasSpine ? SPINE_CEILING : fileBudget; + const section = renderCluster(rc.c, first ? cap : Infinity); + const text = sectionText(section.parts); + const sectionLen = text.length + (!first && text.length > 0 ? GAP_MARKER.length : 0); + if (first) { + renderedClusters.set(rc.idx, section); + anyClusterShrunk = anyClusterShrunk || section.shrunk; chosenIndices.add(rc.idx); - projectedChars += shrunk !== null ? shrunk.length : sectionLen; + projectedChars += sectionLen; continue; } // A spine cluster (the rendered call path) is the flow answer — include it @@ -4462,6 +4846,7 @@ export class ToolHandler { const fits = projectedChars + sectionLen <= fileBudget; const spineFits = rc.c.hasSpine && projectedChars + sectionLen <= SPINE_CEILING; if (!fits && !spineFits) continue; + renderedClusters.set(rc.idx, section); chosenIndices.add(rc.idx); projectedChars += sectionLen; } @@ -4469,12 +4854,19 @@ export class ToolHandler { // Emit chosen clusters in source order so the file reads top-to-bottom. let fileSection = ''; const allSymbols: string[] = []; + const sectionRanges: ExploreLineRange[] = []; + const coveredRanges: ExploreLineRange[] = []; for (let i = 0; i < clusters.length; i++) { if (!chosenIndices.has(i)) continue; const cluster = clusters[i]!; - const section = shrunkSections.get(i) ?? buildSection(cluster); - if (fileSection.length > 0) fileSection += GAP_MARKER; - fileSection += section; + const section = renderedClusters.get(i)!; + const text = sectionText(section.parts); + if (text.length > 0) { + if (fileSection.length > 0) fileSection += GAP_MARKER; + fileSection += text; + } + sectionRanges.push(...section.parts.map((p) => p.range)); + coveredRanges.push(...section.covered); allSymbols.push(...cluster.symbols); } @@ -4484,7 +4876,7 @@ export class ToolHandler { // method is useless (the agent just Reads the rest for the other half), which // is the very fallback explore exists to prevent. A pathological file is // bounded by the cluster SELECTION above + the total hard ceiling. - if (chosenIndices.size < clusters.length || shrunkSections.size > 0) { + if (chosenIndices.size < clusters.length || anyClusterShrunk) { anyFileTrimmed = true; } @@ -4522,18 +4914,62 @@ export class ToolHandler { continue; } - lines.push(fileHeader); - lines.push(''); - lines.push('```' + lang); - lines.push(fileSection); - lines.push('```'); - lines.push(''); + // The undeduped render of the same clusters, needed only if this file ends + // up fully back-referenced AND the whole call finds nothing new to say — + // see `suppressedFallback`. Built lazily: on every other call it is dead + // weight. + const fullClusterParts = fileSection.length === 0 + ? clusters.flatMap((c, i) => (chosenIndices.has(i) ? buildSection(c) : [])) + : []; + emitFileSection({ + header: fileHeader, + body: fileSection, + ranges: sectionRanges, + covered: mergeRanges(coveredRanges), + overhead: 200, + mode: 'clusters', + clipped: chosenIndices.size < clusters.length, + fullBody: sectionText(fullClusterParts), + fullRanges: fullClusterParts.map((p) => p.range), + }); + } - totalChars += fileSection.length + 200; - sourceSpent += fileSection.length; - diag?.recordRender(filePath, 'clusters', fileSection.length, chosenIndices.size < clusters.length); - renderedFilePaths.push(filePath); - filesIncluded++; + // Anti-abandonment restore (CG-18). Dedup withheld everything and nothing new + // took its place — the response would be pointers only, which is the shape + // that reads as "codegraph found nothing" and sends the agent to Read for + // good. Put the top suppressed file back, in full, and keep its pointer off. + // Deliberately checked against `newSourceChars` (source THIS call emitted) + // rather than the response length: the flow and blast-radius sections are + // always there, and they are not what makes a response feel sufficient. + // Cast, not annotation: the only writer is the render loop's `emitFileSection` + // closure, which TypeScript's flow analysis cannot see, so it narrows the + // variable to `null` here and the truthiness check below would be `never`. + const restore = suppressedFallback as SuppressedFallback | null; + if (newSourceChars === 0 && restore) { + if (totalChars + restore.sourceChars + restore.overhead <= renderCeiling) { + lines.splice(restore.at, restore.replacing, ...restore.section); + totalChars += restore.sourceChars + restore.overhead; + sourceSpent += restore.sourceChars; + newSourceChars += restore.sourceChars; + filesIncluded++; + const idx = backReferencedFiles.indexOf(restore.filePath); + if (idx >= 0) backReferencedFiles.splice(idx, 1); + emittedByFile.set(restore.filePath, { + ranges: [...restore.ranges], + bytes: restore.sourceChars, + fingerprint: restore.fingerprint, + }); + diag?.recordRender(restore.filePath, 'clusters', restore.sourceChars, false); + diag?.recordDedup(restore.filePath, 0, []); + } + } + + // 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 + // already has. + if (backReferencedFiles.length > 0) { + lines[verbatimHeaderIdx] += ` (Files marked **"Already sent earlier in this conversation"** are not repeated: their source came back on an earlier codegraph_explore call in THIS conversation and the file has not changed since, so that copy is exact and current — scroll back for it rather than re-fetching or Reading.)`; } // Drift epilogue (#1474). The "verbatim / do not Read" guarantee above @@ -4678,7 +5114,45 @@ export class ToolHandler { // shares account for the hard-ceiling truncation above (CG-4). diag?.finish(finalText, output.length, hardCeiling, filesIncluded); - return this.textResult(finalText); + // Session record (CG-17): only the files that SURVIVED the hard ceiling — + // a section the truncation dropped was never delivered, and recording it + // would let a later call withhold source the agent has never seen. + // + // A back-referenced file records its spans at ZERO bytes (CG-18) — it is + // still source the agent holds for this file, which is what the record + // means; dropping it would let the span age out of the retained window and + // be re-served for nothing. + const emittedFiles: ExploreFileEmission[] = []; + let sourceBytes = 0; + for (const fp of survivors) { + const emitted = emittedByFile.get(fp); + if (!emitted || emitted.ranges.length === 0) continue; + emittedFiles.push({ + path: fp, + ranges: emitted.ranges, + bytes: emitted.bytes, + fingerprint: emitted.fingerprint, + }); + sourceBytes += emitted.bytes; + } + return this.exploreResult(finalText, { + projectRoot, + query, + files: emittedFiles, + sourceBytes, + responseBytes: finalText.length, + }); + } + + /** + * An explore response plus the record of what it emitted (CG-17). The record + * rides the result only as far as {@link execute}, which files it into the + * calling session's state and deletes it — see {@link EXPLORE_EMISSION_KEY}. + */ + private exploreResult(text: string, emission: ExploreEmission): ToolResult { + const result = this.textResult(text); + result[EXPLORE_EMISSION_KEY] = emission; + return result; } /**