diff --git a/gitnexus/src/core/graph/graph.ts b/gitnexus/src/core/graph/graph.ts index 2f49e9f4a..1dbe701b9 100644 --- a/gitnexus/src/core/graph/graph.ts +++ b/gitnexus/src/core/graph/graph.ts @@ -17,6 +17,24 @@ export const createKnowledgeGraph = (): KnowledgeGraph => { // docs/plans/2026-04-20-002-perf-parse-heritage-mro-plan.md (Unit 1). const relationshipsByType = new Map>(); + // Private helpers that encode the dual-index invariant in one place. + // All mutation paths (addRelationship, removeRelationship, removeNode) + // go through these — adding a new mutation method only needs to call + // writeRel / deleteRel, not remember to touch both maps. + const writeRel = (rel: GraphRelationship): void => { + relationshipMap.set(rel.id, rel); + let bucket = relationshipsByType.get(rel.type); + if (bucket === undefined) { + bucket = new Map(); + relationshipsByType.set(rel.type, bucket); + } + bucket.set(rel.id, rel); + }; + const deleteRel = (rel: GraphRelationship): void => { + relationshipMap.delete(rel.id); + relationshipsByType.get(rel.type)?.delete(rel.id); + }; + const addNode = (node: GraphNode) => { if (!nodeMap.has(node.id)) { nodeMap.set(node.id, node); @@ -25,13 +43,7 @@ export const createKnowledgeGraph = (): KnowledgeGraph => { const addRelationship = (relationship: GraphRelationship) => { if (relationshipMap.has(relationship.id)) return; - relationshipMap.set(relationship.id, relationship); - let bucket = relationshipsByType.get(relationship.type); - if (bucket === undefined) { - bucket = new Map(); - relationshipsByType.set(relationship.type, bucket); - } - bucket.set(relationship.id, relationship); + writeRel(relationship); }; /** @@ -42,12 +54,9 @@ export const createKnowledgeGraph = (): KnowledgeGraph => { nodeMap.delete(nodeId); - // Remove all relationships involving this node — clean up both - // indexes in lockstep so the per-type buckets never drift. - for (const [relId, rel] of relationshipMap) { + for (const rel of relationshipMap.values()) { if (rel.sourceId === nodeId || rel.targetId === nodeId) { - relationshipMap.delete(relId); - relationshipsByType.get(rel.type)?.delete(relId); + deleteRel(rel); } } return true; @@ -60,8 +69,7 @@ export const createKnowledgeGraph = (): KnowledgeGraph => { const removeRelationship = (relationshipId: string): boolean => { const rel = relationshipMap.get(relationshipId); if (rel === undefined) return false; - relationshipMap.delete(relationshipId); - relationshipsByType.get(rel.type)?.delete(relationshipId); + deleteRel(rel); return true; }; diff --git a/gitnexus/src/core/ingestion/ast-cache.ts b/gitnexus/src/core/ingestion/ast-cache.ts index 2dc637f91..65da46ab8 100644 --- a/gitnexus/src/core/ingestion/ast-cache.ts +++ b/gitnexus/src/core/ingestion/ast-cache.ts @@ -1,8 +1,24 @@ import { LRUCache } from 'lru-cache'; import Parser from 'tree-sitter'; +/** + * Minimal structural shape consumers need when reading Trees back + * through a phase-dependency boundary. Declared here so phases that + * receive ASTCache via `getPhaseOutput<...>` don't hand-roll their + * own inline structural types that silently drift when ASTCache's + * contract changes. + * + * Typed as `unknown` at the Tree boundary because consumers on the + * other side of the phase-output map don't share tree-sitter's type + * graph (e.g. COBOL's standalone processor). + */ +export interface ASTCacheReader { + get(filePath: string): unknown; + clear(): void; +} + // Define the interface for the Cache -export interface ASTCache { +export interface ASTCache extends ASTCacheReader { get: (filePath: string) => Parser.Tree | undefined; set: (filePath: string, tree: Parser.Tree) => void; clear: () => void; @@ -17,8 +33,20 @@ export const createASTCache = (maxSize: number = 50): ASTCache => { max: effectiveMax, dispose: (tree) => { try { - // NOTE: web-tree-sitter has tree.delete(); native tree-sitter trees are GC-managed. - // Keep this try/catch so we don't crash on either runtime. + // NOTE: web-tree-sitter has tree.delete(); native tree-sitter + // trees are GC-managed and .delete is absent (no-op here). + // + // Single-owner invariant (load-bearing under WASM): a given + // Parser.Tree reference must live in AT MOST ONE ASTCache + // that disposes. The parse-phase chunk-local cache clears + // between chunks; the cross-phase `scopeTreeCache` (also an + // ASTCache today) holds the same Tree by reference. Under + // native tree-sitter this is benign (dispose is a no-op). + // If/when GitNexus adopts web-tree-sitter for sequential + // parsing, the cross-phase cache must either (a) skip + // writing Trees that are already owned by a disposing cache, + // or (b) use tree.copy() per entry. Failing to pick one + // will hand freed memory to scope-resolution. (tree as unknown as { delete?: () => void }).delete?.(); } catch (e) { console.warn('Failed to delete tree from WASM memory', e); diff --git a/gitnexus/src/core/ingestion/languages/python/cache-stats.ts b/gitnexus/src/core/ingestion/languages/python/cache-stats.ts new file mode 100644 index 000000000..e423a7dba --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/python/cache-stats.ts @@ -0,0 +1,32 @@ +/** + * Dev-mode counters for the cross-phase scope-captures parse cache. + * + * Gated by `PROF_SCOPE_RESOLUTION=1`. In production the module-level + * `PROF` constant is `false` and V8 folds every increment site into + * dead code, so the hot path in `captures.ts` stays branch-free. + * + * Extracted from `captures.ts` so the production hot-path module + * doesn't carry a module-global counter and its reset/export surface. + */ + +export const PROF = process.env.PROF_SCOPE_RESOLUTION === '1'; + +let CACHE_HITS = 0; +let CACHE_MISSES = 0; + +export function recordCacheHit(): void { + if (PROF) CACHE_HITS++; +} + +export function recordCacheMiss(): void { + if (PROF) CACHE_MISSES++; +} + +export function getPythonCaptureCacheStats(): { hits: number; misses: number } { + return { hits: CACHE_HITS, misses: CACHE_MISSES }; +} + +export function resetPythonCaptureCacheStats(): void { + CACHE_HITS = 0; + CACHE_MISSES = 0; +} diff --git a/gitnexus/src/core/ingestion/languages/python/captures.ts b/gitnexus/src/core/ingestion/languages/python/captures.ts index f07befed3..e26ab2d70 100644 --- a/gitnexus/src/core/ingestion/languages/python/captures.ts +++ b/gitnexus/src/core/ingestion/languages/python/captures.ts @@ -22,21 +22,7 @@ import { splitImportStatement } from './import-decomposer.js'; import { getPythonParser, getPythonScopeQuery } from './query.js'; import { synthesizeReceiverTypeBinding } from './receiver-binding.js'; import { computePythonArityMetadata } from './arity-metadata.js'; - -// Dev-mode counters for the parse-cache hit-rate. Gated by -// `PROF_SCOPE_RESOLUTION=1` to keep the hot path branch-free in -// production. Surfaced via `getPythonCaptureCacheStats()` so -// benchmarks / debug scripts can verify the cache is being used. -const PROF = process.env.PROF_SCOPE_RESOLUTION === '1'; -let CACHE_HITS = 0; -let CACHE_MISSES = 0; -export function getPythonCaptureCacheStats(): { hits: number; misses: number } { - return { hits: CACHE_HITS, misses: CACHE_MISSES }; -} -export function resetPythonCaptureCacheStats(): void { - CACHE_HITS = 0; - CACHE_MISSES = 0; -} +import { recordCacheHit, recordCacheMiss } from './cache-stats.js'; export function emitPythonScopeCaptures( sourceText: string, @@ -51,8 +37,10 @@ export function emitPythonScopeCaptures( let tree = cachedTree as ReturnType['parse']> | undefined; if (tree === undefined) { tree = getPythonParser().parse(sourceText); - if (PROF) CACHE_MISSES++; - } else if (PROF) CACHE_HITS++; + recordCacheMiss(); + } else { + recordCacheHit(); + } const rawMatches = getPythonScopeQuery().matches(tree.rootNode); const out: CaptureMatch[] = []; diff --git a/gitnexus/src/core/ingestion/languages/python/index.ts b/gitnexus/src/core/ingestion/languages/python/index.ts index 9e6c13de8..102295eff 100644 --- a/gitnexus/src/core/ingestion/languages/python/index.ts +++ b/gitnexus/src/core/ingestion/languages/python/index.ts @@ -22,6 +22,7 @@ export { PYTHON_SCOPE_QUERY } from './query.js'; export { emitPythonScopeCaptures } from './captures.js'; +export { getPythonCaptureCacheStats, resetPythonCaptureCacheStats } from './cache-stats.js'; export { interpretPythonImport, interpretPythonTypeBinding } from './interpret.js'; export { pythonMergeBindings } from './merge-bindings.js'; export { pythonArityCompatibility } from './arity.js'; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index dd5cc3117..025bdbeb7 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -109,13 +109,15 @@ export async function runChunkedParseAndResolve( bindingAccumulator: BindingAccumulator; resolutionContext: ReturnType; usedWorkerPool: boolean; - /** AST cache populated by the sequential parse path. Empty when + /** Cross-phase tree-sitter Tree cache populated by the sequential + * parse path. Distinct from the chunk-local `astCache` used inside + * the parse loop (that one is cleared between chunks). Empty when * every chunk ran via the worker pool (workers can't return native * tree-sitter Trees across the MessageChannel). Downstream phases - * (e.g. scope-resolution) read from this to skip re-parsing the - * same source. See plan + * (scope-resolution) read from this to skip re-parsing the same + * source. See plan * docs/plans/2026-04-20-002-perf-parse-heritage-mro-plan.md (Unit 4). */ - astCache: ASTCache; + scopeTreeCache: ASTCache; }> { const ctx = createResolutionContext(); const symbolTable = ctx.model.symbols; @@ -617,6 +619,6 @@ export async function runChunkedParseAndResolve( // sequential path already parsed. Survives chunk boundaries; the // chunk-local `astCache` above is intentionally NOT exposed // because parse-impl clears it between chunks. - astCache: scopeTreeCache, + scopeTreeCache, }; } diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts index 0f5314efa..a20d1e4b0 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts @@ -65,14 +65,22 @@ export interface ParseOutput { */ readonly usedWorkerPool: boolean; /** - * AST cache populated by the sequential parse path. Empty entries - * for files that ran through the worker pool (workers can't return - * native tree-sitter Trees across the MessageChannel). Downstream - * phases (scope-resolution) read from this to skip re-parsing — - * cache miss is safe and falls back to a fresh parse. See plan + * Cross-phase tree-sitter Tree cache populated by the sequential + * parse path. Separate from the chunk-local `astCache` used *inside* + * the parse phase (which is cleared between chunks) — this one + * survives the whole phase and hands Trees to scope-resolution so + * it can skip a second parse. + * + * Empty entries for files that ran through the worker pool + * (workers can't return native tree-sitter Trees across the + * MessageChannel). Cache miss is safe — consumers fall back to a + * fresh parse. See plan * docs/plans/2026-04-20-002-perf-parse-heritage-mro-plan.md (Unit 4). + * + * Disposed by `scopeResolutionPhase` (the sole consumer) via + * `scopeTreeCache.clear()` after its extract loop finishes. */ - readonly astCache: ASTCache; + readonly scopeTreeCache: ASTCache; } export const parsePhase: PipelinePhase = { diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts index 9cbbcae1d..3129cf236 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts @@ -36,6 +36,7 @@ import { readFileContents } from '../../filesystem-walker.js'; import { runScopeResolution } from './run.js'; import { SCOPE_RESOLVERS } from './registry.js'; import { isDev } from '../../utils/env.js'; +import type { ASTCacheReader } from '../../ast-cache.js'; export interface ScopeResolutionOutput { /** True when at least one language ran. */ @@ -82,9 +83,7 @@ export const scopeResolutionPhase: PipelinePhase = { // skip a second tree-sitter parse. Cache miss is safe (re-parses). // Worker-mode parses leave the cache empty for those files; they // also fall back to a fresh parse — no correctness impact. - const { astCache } = getPhaseOutput<{ - astCache: { get(path: string): unknown; clear(): void }; - }>(deps, 'parse'); + const { scopeTreeCache } = getPhaseOutput<{ scopeTreeCache: ASTCacheReader }>(deps, 'parse'); let totalFiles = 0; let totalImports = 0; @@ -117,7 +116,7 @@ export const scopeResolutionPhase: PipelinePhase = { { graph: ctx.graph, files, - treeCache: astCache, + treeCache: scopeTreeCache, onWarn: (msg) => { if (isDev) console.warn(`[scope-resolution:${lang}] ${msg}`); }, @@ -148,7 +147,7 @@ export const scopeResolutionPhase: PipelinePhase = { // never read them, and tree-sitter Trees hold native-heap memory // under WASM runtimes. ASTCache.clear() fires the LRU dispose // handler which calls tree.delete?.() on each retained Tree. - astCache.clear(); + scopeTreeCache.clear(); if (!anyRan) return NOOP_OUTPUT; diff --git a/gitnexus/test/unit/mro-processor.test.ts b/gitnexus/test/unit/mro-processor.test.ts index 465face1d..d313fb2e5 100644 --- a/gitnexus/test/unit/mro-processor.test.ts +++ b/gitnexus/test/unit/mro-processor.test.ts @@ -1740,4 +1740,58 @@ describe('computeMRO', () => { } }); }); + + // ---- PHM parent-order pinning ------------------------------------------ + // + // PHM Unit 2 split buildAdjacency's single forEachRelationship into three + // typed iterations (EXTENDS, IMPLEMENTS, HAS_METHOD). Parent enumeration + // now runs ALL EXTENDS edges before ANY IMPLEMENTS edges. For classes + // with parents added in interleaved order, this re-orders `parentMap` + // and any C3 linearization that consumes it. + // + // Python (single EXTENDS model) and Java/C# (resolveCsharpJava partitions + // by edge type regardless of order) are unaffected in practice. This + // test pins the new behavior so a future "simplification" back to a + // single loop would surface as a deliberate change rather than a silent + // semantic drift. + describe('PHM: interleaved EXTENDS + IMPLEMENTS parent ordering', () => { + it('class methods win regardless of the order EXTENDS/IMPLEMENTS edges were added', () => { + const graph = createKnowledgeGraph(); + // C extends Base (class) AND implements Iface (interface). Edges + // added in INTERLEAVED order: IMPLEMENTS first, then EXTENDS. + // Under the old single-loop adjacency, parentMap[C] would be + // [IfaceId, BaseId]. Under the new grouped adjacency, + // parentMap[C] is [BaseId, IfaceId] (EXTENDS bucket first). + // + // For resolveCsharpJava, class-method-wins is invariant to parent + // order — both produce the same winner. This test encodes that + // invariant, guarding the behavioral claim that 'Java/C# are + // unaffected' in the PHM commit message. + addClass(graph, 'Base', 'java'); + addClass(graph, 'C', 'java'); + addClass(graph, 'Iface', 'java', 'Interface'); + addMethod(graph, 'Base', 'greet'); + addMethod(graph, 'Iface', 'greet', 'Interface'); + addMethod(graph, 'C', 'greet'); + + // Add IMPLEMENTS BEFORE EXTENDS to exercise interleaving. + addImplements(graph, 'C', 'Iface'); + addExtends(graph, 'C', 'Base'); + + const result = computeMRO(graph); + const cId = generateId('Class', 'C'); + const entry = result.entries.find((e) => e.classId === cId); + expect(entry).toBeDefined(); + const mro = entry!.mro; + + // Grouped iteration yields EXTENDS parents first. This pin fails + // loudly if a future refactor reverts the typed-bucket iteration + // to a single full-graph scan and restores insertion-order + // semantics. + // Grouped EXTENDS-before-IMPLEMENTS iteration produces this exact + // MRO for C: [Base, Iface]. A single-loop reversion would yield + // [Iface, Base] (IMPLEMENTS added first in this test). + expect(mro).toEqual(['Base', 'Iface']); + }); + }); }); diff --git a/gitnexus/test/unit/scope-resolution/python/cached-tree-parity.test.ts b/gitnexus/test/unit/scope-resolution/python/cached-tree-parity.test.ts new file mode 100644 index 000000000..f5db958b7 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/python/cached-tree-parity.test.ts @@ -0,0 +1,78 @@ +/** + * Parity guard for the cross-phase tree cache (PHM Unit 5). + * + * `emitPythonScopeCaptures(src, path)` re-parses internally; + * `emitPythonScopeCaptures(src, path, cachedTree)` skips the parse. The + * two paths MUST return identical `CaptureMatch[]`. A future change + * that (a) mutates Trees before caching, (b) conditionally branches + * the capture query on cached vs fresh Trees, or (c) leaks state + * through module-level caches would break this — and no other test + * today asserts the equivalence. + * + * Keeps the wins from the cache-hit path honest. + */ +import { describe, it, expect } from 'vitest'; +import { + emitPythonScopeCaptures, + resetPythonCaptureCacheStats, + getPythonCaptureCacheStats, +} from '../../../../src/core/ingestion/languages/python/index.js'; +import { getPythonParser } from '../../../../src/core/ingestion/languages/python/query.js'; + +const FIXTURE = ` +from typing import List + +class Base: + def greet(self) -> str: + return "hi" + +class Child(Base): + def shout(self, items: List[str]) -> None: + for item in items: + print(item.upper()) + +def top(c: Child) -> None: + c.greet() + c.shout([]) +`; + +function normalizeCaptures(caps: readonly Record[]): unknown[] { + // CaptureMatch is a Record. Compare by structural JSON + // so Node references don't create false negatives. + return caps.map((m) => { + const out: Record = {}; + for (const [tag, cap] of Object.entries(m)) { + const c = cap as { range?: unknown; text?: unknown }; + out[tag] = { range: c.range, text: c.text }; + } + return out; + }); +} + +describe('emitPythonScopeCaptures cache-hit parity', () => { + it('returns identical captures whether cachedTree is supplied or not', () => { + const fresh = emitPythonScopeCaptures(FIXTURE, 'fixture.py'); + const tree = getPythonParser().parse(FIXTURE); + const cached = emitPythonScopeCaptures(FIXTURE, 'fixture.py', tree); + + expect(cached).toHaveLength(fresh.length); + expect(normalizeCaptures(cached)).toEqual(normalizeCaptures(fresh)); + }); + + it('counters stay at zero baseline after reset regardless of whether PROF is active', () => { + // The PROF gate is evaluated at module load, so we can't toggle + // counters on mid-test. What we CAN assert deterministically is + // that reset zeros the counters and repeated reads yield the same + // zeroed snapshot (counter API shape invariant). + resetPythonCaptureCacheStats(); + expect(getPythonCaptureCacheStats()).toEqual({ hits: 0, misses: 0 }); + // Running the emit path should not mutate the counters unless PROF + // was on at module load. Whichever state, calling reset again must + // return to zero. + const tree = getPythonParser().parse(FIXTURE); + emitPythonScopeCaptures(FIXTURE, 'fixture.py', tree); + emitPythonScopeCaptures(FIXTURE, 'fixture.py'); + resetPythonCaptureCacheStats(); + expect(getPythonCaptureCacheStats()).toEqual({ hits: 0, misses: 0 }); + }); +});