fix(ts): ignore non-runtime dynamic import text - #3343
Conversation
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Replaces the JS/TS dynamic-import rescue's regex-plus-//-guard with an AST-backed filter: candidate import('…') matches are kept only when tree-sitter parses that byte offset as a runtime call_expression to import, so occurrences in comments, strings, regexes, and TypeScript typeof import(...) type positions no longer create dependency edges while live template interpolations like `${import('./x')}` still do. _js_runtime_import_starts picks the grammar by file suffix and returns an empty set on any parse failure, degrading to emitting no rescued edges rather than false positives. Switches the scan to byte-level parsing/matching to keep offsets aligned with tree-sitter.
Worth a look
- Dynamic import rescue now fails closed when parser unavailable —
graphify/extract.py:1488· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Parse failure now silently drops all dynamic-import rescue edges —
graphify/extract.py:1489· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1766 functions depend on the 255 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 523 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 123 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 24 more — each is listed as a finding
Verification — 1766 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1601 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_rescue\_js\_dynamic\_imports.
The verifier did not have enough to check \_rescue\_js\_dynamic\_imports, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 31 more finding(s) on lines outside this diff (see the check run).
| return starts | ||
|
|
||
|
|
||
| def _rescue_js_dynamic_imports(path: Path, result: dict) -> None: |
There was a problem hiding this comment.
_rescue_js_dynamic_imports()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
This coupling delta is confined to the rescue helper’s grammar selection and iterative tree-sitter walk.
The helper runs only after the byte matcher finds candidates, and the reparse is required to align those byte offsets with runtime call nodes while excluding comments, strings, regexes, and type contexts.
No unrelated extraction refactor was added; the existing normal-path and parser-failure regressions cover the behavior described above.
|
Addressed the two parser-failure findings from review I reproduced the loss of a valid module-scope rescue edge when the validator failed. The helper now distinguishes validator failure from an empty runtime-import set, preserves the prior guarded lexical rescue, and emits a Added a regression that simulates TypeScript grammar initialization failure and verifies the valid edge is retained. The normal false-positive coverage remains in place. Candidate |
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Validates JS/TS dynamic-import rescue candidates against a real tree-sitter parse so import('…') in comments, strings, regexes, and TypeScript typeof import(...) type positions no longer create phantom dependency edges, while template interpolations like ${import('./x')} still resolve. When the grammar can't load or parse, _rescue_js_dynamic_imports emits a RuntimeWarning and falls back to the old //-prefix-guarded lexical rescue rather than dropping edges. The rescue now works on raw bytes throughout to keep offsets aligned with the parsed tree.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1768 functions depend on the 257 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 523 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 123 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 24 more — each is listed as a finding
Verification — 1768 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1603 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_rescue\_js\_dynamic\_imports.
The verifier did not have enough to check \_rescue\_js\_dynamic\_imports, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 31 more finding(s) on lines outside this diff (see the check run).
| return starts | ||
|
|
||
|
|
||
| def _rescue_js_dynamic_imports(path: Path, result: dict) -> None: |
There was a problem hiding this comment.
_rescue_js_dynamic_imports()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
I checked the coupling increase in _rescue_js_dynamic_imports(). The added validator call is needed to distinguish real runtime imports from comment/string text, and the warning makes the parser-failure fallback visible. Grammar selection and AST traversal are already isolated in _js_runtime_import_starts(); the existing resolution and deduplication logic stays in the rescue function. I am keeping this boundary because moving calls behind another wrapper would hide the measured dependency without removing it. The 19 dynamic-import tests and 284 related tests passed, and the Graphify check passed on b31cc43.
1 participant
The JavaScript dynamic import rescue matcher treated import-like text inside comments, strings, template text, regex literals, and TypeScript type contexts as executable dynamic imports.
That produced dependency edges for code that never runs.
The rescue path now reparses candidate files with tree-sitter and keeps only runtime import call nodes.
The byte matcher remains lazy, computes one source-offset set per file, and uses iterative traversal.
33362d9: false and real dynamic edges, 6 nodes and 5 edges.b31cc43: real dynamic edge only, 6 nodes and 4 edges. The after screenshot is captured on this head; the follow-up also preserves valid edges when parser validation fails.Verification on candidate
b31cc43:tests/test_js_dynamic_imports.py: 19 passed.git diff --checkpass.RuntimeWarningdescribing the degraded fallback.When tree-sitter cannot validate candidates, the implementation preserves the prior guarded lexical rescue so valid edges are retained and surfaces the precision tradeoff explicitly in a warning; normal parser-success runs continue to filter non-runtime text.
The full suite was not rerun after the follow-up.
An earlier broader run before this follow-up reported 5,175 passed, 92 skipped, and 12 environment or pre-existing failures involving Unix socket permissions, an unavailable OpenAI dependency, and blocked DNS.
The portable fixture and reproduction command are pinned in the separate evidence commit.