Skip to content

fix(formula): let an authoring surface declare the binding roots it mounts (ExprSchemaHint.roots) - #18696

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-18554-formula-page-block-scope
Sep 17, 2026
Merged

huangyiirene merged 1 commit into
mainfrom
claude/issue-18554-formula-page-block-scope

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #18554

Clause-②: yes (widening)

ExprSchemaHint gains roots — an authoring surface naming the binding roots it mounts beyond the platform baseline (SCOPE_ROOTS), so validateExpression can accept them without standing down on everything else.

validateExpression('predicate', "page.selectedProjectId != ''", {
  scope: 'record',
  roots: ['page'],      // what this surface mounts beyond the baseline
});                     // -> ok; `status == 'done'` at the same site is still an error

The premises, re-measured on this base

Measured against packages/formula/dist built from origin/main at 6dfa3ea772, before any edit — script and raw output in the report comment on #18554.

probe reading
scope 'flattened' + status == 'done' clean (the shorthand the narrowing exists to catch)
scope 'flattened' + page.selectedProjectId != '' clean
scope 'record' + status == 'done' ERROR, names record.status (the narrowing that is right)
scope 'record' + page.selectedProjectId != '' ERROR: bare reference page … Write record.page.
scope 'record' + current_user.id == 'x' (control) clean — so it is page specifically
SCOPE_ROOTS.includes('page') / ('current_user') false / true
a roots hint passed to validateExpression changed nothing

Every corner and the control reproduce. The card's sharpest claim reproduces too: the refused spelling is the one packages/spec/src/ui/page.zod.ts:390 publishes itself — its visibleWhen describe names the contract-bound roots as record, current_user and page state in the page.VAR form, and ends e.g. "page.selectedProjectId != ''". The prescription record.page names nothing under record on any layer.

One reading is narrower than the thread records it. The filer's amendment adds that under flattened with a field catalog page.* produces a non-blocking did-you-mean warning. Measured here, that warning is conditional on the catalog holding a near-miss: with fields: ['pages','status'] it warns; with fields: ['status','amount'] or ['page_size'] there is no warning at all — page.* is silently clean. The two faces therefore differ by more than strict/lenient in only some catalogs; in the page-draft case the card describes (no catalog at all) flattened is simply silent.

The shape, and why not the other two

⛔ The repair is not "make the validator permissive at that surface". Trading a false refusal for a silent acceptance is the worse of the two directions here, so the surface declares which roots it binds and every other name keeps the verdict it had.

  • ⛔ Not (i), widening SCOPE_ROOTS. That is one accept set for every surface, and page is genuinely unbound at the hook and validation-rule surfaces — this card's false positive traded for that card's false negative.
  • This is (ii), scoped to the direction that was missing. The engine already had the closed-set mechanism: collectCelRootIdentifiers reads the AST and lets a surface refuse a baseline root it never mounts, and SCOPE_ROOTS's own docblock says that is how a closed surface expresses itself. What had no expression was the opposite direction — a surface that binds more than the baseline. roots is that, and only that. The two directions stay two mechanisms.
  • (iv), the filer's amendment, stays available and is not foreclosed. firstUndeclaredReference(source, knownNames) is published and @objectstack/lint's view/page gate already supplies exactly such a list (VIEW_PAGE_EXTRA_ROOTS = ['current_user', 'page']). What that route costs a consumer is everything else validateExpression does — the compile check, the braces hint, field existence, the role catalog, the type-soundness pass. This key is the same vocabulary reaching the shared validator, so a surface can declare its roots without dropping out of the one validator ADR-0032 §Decision 1 exists to keep single.

What it does and does not move

  • It only ever adds. A declared root is registered alongside the baseline, never instead of it: passing roots can turn a refusal into an acceptance and never the reverse. Re-running the measurement above against the built change moves exactly one line — the roots probe, ERROR to clean. All four corners and the control are byte-identical.
  • Declaring a root is not becoming permissive. At a surface declaring page: the bare-field shorthand is still a hard error, an undeclared root (wizard.step == 2) is still a hard error, and a source mixing both still refuses on the field.
  • A mistyped root is sent to the root. With roots declared, a namespace reference within the shared edit-distance threshold of one of them (pge.selectedProjectId) is named an unbound root and pointed at page, instead of being handed record.pge. Three guards keep that from ever prescribing the wrong fix: no declared roots, a name in fields, or no near-miss all fall back to the existing message. A bare value reference (pge == 'x') keeps record.pge — nothing there says it is a mistyped root rather than a mistyped field.
  • introspectScope advertises what the validator accepts, from the same declaration, so an accepted root is never one an author has no way to discover.
  • Every existing call site is unmoved. No hint, roots absent and roots: [] each reproduce the previous verdict and the previous prescription. No caller in this repo passes roots yet; this PR adds the expression, not an adoption.

Serial constraints on this seam

Held: the diff does not touch packages/formula/src/types.ts (EvalContext, so #18318's api member is untouched) and adds no can (#18545). The extension point is plain declarative data — a list of names the caller already knows, not a resolver callback — which is the same shape #18545's ruling fixes for permission data, so nothing here narrows that change's options.

⚠️ One observation for the seat, not acted on: origin carries a branch claude/issue-18545-formula-can-function with zero commits ahead of main — the empty-branch landing marker a dispatched dev pushes before its first edit. No file overlap with this PR either way.

Verification

Final head 4f385ea7e7; every reading below is from that head.

  • pnpm --filter @objectstack/formula test — 30 files / 873 tests passed (17 of them new).
  • pnpm --filter @objectstack/formula typecheck — green (tsc --noEmit + check:test-typecheck, test layer compiles, ledger held).
  • Gates, one count and one reconciliation on the final head. dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives 57 families from the three real changed paths; all 57 ran with their exit codes captured to disk before anything was read. --ran verdict: 57 derived famil(ies) accounted for — 57 run, 0 NOT-MEASURED, 0 UNRUN. Three (check:dual-build-cjs-loads, check:lean-entry-closure, check:type-check-debt) first exited 3 PREREQUISITE NOT MET and were re-run to green after turbo run build --filter='./packages/*' --filter='./packages/*/*' (72/72 successful). ⭐ check:cross-package-test-inputs passed — no spurious red, consistent with fix(gates): judge a cross-package walk root against the index, not the working tree #18641.
  • Reverse verification, one-shot. From the committed state, the record-scope arm was reverted to firstUndeclaredReference(source) — mutation proved on disk by anchor counts (new=1 old=0 to new=0 old=1) and a moved blob hash (e751cb54 to fa72f895), not by an editor's exit code. Predicted direction: turns red, and it does — 2 of 17 pins fail (resolves once the surface declares 'page' as one of its roots; judges the declared root and the bare field in one source). The mistyped-root pins stay green, because that arm is the refusal message and the mutation was the accept side — the partition is the expected one. Restored with git checkout HEAD -- naming the file by absolute path under an EXIT/INT/TERM trap; restoration proved by blob hash back to e751cb54 and an empty git diff HEAD, then the file re-run green (17/17). The test imports ./validate as package-relative source, so no dist sits in this ablation's resolution path and no rebuild leg applies.
  • Lint, a declared narrowing rather than the repo-level scan. pnpm lint (eslint . --no-inline-config) is CI's run. Ran instead: eslint --no-inline-config --format json over the two changed source files — 0 errors, 0 warnings, 2 files counted from the JSON. Population, read from ESLint's own API rather than guessed: 6815 tracked files carry a linted extension and ESLint ignores none of them globally. The narrowing is safe because this repo's single eslint.config.mjs never enables type-aware linting for any file — parserOptions.project and projectService appear zero times in it, and the config says so in its own prose with a positive control — so no rule resolves types across files and this diff cannot move the verdict of a file it does not contain. The changeset file carries no linted extension.

Acceptance notes

Out of scope for this PR, noted rather than filed:

  • introspectScope's baseline roots are still surface-blind. It advertises input, os and vars to every caller regardless of what the surface binds — the accept-side twin this card's body names, already tracked as objectui#9645. This PR makes declared roots joinable to that list but does not narrow it, because narrowing what a surface advertises is that card's verdict to give. Carrier for the finding: objectui#9645.
  • The flattened did-you-mean is catalog-conditional, as measured above. Not a defect — the threshold is nearestName's shared one and behaving as designed — but the thread records the warning unconditionally, so the reading is corrected here where a reviewer will see it. No carrier needed; it is a correction to a reading, not to code.

Contract review

This PR moves what the validator accepts, so Clause-②: yes (widening) is declared at column 0 above and a patch-above changeset (minor) ships with it — ⛔ skip-changeset would be wrong here. needs:contract-review is on the card; applying it to this PR is the seat's stroke, not this one's.


Generated by Claude Code

…mounts

`ExprSchemaHint.roots` — the roots a surface binds beyond the platform
baseline (`SCOPE_ROOTS`), declared as plain data by the caller.

A page component's `visibleWhen` binds three roots at runtime and the hint
could express neither of the two shapes it needs: `scope: 'record'` refused
`page.selectedProjectId != ''` — the worked example `page.zod.ts`'s own
`visibleWhen` describe ends with — and prescribed `record.page`, which names
nothing on any layer; `scope: 'flattened'` accepted that and accepted a bare
`status == 'done'` with it, which is the shorthand the narrowing exists to
catch.

Declaring a root is not becoming permissive: the bare-field shorthand, an
undeclared root and a typo of a declared root all stay hard errors. The key
only ever adds, so a call site that does not pass it keeps its verdict and its
prescription byte for byte.

Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/formula, touching 5 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via validateExpression (symbol, a top-level function))
  • content/docs/data-modeling/formulas.mdx (via validateExpression (symbol, a top-level function))
What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: current_user (literal, 31 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1bc22b3dcddc8a30b4826da8625e7787d5518a8f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from a099c46c0f0def2ee50fd8685d0396f3da3ed0d0 — the merge of head 4f385ea7e767966c3e8385c260a3c701954e2049 into base 1bc22b3dcddc8a30b4826da8625e7787d5518a8f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a099c46c0f0def2ee50fd8685d0396f3da3ed0d0 && git checkout a099c46c0f0def2ee50fd8685d0396f3da3ed0d0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1bc22b3dcddc8a30b4826da8625e7787d5518a8f 4f385ea7e767966c3e8385c260a3c701954e2049 && git checkout -B drift-repro 1bc22b3dcddc8a30b4826da8625e7787d5518a8f && git merge --no-ff 4f385ea7e767966c3e8385c260a3c701954e2049

node scripts/docs-audit/affected-docs.mjs --json 1bc22b3dcddc8a30b4826da8625e7787d5518a8f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 1bc22b3dcddc8a30b4826da8625e7787d5518a8f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

needs:contract-review cleared from both carriers — provenance for the stroke, written 2026-09-17T15:35Z.

contract-review record card #18554, comment 5716968600 — ## Contract review, Served-tier: present, VERDICT: PASS
head judged by that record 4f385ea7e767966c3e8385c260a3c701954e2049 — the head this PR still carries, ⛔ not a predecessor
carriers card #18554 cleared · PR #18696 cleared, one stroke, label-write.mjs with read-back on each

Pre-landing checks, all three re-taken at this stroke — ⛔ none carried over from earlier in the round.

  • ① the in-seat clause-② review is in case, in the template's own shape, at the tier the fuse reads.
  • ② node scripts/pm/check-clause2-carriers.mjs --pair 18696 → exit 0, and it now also reports that a review of record names this head.
  • ③ CI on 4f385ea7e7: 34 distinct check names, 0 pending, 0 non-green. All seven required contexts green under their registry names. The five skips — Auto Label · Check PR Size · Packed-tarball smoke (opt-in) · Build Docs · Console Pin Gate — are each on check-expected-skips' EXPECTED_SKIPS roster with a declared reason this diff satisfies (two are the labeled-event re-fire, one wants a needs:pack-smoke label this PR does not carry, and two are ci.yml's docs / console filters, which three paths under packages/formula and .changeset/ do not move).

⚠️ check-expected-skips.mjs itself cannot run in this checkout — ERR_MODULE_NOT_FOUND: yaml, no node_modules — so the roster was matched by hand against origin/main's copy, name by name. ⛔ Recorded as a hand match rather than passed off as a tool reading: the script fails loudly rather than green, which is the only reason a hand match is admissible here at all.

Governed-surface predicate, derived by the script from this PR's own file list rather than from a list this seat typed: check-governed-merges.mjs --pr 18696 → 0 of 3 paths hit the register ⇒ ordinary queue landing.

⇒ Going ready and into the merge queue (SQUASH). This seat follows it to MERGED.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 17, 2026 15:35
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit e75cc3c Sep 17, 2026
43 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-18554-formula-page-block-scope branch September 17, 2026 15:58
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