Skip to content

Commit 7125007

Browse files
os-zhuangclaude
andauthored
fix(formula): null-guarded numeric formulas + floor/ceil + reject date arithmetic (#3306) (#3348)
* chore: bump objectui to 3b2e4d98d904 fix(list): route remaining system-field groupings through shared classifier (#2706) objectui@3b2e4d98d904d695a8372c394d46b81673011270 * fix(formula): null-guarded numeric formulas + floor/ceil + reject date arithmetic (#3306) Shipped template `Field.formula` fields (hr_employee.tenure_years, hr_time_off_request.days) silently evaluated to null on the pinned runtime. Three independent CEL-engine gaps, now closed: 1. `cond ? <value> : null` — cel-js's ternary unifier rejects a concrete int/double/string branch against null (even `true ? 5 : null` faulted), so the blessed null-guard idiom nulled. AST pre-pass wraps the non-null branch in dyn() (value-preserving, null-branch-only, idempotent) so it compiles AND evaluates. Applied in compile(), evaluate(), and the build soundness check. Mirrors the #3183 rewriteTemporalEquality pattern. 2. floor/ceil were unregistered — `floor(...)` faulted and nulled. Registered (return int, round toward -inf/+inf, NOT trunc) + added to the catalog (CEL_STDLIB_FUNCTIONS + objectstack-formula SKILL.md). 3. Date arithmetic (`date - date + 1`, `today() + 30`) compiled clean (dyn operands) but always nulls at runtime. The soundness check now types date/datetime fields as Timestamp and flags date/duration ARITHMETIC against a number as a hard build ERROR pointing at daysBetween/daysFromNow/ addDays. Sound by construction: ordering, equality, and string concat of a date field stay clean (runtime-tolerated); a `!= null` guard no longer masks the inner fault (== null no-op overloads in the check-only env). Tests: null-guard idiom (compile+eval, incl. the two hr templates), floor/ceil semantics, date-arith RED on the shipped bug (incl. behind a null guard) and GREEN on every runtime-tolerated date use, stack-level validateStackExpressions. Fixes #3306. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 62a2117 commit 7125007

8 files changed

Lines changed: 420 additions & 55 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
**Stored `Field.formula` fields that compute dates/durations no longer silently evaluate to `null` (#3306).** Three independent CEL gaps made shipped template formulas (e.g. `hr_employee.tenure_years`, `hr_time_off_request.days`) return `null` with no parse/build/runtime error:
6+
7+
1. **The null-guard idiom `cond ? <value> : null` now compiles and evaluates.** cel-js's ternary type-unifier rejects a concrete `int`/`double`/`string` branch against `null` — so even `true ? 5 : null` faulted *"Ternary branches must have the same type"* and the whole formula nulled. A `Field.formula` is inherently nullable and the catalog blesses both ternary and `== null`, so this is the canonical "compute value, else blank" shape. An AST pre-pass (mirroring the #3183 temporal-equality rewrite) wraps the non-null branch in `dyn(...)` — value-preserving, null-branch-only, idempotent — so it type-checks and runs. Applied in `compile()`, `evaluate()`, and the build soundness check alike.
8+
9+
2. **`floor(x)` / `ceil(x)` are now registered** (parallel to `round`/`abs`) and advertised in the catalog. They round toward −∞ / +∞, so `floor(-1.2) == -2` — NOT interchangeable with integer division's round-toward-zero. Previously `floor(...)` faulted `found no matching overload` and the formula nulled.
10+
11+
3. **Date arithmetic is now a build-time ERROR instead of a silent runtime `null`.** `record.end_date - record.start_date + 1`, `today() + 30`, `record.date + n` type-check clean (operands are `dyn`) but always fault at runtime and never recover (a date string is not numeric, so hydration can't rescue it). The build soundness check now types `date`/`datetime` fields as `google.protobuf.Timestamp` and flags date/duration **arithmetic against a number** with a corrective message pointing at `daysBetween(a, b)` / `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)`. Sound by construction — ordering (`date < today()`, `date < "2026-01-01"` string-lex), equality (#3183), and string concatenation (`"Due: " + date`) are all runtime-tolerated and never flagged; only arithmetic against a number is. A `!= null` guard on a date field no longer masks the inner fault (`== null` no-op overloads registered in the check-only env).
12+
13+
> **Heads-up for downstream:** (3) adds a NEW build-time error. A stored formula or predicate doing arithmetic on a `date`/`datetime` field (`end - start + 1`, `today() + 30`) that previously built (and nulled at runtime) will now fail `objectstack build` / `validateStackExpressions` with a message telling you to use `daysBetween` / `daysFromNow` / `addDays`. This only fires for genuinely-broken expressions that already returned `null`.
14+
15+
Fixes #3306.

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,17 @@ describe('celEngine', () => {
465465
expect(r).toEqual({ ok: true, value: true });
466466
});
467467

468-
it('abs / round / min / max', () => {
468+
it('abs / round / floor / ceil / min / max', () => {
469469
expect(celEngine.evaluate(cel('abs(record.x)'), { record: { x: -3.5 } })).toEqual({ ok: true, value: 3.5 });
470470
expect(celEngine.evaluate(cel('round(2.6)'), {})).toEqual({ ok: true, value: 3 });
471+
expect(celEngine.evaluate(cel('floor(6.7)'), {})).toEqual({ ok: true, value: 6 });
472+
expect(celEngine.evaluate(cel('ceil(6.1)'), {})).toEqual({ ok: true, value: 7 });
473+
// floor/ceil round toward −∞/+∞, NOT toward zero — the whole reason they are
474+
// not interchangeable with integer division (#3306).
475+
expect(celEngine.evaluate(cel('floor(-1.2)'), {})).toEqual({ ok: true, value: -2 });
476+
expect(celEngine.evaluate(cel('ceil(-1.2)'), {})).toEqual({ ok: true, value: -1 });
477+
// A record number field arrives as a cel-js `double`; `floor` must accept it.
478+
expect(celEngine.evaluate(cel('floor(record.x / 365.0)'), { record: { x: 2318 } })).toEqual({ ok: true, value: 6 });
471479
expect(celEngine.evaluate(cel('min(record.a, record.b)'), { record: { a: 3, b: 7 } })).toEqual({ ok: true, value: 3 });
472480
expect(celEngine.evaluate(cel('max(record.a, record.b)'), { record: { a: 3, b: 7 } })).toEqual({ ok: true, value: 7 });
473481
});
@@ -495,6 +503,7 @@ describe('celEngine', () => {
495503
daysBetween: 'daysBetween(today(), daysFromNow(7))', date: 'date("2026-03-15")',
496504
addDays: 'addDays(today(), 7)', addMonths: 'addMonths(today(), 3)',
497505
datetime: 'datetime("2026-03-15T08:00:00Z")', abs: 'abs(-3.5)', round: 'round(2.6)',
506+
floor: 'floor(6.7)', ceil: 'ceil(6.1)',
498507
min: 'min(1, 2)', max: 'max(1, 2)', upper: 'upper("hi")', lower: 'lower("HI")',
499508
trim: 'trim(" x ")', contains: 'contains("hello", "ell")', startsWith: 'startsWith("hi", "h")',
500509
endsWith: 'endsWith("hi", "i")', matches: 'matches("a1", "a.")', joinNonEmpty: 'joinNonEmpty(["a", "b"], "-")',
@@ -588,4 +597,46 @@ describe('celEngine', () => {
588597
.toEqual({ ok: true, value: false });
589598
});
590599
});
600+
601+
// #3306 — the blessed null-guard idiom `cond ? <value> : null`. cel-js's ternary
602+
// unifier rejects a concrete branch against `null`; the engine's AST rewrite
603+
// wraps the non-null branch in `dyn(...)` so it compiles AND evaluates, and the
604+
// null branch still yields null.
605+
describe('null-guarded formula idiom (#3306)', () => {
606+
const now = new Date('2026-07-20T00:00:00Z');
607+
608+
it('`cond ? <number> : null` evaluates instead of silently nulling', () => {
609+
expect(celEngine.evaluate(cel('true ? 5 : null'), {})).toEqual({ ok: true, value: 5 });
610+
expect(celEngine.evaluate(cel('false ? 5 : null'), {})).toEqual({ ok: true, value: null });
611+
// null-first order (either branch may be the null literal).
612+
expect(celEngine.evaluate(cel('true ? null : 5'), {})).toEqual({ ok: true, value: null });
613+
});
614+
615+
it('compiles the idiom that used to fault type-checking', () => {
616+
// Was: ERR[type] "Ternary branches must have the same type, got 'int' and 'null'".
617+
expect(celEngine.compile('true ? 5 : null').ok).toBe(true);
618+
expect(celEngine.compile('cond ? daysBetween(a, b) + 1 : null').ok).toBe(true);
619+
});
620+
621+
it('the shipped hr templates now compute (tenure_years, time_off.days)', () => {
622+
// tenure_years — daysBetween/365 integer division, null when hire_date unset.
623+
const tenure = cel('record.hire_date != null ? daysBetween(record.hire_date, today()) / 365 : null');
624+
expect(celEngine.evaluate(tenure, { now, record: { hire_date: '2020-03-15' } }))
625+
.toEqual({ ok: true, value: 6 });
626+
expect(celEngine.evaluate(tenure, { now, record: { hire_date: null } }))
627+
.toEqual({ ok: true, value: null });
628+
// time_off.days — inclusive calendar-day span.
629+
const days = cel('record.start_date != null && record.end_date != null ? daysBetween(record.start_date, record.end_date) + 1 : null');
630+
expect(celEngine.evaluate(days, { now, record: { start_date: '2026-06-20', end_date: '2026-06-24' } }))
631+
.toEqual({ ok: true, value: 5 });
632+
});
633+
634+
it('leaves a genuine type mismatch and same-typed / non-null ternaries alone', () => {
635+
// Not a null-guard → still an honest type error (we only relax `… : null`).
636+
expect(celEngine.compile('true ? "a" : 5').ok).toBe(false);
637+
// Same-typed branches never needed the rewrite and are unchanged.
638+
expect(celEngine.evaluate(cel('true ? "a" : "b"'), {})).toEqual({ ok: true, value: 'a' });
639+
expect(celEngine.evaluate(cel('false ? 1 : 2'), {})).toEqual({ ok: true, value: 2 });
640+
});
641+
});
591642
});

0 commit comments

Comments
 (0)