Skip to content

fix(diagram): collapsed-to-collapsed edges carry no per-foreign-key style - #1545

Merged
dimitri-yatsenko merged 1 commit into
masterfrom
diagram/uniform-bundle-edges
Aug 19, 2026
Merged

fix(diagram): collapsed-to-collapsed edges carry no per-foreign-key style#1545
dimitri-yatsenko merged 1 commit into
masterfrom
diagram/uniform-bundle-edges

Conversation

@dimitri-yatsenko

@dimitri-yatsenko dimitri-yatsenko commented Aug 19, 2026

Copy link
Copy Markdown
Member

The problem

A collapsed node stands for a set of tables. An edge between two collapsed nodes therefore represents a bundle — every foreign key between the two sets of tables, drawn once. _apply_collapse built that edge like this:

if not new_graph.has_edge(new_src, new_dest):
    new_graph.add_edge(new_src, new_dest, **data)

First member wins, no aggregation. So multi (cardinality), primary (primary-vs-secondary) and aliased (renamed) — each a property of one foreign key — got attributed to a set of them, and the resulting style depended on graph traversal order.

Two schemas with identical structure, differing only in declaration order:

before:  order 'a' -> penwidth=2    style=solid
         order 'b' -> penwidth=0.75 style=dashed
after:   order 'a' -> penwidth=2    style=solid
         order 'b' -> penwidth=2    style=solid

It is stable within a process, which is why it went unnoticed: the same script always draws the same picture, but two environments can disagree, and a committed SVG is not reproducible.

The rule

An edge between two collapsed nodes claims none of the per-foreign-key properties: every bundle edge renders identically (solid, penwidth 2), in both the graphviz and mermaid paths.

This is about collapsed nodes, not schemas. A collapsed node may stand for any subset of tables — Diagram(schema).collapse() + Diagram(OneTable) leaves the unexpanded remainder as a group — and the rule applies to any edge between two such nodes.

An edge with only one collapsed end is unchanged. It still names a single table, so its cardinality and primary-vs-secondary styling stay meaningful and are preserved:

one end expanded, multi-valued FK -> penwidth=0.75 solid
one end expanded, secondary FK    -> penwidth=0.75 dashed

Why this shape

#1533 established that weight encodes cardinality and only cardinality. This follows from it: a bundle has no single cardinality to report, so it reports none. The alternative — aggregate over the bundle (thick only if every member is 1:1, dashed only if every member is secondary) — is well-defined and order-independent too, but it invents a second meaning for the same visual cues, one that holds only at the collapsed level. Uniform styling keeps one meaning per cue.

Tests

tests/integration/test_diagram_bundle_edge.py:

  • test_bundle_edges_are_uniform — a bundle mixing a primary 1:1 foreign key with a secondary one renders uniform.
  • test_bundle_style_is_order_independent — declaring the members in the opposite order changes nothing. This is the regression that would have caught the original bug.
  • test_one_collapsed_end_preserves_edge_style — an edge into an expanded table keeps its own thin weight, so the bundle rule cannot leak.

All three pass on both backends, alongside the existing cardinality guard from #1533:

$ pytest tests/integration/test_diagram_bundle_edge.py tests/integration/test_diagram_edge_weight.py -q
8 passed in 15.09s

(pip install "testcontainers[mysql]" is needed for the session fixtures; without it these error at setup rather than fail.)

Downstream

Found while adding a generator for the diagrams in datajoint/datajoint-docs#265, where the prose claimed "a heavier edge carries a larger bundle" and the figure beneath it disagreed — lab → session and session → imaging bundle two foreign keys each and rendered 0.75 and 2. That prose is corrected separately; this changes what the renderer draws.

Once this ships, collapsed figures committed in datajoint-docs need regenerating — tracked on datajoint/datajoint-docs#246, which already covers the release-time re-verification step.

…tyle

A collapsed node stands for a set of tables, so an edge between two collapsed
nodes represents a bundle: every foreign key between the two sets.
`_apply_collapse` gave that edge the attributes of whichever member it visited
first, with no aggregation, so cardinality (`multi`), primary-vs-secondary
(`primary`) and renaming (`aliased`) -- all properties of a single foreign key --
were attributed to a set of them, and the drawn style depended on graph traversal
order.

Two schemas with identical structure but opposite declaration order rendered the
same bundle as penwidth 2 solid and as penwidth 0.75 dashed. Bundle edges are now
marked and rendered uniformly (solid, penwidth 2) in both the graphviz and mermaid
paths.

This is about collapsed nodes, not schemas: a collapsed node may stand for any
subset of tables, as with `Diagram(schema).collapse() + Diagram(OneTable)`.

An edge with only one collapsed end is unchanged. It still names a single table,
so its cardinality and primary-vs-secondary styling remain meaningful and are
preserved.

Extends #1533, which established that weight encodes cardinality and only
cardinality; a bundle has no single cardinality to report. Surfaced while adding
a generator for the diagrams in datajoint-docs#265.

@MilagrosMarin MilagrosMarin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the implementation. _apply_collapse cleanly separates the two cases — both ends collapsed strips primary/multi/aliased and marks bundle, one end collapsed preserves the data untouched — and both the graphviz and mermaid paths honor it. test_bundle_style_is_order_independent genuinely reverses the declaration order rather than asserting on a single arrangement, so it's a real regression guard for the original bug.

Agreed on uniform-over-aggregate. Aggregation is well-defined, but it gives thick/dashed a second meaning that only holds at the collapsed level, and #1533's whole point was that weight means one thing. Reporting nothing is the honest read of a set that has no single cardinality.

One follow-on, not about this PR's files: bundle edges render penwidth 2 solid, which is exactly what the docs spec table assigns to "1:1 dependency" + "primary foreign key" — and that section currently says "line weight encodes cardinality, and only cardinality — it is binary," with no mention of bundles. This PR ships no docs, so the obligation lands on datajoint-docs#265's § Edge Styles. Worth a row there before this ships, so the normative table isn't contradicted by the renderer. Context disambiguates in practice (both endpoints are collapsed group boxes), so it's spec coverage rather than a rendering problem.

Approving.

@dimitri-yatsenko
dimitri-yatsenko merged commit 8e6ef31 into master Aug 19, 2026
17 checks passed
@dimitri-yatsenko
dimitri-yatsenko deleted the diagram/uniform-bundle-edges branch August 19, 2026 23: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.

2 participants