UI: Fix Graph layout for TaskGroup tasks wired to external nodes#67720
Open
vatsrahul1001 wants to merge 1 commit into
Open
UI: Fix Graph layout for TaskGroup tasks wired to external nodes#67720vatsrahul1001 wants to merge 1 commit into
vatsrahul1001 wants to merge 1 commit into
Conversation
Open ``@task_group`` rendered with vertically-stacked internals and edges crossing the boundary whenever an internal task had a direct dependency on a node outside the group (an "escape edge" that bypassed the group's entry/exit interface). Dag execution was unaffected. Two underlying issues, both in the ELK graph-layout refactor from #65031: 1. ``hasUniformExternalConnectivity`` was too lenient — it fired whenever externally-connected children separately shared the same external sources OR the same external targets, instead of the canonical fan-in/fan-out pattern where every child has the same full ``(sources, targets)`` profile. On mixed-profile groups (entry + exits), it incorrectly fired and collapsed the author's deliberately- wired escape edges into a single group-level edge, hiding the intent. 2. When the optimisation did fire on an open group, ``rewriteGroupEdges`` was tuned for closed groups and dropped the group's internal edges too, leaving ELK with no internal-layout information for the children (the visible symptom in #67714). Fix: tighten ``hasUniformExternalConnectivity`` to require the full profile to match across externally-connected children, and add a ``preserveInternal`` option to ``rewriteGroupEdges`` so the canonical fan-in/fan-out path keeps internals intact. Closes: #67714
vatsrahul1001
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton and
shubhamraj-git
as code owners
vatsrahul1001
added
kind:bug
Contributor
Author
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
1 participant
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.
Closes #67714.
#65031 could be cause of it
Fix
hasUniformExternalConnectivityto require every externally-connected child to share the same full(sources, targets)profile. Mixed entry/exit shapes (like the reporter's repro) now fall back to rendering individual crossing edges, preserving the author's explicit dependency intent. The canonical "cleanup group" fan-in/fan-out pattern that the optimization was designed for (every child has same upstream AND same downstream) still triggers the collapse.preserveInternaloption torewriteGroupEdges. The open-group call site sets it so the collapse optimization preserves internal edges for the subsequent extraction loop; the closed-group call site keeps the default (drop internals — they're not laid out when the group is collapsed).Tests
New
elkGraphUtils.test.tscovers:hasUniformExternalConnectivity— vanilla TaskGroup, mixed-profile 3.2.2 Graph View breaks when TaskGroup outputs connect internal tasks to external downstream tasks #67714 trigger, canonical fan-in/fan-outgenerateElkGraphfor the 3.2.2 Graph View breaks when TaskGroup outputs connect internal tasks to external downstream tasks #67714 repro: internal edges preserved AND escape edges rendered individuallygenerateElkGraphfor the canonical fan-in/fan-out shape: collapse still fires, internal edges kept (exercisespreserveInternal: true)generateElkGraphregression guards: simple open TaskGroup, closed TaskGroupVerified the new tests fail against
mainwithout the fix and pass with it.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines