diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index c1a75213c..49d602f8f 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -28,6 +28,7 @@ import { semverCompare } from '../../../../infrastructure/update-check.js'; import { getOrCreatePerDbChunkStmt } from '../../../../shared/chunked-stmt-cache.js'; import { normalizePath, TS_NATIVE_CONFIDENCE_FLOOR } from '../../../../shared/constants.js'; import { toErrorMessage } from '../../../../shared/errors.js'; +import { CALLABLE_SYMBOL_KINDS } from '../../../../shared/kinds.js'; import { CODEGRAPH_VERSION } from '../../../../shared/version.js'; import type { BetterSqlite3Database, @@ -321,6 +322,7 @@ async function runPostNativeStructure( */ async function runDataflowVertexPass( ctx: PipelineContext, + isFullBuild: boolean, changedFiles: string[] | undefined, ): Promise { if (ctx.opts.dataflow === false) return; @@ -328,6 +330,19 @@ async function runDataflowVertexPass( const native = loadNative(); if (!native?.extractDataflowAnalysis) return; + // Quiet incremental: no files changed → no new dataflow edges/vertices to + // add, nothing to do. Without this, an incremental no-op rebuild falls + // through to the "full build" branch below just like a genuine full + // build would (changedFiles is `[]`, not `undefined`, but the `else` + // below treats both the same) and re-scans every eligible file for + // nothing — mirrors backfillEdgeTechniquesAfterNativeOrchestrator's + // identical guard just above (#2483 follow-up: caught by the perf-canary + // "No-op rebuild" benchmark after broadening which files that full-build + // branch considers eligible). + if (!isFullBuild && changedFiles && changedFiles.length === 0) { + return; + } + // Determine which files to process: changed files for incremental, all for full builds. let filesToProcess: string[]; if (changedFiles && changedFiles.length > 0) { @@ -338,23 +353,29 @@ async function runDataflowVertexPass( // (a) Non-native language files — NATIVE_SUPPORTED_EXTENSIONS doesn't cover them, // so extractDataflowAnalysis returns null; the wasmStubs path calls buildDataflowEdges // which writes both edges AND vertices for those files. - // (b) Native-language files with dataflow edges already written by the Rust orchestrator - // (flows_to/returns/mutates) — those need vertex rows to connect them. + // (b) Native-language files with at least one function/method definition — every + // such definition gets its own param/return/local vertices regardless of whether + // it participates in any INTER-procedural flow (#2483: a leaf function with no + // cross-function argument/assignment/mutation relationships still has params and a + // return worth recording — extractDataflowAnalysis's vertex output isn't gated on + // argFlows/assignments/mutations being non-empty). An earlier version of this filter + // scoped to files with existing `dataflow` EDGE rows instead, which wrongly skipped + // vertex extraction for every file whose functions happen to have no inter-procedural + // edges — verified empirically: a repro fixture with plain param/return-only functions + // (no cross-function dataflow at all) produced zero dataflow_vertices rows on the + // native engine while WASM correctly recorded them. // - // Skipping native-language files with no dataflow edges is safe: extractDataflowAnalysis - // would return argFlows=[], assignments=[], mutations=[] for them, producing zero vertices - // and zero inter-procedural edges. Excluding them avoids O(n_total_files) re-analysis on - // every full build (codegraph itself: ~2000 files, ~50-80% with no dataflow edges). - const filesWithDataflow = new Set( + // Excluding files with NO function/method definitions at all (pure type/data files) is + // still a safe, meaningful prune — they can never produce vertices either way — and avoids + // O(n_total_files) re-analysis on every full build. + const filesWithFunctions = new Set( ( ctx.db .prepare( - `SELECT DISTINCT n.file - FROM dataflow d - JOIN nodes n ON n.id = d.source_id - WHERE n.file IS NOT NULL`, + `SELECT DISTINCT file FROM nodes + WHERE file IS NOT NULL AND kind IN (${[...CALLABLE_SYMBOL_KINDS].map(() => '?').join(',')})`, ) - .all() as { file: string }[] + .all(...CALLABLE_SYMBOL_KINDS) as { file: string }[] ).map((r) => r.file), ); @@ -368,8 +389,8 @@ async function runDataflowVertexPass( const ext = path.extname(f).toLowerCase(); // Non-native files: always include (WASM handles them via wasmStubs path). if (!NATIVE_SUPPORTED_EXTENSIONS.has(ext)) return true; - // Native files: only include when Rust wrote dataflow edges for them. - return filesWithDataflow.has(f); + // Native files: only include when they have at least one function/method definition. + return filesWithFunctions.has(f); }); } @@ -2941,7 +2962,7 @@ export async function tryNativeOrchestrator( // Languages where Rust has no dataflow rules are silently skipped; a WASM // fallback for those is tracked in issue #1614. if (ctx.opts.dataflow !== false && !needsAnalysisFallback) { - await runDataflowVertexPass(ctx, result.changedFiles); + await runDataflowVertexPass(ctx, !!result.isFullBuild, result.changedFiles); } session.close(); diff --git a/tests/integration/issue-2483-native-dataflow-vertices-leaf-functions.test.ts b/tests/integration/issue-2483-native-dataflow-vertices-leaf-functions.test.ts new file mode 100644 index 000000000..49c145e15 --- /dev/null +++ b/tests/integration/issue-2483-native-dataflow-vertices-leaf-functions.test.ts @@ -0,0 +1,164 @@ +/** + * Integration test for #2483: the native engine's full-build dataflow-vertex + * pass (`runDataflowVertexPass` in `native-orchestrator.ts`, P6) scoped + * itself to files that already had `dataflow` EDGE rows (flows_to/returns/ + * mutates) from the Rust orchestrator, skipping vertex extraction entirely + * for every other native-language file. + * + * That scoping conflated "has inter-procedural dataflow edges" with "needs + * vertex rows at all" — a plain leaf function with params and a return, but + * no calls to or from any other function, legitimately has NO dataflow + * edges yet still has vertex-worthy params/returns + * (`extractDataflowAnalysis`'s vertex output isn't gated on argFlows/ + * assignments/mutations being non-empty). The WASM engine has always + * recorded these vertices unconditionally; the native engine silently + * dropped them for every file with zero inter-procedural edges — in + * practice, most files in a typical codebase. + * + * Fix: the full-build file-selection filter now includes any native- + * language file with at least one function/method definition (checked via + * the `nodes` table, kind IN CALLABLE_SYMBOL_KINDS), not just files that + * happen to already have `dataflow` edge rows. + * + * Follow-up: broadening that filter also broadened how many files a + * literal no-op incremental rebuild (zero files changed) re-scans, since + * `changedFiles === []` fell into the same "full build" branch as + * `changedFiles === undefined` — caught by the perf-canary "No-op rebuild" + * benchmark. Fixed by adding the same "quiet incremental: nothing changed" + * early return `backfillEdgeTechniquesAfterNativeOrchestrator` already had + * for the identical `isFullBuild=false, changedFiles=[]` case. The second + * describe block below locks in that a no-op rebuild doesn't lose the + * vertices a prior build already inserted. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const FIXTURE = { + 'leaf.js': ` +function greet(name) { + return name + '!'; +} + +function shout(word) { + return word.toUpperCase(); +} +`, +}; + +function writeFixture(rootDir: string) { + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(rootDir, rel), content); + } +} + +function readDataflowVertices(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n.name AS func_name, dv.kind, dv.name + FROM dataflow_vertices dv + JOIN nodes n ON n.id = dv.func_id + ORDER BY n.name, dv.kind, dv.name`, + ) + .all() as Array<{ func_name: string; kind: string; name: string | null }>; + } finally { + db.close(); + } +} + +describe.skipIf(!isNativeAvailable())( + 'native full-build dataflow vertices for leaf functions (#2483)', + () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-2483-native-dfv-')); + writeFixture(tmpDir); + await buildGraph(tmpDir, { + engine: 'native', + incremental: false, + dataflow: true, + skipRegistry: true, + }); + }, 60_000); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('records param and return vertices for a leaf function with no inter-procedural dataflow', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const vertices = readDataflowVertices(dbPath); + expect( + vertices.some((v) => v.func_name === 'greet' && v.kind === 'param' && v.name === 'name'), + `missing greet param vertex; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + expect( + vertices.some((v) => v.func_name === 'greet' && v.kind === 'return'), + `missing greet return vertex; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + }); + + it('records vertices for every leaf function in the file, not just the first', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const vertices = readDataflowVertices(dbPath); + expect( + vertices.some((v) => v.func_name === 'shout' && v.kind === 'param' && v.name === 'word'), + `missing shout param vertex; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + expect( + vertices.some((v) => v.func_name === 'shout' && v.kind === 'return'), + `missing shout return vertex; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + }); + }, +); + +describe.skipIf(!isNativeAvailable())( + 'native no-op incremental rebuild preserves dataflow vertices (#2483 follow-up)', + () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-2483-native-noop-')); + writeFixture(tmpDir); + await buildGraph(tmpDir, { + engine: 'native', + incremental: false, + dataflow: true, + skipRegistry: true, + }); + // Second build with zero source changes — an incremental no-op. + await buildGraph(tmpDir, { + engine: 'native', + incremental: true, + dataflow: true, + skipRegistry: true, + }); + }, 60_000); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("still has both leaf functions' vertices after a no-op incremental rebuild", () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const vertices = readDataflowVertices(dbPath); + expect( + vertices.some((v) => v.func_name === 'greet' && v.kind === 'param' && v.name === 'name'), + `missing greet param vertex after no-op rebuild; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + expect( + vertices.some((v) => v.func_name === 'shout' && v.kind === 'param' && v.name === 'word'), + `missing shout param vertex after no-op rebuild; got: ${JSON.stringify(vertices)}`, + ).toBe(true); + }); + }, +);