Skip to content

Fix Split edge default and block mixed slot export#185

Open
kishore08-07 wants to merge 1 commit intoopen-rmf:mainfrom
kishore08-07:split
Open

Fix Split edge default and block mixed slot export#185
kishore08-07 wants to merge 1 commit intoopen-rmf:mainfrom
kishore08-07:split

Conversation

@kishore08-07
Copy link

@kishore08-07 kishore08-07 commented Mar 18, 2026

Bug fix

Fixed bug

This PR fixes #183 by improving the behavior of Split operations in the Diagram Editor to make them more intuitive, consistent, and error-resistant.

Previously:

  • Split edges defaulted to keyed, making edge labeling less discoverable for users.
  • The editor allowed mixed output types (keyed and sequence) from the same Split node.
  • Export was not proactively blocked for this invalid Split state.

Fix applied

  • Default Split edge type preference now starts with sequence.
  • Added Split export validation that rejects mixed keyed and sequence outputs from the same Split source.
  • Export button is disabled when mixed Split slot types exist.
  • Conflicting Split edges are highlighted in error color until the mismatch is resolved.
  • Added regression tests for both behaviors.
Screencast.from.2026-03-18.14-11-01.webm

GenAI Use

We follow OSRA's policy on GenAI tools

  • I used a GenAI tool in this PR.
  • I did not use GenAI

Generated-by:
GPT-5.3-Codex

…rt, and highlight conflicts

Signed-off-by: kishore08-07 <kishorebsm8@gmail.com>
@mxgrey
Copy link
Contributor

mxgrey commented Mar 18, 2026

Thanks for opening this contribution. It's great to see the community identify pain points and put forward solutions.

I'm not certain, but I suspect we're going to see merge conflicts between this and #182, so just a heads up on that.

Added Split export validation that rejects mixed keyed and sequence outputs from the same Split source.

It's actually intentional that the Split operation supports a mix of keys and sequences. See the 💡Tip in this section of the handbook. We should revert this restriction.

Export button is disabled when mixed Split slot types exist.

I think it's dangerous for us to ever completely disable the export button. There may be times when a user wants to save a file in a bad state because they want to create a backup or need to exit the program, and they want to come back later to fix it.

I think a better approach would be a pop-up warning telling the user that the workflow is in a broken state and verifying that they want to save it, similar to the warning that a Save dialog usually gives if you try to save a file with a name that is already in use. For now we should revert this disabling behavior. The pop-up warning approach (or some other way to warn the user when the diagram is in a bad state) can be added as a separate PR.

@mxgrey
Copy link
Contributor

mxgrey commented Mar 18, 2026

Note this error that shows up while running the unit tests:

 error: crossflow_diagram_editor@0.0.7: Frontend has changed, run `BUILD_FRONTEND=1 cargo build` to update it

You'll need to run BUILD_FRONTEND=1 cargo build then commit and push the resulting changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

[Diagram Editor] Split operation improvements

2 participants