Skip to content

fix(ts): preserve runtime dynamic imports in comparisons - #3220

Open
zfaustk wants to merge 1 commit into
Graphify-Labs:v8from
zfaustk:fix/ts-import-runtime-normalizer-3210
Open

fix(ts): preserve runtime dynamic imports in comparisons#3220
zfaustk wants to merge 1 commit into
Graphify-Labs:v8from
zfaustk:fix/ts-import-runtime-normalizer-3210

Conversation

@zfaustk

@zfaustk zfaustk commented Aug 30, 2026

Copy link
Copy Markdown

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=short

Result: 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

  • This PR introduces breaking changes and requires careful review.

Attribution

N/A

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the review guidelines
  • User-facing text is internationalized using the nls service (no new user-facing text introduced in this change)

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment thread graphify/extract.py
return False


def _normalize_ts_import_types(source: bytes, *, tsx: bool = False) -> bytes | None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression_normalize_ts_import_types()

high coupling complexity (Ca·Ce = 25).

Grounded coupling-delta finding (deterministic), not an LLM guess.

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.

TS: #3154 import(...) normalizer erases runtime dynamic imports; with import ("…") the dependency is lost entirely (regression in 0.9.52)

1 participant