Skip to content

feat(merge): link a type declaration two repos share - #3008

Closed
durmazoguzhan wants to merge 1 commit into
Graphify-Labs:v8from
durmazoguzhan:feat-cross-repo-shared-types
Closed

feat(merge): link a type declaration two repos share#3008
durmazoguzhan wants to merge 1 commit into
Graphify-Labs:v8from
durmazoguzhan:feat-cross-repo-shared-types

Conversation

@durmazoguzhan

Copy link
Copy Markdown
Contributor

Closes #3007.

merge-graphs prefixes every node id with its repo tag, so a contract type both repos declare arrives as two unconnected nodes:

p_CatalogService::...syncproductupserttosearchevent         <- the producing method references it
p_ElasticsearchService::...syncproductupserttosearchevent    <- ProductUpsertConsumer references it

Same namespace, same name, nothing between them, so a walk from the producer to the consumer finds no route and one flow reads as two unrelated halves.

The merge now adds an edge between sourced type declarations that agree on namespace and name and come from different repos:

p_CatalogService::...event  --same_type_as-->  p_ElasticsearchService::...event

Edges, not node merging. Two repos can hold copies of a contract that have drifted, and collapsing them would hide that, while a link lets the traversal cross with each side keeping its own members, file and provenance. That also keeps it clear of #296 and #207, which are about merging duplicates rather than relating them.

Why namespace plus name

I expected to need something structural and measured first. Between two .NET services with 1440 and 262 sourced type declarations:

  • 7 pairs agree on namespace and name
  • all 7 are the shared contracts: EventManager.Models.ClearCacheEvent, IndexCompletedEvent, PrepareElasticDataEvent, SyncPriceToSearchEvent, SyncProductDeleteToSearchEvent, SyncProductUpsertToSearchEvent, UpdateVariantElasticPublishedEvent
  • nothing else matches, so there is no false pair to weigh against them

Keying on the bare name is what makes this unsafe, and it fails at once: both services carry their own Settings and Configuration. The namespace is doing the work, and the test suite pins that.

What it buys

On that pair the pass adds 7 edges and the producer to consumer walk becomes three hops:

.UpdateProductOnSearchAppAsync()  (CatalogService)
  -> SyncProductUpsertToSearchEvent  (CatalogService)
  -> SyncProductUpsertToSearchEvent  (ElasticsearchService)   [same_type_as]
  -> ProductUpsertConsumer           (ElasticsearchService)

The last hop is a reverse traversal, since the consumer references the event rather than the other way round, so this shows up on an undirected walk. That direction comes from references and is not something this pass changes.

The producer's first edge comes from #2997. Without that fix the CatalogService half of this walk does not exist, so the two are worth taking together, though neither depends on the other to build.

Tests

tests/test_cross_repo_shared_types.py, 6 tests through the CLI the way test_merge_graphs_cli.py does: the cross-repo link itself, the same name in different namespaces staying unlinked, two declarations inside one repo staying unlinked, a type with no namespace staying unlinked, non-type nodes staying unlinked, and a sourceless stub staying unlinked.

$ uv run pytest tests/test_cross_repo_shared_types.py -q
6 passed

$ uv run pytest tests/test_merge_graphs_cli.py tests/test_global_graph.py tests/test_serve.py -q
170 passed

Full suite excluding tests/test_skillgen.py: 4822 passed, 1 failed. The failure is test_labeling.py::test_label_communities_batches_when_over_batch_size, which fails the same way on a clean v8 checkout here (4816 passed, 1 failed) and passes on its own in both, so it is order dependent rather than mine. test_skillgen.py fails here on clean v8 too, which is why it is out of that run. ruff check clean on the three files.

Two things I would change on request

The pass runs unconditionally in merge-graphs. A --no-shared-types opt-out is easy to add if you would rather this be opt-in, the way --no-dedup works.

If the cluster graphs in #2134 land, the same pass belongs in cluster build. The two do not compete: cluster links model connections that need declaring, such as an API call with no shared symbol, and this derives the ones both repos already name.

)

merge-graphs prefixes every node id with its repo tag, so a contract type both
services declare becomes two unconnected nodes. On a message bus that is the hop
worth having: the producer names the message type in one repo, the consumer
implements IConsumer<T> on it in the other, and the merged graph joins neither.

The merge now adds a `same_type_as` edge between sourced type declarations that
agree on namespace and name and come from different repos. Edges rather than node
merging: two repos can hold copies of a contract that have drifted, and
collapsing them would hide that, while a link lets a traversal cross with each
side keeping its own members, file and provenance.

Namespace agreement carries the precision. Between two .NET services with 1440
and 262 declared types, 7 pairs match on namespace and name and all 7 are the
shared EventManager.Models contracts, with nothing else matching. Keying on the
bare name instead would immediately pair each service's own Settings and
Configuration classes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NCycXc9pjGzovxrZqojPUE

@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. No changes could be formally verified in this run.


Graphify review — findings

Adds a same_type_as cross-repo linking pass: new graphify/cross_repo_types.py with link_shared_type_declarations, wired into dispatch_command's merge-graphs path to connect sourced type declarations that share namespace and name across different repos (edges only, no node merging). Includes tests in test_cross_repo_shared_types.py covering the match, and the non-matches (same name different namespace, same-repo duplicates, missing namespace, non-type nodes, sourceless stubs).

No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 250 functions depend on the 64 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 120 callees
  • new: _stale_graph_sources() — 7 callers, 6 callees
  • new: _run_hook_guard() — 4 callers, 7 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

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

Formal verification

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

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

@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.49 via authorship-preserving cherry-pick so you keep contributor-graph credit. Thanks @durmazoguzhan! Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.49

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.

merge-graphs: a contract type both repos declare stays two unconnected nodes

2 participants