diff --git a/gitnexus/src/core/ingestion/binding-accumulator.ts b/gitnexus/src/core/ingestion/binding-accumulator.ts index 306ef9d06..3d0c3708f 100644 --- a/gitnexus/src/core/ingestion/binding-accumulator.ts +++ b/gitnexus/src/core/ingestion/binding-accumulator.ts @@ -60,6 +60,7 @@ export class BindingAccumulator { private readonly _fileScopeByFile = new Map(); private _totalBindings = 0; private _finalized = false; + private _disposed = false; /** * Append bindings for a file. Safe to call multiple times for the same file. @@ -100,6 +101,40 @@ export class BindingAccumulator { this._finalized = true; } + /** + * Release the accumulator's heap footprint. Clears both internal storage + * maps and resets `_totalBindings` to zero. Idempotent and orthogonal to + * `finalize()` — calling `dispose()` does not change the finalized state. + * + * Post-dispose contract: all read methods return empty/undefined state + * matching a never-appended-to accumulator. Specifically: + * - `fileCount === 0` + * - `totalBindings === 0` + * - `files()` yields an empty iterator + * - `getFile(x)` returns `undefined` for all `x` + * - `fileScopeEntries(x)` returns `[]` for all `x` + * - `estimateMemoryBytes()` returns `0` + * + * If `dispose()` is called **before** `finalize()`, subsequent `appendFile` + * calls succeed — the accumulator behaves like a fresh one. If called + * **after** `finalize()`, subsequent `appendFile` calls throw the existing + * "finalized" error. + * + * Added in response to PR #743 Codex adversarial review (plan + * 2026-04-09-005): the pipeline disposes the accumulator after the + * ExportedTypeMap enrichment loop consumes its file-scope entries, so + * the heap is released before Phase 14 (`runCrossFileBindingPropagation`) + * and `runGraphAnalysisPhases` begin their long-running work. When Phase 9 + * wires a consumer into that stage, the dispose call should move later in + * the pipeline or be removed entirely. + */ + dispose(): void { + this._allByFile.clear(); + this._fileScopeByFile.clear(); + this._totalBindings = 0; + this._disposed = true; + } + /** Get all bindings for a file, or undefined if the file is unknown. */ getFile(filePath: string): readonly BindingEntry[] | undefined { return this._allByFile.get(filePath); diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 5f6e55c0a..9f3c0c8e8 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -1721,6 +1721,17 @@ export const runPipelineFromRepo = async ( ); } + // PR #743 Codex adversarial review follow-up (plan 2026-04-09-005): + // Release the accumulator's heap footprint now. The ExportedTypeMap + // enrichment loop above is the only current consumer, and the dev + // telemetry log just captured peak state. Phase 14 and + // runGraphAnalysisPhases do not read the accumulator today — keeping + // it alive through those long-running phases pins heap for no reason. + // When Phase 9 wires a consumer into runCrossFileBindingPropagation, + // move this dispose() call to after that consumer completes or delete + // it entirely if the consumer takes lifecycle ownership. + bindingAccumulator.dispose(); + // ── Phase 14: Cross-file binding propagation (topological level sort) ── await runCrossFileBindingPropagation( graph, diff --git a/gitnexus/src/core/ingestion/type-env.ts b/gitnexus/src/core/ingestion/type-env.ts index 3e621ce58..211deddbd 100644 --- a/gitnexus/src/core/ingestion/type-env.ts +++ b/gitnexus/src/core/ingestion/type-env.ts @@ -1256,11 +1256,29 @@ export const buildTypeEnv = ( throw new Error(`TypeEnv.flush called twice for ${filePath} — flush is single-use`); } flushed = true; + // PR #743 Codex adversarial review follow-up (plan 2026-04-09-005): + // Narrow flush() to iterate only the FILE_SCOPE entry, mirroring the + // worker-path narrowing in parse-worker.ts (commit 803631fe). Before + // this change, both execution paths had the same asymmetry bug: the + // worker path was fixed but the sequential path (this code) still + // wrote function-scope entries into long-lived accumulator storage + // that no consumer reads until Phase 9 lands. + // + // Phase 9 reversion: when a downstream consumer of function-scope + // bindings exists, restore the nested iteration: + // + // for (const [scope, scopeMap] of env) { + // for (const [varName, typeName] of scopeMap) { + // entries.push({ scope, varName, typeName }); + // } + // } + // + // See BindingAccumulator class JSDoc and FileAllScopeBindings JSDoc in + // parse-worker.ts for the full reversion checklist. + const fileScope = env.get(FILE_SCOPE) ?? EMPTY_FILE_SCOPE; const entries: BindingEntry[] = []; - for (const [scope, scopeMap] of env) { - for (const [varName, typeName] of scopeMap) { - entries.push({ scope, varName, typeName }); - } + for (const [varName, typeName] of fileScope) { + entries.push({ scope: '', varName, typeName }); } if (entries.length > 0) { accumulator.appendFile(filePath, entries); diff --git a/gitnexus/test/unit/binding-accumulator.test.ts b/gitnexus/test/unit/binding-accumulator.test.ts index 93cf7e208..2df30aa82 100644 --- a/gitnexus/test/unit/binding-accumulator.test.ts +++ b/gitnexus/test/unit/binding-accumulator.test.ts @@ -498,4 +498,95 @@ describe('BindingAccumulator', () => { expect(exportedTypeMap.has('src/missing.ts')).toBe(false); }); }); + + // ------------------------------------------------------------------------- + // PR #743 Codex adversarial review follow-up (plan 2026-04-09-005): + // BindingAccumulator.dispose() releases the accumulator's heap footprint + // after the enrichment loop has consumed everything it needs. Post-dispose + // reads return empty/undefined without throwing, matching "never-appended" + // state. Idempotent and orthogonal to finalize(). + // ------------------------------------------------------------------------- + + describe('dispose', () => { + it('empties all read methods after dispose', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/a.ts', [ + { scope: '', varName: 'x', typeName: 'X' }, + { scope: 'fn@10', varName: 'y', typeName: 'Y' }, + ]); + acc.appendFile('src/b.ts', [{ scope: '', varName: 'z', typeName: 'Z' }]); + + // Sanity: pre-dispose state is populated. + expect(acc.fileCount).toBe(2); + expect(acc.totalBindings).toBe(3); + + acc.dispose(); + + // Post-dispose state: all read methods return empty/undefined. + expect(acc.fileCount).toBe(0); + expect(acc.totalBindings).toBe(0); + expect([...acc.files()]).toEqual([]); + expect(acc.getFile('src/a.ts')).toBeUndefined(); + expect(acc.getFile('src/b.ts')).toBeUndefined(); + expect(acc.fileScopeEntries('src/a.ts')).toEqual([]); + expect(acc.fileScopeEntries('src/b.ts')).toEqual([]); + }); + + it('is idempotent — calling twice is a no-op', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/a.ts', [{ scope: '', varName: 'x', typeName: 'X' }]); + acc.dispose(); + expect(() => acc.dispose()).not.toThrow(); + expect(acc.fileCount).toBe(0); + expect(acc.totalBindings).toBe(0); + }); + + it('works before finalize() — accumulator behaves like a fresh one after dispose', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/a.ts', [{ scope: '', varName: 'x', typeName: 'X' }]); + acc.dispose(); + // Not finalized, so appends still work post-dispose. + expect(() => + acc.appendFile('src/b.ts', [{ scope: '', varName: 'y', typeName: 'Y' }]), + ).not.toThrow(); + expect(acc.fileCount).toBe(1); + expect(acc.totalBindings).toBe(1); + expect(acc.getFile('src/b.ts')).toHaveLength(1); + expect(acc.getFile('src/a.ts')).toBeUndefined(); + }); + + it('works after finalize() — append still throws, reads return empty', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/a.ts', [{ scope: '', varName: 'x', typeName: 'X' }]); + acc.finalize(); + acc.dispose(); + // Finalized, so appends throw even post-dispose. + expect(() => + acc.appendFile('src/b.ts', [{ scope: '', varName: 'y', typeName: 'Y' }]), + ).toThrow(/finalized/); + // But reads return empty. + expect(acc.fileCount).toBe(0); + expect(acc.totalBindings).toBe(0); + expect(acc.getFile('src/a.ts')).toBeUndefined(); + }); + + it('estimateMemoryBytes drops to zero after dispose', () => { + const acc = new BindingAccumulator(); + // Populate a large batch to give the estimate a non-trivial baseline. + for (let i = 0; i < 100; i++) { + acc.appendFile(`src/file${i}.ts`, [ + { scope: '', varName: `var${i}a`, typeName: 'string' }, + { scope: '', varName: `var${i}b`, typeName: 'number' }, + ]); + } + const preDisposeBytes = acc.estimateMemoryBytes(); + expect(preDisposeBytes).toBeGreaterThan(0); + + acc.dispose(); + + // After dispose, the iteration over `_allByFile` in estimateMemoryBytes + // has zero files to walk, so the returned value is exactly 0. + expect(acc.estimateMemoryBytes()).toBe(0); + }); + }); }); diff --git a/gitnexus/test/unit/type-env.test.ts b/gitnexus/test/unit/type-env.test.ts index c8691f79e..5ec3cebe2 100644 --- a/gitnexus/test/unit/type-env.test.ts +++ b/gitnexus/test/unit/type-env.test.ts @@ -5878,7 +5878,14 @@ function process() { expect(userEntry!.scope).toBe(''); }); - it('flushes function-scoped bindings into accumulator', () => { + // PR #743 Codex adversarial review follow-up (plan 2026-04-09-005): + // flush() was narrowed to file-scope-only to match the worker-path + // narrowing in commit 803631fe. Function-scope entries are dropped at + // the flush seam and never reach the accumulator until a Phase 9 + // consumer lands. This test was previously the positive assertion that + // function-scope entries DID land in the accumulator; it is now a + // negative assertion guarding the narrowing. + it('does NOT flush function-scoped bindings into accumulator (narrowed per PR #743 Codex review)', () => { const code = `function process() {\n const result: Response = fetch();\n}`; const tree = parse(code, TypeScript.typescript); const typeEnv = buildTypeEnv(tree, 'typescript'); @@ -5886,12 +5893,37 @@ function process() { typeEnv.flush('/src/test.ts', acc); + // With only a function-scope binding (`result` inside `process()`) and + // no file-scope bindings, the accumulator should have nothing for this + // file — the function-scope entry is dropped at the flush boundary. const entries = acc.getFile('/src/test.ts'); + expect(entries).toBeUndefined(); + expect(acc.fileCount).toBe(0); + expect(acc.totalBindings).toBe(0); + }); + + it('narrows mixed file-scope and function-scope env to file-scope only', () => { + // Core narrowing assertion: a realistic file with BOTH file-scope and + // function-scope bindings flushes only the file-scope subset. This is + // the R1 red/green signal for plan 2026-04-09-005. + const code = `const dbClient: Database = connectDb();\nfunction handleRequest() {\n const localRequest: Request = parseRequest();\n const localUser: User = loadUser();\n}`; + const tree = parse(code, TypeScript.typescript); + const typeEnv = buildTypeEnv(tree, 'typescript'); + const acc = new BindingAccumulator(); + + typeEnv.flush('/src/service.ts', acc); + + const entries = acc.getFile('/src/service.ts'); expect(entries).toBeDefined(); - const resultEntry = entries!.find((e) => e.varName === 'result'); - expect(resultEntry).toBeDefined(); - expect(resultEntry!.typeName).toBe('Response'); - expect(resultEntry!.scope).not.toBe(''); + // Exactly one entry: the file-scope `dbClient`. The two function-scope + // entries (`localRequest`, `localUser`) are dropped. + expect(entries).toHaveLength(1); + expect(entries![0].scope).toBe(''); + expect(entries![0].varName).toBe('dbClient'); + expect(entries![0].typeName).toBe('Database'); + // Function-scope entries are absent from the accumulator. + expect(entries!.find((e) => e.varName === 'localRequest')).toBeUndefined(); + expect(entries!.find((e) => e.varName === 'localUser')).toBeUndefined(); }); it('flushes nothing for an empty TypeEnv', () => {