feat(workflow-executor): evaluate deterministic condition steps without AI - #1837
Open
Scra3 wants to merge 9 commits into
Open
feat(workflow-executor): evaluate deterministic condition steps without AI#1837Scra3 wants to merge 9 commits into
Scra3 wants to merge 9 commits into
Conversation
…ad of coercing to Full AI An out-of-enum executionType on a condition step was silently coerced to fully-automated by the schema's .catch, handing the decision to the AI — the exact opposite of the upcoming deterministic mode's intent (PRD-472). The schema now rejects unknown values, and every mapper parse failure is wrapped in InvalidStepDefinitionError so the orchestrator reports the run as malformed instead of logging-and-dropping it on every poll. Part of PRD-472 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ut AI Decision steps in the new Automatic mode carry their branching logic as build-time preRecordedArgs (optionConditions + fallbackOption, wire-final operator names from the PRD-472 contract). The executor now resolves each condition's value from the run's Get Data outputs and evaluates top-to-bottom, first-match-wins — never calling the AI and never awaiting input, because the builder chose this mode precisely to remove AI judgement from the branch. A null/missing/unresolvable value is "not met" (met: null), never an error, and no match selects the fallback, so the step can never end undefined. The evaluation trace is persisted in executionParams for the run view; unknown operators are rejected at the schema boundary so a run never reaches evaluation with a comparison it cannot honor. Part of PRD-472 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 new issues
|
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (6) 🛟 Help
|
…safe and routable
The deterministic evaluator could turn a data or config mismatch into a silent
misroute — a `status: 'success'` step carrying the wrong branch.
- Numeric strings: Sequelize returns Postgres/MySQL `numeric`/`decimal`/`bigint`
columns as strings while datasource-sequelize maps those types to the `Number`
primitive, so the builder's `value: 100` met `"150.00"` at runtime and every
comparison bailed → no option matched → silent fallback. A strictly numeric
string is now coerced against a real number (both sides stay uncoerced when
neither is a number, since ordering operators are Number/Date-only per the
contract); `'abc'` vs `100` is still not evaluable.
- Selected option: the deterministic path emitted `matchedOption ?? fallbackOption`
without checking `step.options`, unlike the manual path. `optionConditions` and
`options` are two different server-side derivations, so drift produced a success
outcome the orchestrator cannot route and the run died far from the cause. It now
throws `InvalidStepDefinitionError` before persisting anything.
- `not_equal` contradicted the file's own documented policy ("a type mismatch can
never satisfy a negated operator") by returning true on mismatch; equality is now
tri-state, so a mismatch satisfies neither `equal` nor `not_equal`.
- Timezone: an offset-less ISO datetime was parsed host-local, so "deterministic"
evaluation varied per machine. Offset-less datetimes are read as UTC, pinned by a
test that runs under a non-UTC TZ.
- `contains`/`not_contains` were extended to array membership beyond the contract
(§1: String only). Restricted back to strings — dead flexibility the builder never
emits, and the "mismatch is never satisfied" invariant keeps it from misrouting.
- Removed the unreachable `default` branch by replacing the operator switch with an
exhaustive lookup keyed by `ConditionOperator`, so a new operator fails to compile
instead of silently returning null (lint's `default-case` forbids a bare switch).
Also asserts three spec behaviors that were unasserted: deterministic mode ignores
`incomingPendingData`, `or` matches on a mix of not-evaluable and true, and an
unresolvable reference satisfies neither `blank` nor `present`.
Part of PRD-472
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scra3
marked this pull request as ready for review
August 19, 2026 06:30
…isons Date.parse normalises an impossible calendar date — 2026-02-30 becomes March 2nd — so a typo in a condition compared against a date the author never wrote, and matched. Dates are now round-tripped and rejected when the parse moved them. A value-bearing operator with no value reached the evaluator and compared against undefined, which is not evaluable, so the option silently never matched and the run took the fallback. The schema now refuses it, in line with what the orchestrator already refuses at parse time. Part of PRD-472 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…om the wire A condition step is deterministic iff it carries preRecordedArgs.optionConditions. The orchestrator derived the mode from that very presence, so the wire carried two sources of truth for one fact — and the mode was the half an older executor could not read. Dropping it also removes the reason the mode existed: a deterministic gateway publishes with aiDecision stripped, so it now arrives as Manual. An executor blind to the args degrades to a visible manual decision, never a silent AI one. An unknown mode still fails loud (no .catch), and malformed args are still rejected rather than dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…an ignored choice isDeterministicConditionStep replaces the inline truthiness check: the invariant gets a name instead of a comment, and narrowing the step type removes the second parameter that only existed to carry it. A deterministic step already ignored incomingPendingData by design, but silently: a mixed-version fleet can pause the step on an args-blind instance, and the user's click then vanished with no log line. It now warns. Tests: the four .min(1) guards on sourceStepId / fieldName / option / fallbackOption were mutation-provably dead — an empty reference resolves to "not found", so the step routed to its fallback instead of reporting the broken config, the same failure the value-less-operator refinement exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ould not resolve A step that reaches its fallback because a Get Data never ran reports success, so the run view is the only place saying why. An operator watching the logs saw nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…never loaded A deterministic condition reading a field the Get Data step did not load resolved to "not met", so the run took its fallback and reported success — the decision looked taken when its input never arrived. It now throws ConditionSourceNotLoadedError, naming the field and the step. This is reachable from the editor, not just from hand-edited BPMN: a Get Data step may let the AI pick its fields, and then no build-time validation can know which ones will come back. The orchestrator's parse-time check is structurally unable to cover it, so the runtime has to. Replaces the Warn added in d8cc48e, and aligns the Condition step with every other type (FieldNotFoundError, RelationNotFoundError, ActionNotFoundError) — it was the only one swallowing an unresolvable reference. A resolved value that is null still counts as not met, per spec: data must not fail a decision. Only a missing reference does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…in ones The catch in getAvailableRuns only pushed WorkflowExecutorError into malformed; anything else was logged and dropped. Nothing reached the orchestrator, so the run stayed pending and came back on every poll, failing the same way forever. Every hydration path currently throws a domain error, so this branch is unreachable today — but it is unreachable by construction, not by proof. A future mapper throwing a TypeError would silently reintroduce the stuck-run bug this epic already fixed once. toMalformedInfo now tolerates a non-array workflowHistory: it is called for runs that are broken in arbitrary ways, and it used to rethrow while building the report. A run so malformed that no step can be identified still cannot be reported — updateStepExecution needs a stepId — so that case stays a log. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Base automatically changed from
feature/prd-472-deterministic-decision-step
to
main
August 27, 2026 10:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Why
PRD-472: Decision steps can now be configured with explicit conditions instead of a natural-language question. The runtime must evaluate them itself — no LLM call, no human wait.
Stacked on #1836 (the version gating).
What
preRecordedArgs { optionConditions, fallbackOption }. The orchestrator derived the mode from that very presence, so carrying both put two sources of truth for one fact on the wire — and the mode was the half an older executor could not read. A deterministic gateway publishes withaiDecisionstripped, so it arrives asManual: an executor blind to the args degrades to a visible manual decision, never a silent AI one.ConditionStepDefinitionSchemakeepsexecutionTypeatmanual | fully-automatedwith no.catch, so an unknown mode still fails loud. Operators and aggregators are enum-validated, and malformedpreRecordedArgsare rejected rather than dropped — a broken deterministic config can never degrade to a manual/AI decision.doExecute, keyed on the presence ofpreRecordedArgs: resolves each condition's value from the run history (sourceStepId+fieldNameagainst the Get Data step's persisted fields, most-recent occurrence for loops), evaluates top-to-bottom with first-match-wins and and/or aggregation.deterministic-condition-evaluator.ts): null/unresolvable value →met: null(not evaluable, never an error), type mismatch → false including for negated operators, ISO dates compared as timestamps.executionParamspersisted as{ evaluations[{ option, outcome, conditions[{ index, met }] }], selectedOption, usedFallback }— the data the run view needs to show why an option won.Tests
Full package suite: 1575 passed / 0 failed. 24 evaluator unit tests (happy / null / type-mismatch per operator) + 10 deterministic-branch executor tests.
Part of PRD-472
🤖 Generated with Claude Code
Note
Evaluate deterministic condition steps without AI in
ConditionStepExecutorConditionStepExecutor.doExecutethat evaluates condition steps carryingpreRecordedArgsusing a new pureevaluateOperatorfunction, with first-match-wins option selection andand/oraggregators. No AI call or user input is needed.DeterministicConditionSchema,OptionConditionsSchema) and aCONDITION_OPERATORSconstant so unknown operators and malformed deterministic configs are rejected at the schema boundary.ConditionEvaluation) inConditionStepExecutionDatafor deterministic outcomes.ForestServerWorkflowPort.getAvailableRuns: unexpected errors are now wrapped inHydrationFailedErrorand reported as malformed instead of being silently logged and skipped.executionTypevalues for condition steps are no longer coerced via.catch; they now raiseInvalidStepDefinitionError. Missing referenced source fields in deterministic evaluation throwConditionSourceNotLoadedError.toMalformedInfono longer throws whenworkflowHistoryis not an array.Macroscope summarized 8748f80.