fix(diagram): collapsed-to-collapsed edges carry no per-foreign-key style - #1545
Conversation
…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
left a comment
There was a problem hiding this comment.
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.
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_collapsebuilt that edge like this:First member wins, no aggregation. So
multi(cardinality),primary(primary-vs-secondary) andaliased(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:
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:
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:
(
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 → sessionandsession → imagingbundle 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.