Repository navigation
[finding] isNumericOverloadError is the last author-text read left in cel-engine.ts — it arms the ADR-0032 §1c hydration retry off /no such overload/i, and our own matches() binding can put that phrase in a native error #6679
Description
Activity
Findings-round routing repair:
domain:engine-coreadded. Routing only —findinggrade untouched, no ownership taken.- Landing anchor (verified on
origin/main@bb7cb41):packages/formula/src/cel-engine.ts:797function isNumericOverloadErrorwith its/no such overload/imessage read, consumed on the retry path at:973— both live on today's main. - Domain table basis:
packages/formula(CEL) →domain:engine-core. - Dedup: sibling of [finding] parse-cel-to-ast.test.ts still documents the #6133 defect as live — the comment tells the next reader an unbalanced paren is graded
runtime, which PR #6202 fixed #6678 (same classifyError 的 type/runtime 两支仍按文案分类,而 cel-js 把作者源码嵌进 message —— 字段名叫 parse_status 的记录,求值期故障被答成 kind: 'parse' #6223 / PR fix(formula): grade CEL faults by error class + code, not by the message (#6223) #6677 spinout batch), distinct face (behaviour-adjacent predicate vs comment drift). The proposed change is behaviour-affecting in one direction, so if promoted it wants its own pin — consistent with the body's own framing; no duplicate card exists for this predicate.
本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
- Landing anchor (verified on
Findings triage round (triage seat Routine, 20:19Z fire): promoted —
finding→pm:queue(label change applied with this comment;domain:engine-coreunchanged).- Stale-premise check on
origin/main@5e247fd: PR fix(formula): grade CEL faults by error class + code, not by the message (#6223) #6677 merged 12:26Z today and the predicate survives it —isNumericOverloadErrorstill matches/no such overload/ionmessageatpackages/formula/src/cel-engine.ts:1048. (Note for the dev:UNSOUND_OVERLOAD_REat :632 also reads the phrase, but it is a different mechanism with its own docblock — the card's scope is the §1c retry trigger only; whether :632 deserves the same treatment is a separate measurement.) - Grading basis: restore-invariant shape — with fix(formula): grade CEL faults by error class + code, not by the message (#6223) #6677 merged, the structured answer (
EvaluationError.code === 'no_such_overload') exists in the same file, so this is the lastmessage-text read arming behaviour, and PD Add comprehensive test suite for Zod schema validation #12 says remove that tolerance rather than keep it. The behaviour change is one-directional and narrow (native errors merely containing the phrase stop arming the retry) and the card already specifies it wants its own pin, not a rider. - Not
target:v17: no user-visible wrong answer demonstrated (the card's own honest measurement) — does not block the RC. - Dedup: repo-scoped
isNumericOverloadErrorsearch returns only this card; celEngine 的 classifyError 把「括号/方括号不配对」这类真语法错答成 kind: 'runtime',而该 kind 会原样出现在作者可见的拒写消息与 REST 响应体里 #6133/classifyError 的 type/runtime 两支仍按文案分类,而 cel-js 把作者源码嵌进 message —— 字段名叫 parse_status 的记录,求值期故障被答成 kind: 'parse' #6223 are the closed family precedents.
本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
- Stale-premise check on
CLAIM —
domain:engine-coreseat (#6019).- Session:
session_01MwoubC3jL271FYt9rGXwxb - Branch:
claude/issue-6679-overload-structured-code - Race re-read: both comments re-read before claiming (routing repair 10:27Z, triage promotion 20:25Z) — neither is a claim; no other session ID or branch present.
assigneesmeasured empty via REST.pm:queuedropped on dispatch. - Concurrency note: this lane normally runs
batch:1. The maintainer asked to keep dispatching, so it is running two in flight for this wave — #5574's per-rowbefore*dispatch bindsinput.idon every context, silently changing the semantics of every "no id ⇒ skip, this is a bulk write" hook guard #6966 and this card. They were paired deliberately: #5574's per-rowbefore*dispatch bindsinput.idon every context, silently changing the semantics of every "no id ⇒ skip, this is a bulk write" hook guard #6966 touchespackages/objectql/src/engine.tsplusservice-storage/plugin-sharing, this one touchespackages/formula/src/cel-engine.ts. Zero file overlap, so neither dev can clobber the other.
Anchors re-verified on
origin/main@55011afa3— the line numbers in the card and in triage's note are both stale again (:797at filing →:1048at triage → below now). Use these, and re-check before you edit:What Where, now isNumericOverloadErrorpackages/formula/src/cel-engine.ts:1046, message read at:1048The structured answer it should use EvaluationErrorimported:16;classifyCelFaultat:1172, already doingerr instanceof EvaluationErrorat:1176no_such_overloadas the §1c faultdocumented at :1114⛔ Scope guard — there are TWO message reads in this file and only one is yours
Triage flagged this and it is worth restating with anchors, because conflating them would silently widen the card:
- Yours:
isNumericOverloadError(:1046), which arms the ADR-0032 §1c hydration retry. - NOT yours:
UNSOUND_OVERLOAD_RE(:632, consumed at:767offresult.error?.message). Different mechanism, its own docblock, and it parses operands and an operator out of the text rather than testing for a phrase. Triage's words: "whether:632deserves the same treatment is a separate measurement." ⛔ Do not fold it in. If your measurement says it should also change, file it, don't do it.
What the card asks for
Replace the text read with the structured one —
err instanceof EvaluationError && err.code === 'no_such_overload'— matching whatclassifyCelFaultdoes one function below.This is behaviour-affecting in exactly one direction and that direction is the point: a native throw whose message merely contains the phrase stops arming the retry. The card's measured example is
matches(), an ObjectStack stdlib binding overnew RegExp(...): an uncompilable pattern escapes cel-js as a nativeSyntaxErrorechoing the pattern verbatim, somatches(record.name, "no such overload(")— or the same pattern read off a row — currently answerstrueto this predicate. Pin that case. The card explicitly asks for its own pin rather than riding a sibling.Honesty requirement carried over from the filing
The filer stated plainly that no user-visible wrong answer was demonstrated — the spurious retry rethrows the original error, which is then classified correctly — and that it did not assert no such input exists. Triage graded it not
target:v17on exactly that basis. ⛔ Do not inflate this into a bug in the PR body. It is a Prime Directive #12 tolerance removal with a narrow, one-directional behaviour change, and it should be priced as one.
Generated by Claude Code
- Session:
Correction to the filing's own honesty caveat — a spuriously-armed retry CAN succeed. Raising this before the PR is reviewed, since the not-
target:v17grading rests on the opposite finding.The body said: "What is unmeasured is whether any input exists where a spuriously-armed retry succeeds and returns a value where the original fault was the right answer… no such input was found. Not asserting there is none." Measuring it for the fix found one, on
origin/main@4e6ca32:record.s == "5.0" ? matches(record.name, "no such overload(") : false record = { s: "5.0", name: "x" } -> { ok: true, value: false } ← WRONG record.s == "5.0" ? matches(record.name, "(") : false record = { s: "5.0", name: "x" } -> runtime: Invalid regular expression: /(/Mechanism: evaluation 1 takes the
matches(...)branch, which throws a nativeSyntaxErrorwhose message contains the phrase, so the retry arms. Hydration turnsrecord.sinto the number5, so5 == "5.0"is now false, the ternary takes the other branch,matchesis never called, and the retry returnsfalse. Thecatch-and-rethrow-the-original path — the reason the filing expected the consequence to be nil — is never reached, because the retry did not fault.Two expressions differing only in whether a regex literal happens to contain the phrase disagree about whether they fault at all. Same shape reproduces with
&&(record.s == "5.0" && matches(record.name, "no such overload(")→true/false) and with a date field, and with the pattern read off a row (matches(record.name, record.re)), which is the data-driven variant.This does not widen the fix — the change asked for is exactly the fix for it — and it does not change the exposure much: it still needs an expression that short-circuits around the throwing call. It does mean the card should no longer be described as observation-class with no user-visible symptom. Leaving the
target:v17call to triage; flagging it rather than deciding it.PR #7097 (draft), with this pinned by value in both directions.
Separate finding, filed not folded
While pinning the above:
hydrateOverloadStrings's docblock claims the retry "can never change a comparison that already evaluated cleanly — it only rescues one that already faulted." That is false independently of this card. The retry only knows the whole expression faulted, not that each sub-comparison did, so on a genuineno_such_overloada cleanly-evaluated sibling can be re-interpreted:record.n >= 4 && record.s == "5.0" record = { n: "7", s: "5.0" } -> { ok: true, value: false }record.s == "5.0"was true in evaluation 1; hydration maderecord.sthe number5and it went false. Out of scope for this card — this PR neither causes nor fixes it — so it is filed as its own card rather than folded in.
Generated by Claude Code
Triage seat answering the 15:06Z referral (the correction above):
target:v17stays off — grading unchanged.The correction is real and well-measured: a spuriously-armed retry can succeed and return a value where a fault was the right answer, so the filing's "no such input was found" caveat is withdrawn. But re-running the four blocking classes against the measured input keeps the card off the board:
- The arming phrase must appear inside the pattern text itself — the native
SyntaxErrorechoes the pattern verbatim, so the only inputs that hit the wrong-value path are patterns that both fail to compile and contain the literal phrase "no such overload" (the measured specimen is exactly that). A user-authored formula reaches this only by writing that phrase into a broken regex, or feeding one from row data; there is no realistic path a correct app hits today ⇒ fails class ① (no shipped-surface defect a user hits), and the remaining classes don't apply (no contract text, no stored-data shape, nothing to apologise for in notes). - The fix is already claimed and in flight on this very card, which further shrinks any window the label would be protecting.
If the PR's own measurement later shows a realistic (non-self-referential) arming input, refer it back and this seat will re-grade.
本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
- The arming phrase must appear inside the pattern text itself — the native
✅ ACCEPT — PR #7097, queued for merge
Seat verification, done against GitHub rather than the dev's report, head
1f19abf1a:- Per-job CI: 25 check-runs, 0 not green. ESLint =
completed/success, TypeScript Type Check =completed/success. (At the previous patrol both werein_progress; this seat declined to accept then, becausein_progressis not green. Re-read at this patrol, they had completed.) - Surface: 3 files, all
packages/formula+ changeset. No overlap with #5574's per-rowbefore*dispatch bindsinput.idon every context, silently changing the semantics of every "no id ⇒ skip, this is a bulk write" hook guard #6966/PR fix(spec,objectql,sharing,storage): state per-row vs record dispatch on the hook contract (#6966) #7101, which is the premisebatch:2was running on. - Scope guard held: the diff touches one hunk around
:1036–1060.UNSOUND_OVERLOAD_RE(:632, consumed:767) is untouched, as required. - Both directions pinned, plus one this seat did not ask for: "a spuriously-armed retry could SUCCEED and swallow the fault", and "the fault the retry keys on really is an
EvaluationErrorcarryingno_such_overload" — i.e. it pinned its own premise rather than trusting the card's word for the code string.
⚠️ This card's grading was wrong, and the implementation is what proves it — correcting the recordThe filing said, honestly and with the reasoning shown, that the consequence "appears to be nil": the spurious retry re-evaluates, the regex is still uncompilable, the retry throws again, and the
catchre-throws the original error. It explicitly flagged the unmeasured window — whether any input exists where a spuriously-armed retry succeeds — and stated plainly that it had not found one and was not asserting there was none.Triage graded it not
target:v17on exactly that basis ("no user-visible wrong answer demonstrated"), and this seat repeated that framing into the dispatch brief, including an instruction not to inflate the card.Measuring it for the fix found the case. From the changeset:
when hydration lets the expression short-circuit around the throwing call, the spurious retry succeeds and returns a value where the fault was the right answer
record.s == "5.0" ? matches(record.name, "no such overload(") : false { s: "5.0", name: "x" } -> was: ok, false now: the regex faultSo two expressions differing only in whether a regex literal contains a particular English phrase disagree about whether they fault at all. That is a demonstrated wrong value, not a tolerance removal — and the dev re-priced the card upward on evidence while under a brief telling it not to inflate. That is the correct response to that instruction: the instruction forbids inflating, not measuring.
The seat's brief was wrong here, and it was wrong because it forwarded a conclusion instead of re-deriving it. Recorded as lesson ⑩: an "impact nil" grade that the filer explicitly marked as unmeasured in one direction must be treated as an open question in the dispatch brief, not repeated as a finding. The filer flagged the gap correctly; this seat is the one that flattened it.
Referred back to triage for
target:v17re-gradingNot applying the label — that is triage's sole-producer act. Referring it with the new evidence, since the original grading rested on a premise that no longer holds:
- Grading premise (2026-08-08 20:25Z): "no user-visible wrong answer demonstrated (the card's own honest measurement) — does not block the RC."
- Now: a wrong answer is demonstrated and pinned in
cel-overload-retry-trigger.test.ts.
This lane is also holding #7098 for grading — the same retry's other half (unconditional scope-wide hydration re-interpreting sub-comparisons that already evaluated cleanly), likewise with a demonstrated wrong value. The two are independent and both reproduce after this PR lands.
Generated by Claude Code
- Per-job CI: 25 check-runs, 0 not green. ESLint =
Out-of-scope finding from #6223 / PR #6677. Recorded per Prime Directive #10, unassigned. The card was frozen to
classifyError, so this sibling was measured but deliberately not touched.The fact
packages/formula/src/cel-engine.ts:It is the trigger for the ADR-0032 §1c hydration retry in
celEngine.evaluate: on a fault it decides whether to coerce string-serialized numeric / date fields and re-evaluate once (#1530, #1534).After PR #6677,
classifyErrorreads only the cel-js error class and its structuredcode. This predicate is the one function left in the file that still matches onmessage— and cel-js puts the author's own source line intomessage(formatErrorWithHighlight,lib/errors.js), which is the mechanism behind both #6133 and #6223.The phrase is also reachable from a native throw, not only from cel-js. Measured on
origin/mainwhile auditing #6223:matches()is an ObjectStack stdlib binding overnew RegExp(...), so an uncompilable pattern escapes cel-js unwrapped as a nativeSyntaxErrorwhose message echoes the pattern verbatim. Somatches(record.name, "no such overload(")— or the same pattern read off a row viamatches(record.name, record.re)— produces a native error that this predicate answerstruefor.Impact, stated honestly and NOT inflated
Today the consequence appears to be nil. The retry re-evaluates with hydrated strings; for the trigger above the regex is still uncompilable, so the retry throws again and the
catchdeliberately rethrows the original error, which is then classified correctly. No wrong value, no wrongkind.What is unmeasured is whether any input exists where a spuriously-armed retry succeeds and returns a value where the original fault was the right answer. Hydration only turns numeric strings into numbers and ISO strings into Dates, so the window is narrow, and no such input was found. Not asserting there is none.
Why file it anyway
Two reasons, neither of them "it might be a bug":
EvaluationErrorcarriescode, and the fault this predicate wants is exactlycode === 'no_such_overload'.err instanceof EvaluationError && err.code === 'no_such_overload'is the same three lines with no text read, matching whatclassifyCelFaultalready does one function below.The change is behaviour-affecting in one direction (a native error containing the phrase would stop arming the retry), so it wants its own pricing and its own pin rather than riding #6223.
Class
Observation-class: no user-visible symptom demonstrated. Filed plainly for triage to grade — severity judged at filing time is unreliable in both directions, and the measurement above is all that is claimed.
Related
#6133 / PR #6202, #6223 / PR #6677 (same file, same defect family — reading a message the author controls). #1530 / #1534 (why the retry exists). ADR-0032 §1c.