Skip to content

fix(scripts): resolve token-benchmark.ts --perf imports to real nested module paths#2041

Open
carlos-alm wants to merge 1 commit into
fix/issue-1906-scripts-token-benchmark-ts-runsession-exceedsfrom
fix/issue-1907-scripts-token-benchmark-ts-perf-dynamic
Open

fix(scripts): resolve token-benchmark.ts --perf imports to real nested module paths#2041
carlos-alm wants to merge 1 commit into
fix/issue-1906-scripts-token-benchmark-ts-runsession-exceedsfrom
fix/issue-1907-scripts-token-benchmark-ts-perf-dynamic

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

runPerfBenchmarks (and its callers buildCodegraph/buildSessionOptions) in scripts/token-benchmark.ts dynamically imported/spawned codegraph's own modules via flat paths that never existed:

await import(pathToFileURL(path.join(root, 'src', 'builder.js')).href);   // real: src/domain/graph/builder.ts
await import(pathToFileURL(path.join(root, 'src', 'queries.js')).href);   // real: src/domain/queries.ts
await import(pathToFileURL(path.join(root, 'src', 'native.js')).href);    // real: src/infrastructure/native.ts
await import(pathToFileURL(path.join(root, 'src', 'parser.js')).href);    // real: src/domain/parser.ts
execFileSync('node', [path.join(root, 'src', 'cli.js'), ...]);            // real: src/cli.ts (no compiled .js)

src/ has no compiled .js siblings — only .ts sources — and none of these live flatly at src/ root except cli.ts. Confirmed via direct execution that all five paths throw Cannot find module / ERR_MODULE_NOT_FOUND today, so --perf (and, since buildCodegraph/buildSessionOptions are also used by the main non---perf flow, the base benchmark run too) has never actually worked against this repo's own source.

Fix

  • The four --perf imports now go through srcImport() from scripts/lib/bench-config.ts with the correct nested subpaths — the same .ts-preferring resolution benchmark.ts, query-benchmark.ts, incremental-benchmark.ts, and resolution-benchmark.ts already use 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 (only the parent script itself gets those flags from its own invocation). Extracted the fix into a new scripts/lib/cli-invocation.ts::resolveCliNodeArgs(), mirroring the exact pattern tests/integration/cli.test.ts already uses (NODE_TS_FLAGS), so both call sites (buildCodegraph, buildSessionOptions) share one correct, testable implementation instead of two duplicated broken ones.

Test plan

  • Added tests/unit/cli-invocation.test.ts — verifies resolveCliNodeArgs() resolves to a real, existing cli.ts + loader, and that spawning the CLI with the resolved argv actually runs (--version succeeds). Verified this test fails against the pre-fix code (Cannot find module .../src/cli.js).
  • Added tests/unit/bench-config-src-import.test.ts — verifies srcImport() resolves the exact nested subpaths runPerfBenchmarks needs (domain/graph/builder.js, domain/queries.js, infrastructure/native.js, domain/parser.js) to modules exporting the expected symbols.
  • Manually exercised the fixed buildCodegraph code path end-to-end against a throwaway fixture directory — confirmed it spawns the CLI, builds a real .codegraph/graph.db, and exits cleanly.
  • npx vitest run tests/unit/cli-invocation.test.ts tests/unit/bench-config-src-import.test.ts — both new files pass.
  • npm run lint — clean (scripts/ is outside Biome's scope per biome.json; new tests/ files pass lint).
  • codegraph diff-impact --staged -T — 4 functions changed, 5 transitive callers affected, all within scripts/token-benchmark.ts and the new scripts/lib/cli-invocation.ts as expected.
  • npm test (full suite) — no new failures introduced by this change. The suite in this fresh worktree does show ~102 pre-existing failures unrelated to this fix (stale prebuilt native addon vs. unreleased Rust changes already on this branch — e.g. native.leidenCommunities, busy-timeout threading, cross-language call rejection), matching already-open issues Same -f/--file array-vs-String native crash affects triage, interfaces, implementations, and other NativeRepository methods #1815 and Thread config.db.busyTimeoutMs into the Rust native DB layer (NativeDatabase::open_readonly/open_read_write) #1882; confirmed by git history that none of the affected test files were touched by this diff.

Fixes #1907

…d 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
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes scripts/token-benchmark.ts's --perf path and CLI-spawning bugs that caused ERR_MODULE_NOT_FOUND errors. All five broken module references (builder.js, queries.js, native.js, parser.js, and cli.js) are corrected by reusing existing patterns from the rest of the benchmark suite.

  • The four dynamic import() calls in runPerfBenchmarks now route through the existing srcImport() helper with the correct nested subpaths (domain/graph/builder.js, etc.) instead of non-existent flat paths under src/.
  • The two execFileSync('node', [cliPath, ...]) call sites now use a new resolveCliNodeArgs() utility that correctly supplies --experimental-strip-types and --import <loader> flags (required because child processes don't inherit these from the parent invocation), mirroring the pattern already used by tests/integration/cli.test.ts.
  • Regression tests are added for both helpers, verifying paths exist on disk and that the real CLI spawns successfully.

Confidence Score: 5/5

This PR fixes previously non-functional code paths; the changes are narrow, well-tested, and mirror patterns already proven in the integration test suite.

All five broken references (four dynamic imports and one CLI exec path) are replaced with correct equivalents that reuse well-established helpers already present in the codebase. The loader URL derivation in cli-invocation.ts is computed from import.meta.url of the file itself, making it path-stable regardless of how the script is invoked. The two new unit tests exercise both the file-existence invariant and an end-to-end CLI spawn, covering exactly the class of bug being fixed. No pre-existing passing tests were modified.

No files require special attention; all four changed files are straightforward.

Important Files Changed

Filename Overview
scripts/lib/cli-invocation.ts New helper that centralises node argv construction for spawning the TypeScript CLI, correctly mirrors the NODE_TS_FLAGS pattern from the integration test suite
scripts/token-benchmark.ts Replaces five broken module/CLI paths with the correct srcImport() and resolveCliNodeArgs() calls; SRC_DIR is always the local src/ regardless of --npm mode, which is consistent with pre-existing intent
tests/unit/cli-invocation.test.ts Verifies that resolveCliNodeArgs() returns an argv pointing at real files on disk and that spawning the CLI with those args succeeds
tests/unit/bench-config-src-import.test.ts Locks in the exact srcImport() subpaths used by runPerfBenchmarks and checks that the expected symbols are exported

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    TB["token-benchmark.ts"]

    TB -->|"buildCodegraph / buildSessionOptions"| CLI_ARGS["resolveCliNodeArgs(root)\nscripts/lib/cli-invocation.ts"]
    CLI_ARGS --> SPAWN["execFileSync('node',\n[--experimental-strip-types,\n--import loaderUrl,\nsrc/cli.ts, ...])\n✅ was: src/cli.js (never existed)"]

    TB -->|"runPerfBenchmarks"| PERF["srcImport(SRC_DIR, subpath)\nscripts/lib/bench-config.ts"]
    PERF --> CHECK{".ts exists?"}
    CHECK -->|Yes| TS_URL["file://…/src/domain/graph/builder.ts\nfile://…/src/domain/queries.ts\nfile://…/src/infrastructure/native.ts\nfile://…/src/domain/parser.ts\n✅ was: src/builder.js etc."]
    CHECK -->|No| JS_URL["file://…/file.js (dist mode)"]

    CLI_ARGS -.->|"loader URL relative to\ncli-invocation.ts"| LOADER["scripts/ts-resolve-loader.ts\n(registers .ts ESM hook)"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    TB["token-benchmark.ts"]

    TB -->|"buildCodegraph / buildSessionOptions"| CLI_ARGS["resolveCliNodeArgs(root)\nscripts/lib/cli-invocation.ts"]
    CLI_ARGS --> SPAWN["execFileSync('node',\n[--experimental-strip-types,\n--import loaderUrl,\nsrc/cli.ts, ...])\n✅ was: src/cli.js (never existed)"]

    TB -->|"runPerfBenchmarks"| PERF["srcImport(SRC_DIR, subpath)\nscripts/lib/bench-config.ts"]
    PERF --> CHECK{".ts exists?"}
    CHECK -->|Yes| TS_URL["file://…/src/domain/graph/builder.ts\nfile://…/src/domain/queries.ts\nfile://…/src/infrastructure/native.ts\nfile://…/src/domain/parser.ts\n✅ was: src/builder.js etc."]
    CHECK -->|No| JS_URL["file://…/file.js (dist mode)"]

    CLI_ARGS -.->|"loader URL relative to\ncli-invocation.ts"| LOADER["scripts/ts-resolve-loader.ts\n(registers .ts ESM hook)"]
Loading

Reviews (1): Last reviewed commit: "fix(scripts): resolve token-benchmark.ts..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

4 functions changed5 callers affected across 1 files

  • resolveCliNodeArgs in scripts/lib/cli-invocation.ts:23 (1 transitive callers)
  • buildCodegraph in scripts/token-benchmark.ts:164 (3 transitive callers)
  • buildSessionOptions in scripts/token-benchmark.ts:187 (3 transitive callers)
  • runPerfBenchmarks in scripts/token-benchmark.ts:319 (2 transitive callers)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant