diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index c715f59b7..f7b4be0d9 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -246,5 +246,10 @@ "_rebaselined_1432_member_call_callee_name": "#1432 (Zig): the shared callable-flow reader no longer names a callee by simple name for a MEMBER call (`@callable-flow.direct-callee-name` requires a direct designator: `f(x)`, `ns.f(x)`), and a member call is a field-stored-callable invoke only when a MEMBER store (`o.f = handler`) or a declared callable-typed field is visible - a same-named plain binding no longer gates it. CAPTURE-EMISSION CHANGE, not fixture growth (fixture_count unchanged). Only drift: `users.map { it.name }.forEach { name -> println(name) }` (kotlin-lambda-scopes/App.kt) loses `direct-callee-name|forEach` (member call). capture_groups_fp 2334 (unchanged). Prior a184f8ff0ae40d246db855b63f7ff26bda3afac03e5f4c76e4593c7e2cefce54 -> 5a181af0dbc9451937da0964c40d3f3f9820914ca429d873bb5c812b5e2b9284.", "_rebaselined_1432_rebase_onto_2960": "#1432 rebase onto main @ aac7515d: the kotlin fingerprint is a COMBINATION of two independent changes, so neither side of the merge conflict was correct on its own and resolving it by picking a side would have committed a fingerprint no run can reproduce. main's #2960 added four declared-package fixture files (fixture_count 137 -> 141, capture_groups_fp 2334 -> 2367); this branch's `_rebaselined_1432_member_call_callee_name` drops `direct-callee-name|forEach` from one member call. Recomputed under both: capture_groups_fp 2367 and fixture_count 141 match main's committed counts EXACTLY (this branch's change is emission-only and moves no count), capture_groups_small/large stay 4753/15203 (the SYNTHETIC scaling source, which neither change touches), scaling 1.003 < 1.5, and the other 14 languages report ok against their committed baselines in the same run - which is the check that the rebase replayed nothing else into the capture stream. Attribution is exact rather than inferred: moving test/fixtures/lang-resolution/kotlin-import-package-evidence aside and re-running returns kotlin to 5a181af0dbc9451937da0964c40d3f3f9820914ca429d873bb5c812b5e2b9284 byte-for-byte with fixture_count back at 137 and capture_groups_fp back at 2334 - this branch's pre-rebase value - so the whole delta is #2960's corpus growth layered on top, with nothing else moving. Prior (this branch, pre-rebase) 5a181af0dbc9451937da0964c40d3f3f9820914ca429d873bb5c812b5e2b9284 and (main) f98e7e936afbce0e99588285cfc603bf945fd58c5de45271860509a5d90eb832 -> 973d702510002dda76166e017c5eca90cae37a139f5a512877b7a1b04ad19dc5.", "_rebaselined_1432_merge_main_2885": "#1432 merge of main @ 212e007a: the kotlin fingerprint is again a COMBINATION of two independent changes \u2014 main's #2885 JVM property accessors / interface-abstract (4753/15203 -> 5753/18403, capture_groups_fp 2367 -> 2563) and this branch's `_rebaselined_1432_member_call_callee_name` (drops `direct-callee-name|forEach` from one member call). Neither side's value reproduces under the merged tree. Recomputed under both: counts 5753/18403/2563 and fixture_count 141 match main's committed counts EXACTLY (this branch's change is emission-only), scaling 1.061 < 1.5, and csharp / cpp / typescript measure byte-for-byte at this branch's committed values (main did not touch them since the merge-base) while the other 11 languages report ok \u2014 the check that the merge replayed nothing else into the capture stream. Prior (main) aeafc7a87402c933786ef582b7c98683b1822b78fa909e605cb97552867fa0d5 and (this branch) 973d702510002dda76166e017c5eca90cae37a139f5a512877b7a1b04ad19dc5 -> a9d3f0db7547ff47856159debf15d2a6f427efca97a6af27b4004174ed432132." + }, + "typescript-deep-chain": { + "fingerprint": "c4dd97ad7be50a554930237f79035993ffa67957de6dc8c1d6606518071204b2", + "scaling_budget": 1.5, + "_added": "#3354 review: one TS `handler || w === \"k0\" || \u2026` chain, operand count = entity count (250 -> 800). Guards callable-flow value-alternative expansion (valueAlternatives + per-leaf visibility walks) against per-leaf cost that grows with chain depth: before the fix this measured 7.5-8.0 (super-linear; 8000 operands overflowed the stack), after it 1.11-1.22. Synthetic-only fingerprint (no fixture corpus); identical before and after the fix." } } diff --git a/gitnexus/bench/scope-capture/measure.mjs b/gitnexus/bench/scope-capture/measure.mjs index 7a4e4057b..faf60e357 100644 --- a/gitnexus/bench/scope-capture/measure.mjs +++ b/gitnexus/bench/scope-capture/measure.mjs @@ -309,6 +309,21 @@ const LANGS = [ ` getId(): number { return this.id; }\n` + ` setName(v: string): void { this.name = v; }\n}\n\n`, }, + { + // One `a || b || …` chain whose operand count is the entity count (#3354 + // review): each operand is one more nesting level, so this guards the + // callable-flow value-alternative expansion against per-leaf work that + // grows with chain depth (ratio ~8 before the fix, and 8000 operands + // overflowed the stack). No fixture corpus — the fingerprint is the synthetic + // source alone; `typescript` above already covers the TS fixtures. + name: 'typescript-deep-chain', + emit: emitTsScopeCaptures, + exts: ['.ts'], + file: 'bench-chain.ts', + header: 'function handler() {}\n\nexport function isKw(w: string) {\n const f = handler', + unit: (n) => ` || w === "k${n}"`, + footer: ';\n return f;\n}\n', + }, { name: 'javascript', emit: emitJsScopeCaptures, @@ -371,7 +386,9 @@ function measureLang(lang) { // Correctness fingerprint over the fixture corpus + a fixed 20-entity source. const perFixture = []; let groups = 0; - for (const { key, absPath } of collectFixtures(lang.fixturePrefix, lang.exts)) { + const fixtures = + lang.fixturePrefix === undefined ? [] : collectFixtures(lang.fixturePrefix, lang.exts); + for (const { key, absPath } of fixtures) { const matches = lang.emit(fs.readFileSync(absPath, 'utf8'), absPath); groups += matches.length; perFixture.push(`${key}\t${matches.length}\t${digestCaptures(matches)}`); diff --git a/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts b/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts index 5de2f9bd6..531d6bca4 100644 --- a/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts +++ b/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts @@ -215,6 +215,25 @@ interface ValueBindingIndex { string, ReadonlyMap >; + /** + * Every node id a visibility walk can stop at: the region ids of the three + * maps above plus the formal owners. Any other ancestor fails every check, + * so the walks jump from anchor to anchor instead of visiting it. + */ + readonly anchorIds: ReadonlySet; + /** + * Nearest anchor at-or-above a node, memoized for the whole file. Each + * alternative of a long `a || b || …` chain starts its walk at a leaf as + * deep as the chain is long; without the memo every leaf re-walked the same + * spine to the root, so the chain cost grew quadratically in its length. + */ + readonly nearestAnchorById: Map; + /** + * Parent of every named node, recorded by the one DFS. tree-sitter's + * `parent` is not a pointer read: it re-descends from the root, so it costs + * the node's depth, and a leaf of a long chain is as deep as the chain. + */ + readonly parentById: ReadonlyMap; } /** @@ -223,16 +242,21 @@ interface ValueBindingIndex { * One explicit DFS supplies all phases below. Query-backed emitters may still * perform their existing query walk; this helper never reparses and remains * linear in AST size (the scope-capture benchmark guards the scaling ratio). + * That includes a value-selecting source: `valueAlternatives` expands a + * chain iteratively into disjoint leaves, and each leaf's visibility walk + * reuses the memoized anchor spine instead of re-walking to the root (the + * `typescript-deep-chain` benchmark case guards that one). */ export function synthesizeCallableFlowCaptures( root: SyntaxNode, options: CallableFlowCaptureOptions, ): readonly CaptureMatch[] { - const nodes = collectNodes(root); + const parentById = new Map(); + const nodes = collectNodes(root, parentById); const functions = collectFunctions(nodes, options); const knownCallableNames = new Set(functions.map((fn) => fn.name)); const assignments = collectAssignments(nodes, options); - const valueBindings = buildValueBindingIndex(nodes, assignments, functions, options); + const valueBindings = buildValueBindingIndex(nodes, parentById, assignments, functions, options); const out: CaptureMatch[] = []; for (const assignment of assignments) { @@ -260,7 +284,9 @@ export function synthesizeCallableFlowCaptures( return out; } -function collectNodes(root: SyntaxNode): SyntaxNode[] { +/** Every named node in document order; also records each one's parent into + * `parentById` (see `ValueBindingIndex.parentById`). */ +function collectNodes(root: SyntaxNode, parentById: Map): SyntaxNode[] { const out: SyntaxNode[] = []; const stack: SyntaxNode[] = [root]; while (stack.length > 0) { @@ -269,7 +295,9 @@ function collectNodes(root: SyntaxNode): SyntaxNode[] { const children = node.namedChildren; for (let i = children.length - 1; i >= 0; i--) { const child = children[i]; - if (child !== null) stack.push(child); + if (child === null) continue; + parentById.set(child.id, node); + stack.push(child); } } return out; @@ -353,6 +381,7 @@ function collectAssignments( function buildValueBindingIndex( nodes: readonly SyntaxNode[], + parentById: ReadonlyMap, assignments: readonly AssignmentParts[], functions: readonly FunctionInfo[], options: CallableFlowCaptureOptions, @@ -440,14 +469,56 @@ function buildValueBindingIndex( if (functionOwner !== undefined) byRegion.set(functionOwner.id, signature); } } + const anchorIds = new Set(); + for (const index of [assignmentRegionIdsByName, memberStoreRegionIdsByName]) { + for (const regionIds of index.values()) for (const id of regionIds) anchorIds.add(id); + } + for (const byRegion of signatureByNameAndRegion.values()) { + for (const id of byRegion.keys()) anchorIds.add(id); + } + for (const owner of formalByOwner.keys()) if (owner !== undefined) anchorIds.add(owner); return { assignmentRegionIdsByName, memberStoreRegionIdsByName, formalByOwner, signatureByNameAndRegion, + anchorIds, + parentById, + nearestAnchorById: new Map(), }; } +/** A node the DFS did not reach (none in practice) falls back to tree-sitter. */ +function parentOf(node: SyntaxNode, bindings: ValueBindingIndex): SyntaxNode | null { + return bindings.parentById.get(node.id) ?? node.parent; +} + +/** + * The nearest anchor at-or-above `start` (see `ValueBindingIndex.anchorIds`), + * or null when none is. Every node visited on the way is memoized, so across + * one file each node's `parent` is taken at most once by these walks. + */ +function nearestAnchor(start: SyntaxNode | null, bindings: ValueBindingIndex): SyntaxNode | null { + const visited: number[] = []; + let node = start; + let found: SyntaxNode | null = null; + while (node !== null) { + const cached = bindings.nearestAnchorById.get(node.id); + if (cached !== undefined) { + found = cached; + break; + } + visited.push(node.id); + if (bindings.anchorIds.has(node.id)) { + found = node; + break; + } + node = parentOf(node, bindings); + } + for (const id of visited) bindings.nearestAnchorById.set(id, found); + return found; +} + /** True when a pointer/parenthesized declarator sits between the declaration * and its binding identifier — the shape of a callable-typed variable, never * of a plain prototype. Only C/C++ supply signature-declaration node types, @@ -509,8 +580,11 @@ function isVisibleValueBinding( ) { return true; } - let node: SyntaxNode | null = input; - while (node !== null) { + for ( + let node = nearestAnchor(input, bindings); + node !== null; + node = nearestAnchor(parentOf(node, bindings), bindings) + ) { if (assignmentRegionIds?.has(node.id) === true) return true; if ( options.functionNodeTypes.has(node.type) && @@ -518,7 +592,6 @@ function isVisibleValueBinding( ) { return true; } - node = node.parent; } if (bindings.formalByOwner.get(undefined)?.has(name) === true) return true; // A declared callable-typed binding (file-scope `void (*fp)(int);`) is a @@ -548,10 +621,12 @@ function isVisibleMemberStore( if (regionIds !== undefined) { const providerOwner = options.lexicalFunctionOwner?.(input); if (providerOwner !== undefined && regionIds.has(providerOwner.id)) return true; - let node: SyntaxNode | null = input; - while (node !== null) { + for ( + let node = nearestAnchor(input, bindings); + node !== null; + node = nearestAnchor(parentOf(node, bindings), bindings) + ) { if (regionIds.has(node.id)) return true; - node = node.parent; } } return visibleCallableSignature(input, name, bindings, options) !== undefined; @@ -570,11 +645,13 @@ function visibleCallableSignature( const signature = byRegion.get(providerOwner.id); if (signature !== undefined) return signature; } - let node: SyntaxNode | null = input; - while (node !== null) { + for ( + let node = nearestAnchor(input, bindings); + node !== null; + node = nearestAnchor(parentOf(node, bindings), bindings) + ) { const signature = byRegion.get(node.id); if (signature !== undefined) return signature; - node = node.parent; } return undefined; } @@ -626,28 +703,58 @@ const VALUE_SELECTING_OPERATORS = new Set(['??', '||', 'or']); * `options.valueAlternatives` is consulted first for grammars whose shape the * field-based rule below cannot see. */ -function valueAlternatives(node: SyntaxNode, options: CallableFlowCaptureOptions): SyntaxNode[] { +function valueAlternatives( + node: SyntaxNode, + options: CallableFlowCaptureOptions, +): readonly SyntaxNode[] { + // Explicit stack, not recursion: `a || b || …` nests one level per operand, + // so a generated keyword table thousands of operands long overflowed the + // call stack (and re-copied every partial result at each level). + const out: SyntaxNode[] = []; + const pending: SyntaxNode[] = [node]; + for (let current = pending.pop(); current !== undefined; current = pending.pop()) { + const branches = valueBranches(current, options); + if (branches === undefined) { + out.push(current); + continue; + } + // Reverse push keeps the left-to-right branch order on output. + for (let i = branches.length - 1; i >= 0; i--) { + const branch = branches[i]; + if (branch !== undefined) pending.push(branch); + } + } + return out; +} + +/** One level of `valueAlternatives`: the branches `node` selects between, or + * undefined when it is opaque (its own single alternative). */ +function valueBranches( + node: SyntaxNode, + options: CallableFlowCaptureOptions, +): readonly SyntaxNode[] | undefined { let inner = node; while (inner.type.includes('parenthesized') && inner.namedChildCount === 1) { - inner = inner.namedChild(0)!; + const child = inner.namedChild(0); + if (child === null) break; + inner = child; } const provided = options.valueAlternatives?.(inner); if (provided !== undefined) { - if (provided.length === 1 && provided[0]?.id === inner.id) return [node]; - return provided.flatMap((branch) => valueAlternatives(branch, options)); + return provided.length === 1 && provided[0]?.id === inner.id ? undefined : provided; } const consequence = inner.childForFieldName('consequence'); const alternative = inner.childForFieldName('alternative'); if (consequence !== null && alternative !== null && inner.childForFieldName('condition')) { - return [...valueAlternatives(consequence, options), ...valueAlternatives(alternative, options)]; + return [consequence, alternative]; } const left = inner.childForFieldName('left'); const right = inner.childForFieldName('right'); const operator = inner.childForFieldName('operator')?.type; if (left !== null && right !== null && operator && VALUE_SELECTING_OPERATORS.has(operator)) { - return [...valueAlternatives(left, options), ...valueAlternatives(right, options)]; + return [left, right]; } - return [node]; + return undefined; } function emitAssignmentFact( diff --git a/gitnexus/test/unit/scope-resolution/callable-flow-captures.test.ts b/gitnexus/test/unit/scope-resolution/callable-flow-captures.test.ts index f63ea10c3..1d17e626e 100644 --- a/gitnexus/test/unit/scope-resolution/callable-flow-captures.test.ts +++ b/gitnexus/test/unit/scope-resolution/callable-flow-captures.test.ts @@ -13,6 +13,7 @@ import { describe, it, expect } from 'vitest'; import { synthesizeCallableFlowCaptures } from '../../../src/core/ingestion/utils/callable-flow-captures.js'; import { getJsParser } from '../../../src/core/ingestion/languages/javascript/query.js'; +import { getTreeSitterBufferSize } from '../../../src/core/ingestion/constants.js'; const OPTIONS = { functionNodeTypes: new Set(['function_declaration', 'arrow_function', 'function_expression']), @@ -25,7 +26,7 @@ const OPTIONS = { } as const; function factsFor(src: string): Array> { - const tree = getJsParser().parse(src); + const tree = getJsParser().parse(src, undefined, { bufferSize: getTreeSitterBufferSize(src) }); if (tree === null) throw new Error('parse failed'); return synthesizeCallableFlowCaptures(tree.rootNode, OPTIONS).map((match) => { const out: Record = {}; @@ -114,4 +115,24 @@ describe('synthesizeCallableFlowCaptures (shared synthesizer, #2522)', () => { }); expect(suppressed.filter((match) => match['@callable-flow.seed'] !== undefined)).toEqual([]); }); + + it('expands a 10000-operand `||` chain and keeps its callable and formal operands (#3354 review)', () => { + // Each `||` nests one level deeper, so the expansion used to recurse once + // per operand (stack overflow near 8000) and walk each leaf's full depth. + // The formal `w` sits at the deepest leaf: its visibility still has to be + // found through the enclosing function, 10000 levels up. + const operands = Array.from({ length: 10000 }, (_, i) => `w === "k${i}"`); + operands.splice(5000, 0, 'target'); + operands.unshift('w'); + const facts = factsFor( + `function target() {}\nfunction isKw(w) {\n const f = ${operands.join(' || ')};\n f();\n}\n`, + ); + expect(byTag(facts, '@callable-flow.seed')).toMatchObject([ + { '@callable-flow.destination': 'f', '@callable-flow.target-name': 'target' }, + ]); + expect(byTag(facts, '@callable-flow.copy')).toMatchObject([ + { '@callable-flow.destination': 'f', '@callable-flow.source': 'w' }, + ]); + expect(byTag(facts, '@callable-flow.invoke')).toMatchObject([{ '@callable-flow.callee': 'f' }]); + }); });