Skip to content
11 changes: 6 additions & 5 deletions scripts/benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import path from 'node:path';
import { performance } from 'node:perf_hooks';
import { fileURLToPath } from 'node:url';
import Database from 'better-sqlite3';
import { resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';

// ── Parent process: fork one child per engine, assemble final output ─────
Expand Down Expand Up @@ -94,6 +94,7 @@ const INCREMENTAL_RUNS = 3;
const QUERY_RUNS = 5;
const QUERY_WARMUP_RUNS = 3;
const PROBE_FILE = path.join(root, 'src', 'domain', 'queries.ts');
const BENCH_EXCLUDE = [...resolveBenchmarkExcludes()];

function median(arr) {
const sorted = [...arr].sort((a, b) => a - b);
Expand Down Expand Up @@ -131,7 +132,7 @@ console.log = (...args) => console.error(...args);
if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);

const buildStart = performance.now();
const buildResult = await buildGraph(root, { engine, incremental: false });
const buildResult = await buildGraph(root, { engine, incremental: false, exclude: BENCH_EXCLUDE });
const buildTimeMs = performance.now() - buildStart;

// Warmed median of QUERY_RUNS samples with `noTests: true` to match the
Expand All @@ -156,7 +157,7 @@ try {
const noopTimings = [];
for (let i = 0; i < INCREMENTAL_RUNS; i++) {
const start = performance.now();
await buildGraph(root, { engine, incremental: true });
await buildGraph(root, { engine, incremental: true, exclude: BENCH_EXCLUDE });
noopTimings.push(performance.now() - start);
}
noopRebuildMs = Math.round(median(noopTimings));
Expand All @@ -173,7 +174,7 @@ try {
for (let i = 0; i < INCREMENTAL_RUNS; i++) {
fs.writeFileSync(PROBE_FILE, original + `\n// probe-${i}\n`);
const start = performance.now();
const res = await buildGraph(root, { engine, incremental: true });
const res = await buildGraph(root, { engine, incremental: true, exclude: BENCH_EXCLUDE });
oneFileRuns.push({ ms: performance.now() - start, phases: res?.phases || null });
}
oneFileRuns.sort((a, b) => a.ms - b.ms);
Expand All @@ -185,7 +186,7 @@ try {
} finally {
fs.writeFileSync(PROBE_FILE, original);
try {
await buildGraph(root, { engine, incremental: true });
await buildGraph(root, { engine, incremental: true, exclude: BENCH_EXCLUDE });
} catch {
// Cleanup rebuild failed — probe file is already restored, move on
}
Expand Down
18 changes: 9 additions & 9 deletions scripts/incremental-benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import fs from 'node:fs';
import path from 'node:path';
import { performance } from 'node:perf_hooks';
import { fileURLToPath } from 'node:url';
import { resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { isWorker, workerEngine, forkEngines } from './lib/fork-engine.js';

// ── Parent process: fork one child per engine, assemble final output ─────
Expand Down Expand Up @@ -181,14 +181,14 @@ const PROBE_FILE = path.join(root, 'src', 'domain', 'queries.ts');
// CI-amplified false regressions on sub-30ms metrics like No-op rebuild.
const WARMUP_RUNS = 2;

// Resolution-benchmark fixtures live under the repo root and get pulled into
// every self-build sweep. They are hand-annotated test corpora — not real
// codegraph codeand a single heavy grammar (e.g. tree-sitter-verilog) can
// add hundreds of milliseconds to fullBuildMs purely from fixture parsing
// (#1112). Exclude them so adding native support for a new language doesn't
// silently inflate the incremental-benchmark numbers.
const BENCH_EXCLUDE = ['tests/benchmarks/resolution/fixtures/**'];
const BUILD_OPTS = { engine, exclude: BENCH_EXCLUDE };
// Resolution-benchmark fixtures (`BENCHMARK_EXCLUDES` in scripts/lib/bench-config.ts)
// are excluded from every benchmark `buildGraph` call. See that constant for the
// full rationaleshort version: hand-annotated fixtures aren't representative
// of real source, and heavyweight grammars (#1107) silently inflate timings.
// `resolveBenchmarkExcludes` returns `[]` in `--npm` mode so the baseline (an
// older published version that ignores `opts.exclude`) and the dev run sweep
// the same corpus.
const BUILD_OPTS = { engine, exclude: [...resolveBenchmarkExcludes()] };

function median(arr) {
const sorted = [...arr].sort((a, b) => a - b);
Expand Down
51 changes: 51 additions & 0 deletions scripts/lib/bench-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,57 @@ import { pathToFileURL } from 'node:url';

import { getBenchmarkVersion } from '../bench-version.js';

/**
* Globs excluded from every benchmark's `buildGraph(root, ...)` invocation.
*
* Resolution-benchmark fixtures (`tests/benchmarks/resolution/fixtures/**`)
* are hand-annotated scaffolding for the static-resolution test suite, not
* representative source code. They inflate dogfooding timing measurements
* disproportionately whenever a new-language PR lands a heavyweight grammar
* (e.g. tree-sitter-verilog added ~850ms to native fullBuildMs in #1107).
* Excluding them here keeps build/query/incremental benchmarks measuring
* codegraph's own source rather than its test fixtures.
*
* NOTE: callers should generally prefer `resolveBenchmarkExcludes()` instead
* of this constant. The helper returns `[]` in `--npm` mode so the dev-vs-
* baseline corpus stays consistent — published versions before this PR
* silently dropped `opts.exclude`, which would otherwise leave the baseline
* sweeping fixtures while the dev run skipped them.
*/
export const BENCHMARK_EXCLUDES: readonly string[] = [
'tests/benchmarks/resolution/fixtures/**',
];

/**
* `BENCHMARK_EXCLUDES` in local mode; `[]` in `--npm` mode.
*
* `--npm` benchmarks load `buildGraph` from a previously-published version
* via `srcImport(srcDir, ...)`. Releases before `BuildGraphOpts.exclude`
* landed don't recognise the option and silently drop it, so passing the
* excludes to a stale baseline would make it sweep ~745 files while the dev
* run sweeps ~607 — a corpus-mismatch that disguises measurement-shift as a
* perf delta. Emitting `[]` in `--npm` mode keeps the comparison apples-to-
* apples; the warning makes the methodology shift explicit.
*
* Idempotent across calls (the warning is printed on the first invocation
* only — `process.stderr.write` is a no-op but the helper is conceptually
* "compute once, return constant"); intentionally synchronous because
* `parseArgs` is.
*/
let warnedAboutNpmExcludeSkip = false;
export function resolveBenchmarkExcludes(): readonly string[] {
const { npm } = parseArgs();
if (!npm) return BENCHMARK_EXCLUDES;
if (!warnedAboutNpmExcludeSkip) {
console.error(
'Note: --npm mode skips BENCHMARK_EXCLUDES so the baseline and dev runs sweep the same corpus. ' +
'Published versions before #1134 ignore opts.exclude silently; passing it would skew dev timings down by ~138 fewer files.',
);
warnedAboutNpmExcludeSkip = true;
}
return [];
}

// On Windows, `npm` is `npm.cmd` and Node refuses to spawn `.cmd`/`.bat`
// without `shell: true` (since Node 18.20 / 20.15). When `shell: true`, the
// Windows `cmd.exe` shell resolves bare `npm` to `npm.cmd` automatically, so
Expand Down
4 changes: 2 additions & 2 deletions scripts/query-benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import path from 'node:path';
import { performance } from 'node:perf_hooks';
import { fileURLToPath } from 'node:url';
import Database from 'better-sqlite3';
import { resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';

// ── Parent process: fork one child per engine, assemble final output ─────
Expand Down Expand Up @@ -254,7 +254,7 @@ function benchDiffImpact(hubName) {

// Build graph for this engine
if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);
await buildGraph(root, { engine, incremental: false });
await buildGraph(root, { engine, incremental: false, exclude: [...resolveBenchmarkExcludes()] });

const targets = workerTargets() || selectTargets();
console.error(`Targets: hub=${targets.hub}, mid=${targets.mid}, leaf=${targets.leaf}`);
Expand Down
11 changes: 9 additions & 2 deletions src/domain/graph/builder/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,15 @@ function setupPipeline(ctx: PipelineContext): void {
initSchema(ctx.db);

ctx.config = loadConfig(ctx.rootDir);
if (ctx.opts.exclude && ctx.opts.exclude.length > 0) {
ctx.config = { ...ctx.config, exclude: [...ctx.config.exclude, ...ctx.opts.exclude] };
// Merge caller-supplied excludes on top of the file-config excludes so
// programmatic callers (e.g. benchmark scripts) can extend exclusion
// without mutating .codegraphrc.json. Native orchestrator picks this up
// automatically — it reads exclude off the serialized ctx.config below.
if (ctx.opts.exclude?.length) {
ctx.config = {
...ctx.config,
exclude: [...(ctx.config.exclude ?? []), ...ctx.opts.exclude],
};
}
ctx.incremental =
ctx.opts.incremental !== false && ctx.config.build && ctx.config.build.incremental !== false;
Expand Down
9 changes: 5 additions & 4 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1063,13 +1063,14 @@ export interface BuildGraphOpts {
complexity?: boolean;
cfg?: boolean;
scope?: string[];
skipRegistry?: boolean;
/**
* Extra exclude globs appended to `config.exclude`. Lets benchmark scripts
* skip fixture directories that bloat self-build timings without permanently
* affecting `.codegraphrc.json` for normal users.
* Glob patterns merged on top of `config.exclude` for this build only.
* Lets callers extend exclusion programmatically without writing a config
* file — used by the benchmark scripts to skip resolution-benchmark
* fixtures that aren't representative of real code.
*/
exclude?: string[];
skipRegistry?: boolean;
}

/** Build timing result from buildGraph. */
Expand Down
15 changes: 15 additions & 0 deletions tests/benchmarks/regression-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,20 @@ const SKIP_VERSIONS = new Set(['3.8.0']);
* (24.3 → 45.6ms) is consistent with the other depths. Tracked in #1113
* alongside depth 1, depth 5, and Query time; remove all four once
* 3.11.0+ data confirms the new steady-state.
*
* - 3.10.0:DB bytes/file — one-time per-file methodology shift introduced
* by #1134, which excludes `tests/benchmarks/resolution/fixtures/**`
* from the dogfooding `buildGraph` sweep so heavyweight new grammars
* (e.g. Verilog #1107) no longer inflate timing. The 3.10.0 baseline
* was measured with fixtures in the corpus (~745 files); dev now
* measures the codegraph source alone (~607 files). DB content is
* dominated by `src/`, so total bytes stay roughly constant while the
* denominator drops, inflating `dbSizeBytes/file` from 41614 → ~52211
* (+25%, exactly at the threshold). This is the same shape as the old
* `3.10.0:Full build` exemption (now removed because Full build absolute
* timing actually improved) — a measurement-scope artifact, not a real
* regression in the schema or extraction layer. Exempt this release;
* remove once 3.11.0+ data is captured under the post-#1134 methodology.
*/
const KNOWN_REGRESSIONS = new Set([
'3.9.6:Build ms/file',
Expand All @@ -222,6 +236,7 @@ const KNOWN_REGRESSIONS = new Set([
'3.10.0:fnDeps depth 3',
'3.10.0:fnDeps depth 5',
'3.10.0:Query time',
'3.10.0:DB bytes/file',
]);

/**
Expand Down
99 changes: 99 additions & 0 deletions tests/integration/config-include-exclude.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,103 @@ describe('config.include / config.exclude (issue #981)', () => {
// Paths are already relative to each run's own tmpDir so they compare directly.
expect(nativeFiles).toEqual(wasmFiles);
});

// ── opts.exclude (programmatic, no on-disk config) ───────────────

async function buildWithOptsExclude(
root: string,
engine: EngineName,
optsExclude: string[],
): Promise<string[]> {
clearConfigCache();
const dbDir = path.join(root, '.codegraph');
if (fs.existsSync(dbDir)) fs.rmSync(dbDir, { recursive: true, force: true });
await buildGraph(root, { engine, exclude: optsExclude, skipRegistry: true });
const files = readFileRows(path.join(dbDir, 'graph.db'));
return files.map((f) => f.replace(/\\/g, '/')).sort();
}

it('wasm: opts.exclude rejects matching files without writing config', async () => {
const root = fs.mkdtempSync(path.join(tmpDir, 'opts-wasm-'));
writeFixture(root);
const files = await buildWithOptsExclude(root, 'wasm', ['**/*.test.js', '**/*.spec.js']);
expect(files).toContain('src/math.js');
expect(files).not.toContain('src/math.test.js');
expect(files).not.toContain('src/util.spec.js');
});

itNative('native: opts.exclude rejects matching files without writing config', async () => {
const root = fs.mkdtempSync(path.join(tmpDir, 'opts-native-'));
writeFixture(root);
const files = await buildWithOptsExclude(root, 'native', ['**/*.test.js', '**/*.spec.js']);
expect(files).toContain('src/math.js');
expect(files).not.toContain('src/math.test.js');
expect(files).not.toContain('src/util.spec.js');
});

// ── opts.exclude incremental round trip ──────────────────────────
//
// Greptile feedback on PR #1134: the opts.exclude tests above always wipe
// the DB before building, so they only exercise the fresh-build path. The
// scenario where files that were previously indexed become excluded on a
// subsequent incremental run (i.e. opts.exclude changes between builds
// against the same DB) was untested. This round trip locks in the
// collect → detect behaviour: the second build must observe the newly
// excluded files as removals and drop them from file_hashes.

async function buildSameDb(
root: string,
engine: EngineName,
optsExclude: string[] | undefined,
): Promise<string[]> {
clearConfigCache();
await buildGraph(root, {
engine,
...(optsExclude !== undefined ? { exclude: optsExclude } : {}),
skipRegistry: true,
});
const files = readFileRows(path.join(root, '.codegraph', 'graph.db'));
return files.map((f) => f.replace(/\\/g, '/')).sort();
}

it('wasm: opts.exclude introduced on second incremental build drops previously-indexed files', async () => {
const root = fs.mkdtempSync(path.join(tmpDir, 'opts-inc-wasm-'));
writeFixture(root);
// Wipe DB so the first build is a clean baseline that indexes everything.
const dbDir = path.join(root, '.codegraph');
if (fs.existsSync(dbDir)) fs.rmSync(dbDir, { recursive: true, force: true });

// First build: no exclude — every supported file is indexed.
const firstFiles = await buildSameDb(root, 'wasm', undefined);
expect(firstFiles).toContain('src/math.test.js');
expect(firstFiles).toContain('src/util.spec.js');

// Second build against the same DB with exclude — previously-indexed
// test files must be detected as removals and disappear from file_hashes.
const secondFiles = await buildSameDb(root, 'wasm', ['**/*.test.js', '**/*.spec.js']);
expect(secondFiles).toContain('src/math.js');
expect(secondFiles).toContain('src/util.js');
expect(secondFiles).not.toContain('src/math.test.js');
expect(secondFiles).not.toContain('src/util.spec.js');
});

itNative(
'native: opts.exclude introduced on second incremental build drops previously-indexed files',
async () => {
const root = fs.mkdtempSync(path.join(tmpDir, 'opts-inc-native-'));
writeFixture(root);
const dbDir = path.join(root, '.codegraph');
if (fs.existsSync(dbDir)) fs.rmSync(dbDir, { recursive: true, force: true });

const firstFiles = await buildSameDb(root, 'native', undefined);
expect(firstFiles).toContain('src/math.test.js');
expect(firstFiles).toContain('src/util.spec.js');

const secondFiles = await buildSameDb(root, 'native', ['**/*.test.js', '**/*.spec.js']);
expect(secondFiles).toContain('src/math.js');
expect(secondFiles).toContain('src/util.js');
expect(secondFiles).not.toContain('src/math.test.js');
expect(secondFiles).not.toContain('src/util.spec.js');
},
);
});
Comment on lines +196 to 280
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 opts.exclude incremental coverage gap

Both new tests always wipe the DB before building, so they only exercise the fresh-build path. The scenario where files that were previously indexed become excluded on a subsequent incremental run (i.e. opts.exclude changes between builds against the same DB) is untested. In practice this path should work — collectFiles would omit the newly-excluded files and detectChanges would surface them as removals — but a short test covering one incremental round trip would lock in that behaviour and guard against regressions in the collect/detect stages.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b008ffb — added wasm + native parity tests opts.exclude introduced on second incremental build drops previously-indexed files that build the fixture twice against the same DB (first without exclude, then with). The second build observes the previously-indexed test files as removals via detectChanges, and they are dropped from file_hashes. Verified locally: first build indexes 5 files, second build reports "0 changed, 2 removed", and asserts the test/spec files no longer appear in file_hashes.

Loading