Skip to content

fix(formula): arm the CEL hydration retry off cel-js's structured code (#6679) - #7097

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-6679-overload-structured-code
Aug 9, 2026
Merged

fix(formula): arm the CEL hydration retry off cel-js's structured code (#6679)#7097
os-zhuang merged 1 commit into
mainfrom
claude/issue-6679-overload-structured-code

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #6679.

What changed

packages/formula/src/cel-engine.tsisNumericOverloadError, the predicate that decides whether celEngine.evaluate runs the ADR-0032 §1c hydration retry, no longer reads the error's prose:

-  const message = err instanceof Error ? err.message : String(err);
-  return /no such overload/i.test(message);
+  return err instanceof EvaluationError && err.code === CEL_NO_SUCH_OVERLOAD_CODE;

This was the last message-text read in the file that armed behaviour after #6223 / PR #6677 deleted the keyword table in classifyError. The structured answer already lived one function below — classifyCelFault does err instanceof EvaluationError and reads err.code — so this is the same rule applied to the one place still exempt from it.

What was measured

Against cel-js 8.0.0 as installed, not against the card:

Claim Measured
The §1c fault is an EvaluationError err.constructor.name === 'EvaluationError', instanceof true
Its code 'no_such_overload' (raised from lib/operators.js)
matches() escapes cel-js unwrapped matches(record.name, "no such overload(")Invalid regular expression: /no such overload(/: Unterminated group — a native SyntaxError, no cel-js source highlight
…and from a row too matches(record.name, record.re) with re off the record — same message

The pinning against cel-js is itself a test, so an upstream re-code or re-class goes red here rather than silently disarming the retry.

The behaviour change, and a correction to the filing

The change is one-directional, as filed: a native throw whose message merely contains the phrase stops arming the retry. Genuine no_such_overload faults are untouched.

The filing recorded this as observation-class — "Today the consequence appears to be nil… What is unmeasured is whether any input exists where a spuriously-armed retry succeeds" — and triage graded it not target:v17 on that basis. Measuring it for the fix found such an input, so the pricing here is a fix rather than a tolerance removal:

record.s == "5.0" ? matches(record.name, "no such overload(") : false
  { s: "5.0", name: "x" }   ->  was: { ok: true, value: false }   now: the regex fault
record.s == "5.0" ? matches(record.name, "(") : false
  { s: "5.0", name: "x" }   ->  the regex fault                   (unchanged)

Evaluation 1 takes the matches(...) branch and throws natively; the phrase armed the retry; hydration made record.s the number 5, so 5 == "5.0" went false, the ternary took the other branch, matches was never called, and the retry returned false. Two expressions differing only in whether a regex literal happens to contain the phrase disagreed about whether they fault at all. The same shape reproduces with && and with a date field. This does not change the grading ask — the exposure still needs an expression that short-circuits around the throwing call — but the honest description is no longer "no user-visible symptom".

Faults are otherwise unchanged: a native throw carries no cel-js contract, so it is still runtime (#6223).

Tests

New file packages/formula/src/cel-overload-retry-trigger.test.ts, 7 pins, both directions. Arming is observed without an export by counting record reads — hydration walks the live scope with Object.entries before re-evaluating, so an armed retry always re-reads the record.

Reverse-verification — the pins were written before the fix, run against the unmodified file, with red/green predicted per case:

Case Predicted Actual
native throw + phrase does not re-evaluate red red (reads 2, expected 1)
pattern from a ROW does not re-evaluate red red (reads 2, expected 1)
spuriously-armed retry could SUCCEED red red (ok true, expected false)
native throw without the phrase unchanged green green
numeric-string field still hydrates (#1534) green green
date-string field still hydrates (#1530) green green
the fault is EvaluationError / no_such_overload green green

No deviation. All 7 green after the fix.

Scope

Deliberately not touched: UNSOUND_OVERLOAD_RE (cel-engine.ts:632, consumed at :767 off result.error?.message). It is a different mechanism with its own docblock, parsing operands and an operator out of the text rather than testing for a phrase — triage's "whether :632 deserves the same treatment is a separate measurement" still holds, and this PR does not make it.

One finding measured while pinning this and filed separately rather than folded in: hydrateOverloadStrings's docblock claims the retry "can never change a comparison that already evaluated cleanly". That is false independently of this card — on a genuine no_such_overload fault, a sibling sub-comparison that evaluated cleanly can still be re-interpreted (record.n >= 4 && record.s == "5.0" with { n: "7", s: "5.0" } returns false, because hydration makes record.s the number 5). This PR neither causes nor fixes it.

Gates

pnpm lint (ESLint) and pnpm typecheck clean. check:empty-changeset, check:changeset-gate-self-tests, check:adr-anchors, check:error-code-casing, check:nul-bytes, check:doc-authoring, check:published-files, check:objectui-changeset, check:docs-audit-scope, check:required-contexts all pass. Suites: @objectstack/formula 568, @objectstack/objectql 2786, @objectstack/lint 1771, @objectstack/service-automation 885 — all passing.

Related

#6679 · #6677 / #6223 (same file, same defect family) · #1530 / #1534 (why the retry exists) · ADR-0032 §1c


Generated by Claude Code

#6679)

`isNumericOverloadError` decided whether to run the ADR-0032 §1c hydration
retry by testing `/no such overload/i` against `err.message` — the last
message-text read in `cel-engine.ts` that armed behaviour after #6223 /
PR #6677 closed the same hole in `classifyError`. It now reads
`err instanceof EvaluationError && err.code === 'no_such_overload'`, the
class-and-code rule `classifyCelFault` already follows one function below.

The phrase was reachable from a native throw: the `matches()` stdlib binding
is `new RegExp(String(re)).test(...)`, so an uncompilable pattern escapes
cel-js unwrapped as a `SyntaxError` echoing the pattern verbatim, from the
source or from a row.

Measuring that for the fix found a case the filing expected might not exist:
when hydration lets the expression short-circuit around the throwing call,
the spurious retry succeeds and returns a value where the fault was right.

    record.s == "5.0" ? matches(record.name, "no such overload(") : false
      { s: "5.0", name: "x" }  ->  was: ok/false      now: the regex fault
    record.s == "5.0" ? matches(record.name, "(") : false
      { s: "5.0", name: "x" }  ->  the regex fault    (unchanged)

Both directions pinned in `cel-overload-retry-trigger.test.ts`: a native
throw carrying the phrase no longer arms the retry, and a genuine cel-js
`no_such_overload` still does (#1530, #1534).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CkomWBsADMsq174GmqhxhC
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 9, 2026 3:06pm

Request Review

@github-actions

github-actions Bot commented Aug 9, 2026

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.

@github-actions github-actions Bot added the size/m label Aug 9, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 9, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 9, 2026 15:35
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 9, 2026
Merged via the queue into main with commit 29b94ed Aug 9, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-6679-overload-structured-code branch August 9, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants