Skip to content

feat: Pipeline editor - Conditional execution - #2574

Open
Ark-kun wants to merge 2 commits into
masterfrom
feat--Conditional-execution
Open

feat: Pipeline editor - Conditional execution#2574
Ark-kun wants to merge 2 commits into
masterfrom
feat--Conditional-execution

Conversation

@Ark-kun

@Ark-kun Ark-kun commented Jul 24, 2026

Copy link
Copy Markdown

Description

Adds support for the conditional execution feature (TaskSpec.isEnabled). When user selects a Task node, the Config tab in right sidebar allows the user to configure "Enable task": The user can set it to "True" (default) "False" or "Conditional".

  • True: Unset TaskSpec.isEnabled
  • False: Set TaskSpec.isEnabled to "false"
  • Conditional: Adds a new virtual input named "Is enabled?" to the tasks's node visual representation. The user can connect something (task output, graph input) to that input just like with normal inputs. Internally, the connected argument is assigned to the TaskSpec.isEnabled attribute (similar to TaskSpec.arguments["..."]).
image image image

Backend PR: TangleML/tangle#301

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 Jul 24, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: feat--Conditional-execution/9c91bfc

@Ark-kun
Ark-kun force-pushed the feat--Conditional-execution branch 2 times, most recently from 0d3e9f1 to c119b29 Compare July 29, 2026 00:02
@Ark-kun Ark-kun changed the title feat: Conditional execution feat: Pipeline editor - Conditional execution Jul 29, 2026
@Ark-kun
Ark-kun marked this pull request as ready for review July 29, 2026 06:46
@Ark-kun
Ark-kun requested a review from a team as a code owner July 29, 2026 06:46
Comment thread src/models/componentSpec/serialization/yamlDeserializer.ts
Comment thread src/utils/conditionalExecution.ts
Comment thread src/models/componentSpec/serialization/yamlDeserializer.ts
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx
Comment thread src/utils/conditionalExecution.ts
@camielvs

Copy link
Copy Markdown
Collaborator

🤖 This is an AI-generated code review comment.

The inline comments cover the line-anchored issues. A few findings are cross-cutting or land outside this PR’s diff, so collecting them here.

Design consequence — sentinel leaks through createSubgraph

createSubgraph.ts promotes each incoming boundary binding to a graph input whose name is first.targetPortName. So wrapping a gated task whose gate source sits outside the selection mints a user-visible pipeline input named __is_enabled__. Relatedly, any component that legitimately declares an input named __is_enabled__ would have its real argument swallowed into isEnabled. The reserved-port approach is the right call, but subgraph promotion (and packing/unpacking) needs to special-case the sentinel. (Line is outside this PR’s diff, hence not inline.)

UX gaps

  • No canvas signal for a disabled task. isEnabled: "false" renders identically to an enabled task — you must select the node and open Config to see it. Highest value-per-line addition here: dim the node or add a badge.
  • “Conditional” with nothing wired is a silent no-op. It serializes to no isEnabled and the task just runs. The only signal is subdued sidebar text on a normally-closed panel, and validateSpec has no rule. Add a warning issue so it surfaces in the normal pre-submit flow.
  • Bad upstream value → mid-run System Error (the case Maksym hit in the thread). An empty output takes a different backend branch and produces the misleading “does not have value” text — worth a backend message fix (Alexey). On the UI side, warn when the connected output’s declared type isn’t String/Boolean.
  • No beta flag (as Camiel requested in the thread). ExistingFlags + useFlagValue is ~3 lines. Gate display only (the select item + the virtual port) — never the serializers, otherwise a spec authored flag-on becomes lossy for a flag-off user.

Test gaps

  • setEnableTaskMode transitions — binding cleanup when leaving conditional, and undo grouping.
  • Subgraph rejection of the “Enable task” control.
  • createSubgraph / unpackSubgraph carrying a task with a conditional binding.
  • Boolean and uppercase/whitespace isEnabled literals.

Verdict

Design is right, execution mostly clean. Modeling the gate as an ordinary binding to a reserved port — rather than a bespoke widget — means edges, undo, copy/paste, and round-trip all come for free, and the serializer/deserializer tests are solid. Dropping the never-implemented PredicateType for ArgumentType makes the type honest. The three inline-flagged bugs (subgraph gate, literal normalization, fail-open dangling reference) are the blockers.

Copy link
Copy Markdown
Collaborator

@Ark-kun most of the Ai's findings above we can address in a follow-up once we have this shipped under a beta flag. I already have some notes and thoughts about how we can improve canvas UX for various conditional states

Comment thread src/utils/annotations.ts
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx
@Ark-kun

Ark-kun commented Aug 7, 2026

Copy link
Copy Markdown
Author

@Ark-kun most of the Ai's findings above we can address in a follow-up once we have this shipped under a beta flag. I already have some notes and thoughts about how we can improve canvas UX for various conditional states

Thank you, @camielvs . I've added a beta flag.

camielvs
camielvs previously approved these changes Aug 18, 2026

@camielvs camielvs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's ship this and I will follow-up with improvements and address the AI review when I have time

@camielvs camielvs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, let's hold for now. I will prioritize checking for an alternative system than annotation-based as it may be irreversivble if we commit now.

Give me a couple days :)

@camielvs
camielvs dismissed their stale review August 18, 2026 01:38

need to do a bit or tech research before verify and accepting this approach

@camielvs
camielvs force-pushed the feat--Conditional-execution branch from 1184413 to 55706d9 Compare August 19, 2026 21:49

camielvs commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

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

@camielvs
camielvs force-pushed the feat--Conditional-execution branch from 55706d9 to de8e0ac Compare August 20, 2026 17:28
@camielvs
camielvs force-pushed the feat--Conditional-execution branch 4 times, most recently from e4c7524 to 2b3d892 Compare August 20, 2026 19:30
Adds support for the conditional execution feature (`TaskSpec.isEnabled`).
When user selects a Task node, the Config tab in right sidebar allows the user to configure "Enable task":
The user can set it to "True" (default) "False" or "Conditional".
* True: Unset TaskSpec.isEnabled
* False: Set TaskSpec.isEnabled to "false"
* Conditional: Adds a new virtual input named "Is enabled?" to the tasks's node visual representation. The user can connect something (task output, graph input) to that input just like with normal inputs. Internally, the connected argument is assigned to the TaskSpec.isEnabled attribute (similar to TaskSpec.arguments["..."]).
@camielvs
camielvs force-pushed the feat--Conditional-execution branch from 2b3d892 to 9c91bfc Compare August 20, 2026 21:47

@camielvs camielvs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved as part of the upstream conditional execution stack, which refines the frontend architecture and UX

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.

3 participants