fix(ts): preserve runtime dynamic imports in comparisons - #3220
Conversation
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. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Replaces the regex-based TypeScript import(...) type-argument normalizer with a tree-sitter-driven pass: _normalize_ts_import_types now parses the source, uses _ts_import_is_code to skip imports in comments/strings/regex/templates, masks candidates, and rewrites only placeholders that sit inside call/new-expression type_arguments, with _ts_mask_candidate_is_malformed requiring an adjacent parse error before touching an ambiguous a < import("x") > (b) so genuine runtime comparisons are left byte-for-byte intact. Threads a tsx flag through so .tsx and Vue TSX files parse with the correct grammar, and if tree-sitter fails to load or parse the source is returned unchanged. Also loosens the dynamic-import rescue to match import ( with whitespace before the paren in both the bail check and the specifier regex.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1769 functions depend on the 253 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 520 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
extract_js()— 85 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 25 more — each is listed as a finding
Verification — 1769 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: 1604 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_js.
The verifier did not have enough to check extract\_js, 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
Could not verify: Could not verify extract\_vue.
The verifier did not have enough to check extract\_vue, 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
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
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; 32 more finding(s) on lines outside this diff (see the check run).
| return False | ||
|
|
||
|
|
||
| def _normalize_ts_import_types(source: bytes, *, tsx: bool = False) -> bytes | None: |
There was a problem hiding this comment.
_normalize_ts_import_types()
high coupling complexity (Ca·Ce = 25).
Grounded coupling-delta finding (deterministic), not an LLM guess.
What it does
Fixes #3210
The TypeScript
import(...)normalizer could mistake a runtime dynamic import between comparison operators for a type argument. In semicolon-less code this removed the function-level dependency edge; with whitespace before(it could remove the dependency entirely. The normalizer now masks only parser-identified call/new type-argument ranges and uses a narrow error-adjacency check to leave valid runtime comparisons unchanged. The rescue path accepts the same whitespace-before-parenthesis spelling.How to test
Focused local command on current
v8:uv run --frozen pytest -q tests/test_ts_import_type_arguments.py tests/test_js_dynamic_imports.py tests/test_js_dynamic_import_affected.py tests/test_vue_extraction.py --tb=shortResult: 44 passed.
An independent 48-case runtime matrix preserved both function-level and file-level edges for comparison, call, conditional, collection, arrow, and whitespace contexts.
graphify update .completed with only the known optional-parser/Luau warnings. The full local suite has 16 baseline/environment failures (skillgen historical-ref and Ollama tests); those are not claimed as passed and remain for hosted CI.Follow-ups
None identified.
Breaking changes
Attribution
N/A
Review checklist
nlsservice (no new user-facing text introduced in this change)