fix(ui): drop nodeDepth partitioning so ELK derives lineage layers from edges#29224
Open
chirag-madlani wants to merge 3 commits into
Open
fix(ui): drop nodeDepth partitioning so ELK derives lineage layers from edges#29224chirag-madlani wants to merge 3 commits into
chirag-madlani wants to merge 3 commits into
Conversation
…om edges Pinning each node to its backend nodeDepth via elk.partitioning forced a multi-branch node into the column of its first depth, misaligning its other edges. Let ELK's layered algorithm derive layers from edge topology and use BRANDES_KOEPF node placement for a cleaner, less rigid horizontal flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
Contributor
Contributor
🟡 Playwright Results — all passed (9 flaky)✅ 4311 passed · ❌ 0 failed · 🟡 9 flaky · ⏭️ 88 skipped
🟡 9 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
BRANDES_KOEPF aligned a node to one child (top-biased), so a source with children Y and Z sat parallel to Y instead of vertically centered. Switch node placement to NETWORK_SIMPLEX, which assigns cross-axis coordinates by minimizing weighted edge deviation, keeping a source balanced between its children. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NETWORK_SIMPLEX (and bare Brandes-Köpf) resolve a symmetric source's placement to a top-aligned corner because all alignments tie on edge length, so the source did not sit centered between its upstream/downstream branches. Setting elk.layered.nodePlacement.bk.fixedAlignment to BALANCED averages the four extreme alignments into a centroid, restoring vertical centering while keeping edge-aware, straight-segment placement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review ✅ ApprovedRemoves nodeDepth partitioning from the lineage graph to resolve node misalignment and cyclic graph crashes, allowing ELK to derive layers from edge topology. Unit tests successfully updated to reflect the new layout configuration. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Describe your changes:
Fixes #29225
I dropped the
nodeDepth-based ELK partitioning in the lineage graph and let ELK'slayeredalgorithm derive layers from edge topology (withBRANDES_KOEPFnode placement) because pinning each node to its single backendnodeDepthforced a multi-branch node into the column of its first depth while its other edges fanned out to different columns, producing a visually misaligned graph. The result is a cleaner, less rigid horizontal flow, and it also removes the partition-on-cyclic-graph crash source.Type of change:
High-level design:
N/A — small change (3 files): removed the per-node
elk.partitioning.partitionhint ingetELKLayoutedElements, and removedelk.partitioning.activatewhile switchingelk.layered.nodePlacement.strategyfromSIMPLEtoBRANDES_KOEPFinELKUtil. Focus/root centering is unaffected — it is handled separately bycenterNodePosition, not by partitioning.Tests:
Use cases covered
Unit tests
ELKUtil.test.tsto match the new layout options (19/19 pass).Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
ELKUtilunit suite passes.nodeDepthin layout and that focus centering is independent.UI screen recording / screenshots:
To be attached.
Checklist:
Greptile Summary
This PR fixes a visual misalignment in the lineage graph by dropping ELK's node partitioning feature, which was forcing each node into the column dictated by its single backend
nodeDepthvalue. As a side effect this also eliminates a crash path when partitioning encounters a cyclic graph.EntityLineageLayoutUtils.ts: Removes the per-nodeelk.partitioning.partitionhint so nodes are no longer locked to a fixed column; disconnected or multi-branch nodes can now settle into the layer that best satisfies all their edges.ELKUtil.ts: Dropselk.partitioning.activateand upgrades node placement fromSIMPLEtoBRANDES_KOEPFwithbk.fixedAlignment: BALANCED, which averages the four extreme Brandes-Köpf alignments to keep symmetric nodes vertically centred.ELKUtil.test.ts: Keeps unit coverage in sync — all 19 tests now reflect the new options, including an explicittoBeUndefinedcheck confirming partitioning is off.Confidence Score: 5/5
Safe to merge — a focused, well-explained layout-only change with no backend or data-model impact.
The change removes a specific, narrowly-scoped feature (ELK partitioning) that was causing node misalignment and cyclic-graph crashes, replaces it with a well-supported ELK algorithm setting, and updates all affected unit tests. The runtime fallback in getELKLayoutedElements is already in place if ELK throws, and focus/centering logic is unchanged.
No files require special attention.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(ui): use BALANCED Brandes-Köpf align..." | Re-trigger Greptile