feat(objectql,cli): backfillSummaryNulls accepts recomputeUndefinedOnEmpty — a just-declared min/max/avg roll-up can be filled on request (#15064) - #15708
Conversation
…Empty — a just-declared min/max/avg roll-up can be filled on request Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ckfill-summary-nulls-scope
…and the CLI wiring double drains stdout Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ut double Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ckfill-summary-nulls-scope
…a clean ablation control Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…OUND now has a producer declaring 404 (#15064) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 2 package(s): 13 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 34 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b3eeb537aac751d634c8d2ffcc26ca3eac76c00b && git checkout b3eeb537aac751d634c8d2ffcc26ca3eac76c00b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fa85759963fcf25a55cd9a159c8e9c44bb9dde13 87385b56114f86c5cb5d1f5cd4f259b89070593f && git checkout -B drift-repro fa85759963fcf25a55cd9a159c8e9c44bb9dde13 && git merge --no-ff 87385b56114f86c5cb5d1f5cd4f259b89070593f
node scripts/docs-audit/affected-docs.mjs --json fa85759963fcf25a55cd9a159c8e9c44bb9dde13
|
Contract review at
|
| reading | value |
|---|---|
error-catalog.mdx — INVALID_FIELD Cause |
「A field name in the request does not exist on the target object」, at 400 |
engine.ts:1098 |
err.code = 'INVALID_FIELD'; — a live producer |
engine.ts:1092, in the code's own words |
「INVALID_FIELD, not a new code, and 400 rather than 500」 |
the sibling instance (:993-997, INVALID_SORT) |
「a host … answers the same envelope on both doors」 — a pattern, not a one-off |
FIELD_NOT_FOUND producers in packages/**/*.ts (non-test) |
1 — the enum declaration in spec/src/api/errors.zod.ts:90. No producer exists |
⇒ This PR would give a never-emitted 404 code its first producer for a condition the catalog already assigns to a 400 code — the two-codes-one-condition drift ADR-0112 exists to prevent. And it bites in practice: two of the PR's own pinned refusals name a field that does exist (a real non-summary field; a real roll-up on an object the run was told not to walk), so the 404's Fix sends an operator to go confirm a field that is right there.
error-catalog.mdx, and the row went out as 「is the catalog sentence still true of every producer?」 The reviewer answered that the sentence is not the thing to fix — widening a 404's prose would encode the wrong code rather than correct it. The row was disposed of, not deferred, and it inverted the fix.
Patch round dispatched for items 1–4: the code change to INVALID_FIELD / 400 with its pins retargeted; restoring scripts/error-status-unpinned-baseline.json to origin/main's blob (a no-op against main — ⛔ not the maintainer-only baseline-expanding path — because the code change hands FIELD_NOT_FOUND its producer back); adding the new axis to the catalog's INVALID_FIELD entry and leaving FIELD_NOT_FOUND alone; and one stale TSDoc line on SummaryBackfillReport.nullRows. The reviewer records no escalations — every item is answerable from the tree's own recorded conventions.
FIELD_NOT_FOUND is meant to get its first producer here, that is a catalog-level decision requiring the INVALID_FIELD entry to be narrowed at the same time plus a rationale for 404 on an option value. Flagged so it can be overruled deliberately rather than by default.
Adoption record: verdict produced by a context-isolated fable review subagent fed only the card, the ruling, the dev report and this PR — ⛔ never this seat's conclusions — transcript-verified at 140 "model":"claude-fable-5-1" stamps and no other model over 879,211 bytes, cross-checked against the verify-lock ledger's own independent record of its acquisitions, and adopted verbatim.
Generated by Claude Code
…LID_FIELD / 400 — the code every axis naming a field already takes (#15064) Contract-review patch round on PR #15708 (verdict 5550099353), items 1, 3, 4. - `resolveRecomputeScope` stamps `code = 'INVALID_FIELD'`, `status = 400` and `field = unresolved[0]` beside `fields`, the shape of the engine's sibling producers (`assertProjectionHasNoDottedPaths`, `undeclaredWriteFieldErrors`). `FIELD_NOT_FOUND` / 404 had no producer in this repo; handing a never-emitted 404 its first producer for a condition the catalog already assigns to a 400 is the two-codes-one-condition drift ADR-0112 exists to prevent — and two of the refused shapes name a field that exists (a real non-summary field; a roll-up on an object `objects` left out), so this is an option value that could not be applied, not an addressed resource that was not found. - Pins retargeted: `summary-backfill.test.ts` "REFUSES a name it cannot resolve" (six code/status assertions, now also `field`/`fields`) and `summary-nulls.test.ts` "a refused scope entry … reaches the --json error envelope" (`code`). - `content/docs/api/error-catalog.mdx`: the `INVALID_FIELD` Cause gains the `recomputeUndefinedOnEmpty` / `--recompute-undefined-on-empty` axis; the `FIELD_NOT_FOUND` entry is untouched. - `SummaryBackfillReport.nullRows` TSDoc counts the named min/max/avg holes. - Option TSDoc, resolver docblock and the changeset paragraph updated wherever FIELD_NOT_FOUND / 404 was named. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…LD_NOT_FOUND has no producer again (#15064) Patch-round item 2 (verdict 5550099353): with the refusal answering INVALID_FIELD / 400, FIELD_NOT_FOUND loses the producer that commit 7c96f73 ratcheted the baseline for, so `scripts/error-status-unpinned-baseline.json` goes back to origin/main's blob 0596eb3 and the PR no longer touches the file. A no-op against main; not the maintainer-only baseline expansion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ibes its producer — one message for every unresolved entry, and INVALID_FIELD's siblings named as the projection and write axes (#15064) Contract re-review verdict 5550562004 (FAIL, patch round, pinned to 1914df8), two prose items, no behaviour change: 1. content/docs/api/error-catalog.mdx, INVALID_FIELD Cause: the appended sentence said the three refusal shapes each "gets its own message", which in this entry's idiom means distinct messages. resolveRecomputeScope emits ONE Error for every unresolved entry, so a typo, a real non-summary field and a roll-up on an object left out receive identical text. The sentence now says they are refused alike, one message naming every unresolved entry and how many objects the run walked. FIELD_NOT_FOUND's entry is untouched. 2. "the code every other axis that names a field answers" was over-general: the sort axis names a field and answers INVALID_SORT (engine.ts, assertOrderByIsMaterializable; assertSortFieldsExist at the ingress). Narrowed to "the projection and write axes that name a field" in the option TSDoc, the changeset paragraph (it compiles into release notes) and the REFUSES test title, each now also stating that sorting keeps INVALID_SORT. No pin reds on prose alone: check:error-status-conformance reads headings, HTTP Status lines and quick-reference rows; error-catalog-docs.test.ts reads headings and the member count. typecheck and the two vitest files stay green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Closes #15064
Maintainer ruling on #15064 (director seat, batch #39, verbatim 「同意」): option A —
backfillSummaryNullsgains a caller-supplied scope; the predicate and the unscoped run do not move. ⛔ B (relaxingsummaryNullIsBackfillable) and C (a second definition of a roll-up's value) are not taken.What was wrong
A roll-up value has three producers — the insert-time seed, the child-write recompute, and this one-off backfill — and declaring a summary field on an object that already has rows reaches none of them (measured on this tree:
initializeSummaryFieldsseeds nothing formin/max/avgand is create-time;recomputeSummariesis called only from insert/update/delete; the backfill is called only from the CLI). Forcount/sumthe backfill repairs that as a side effect. Formin/max/avgit could not:summaryNullIsBackfillable(fn)decides on the function alone, so 「never computed」 and 「no child rows」 were indistinguishable,partitionDescriptorsdropped the object beforescannedObjects.push, and the report saidfilled: 0— the false all-clear behind cloud#1908 / cloud PR #1941.What changes
@objectstack/objectql—packages/objectql/src/summary-backfill.ts(no change tosummary-aggregate.ts, no helper was needed; ⛔engine.tsuntouched — it is #15225's serial resource)SummaryBackfillOptions.recomputeUndefinedOnEmpty?: string[]—object.fieldroll-ups the caller KNOWS were never computed. A namedmin/max/avgis walked like acount: everyNULLparent is recomputed through the sameaggregateSummaryValuethe engine writes. A parent whose aggregate is the empty-set reading (null, no child rows) already holds the engine's own value, so it is neither counted as a hole nor written — the scoped run stays idempotent in the 「re-run until it reports zero」 sense. Naming acount/sumis accepted and changes nothing, so a publish path can pass every column it just declared without knowing the empty-set list.objectsleft out) is refused before any row is read, dry run or apply: ADR-0112 envelopecode: 'INVALID_FIELD',status: 400— the code the projection and write axes that name a field already answer, while sorting keepsINVALID_SORT(the engine's own rule atassertProjectionHasNoDottedPaths/undeclaredWriteFieldErrors: one condition keeps one wire code however the caller reached it, the same envelope on both doors; two of the refused shapes name a field that exists, so this is an option value that could not be applied, not a resource that was not found),fieldnaming the first unresolved entry andfieldsall of them. A silent no-op there would be the false all-clear this option exists to end.SummaryBackfillReport.recomputedUndefinedOnEmpty: string[]— the complement ofskippedUndefinedOnEmpty, sameobject.field (fn)spelling,[]on an unscoped run.SummaryBackfillFieldOutcome.fnwidens from'count' | 'sum'toSummaryDescriptor['fn'].formatSummaryBackfillReportlists the scoped columns under 「Recomputed on request」 and explains aNULLthat remains; the unscoped wording is byte-identical.@objectstack/cli—packages/cli/src/commands/migrate/summary-nulls.ts:--recompute-undefined-on-empty object.field(repeatable) passes the scope through; the--applyconfirmation names the columns; a refusal reaches the--jsonenvelope with itscodeand exits 1 (existing path).Docs:
content/docs/deployment/cli.mdx— the flag and the case it exists for;content/docs/api/error-catalog.mdx— theINVALID_FIELDCause gains therecomputeUndefinedOnEmpty/--recompute-undefined-on-emptyaxis — a typo, a real non-summary field, or a roll-up on an object left out are refused alike, one message naming every unresolved entry and how many objects the run walked, which is whatresolveRecomputeScopeemits (theFIELD_NOT_FOUNDentry is untouched). Changeset:@objectstack/objectqlminor,@objectstack/climinor — an optional parameter on a published exported function, a new report key and a new CLI flag are each a purely additive widening of a published surface (bump-level rule, 2026-09-04); thefix-shaped motivation does not lower it. Ratchet: none —scripts/error-status-unpinned-baseline.jsonis byte-identical tomain(blob0596eb336);FIELD_NOT_FOUNDkeeps no producer, andcheck:error-status-conformanceis green with the baseline untouched.Why this spelling, and what was rejected
The ruling left the spelling to the dev.
recomputeUndefinedOnEmpty: string[](a list ofobject.fieldnames under the name the card floated) was chosen because it reads beside the vocabulary the module already has: the report'sskippedUndefinedOnEmptyis the list an operator sees today, and this option is its answer — 「recompute these undefined-on-empty roll-ups」 — withrecomputedUndefinedOnEmptyclosing the round trip in the report. Rejected: a booleanrecomputeUndefinedOnEmpty(coarser than the fact the caller holds — it sweeps everymin/max/avgon the object and cannot be checked for a typo, so a wrong object name would be exactly the silent no-op again); aSummaryDescriptor[]list (a caller-built descriptor is the second definition C refuses — a name resolved against the engine's own index keeps one);fields?: string[](besideobjects?it reads as a restriction filter, the inverse of its meaning);neverComputed?: string[](states the fact but ties to nothing the module already says, and reads oddly as a CLI flag).The unscoped run is what it was
Pinned whole against the report JSON and formatter lines captured on
origin/main791a0cbe6before this change existed (summary-backfill.test.ts, 「UNSCOPED: the run is what it always was — report and wording byte-for-byte」): walk, writes, every counter,skippedUndefinedOnEmpty, and the human-readable output are byte-for-byte identical. The one visible delta on an unscoped run is the additive report keyrecomputedUndefinedOnEmpty: [].summaryNullIsBackfillableis untouched.Pins (the ruling's: card repro +
countcontrol)packages/objectql/src/summary-backfill.test.ts— 10 new, 20/20 green: the card's repro (parent with children, then amax) filled to32under the scope and leftNULL+ listed underskippedUndefinedOnEmptywithout it; the childless parent keptNULL, neither counted nor written; thecountcontrol byte-identical to the unscoped outcome;min/max/avgall computed (10/32/21); the next child write moves the column exactly as the engine's recompute would (32→40); dry-run/apply agreement; idempotence; never overwrites a stored value; the six refusal shapes assertcode+status+field/fields; the formatter.packages/cli/src/commands/migrate/summary-nulls.test.ts— 6/6: dry-run-by-default, the flag's shape, every--recompute-undefined-on-emptyentry reachingbackfillSummaryNullsin order, the unscoped run handingundefinedthrough, and theINVALID_FIELDrefusal landing in the--jsonenvelope with exit 1.Ablation (on-disk proof, absolute-path trap restore)
Mutation:
scope.has(rollupKey(desc))→ a key that never matches (the scope is never applied), proven on disk by anchor counts (1→0, marker0→1) and blob hash (≠HEAD blob). The subject is imported relatively by its test (./summary-backfill.js), so no dist leg exists. Predicted before running: the six scoped pins red; the unscoped byte-for-byte pin, thecountcontrol, the never-overwrites pin, the refusal pin and the ten pre-existing tests green. Observed on57b5d0f28(source identical to head): 6 red / 14 green, exactly the prediction. Restore bygit checkout HEAD -- ABS_PATH, proven by blob hash==HEAD blob,git diff HEADempty,git status --porcelainempty. (A first run went 7 red because the control itself assertednullRows + 1, which reads the scoped hole count — that assertion was removed so the control is a control; the re-run is the one reported.)Reverse verification of the cross-package type widening: a key
SummaryBackfillOptionsdoes not declare planted in the CLI call →pnpm --filter @objectstack/cli exec tsc --noEmitred withTS2353 … 'bogusKeyReverse15064' does not exist in type 'SummaryBackfillOptions'(so tsc reads the rebuilt.d.ts); restore proven by hash.Patch round — contract review
5550099353(FAIL, patch round, pinned to7c96f738e)Appended commits only —
f6ed5b839(items 1, 3, 4),83a41f93b(item 2), and merge1914df81foforigin/main8e500f23e(clean by content: none of this PR's files moved on main, each of this PR's 7 diff paths blob-equal to its pre-merge self — the baseline is a path in the branch's history, restored to base before the merge, not in the PR — and no generated artifact merged on both sides). No rebase, no amend, no force-push.resolveRecomputeScopeanswersINVALID_FIELD/ 400 and stampsfield = unresolved[0]besidefields. Pin:summary-backfill.test.ts「REFUSES a name it cannot resolve」 — with the source reverted to7c96f738eit reds onexpected 'FIELD_NOT_FOUND' to be 'INVALID_FIELD'(1 failed / 19 passed), restore blob-proven; at head 20/20.summary-nulls.test.ts「a refused scope entry (INVALID_FIELD) …」 retargeted, 6/6.scripts/error-status-unpinned-baseline.jsonrestored tomain's blob0596eb336; it is no longer in this PR's diff. Pairing proof withpnpm check:error-status-conformance(source-read, on a tree with nodist/): head code + old baseline → exit 1 「FIELD_NOT_FOUND: documented with an HTTP status, but no producer declares one」; old code + restored baseline → exit 1 「baselined as unpinned, but a producer now declares its status」; both → exit 0 「every derivable runtime status is documented, and every documented status is reachable」.INVALID_FIELDCause inerror-catalog.mdxgains the axis; theFIELD_NOT_FOUNDentry is untouched. No gate parses the Cause prose:check:error-status-conformancereads entry headings,**HTTP Status:**lines and the quick-reference rows;packages/spec'serror-catalog-docs.test.tsreads headings and the member count (3/3 at head).SummaryBackfillReport.nullRowsTSDoc now counts the namedmin/max/avgholes.Docs-drift list at this head: 13 pages, 11 of them anchored on the
INVALID_FIELDstring literal — read and classified as anchor-noise from the code swap (each statesINVALID_FIELD/400 for a read- or write-path field condition; none states a closed producer list, so none is falsified);content/docs/releases/v17.mdxis read-only and its sentence stays true.Patch round 2 — contract re-review
5550562004(FAIL, patch round, pinned to1914df81f)Appended commit
87385b561only — prose in four files, no behaviour change; no rebase, no amend, no force-push.INVALID_FIELDCause sentence now describes its producer. Round 1's wording said the refused shapes each 「gets its own message」, which in this entry's own idiom (its two prior uses, anddata-modeling/queries.mdx's 「with distinct messages」) means distinct messages — butresolveRecomputeScopebuilds oneErrorfor every unresolved entry, so a typo, a real non-summary field and a roll-up on an object left out receive identical text. The sentence now reads 「… when an entry is not a roll-up owned by an object the run walks — a typo, a real non-summary field, or a roll-up on an object--objectleft out are refused alike, one message naming every unresolved entry and how many objects the run walked」 — 「how many」 because the message carries the count of objects walked (of the N object(s) this run walks), not their names.FIELD_NOT_FOUND's entry is byte-identical to merge-base; it sits one line lower (332) because the sentence is one line longer. Pin: none — no gate or test parses the Cause prose:check:error-status-conformancereads entry headings,**HTTP Status:**lines and the quick-reference rows;packages/spec'serror-catalog-docs.test.tsreads headings and the member count (both green at this head, which measures nothing about the sentence); the docs-accuracy audit is the only tripwire. The alternative the verdict left open and did not order — making the sentence true in code by classifying the three shapes into distinct messages, one pin per shape — was not taken: that is a behaviour change outside a prose round and a PM call; this seat's view is on the card.INVALID_SORT(engine.tsassertOrderByIsMaterializable;assertSortFieldsExistat the ingress). Now 「the code the projection and write axes that name a field answer; sorting keepsINVALID_SORT」 in the option TSDoc (summary-backfill.ts), the changeset paragraph (it compiles into release notes, so the generalisation would have shipped), the REFUSES test title, and this body's 「What changes」 above. The commit message off6ed5b839is history and keeps its wording. Pin: none; bothtypechecks and both vitest files stay green.Verification
Re-review round at
87385b561(every exit captured before any pipe; one verify-lock hold — ledgerlabel=bash …/issue-15064-r3/heavy.sh, waited 148 s, held 171 s, never polled):turbo run build --filter='@objectstack/cli...'57/57;@objectstack/objectqlvitestsrc/summary-backfill.test.tsexit 0, 20/20;@objectstack/clivitestsrc/commands/migrate/summary-nulls.test.tsexit 0, 6/6; bothtypechecks exit 0 (--listFiles:tsconfig.jsonlists the edited source,tsconfig.test.jsonthe edited test). The 89-commanddispatch-gatesunion re-derived at87385b561and run in full, reconciled with--ran(「89 derived famil(ies) accounted for — 89 run, 0 NOT-MEASURED」): 86 exit 0 — includingcheck:error-status-conformance(「every derivable runtime status is documented, and every documented status is reachable」),check:error-code-casing,check:nul-bytes,check:docs-audit-scope, the changeset gates (check:empty-changeset,check:adr-0087-registration,check-changeset-no-majorin both spellings,check:changeset-gate-self-tests,check-changeset-fixed),check:docs-single-h1,check:docs-redirects,check:docs-transcript-drift,check:cli-examples-parity,check-docs-section-name, speccheck:docs/check:variant-docs,check:i18n,check:comment-mask-corpus— and the same three prerequisite refusals as round 1, NOT MEASURED locally, ⛔ not green: speccheck:skill-examples(exit 1,client-react/distnot built),check:dual-build-cjs-loads(exit 3, PREREQUISITE NOT MET),check:i18n-coverage(exit 3, COULD NOT MEASURE —connector-mcp/distabsent); CI'sLint & Repo Gatesis their measurement. specerror-catalog-docs.test.ts3/3. eslint on the two touched TS files via--format json: 2 files, 0 errors, 0 warnings — a measured narrowing (eslint.config.mjs「never enables type-aware linting … for ANY file」, so this diff cannot move an untouched file's verdict).content/docs/releases/**untouched (0 paths) with the firing control oncontent/docs/**naming the two edited pages. Control-byte scan of the four edited files: 0 hits.Patch round at
1914df81f(its ownos-dev-reportcomment on #15064 carries every exit code):@objectstack/objectqltargeted vitest 20/20 andtypecheck0;@objectstack/clitargeted vitest 6/6 andtypecheck0 (tsconfig.json --listFileslists the edited test file, so the green covers it);pnpm check:error-status-conformance0; the 89-commanddispatch-gatesunion run at1914df81fand reconciled with--ran(89 derived / 89 run / 0 unrun) — 86 exit 0, three prerequisite refusals from packages outside the built closure (spec check:skill-examples,check:dual-build-cjs-loads,check:i18n-coverage: NOT MEASURED locally; CI'sLint & Repo Gatesis the measurement).First delivery, at
7c96f738e:pnpm --filter @objectstack/objectql typecheck(tsc + scripts + test-typecheck) green;pnpm --filter @objectstack/cli typecheckgreen — the only downstream consumer of the widened types (pnpm --filter '...@objectstack/objectql'direction: downstream) is the CLI, by grep;dispatch-gatesderived union run in full and reconciled with--ran; eslint on the four touched TS files via--format json: 4 files, 0 errors, 0 warnings — a measured narrowing, sinceeslint.config.mjs「never enables type-aware linting … for ANY file」 so this diff cannot move an untouched file's verdict.Out of scope, for the cloud seat: cloud#1908 / cloud PR #1941 can wire
recomputeUndefinedOnEmptyinto the AI publish path once this releases (Clause-②: yes— a published exported function widens).🤖 Generated with Claude Code
https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Generated by Claude Code
Generated by Claude Code