Skip to content

fix(extract): resolve relative JS/TS imports with a ?query or #fragment (#4860) - #4876

Closed
Jitendarmahara wants to merge 2 commits into
Graphify-Labs:v8from
Jitendarmahara:fix/js-import-query-string
Closed

Jitendarmahara wants to merge 2 commits into
Graphify-Labs:v8from
Jitendarmahara:fix/js-import-query-string

Conversation

@Jitendarmahara

Copy link
Copy Markdown
Contributor

Problem

Relative ES module imports that carry a cache-busting query string or fragment
are not resolved:

import { foo } from './lib/foo.js?v=134';

Instead of an imports_from edge to lib/foo.js, graphify creates a phantom
node (ref_lib_foo_js_v_134) and the real file gets no inbound edge. The
dependent per-symbol imports edges and cross-file calls are lost too, since
they depend on the module resolving first.

Root cause

_resolve_js_module_path (graphify/extractors/resolution.py) resolves a
relative specifier with start_dir / raw without stripping the ?query /
#fragment part, so the path never matches a real file. The Markdown extractor
already strips these (graphify/extractors/markdown.py).

Fix

Strip the ?query / #fragment from the relative specifier before resolving,
mirroring the Markdown extractor. Only the relative (.-prefixed) branch is
touched; bare and alias specifiers are unchanged.

Verification

Commands run locally (Python 3, uv):

  • uv run pytest tests/test_import_extension_resolution.py -> 45 passed
    (6 new regression tests; the 4 core ones fail without the fix, confirming the
    phantom ref_..._v_134 node, and pass with it).
  • uv run ruff check . -> clean.
  • uv run pyright -> no new errors introduced by this change (project baseline
    unchanged).
  • Full suite uv run pytest tests -> 8653 passed. The 7 failures in
    test_ext_tagged_id_clash.py are pre-existing on v8 HEAD (identical with
    this change reverted) and unrelated to this fix.

Scope / limitations

Fixes #4860

Relative ES module specifiers that carry a cache-busting query string or
fragment (`./foo.js?v=123`, `./foo.js#frag`) were resolved verbatim via
`start_dir / raw`, so the target file never matched. graphify emitted a
phantom `ref_..._v_123` node instead of an `imports_from` edge, and the
dependent per-symbol `imports` and cross-file `calls` edges were lost too.

Strip the `?query` / `#fragment` from the relative specifier in
`_resolve_js_module_path` before resolving, mirroring the existing Markdown
extractor normalization (markdown.py). Browsers and bundlers resolve the
module as if the suffix were absent. Only the relative branch is affected;
bare and alias specifiers are untouched.

Adds regression tests that fail before the change: unit coverage for the
resolver (query, fragment, both) and an end-to-end extract_js check that the
real file node receives the edge with no phantom, plus a guard that external
modules still resolve externally.

Fixes Graphify-Labs#4860
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @Jitendarmahara. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

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

No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.

Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 sampled, not proven).

Formal verification. PR-changed functions: 1/1 verified (0 proven, 1 may-equivalent, 0 distinguished) · 0 not verified.


Graphify review — findings

Fixes JS import resolution for relative specifiers carrying a ?query or #fragment suffix (e.g. ./lib/foo.js?v=134): _resolve_js_module_path now strips the suffix before resolving, so the import maps to the real file node instead of a phantom ref_..._v_134 target. The strip only runs in the relative-specifier branch, so tsconfig-alias imports with a suffix still take the old path, while bare external modules like lodash-es fall through to the external reference path as before. New tests cover query, fragment, and combined suffixes, plus the end-to-end imports_from and per-symbol imports edges for the reproducer.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 3320 functions depend on the 255 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract_js() — 97 callers, 7 callees
  • worse: _resolve_js_module_path() — 45 callers, 9 callees

Verification — 3320 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: 1397 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

34 of 469 test file(s) selected (7%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_build.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_ejs_extractor.py — impact
  • tests/test_ext_tagged_id_clash.py — impact
  • tests/test_extract.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_import_extension_resolution.py — impact, changed-test
  • tests/test_indirect_call_block_scoped_shadow.py — impact
  • tests/test_js_exported_scalar_bindings.py — impact
  • tests/test_js_member_collisions.py — impact
  • tests/test_js_walk_order.py — impact
  • tests/test_language_override_imports.py — impact
  • tests/test_languages.py — impact
  • tests/test_loose_sibling_import_resolution.py — impact
  • tests/test_multilang.py — impact
  • tests/test_package_json_subpath_imports.py — impact
  • tests/test_pascal.py — impact
  • tests/test_pascal_resolution.py — impact
  • tests/test_phantom_external_import.py — impact
  • tests/test_pr10_resolution_overrides.py — impact
  • tests/test_python_import_resolution.py — impact
  • tests/test_python_member_call_language_gate.py — impact
  • tests/test_python_parse_memoization.py — impact
  • tests/test_rationale.py — impact
  • tests/test_resolve_memoization.py — impact
  • tests/test_source_key_memoization.py — impact
  • tests/test_src_layout_import_resolution.py — impact
  • tests/test_svelte_extraction.py — impact
  • tests/test_ts_new_expression_calls.py — impact
  • tests/test_typescript_module_extensions.py — impact
  • tests/test_unmapped_at_alias_resolution.py — impact
  • tests/test_vue_extraction.py — impact
  • tests/test_walk_python_tree_iterative.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

No difference found (not proven): No behavior difference found in \_resolve\_js\_module\_path (not a proof).

The verifier ran both versions of \_resolve\_js\_module\_path 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.

· 2 more finding(s) on lines outside this diff (see the check run).

@safishamsi

Copy link
Copy Markdown
Member

Landed in v0.9.98, now on PyPI (pip install graphifyy==0.9.98). A relative JS/TS import with a ?query or #fragment suffix resolves to the real module instead of a phantom node. Thanks for the fix, @Jitendarmahara.

@Jitendarmahara

Jitendarmahara commented Oct 31, 2026 via email

Copy link
Copy Markdown
Contributor Author

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.

Relative JS imports with a query string (./foo.js?v=123) are not resolved

2 participants