Skip to content

fix: deduplicate bidirectional relation arrows#52

Merged
Zir0-93 merged 2 commits into
masterfrom
fix/deduplicate-bidirectional-arrows
May 25, 2026
Merged

fix: deduplicate bidirectional relation arrows#52
Zir0-93 merged 2 commits into
masterfrom
fix/deduplicate-bidirectional-arrows

Conversation

@Zir0-93
Copy link
Copy Markdown
Member

@Zir0-93 Zir0-93 commented May 25, 2026

Summary

  • Bidirectional relations between the same component pair (A→B and B→A) were rendered as two separate arrows, creating visually duplicate connections
  • Added pair-level deduplication using a canonical key so only one arrow is drawn per component pair
  • Bumps version to 3.10.1

Test plan

  • All 195 tests pass (0 failures)
  • Verified with SmartTube PR #5797 striff: no more duplicate arrows between AppPrefs and MediaServiceManager

🤖 Generated with Claude Code

When two components have relations in both directions (A→B and B→A),
both were rendered as separate arrows, creating visually duplicate
connections. Added pair-level deduplication using a canonical key so
only one arrow is drawn per component pair.

Bumps version to 3.10.1.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52919801c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +46 to +49
String pair = pairKey(currCmpRel.originalComponent().uniqueName(),
currCmpRel.targetComponent().uniqueName());
if (!renderedPairs.add(pair)) {
continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve deterministic direction before deduplicating pairs

This pair-level early continue makes the rendered edge depend on whichever direction is visited first, but both loops iterate Set/HashSet (diagramCmps and significantRels) with no stable order. In genCode, the selected direction affects diff styling because middle-arrow color is chosen from one of the two directional relations (currCmpRel vs reverseRel), so for equal-strength bidirectional links (common for ASSOCIATION/WEAK_ASSOCIATION) the surviving arrow can be colored from the unchanged side and hide an added/deleted change, and output can vary across runs.

Useful? React with 👍 / 👎.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Zir0-93 Zir0-93 merged commit 38e8030 into master May 25, 2026
5 checks passed
@Zir0-93 Zir0-93 deleted the fix/deduplicate-bidirectional-arrows branch May 25, 2026 15:58
Zir0-93 added a commit that referenced this pull request May 25, 2026
…#53)

Address code review feedback on #52: the pair dedup could render
arrows in non-deterministic direction (HashSet iteration order varies)
and pick the wrong diff color when the reverse direction had the change.

Now prefers the direction with diff coloring (added/deleted) for
accurate change visualization, and falls back to canonical
(alphabetical) ordering for deterministic output across runs.

Bumps version to 3.10.2.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant