Skip to content

Commit 078e28b

Browse files
os-zhuangclaude
andauthored
fix(formula): cover a Date-valued binding in the temporal-equality rewrite (#7168) (#8160)
* fix(formula): cover a Date-valued binding in the temporal-equality rewrite (#7168) `record.due == previous.due` — `previous` hydrated by the driver as a `Date`, `record` parsed from a JSON payload as `"2026-06-20"` — compared a string against a Timestamp and answered a silent `false` (`!=` answered a silent `true`). No fault, no log line: the wrong answer is shaped like a legitimate one. `rewriteTemporalEquality` (#3183) already coerced the string operand with `date(...)` when its counterpart was a temporal CALL. It now also fires when the counterpart is a Date-valued BINDING. A binding's runtime type is not in the AST, so this arm reads the evaluation scope — the same discipline `rewriteFaultedOperands` (#7098) uses — and its verdict is never cached against the source, because it is a property of the row and not of the expression. What IS cached per source is the analysis: which occurrences could depend on a scope at all, so sources without one keep the memoized static path. The coercion requires the counterpart to be a real `Date` AND this operand to be an ISO-8601 string that parses, which is what keeps the fix from becoming the mirror-image defect. `"5"` and `"05"` are different strings that both parse to 2001-05-01; wrapping unconditionally would invent an equality between them and turn a correct `false` into a silent `true`. Fact 1 of the card (cross-type `in` membership) is deliberately untouched, per the maintainer ruling of 2026-08-12: record-and-defer pending a measured victim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C8pAprWdmtecFsEprZax4 * docs(formulas): name the Date-valued-binding counterpart in the temporal-equality callout (#7168) The callout describes the rewrite this PR extends, and its only named counterpart was a temporal call — so after the code change it stayed true but became incomplete in the direction that matters: a reader would still believe `record.due == previous.due` silently answers `false`. Extends that paragraph only: the counterpart may be a temporal call OR a binding holding a `Date`, named through the reachable mixed-provenance shape. Also states the fence, so the sentence cannot be read as "any string now matches a date": the coercion needs one side to be a real `Date` and the other an ISO-8601 string. No other prose touched, and cross-type `in` membership is deliberately not mentioned — it is record-and-defer per the maintainer ruling on #7168. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C8pAprWdmtecFsEprZax4 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9172fa2 commit 078e28b

4 files changed

Lines changed: 395 additions & 12 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/formula": patch
3+
---
4+
5+
fix(formula): `==` / `!=` between a date STRING field and a Date-valued binding no longer answers a silent `false` (#7168)
6+
7+
A mixed-provenance comparison — the shape a hook or validation predicate writes
8+
every day — returned the wrong boolean with no fault and no log line:
9+
10+
```text
11+
record.due == previous.due
12+
with { record: { due: "2026-06-20" }, previous: { due: Date(2026-06-20T00:00:00Z) } }
13+
-> { ok: true, value: false } // same field, same instant
14+
```
15+
16+
`previous` arrives from the driver hydrated as a `Date`; `record` arrives from a
17+
JSON payload as a `"YYYY-MM-DD"` string. cel-js compares a `string` against a
18+
`google.protobuf.Timestamp` and never matches, so the predicate answered `false`
19+
— and `!=` on the same pair answered `true`. Nothing errored, so nothing pointed
20+
at it. This is the failure class that hurts most in an AI-authored filter: the
21+
wrong answer is shaped exactly like a legitimate one.
22+
23+
`rewriteTemporalEquality` already fixed this for a temporal **call** counterpart
24+
(`record.due == today()`, #3183) by coercing the string operand with `date(...)`.
25+
It now covers a Date-valued **binding** counterpart as well. A binding's runtime
26+
type is not visible in the AST, so this arm is decided per row against the values
27+
in the evaluation scope, and its verdict is deliberately never cached against the
28+
expression source.
29+
30+
**Comparisons that change answer** — one operand an ISO-8601 date/date-time
31+
string, the other a binding holding a `Date`:
32+
33+
- `record.due == previous.due` (same instant) — was `false`, now `true`
34+
- `record.due != previous.due` (same instant) — was `true`, now `false`
35+
- either operand order, and a `"…T14:33:00Z"` string against the same instant
36+
37+
**Comparisons that deliberately do NOT change** — the coercion requires the
38+
counterpart to be a real `Date` *and* this operand to be an ISO-8601 string that
39+
parses, so everything below answers exactly as it did before:
40+
41+
- two strings — `"2026-06-20" == "2026-06-20"` stays STRING equality
42+
- two `Date`s — already compared as instants
43+
- a different calendar day — stays `false`
44+
- a non-date string against a `Date` (`"hello"`) — stays `false`
45+
- a **numeric** string against a `Date` (`"5"`) — stays `false`. Load-bearing:
46+
`new Date("5")` and `new Date("05")` both parse to 2001-05-01, so coercing here
47+
would invent an equality between two different strings
48+
- a date-ONLY string against a `Date` carrying wall-clock time — stays `false`.
49+
`date()` parses, it does not truncate to a calendar day, and those are
50+
genuinely different instants; truncating both sides would turn a correct
51+
`false` into a wrong `true` for real datetime comparisons
52+
- ordering (`<` / `>=`) is untouched — that path is ADR-0032 §1c's retry
53+
- a string LITERAL counterpart is untouched — it is not a binding
54+
55+
Cross-type `in` membership (`record.n in [1, 7]` with `n: "7"`) is a separate
56+
clean-path question and is unchanged, deferred by maintainer ruling on #7168
57+
pending a measured victim.

content/docs/data-modeling/formulas.mdx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -452,15 +452,23 @@ easiest way to ship a formula whose sign is inverted on every row.
452452
</Callout>
453453

454454
<Callout type="info">
455-
**`dateField == today()` now matches (#3183).** A `date` field reads back as a
456-
`YYYY-MM-DD` string, and CEL treats a string and a timestamp as unequal — so the
457-
natural "is it due today" predicate used to silently return `false`. The engine
458-
now rewrites temporal `==` / `!=` comparisons (coercing the field operand with
459-
`date(...)`), so `record.due_date == today()` matches on the calendar day. This
460-
applies to formulas, defaults, validation rules, and hook/flow conditions.
461-
Ordering (`< > <= >=`) and string equality (`record.d == "2026-06-20"`) were
462-
always fine. This is the read-side counterpart to the date-arithmetic build
463-
error above — equality is rewritten and works; `+`/`-` arithmetic is rejected.
455+
**`dateField == today()` now matches (#3183, #7168).** A `date` field reads back
456+
as a `YYYY-MM-DD` string, and CEL treats a string and a timestamp as unequal — so
457+
the natural "is it due today" predicate used to silently return `false`. The
458+
engine now rewrites temporal `==` / `!=` comparisons (coercing the field operand
459+
with `date(...)`), so `record.due_date == today()` matches on the calendar day.
460+
The counterpart may be a temporal **call** (`today()`, `now()`, `daysFromNow()`,
461+
`daysAgo()`) or a **binding that holds a `Date`** — which covers the
462+
mixed-provenance comparison `record.due == previous.due`, where `previous`
463+
arrives from the driver as a `Date` while `record` arrives from a JSON payload as
464+
a `YYYY-MM-DD` string (#7168). This applies to formulas, defaults, validation
465+
rules, and hook/flow conditions. Ordering (`< > <= >=`) and string equality
466+
(`record.d == "2026-06-20"`) were always fine. The coercion is deliberately
467+
narrow — it requires one side to be a real `Date` and the other an ISO-8601
468+
date/date-time string — so a non-date string compared against a `Date` still
469+
answers `false` instead of being coerced into a match. This is the read-side
470+
counterpart to the date-arithmetic build error above — equality is rewritten and
471+
works; `+`/`-` arithmetic is rejected.
464472
</Callout>
465473

466474
### Financial Calculations

packages/formula/src/cel-engine.test.ts

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,79 @@ describe('celEngine', () => {
557557
expect(rewriteTemporalEquality('$'.repeat(5000))).toBe('$'.repeat(5000));
558558
expect(rewriteTemporalEquality('now('.repeat(2000))).toBe('now('.repeat(2000));
559559
});
560+
561+
// #7168 — the Date-valued BINDING arm. The counterpart is not a temporal
562+
// call but a binding that HOLDS a Date at evaluation time, which the AST
563+
// cannot see — so this arm only fires when a scope is supplied.
564+
describe('Date-valued binding counterpart (#7168)', () => {
565+
const day = new Date('2026-06-20T00:00:00Z');
566+
const mixed = { record: { due: '2026-06-20' }, previous: { due: day } };
567+
568+
it('wraps the ISO-string operand when its counterpart is a Date-valued binding', () => {
569+
expect(rewriteTemporalEquality('record.due == previous.due', mixed))
570+
.toBe('date(record.due) == previous.due');
571+
expect(rewriteTemporalEquality('previous.due == record.due', mixed))
572+
.toBe('previous.due == date(record.due)');
573+
expect(rewriteTemporalEquality('record.due != previous.due', mixed))
574+
.toBe('date(record.due) != previous.due');
575+
});
576+
577+
it('never fires without a scope — the runtime type of a binding is not in the AST', () => {
578+
expect(rewriteTemporalEquality('record.due == previous.due'))
579+
.toBe('record.due == previous.due');
580+
});
581+
582+
it('is idempotent — an already-coerced operand is not re-wrapped', () => {
583+
expect(rewriteTemporalEquality('date(record.due) == previous.due', mixed))
584+
.toBe('date(record.due) == previous.due');
585+
});
586+
587+
it('composes with the temporal-call arm on one expression', () => {
588+
expect(rewriteTemporalEquality('record.due == today() && record.due == previous.due', mixed))
589+
.toBe('date(record.due) == today() && date(record.due) == previous.due');
590+
});
591+
592+
// ── The fence: nothing else may start being coerced ──────────────────
593+
it('leaves every comparison that is NOT string-vs-Date untouched', () => {
594+
// two ISO strings — the author's string equality, still string equality
595+
expect(rewriteTemporalEquality('record.due == previous.due',
596+
{ record: { due: '2026-06-20' }, previous: { due: '2026-06-20' } }))
597+
.toBe('record.due == previous.due');
598+
// two Dates — already compares as instants, nothing to coerce
599+
expect(rewriteTemporalEquality('record.due == previous.due',
600+
{ record: { due: day }, previous: { due: day } }))
601+
.toBe('record.due == previous.due');
602+
// a non-date string opposite a Date — a genuine mismatch, not a
603+
// serialization artifact. `date("hello")` is an Invalid Date.
604+
expect(rewriteTemporalEquality('record.a == previous.b',
605+
{ record: { a: 'hello' }, previous: { b: day } }))
606+
.toBe('record.a == previous.b');
607+
// a NUMERIC string opposite a Date. Load-bearing: `new Date("5")` and
608+
// `new Date("05")` both parse to 2001-05-01, so coercing here would
609+
// invent an equality between two different strings.
610+
expect(rewriteTemporalEquality('record.a == previous.b',
611+
{ record: { a: '5' }, previous: { b: day } }))
612+
.toBe('record.a == previous.b');
613+
// a number opposite a Date — not a string, nothing to coerce
614+
expect(rewriteTemporalEquality('record.a == previous.b',
615+
{ record: { a: 5 }, previous: { b: day } }))
616+
.toBe('record.a == previous.b');
617+
// null / absent operands
618+
expect(rewriteTemporalEquality('record.a == previous.b',
619+
{ record: { a: null }, previous: { b: day } }))
620+
.toBe('record.a == previous.b');
621+
expect(rewriteTemporalEquality('record.a == previous.b', { previous: { b: day } }))
622+
.toBe('record.a == previous.b');
623+
// ORDERING against a Date binding is untouched — this arm is `==`/`!=`
624+
// only; `<`/`>` against a string-serialized field is ADR-0032 §1c's
625+
// retry path (#7098), which fires on a fault and is not clean-path.
626+
expect(rewriteTemporalEquality('record.due >= previous.due', mixed))
627+
.toBe('record.due >= previous.due');
628+
// a string LITERAL counterpart is not a binding
629+
expect(rewriteTemporalEquality('record.due == "2026-06-20"', mixed))
630+
.toBe('record.due == "2026-06-20"');
631+
});
632+
});
560633
});
561634

562635
// #3183 — the end-to-end runtime behavior the rewrite delivers: a `Field.date`
@@ -598,6 +671,84 @@ describe('celEngine', () => {
598671
});
599672
});
600673

674+
// #7168 — the end-to-end behavior of the Date-valued-binding arm. The
675+
// reachable shape is a MIXED-PROVENANCE comparison: `previous` hydrated by the
676+
// driver as a `Date`, `record` parsed from a JSON payload as a `YYYY-MM-DD`
677+
// string. Same logical field, same instant, and the predicate answered a
678+
// silent `false` — `{ ok: true, value: false }`, no fault, no log line.
679+
describe('date-string == Date-valued binding runtime fix (#7168)', () => {
680+
const now = new Date('2026-06-20T08:00:00Z');
681+
const midnight = new Date('2026-06-20T00:00:00Z');
682+
const row = (due: unknown, prev: unknown) => ({ now, record: { due }, previous: { due: prev } });
683+
684+
it('a same-day mixed-provenance comparison answers true instead of a silent false', () => {
685+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('2026-06-20', midnight)))
686+
.toEqual({ ok: true, value: true });
687+
// operand order does not matter
688+
expect(celEngine.evaluate(cel('previous.due == record.due'), row('2026-06-20', midnight)))
689+
.toEqual({ ok: true, value: true });
690+
// `!=` was the same defect inverted — it answered a silent `true`
691+
expect(celEngine.evaluate(cel('record.due != previous.due'), row('2026-06-20', midnight)))
692+
.toEqual({ ok: true, value: false });
693+
});
694+
695+
it('a datetime string matches the same INSTANT (date() parses, it does not truncate)', () => {
696+
const afternoon = new Date('2026-06-20T14:33:00Z');
697+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('2026-06-20T14:33:00Z', afternoon)))
698+
.toEqual({ ok: true, value: true });
699+
});
700+
701+
// ── The fence: what SHOULD stay false still does ──────────────────────
702+
it('a different calendar day stays false', () => {
703+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('2026-06-19', midnight)))
704+
.toEqual({ ok: true, value: false });
705+
expect(celEngine.evaluate(cel('record.due != previous.due'), row('2026-06-19', midnight)))
706+
.toEqual({ ok: true, value: true });
707+
});
708+
709+
it('a date-ONLY string against a Date carrying wall-clock time stays false', () => {
710+
// Deliberately not "fixed": these are genuinely different instants, and
711+
// truncating both sides to a calendar day would flip a correct `false`
712+
// into a wrong `true` for real datetime comparisons.
713+
expect(celEngine.evaluate(cel('record.due == previous.due'),
714+
row('2026-06-20', new Date('2026-06-20T14:33:00Z'))))
715+
.toEqual({ ok: true, value: false });
716+
});
717+
718+
it('non-date and numeric strings against a Date stay false — no invented equality', () => {
719+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('hello', midnight)))
720+
.toEqual({ ok: true, value: false });
721+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('5', midnight)))
722+
.toEqual({ ok: true, value: false });
723+
expect(celEngine.evaluate(cel('record.due == previous.due'), row(null, midnight)))
724+
.toEqual({ ok: true, value: false });
725+
});
726+
727+
it('a string-vs-string comparison keeps STRING equality, including the lenient-parse pairs', () => {
728+
// "5" and "05" are different strings; both parse to 2001-05-01 as dates.
729+
// Neither side is a Date binding, so no coercion happens and they differ.
730+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('5', '05')))
731+
.toEqual({ ok: true, value: false });
732+
expect(celEngine.evaluate(cel('record.due == previous.due'), row('2026-06-20', '2026-06-20')))
733+
.toEqual({ ok: true, value: true });
734+
});
735+
736+
it('the verdict is per ROW, not cached against the source', () => {
737+
// The binding arm depends on values, not on the expression — so the same
738+
// source must be re-decided for every scope. If its result were memoized
739+
// under the source (as the temporal-CALL arm safely is), row 2 would
740+
// evaluate `date(record.due) == previous.due`, comparing a Timestamp
741+
// against a string, and answer a wrong `false`.
742+
const src = cel('record.due == previous.due');
743+
expect(celEngine.evaluate(src, row('2026-06-20', midnight))).toEqual({ ok: true, value: true });
744+
expect(celEngine.evaluate(src, row('2026-06-20', '2026-06-20'))).toEqual({ ok: true, value: true });
745+
// …and in the other order, to catch poisoning either way.
746+
const src2 = cel('previous.due == record.due');
747+
expect(celEngine.evaluate(src2, row('2026-06-20', '2026-06-20'))).toEqual({ ok: true, value: true });
748+
expect(celEngine.evaluate(src2, row('2026-06-20', midnight))).toEqual({ ok: true, value: true });
749+
});
750+
});
751+
601752
// #3306 — the blessed null-guard idiom `cond ? <value> : null`. cel-js's ternary
602753
// unifier rejects a concrete branch against `null`; the engine's AST rewrite
603754
// wraps the non-null branch in `dyn(...)` so it compiles AND evaluates, and the

0 commit comments

Comments
 (0)