feat(workflow-executor): evaluate the operators a list view filter offers, relative dates included - #1880
feat(workflow-executor): evaluate the operators a list view filter offers, relative dates included#1880Scra3 wants to merge 10 commits into
Conversation
7 new issues
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (5)
🛟 Help
|
…fers, relative dates included The deterministic Decision accepted 12 operators, three of which the editor could never build, and none of the date family beyond equality. A Decision on a date could not say "before", "after" or "in the previous 7 days", which is the first thing anyone writes on a date. The operator set is now the one a list view filter offers, 23 names: the three orphans (>=, <=, not_in) go, and 14 arrive — before/after, past/future, today/yesterday, previous_x_days(_to_date), before/after_x_hours_ago, starts_with/ends_with/i_contains, includes_all. Relative dates read an injected Clock, never the machine's. The orchestrator sends the project's timezone on the run (mapper falls back to UTC when unset or when the orchestrator predates it); the step executor builds one clock per step so every row sees the same instant. Day windows are half open in that zone, mirroring datasource-toolkit's time transforms, and a calendar date is read at midnight in that zone rather than UTC, otherwise Honolulu's own today would count as yesterday. i_contains folds case and keeps accents, measured against Postgres ILIKE. The trace persists evaluatedAt and timezone, without which a check on "previous 7 days" cannot be explained a day later. Product decisions recorded on PRD-1147: evaluation instant, project timezone, Postgres semantics for text, all operators in one delivery, and a retry that may route differently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…idate the project zone The start of a day window is included for a calendar date and excluded for a datetime, as datasource-toolkit's time transforms do. An unknown IANA zone falls back to UTC. includes_all is never met on an empty list. The SQL datetime form with a space is read as UTC too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… parse Postgres datetime forms A mid-day bound (past, before_x_hours_ago, the end of previous_x_days_to_date) is read at the start of its day when the value is a calendar date, as the toolkit formats the bound with toISODate for a Dateonly column. The SQL datetime form with a bare-hour or detached offset parses again. Schema tests cover every operator of the contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
45b4bf8 to
82e89d6
Compare
…compare The builder offers is greater than and is less than on a Time column, and both were never met: compare had no branch for a time of day, so the step routed to the fallback on every record with nothing in the trace to explain it.
The builder's time widget writes the hour and minutes only while a Time column holds the seconds too, so is, is not and is in on a Time column could never be met from the editor.
hercemer42
left a comment
hercemer42
left a comment
There was a problem hiding this comment.
Spec (PRD-1147, PRD-1130): conforms — the 23-operator set, the injected clock, the project-zone/UTC rule and the evaluatedAt/timezone trace are in place and pinned; "today included up to the instant" for previous_x_days_to_date holds for datetime columns, and a Dateonly column excludes today exactly as the list filter does (deliberate, documented in the package CLAUDE.md).
Applies to: the PR as a whole
Claude Fable 5.1 (claude-fable-5-1): Should fix — The next engineer touching either half of the relative-date rule (the front anchoring the list filter on the browser's zone, the orchestrator sending project.timezone) finds no decision record outside this package's CLAUDE.md and PRD-1147.
The diff fixes two cross-repo decisions that are hard to reverse once workflows are published on them: relative dates evaluate in the project's zone with a UTC fallback, never the viewer's or the host's (run-to-available-step-mapper.ts:182-190), and i_contains follows Postgres ILIKE across dialects (deterministic-condition-evaluator.ts:279-283); no ADR in docs/adr/** covers either (the org-wide search on timezone returns nothing).
Record them with /adr, one per decision.
|
|
||
| // Without this, "is greater than" on a Time column can never be met: the builder offers the | ||
| // operator, and the step would route to the fallback on every record without saying why. | ||
| const actualTime = toTimeOfDay(actual); |
There was a problem hiding this comment.
Applies to: deterministic-condition-evaluator.ts:111-125 (scalarEqual, not in this diff). This line is where the time-of-day rule is applied, and it is the only place.
Claude Fable 5.1 (claude-fable-5-1): Should fix — A Decision "is 08:30" on a Time column never matches a record stored as 08:30:00, and "is not 08:30" matches it, while "is greater than" and "is less than" both treat the pair as one instant.
compare pads and orders a time of day at :161-168, but scalarEqual (:111-125), which backs equal, not_equal, in and includes_all, still ends on typeof actual === typeof expected ? false : null, so '08:30:00' and '08:30' are two different strings; a Postgres or MySQL Time column arrives as HH:mm:ss and the builder's <input type="time"> writes HH:mm.
Parse both sides to milliseconds since midnight in one helper used by scalarEqual and compare, which also settles the fractional-precision ordering already raised on this PR.
Reproduction
- Time column
openingTimestored08:30:00in Postgres, read by a Get Data step. - Decision: option A
openingTimeis08:30(the time widget writes08:30), option BopeningTimeis not08:30, fallback C. - Run on that record: A is
met: false, B ismet: true; the list view filter "is 08:30" matches the same row.
There was a problem hiding this comment.
Right on both counts, and fixed in ceaaae3 — with the parse you suggest rather than the padding I had.
toMillisOfDay parses a time of day to milliseconds since midnight, and both scalarEqual and compare use it, so equal, not_equal, in, includes_all, greater_than and less_than all read 08:30 and 08:30:00 as one instant. That also settles the fractional-precision ordering raised by the bot on the same lines: 08:30:00.1 and 08:30:00.10 now compare equal instead of ordering by text length. A time nobody can be at (25:00, 08:70) parses to nothing rather than to a number.
Six assertions cover it: the two formats through equality and membership, the fraction, the out-of-range hours and minutes, and a non-time value.
Worth noting the starting point, since your reproduction understates it: before this PR greater_than and less_than on a Time column were never met at all, in any format — compare had no branch for a time of day, so the step took the fallback on every record with nothing in the trace.
previous week, month, quarter and year, and the same four up to the clock, with the toolkit's bounds: the whole previous period end excluded, or the current one from its start to now.
The Filters registry gained is not in (#9957), so the Decision offers it too. Membership answers true, false or not-comparable, so a value the members cannot be compared to does not satisfy the negated operator.
Comparing the strings ordered 08:30:00.10 before 08:30:00.1, the same instant written twice, and left equality to a padding rule. One parse now backs equality and ordering, and a time nobody can be at is refused.
2 participants
fixes PRD-1147, fixes PRD-1130
Why
A deterministic Decision on a Date field could only say
is,is not,is present,is blank. Forest exposesbefore/afterand a relative family for dates, none of which were in the contract, so the first condition anyone writes on a date could not be written. Meanwhile the contract carried three operators (>=,<=,not_in) that the editor can never build, since the front registry does not define them.What
Operator set = what a list view filter offers.
CONDITION_OPERATORSgoes from 12 to 23: the three orphans leave, fourteen arrive.before,afterpast,future,today,yesterday,previous_x_days,previous_x_days_to_date,before_x_hours_ago,after_x_hours_agostarts_with,ends_with,i_containsincludes_allRelative dates read an injected
Clock,{ now, timezone }, never the machine's. The evaluator stays pure. The step executor builds one clock per step so every row of a Decision sees the same instant.Timezone is the project's, carried on the run.
AvailableStepExecution.timezoneis new; the mapper falls back toUTCwhen the project has none or the orchestrator predates the field. The machine's zone is never used: the fleet runs several executor instances and the same run must route the same on each.Semantics mirror
datasource-toolkit's time transforms, which is what the list filter runs on: day windows exclude their end and, like the toolkit'sGreaterThan/GreaterThanOrEqualsplit, include their start for a calendar date only,previous_x_daysexcludes today,previous_x_days_to_dateincludes it up to the instant. A calendar date (Dateonly) is read at midnight in the project zone, otherwise a Honolulu record's own today is counted as yesterday.i_containsfolds case and keeps accents, measured on Postgres:'É' ILIKE '%é%'holds,'é' ILIKE '%e%'does not.The trace records
evaluatedAtandtimezone. A relative condition makes routing depend on the clock; without the instant, a check on "previous 7 days" cannot be explained a day later.Decisions taken with product (on PRD-1147)
Compatibility
Dropping
greater_than_or_equal,less_than_or_equalandnot_inmakes a published workflow that still carries one of them fail at parse on this executor (and be refused by the orchestrator's parser). The editor never offered them, and the deterministic Decision has only been open on executor 1.27.0 since 2 September, so no such workflow should exist; a hand-edited BPMN is the only way to have one. The orchestrator and the front move their gate back to the sentinel until this ships, since 1.27.0 refuses the fourteen new operators.Paired with
project.timezoneon the run.Until the orchestrator sends the timezone, relative dates resolve in UTC.
Tests
1724 passing. Every new operator is pinned with a fixed clock in
Europe/Paris, the calendar-date case inPacific/Honolulu, and the condition executor proves the context timezone reaches the evaluator with fake timers straddling midnight.Note
Add list-view filter operators and timezone-aware date evaluation to conditions
greater_than_or_equal,less_than_or_equal, andnot_infromCONDITION_OPERATORSin step-definition.ts; addsincludes_all,not_contains,starts_with,ends_with,i_contains,before,after,past,future,today,yesterday,previous_x_days,previous_x_days_to_date,before_x_hours_ago, andafter_x_hours_agoClockin deterministic-condition-evaluator.ts so relative date and time operators evaluate against a captured instant and project timezone;ConditionStepExecutorpersists the evaluation timestamp and timezone in execution paramstimezonefield toExecutionContextandAvailableStepExecutionSchema, validated as an IANA zone via Luxon;toAvailableStepExecutionin run-to-available-step-mapper.ts forwards the run timezone and falls back toUTCfor missing or invalid valuesluxonas a runtime dependency in package.json for IANA zone validation and timezone-aware date mathConditionStepDefinitionSchemarejects the three removed operators at the schema boundary;AvailableStepExecutionSchemarejects available-step values that omittimezoneor contain an invalid IANA zone;ExecutionContext.timezoneis now required, so all callers and test fixtures must supply it;greater_thanandless_thannow use strict comparison instead of inclusiveChanges since #1880 opened
workflow-executorpackage's deterministic condition evaluation system [0148a2a]memberOfutility function indeterministic-condition-evaluatorto return tri-state results (true/false/null) instead of boolean, addednot_inoperator to theEVALUATORSmap that evaluates to true only when candidate is definitively not a member of the expected list, and updatedinoperator to check for definite membership using tri-state result [fe1ff08]CONDITION_OPERATORSconstant instep-definitiontypes to includenot_inoperator, increasing the total count from 31 to 32 operators [fe1ff08]not_inoperator behavior and updated operator validation tests to acceptnot_inas a valid operator [fe1ff08]CLAUDE.mdto reflect the addition ofnot_inoperator and clarify tri-state membership semantics [fe1ff08]Macroscope summarized 3486a64.