Skip to content

feat: Finalize Conditional Execution - #2657

Merged
camielvs merged 1 commit into
masterfrom
08-20-feat_finalize_conditional_execution
Aug 24, 2026
Merged

feat: Finalize Conditional Execution#2657
camielvs merged 1 commit into
masterfrom
08-20-feat_finalize_conditional_execution

Conversation

@camielvs

@camielvs camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Description

Final pass over conditional execution. Nothing changes about what gets sent to the backend — this is UI, vocabulary and test coverage. The visual direction is borrowed from the exploration in #2649.

"Run when", not "isEnabled". isEnabled is spec jargon. The UI now says Run when, and the two literal choices read Always / Never rather than true/false.

Conditional execution gets its own purple box in the Config tab. A single switch turns it on for a task. When it's on, the Run when control appears inside the box — either the Always/Never toggle, or, if something is wired into it, the upstream source it follows (→ Flag.flag), formatted the same way bound inputs are formatted everywhere else in the editor.

On the node, the condition is separate from the inputs. It used to be injected into the task's input list as a fake input, which meant the list had to know about it in every place it did anything (splitting, condensing, counting). It's now its own purple row above the inputs with its own handle, which means:

  • it stays visible when a node's inputs are condensed, instead of being counted into "+3 more"
  • the input-list code went back to exactly what it is on master — the whole special case is gone
  • collapsed nodes get the same treatment: a purple handle plus a branch icon

Dragging off the "Run when" handle no longer creates a graph input called __is_enabled__. The editor names an auto-created input after the port you dragged from, and the reserved port name was leaking into the user's pipeline. It now creates an input called run_condition, typed String — that's the form the condition is actually read in, and it's what lets the input connect to the ports components declare.

Naming and comments. Internal names now say what they are (resetRunCondition, runConditionBinding, setRunCondition, …). Comments that narrated the code were deleted; the ones left explain a decision the code can't.

Tests. New coverage for the shared helpers, the enable/disable actions (including that switching conditional execution off clears both the literal and the connection while leaving other connections alone), the node rendering, the auto-created input, and each of the fixes below.

Fixes from review

  • A hand-written or SDK-generated pipeline can set the condition to an unquoted false. That now reads as Never; before, it showed as Always while the backend skipped the task — the display and the behaviour disagreed.
  • A condition pointing at something that no longer exists now shows what it pointed at, instead of quietly falling back to Always. Loading such a pipeline also keeps the condition rather than leaving the task ungated.
  • Grouping tasks into a subgraph, and ungrouping them again, no longer leaks the internal port name into the pipeline. A promoted condition becomes a readable run_condition input, and a fixed condition survives the round trip.
  • Dragging off the handle of a task set to Never no longer ungates it. The auto-created input now starts out holding the task's own condition, so the task keeps running when it was set to.
  • The Run when control is now properly labelled for screen readers.
  • One violet palette, one icon and one stated reason for both, so the condition row can't drift apart between the full and collapsed node.

Related Issue and Pull requests

Stacked on #2651. UI direction borrowed from #2649. Followed by #2658, which stops conditional execution being set where the backend won't honour it.

Type of Change

  • Improvement

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Screenshots (if applicable)

image.png

image.png

image.png

image.png

Test Instructions

The conditional-execution flag is off by default — turn it on in Settings first.

  1. Select a task → Config tab → toggle Conditional execution. The purple box expands to show Run when, and the node grows a purple row.
  2. Flip Run when between Always and Never; the node's row should follow.
  3. Condense the node's inputs — the condition row stays visible.
  4. Drag from the node's purple handle onto empty canvas. You should get a run_condition String graph input, not one named __is_enabled__.
  5. Set a task to Never, then drag off its handle. The new input should already hold false, and the task should still read as gated off — not silently switch to running.
  6. Wire a task output into the handle. Both the node and the Config panel should read → Task.output.
  7. Delete that edge. The task stays conditional and resets to Always rather than silently staying Never.
  8. Export the pipeline YAML and re-import it — the condition should survive the round trip.
  9. Hand-edit a pipeline's YAML to set a task's condition to a bare false (no quotes), then open it. The task should read Never.
  10. Select a conditional task and a neighbour, group them into a subgraph, then ungroup them. The condition should come back intact, and no input named __is_enabled__ should appear anywhere.

Additional Comments

Turning conditional execution off deliberately clears the condition (both the literal and any connection) rather than remembering it, so re-enabling starts from Always. That's the trade discussed in #2651.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: 08-20-feat_finalize_conditional_execution/5ff8846

camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Comment thread src/utils/conditionalExecution.ts Outdated
Comment thread src/routes/v2/pages/Editor/store/actions/io.actions.ts Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.test.tsx
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.tsx Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.tsx Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeSimplified.tsx Outdated
Comment thread src/utils/componentSpec.ts
@camielvs

Copy link
Copy Markdown
Collaborator Author

🤖 This is an AI-generated review summary.

Carry-over audit: does this stack close out #2574's review comments?

#2574's comments aren't meant to be actioned there — the expectation is that #2651 and #2657 deal with them. This is the audit of that, posted at the top of the stack so it's in one place. 23 comments on the root PR: 10 resolved, 8 still open, 1 changed shape, 1 partial.

Resolved by #2651 / #2657

# Comment Where it was addressed
1 as ArgumentType cast on the literal #2651 — literals now flow through typed helpers, no cast
2 EnableTaskMode cast #2651 — the mode enum is gone with the dropdown
3 Two sources of truth / annotation namespace #2651 — the annotation is removed entirely; conditional-ness is derived
4 Re-export through annotations.ts #2651 — no longer needed
5 is_enabled isn't a real input, don't render it in the input list #2657 — own row above the inputs, input-list code back to master
6 Ark-kun: "a separate middle section" for it #2657 — that's what the Run-when row is
7 No canvas signal that a task is disabled #2657 — purple handle + branch icon on collapsed nodes
8 "Conditional with nothing wired is a silent no-op" #2657 — Always / Never literal means there's always a defined state
9 Beta flag #2657 — behind a flag, off by default
10 Mode-transition tests #2651 / #2657 — 20 new tests across the two

Still open

# Comment Status
1 isEnabled may arrive as a boolean / "False" — strict === "false" is wrong Not fixed. Now centralized in toConditionLiteral, so it's a one-line fix — see the inline comment on conditionalExecution.ts:23.
2 Conditional execution on subgraph nodes (r3686778763 never answered) Not fixed — see the inline comment on ConfigurationSection.tsx:198.
3 Fail-open when the reference can't be resolved (yamlDeserializer.ts:161, :200) Unchanged by either PR.
4 Sentinel port name leaking through createSubgraph.ts:131 Unchanged. #2657 fixes the leak into the auto-created graph input name, not this path.
5 A component legitimately declaring an input called __is_enabled__ Unchanged.
6 Duplicated reference regexes (yamlDeserializer.ts:28-29 vs conditionalExecution.ts:29-30) Unchanged.
7 No warning when the connected output isn't String/Boolean Unchanged.
8 Test gaps: createSubgraph/unpackSubgraph with a conditional binding; boolean and uppercase literals Unchanged.

Changed shape

Boolean port type / quick-connect compatibility. The original comment was about the __is_enabled__ port; #2657 removes that surface but re-introduces the same mismatch on the auto-created graph input, which is typed Boolean in io.actions.ts:241. r3686547226 recommended String or no type at all, and that was accepted on the root PR — inline comment on io.actions.ts:241.

Partially addressed

Accessible labelling. The Config panel switch now has a real <Label htmlFor>, which is the bigger half. The Run-when Tabs (Always / Never) still has no accessible name.

Cross-stack note

resolveConditionalReference duplicates JsonSerializer.bindingToArgument. It's introduced in #2651, so the inline comment lives there — flagging it here so it doesn't get lost when the stack merges.

@camielvs
camielvs force-pushed the 08-20-feat_finalize_conditional_execution branch from 325c67e to 2d0ce95 Compare August 21, 2026 16:52
@camielvs
camielvs marked this pull request as ready for review August 21, 2026 17:26
@camielvs
camielvs requested a review from a team as a code owner August 21, 2026 17:26
Comment thread src/routes/v2/pages/Editor/store/actions/io.actions.ts
@camielvs
camielvs requested a review from Mbeaulne August 21, 2026 22:38
@camielvs
camielvs force-pushed the 08-20-feat_finalize_conditional_execution branch from 2d0ce95 to 20b5ec3 Compare August 21, 2026 22:55

camielvs commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Merge activity

  • Aug 24, 6:28 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 24, 6:36 PM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 24, 6:40 PM UTC: @camielvs merged this pull request with Graphite.

@camielvs
camielvs changed the base branch from 08-20-feat_conditional_execution_-_option_1 to graphite-base/2657 August 24, 2026 18:29
camielvs added a commit that referenced this pull request Aug 24, 2026
## Description

Implements **Option 1 — Derived**. Two alternatives were prototyped alongside it (#2652, #2654); this is the one we settled on. Also a general pass over the UI, UX and frontend architecture of the feature.

### The question these options were answering

`TaskSpec.isEnabled` decides whether a task runs. It either holds a literal (`"false"`) or points at an upstream value — a graph input, or another task's output.

The editor needs to know one thing the spec doesn't record: whether the user wants the conditional UI on this task at all — the extra handle on the node, the condition control in the Config panel. A task with no `isEnabled` looks exactly like a task that was never meant to be conditional, so where does that piece of state live?

### The three answers

- **Option 2 — Annotation (#2652):** keep an `isConditional` annotation on the task, fully decoupled from `isEnabled`. The annotation controls the UI; `isEnabled` controls the backend.
- **Option 4 — Always-on (#2654):** don't track it. Show the conditional UI on every task, always.
- **Option 1 — Derived (this PR):** don't track it either, but infer it. A task is conditional when it *has* an `isEnabled` value — a literal on the task, or a connection to the reserved port.

### Why derived

**Two sources of truth drift, and this one drifts immediately.** Pipelines don't only come from our editor — they come from the SDK, from other tools, from hand-written YAML. Those have `isEnabled` set and no annotation, so the annotation version opens them showing no condition and no edge: the connection is right there in the file and invisible on the canvas. The fix would be to backfill the annotation on import, which is deriving it anyway, one round trip late.

**It keeps editor state out of the user's pipeline.** Conditional-ness is already expressed by `isEnabled`. An annotation would write a second copy of it into the file for something we can compute.

**Most tasks aren't conditional.** Always-on avoids the state problem, but it spends vertical space on every node and adds a concept to every task for the benefit of the few that use it. A switch keeps it opt-in and keeps ordinary nodes looking ordinary.

**One question, one answer.** `isTaskConditional(task, spec)` is what the node, the panel and serialization all ask, so they can't disagree.

The honest cost: deriving couples the switch to the value. Turning it on has to write something (`"true"` — Always), and turning it off has to clear it, so "conditional but not configured yet" isn't a state we can represent. We think that's fine — Always is a sensible starting point, and the alternative was the drift above.

### Also in here

- **The condition control was a three-way dropdown** (*Enable task*: True / False / Conditional), which folded two different questions into one control — "should this be conditional" and "what's the condition". It's now a switch for the first and a toggle for the second.
- **Deleting the condition edge resets the task to enabled** instead of resurfacing the previous literal. For an ordinary input, bringing back the old literal when you disconnect it is helpful. For a condition it isn't: resurfacing `"false"` would leave the task silently disabled after the user deliberately removed the condition.

## Related Issue and Pull requests

Stacked on #2574. Alternatives: #2652 (annotation), #2654 (always-on). Followed by #2657, which finishes the UI.

## Type of Change

- [x] New feature

## Checklist

- [ ] I have tested this does not break current pipelines / runs functionality
- [ ] I have tested the changes on staging

## Screenshots (if applicable)

<!-- Include any screenshots that might help explain the changes or provide visual context -->

## Test Instructions

The `conditional-execution` flag gates all of this.

1. Select a task → **Config** tab → toggle **Conditional task** on, then set the condition to False and confirm the task's `isEnabled` follows in the YAML.
2. Toggle it back off and confirm `isEnabled` is cleared and no stray annotation is written.
3. Connect a graph input or an upstream task output to the condition handle, then delete that edge — the task should stay conditional and reset to enabled.
4. Import a pipeline whose YAML sets `isEnabled` to a reference (e.g. one produced by the SDK) and confirm the editor shows both the conditional UI and the edge without us having written anything into the file first.

## Additional Comments

<!-- Add any additional context or information that reviewers might need to know regarding this PR -->
@camielvs
camielvs changed the base branch from graphite-base/2657 to master August 24, 2026 18:34
@camielvs
camielvs force-pushed the 08-20-feat_finalize_conditional_execution branch from 20b5ec3 to 5ff8846 Compare August 24, 2026 18:35
@camielvs
camielvs merged commit 936627b into master Aug 24, 2026
17 checks passed
@camielvs
camielvs deleted the 08-20-feat_finalize_conditional_execution branch August 24, 2026 18:40
camielvs added a commit that referenced this pull request Aug 24, 2026
…ns (#2658)

## Description

Conditional execution only works on tasks that run a container. The backend rejects it on subgraphs, so a pipeline that gates one fails at submission with a raw error the user can't act on.

**The control is hidden for subgraphs.** The Conditional execution box no longer appears when the selected node is a subgraph, so it can't be switched on by accident.

**Validation covers what hiding the control can't.** A pipeline can arrive with the condition already set — from the SDK, from hand-written YAML, or from an earlier version of the editor. A subgraph with a run condition is now flagged as an error that blocks submission, with a **Remove Condition** fix offered in the issue panel.

Three related checks, since the same panel was already open:

- **A component that declares an input named `__is_enabled__`** collides with the name the editor reserves for run conditions. Connecting such an input would silently save it as the task's run condition and drop the argument. It's a warning as soon as the component is used, and an error once the input is actually connected.
- **A condition wired to a value that can't be read as true or false** — a number, say — is an error. Anything else blocks submission too, and for the same reason: the run doesn't fail up front, it fails at the moment the task is reached, after everything upstream has already spent its compute. Catching it before submission is worth doing.
- **A fixed condition that isn't true or false** — someone's hand-written `yes`, or a value the editor can't make sense of — is an error, and the message quotes the offending value. Casing and stray whitespace are fine. A condition that's wired up rather than fixed is left alone, since its value isn't known until the run.

## Related Issue and Pull requests

Stacked on #2657.

## Type of Change

- [x] Bug fix

## Checklist

- [ ] I have tested this does not break current pipelines / runs functionality
- [ ] I have tested the changes on staging

## Screenshots (if applicable)

<!-- Include any screenshots that might help explain the changes or provide visual context -->

## Test Instructions

The `conditional-execution` flag is off by default — turn it on in Settings first.

1. Select a subgraph node → **Config** tab. There should be no Conditional execution box. Select an ordinary task and confirm it's still there.
2. Hand-edit a pipeline's YAML so a subgraph task is gated on a condition, then open it. The issue panel should show an error, submission should be blocked, and **Remove Condition** should clear it.
3. Gate an ordinary task on a number-typed output. You should get an error explaining the value can't be read as a condition, and submission should be blocked. Re-wire it to a String or Boolean output and the error should clear.
4. Hand-edit a task's condition to something like `yes`. You should get an error quoting that value. Change it to `TRUE` or ` false ` and it should be accepted.

## Additional Comments

The reserved-name check is a guard against a name collision rather than something a user is likely to hit — a component would have to declare an input called `__is_enabled__`. It's here because the failure is silent: the argument disappears on save with nothing to indicate why.

The type check accepts `str`, `bool` and `text` alongside the canonical type names. Since it blocks submission, a hand-written component using a lowercase alias shouldn't be caught by it.

One thing deliberately not done: the accepted conditions aren't normalized on load. The checks read leniently but write nothing — quietly rewriting a value in someone's pipeline is worse than reporting it.
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.

2 participants