diff --git a/src/models/componentSpec/__tests__/serialization/yamlDeserializer.test.ts b/src/models/componentSpec/__tests__/serialization/yamlDeserializer.test.ts index e9670182cf..a48e68eddc 100644 --- a/src/models/componentSpec/__tests__/serialization/yamlDeserializer.test.ts +++ b/src/models/componentSpec/__tests__/serialization/yamlDeserializer.test.ts @@ -1,6 +1,5 @@ import { beforeEach, describe, expect, it } from "vitest"; -import { EDITOR_CONDITIONAL_EXECUTION_ANNOTATION } from "@/utils/annotations"; import { IS_ENABLED_PORT_NAME } from "@/utils/conditionalExecution"; import { IncrementingIdGenerator } from "../../factories/idGenerator"; @@ -127,11 +126,8 @@ describe("YamlDeserializer", () => { const consumer = spec.tasks.find((t) => t.name === "Consumer"); const producer = spec.tasks.find((t) => t.name === "Producer"); - // Conditional mode: entity value cleared, mode annotation set. + // The binding is the sole record of the reference. expect(consumer?.isEnabled).toBeUndefined(); - expect( - consumer?.annotations.get(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION), - ).toBe("true"); // A binding to the reserved port drives the connection. const binding = spec.bindings.find( @@ -160,9 +156,6 @@ describe("YamlDeserializer", () => { const task = spec.tasks.at(0); expect(task?.isEnabled).toBe("false"); - expect( - task?.annotations.get(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION), - ).toBeUndefined(); expect(spec.bindings.length).toBe(0); }); diff --git a/src/models/componentSpec/entities/componentSpec.ts b/src/models/componentSpec/entities/componentSpec.ts index bb65be9eed..fa15d22dfc 100644 --- a/src/models/componentSpec/entities/componentSpec.ts +++ b/src/models/componentSpec/entities/componentSpec.ts @@ -1,6 +1,8 @@ import { computed } from "mobx"; import { idProp, Model, model, modelAction, prop } from "mobx-keystone"; +import { IS_ENABLED_PORT_NAME } from "@/utils/conditionalExecution"; + import { Annotations } from "../annotations"; import { collectValidationIssues } from "../validation/collectIssues"; import type { @@ -105,15 +107,28 @@ export class ComponentSpec extends Model({ this.bindings.push(binding); } + @modelAction + private clearGateLiteral(binding: Binding | undefined) { + if (!binding || binding.targetPortName !== IS_ENABLED_PORT_NAME) return; + const task = this.tasks.find((t) => t.$id === binding.targetEntityId); + task?.setIsEnabled(undefined); + } + @modelAction removeBinding(index: number) { - return this.bindings.splice(index, 1)[0]; + const removed = this.bindings.splice(index, 1)[0]; + this.clearGateLiteral(removed); + return removed; } @modelAction removeBindingBy(predicate: (b: Binding) => boolean): Binding | undefined { const idx = this.bindings.findIndex(predicate); - if (idx >= 0) return this.bindings.splice(idx, 1)[0]; + if (idx >= 0) { + const removed = this.bindings.splice(idx, 1)[0]; + this.clearGateLiteral(removed); + return removed; + } return undefined; } @@ -130,6 +145,9 @@ export class ComponentSpec extends Model({ removed.push(this.bindings.splice(i, 1)[0]); } } + for (const binding of removed) { + this.clearGateLiteral(binding); + } return removed; } @@ -202,7 +220,7 @@ export class ComponentSpec extends Model({ deleteEdgeById(bindingId: string): boolean { const idx = this.bindings.findIndex((b) => b.$id === bindingId); if (idx < 0) return false; - this.bindings.splice(idx, 1); + this.clearGateLiteral(this.bindings.splice(idx, 1)[0]); return true; } diff --git a/src/models/componentSpec/serialization/yamlDeserializer.ts b/src/models/componentSpec/serialization/yamlDeserializer.ts index 52d5778279..1c0ea4ce09 100644 --- a/src/models/componentSpec/serialization/yamlDeserializer.ts +++ b/src/models/componentSpec/serialization/yamlDeserializer.ts @@ -1,5 +1,4 @@ import { - EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, IS_ENABLED_PORT_NAME, isConditionalArgument, } from "@/utils/conditionalExecution"; @@ -129,21 +128,10 @@ export class YamlDeserializer { } } - // A reference-valued `isEnabled` is the "Conditional" mode: it becomes a - // binding to the reserved port (see buildBindings) and the entity keeps - // `isEnabled` empty. Literal values (e.g. "false") stay on the entity. + // A reference-valued `isEnabled` becomes a binding to the reserved port + // (see buildBindings) and the entity keeps `isEnabled` empty. Literal + // values (e.g. "false") stay on the entity. const conditionalEnabled = isConditionalArgument(taskJson.isEnabled); - if ( - conditionalEnabled && - !annotationItems.some( - (a) => a.key === EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, - ) - ) { - annotationItems.push({ - key: EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, - value: "true", - }); - } const args: Argument[] = []; if (taskJson.arguments) { @@ -209,7 +197,7 @@ export class YamlDeserializer { tasks, targetTask.$id, IS_ENABLED_PORT_NAME, - taskJson.isEnabled as ArgumentType, + taskJson.isEnabled, ); if (binding) bindings.push(binding); } diff --git a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/ConfigurationSection.tsx b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/ConfigurationSection.tsx index d4ec8cfcfb..f052d71576 100644 --- a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/ConfigurationSection.tsx +++ b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/ConfigurationSection.tsx @@ -11,15 +11,9 @@ import { import { useFlagValue } from "@/components/shared/Settings/useFlags"; import { ColorPicker } from "@/components/ui/color"; import { BlockStack, InlineStack } from "@/components/ui/layout"; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from "@/components/ui/select"; import { Separator } from "@/components/ui/separator"; import { Switch } from "@/components/ui/switch"; +import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Heading, Paragraph } from "@/components/ui/typography"; import type { Task } from "@/models/componentSpec"; import { useAnalytics } from "@/providers/AnalyticsProvider"; @@ -27,13 +21,14 @@ import { useSpec } from "@/routes/v2/shared/providers/SpecContext"; import type { AnnotationConfig, Annotations } from "@/types/annotations"; import { EDITOR_COLLAPSED_ANNOTATION, - EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, TASK_COLOR_ANNOTATION, } from "@/utils/annotations"; -import { IS_ENABLED_PORT_NAME } from "@/utils/conditionalExecution"; +import { + isTaskConditional, + resolveConditionalReference, +} from "@/utils/conditionalExecution"; import { ISO8601_DURATION_ZERO_DAYS } from "@/utils/constants"; -import type { EnableTaskMode } from "./taskConfig.actions"; import { useTaskConfigActions } from "./useTaskConfigActions"; interface ConfigurationSectionProps { @@ -52,30 +47,30 @@ export const ConfigurationSection = observer(function ConfigurationSection({ setTaskColor, clearProviderAnnotations, setCollapsed, - setEnableTaskMode, + setTaskConditional, + setTaskCondition, } = useTaskConfigActions(); const isSubgraph = task.subgraphSpec !== undefined; - const isConditionalConnected = - spec?.bindings.some( - (b) => - b.targetEntityId === task.$id && - b.targetPortName === IS_ENABLED_PORT_NAME, - ) ?? false; - const isConditional = - task.annotations.get(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION) === "true" || - isConditionalConnected; - const enableMode: EnableTaskMode = isConditional - ? "conditional" - : task.isEnabled === "false" - ? "false" - : "true"; + const isConditional = isTaskConditional(task); + const conditionReference = resolveConditionalReference(task, spec); + const conditionValue = + task.isEnabled === "true" || task.isEnabled === "false" + ? task.isEnabled + : ""; - const handleEnableModeChange = (value: string) => { + const handleConditionalChange = (checked: boolean) => { if (!spec) return; - const mode = value as EnableTaskMode; - setEnableTaskMode(spec, task, mode); - track("v2.pipeline_editor.task_details.enable_task.change", { mode }); + setTaskConditional(spec, task, checked); + track("v2.pipeline_editor.task_details.conditional_task.toggle", { + conditional: checked, + }); + }; + + const handleConditionChange = (value: string) => { + const enabled = value === "true"; + setTaskCondition(task, enabled); + track("v2.pipeline_editor.task_details.task_condition.change", { enabled }); }; const cacheDisabled = @@ -196,33 +191,42 @@ export const ConfigurationSection = observer(function ConfigurationSection({ <> - + - Enable task + Conditional task - + - {isConditional && !isConditionalConnected && ( - - Connect a task output or pipeline input to the β€œIs enabled?” - port on the node. - + + {isConditional && ( + + + Condition + + {conditionReference ? ( + + {JSON.stringify(conditionReference)} + + ) : ( + + + + True + + + False + + + + )} + )} diff --git a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/taskConfig.actions.ts b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/taskConfig.actions.ts index 1bd30028f9..522ebfd3a5 100644 --- a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/taskConfig.actions.ts +++ b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/taskConfig.actions.ts @@ -9,9 +9,6 @@ import { import { IS_ENABLED_PORT_NAME } from "@/utils/conditionalExecution"; import { ISO8601_DURATION_ZERO_DAYS } from "@/utils/constants"; -/** The three "Enable task" choices exposed in the Config tab. */ -export type EnableTaskMode = "true" | "false" | "conditional"; - export function toggleCacheDisable( undo: UndoGroupable, task: Task, @@ -61,30 +58,35 @@ export function setCollapsed( }); } -export function setEnableTaskMode( +export function setTaskConditional( undo: UndoGroupable, spec: ComponentSpec, task: Task, - mode: EnableTaskMode, + conditional: boolean, ) { - undo.withGroup("Set enable task", () => { - if (mode === "conditional") { - // The connection is modelled as a binding the user draws to the virtual - // "Is enabled?" port; here we just enter conditional mode so the port - // shows. `isEnabled` is derived from that binding at serialize time. + undo.withGroup("Toggle conditional task", () => { + if (conditional) { task.annotations.set(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, "true"); - task.setIsEnabled(undefined); return; } - // Leaving conditional mode: drop any connection to the reserved port. spec.removeAllBindingsBy( (b) => b.targetEntityId === task.$id && b.targetPortName === IS_ENABLED_PORT_NAME, ); task.annotations.remove(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION); - task.setIsEnabled(mode === "false" ? "false" : undefined); + task.setIsEnabled(undefined); + }); +} + +export function setTaskCondition( + undo: UndoGroupable, + task: Task, + enabled: boolean, +) { + undo.withGroup("Set task condition", () => { + task.setIsEnabled(enabled ? "true" : "false"); }); } diff --git a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/useTaskConfigActions.ts b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/useTaskConfigActions.ts index 5ad5d56637..820fd698f9 100644 --- a/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/useTaskConfigActions.ts +++ b/src/routes/v2/pages/Editor/nodes/TaskNode/context/TaskDetails/components/useTaskConfigActions.ts @@ -4,8 +4,9 @@ import { clearProviderAnnotations, saveAnnotation, setCollapsed, - setEnableTaskMode, setTaskColor, + setTaskCondition, + setTaskConditional, toggleCacheDisable, } from "./taskConfig.actions"; @@ -17,7 +18,8 @@ export function useTaskConfigActions() { saveAnnotation: saveAnnotation.bind(null, undo), setTaskColor: setTaskColor.bind(null, undo), setCollapsed: setCollapsed.bind(null, undo), - setEnableTaskMode: setEnableTaskMode.bind(null, undo), + setTaskConditional: setTaskConditional.bind(null, undo), + setTaskCondition: setTaskCondition.bind(null, undo), clearProviderAnnotations: clearProviderAnnotations.bind(null, undo), }; } diff --git a/src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx b/src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx index bfadd71c9b..187f10f710 100644 --- a/src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx +++ b/src/routes/v2/shared/nodes/TaskNode/TaskNode.tsx @@ -23,7 +23,6 @@ import { useSharedStores } from "@/routes/v2/shared/store/SharedStoreContext"; import { AggregatorOutputType } from "@/types/aggregator"; import { EDITOR_COLLAPSED_ANNOTATION, - EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, isPipelineAggregator, TASK_COLOR_ANNOTATION, } from "@/utils/annotations"; @@ -31,6 +30,7 @@ import { isSecretArgument } from "@/utils/componentSpec"; import { IS_ENABLED_INPUT_LABEL, IS_ENABLED_PORT_NAME, + isTaskConditional, } from "@/utils/conditionalExecution"; import { ISO8601_DURATION_ZERO_DAYS } from "@/utils/constants"; import type { ExecutionStatusStats } from "@/utils/executionStatus"; @@ -306,13 +306,11 @@ export const TaskNode = observer(function TaskNode({ const connectedPorts = resolveConnectedPortNames(entityId, spec); const inputDisplayData = resolveInputDisplayData(task, entityId, spec); - // In "Conditional" execution mode a virtual "Is enabled?" input is exposed so - // the user can connect an upstream value that gates the task. The connection - // itself is an ordinary binding to the reserved port, so its display value and - // connected state are already resolved above. - const isConditionalExecution = - task.annotations.get(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION) === "true" || - connectedPorts.inputs.has(IS_ENABLED_PORT_NAME); + // A conditional task exposes a virtual "Is enabled?" input so the user can + // connect an upstream value that gates it. A connection is an ordinary binding + // to the reserved port, so its display value and connected state are already + // resolved above; a literal lives on the entity and is filled in here. + const isConditionalExecution = isTaskConditional(task); const displayInputs: TaskNodeInput[] = isConditionalExecution ? [ ...inputs, @@ -323,6 +321,13 @@ export const TaskNode = observer(function TaskNode({ }, ] : inputs; + const inputDisplayValues = + isConditionalExecution && typeof task.isEnabled === "string" + ? { + [IS_ENABLED_PORT_NAME]: task.isEnabled, + ...inputDisplayData.values, + } + : inputDisplayData.values; const isSelected = isEditorVisualNodeSelected(editor, id, !!selected); @@ -357,7 +362,7 @@ export const TaskNode = observer(function TaskNode({ subgraphExecutionStats, onOutputTypeChange: handleOutputTypeChange, digest: task.componentRef.digest, - inputDisplayValues: inputDisplayData.values, + inputDisplayValues, secretInputNames: inputDisplayData.secretInputNames, onNodeClick: handleClick, onInputClick: handleInputClick, diff --git a/src/utils/annotationKeys.ts b/src/utils/annotationKeys.ts index 3565925d7d..3aad79d908 100644 --- a/src/utils/annotationKeys.ts +++ b/src/utils/annotationKeys.ts @@ -20,3 +20,5 @@ export const ZINDEX_ANNOTATION = "zIndex"; export const SDK_ANNOTATION = "sdk"; export const TASK_COLOR_ANNOTATION = "tangleml.com/editor/task-color"; export const EDGE_CONDUITS_ANNOTATION = "tangleml.com/editor/edge-conduits"; +export const EDITOR_CONDITIONAL_EXECUTION_ANNOTATION = + "tangleml.com/editor/conditional-execution"; diff --git a/src/utils/annotations.ts b/src/utils/annotations.ts index d79637fce3..138a218bf2 100644 --- a/src/utils/annotations.ts +++ b/src/utils/annotations.ts @@ -6,6 +6,7 @@ import type { AnnotationConfig, Annotations } from "@/types/annotations"; import { EDGE_CONDUITS_ANNOTATION, EDITOR_COLLAPSED_ANNOTATION, + EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, EDITOR_FLOW_DIRECTION_ANNOTATION, EDITOR_POSITION_ANNOTATION, FLEX_NODES_ANNOTATION, @@ -19,7 +20,6 @@ import { import type { ComponentSpec } from "./componentSpec"; export * from "./annotationKeys"; -export { EDITOR_CONDITIONAL_EXECUTION_ANNOTATION } from "./conditionalExecution"; export const DISPLAY_NAME_MAX_LENGTH = 100; const PIPELINE_AGGREGATOR_ANNOTATION = "is_input_aggregator"; @@ -34,6 +34,7 @@ export const SYSTEM_ANNOTATIONS = [ EDITOR_FLOW_DIRECTION_ANNOTATION, TASK_COLOR_ANNOTATION, EDGE_CONDUITS_ANNOTATION, + EDITOR_CONDITIONAL_EXECUTION_ANNOTATION, ]; export const DEFAULT_COMMON_ANNOTATIONS: AnnotationConfig[] = [ diff --git a/src/utils/conditionalExecution.ts b/src/utils/conditionalExecution.ts index 52eb3b4f4c..aa00629c59 100644 --- a/src/utils/conditionalExecution.ts +++ b/src/utils/conditionalExecution.ts @@ -1,4 +1,6 @@ -import type { ArgumentType } from "./componentSpec"; +import type { ArgumentType, ComponentSpec, Task } from "@/models/componentSpec"; + +import { EDITOR_CONDITIONAL_EXECUTION_ANNOTATION } from "./annotationKeys"; import { isGraphInputArgument, isTaskOutputArgument } from "./componentSpec"; /** @@ -12,29 +14,70 @@ export const IS_ENABLED_PORT_NAME = "__is_enabled__"; /** Human-readable label shown for the virtual "Is enabled?" input. */ export const IS_ENABLED_INPUT_LABEL = "Is enabled?"; -/** - * Annotation key marking that a task is in "Conditional" enable mode. Lives here - * (rather than in the UI-oriented `@/utils/annotations` module) so the model - * serializers can reference it without pulling the React component tree into the - * `@/models/componentSpec` barrel's module graph. - */ -export const EDITOR_CONDITIONAL_EXECUTION_ANNOTATION = - "editor.conditional-execution"; - const GRAPH_INPUT_REGEX = /^\{\{inputs\.([^}]+)\}\}$/; const TASK_OUTPUT_REGEX = /^\{\{tasks\.([^.]+)\.outputs\.([^}]+)\}\}$/; /** * True when an `isEnabled` value is a reference to an upstream value (a graph - * input or a sibling task output) β€” i.e. the "Conditional" mode β€” rather than a - * plain literal such as `"false"`. + * input or a sibling task output) rather than a plain literal such as `"false"`. */ export function isConditionalArgument( value: ArgumentType | undefined, -): boolean { +): value is ArgumentType { if (value === undefined) return false; if (typeof value === "string") { return GRAPH_INPUT_REGEX.test(value) || TASK_OUTPUT_REGEX.test(value); } return isGraphInputArgument(value) || isTaskOutputArgument(value); } + +function findConditionalBinding( + spec: ComponentSpec | null | undefined, + taskId: string, +) { + return spec?.bindings.find( + (b) => + b.targetEntityId === taskId && b.targetPortName === IS_ENABLED_PORT_NAME, + ); +} + +/** + * The annotation is the single source of truth for conditional mode. A condition + * present in the spec but not annotated is deliberately not enough: the editor + * never rewrites a spec on open, so such a task reads as non-conditional until + * the user turns the switch on. + */ +export function isTaskConditional(task: Task): boolean { + return ( + task.annotations.get(EDITOR_CONDITIONAL_EXECUTION_ANNOTATION) === "true" + ); +} + +/** + * The upstream reference a task is gated on, in the shape it serializes to, or + * undefined when the task is gated on a literal or has no condition yet. + */ +export function resolveConditionalReference( + task: Task, + spec: ComponentSpec | null | undefined, +): ArgumentType | undefined { + const binding = findConditionalBinding(spec, task.$id); + if (!binding || !spec) return undefined; + + const sourceTask = spec.tasks.find((t) => t.$id === binding.sourceEntityId); + if (sourceTask) { + return { + taskOutput: { + taskId: sourceTask.name, + outputName: binding.sourcePortName, + }, + }; + } + + const sourceInput = spec.inputs.find((i) => i.$id === binding.sourceEntityId); + if (sourceInput) { + return { graphInput: { inputName: sourceInput.name } }; + } + + return undefined; +}