fix(extract): stop cross-project edges from platform imports and shared npm deps (#3237) - #3262
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 1 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Routes JVM/Android platform and Kotlin-stdlib imports (roots like java., android., kotlin.) to the external ref namespace via _JVM_PLATFORM_PACKAGE_ROOTS in _import_java and _import_kotlin, so their bare last-segment targets can no longer byte-collide with unrelated repo nodes or ride the alias index onto same-stem files in a monorepo; unresolvable platform imports drop as external instead of dangling. Mints package.json dependency entries as a shared type="module" node under _make_id("ref", key) in extract_json, so two manifests naming the same npm package collapse to one anchor and JS bare specifiers land on it rather than being joined across projects. Exempts equal-labelled module-vs-module id collisions from the collision warning in _report_id_collision since those shared anchors collapse losslessly by design.
Worth a look
- package.json dependency node ids changed from bare package ids to ref-prefixed ids —
graphify/extractors/json_config.py:200· 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 — 2000 functions depend on the 304 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 524 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 62 callers, 13 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 123 callees - new:
build()— 42 callers, 4 callees - …and 31 more — each is listed as a finding
Verification — 2000 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: 1787 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_report\_id\_collision (not a proof).
The verifier ran both versions of \_report\_id\_collision 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 \_import\_java.
The verifier did not have enough to check \_import\_java, 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)
Could not verify: Could not verify \_import\_kotlin.
The verifier did not have enough to check \_import\_kotlin, 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)
Could not verify: Could not verify extract\_json.
The verifier did not have enough to check extract\_json, 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
· 39 more finding(s) on lines outside this diff (see the check run).
There was a problem hiding this comment.
Pull request overview
This PR addresses monorepo cross-project edge pollution caused by name-colliding import targets and shared npm dependency nodes, by introducing a non-collidable external ref namespace for platform imports and registry package anchors.
Changes:
- Namespace JVM/Android/Kotlin platform import targets under
refto prevent collisions with repo-local nodes during build-time alias remapping. - Namespace
package.jsondependency targets underrefand mark them astype="module"to model registry packages as shared external anchors. - Add an end-to-end regression test suite covering the reported collision scenarios and expected resolution behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_monorepo_import_collisions.py | Adds regression tests reproducing and preventing cross-project binding via platform imports and shared npm deps. |
| graphify/extractors/json_config.py | Changes dependency target node IDs to ref_* and marks them as type="module" to create a shared registry anchor node. |
| graphify/extract.py | Routes JVM/Android/Kotlin platform imports to ref_* targets to prevent repo-collidable bare-stem IDs. |
| graphify/dedup.py | Suppresses collision warnings for intentional type="module" duplicates with the same normalized label. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| add_node(dep_nid, key, line, file_type="concept", | ||
| node_type="module") |
There was a problem hiding this comment.
Kept file_type="concept" deliberately: it's been this node's classification since it was introduced (0.9.55's json_config fix kept it too), and the node models an external registry package, not repo code. The anchor behavior doesn't ride on file_type — imports bind by node id, and both the disambiguation and dedup exemptions gate on type == "module" + label only. Switching to code would surface npm packages in code-centric views, which seems like the wrong trade for this PR's scope.
v8 independently landed the ref-namespaced dependency id and re-sourced the dep edge at the manifest file (Graphify-Labs#1764 self-loop fix), so the shared-anchor test now asserts both manifests wire to the one type=module node from their file nodes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
3dc2d31 to
eee8284
Compare
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.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Routes JVM/Android/Kotlin-stdlib platform imports (roots like java., android., kotlin.) to the external ref namespace instead of a bare last-segment id, so import java.util.UUID no longer byte-collides with an npm uuid dependency node or rides build's alias index onto an unrelated same-stem file. Stamps package.json dependency ref nodes with type="module" so manifests declaring the same package share one anchor under the #1327 module exemption, and suppresses the id-collision warning in _report_id_collision for equal-labelled module-vs-module survivors. Repo-local FQN imports still resolve to their real node.
Worth a look
- Absolute source paths make cross-project direct-edge check ineffective —
tests/test_monorepo_import_collisions.py:232· 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 — 2089 functions depend on the 316 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 548 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
deduplicate_entities()— 71 callers, 22 callees - new:
build_merge()— 62 callers, 13 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
build()— 42 callers, 4 callees - …and 34 more — each is listed as a finding
Verification — 2089 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: 1876 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_report\_id\_collision (not a proof).
The verifier ran both versions of \_report\_id\_collision 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 \_import\_java.
The verifier did not have enough to check \_import\_java, 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)
Could not verify: Could not verify \_import\_kotlin.
The verifier did not have enough to check \_import\_kotlin, 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)
Could not verify: Could not verify extract\_json.
The verifier did not have enough to check extract\_json, 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
· 42 more finding(s) on lines outside this diff (see the check run).
Closes #3237 (classes 1 and 2). Rebased onto
v8(0.9.55).The problem
Two mechanisms produced confident (EXTRACTED) cross-project edges between symbols that merely share a name — 20 of the 25 cross-project edges the reporter audited:
JVM/Android platform imports emit a repo-collidable bare-stem target.
_import_kotlin/_import_javatarget_make_id(last_segment). Forimport java.util.UUIDthat byte-collides with any unrelated node collapsing touuid, and even with nothing owning the bare id it can ride build.py's pre-migration alias index (Silent data loss: doc files with the same name in different directories produce colliding node IDs across extraction chunks #1504) onto whichever unrelated same-stem file uniquely claims it —import android.graphics.Colorin project A binds to project B'sui/theme/Color.ktfile node. Reproduces on 0.9.55 from a four-file corpus.N manifests declaring the same npm package still fragment. With 0.9.55's
ref_<pkg>ids, two package.json files now mint the same id from different files, so_disambiguate_colliding_node_idssalts them apart per file and dedup prints a "minted by two different files … dropping" WARNING — for a collapse that is exactly the intent (one registry package).The change
java.,javax.,jakarta.,kotlin.,kotlinx.,android.,androidx.,dalvik.) target_make_id("ref", fqn)instead of the bare stem — the same cure_resolve_js_import_targetapplies to unresolvable JS imports (Unresolved bare npm import gets aliased onto an unrelated same-named local file (cross-language phantom edge) #1638). These roots are never repo-local, so the Kotlin: _KOTLIN_CONFIG declares node typeimport_header, bundled grammar emitsimport— every Kotlin imports edge is silently dropped (45% -> 95% dep recall with a 2-line fix) #2526 corpus resolver could not have rewritten them anyway; the ref target matches no repo node and build drops the edge as an external import, the behavior every other external import already has. Repo-shaped imports keep the bare stem and the Kotlin: _KOTLIN_CONFIG declares node typeimport_header, bundled grammar emitsimport— every Kotlin imports edge is silently dropped (45% -> 95% dep recall with a 2-line fix) #2526 repo-local rewrite is untouched (control test).type="module", modeling the registry package as one shared external anchor — the issue's own suggestion. That puts it under the Swiftimportsedges are silently dropped from the final graph #1327 module-anchor exemption in_disambiguate_colliding_node_ids, so N manifests share ONE node; a matching exemption in dedup's collision reporter silences the "minted by two different files" warning for identical-label module anchors, whose collapse is the intent, not a loss. (file_typestaysconcept, its classification since the node was introduced — binding is by id, and both exemptions gate ontype.)A side effect worth having: a TS
import { v4 } from "uuid"(which targets_make_id("ref", "uuid")since #2457) lands on the manifest's dependency node instead of dangling, so code links to the manifest that declares the package.Class 3 (minified-bundle symbol nodes) is a separate detection question and is not touched here; the non-imported
Result-supertype case in class 1 is the type-stub fragmentation tracked in #3252.Tests
tests/test_monorepo_import_collisions.py— 7 tests: the UUID byte-collision and the Color alias-ride (built end-to-end from the issue's shapes) no longer cross projects; a repo-local Kotlin FQN import still resolves (#2526 control); Kotlin and Java platform imports emitref_-namespaced targets while repo-shaped imports keep the bare stem; both manifests wire to exactly one sharedtype=moduleregistry node with no direct project-to-project edge; and the TS bare-specifier→manifest bind exists. Against 0.9.55 without this change, 4 of 7 fail (the other three cover the ground 0.9.55's json_config fix already holds, kept as regression guards). Related suites unchanged; the full suite matches a fresh same-versionv8baseline.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q