Skip to content

Commit 771a64c

Browse files
committed
fix(scripts): resolve token-benchmark.ts --perf imports to real nested module paths
runPerfBenchmarks and its callers (buildCodegraph, buildSessionOptions) dynamically imported/spawned codegraph's own modules via flat paths (src/builder.js, src/queries.js, src/native.js, src/parser.js, src/cli.js) that never existed — src/ has no compiled .js siblings, and the real modules live nested (domain/graph/builder.ts, domain/queries.ts, infrastructure/native.ts, domain/parser.ts), mirroring dist/'s layout. Fix the four --perf imports by reusing srcImport() from scripts/lib/bench-config.ts — the same .ts-preferring resolution the other benchmark scripts (benchmark.ts, query-benchmark.ts, incremental-benchmark.ts, resolution-benchmark.ts) already rely on for this exact problem. The CLI-spawning cliPath had the same defect plus a second one: even with a correct path, a spawned child `node` process doesn't inherit the parent's --experimental-strip-types/--import <loader> flags needed to run TypeScript source directly. Extract the fix into a new scripts/lib/cli-invocation.ts (mirroring tests/integration/cli.test.ts's NODE_TS_FLAGS pattern) so both call sites share one correct implementation and it can be unit-tested directly. Impact: 4 functions changed, 5 affected
1 parent 5933236 commit 771a64c

4 files changed

Lines changed: 131 additions & 15 deletions

File tree

scripts/lib/cli-invocation.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Resolve `node` argv for spawning the codegraph CLI as a subprocess.
3+
*
4+
* Benchmark scripts (`token-benchmark.ts`) need to run the actual
5+
* `codegraph` CLI (`build`, `mcp`) against a target directory, not just
6+
* import its programmatic API. This repo ships TypeScript source with no
7+
* compiled `dist/` required for local development, so `node <root>/src/cli.js`
8+
* fails outright — only `cli.ts` exists (#1907). Even after fixing the
9+
* extension, a spawned child process doesn't inherit the parent's
10+
* `--experimental-strip-types` / `--import <loader>` flags the way
11+
* statically- or dynamically-imported modules in the parent process do, so
12+
* those flags must be passed explicitly. Mirrors the pattern already used by
13+
* `tests/integration/cli.test.ts`'s `NODE_TS_FLAGS`.
14+
*/
15+
import path from 'node:path';
16+
17+
/**
18+
* @param root Absolute path to the codegraph repo root (the directory
19+
* containing `src/`, not `src/` itself).
20+
* @returns argv (excluding `node` itself) to spawn `src/cli.ts` with the
21+
* flags required to run TypeScript source directly.
22+
*/
23+
export function resolveCliNodeArgs(root: string): string[] {
24+
const loaderUrl = new URL('../ts-resolve-loader.ts', import.meta.url).href;
25+
return ['--experimental-strip-types', '--import', loaderUrl, path.join(root, 'src', 'cli.ts')];
26+
}

scripts/token-benchmark.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,23 @@ import { parseArgs } from 'node:util';
2727

2828
import { ISSUES, extractAgentOutput, validateResult } from './token-benchmark-issues.js';
2929
import { getBenchmarkVersion } from './bench-version.js';
30+
import { srcImport } from './lib/bench-config.js';
3031
import { median, round1, timeMedian } from './lib/bench-timing.js';
32+
import { resolveCliNodeArgs } from './lib/cli-invocation.js';
3133
import { extractUsageMetrics, tallyToolCalls } from './lib/session-metrics.js';
3234

3335
const __dirname = path.dirname(fileURLToPath(import.meta.url));
3436
const root = path.resolve(__dirname, '..');
3537
const pkg = JSON.parse(fs.readFileSync(path.join(root, 'package.json'), 'utf8'));
3638
const benchVersion = getBenchmarkVersion(pkg.version, root);
3739

40+
// Codegraph's own module tree lives nested under src/ (mirrors dist/'s
41+
// layout), not flat — see scripts/lib/bench-config.ts's srcImport() for the
42+
// same .ts-preferring resolution used by the other benchmark scripts.
43+
const SRC_DIR = path.join(root, 'src');
44+
45+
const CLI_NODE_ARGS = resolveCliNodeArgs(root);
46+
3847
// Redirect console.log to stderr so only JSON goes to stdout
3948
const origLog = console.log;
4049
console.log = (...args) => console.error(...args);
@@ -153,10 +162,9 @@ function checkoutCommit(nextjsDir, sha) {
153162
// ── Graph building ────────────────────────────────────────────────────────
154163

155164
async function buildCodegraph(nextjsDir) {
156-
const cliPath = path.join(root, 'src', 'cli.js');
157165
console.error('Building codegraph graph for Next.js...');
158166
const start = performance.now();
159-
execFileSync('node', [cliPath, 'build', nextjsDir], {
167+
execFileSync('node', [...CLI_NODE_ARGS, 'build', nextjsDir], {
160168
cwd: nextjsDir,
161169
stdio: 'pipe',
162170
timeout: 600_000, // 10 min
@@ -189,12 +197,11 @@ function buildSessionOptions(mode, nextjsDir) {
189197

190198
if (mode === 'codegraph') {
191199
const dbPath = path.join(nextjsDir, '.codegraph', 'graph.db');
192-
const cliPath = path.join(root, 'src', 'cli.js');
193200
options.mcpServers = {
194201
codegraph: {
195202
type: 'stdio',
196203
command: 'node',
197-
args: [cliPath, 'mcp', '-d', dbPath],
204+
args: [...CLI_NODE_ARGS, 'mcp', '-d', dbPath],
198205
},
199206
};
200207
}
@@ -310,19 +317,12 @@ async function runQueryBenchmarks(hubName, dbPath, fnDepsData, fnImpactData) {
310317
* Reuses the same codegraph APIs as the existing benchmark scripts.
311318
*/
312319
async function runPerfBenchmarks(nextjsDir) {
313-
const { pathToFileURL } = await import('node:url');
314-
const { buildGraph } = await import(
315-
pathToFileURL(path.join(root, 'src', 'builder.js')).href
316-
);
320+
const { buildGraph } = await import(srcImport(SRC_DIR, 'domain/graph/builder.js'));
317321
const { fnDepsData, fnImpactData, statsData } = await import(
318-
pathToFileURL(path.join(root, 'src', 'queries.js')).href
319-
);
320-
const { isNativeAvailable } = await import(
321-
pathToFileURL(path.join(root, 'src', 'native.js')).href
322-
);
323-
const { isWasmAvailable } = await import(
324-
pathToFileURL(path.join(root, 'src', 'parser.js')).href
322+
srcImport(SRC_DIR, 'domain/queries.js')
325323
);
324+
const { isNativeAvailable } = await import(srcImport(SRC_DIR, 'infrastructure/native.js'));
325+
const { isWasmAvailable } = await import(srcImport(SRC_DIR, 'domain/parser.js'));
326326

327327
const dbPath = path.join(nextjsDir, '.codegraph', 'graph.db');
328328

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Unit tests for scripts/lib/bench-config.ts's srcImport().
3+
*
4+
* Regression coverage for #1907: scripts/token-benchmark.ts's `--perf`
5+
* dynamic imports used flat paths (e.g. `src/builder.js`, `src/queries.js`,
6+
* `src/native.js`, `src/parser.js`) that never existed — the real modules
7+
* live nested (`src/domain/graph/builder.ts`, `src/domain/queries.ts`,
8+
* `src/infrastructure/native.ts`, `src/domain/parser.ts`), mirroring dist/'s
9+
* layout. This locks in that the exact nested subpaths token-benchmark.ts
10+
* now imports via srcImport() resolve to real, importable modules exporting
11+
* the symbols runPerfBenchmarks() needs.
12+
*/
13+
14+
import path from 'node:path';
15+
import { fileURLToPath } from 'node:url';
16+
import { describe, expect, it } from 'vitest';
17+
import { srcImport } from '../../scripts/lib/bench-config.js';
18+
19+
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
20+
const srcDir = path.join(repoRoot, 'src');
21+
22+
describe('srcImport() against the module paths used by token-benchmark.ts --perf', () => {
23+
it('resolves domain/graph/builder.js to a module exporting buildGraph', async () => {
24+
const mod = await import(srcImport(srcDir, 'domain/graph/builder.js'));
25+
expect(typeof mod.buildGraph).toBe('function');
26+
});
27+
28+
it('resolves domain/queries.js to a module exporting fnDepsData, fnImpactData, statsData', async () => {
29+
const mod = await import(srcImport(srcDir, 'domain/queries.js'));
30+
expect(typeof mod.fnDepsData).toBe('function');
31+
expect(typeof mod.fnImpactData).toBe('function');
32+
expect(typeof mod.statsData).toBe('function');
33+
});
34+
35+
it('resolves infrastructure/native.js to a module exporting isNativeAvailable', async () => {
36+
const mod = await import(srcImport(srcDir, 'infrastructure/native.js'));
37+
expect(typeof mod.isNativeAvailable).toBe('function');
38+
});
39+
40+
it('resolves domain/parser.js to a module exporting isWasmAvailable', async () => {
41+
const mod = await import(srcImport(srcDir, 'domain/parser.js'));
42+
expect(typeof mod.isWasmAvailable).toBe('function');
43+
});
44+
});

tests/unit/cli-invocation.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* Unit tests for scripts/lib/cli-invocation.ts
3+
*
4+
* Regression coverage for #1907: scripts/token-benchmark.ts spawned the
5+
* codegraph CLI via `path.join(root, 'src', 'cli.js')` — a file that never
6+
* existed (src/ has no compiled .js output; only src/cli.ts) — and, even
7+
* after fixing the extension, a spawned child `node` process doesn't
8+
* inherit the --experimental-strip-types / --import <loader> flags the
9+
* parent script itself needs to run TypeScript source. resolveCliNodeArgs()
10+
* centralizes the correct invocation so it can be exercised directly.
11+
*/
12+
13+
import { execFileSync } from 'node:child_process';
14+
import fs from 'node:fs';
15+
import path from 'node:path';
16+
import { fileURLToPath } from 'node:url';
17+
import { describe, expect, it } from 'vitest';
18+
import { resolveCliNodeArgs } from '../../scripts/lib/cli-invocation.js';
19+
20+
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
21+
22+
describe('resolveCliNodeArgs', () => {
23+
it('resolves the strip-types flag, the loader, and an existing cli.ts entry point', () => {
24+
const args = resolveCliNodeArgs(repoRoot);
25+
26+
expect(args[0]).toBe('--experimental-strip-types');
27+
expect(args[1]).toBe('--import');
28+
expect(args[2]).toMatch(/^file:.*ts-resolve-loader\.ts$/);
29+
expect(args[3]).toBe(path.join(repoRoot, 'src', 'cli.ts'));
30+
31+
// Both the loader and the CLI entry point must exist on disk — this is
32+
// exactly the class of bug #1907 fixed (paths pointing at files that
33+
// were never there).
34+
expect(fs.existsSync(new URL(args[2]))).toBe(true);
35+
expect(fs.existsSync(args[3])).toBe(true);
36+
});
37+
38+
it('spawns the real CLI successfully via the resolved argv (mirrors tests/integration/cli.test.ts)', () => {
39+
const args = resolveCliNodeArgs(repoRoot);
40+
const out = execFileSync('node', [...args, '--version'], {
41+
encoding: 'utf-8',
42+
timeout: 30_000,
43+
});
44+
expect(out.trim()).toMatch(/^\d+\.\d+\.\d+/);
45+
});
46+
});

0 commit comments

Comments
 (0)