h-dv/code-index

Fork 0

bug: the JS/TS plugin never captures CommonJS require() — express's entire module graph is invisible (410 calls, 0 imports) #56

New issue

Closed

opened 2026-07-29 18:18:34 +02:00 by buildagent · 2 comments

buildagent commented

2026-07-29 18:18:34 +02:00

Member

Found by #45's baseline, which recorded js-express imports=0 as an oddity. Investigated for #46. Measured, not inferred.

The defect

crates/plugins/src/typescript.rs (the single plugin for .ts/.tsx/.js/.jsx/.mjs/.cjs) emits imports only from the import_statement node — see the match arm at typescript.rs:286 and the flattening at :624-716. A CommonJS require() is a call_expression, so it is never seen. The only two textual matches for "require" in the plugin are required_parameter (unrelated).

Reproduction

// main.js
const helper = require("./helper");
const { thing } = require("./other");
module.exports = { go: () => helper.doIt() };
// esm.mjs
import helper2 from "./helper";

$ code-index index --root probe --db probe.db
$ sqlite3 probe.db "SELECT f.path, i.module FROM imports i JOIN files f ON f.id=i.file_id"
esm.mjs|./helper          <- ESM captured
                          <- both require() calls: NOTHING

Scale of the impact

repo	files with `require(`	`require(` calls	ESM import lines
js-express	131	410	0
ts-zod	2	2	1006

express is 100% CommonJS, so its entire module graph is invisible to the resolver. Every import-based resolution path is starved: tier 1b's file-key and package-tail reachability arms, tier 3's import boost, and tier 1R's origin gate all join imports.

Why this matters for #46

#46 asks how much of the 3.4× cross-language resolution spread is a denominator artifact vs a real plugin gap. For js-express the answer is now measurable:

resolved                            4 154
unresolved but name-matches an
  in-project javascript symbol      7 732
unresolved total                   15 594

So the internal-eligible rate is 4154/(4154+7732) = 35%, not the headline 21%. A large share of those 7 732 are plausibly reachable if the module graph existed.

Note this also exonerates ts-zod: it is essentially all-ESM (1 006 import lines, 2 requires), so its 13.2% is not this gap and remains best explained as denominator composition (type-level refs into TS lib built-ins).

Why I did not just fix it

This is a recall change, and recall changes are where this project has repeatedly drawn blood — I037's first fix was wrong and shipped anyway. Feeding 410 new import edges into three resolution tiers can mint phantoms, so it needs the full treatment rather than a drive-by:

capture require("...") (and likely import(...)) as imports, with the same shape ESM produces;
decide the module normalisation so existing joins (i.module = k.key, the IMPORT_SEGMENTS GLOBs) work unchanged;
handle destructuring (const { a, b } = require(...)) — the per-binding split in #31 item 2 is adjacent;
clamp with precision_gate (phantom == 0) and the corpus mutation guard;
re-bless tests/corpus/baseline.json with the reason (imports and resolved will both move for js-express) — the ratchet will fail loudly until then, which is the intended behaviour;
ship a re-heal migration, since extraction changes: existing DBs need re-parsing, so this is the m0021/m0022 pattern (invalidate stat+hash), not the m0023/m0024 resolve-only pattern.

Acceptance

require() and dynamic import() captured as imports in .js/.cjs/.mjs (and .ts where used)
destructured requires produce one row per binding, or the limitation is documented
phantom_count == 0 across all 7 languages; corpus mutation guard still 0 rebinds / 7 056 sites
js-express resolved measurably improves; baseline re-blessed with a written reason
re-heal migration following the m0021/m0022 (extraction-changed) pattern
a hermetic test with the 3-file probe above, asserting BOTH that require-imports appear and that a cross-file call through one resolves

Severity: medium

No phantom, no crash — it is a silent recall ceiling for an entire module system, and CommonJS is still most of the Node ecosystem. It has been invisible because no fixture used require().

Found by #45's baseline, which recorded `js-express imports=0` as an oddity. Investigated for #46. **Measured, not inferred.** ## The defect `crates/plugins/src/typescript.rs` (the single plugin for `.ts/.tsx/.js/.jsx/.mjs/.cjs`) emits imports only from the `import_statement` node — see the match arm at `typescript.rs:286` and the flattening at `:624-716`. A CommonJS `require()` is a **`call_expression`**, so it is never seen. The only two textual matches for "require" in the plugin are `required_parameter` (unrelated). ### Reproduction ```js // main.js const helper = require("./helper"); const { thing } = require("./other"); module.exports = { go: () => helper.doIt() }; // esm.mjs import helper2 from "./helper"; ``` ``` $ code-index index --root probe --db probe.db $ sqlite3 probe.db "SELECT f.path, i.module FROM imports i JOIN files f ON f.id=i.file_id" esm.mjs|./helper <- ESM captured <- both require() calls: NOTHING ``` ## Scale of the impact | repo | files with `require(` | `require(` calls | ESM import lines | |---|---|---|---| | **js-express** | **131** | **410** | **0** | | ts-zod | 2 | 2 | 1006 | express is **100% CommonJS**, so *its entire module graph is invisible to the resolver*. Every import-based resolution path is starved: tier 1b's file-key and package-tail reachability arms, tier 3's import boost, and tier 1R's origin gate all join `imports`. ## Why this matters for #46 #46 asks how much of the 3.4× cross-language resolution spread is a denominator artifact vs a real plugin gap. For js-express the answer is now measurable: ``` resolved 4 154 unresolved but name-matches an in-project javascript symbol 7 732 unresolved total 15 594 ``` So the **internal-eligible rate is 4154/(4154+7732) = 35%**, not the headline 21%. A large share of those 7 732 are plausibly reachable *if the module graph existed*. Note this also **exonerates ts-zod**: it is essentially all-ESM (1 006 import lines, 2 requires), so its 13.2% is *not* this gap and remains best explained as denominator composition (type-level refs into TS lib built-ins). ## Why I did not just fix it This is a **recall** change, and recall changes are where this project has repeatedly drawn blood — I037's first fix was wrong and shipped anyway. Feeding 410 new import edges into three resolution tiers can mint phantoms, so it needs the full treatment rather than a drive-by: 1. capture `require("...")` (and likely `import(...)`) as imports, with the same shape ESM produces; 2. decide the `module` normalisation so existing joins (`i.module = k.key`, the `IMPORT_SEGMENTS` GLOBs) work unchanged; 3. handle destructuring (`const { a, b } = require(...)`) — the per-binding split in #31 item 2 is adjacent; 4. **clamp with `precision_gate` (phantom == 0) and the corpus mutation guard**; 5. re-bless `tests/corpus/baseline.json` with the reason (`imports` and `resolved` will both move for js-express) — the ratchet will fail loudly until then, which is the intended behaviour; 6. ship a re-heal migration, since extraction changes: existing DBs need re-parsing, so this is the m0021/m0022 pattern (invalidate stat+hash), **not** the m0023/m0024 resolve-only pattern. ## Acceptance - [ ] `require()` and dynamic `import()` captured as imports in `.js/.cjs/.mjs` (and `.ts` where used) - [ ] destructured requires produce one row per binding, or the limitation is documented - [ ] `phantom_count == 0` across all 7 languages; corpus mutation guard still 0 rebinds / 7 056 sites - [ ] js-express `resolved` measurably improves; baseline re-blessed with a written reason - [ ] re-heal migration following the m0021/m0022 (extraction-changed) pattern - [ ] a hermetic test with the 3-file probe above, asserting BOTH that require-imports appear and that a cross-file call through one resolves ## Severity: medium No phantom, no crash — it is a silent recall ceiling for an entire module system, and CommonJS is still most of the Node ecosystem. It has been invisible because no fixture used `require()`.

buildagent added the

labels

2026-07-29 18:18:41 +02:00

buildagent commented

2026-07-29 18:57:11 +02:00

Author

Member

Part 1 shipped (`4a98948` + `9bf792f`). Part 2 BLOCKED with evidence — and my original estimate here was wrong.

What shipped: the CommonJS export surface (precision)

collect_cjs_exports reads module.exports / exports.x and marks module-level symbols Exported or File instead of Unknown. A file with no detectable exports keeps the old conservative Unknown — global scripts and unmodelled re-export idioms must not lose real resolutions (pinned by bare_cjs_without_exports_stays_unknown).

Measured on express: resolved 4154 → 4152, gained 0, lost 2. Both removed bindings are verified phantoms — a local id in examples/route-middleware and test/app.router.js binding to examples/mvc/controllers/user/index.js's module-private id. Pure precision, no recall cost.

m0025 is the extraction re-heal (stat+hash invalidated, m0021/m0022 pattern), because visibility is recorded at parse time and existing DBs keep their stale unknown rows otherwise.

One subtlety that produced a wrong result before I caught it: the export set must contain only names another file can refer to a symbol by, not every value that escapes. express writes exports.request = req; collecting the right-hand identifier made the private req Exported again and resurrected the very phantom class this removes. Keys only.

Part 2 — require() capture — implemented, measured, REVERTED

It worked, in every binding form, and correctly declined computed/interpolated specifiers:

main.js:1  ./helper      alias=helper        plain
main.js:2  ./other       alias=thing         shorthand destructure
main.js:2  ./other       alias=renamed       renamed destructure
main.js:3  ./arr         alias=first         array pattern
main.js:4  ./deep        alias=deep          member on the call
main.js:5  ./sideeffect  alias=(none)        bare statement
main.js:9  ./static      alias=plain         static template
           require(someVar)  -> correctly SKIPPED
           require(`./x${y}`) -> correctly SKIPPED

express: imports 0 → 388. And then:

resolved +6 / −2, and all six gained are PHANTOMS. lib/application.js:221 binds a local fn into test/utils.js#fn — a production file into a test file.

Root cause is upstream of the capture: require("./utils") from lib/ stem-matches both lib/utils.js and test/utils.js, because tier 1b's file-key arm compares stems and ignores relative-path semantics. Landing correct import rows simply widens reachability through that existing hole. Net +4 phantoms against the guarantee that is this product's core claim, so it is not shippable.

The code is reverted rather than left dead. It is straightforward to restore once relative specifiers are path-resolved.

Correcting my own numbers in this issue

Two things I asserted above are wrong and should not be relied on:

"internal-eligible rate is 35%, a large share plausibly reachable if the module graph existed." Too optimistic. The dominant CommonJS shape is util.method() — a member call through a module object — which needs receiver typing, not an import edge. A minimal 2-file probe (const util = require("./lib/util"); util.uniqueHelperFn(1)) still does not resolve with imports captured. The realistic upside is single digits, not thousands.
An intermediate claim of "25 pre-existing phantoms eliminated" was an artifact of a bug in my own patch (visibility_for was handed the declaration node, which has no name field, so every var looked up as "" and got File-scoped regardless of export status). With correct semantics those phantoms remain. Only 2 are removed.

Acceptance status

require() / dynamic import() captured — implemented, reverted, blocked on relative-specifier resolution
destructured requires produce one row per binding (proven, in the reverted work)
phantom_count == 0 across all 7 languages; mutation guard 0 rebinds / 7 056 sites
js-express resolved improves — it decreases by 2, and that is the correct direction (phantom removal)
baseline re-blessed, reason recorded in the file; the ratchet caught the change first (resolved 4154 -> 4152 (-2))
re-heal migration, extraction pattern, with a test that pins both invalidations
hermetic tests; two existing contract tests deliberately flipped with the reasoning inline

Follow-up to file

Tier 1b's file-key arm should path-resolve relative specifiers. ./utils from lib/ must mean lib/utils.js, not any file whose stem is utils. That unblocks part 2 and is very likely removing phantoms in the other languages too.

## Part 1 shipped (`4a98948` + `9bf792f`). Part 2 BLOCKED with evidence — and my original estimate here was wrong. ### What shipped: the CommonJS export surface (precision) `collect_cjs_exports` reads `module.exports` / `exports.x` and marks module-level symbols `Exported` or `File` instead of `Unknown`. A file with **no** detectable exports keeps the old conservative `Unknown` — global scripts and unmodelled re-export idioms must not lose real resolutions (pinned by `bare_cjs_without_exports_stays_unknown`). Measured on express: **resolved 4154 → 4152, gained 0, lost 2.** Both removed bindings are verified phantoms — a local `id` in `examples/route-middleware` and `test/app.router.js` binding to `examples/mvc/controllers/user/index.js`'s module-private `id`. Pure precision, no recall cost. `m0025` is the extraction re-heal (stat+hash invalidated, m0021/m0022 pattern), because visibility is recorded at parse time and existing DBs keep their stale `unknown` rows otherwise. One subtlety that produced a wrong result before I caught it: the export set must contain only names another file can refer to a symbol **by**, not every value that escapes. express writes `exports.request = req`; collecting the right-hand identifier made the private `req` `Exported` again and resurrected the very phantom class this removes. **Keys only.** ### Part 2 — require() capture — implemented, measured, REVERTED It worked, in every binding form, and correctly declined computed/interpolated specifiers: ``` main.js:1 ./helper alias=helper plain main.js:2 ./other alias=thing shorthand destructure main.js:2 ./other alias=renamed renamed destructure main.js:3 ./arr alias=first array pattern main.js:4 ./deep alias=deep member on the call main.js:5 ./sideeffect alias=(none) bare statement main.js:9 ./static alias=plain static template require(someVar) -> correctly SKIPPED require(`./x${y}`) -> correctly SKIPPED ``` express: **imports 0 → 388**. And then: **resolved +6 / −2, and all six gained are PHANTOMS.** `lib/application.js:221` binds a local `fn` into `test/utils.js#fn` — a production file into a test file. Root cause is upstream of the capture: `require("./utils")` from `lib/` stem-matches **both** `lib/utils.js` and `test/utils.js`, because tier 1b's file-key arm compares stems and ignores relative-path semantics. Landing correct import rows simply widens reachability through that existing hole. Net +4 phantoms against the guarantee that is this product's core claim, so it is not shippable. The code is reverted rather than left dead. It is straightforward to restore once relative specifiers are path-resolved. ### Correcting my own numbers in this issue Two things I asserted above are **wrong** and should not be relied on: 1. **"internal-eligible rate is 35%, a large share plausibly reachable if the module graph existed."** Too optimistic. The dominant CommonJS shape is `util.method()` — a member call through a module object — which needs **receiver typing**, not an import edge. A minimal 2-file probe (`const util = require("./lib/util"); util.uniqueHelperFn(1)`) still does **not** resolve with imports captured. The realistic upside is single digits, not thousands. 2. **An intermediate claim of "25 pre-existing phantoms eliminated"** was an artifact of a bug in my own patch (`visibility_for` was handed the *declaration* node, which has no `name` field, so every `var` looked up as `""` and got File-scoped regardless of export status). With correct semantics those phantoms remain. Only 2 are removed. ### Acceptance status - [ ] `require()` / dynamic `import()` captured — **implemented, reverted, blocked** on relative-specifier resolution - [x] destructured requires produce one row per binding (proven, in the reverted work) - [x] `phantom_count == 0` across all 7 languages; mutation guard 0 rebinds / 7 056 sites - [ ] js-express `resolved` improves — **it decreases by 2, and that is the correct direction** (phantom removal) - [x] baseline re-blessed, reason recorded in the file; the ratchet caught the change first (`resolved 4154 -> 4152 (-2)`) - [x] re-heal migration, extraction pattern, with a test that pins both invalidations - [x] hermetic tests; two existing contract tests deliberately flipped with the reasoning inline ### Follow-up to file **Tier 1b's file-key arm should path-resolve relative specifiers.** `./utils` from `lib/` must mean `lib/utils.js`, not any file whose stem is `utils`. That unblocks part 2 and is very likely removing phantoms in the other languages too.

buildagent referenced this issue

2026-07-29 18:57:38 +02:00

bug: tier 1b stem-matches relative import specifiers — require("./utils") from lib/ reaches test/utils.js #57

dhoyer referenced this issue from a commit

2026-07-29 21:25:54 +02:00

fix: analyze the CommonJS export surface (#56 part 1, precision)

dhoyer referenced this issue from a commit

2026-07-29 21:25:54 +02:00

fix: m0025 re-heal for #56 part 1, and make the #55 ratio test

dhoyer referenced this issue from a commit

2026-07-29 21:25:54 +02:00

feat: capture CommonJS require() as imports (#56 part 2, re-landed)

buildagent referenced this issue

2026-07-29 21:26:30 +02:00

I039: OSS corpus test system + the four defects it found (v0.9.0, schema v25) #58

buildagent referenced this issue from a commit

2026-08-03 14:06:57 +02:00

I039: OSS corpus test system + resolver fixes (v0.9.0, schema v25) (#58)

buildagent commented

2026-08-05 10:15:22 +02:00

Author

Member

Fixed and released in v0.9.0 (schema v25, migration m0025_commonjs_module_graph — an extraction re-heal, so it re-parses on first run).

Both halves shipped: the export-surface analysis (part 1) and require() import capture (part 2). express went from 410 require() calls and 0 imports to a real module graph.

Part 2 has some history worth recording, since it landed twice:

First landing measured +6 phantoms and was reverted rather than shipped.
Root cause was #57 (relative specifiers were stem-matched, so require("./utils") from lib/ could reach test/utils.js). #57 was fixed first, then part 2 re-landed with zero phantoms.
Two bugs found on the way: visibility_for was handed the declaration node, which has no name field, so every var looked up as "" — this also produced a false "25 phantoms eliminated" claim that had to be retracted. And exports.request = req resurrected a phantom by collecting the right-hand identifier, which marked private req as Exported; collect_cjs_exports now collects keys only.

Verified live: the released binary indexes require('./util') as module=./util, alias=helper with 0 phantoms.

Fixed and released in **v0.9.0** (schema v25, migration `m0025_commonjs_module_graph` — an extraction re-heal, so it re-parses on first run). Both halves shipped: the export-surface analysis (part 1) and `require()` import capture (part 2). express went from **410 `require()` calls and 0 imports** to a real module graph. Part 2 has some history worth recording, since it landed twice: 1. First landing measured **+6 phantoms** and was **reverted** rather than shipped. 2. Root cause was #57 (relative specifiers were stem-matched, so `require("./utils")` from `lib/` could reach `test/utils.js`). #57 was fixed first, then part 2 re-landed with **zero phantoms**. 3. Two bugs found on the way: `visibility_for` was handed the *declaration* node, which has no `name` field, so every `var` looked up as `""` — this also produced a false "25 phantoms eliminated" claim that had to be retracted. And `exports.request = req` resurrected a phantom by collecting the right-hand identifier, which marked private `req` as Exported; `collect_cjs_exports` now collects **keys only**. Verified live: the released binary indexes `require('./util')` as `module=./util, alias=helper` with 0 phantoms.

buildagent referenced this issue

2026-08-05 10:15:29 +02:00

bug: tier 1b stem-matches relative import specifiers — require("./utils") from lib/ reaches test/utils.js #57

buildagent closed this issue

2026-08-05 10:15:41 +02:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

h-dv/code-index#56

No description provided.

Rows
Columns

bug: the JS/TS plugin never captures CommonJS require() — express's entire module graph is invisible (410 calls, 0 imports) #56

The defect

Reproduction

Scale of the impact

Why this matters for #46

Why I did not just fix it

Acceptance

Severity: medium

Part 1 shipped (4a98948 + 9bf792f). Part 2 BLOCKED with evidence — and my original estimate here was wrong.

What shipped: the CommonJS export surface (precision)

Part 2 — require() capture — implemented, measured, REVERTED

Correcting my own numbers in this issue

Acceptance status

Follow-up to file

Part 1 shipped (`4a98948` + `9bf792f`). Part 2 BLOCKED with evidence — and my original estimate here was wrong.