fix(dart): resolve import/export uris to file nodes (#2329) - #2570
fix(dart): resolve import/export uris to file nodes (#2329)#2570pedroteixeira wants to merge 3 commits into
Conversation
Dart import edges were created but never resolved: the target was the raw
uri string as a node with `source_file: None`, so nothing connected it to
the file it named. Reverse traversal was therefore blind -- `affected`
returned "No affected nodes found" for a file whose importer is visible on
line 5 of the source -- while Python and TypeScript resolved, because both
have a resolver and Dart had none.
Measured on a real Flutter app (433 files under lib/, graphify 0.9.37):
internal import/export edges resolved to a file node
before 0 / 2129 (0.0%)
after 2129 / 2129 (100.0%)
external (dart:, third-party package:) 982, unchanged
Dart resolution is deterministic -- no extension guessing, no index file,
no alias config -- so `_resolve_dart_import_target` only handles three
forms: `dart:` (always external), a relative path, and `package:x/y.dart`
(the `lib/` of the package named `x`, found by walking up to the nearest
pubspec.yaml, including the members of a pub `workspace:` block so a
monorepo import of a sibling package resolves too).
The one subtlety is worth stating, since a plain filesystem join gets it
wrong: a relative uri resolves against the importing library's OWN uri, and
under `lib/` that uri is `package:`, not `file:`. RFC 3986 drops `..`
segments that would escape the base, so from `package:app/core/data/x.dart`
an import of `../../../features/y.dart` clamps at the package root and
resolves to `package:app/features/y.dart`. That is live, compiling Dart --
it appeared in 3 files of the corpus above -- and a filesystem join reports
it as a miss. Outside `lib/` (bin/, tool/) the base really is a file uri, so
those keep the unclamped join.
Unresolved uris keep their existing external-node behaviour untouched, so
this changes nothing for third-party packages or the SDK.
Adds 8 tests; 5 of them fail without the dart.py change.
Co-Authored-By: Claude Opus 5 (1M context)
Claude-Session: https://claude.ai/code/session_01RV2wTmCk7ZJs531DF9KeW8
![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.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds Dart import/export URI resolution to the extractor. A new _resolve_dart_import_target helper (plus supporting pubspec/workspace parsing and package-index caching) is introduced in resolution.py to translate package:, relative, and SDK URIs into on-disk file paths, and dart.py is updated to use it so import/export edges point at resolved file node ids instead of bare string nodes. A new TestDartImportResolution test class and related test changes exercise these resolution paths. The stated intent is to make Dart import edges resolve to the imported file's node id so reverse traversal ("who imports this file") works, matching existing Python/TypeScript behavior (#2329). The changed-symbols list also references various JS/TS/Python resolution and PascalCase helpers, but the diff surface shown is centered on the Dart extractor, the new Dart resolution logic, and its tests.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1363 functions depend on the 165 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract_dart()— 9 callers, 7 callees - new:
_resolve_dart_import_target()— 4 callers, 3 callees
Verification — 1363 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: 637 function(s) in the blast radius were not formally verified this run
· 2 grounded finding(s) anchored inline below.
| def _resolve_dart_import_target(raw: str, str_path: str) -> "Path | None": | ||
| """Resolve a Dart `import`/`export`/`part` URI to a real file on disk. | ||
|
|
||
| Dart resolution is fully deterministic — there is no extension guessing, no | ||
| index file, and no alias config, so only three forms exist: | ||
|
|
||
| 'dart:async' SDK, always external -> None | ||
| '../cell/cell_widget.dart' relative to the importing file | ||
| 'package:app/x/y.dart' `lib/x/y.dart` of package `app` | ||
|
|
||
| Returns the resolved path, or None when the URI names something outside the | ||
| scanned corpus (the SDK, a pub dependency, a dangling path) — mirroring | ||
| `_resolve_c_include_path`, so the caller keeps its existing external-node | ||
| behaviour untouched. | ||
| """ | ||
| if not raw: | ||
| return None | ||
| if raw.startswith("dart:"): | ||
| return None | ||
|
|
||
| try: | ||
| start_dir = Path(str_path).parent | ||
| except Exception: | ||
| return None | ||
|
|
||
| if raw.startswith("package:"): | ||
| pkg, _, sub = raw[len("package:"):].partition("/") |
There was a problem hiding this comment.
_resolve_dart_import_target() — high coupling complexity (Ca·Ce = 12)
Graphify suggests a fix:
| def _resolve_dart_import_target(raw: str, str_path: str) -> "Path | None": | |
| """Resolve a Dart `import`/`export`/`part` URI to a real file on disk. | |
| Dart resolution is fully deterministic — there is no extension guessing, no | |
| index file, and no alias config, so only three forms exist: | |
| 'dart:async' SDK, always external -> None | |
| '../cell/cell_widget.dart' relative to the importing file | |
| 'package:app/x/y.dart' `lib/x/y.dart` of package `app` | |
| Returns the resolved path, or None when the URI names something outside the | |
| scanned corpus (the SDK, a pub dependency, a dangling path) — mirroring | |
| `_resolve_c_include_path`, so the caller keeps its existing external-node | |
| behaviour untouched. | |
| """ | |
| if not raw: | |
| return None | |
| if raw.startswith("dart:"): | |
| return None | |
| try: | |
| start_dir = Path(str_path).parent | |
| except Exception: | |
| return None | |
| if raw.startswith("package:"): | |
| pkg, _, sub = raw[len("package:"):].partition("/") | |
| def _resolve_dart_uri(raw: str, start_dir: "Path") -> "Path | None": | |
| """Resolve a non-SDK Dart URI against `start_dir`. | |
| Split out of `_resolve_dart_import_target` so that entry point stays a thin | |
| dispatcher; see its docstring for the URI forms handled here. | |
| """ | |
| if raw.startswith("package:"): | |
| pkg, _, sub = raw[len("package:"):].partition("/") |
There was a problem hiding this comment.
Adopted in fb3ba1c, essentially as suggested. _resolve_dart_import_target is now a thin dispatcher — it guards the empty and dart: cases and derives the start directory — and _resolve_dart_uri selects between _resolve_dart_package_uri and _resolve_dart_relative_uri.
I split the relative branch out as its own function rather than leaving it inline, because it carries the package:-uri clamping rule and its explanation, which is the one non-obvious part of the file; on its own it reads as a unit.
Behaviour is unchanged: same 13 tests, and the corpus in the PR description still measures 2129/2129.
| from graphify.extractors.resolution import _resolve_dart_import_target | ||
|
|
||
|
|
||
| def extract_dart(path: Path) -> dict: |
There was a problem hiding this comment.
extract_dart()
fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Acknowledged, but I don't think there's a fix here — the new callee is the change.
extract_dart previously synthesized its import targets inline (_make_id(pkg) on the raw uri string), which is precisely why they never reached a file node. Resolving them means calling a resolver, so the +1 efferent edge is the feature rather than a side effect. The alternative — inlining the resolution back into extract_dart — would keep the coupling number flat while making the function considerably larger and duplicating logic that resolution.py already owns for JS/TS, C and Lua.
For what it's worth, this is the same shape extract_objc already has: it calls _resolve_c_include_path from the same module for exactly the same reason.
Happy to restructure if you'd rather the call site sit somewhere else.
…pers Addresses the review's coupling finding on `_resolve_dart_import_target` (Ca·Ce = 12) by taking its suggestion: the entry point is now a thin dispatcher that guards the empty/`dart:` cases and derives the start directory, and `_resolve_dart_uri` selects between `_resolve_dart_package_uri` and `_resolve_dart_relative_uri`. Behaviour is unchanged -- same 13 tests, and the same 2129/2129 on the corpus the PR measures. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RV2wTmCk7ZJs531DF9KeW8
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.
Graphify review — findings
This PR adds Dart import/export URI resolution so that Dart import/export/part edges point at the resolved target file's node id instead of a bare string node. Specifically, it introduces a new _resolve_dart_import_target helper (and supporting functions for pubspec name parsing, pub workspace members, package indexing, lib/ root detection, and package/relative/SDK URI handling) in resolution.py, then wires the Dart extractor to call it when emitting import/export edges, falling back to the previous bare-node behavior when resolution fails. It also refactors the Dart extractor's import/export loop into a shared pattern loop and adds a new TestDartImportResolution test class covering the resolution paths.
Worth a look
- export/import edge target uses resolved file path id, not the file node's own id scheme —
graphify/extractors/dart.py:517· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Relative URI resolution ignores
..segments in the file's own path prefix —graphify/extractors/resolution.py· 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 — 1369 functions depend on the 171 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract_dart()— 9 callers, 7 callees
Verification — 1369 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: 643 function(s) in the blast radius were not formally verified this run
· 2 grounded finding(s) anchored inline below.
| add_edge(file_nid, _make_id(str(resolved)), kind, context="import") | ||
| else: |
There was a problem hiding this comment.
export/import edge target uses resolved file path id, not the file node's own id scheme — agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Graphify suggests a fix:
| add_edge(file_nid, _make_id(str(resolved)), kind, context="import") | |
| else: | |
| add_edge(file_nid, _make_id(_file_stem(resolved)), kind, context="import") | |
| else: |
There was a problem hiding this comment.
Measured this rather than argue it, since the finding is flagged unverified — it doesn't reproduce, and I'd rather not change the id scheme on a hunch.
I applied the suggestion verbatim (_make_id(_file_stem(resolved))), reinstalled and re-ran the corpus from the PR description (433 Dart files): 2129/2129 resolved, same as str(resolved). Both forms work, because the id-remap post-pass re-derives the canonical repo-relative form from source_file and washes the difference out.
Keeping str(resolved) for two concrete reasons rather than a preference:
- It is how the file node itself is built —
dart.py:35,file_nid = _make_id(str(path)). Matching the node's own construction is correct by definition; the other form relies on downstream normalization happening to agree. - It's the existing precedent in this repo:
objc.py:165is exactlyadd_edge(file_nid, _make_id(str(resolved)), ...), and its comment says the bare-stem id doesn't survive_disambiguate_colliding_node_idswhen a.h/.mpair exists (Bug: ObjC extractor — 4 bugs causing ~60% of relationships to be silently dropped #1475) — which argues specifically against the_file_stemform.
I also probed the one case where the two could genuinely diverge, since I call .resolve() and the node doesn't: a corpus reached through a symlink. Still 2129/2129.
Happy to switch if there's a scenario I haven't constructed — I just couldn't produce one where the current form is the weaker of the two.
| from graphify.extractors.resolution import _resolve_dart_import_target | ||
|
|
||
|
|
||
| def extract_dart(path: Path) -> dict: |
There was a problem hiding this comment.
extract_dart()
fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…rmalized The review flagged, as an unverified advisory, that relative resolution might ignore `.`/`..` in the importing file's OWN path prefix. It does not -- both the `lib/` root and the start directory are resolved before the relative walk -- but the invariant was only implicit, so this pins it: the clean path and one carrying `zzz/..` must resolve identically, for an exact import and for one that clamps. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RV2wTmCk7ZJs531DF9KeW8
|
Both advisory findings from the The second one — "Relative URI resolution ignores It doesn't reproduce. Both
That also explains why the The invariant was implicit, though, which is a fair thing for a review to flag — so 3cbc858 pins it as a test ( The repeated |
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 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This PR adds Dart import/export URI resolution so that Dart import edges point at the resolved target file's node id rather than a bare string node. Specifically, it introduces a new _resolve_dart_import_target helper (and supporting functions) in resolution.py that handle dart: SDK URIs, relative URIs (with package-root clamping of .. under lib/), and package: URIs — including reading pubspec names and pub workspace members to build a cached package index. The dart.py extractor is updated to call this resolver for imports/exports, and test_dart.py gains a new TestDartImportResolution test class covering these resolution paths.
Worth a look
- Relative Dart URI resolution never applies '.' file-relative base inside lib/ —
graphify/extractors/resolution.py· Escalate · high- 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 — 1371 functions depend on the 173 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract_dart()— 9 callers, 7 callees
Verification — 1371 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: 645 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
| from graphify.extractors.resolution import _resolve_dart_import_target | ||
|
|
||
|
|
||
| def extract_dart(path: Path) -> dict: |
There was a problem hiding this comment.
extract_dart()
fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Fixes #2329.
Dart import edges were created but never resolved to a file node: the target was the raw uri as a node with
source_file: None, so nothing connected it to the file it named. Reverse traversal was blind —affectedreturns "No affected nodes found" for a file whose importer is visible on line 5 — while Python and TypeScript resolved, because both have a resolver inextractors/resolution.pyand Dart had none.Measurement
Real Flutter app, 433 files under
lib/, graphify 0.9.37,GRAPHIFY_FORCE=1 graphify . --code-only, counted with the script from #2329:dart:, third-partypackage:)Reverse traversal works as a result — "most depended on" now answers, and the answer is the shape you'd expect for that codebase (l10n bundle, theme tokens, auth repository, the top-level providers).
Approach
Dart resolution is deterministic — no extension guessing, no index file, no alias config — so
_resolve_dart_import_targethandles exactly three forms:dart:async→ always external../cell/cell_widget.dart→ relative to the importing librarypackage:app/x/y.dart→lib/x/y.dartof the package namedapp, found by walking up to the nearestpubspec.yaml; members of a pubworkspace:block are indexed too, so a monorepo import of a sibling package resolves as well as the file's own packagedart.pythen emits the edge on the resolved file's id, mirroring howobjc.pyalready calls_resolve_c_include_path. An unresolved uri keeps its existing external-node behaviour untouched, so nothing changes for the SDK or third-party packages and the diff stays conservative.The part a filesystem join gets wrong
A relative uri resolves against the importing library's own uri, and under
lib/that uri ispackage:, notfile:. RFC 3986 drops..segments that would escape the base, so:The first is live, compiling Dart — it appeared in 3 files of the corpus above, and the analyzer accepts it. Resolving on the filesystem alone reports a false miss on exactly those imports (99.7% instead of 100%). So the resolver walks the segments against
lib/and clamps there. Outsidelib/(bin/,tool/, a loose script) the base really is a file uri, so those keep the unclamped join — covered by its own test.Tests
8 new tests in
tests/test_dart.py, covering relative,package:self, workspace sibling, export, the clamping case, the outside-lib/case, and that SDK/third-party/dangling uris stay external. 5 of the 8 fail with thedart.pychange reverted.Full suite before and after on this branch: 24 failed / 3880 passed → 24 failed / 3888 passed — the same pre-existing failures (
test_skillgen,test_terraform,test_ollama_retry_cap,test_manifest_ingest,test_install_references), no regression.🤖 Generated with Claude Code