docs(formula): state the false-negative side of firstUndeclaredReference's contract - #16808
Conversation
`firstUndeclaredReference` documented only which error it cannot make
("Acts ONLY on cel-js's `Unknown variable: X` fault, so it cannot
false-positive"). That sentence is true; a contract that declares only
that reads as making neither error. The checker returns exactly one
error, so a first error of another class leaves every undeclared
reference behind it unjudged and the helper answers `null` -- the same
value that means "every reference is rooted".
The amendment states the masking is positional rather than name-keyed,
that `celEngine.compile()` is not a gate against it (it type-checks in
the permissive env, where the roots this env declares `map` are `dyn`),
which two error classes reach that first slot from authored input, and
why widening the regex is the false positive the narrowing buys off.
`flow-variable-scope` carries the same correction from the consumer
side: its "known, deliberate blind spot" note bounded the under-report
to a variable named after a SCOPE_ROOTS member, and measured, that bound
does not hold -- such a name in an operand position terminates the
discovery loop on iteration 0 and every shadow in the source is lost.
Comment-only; no behaviour changes. The formula block is JSDoc on a
published export, so it is emitted into dist/index.d.ts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
📓 Docs Drift Check
What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
落地前检记录 ——
|
| conclusion | 数量 | 名称 |
|---|---|---|
success |
9 | CI · Lint & Type Check · PR Automation · Governed Surface Guard · Docs Drift Check · Check Links · Duplicate Fix Guard · Single-Claim Path Guard · Part-of Closing-Keyword Guard |
skipped |
1 | Pack Smoke (opt-in) |
failure / in_progress |
0 | — |
Lint & Repo Gates 与 TypeScript Type Check 两个 job 跑在 Lint & Type Check 这个 workflow 内,而它是 success —— 一个 workflow 成功意味着它的非跳过 job 全部成功。⇒ 这是一次真实读数,但它的粒度是 workflow,本席不把它说成 check-run 级的亲核。
① 席内契约档 PASS —— 不适用
Clause-②: no,⛔ 无载体挂起 ⇒ 无档位复核可欠。
⛔ ② 双载体机读 —— 本席在这条通道上无法满足,如实记为未清
check-clause2-carriers.mjs --pair 16808 在本席这里 exit 3(repo-scoped REST 403),而 --pair-json 需要一份包含 PR 正文与卡片评论原文的文档。
⇒ ⛔ 本席不制造一次看起来干净的读数。
当刻已知的事实:dev 在其轮次里跑过 --pair 16808 --pair-json,得到 exit 4,C2 MISPLACED —— 原因是本席当时的 claim 评论以反引号开头,CLAIM_COMMENT_MARKER = /^\s*>?\s*Claim(?:ed)?\s*:/mi 根本不把它当作 claim 评论。本席已在 #16412 (comment) 用裸形态重述并带上裸 Clause-②: no 行;
⭐ 旁证但⛔ 不作 ② 的替代:本席在另一对上量到修复有效 —— PR #16816 / 卡 #16711 的 claim 用的是同一套裸形态,其 dev 跑 --pair 16816 得 exit 0「both carriers agree」。⇒ 形态是对的;⛔ 但那是另一对的读数。
⇒ 处置
本 PR 停在 draft。⛔ 不翻 ready、⛔ 不入队、⛔ 不挂 auto-merge。它是 Part of #16412,纯注释 diff,卡片本身另有一项决策待裁(神谕形状,分析在 5580669325)。
node scripts/pm/check-clause2-carriers.mjs --pair 16808 即可补齐 ②,本席会据此落地。
Generated by Claude Code
更正上一条:② 是做得到的,我做了,PR 已入队⛔ 上一条(
我做的两件事,两件都在安全方向上:
读数⭐ 阴性对照发火:同一份文档、只删掉我那条重述 claim ⇒ EXIT=4, 落地前检三条,逐条
正文闭合关键词亲扫(翻 ready 前的那一读):首行 ⇒ 已转 ready 并挂 auto-merge( Generated by Claude Code |
Part of #16412
Items 1 and 2 of the triage 验收口径 are delivered; item 3's stop condition is reached and reported, not acted on. The code diff is comment-only. The deliverable is the reachability reading, and it says the gap is live on all three consumers, with an author-written shape for each.
Clause-②: no
Basis: the diff is comment-only (59 changed lines across two
srcfiles, 0 of them non-comment — measured by filtering the merge-base diff for lines that are not JSDoc). No exported symbol is added or changed, no key lands on a published payload, and no runtime behaviour moves. The published declaration files do change bytes (packages/formula/dist/index.d.tsand.d.mts, 53.45 KB to 55.99 KB) but only as JSDoc text on an already-exported symbol, which the dispatch's own mechanical floor names as not by itself a widening.needs:contract-reviewtherefore does not go up on either carrier.Type notation: cel-js prints map types with angle brackets. Every such type below is rendered with square brackets (
map[dyn, dyn]) so GitHub's body sanitizer does not eat the fragment. The error strings are otherwise verbatim.1. The reachability reading — LIVE on all three consumers
Measured in a worktree at merge base
b38821d1c,packages/formulaand its closure built first. Method: the raw strict-envcheck()verdict beside the helper's answer besidecelEngine.compile(), then each consumer driven through its own real entry point.The mechanism, which is not the one the card's
has()trigger suggestsfirstUndeclaredReferencebuilds a strict env in whichSCOPE_ROOTSare declaredmap.celEngine.compile()— the gate two of the three consumers sit behind — type-checks in the permissive env (CEL_ENV_OPTIONS,unlistedVariablesAreDyn: true), where those same roots aredyn.⇒ Any operator with no
mapoverload applied to a namespace root faults only in the strict env. The source compiles clean, reaches the helper, and the first checker error is ano such overload— so every undeclared reference behind it goes unjudged and the answer isnull.That is a second live class, disjoint from the card's
has()one, and it is the class that survives the compile gate:status == "qualified"Unknown variable: status"status"has(status) && other == "x"has() invalid argumentnulldata == 'x' && status == 'q'no such overload: map[dyn, dyn] == stringnullrecord == 'x' && status == 'q'no such overload: map[dyn, dyn] == stringnullconfig != null && status == 'q'no such overload(same class)nullpage == 'x' && status == 'q'Unknown variable: status"status"The last row is the control that names the mechanism as the declaration, not the name:
pageis passed to the helper as a known field (dyn), so nothing faults on it andstatusis reported normally.What an author writes to get there
A field — or a flow variable — named after a
SCOPE_ROOTSmember, read bare. The list includesdata,config,result,item,event,input,output,user,step,context,params,settings,payload,args,env,parent,trigger,record,previous,vars. These are ordinary field names. The author writesdata == 'x' && status == 'q'meaning two bare field reads, which on arecord-scoped site is exactly the #1928 defect the oracle exists to catch — and the first of the two silences the verdict on both.On a metadata-editing form the collision is not even accidental:
CANONICAL_ROOT_BY_LAYER.metadatais'data', sodatais the root that layer binds.Per consumer
packages/lint/src/validate-visibility-predicates.ts:780—visibility-bare-identifier. LIVE. Repaired at its own call site for thehas()class by #16118 / PR #16413; that repair is ahas(…)span mask and it does not reach this class at all. Driven throughvalidateVisibilityPredicateson the suite's ownformStackfixture:visibleWhenstatus == 'active'visibility-bare-identifier)has(status) && status == 'active'data == 'x' && status == 'active'record == 'x' && status == 'active'config != null && status == 'active'type == 'grid' && status == 'active'Rows 3–5 are the live gap: the predicate publishes clean, and per the rule's own message the console then falls OPEN — the element renders unconditionally and looks exactly like one with no predicate at all. Row 6 is the CEL-type-name variant of the same class; the suite already pins
type == 'grid'alone as a measured blind spot, but it pins it per name, and the masking it causes is source-wide.packages/lint/src/flow-variable-scope.ts:273—shadowedFieldReads. LIVE. No compile gate stands in front of it:warnShadowedFieldReadsinvalidate-expressions.ts:1177calls it on anycelSourceOf(raw)that is non-empty.bareRootsOfasks the oracle withfoundstarting empty, so anullon iteration 0 ends the loop before it has judged anything.With
statusdeclared as both a flow variable and a field of the bound object:status == 'x'["status"]config == 'x' && status == 'y'[]data == 'x' && status == 'y'[]result > 1 && status == 'y'[]status == 'y' && config == 'x'["status"]— same two names, other orderpackages/formula/src/validate.ts:688— therecord-scope bare-ref HARD ERROR. LIVE, and the worst of the three. ThroughvalidateExpression('predicate', src, { scope: 'record' }):status == 'qualified'ok=false— bare referencestatusdata == 'x' && status == 'qualified'ok=true, zero errorsconfig != null && status == 'qualified'ok=true, zero errorsrecord == 'x' && status == 'qualified'ok=true, zero errorsA hard error that gates the build is downgraded to silence, and the formula ships and silently evaluates to
nullat runtime — #1928's own class.packages/formula/src/validate.ts:713— the flattened did-you-mean WARNING. LIVE. Same shape, one severity down. Note the strict env registersschema.fieldsasdynafterSCOPE_ROOTSasmap, and the duplicate registration throws and is swallowed — so a field actually namedconfigkeepsmap, notdyn:amount,status,config,data)amont == 'x'amount?config == 'y' && amont == 'x'data == 'y' && amont == 'x'Re-grading input for the seat
Both previously-unmeasured consumers can receive such a source, and an author can write it — an undeclared reference passing lint as legal, and on
validate.ts:688a hard error going silent. Per the trigger the seat attached, that is thepriority:p1branch, notp3.2. The docblock's false-negative side — the whole code diff
cel-engine.tsgains a section stating what the "cannot false-positive" sentence never said: the checker returns exactly ONE error, so a first error of another class leaves everything behind it unjudged and the answer isnull— the same value that means "every reference is rooted". It states that the masking is positional rather than name-keyed, thatcelEngine.compile()is not a gate against it, which two error classes reach that first slot from authored input, and why widening the regex is precisely the false positive the narrowing buys off.flow-variable-scope.tsgains the same correction from the consumer side. Its "known, deliberate blind spot" note bounded the under-report to a flow variable named after aSCOPE_ROOTSmember. Measured, that bound does not hold: such a name in an operand position ends the discovery loop on iteration 0 and every shadow in the source is lost, whatever it is named. That is a statement about the rule's own behaviour that the reading above falsifies, so it is corrected here rather than left to be read as a bound.3. If the oracle moves — the per-rule output differences, measured
⛔ Not adopted. Measured as a reverted prototype so the seat has numbers instead of adjectives.
Design B — a different checker entry — is not available at the pinned dependency.
@marcbachmann/cel-js@8.0.0exposesparse().check(),Environment.check(expression)and a top-levelcheck(expression); all three returnTypeCheckResult, whose declared shape is{ valid, type?, error? }— a singleerror, no array, and no options parameter to ask for more. Reporting past the first error through a different entry means walking sub-expressions or moving the dependency.Design A — a re-check loop over a neutralised source — was prototyped and measured. The checker's error carries a source
range(measured:{start, end}plus acodesuch asno_such_overload/invalid_macro_argument), so the loop is mechanically available: mask[start, end)with a width-preserving literal, re-check, bounded.Ablation discipline: mutation applied to the committed tree, proven on disk (injected marker 7 occurrences,
env.parse(source).check2 to 1),@objectstack/formularebuilt, presence indist/proven withablation-dist-preflight.mjs(exit 0) because@objectstack/lint's tests resolve@objectstack/formulathroughdist/— it is a registeredKNOWN_UNALIASED_TEST_IMPORTSpair. Restored withgit checkout HEAD --, source blob hash back to88f74c9c= HEAD's, rebuilt, and absence re-proven (--absent, exit 0, "working tree clean against HEAD").⭐ The single most important number: Design A changes ZERO existing test verdicts. Baseline and prototype both:
@objectstack/formula29 files / 827 passed;@objectstack/lint102 files / 3564 passed, 5 skipped. Item 3's "one green test run is not evidence" is exactly right — the suites are blind to this difference, so every output difference below had to be probed directly.Per-rule output differences under Design A, all previously-silent cases starting to report:
visibility-bare-identifierdata == 'x' && status == 'active'visibility-bare-identifierrecord == 'x' && status == 'active'visibility-bare-identifierconfig != null && status == 'active'visibility-bare-identifiertype == 'grid' && status == 'active'shadowedFieldReadsconfig == 'x' && status == 'y'[]["status"]shadowedFieldReadsdata == 'x' && status == 'y'[]["status"]shadowedFieldReadsresult > 1 && status == 'y'[]["status"]validate.ts:688record scopedata == 'x' && status == 'qualified'ok=trueok=false, bare-ref errorvalidate.ts:688record scopeconfig != null && status == 'qualified'ok=trueok=falsevalidate.ts:688record scoperecord == 'x' && status == 'qualified'ok=trueok=falsevalidate.ts:713flattenedconfig == 'y' && amont == 'x'validate.ts:713flatteneddata == 'y' && amont == 'x'Three consequences the seat should weigh, none of which a green suite would have shown:
visibility-bare-identifiergoes silent for an identifier that also appears inside ahas()in the same predicate #16118's repair. Thehas()class staysnullunder it: the error's range forhas(status)is the ARGUMENT span, so masking it yieldshas(true ), which faults identically, and the loop reaches its fixed point and gives up. Card rows 2 and 3 are unchanged by Design A. The two mechanisms are disjoint and the call-site mask is still load-bearing.type == 'grid' && status == 'active'failscelEngine.compile(), so the visibility rule would now emit a type/syntax finding and a bare-identifier finding for one predicate. That is a per-rule output change on inputs that are already being rejected.validate-visibility-predicates.test.tspinstype == 'grid'as a deliberate blind spot with a recorded rationale. Design A's first masking step neutralises thetype == 'grid'node, so the source-wide half of that pin moves.4. Negative control — "cannot false-positive" still holds
17 sources in which every reference is rooted, or which are the legitimate CEL the narrowing exists to protect (
type(record.x) == string, comprehension macros including one whose macro variable shadows a field, both guard idioms, optional chaining, stdlib calls). Each asserted clean on all four surfaces at once — helper,validate.tsrecord scope,validateVisibilityPredicates,shadowedFieldReads.5. The card's five probe rows, all five, pre and post
This PR changes no behaviour, so the regression is a byte-identity claim and is reported as one: the probe output file for the five rows is
diff-identical before and after, on the built tree, and rows 1 and 5 — the two the helper answers correctly today — are among the identical lines.status == "qualified"Unknown variable: status"status""status"has(status) && status == "qualified"has() invalid argumentnullnullhas(status) && other == "x"has() invalid argumentnullnullother == "x" && has(status)Unknown variable: other"other""other"has(record.status) && status == "qualified"Unknown variable: status"status""status"All five reproduce the card's table exactly, at merge base
b38821d1c.Verification
Everything below at
ba03f26bd9, the final commit.pnpm --filter @objectstack/formula --filter @objectstack/lint run test— formula 29 files / 827 passed; lint 102 files / 3564 passed, 5 skipped. WrapperVERDICT command-exit 0.pnpm --filter @objectstack/formula --filter @objectstack/lint run typecheck—VERDICT command-exit 0(each package'stsc --noEmitplus itscheck:test-typecheck).node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 50 commands; all 50 run, 49 exit 0. The one exception is reported below, not rounded off.✓ dispatch-gates --ran: 50 derived famil(ies) accounted for — 50 run, 0 NOT-MEASURED.pnpm check:dual-build-cjs-loadsexited 3 = PREREQUISITE NOT MET, which is NOT MEASURED and not a pass. Its own words, verbatim: "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/." followed by 8 named packages and "… 77 more" (85 in total), then "Runpnpm buildfirst. ⛔ This is NOT a pass: nothing was measured." That is a whole-farm build; it is CI's run, and it is declared here rather than claimed.pnpm check:docs-transcript-driftalso exited 3 on first pass for the same reason, was repaired locally by building@objectstack/lint, and re-ran exit 0 — its verdict line is in the record.origin/main, and 3 files the derivation reads changed across that range (publish-smoke.yml,check-route-envelope.mjs,engine-double-contract.pinned.json). Re-derived aftergit fetch origin main: the 50-command list came back byte-identical, and none of this PR's three paths is in those three families' populations.npx eslint . --no-inline-config --format jsonover the whole repository — not a narrowing: 6351 files linted, 0 errors, 0 warnings, exit 0. (The config declares noparserOptions.projectand no typed rules, so it could only ever have been a narrowing by file count anyway.)pnpm check:nul-bytesexit 0 over 8287 files, plus a directgrep -naPcontrol-character scan of both edited files — no hits.Changeset
.changeset/first-undeclared-reference-false-negative-contract.md,patchfor@objectstack/formulaand@objectstack/lint.Governing text:
.github/workflows/pr-automation.yml, theWHICH LEVELblock. Route 2 — theskip-changesetlabel — is the text I rejected, and it is rejected on a measurement, not a judgement call: the label is for a diff that "releases nothing", and this diff releases something. The amended block is JSDoc on a published export, so tsup emits it intopackages/formula/dist/index.d.tsanddist/index.d.mts— measured, the declaration file grew 53.45 KB to 55.99 KB andgrepfinds the new prose in both.distis@objectstack/formula's entirefilesfield, so those bytes are what npm ships and what a consumer reads on hover. Route 3, the empty-frontmatter changeset, is closed by the gate below that block and was not considered.Level:
patch. TheWHICH LEVELtext raises to at leastminorfor "a purely additive widening of a published package's public surface (a new exported symbol on anindex, a new accepted key or value)". This adds none of those — no symbol, no key, no value, no behaviour — so the act requirespatchand thedocs(commit type does not lower it below that.AGENTS.md's floor is satisfied in the direction it points: an entry rather than none.@objectstack/lint's entry is honest about being the weaker half — its corrected block sits on the internalbareRootsOf, andgrepoverpackages/lint/dist/after a rebuild finds 0 occurrences of the new prose. It is entered because the package is touched and published, per the dispatch.验收备注
The triage 验收口径 in full (issue #16412, comment 5579098338), each item with its disposition.
priority:p1re-grading branch.cel-engine.ts:180's docblock must gain the false-negative side, whatever is decided about behaviour — done; it is the whole diff, plus the same correction on the consumer-side account inflow-variable-scope.ts.visibility-bare-identifiergoes silent for an identifier that also appears inside ahas()in the same predicate #16118 的has(…)遮蔽当作这里的修法 — not used, and now measured rather than assumed: the class this reading found (no such overloadon amap-declared root) is disjoint from thehas()class, survives the compile gate that thehas()class does not, and is untouched by ahas(…)span mask. Design A, conversely, does not reach thehas()class either. Neither mechanism subsumes the other.Stop condition — the oracle's design is not picked here
Triage: 「神谕的形状是一次设计决定,⚠️ 若承接席选定其中一条并认为需要背书,回本卡说明」. The reading says the gap is live and closing it needs the re-check loop or a different checker entry, so this stops at items 1 + 2 and hands back the measurements. The open question is on the card and in the report: is the oracle's contract widened, and along which of the two shapes — given that Design B needs the dependency to move, Design A leaves the
has()class open, and Design A moves an existing pinned blind spot and starts double-reporting on non-compiling sources.Noted, not filed
SCOPE_ROOTSare declaredmapin the strict env and leftdynin the permissive one. That asymmetry is the whole mechanism of the second live class, and it is not documented anywhere as a difference between the two environments. Whether the strict env should declare themdynis a third candidate shape nobody has named — narrower than either design in triage, and it would close this class without touching the checker. ⛔ Noted only; it is a design input for the same decision, not a separate defect.validate-visibility-predicates.test.ts's "a field named after a CEL TYPE is a measured blind spot" pin is accurate about the single-term source it pins and silent about the source-wide masking that same source causes. Extending it would mean pinning current defective behaviour, so it is left alone and named here instead.Generated by Claude Code