From 273cd3ada7f8d4f3f3f158f2dee29cb02f310b37 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 10 Apr 2026 06:25:12 +0100 Subject: [PATCH] fix(SM-15): address all PR #763 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performance (R1) - Changed _fileScopeByFile from Map to Map>. fileScopeGet(filePath, name) is now O(1) — replaces the O(n) linear scan + defensive-copy alloc that ran once per ConstructorBinding entry. fileScopeEntries() reconstructs tuples from Map.entries() for backward compat. - Updated finalize() dev-mode invariant to compare deduplicated Map size rather than raw array length (Map.set deduplicates same-name). Lifecycle (R2) - Documented that Phase 9 intentionally reads pre-finalize because finalize() cannot move before both the worker consumer (line 984) AND the sequential-path writer (line 1061). Pre-finalize reads are safe because finalize() is write-lock-only with no side effects. Replaced the ambiguous "populated but not yet finalized" comment with the full lifecycle ordering explanation. Sequential-path parity (R3) - Wired bindingAccumulator into processCalls at line 797 (sequential path) so verifyConstructorBindings gets the Phase 9 fallback. - Added bindingAccumulator parameter to processAssignmentsFromExtracted signature and wired it at the pipeline.ts call site (line 1026). - Both paths now produce identical Phase 9 behavior for the same code. Tracking comments (R4) - Added "Overlapping mechanism (N of 3)" cross-references at: 1. buildImportedReturnTypes (~line 109) 2. collectExportedBindings (~line 168) 3. Phase 9 fallback in verifyConstructorBindings (~line 563) Each links to the other two and notes future unification. Language coverage (R5) - Added 5 new Phase 9 integration test suites in cross-file-binding.test.ts: JavaScript, C++, C#, PHP, Ruby. Each uses the existing fixture directories and asserts getUser() → User → user.save() resolves. Total cross-file binding tests: 52 (was 37). Quality asymmetry (R6) - Added inline comment at the Phase 9 fallback noting worker-path entries are Tier 0/1 only and that binding accuracy is structurally lower for large repos where the worker path dominates. Tests (+21 new) - 6 fileScopeGet unit tests (happy path, unknown file/name, mixed scopes, post-dispose, duplicate varName last-write-wins) - 15 integration tests across 5 new language suites Verification - tsc --noEmit clean - 3147 unit tests pass (+6 new) - 52 cross-file binding integration tests pass (+15 new) - 1766 resolver integration tests pass - Zero regressions Plan: docs/plans/2026-04-10-001-fix-sm15-review-findings-plan.md Review: https://github.com/abhigyanpatwari/GitNexus/pull/763#issuecomment-4220354242 --- AGENTS.md | 2 +- CLAUDE.md | 102 ++++++++++++- .../src/core/ingestion/binding-accumulator.ts | 82 +++++++---- gitnexus/src/core/ingestion/call-processor.ts | 63 ++++++-- gitnexus/src/core/ingestion/pipeline.ts | 13 +- .../integration/cross-file-binding.test.ts | 137 ++++++++++++++++++ .../test/unit/binding-accumulator.test.ts | 48 ++++++ 7 files changed, 399 insertions(+), 48 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 09c3eeb12..e6cefed11 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,7 +63,7 @@ Generic “core standards” playbooks are often long and stack-specific. For th # GitNexus — Code Intelligence -This project is indexed by GitNexus as **GitNexus** (3298 symbols, 7954 relationships, 185 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **GitNexus** (3883 symbols, 9861 relationships, 225 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first. diff --git a/CLAUDE.md b/CLAUDE.md index 27ab5de74..7b0f175b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,7 +49,107 @@ If always-on instructions grow, load deep conventions via conditional reads (e.g ## GitNexus rules -GitNexus MCP rules are in the `` … `` block in **[AGENTS.md](AGENTS.md)** — load that section when working with MCP tools or the graph index. +GitNexus MCP rules are in the ` +# GitNexus — Code Intelligence + +This project is indexed by GitNexus as **GitNexus** (3883 symbols, 9861 relationships, 225 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. + +> If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first. + +## Always Do + +- **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run `gitnexus_impact({target: "symbolName", direction: "upstream"})` and report the blast radius (direct callers, affected processes, risk level) to the user. +- **MUST run `gitnexus_detect_changes()` before committing** to verify your changes only affect expected symbols and execution flows. +- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- When exploring unfamiliar code, use `gitnexus_query({query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. +- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `gitnexus_context({name: "symbolName"})`. + +## When Debugging + +1. `gitnexus_query({query: ""})` — find execution flows related to the issue +2. `gitnexus_context({name: ""})` — see all callers, callees, and process participation +3. `READ gitnexus://repo/GitNexus/process/{processName}` — trace the full execution flow step by step +4. For regressions: `gitnexus_detect_changes({scope: "compare", base_ref: "main"})` — see what your branch changed + +## When Refactoring + +- **Renaming**: MUST use `gitnexus_rename({symbol_name: "old", new_name: "new", dry_run: true})` first. Review the preview — graph edits are safe, text_search edits need manual review. Then run with `dry_run: false`. +- **Extracting/Splitting**: MUST run `gitnexus_context({name: "target"})` to see all incoming/outgoing refs, then `gitnexus_impact({target: "target", direction: "upstream"})` to find all external callers before moving code. +- After any refactor: run `gitnexus_detect_changes({scope: "all"})` to verify only expected files changed. + +## Never Do + +- NEVER edit a function, class, or method without first running `gitnexus_impact` on it. +- NEVER ignore HIGH or CRITICAL risk warnings from impact analysis. +- NEVER rename symbols with find-and-replace — use `gitnexus_rename` which understands the call graph. +- NEVER commit changes without running `gitnexus_detect_changes()` to check affected scope. + +## Tools Quick Reference + +| Tool | When to use | Command | +|------|-------------|---------| +| `query` | Find code by concept | `gitnexus_query({query: "auth validation"})` | +| `context` | 360-degree view of one symbol | `gitnexus_context({name: "validateUser"})` | +| `impact` | Blast radius before editing | `gitnexus_impact({target: "X", direction: "upstream"})` | +| `detect_changes` | Pre-commit scope check | `gitnexus_detect_changes({scope: "staged"})` | +| `rename` | Safe multi-file rename | `gitnexus_rename({symbol_name: "old", new_name: "new", dry_run: true})` | +| `cypher` | Custom graph queries | `gitnexus_cypher({query: "MATCH ..."})` | + +## Impact Risk Levels + +| Depth | Meaning | Action | +|-------|---------|--------| +| d=1 | WILL BREAK — direct callers/importers | MUST update these | +| d=2 | LIKELY AFFECTED — indirect deps | Should test | +| d=3 | MAY NEED TESTING — transitive | Test if critical path | + +## Resources + +| Resource | Use for | +|----------|---------| +| `gitnexus://repo/GitNexus/context` | Codebase overview, check index freshness | +| `gitnexus://repo/GitNexus/clusters` | All functional areas | +| `gitnexus://repo/GitNexus/processes` | All execution flows | +| `gitnexus://repo/GitNexus/process/{name}` | Step-by-step execution trace | + +## Self-Check Before Finishing + +Before completing any code modification task, verify: +1. `gitnexus_impact` was run for all modified symbols +2. No HIGH/CRITICAL risk warnings were ignored +3. `gitnexus_detect_changes()` confirms changes match expected scope +4. All d=1 (WILL BREAK) dependents were updated + +## Keeping the Index Fresh + +After committing code changes, the GitNexus index becomes stale. Re-run analyze to update it: + +```bash +npx gitnexus analyze +``` + +If the index previously included embeddings, preserve them by adding `--embeddings`: + +```bash +npx gitnexus analyze --embeddings +``` + +To check whether embeddings exist, inspect `.gitnexus/meta.json` — the `stats.embeddings` field shows the count (0 means no embeddings). **Running analyze without `--embeddings` will delete any previously generated embeddings.** + +> Claude Code users: A PostToolUse hook handles this automatically after `git commit` and `git merge`. + +## CLI + +| Task | Read this skill file | +|------|---------------------| +| Understand architecture / "How does X work?" | `.claude/skills/gitnexus/gitnexus-exploring/SKILL.md` | +| Blast radius / "What breaks if I change X?" | `.claude/skills/gitnexus/gitnexus-impact-analysis/SKILL.md` | +| Trace bugs / "Why is X failing?" | `.claude/skills/gitnexus/gitnexus-debugging/SKILL.md` | +| Rename / extract / split / refactor | `.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md` | +| Tools, resources, schema reference | `.claude/skills/gitnexus/gitnexus-guide/SKILL.md` | +| Index, status, clean, wiki CLI commands | `.claude/skills/gitnexus/gitnexus-cli/SKILL.md` | + +` block in **[AGENTS.md](AGENTS.md)** — load that section when working with MCP tools or the graph index. # GitNexus — Code Intelligence diff --git a/gitnexus/src/core/ingestion/binding-accumulator.ts b/gitnexus/src/core/ingestion/binding-accumulator.ts index 3e7563623..d3acbf7a1 100644 --- a/gitnexus/src/core/ingestion/binding-accumulator.ts +++ b/gitnexus/src/core/ingestion/binding-accumulator.ts @@ -139,18 +139,24 @@ const ENTRY_OVERHEAD = 64; // bytes per entry (object overhead + property refs) const MAP_ENTRY_OVERHEAD = 80; // bytes per file entry in the map export class BindingAccumulator { - // Storage is split into two parallel maps so fileScopeEntries() is - // O(n_file_scope) instead of O(n_total). + // Storage is split into two parallel maps so file-scope reads are fast. // - _allByFile holds every BindingEntry (used by getFile, memory estimate). - // - _fileScopeByFile caches the flat [varName, typeName] view of the - // `scope === ''` subset, populated at insert time so reads are O(1) map - // lookup + O(n_file_scope) array return. Both maps carry the same key - // set modulo the `scope === ''` precondition: _allByFile has a key as - // soon as any entry is appended; _fileScopeByFile only has a key once a - // file-scope entry arrives. Code that iterates via files() uses - // _allByFile so files with only function-scope entries remain visible. + // - _fileScopeByFile is a nested Map> for + // O(1) point-lookup via fileScopeGet(). For iteration-based consumers + // (enrichExportedTypeMap), fileScopeEntries() iterates the inner Map. + // Both maps carry the same key set modulo the `scope === ''` precondition: + // _allByFile has a key as soon as any entry is appended; _fileScopeByFile + // only has a key once a file-scope entry arrives. Code that iterates via + // files() uses _allByFile so files with only function-scope entries + // remain visible. + // + // Note: Map.set semantics mean a duplicate varName for the same file + // overwrites the previous value (last-write-wins). This is the correct + // behavior — duplicate top-level bindings in the same file shouldn't + // happen in well-formed source, and if they do the last declaration + // is typically the one the compiler sees. private readonly _allByFile = new Map(); - private readonly _fileScopeByFile = new Map(); + private readonly _fileScopeByFile = new Map>(); private _totalBindings = 0; private _finalized = false; private _disposed = false; @@ -202,15 +208,16 @@ export class BindingAccumulator { } else { this._allByFile.set(filePath, entries.slice()); } - // File-scope fast-path store. Populated lazily on first file-scope entry. - let existingFileScope = this._fileScopeByFile.get(filePath); + // File-scope fast-path store (nested Map for O(1) point-lookup via fileScopeGet). + // Populated lazily on first file-scope entry per file. + let fileScopeMap = this._fileScopeByFile.get(filePath); for (const e of entries) { if (e.scope === '') { - if (existingFileScope === undefined) { - existingFileScope = []; - this._fileScopeByFile.set(filePath, existingFileScope); + if (fileScopeMap === undefined) { + fileScopeMap = new Map(); + this._fileScopeByFile.set(filePath, fileScopeMap); } - existingFileScope.push([e.varName, e.typeName]); + fileScopeMap.set(e.varName, e.typeName); } } this._totalBindings += entries.length; @@ -225,7 +232,7 @@ export class BindingAccumulator { // indicate a bug in `appendFile()` where one map was updated but // not the other. if (process.env.NODE_ENV !== 'production' && !this._finalized) { - for (const [filePath, fileScopeTuples] of this._fileScopeByFile) { + for (const [filePath, fileScopeMap] of this._fileScopeByFile) { const allEntries = this._allByFile.get(filePath); if (allEntries === undefined) { throw new Error( @@ -233,12 +240,16 @@ export class BindingAccumulator { `but no _allByFile entry`, ); } - const projectedCount = allEntries.filter((e) => e.scope === '').length; - if (projectedCount !== fileScopeTuples.length) { + // Count unique file-scope varNames in _allByFile (to match Map dedup + // semantics in _fileScopeByFile where Map.set deduplicates same-name). + const projectedNames = new Set( + allEntries.filter((e) => e.scope === '').map((e) => e.varName), + ); + if (projectedNames.size !== fileScopeMap.size) { throw new Error( `[BindingAccumulator] storage split drift: file ${filePath} has ` + - `${fileScopeTuples.length} file-scope tuples but ${projectedCount} file-scope ` + - `entries in _allByFile`, + `${fileScopeMap.size} file-scope names in Map but ${projectedNames.size} unique ` + + `file-scope varNames in _allByFile`, ); } } @@ -286,21 +297,30 @@ export class BindingAccumulator { /** * Get only scope='' (file-level) entries as [varName, typeName] tuples. - * Backward-compatible with the old workerTypeEnvBindings pattern. + * For iteration-based consumers (e.g., `enrichExportedTypeMap`). * Returns an empty array for an unknown file. * - * O(1) map lookup + O(n_file_scope) defensive-copy construction — does - * NOT walk function-scope entries. See the `_fileScopeByFile` field - * comment for the storage split rationale. + * O(1) map lookup + O(n_file_scope) tuple reconstruction from the inner + * Map. Does NOT walk function-scope entries. * - * The return value is a shallow copy; mutating it does not affect - * subsequent reads or internal state. This encapsulation guard prevents - * a Phase 9 consumer from accidentally corrupting the accumulator via - * `acc.fileScopeEntries(p).push(...)` or similar. + * For point-lookup consumers (e.g., Phase 9 fallback), prefer + * `fileScopeGet(filePath, name)` — O(1) with no allocation. */ fileScopeEntries(filePath: string): readonly (readonly [string, string])[] { - const cached = this._fileScopeByFile.get(filePath); - return cached ? cached.slice() : []; + const map = this._fileScopeByFile.get(filePath); + return map ? [...map.entries()] : []; + } + + /** + * O(1) point-lookup for a single file-scope binding by (filePath, name). + * Returns the typeName if found, `undefined` otherwise. + * + * This is the preferred lookup path for Phase 9 consumers that resolve + * a single callee's return type — avoids the O(n_file_scope) iteration + * and defensive-copy allocation of `fileScopeEntries()`. + */ + fileScopeGet(filePath: string, name: string): string | undefined { + return this._fileScopeByFile.get(filePath)?.get(name); } /** Iterate over all file paths in insertion order. */ diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 2322d0693..db28376b5 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -105,7 +105,13 @@ const MAX_EXPORTS_PER_FILE = 500; const MAX_TYPE_NAME_LENGTH = 256; /** Build a map of imported callee names → return types for cross-file call-result binding. - * Consulted ONLY when SymbolTable has no unambiguous local match (local-first principle). */ + * Consulted ONLY when SymbolTable has no unambiguous local match (local-first principle). + * + * Overlapping mechanism (1 of 3): this is the SymbolTable-backed path. + * See also: + * 2. collectExportedBindings (~line 168) / enrichExportedTypeMap — TypeEnv + graph isExported + * 3. Phase 9 fallback in verifyConstructorBindings (~line 563) — namedImportMap + BindingAccumulator + * A future cleanup should merge these into a single resolution pass. */ export function buildImportedReturnTypes( filePath: string, namedImportMap: ReadonlyMap< @@ -163,8 +169,13 @@ export function buildImportedRawReturnTypes( * quality enrichment"). Both sites populate the same map with subtly * different export-check semantics — this site uses SymbolTable + * graph lookup, the worker loop uses three-candidate-ID graph lookup. - * They must stay in sync until Phase 9 unifies them. If you edit one, - * check the other. */ + * They must stay in sync until unified. If you edit one, check the other. + * + * Overlapping mechanism (2 of 3): this is the TypeEnv + graph isExported path. + * See also: + * 1. buildImportedReturnTypes (~line 109) — namedImportMap + SymbolTable + * 3. Phase 9 fallback in verifyConstructorBindings (~line 563) — namedImportMap + BindingAccumulator + * A future cleanup should merge these into a single resolution pass. */ function collectExportedBindings( typeEnv: { fileScope(): ReadonlyMap }, filePath: string, @@ -559,18 +570,33 @@ const verifyConstructorBindings = ( // (e.g., a return type that TypeEnv resolved via fixpoint in the source // file but was not stored as a SymbolTable returnType annotation). // namedImportMap tells us which source file exported the callee so we - // can look up its file-scope binding directly in the accumulator. + // can look up its file-scope binding via the O(1) fileScopeGet method. + // + // Quality note: worker-path accumulator entries are Tier 0/1 only + // (annotation-declared + same-file constructor inference) — see the + // BindingAccumulator class JSDoc. For large repos where the worker + // path dominates, Phase 9 binding accuracy is structurally lower + // than for sequential-path repos where Tier 2 cross-file propagation + // is available. + // + // Overlapping mechanism note: this is one of three cross-file + // return-type resolution paths in the codebase: + // 1. buildImportedReturnTypes (~line 109) — namedImportMap + + // SymbolTable.lookupExactFull (structure-processor captured) + // 2. collectExportedBindings (~line 168) / enrichExportedTypeMap + // — TypeEnv + graph isExported flag + // 3. This fallback — namedImportMap + BindingAccumulator + // A future cleanup should merge these into a single resolution pass. if (!typeName && bindingAccumulator) { const namedImports = ctx.namedImportMap.get(filePath); const importBinding = namedImports?.get(calleeName); if (importBinding) { - for (const [name, rawType] of bindingAccumulator.fileScopeEntries( + const rawType = bindingAccumulator.fileScopeGet( importBinding.sourcePath, - )) { - if (name === importBinding.exportedName) { - typeName = extractReturnTypeName(rawType); - break; - } + importBinding.exportedName, + ); + if (rawType) { + typeName = extractReturnTypeName(rawType); } } } @@ -779,7 +805,13 @@ export const processCalls = async ( const verifiedReceivers = typeEnv.constructorBindings.length > 0 - ? verifyConstructorBindings(typeEnv.constructorBindings, file.path, ctx) + ? verifyConstructorBindings( + typeEnv.constructorBindings, + file.path, + ctx, + undefined, // graph not available on the sequential path here + bindingAccumulator, // Phase 9 fallback — same as worker path (R3 parity) + ) : new Map(); const receiverIndex = buildReceiverTypeIndex(verifiedReceivers); @@ -2724,12 +2756,19 @@ export const processAssignmentsFromExtracted = ( assignments: ExtractedAssignment[], ctx: ResolutionContext, constructorBindings?: FileConstructorBindings[], + bindingAccumulator?: BindingAccumulator, ): void => { // Build per-file receiver type indexes from verified constructor bindings const fileReceiverTypes = new Map(); if (constructorBindings) { for (const { filePath, bindings } of constructorBindings) { - const verified = verifyConstructorBindings(bindings, filePath, ctx, graph); + const verified = verifyConstructorBindings( + bindings, + filePath, + ctx, + graph, + bindingAccumulator, + ); if (verified.size > 0) { fileReceiverTypes.set(filePath, buildReceiverTypeIndex(verified)); } diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 1996e65b1..9c754ed2f 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -1003,9 +1003,15 @@ async function runChunkedParseAndResolve( // Phase 9: pass the accumulator so processCallsFromExtracted can fall back // to file-scope TypeEnv bindings when the SymbolTable lacks a return type // for a cross-file callee (e.g. var x = getUser() → x: User). - // The accumulator is populated but not yet finalized at this seam — all - // worker-path appendFile calls complete in the chunk loop above, so every - // file-scope binding is available here via fileScopeEntries(). + // + // Lifecycle ordering: the accumulator is populated but NOT yet finalized + // at this seam. finalize() is called later (after the sequential-path + // processCalls which also appends via typeEnv.flush()). Moving finalize() + // before this call would break sequential-path repos. Pre-finalize reads + // are safe because finalize() is a write-lock-only operation with no side + // effects on stored data. All worker-path appendFile calls complete in the + // chunk loop above, so every worker-contributed binding is available via + // fileScopeGet(). bindingAccumulator, ); } @@ -1016,6 +1022,7 @@ async function runChunkedParseAndResolve( deferredAssignments, ctx, deferredConstructorBindings.length > 0 ? deferredConstructorBindings : undefined, + bindingAccumulator, // Phase 9 fallback parity with processCallsFromExtracted (R3) ); } } finally { diff --git a/gitnexus/test/integration/cross-file-binding.test.ts b/gitnexus/test/integration/cross-file-binding.test.ts index 75cea81f6..7e1d3b656 100644 --- a/gitnexus/test/integration/cross-file-binding.test.ts +++ b/gitnexus/test/integration/cross-file-binding.test.ts @@ -380,3 +380,140 @@ describe('Phase 9 — Cross-File Call-Result Binding: Rust', () => { expect(saveCall).toBeDefined(); }); }); + +// ── R5: Missing language coverage (PR #763 review finding #5) ──────────── + +describe('Phase 9 — Cross-File Call-Result Binding: JavaScript', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(CROSS_FILE_FIXTURES, 'js-cross-file'), () => {}); + }, 60000); + + it('detects User class with save and getName methods', () => { + expect(getNodesByLabel(result, 'Class')).toContain('User'); + expect(getNodesByLabel(result, 'Method')).toContain('save'); + expect(getNodesByLabel(result, 'Method')).toContain('getName'); + }); + + it('detects getUser factory and run function', () => { + expect(getNodesByLabel(result, 'Function')).toContain('getUser'); + expect(getNodesByLabel(result, 'Function')).toContain('run'); + }); + + it('resolves u.save() in run() to User#save via cross-file return type', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find( + (c) => c.target === 'save' && c.source === 'run' && c.targetFilePath.includes('models'), + ); + expect(saveCall).toBeDefined(); + }); +}); + +describe('Phase 9 — Cross-File Call-Result Binding: C++', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(CROSS_FILE_FIXTURES, 'cpp-cross-file'), () => {}); + }, 60000); + + it('detects User class with save and get_name methods', () => { + expect(getNodesByLabel(result, 'Class')).toContain('User'); + expect(getNodesByLabel(result, 'Method')).toContain('save'); + expect(getNodesByLabel(result, 'Method')).toContain('get_name'); + }); + + it('detects get_user factory function', () => { + expect(getNodesByLabel(result, 'Function')).toContain('get_user'); + }); + + it('resolves user.save() in process() to User#save via cross-file return type', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find( + (c) => c.target === 'save' && c.source === 'process' && c.targetFilePath.includes('user'), + ); + expect(saveCall).toBeDefined(); + }); +}); + +describe('Phase 9 — Cross-File Call-Result Binding: C#', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(CROSS_FILE_FIXTURES, 'csharp-cross-file'), + () => {}, + ); + }, 60000); + + it('detects User class with Save and GetName methods', () => { + expect(getNodesByLabel(result, 'Class')).toContain('User'); + expect(getNodesByLabel(result, 'Method')).toContain('Save'); + expect(getNodesByLabel(result, 'Method')).toContain('GetName'); + }); + + it('detects GetUser factory and Run method', () => { + expect(getNodesByLabel(result, 'Method')).toContain('GetUser'); + expect(getNodesByLabel(result, 'Method')).toContain('Run'); + }); + + it('resolves u.Save() in Run() to User#Save via cross-file return type', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find( + (c) => c.target === 'Save' && c.source === 'Run' && c.targetFilePath.includes('User'), + ); + expect(saveCall).toBeDefined(); + }); +}); + +describe('Phase 9 — Cross-File Call-Result Binding: PHP', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(CROSS_FILE_FIXTURES, 'php-cross-file'), () => {}); + }, 60000); + + it('detects User class with save and getName methods', () => { + expect(getNodesByLabel(result, 'Class')).toContain('User'); + expect(getNodesByLabel(result, 'Method')).toContain('save'); + expect(getNodesByLabel(result, 'Method')).toContain('getName'); + }); + + it('detects getUser factory function', () => { + expect(getNodesByLabel(result, 'Function')).toContain('getUser'); + }); + + it('resolves $u->save() in run() to User#save via cross-file return type', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find( + (c) => c.target === 'save' && c.source === 'run' && c.targetFilePath.includes('User'), + ); + expect(saveCall).toBeDefined(); + }); +}); + +describe('Phase 9 — Cross-File Call-Result Binding: Ruby', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(CROSS_FILE_FIXTURES, 'rb-cross-file'), () => {}); + }, 60000); + + it('detects User class with save and get_name methods', () => { + expect(getNodesByLabel(result, 'Class')).toContain('User'); + expect(getNodesByLabel(result, 'Method')).toContain('save'); + expect(getNodesByLabel(result, 'Method')).toContain('get_name'); + }); + + it('detects get_user factory method', () => { + expect(getNodesByLabel(result, 'Method')).toContain('get_user'); + }); + + it('resolves user.save in process() to User#save via cross-file return type', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find( + (c) => c.target === 'save' && c.source === 'process' && c.targetFilePath.includes('models'), + ); + expect(saveCall).toBeDefined(); + }); +}); diff --git a/gitnexus/test/unit/binding-accumulator.test.ts b/gitnexus/test/unit/binding-accumulator.test.ts index be394d987..95b1ddef8 100644 --- a/gitnexus/test/unit/binding-accumulator.test.ts +++ b/gitnexus/test/unit/binding-accumulator.test.ts @@ -511,6 +511,54 @@ describe('BindingAccumulator', () => { // state. Idempotent and orthogonal to finalize(). // ------------------------------------------------------------------------- + describe('fileScopeGet (O(1) point lookup)', () => { + it('returns the typeName for a known file-scope binding', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/api.ts', [ + { scope: '', varName: 'getUser', typeName: 'User' }, + { scope: '', varName: 'getPost', typeName: 'Post' }, + ]); + expect(acc.fileScopeGet('src/api.ts', 'getUser')).toBe('User'); + expect(acc.fileScopeGet('src/api.ts', 'getPost')).toBe('Post'); + }); + + it('returns undefined for an unknown file', () => { + const acc = new BindingAccumulator(); + expect(acc.fileScopeGet('nonexistent.ts', 'x')).toBeUndefined(); + }); + + it('returns undefined for an unknown name in a known file', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/api.ts', [{ scope: '', varName: 'getUser', typeName: 'User' }]); + expect(acc.fileScopeGet('src/api.ts', 'missing')).toBeUndefined(); + }); + + it('ignores function-scope entries', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/service.ts', [ + { scope: 'handler@10', varName: 'localDb', typeName: 'Database' }, + { scope: '', varName: 'config', typeName: 'Config' }, + ]); + // Only file-scope entries are indexed by fileScopeGet. + expect(acc.fileScopeGet('src/service.ts', 'config')).toBe('Config'); + expect(acc.fileScopeGet('src/service.ts', 'localDb')).toBeUndefined(); + }); + + it('returns undefined after dispose', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/api.ts', [{ scope: '', varName: 'getUser', typeName: 'User' }]); + acc.dispose(); + expect(acc.fileScopeGet('src/api.ts', 'getUser')).toBeUndefined(); + }); + + it('last-write-wins for duplicate varNames in the same file', () => { + const acc = new BindingAccumulator(); + acc.appendFile('src/api.ts', [{ scope: '', varName: 'getUser', typeName: 'OldType' }]); + acc.appendFile('src/api.ts', [{ scope: '', varName: 'getUser', typeName: 'NewType' }]); + expect(acc.fileScopeGet('src/api.ts', 'getUser')).toBe('NewType'); + }); + }); + describe('dispose', () => { it('empties all read methods after dispose', () => { const acc = new BindingAccumulator();