Skip to content

feat: Conditional Execution - Option 2 - #2652

Closed
camielvs wants to merge 1 commit into
feat--Conditional-executionfrom
08-20-feat_conditional_execution_-_option_2
Closed

feat: Conditional Execution - Option 2#2652
camielvs wants to merge 1 commit into
feat--Conditional-executionfrom
08-20-feat_conditional_execution_-_option_2

Conversation

@camielvs

@camielvs camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Description

Implements Option 2 - Annotation.

Improved UI, UX and frontend architecture for the new Conditional Execution feature.

This option works by controlling isConditional state via an annotation, completely decoupled from isEnabled. isConditional simply toggles whether the UI for the feature is visible or not, while isEnabled controls the information passed to the backend.

Related Issue and Pull requests

Type of Change

  • Bug fix
  • New feature
  • Improvement
  • Cleanup/Refactor
  • Breaking change
  • Documentation update

Checklist

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

Screenshots (if applicable)

Test Instructions

Additional Comments

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: 08-20-feat_conditional_execution_-_option_2/e1ec7f3

camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@camielvs
camielvs force-pushed the feat--Conditional-execution branch from de8e0ac to bbfc2a7 Compare August 20, 2026 18:38
@camielvs
camielvs force-pushed the 08-20-feat_conditional_execution_-_option_2 branch from 5e3ef42 to 7b29ad9 Compare August 20, 2026 18:38
@camielvs
camielvs force-pushed the feat--Conditional-execution branch from bbfc2a7 to 7a522e2 Compare August 20, 2026 19:21
@camielvs
camielvs force-pushed the 08-20-feat_conditional_execution_-_option_2 branch from 7b29ad9 to c3ac694 Compare August 20, 2026 19:21
@camielvs
camielvs force-pushed the feat--Conditional-execution branch from 7a522e2 to e4c7524 Compare August 20, 2026 19:25
@camielvs
camielvs force-pushed the 08-20-feat_conditional_execution_-_option_2 branch 2 times, most recently from 506163e to ae9254b Compare August 20, 2026 19:30
@camielvs
camielvs force-pushed the feat--Conditional-execution branch from e4c7524 to 2b3d892 Compare August 20, 2026 19:30
@camielvs
camielvs force-pushed the 08-20-feat_conditional_execution_-_option_2 branch 2 times, most recently from 93d3313 to cdb316b Compare August 20, 2026 20:34
@camielvs
camielvs force-pushed the feat--Conditional-execution branch from 2b3d892 to 9c91bfc Compare August 20, 2026 21:47
@camielvs
camielvs force-pushed the 08-20-feat_conditional_execution_-_option_2 branch from cdb316b to e1ec7f3 Compare August 20, 2026 21:47

Copy link
Copy Markdown
Collaborator Author

Closing: Opting for Option 1

@camielvs camielvs closed this Aug 20, 2026
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 -->
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