fix: resolve named re-exports through imported bindings - #3358
fix: resolve named re-exports through imported bindings#3358VasuBansal7576 wants to merge 2 commits into
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 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Resolves re-export edges through the shared import/export facts so a barrel that forwards an imported-then-exported binding (import {x}; export {x}) points at the originating declaration rather than the intermediate file. Adds exported_candidates, which walks export origins and star branches and only repoints an edge when exactly one owned candidate survives, so cycles, conflicting named/star exports, and unextracted bindings stay unresolved instead of collapsing to a wrong-but-unique target. Also broadens exported-declaration name collection to cover variable_declaration (not just lexical_declaration), and adds a suite covering renamed forwarding, type-only metadata preservation, and ambiguity cases.
Worth a look
- exported_candidates recursion uses
or {origin}masking empty legitimate results —graphify/extractors/resolution.py:1058· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- node_sources lookup can KeyError when target_id maps to a node without source_file —
graphify/extractors/resolution.py:1097· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Exported destructuring bindings are no longer reported —
graphify/extractors/resolution.py:1295· 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 — 1743 functions depend on the 144 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 540 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
_extract_generic()— 18 callers, 25 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - …and 34 more — each is listed as a finding
Verification — 1743 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: 750 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.
The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, 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 `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_js\_exported\_declaration\_names.
The verifier did not have enough to check \_js\_exported\_declaration\_names, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
· 42 more finding(s) on lines outside this diff (see the check run).
|
Checked all three findings against the pinned upstream baseline and executable fixtures.
Validation: 124 tests passed, including five new review controls and all existing forwarding/ambiguity, symbol-resolution, and JS import-resolution tests. |
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
Resolves re_exports edge targets through the shared import/export facts so an authored barrel re-export points at the real definition — including import {x}; export {x} bridges with no local declaration — by walking export origins with exported_candidates and only rewriting when a single owned origin resolves. Distinguishes a star-export cycle cutoff from a genuinely absent export, and records explicit exports whose bindings extraction never materialized in export_origins so they still count as evidence rather than being dropped. Extends _js_exported_declaration_names to name identifier bindings in variable_declaration nodes alongside the preserved legacy aggregate-pattern facts.
No blocking issues surfaced. 8 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1750 functions depend on the 151 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 542 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
_extract_generic()— 18 callers, 25 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - …and 34 more — each is listed as a finding
Verification — 1750 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: 757 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.
The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, 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 `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_js\_exported\_declaration\_names.
The verifier did not have enough to check \_js\_exported\_declaration\_names, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
· 42 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.55 — landed on |
2 participants
api.tsre-exporting a binding frombridge.tscurrently points at a nonexistent bridge-local symbol when the bridge imports that binding and then exports it.For
base.ts: export const VALUE = 1,bridge.ts: import { VALUE } from './base.js'; export { VALUE }, andapi.ts: export { VALUE } from './bridge.js', the raw graph containsapi -> bridge_valueeven though onlybase_valueexists.Resolve that exact named-export site through explicit export origins and rewrite it only when one origin has an owned declaration.
Keep unresolved explicit origins in the candidate set, so unsupported extraction cannot hide ambiguity.
Branches without that export contribute nothing; explicit named exports override star branches.
Record simple identifier export declarations so exported constants can be distinguished from private symbols.
The existing edge and source location are preserved.
Its transient
target_filefollows the defining source for ID disambiguation and is removed before graph persistence.No destructured-binding implementation or edge deletion is added.
Validation:
git diff --checkpassed.graphify update . --no-clustercompleted. Optional SQL/DM/Robot grammars were unavailable, and the existing Luau fixture emitted a syntax warning.Review follow-up at
0a1f06be8b482505a1c7399686b50a9ddff11457fixes valid-cycle reachability, restores baseline aggregate-pattern declaration facts, and derives transient target-file provenance from the resolved origin.Five added controls cover short/long cycles, source-less nodes, aggregate-pattern compatibility, and identifier aliases.
The linked raw evidence and offline visual remain the original
08dd8c1proof; no new viewer capture is claimed.