Skip to content

fix(formula): cover a Date-valued binding in the temporal-equality rewrite (#7168) - #8160

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-7168-temporal-eq-date-binding
Aug 12, 2026
Merged

fix(formula): cover a Date-valued binding in the temporal-equality rewrite (#7168)#8160
os-zhuang merged 3 commits into
mainfrom
claude/issue-7168-temporal-eq-date-binding

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes#7168

Implements Fact 2 only, per the maintainer ruling of 2026-08-12 11:15Z on the card.

The defect

A mixed-provenance comparison — previous hydrated by the driver as a Date, record parsed from a JSON payload as a "YYYY-MM-DD" string — answered the wrong boolean:

record.due == previous.due
with { record: { due: "2026-06-20" }, previous: { due: Date(2026-06-20T00:00:00Z) } }
-> { ok: true, value: false } // same logical field, same instant

cel-js compares a string against a google.protobuf.Timestamp and never matches, so the predicate answered false — and != on the same pair answered true. No fault, no log line. Measured on origin/main before the fix ({"ok":true,"value":false}), and both are pinned red-without-the-fix below.

The fix

rewriteTemporalEquality (#3183) already coerced the string operand with date(...) when its counterpart was a temporal call (today() / now() / daysFromNow() / daysAgo()). It now also fires when the counterpart is a Date-valued binding.

On the design question the dispatch flagged as unverified — how a Date-valued binding is distinguishable at rewrite time: it is not visible in the AST at all.record.due and previous.due are the same shape whether either holds a Date, a string, or null, and under unlistedVariablesAreDyn the static type says nothing either. So the analogy to the temporal-call arm does not carry: that arm is a pure function of the source and is memoized under it, whereas this verdict is a property of the row.

Rather than force that shape, this arm reads the evaluation scope — the discipline rewriteFaultedOperands (#7098) already established, reusing its scopePath / resolveScopePath / coercionFor primitives. Two consequences, both deliberate:

  • the arm runs only where a scope exists (evaluate), never in compile;
  • its result is never cached against the source, or row 1's verdict would be applied to row 2. What is cached per source is the analysis — which occurrences could depend on a scope at all — so the overwhelming majority of sources keep the memoized static path and never reparse.

Keeping the negative pins

The coercion requires the counterpart to be a real Dateand this operand to be an ISO-8601 string that parses. That guard is what keeps the fix from becoming the mirror-image defect: date() is toDate, i.e. new Date(String(v)), and JS date parsing is lenient enough to invent equalities — "5" and "05" are different strings that both parse to 2001-05-01. Wrapping unconditionally would turn a correct false into a silent true.

Changes answer (string plus Date binding, same instant):

  • record.due == previous.due — was false, now true
  • record.due != previous.due — was true, now false
  • either operand order; a "...T14:33:00Z" string against the same instant

Deliberately unchanged, each pinned:

  • two strings — stays STRING equality; two Dates — already compared as instants
  • a different calendar day — stays false
  • a non-date string ("hello") or a numeric string ("5") against a Date — stays false
  • a date-ONLY string against a Date carrying wall-clock time — stays false. date() parses, it does not truncate to a calendar day, and those are genuinely different instants; truncating both sides would make record.dt == previous.dt a day-granularity comparison, which is the same silent-wrong-answer defect pointing the other way
  • ordering (>=) is untouched — that is ADR-0032 §1c's retry path
  • a string LITERAL counterpart is untouched — it is not a binding

Docs

content/docs/data-modeling/formulas.mdx carries a callout describing this exact rewrite, and its only named counterpart was a temporal call. After this change the paragraph stays true but becomes incomplete in the direction that matters — a reader would still believe record.due == previous.due silently answers false. Prose asserting a mechanism that has since moved stays green forever, because no gate reads prose, so the PR that changes the behaviour is the only one that can honestly change the sentence.

That paragraph is extended to name the binding counterpart and the mixed-provenance shape, plus the fence, so it cannot be read as "any string now matches a date". Scope held deliberately: no other prose touched, nothing under content/docs/releases/**, and cross-type in membership is not mentioned at all — it is record-and-defer per the ruling. (content/docs/** is domain:devx's surface; the engine-core PM seat is filing that declaration.)

Fact 1 is NOT in this PR, and its deferral survives this closure

The card reports two facts. Fact 1 — cross-type in membership (record.n in [1, 7] with n: "7") — is untouched here: the 11:15Z ruling is record-and-defer, no change to clean-path cross-type semantics without a measured victim, with the reopen trigger being the first measured wrong answer attributable to it. It was deliberately not filed as a separate card (zero measured pull), so that ruling comment on #7168 is the durable record. This PR uses Fixes #7168 because the ruling says so explicitly — closing the card does not retract Fact 1's deferral, and a future reader should not read the closure as burying it.

Verification

  • Reverse-verified: with the source reverted to origin/main and the new tests kept, exactly 5 tests fail — all 5 the new arm, including the card's literal defect (expected { ok: true, value: false } to deeply equal { ok: true, value: true }) — while the other 80 pass. Every negative pin is green on both sides, which is what makes it a fence rather than a restatement of the fix.
  • pnpm --filter @objectstack/formula test — 25 files, 643 tests pass; typecheck clean; ESLint clean on both changed files.
  • Downstream consumers that evaluate CEL predicates: @objectstack/objectql 3390 tests, @objectstack/service-automation 940 tests — all pass.
  • Gates: check:driver-conformance, check:query-options-erasure, check:nul-bytes, check:type-check-debt --re-measure (after a full closure build), plus the docs-path family pulled in by the .mdx edit — check:doc-formula-expressions, check:docs-audit-scope, check:quick-reference-counts, check:role-word — and the changeset family. All OK. No baseline raised; --lower not run.
  • main advanced 7 commits mid-task and one of them bumped .objectui-sha, so check:objectui-pin-fresh read stale against the older base. Resolved by merging origin/main (not by hand-editing another lane's pin artifact); green afterwards, and the whole suite plus both named ratchets were re-run against the merged tree.

Generated by Claude Code

…write (#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
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 7:05pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/formula.

4 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/formulas.mdx(via @objectstack/formula)
  • content/docs/data-modeling/validation.mdx(via @objectstack/formula)
  • content/docs/plugins/packages.mdx(via @objectstack/formula)
  • content/docs/protocol/objectui/record-alert.mdx(via @objectstack/formula)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v15.mdx(via @objectstack/formula)
  • content/docs/releases/v16.mdx(via @objectstack/formula)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…ral-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
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 19:24
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 078e28bAug 12, 2026
28 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7168-temporal-eq-date-binding branch August 12, 2026 19:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] two §1c-adjacent silent wrong answers that are NOT on the retry path — cross-type in membership, and == against a Date-valued binding

2 participants

@os-zhuang@claude