Repository navigation
docs(objectql): state the #16074 per-row previous rule in the published docblock - #18625
Merged
huangyiirene merged 3 commits intoSep 17, 2026
Merged
Conversation
…shed docblock `dispatchPerRowBeforeHooks`'s docblock and one test comment still stated the RETIRED rule — per-row `previous` supplied "so a guard can REFUSE the write (throw), not so a rewrite can be aimed". Ruling #16074 (landed in `@objectstack/spec` by PR #17249) admits a row-invariant-in-effect rewrite: the same written key set on every matched row, assigned IN PLACE, kept safe by the engine's `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal. Key-set divergence, a per-row value and a row-conditioned REPLACEMENT of `ctx.input.data` stay out of contract. Prose only; the runtime already follows the new rule. The docblock is published surface — the retired sentence ships in six files of the `@objectstack/objectql@17.4.0` tarball, including `dist/util-Dw5ZTIII.d.ts:3554` on a member of the exported `ObjectQL` class — so a changeset is owed. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude <noreply@anthropic.com>
The docblock is published text (it ships in `dist/util-*.d.ts`), so a changeset is owed. Graded `minor` because this PR declares clause ②: the level axis in `check-changeset-no-major.mjs` refuses a declared clause ② whose every moved package is graded `patch`. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude <noreply@anthropic.com>
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
github-actions
Bot
added
documentation
…s declared The seat overturned its own `Clause-②: yes` grading on #17975 and posted the correction: this card adds no exported symbol, no key and no accepted value, so it widens nothing. The widening was PR #17249's, in `@objectstack/spec`. The `yes` had leaned on a precedent that moved exported SYMBOLS through a barrel; this moves prose, and the four in-repo precedents for a prose repair inside a published package are all `patch`. With no clause ② declared, `check-changeset-no-major`'s level axis stands down, which is what made the `minor` grade necessary in the first place. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Sep 17, 2026
huangyiirene
marked this pull request as ready for review
September 17, 2026 10:20
huangyiirene
deleted the
claude/issue-17975-objectql-previous-docblock
branch
September 17, 2026 10:44
huangyiirene
pushed a commit
that referenced
this pull request
Sep 17, 2026
…ocks Brings in PR #18625 (card #17975), the sibling prose repair in the same contract family, so this branch's wording can be aligned with the spelling that actually shipped. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude <noreply@anthropic.com>
huangyiirene
pushed a commit
that referenced
this pull request
Sep 17, 2026
…bulary, add the changeset Follow-up to the same two sites after merging PR #18625 (card #17975), the sibling prose repair that landed in this package. - Both sites now use the package's shipped spelling for the dispatch model: per matched row, on the single-record shape, ADR-0058 Addendum II D1/D2. - The second site no longer names the opt-in `dispatchUnscopedMultiWrite` dispatch. `wrapDeclarativeHook` is applied at registration (`hook-binder.ts:221`), but no metadata binding can set that flag today, so naming it as an absence case this function observes would be a claim that cannot be shown to be reachable. The truthful statement — any update-shaped context whose prior row is not in hand — is true by construction. - Adds the `patch` changeset: the first docblock is emitted verbatim onto the exported `hookRecordState` declaration in the published `.d.ts`. Prose only: no behaviour change, no assertion change. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Sep 17, 2026
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Sep 28, 2026
…wrapper docblocks (objectstack-ai#18637) Fixes objectstack-ai#18331 Clause-②: no Two docblocks in `packages/objectql/src/hook-wrappers.ts` stated the **retired batch model** for the `before*` dispatch of a predicate (`multi: true`) bulk write **in the present tense**, while ruling objectstack-ai#16074 / ADR-0058 Addendum II (D1/D2) and the engine beneath them both dispatch per matched row and bind that row's pre-image. Prose only: no behaviour change, no assertion change. Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af ## The two sites, located by symbol | site | symbol | retired sentence | |:--|:--|:--| | 1 | the docblock attached to the exported `hookRecordState` (it opens "Choose the record-shaped object the condition formula evaluates against", and describes `pickRecordPayload`) | "Its `before*` dispatch **still fires once for the batch with no prior row**, so that payload is left exactly as it is" | | 2 | `pickPreviousPayload`, section "When it is ABSENT" | "it **fires ONCE for N matched rows**, so there is no single prior record to bind; `previous` stays unbound" | What the code actually does, on this branch's base: `dispatchPerRowBeforeHooks` (`engine.ts`) loops the matched rows and builds each context with `previous: coerceBooleanFields(schema, row)`; the call sites for `beforeUpdate` and `beforeDelete` both take that path whenever the object has hooks and at least one row matched. A live behavioural pin already asserts it — `hook-condition-bulk-previous.test.ts`, "the bulk write objectstack-ai#5037 and objectstack-ai#5038 both rejected now SUCCEEDS, firing per row", boots a real engine, runs `multi: true` with a `previous.done != true` transition condition on `beforeUpdate`, and asserts one dispatch per row. ## R2 — does the text ship? Measured, and the two sites do NOT behave the same Measured the way objectstack-ai#17975 did, against the **real published tarball** (`npm pack @objectstack/objectql@17.4.0`), then again against a local build of this branch's final head. - **Site 1 SHIPS.** The retired sentence is present in the published tarball at `dist/util-Dw5ZTIII.d.ts:8039` and the matching `.d.mts`. The reason is worth recording: `hookRecordState` has **two** leading comment blocks in the source — the detached `pickRecordPayload` docblock, then its own — and the dts bundler emits **both** verbatim onto `declare function hookRecordState(...)` (published `.d.ts` line 8061). That file is the shared chunk behind both the `.` and `./core` entrypoints, so every consumer's editor surfaces the retired sentence on hover. - **Site 2 does NOT ship.** `fires ONCE for N matched rows` is absent from every file in the published tarball. `pickPreviousPayload` is module-private; it appears in `dist/` only as an `{@link}` reference inside another docblock. - After this change, on the final head: site 1's replacement is in `dist/util-DWbR4Mtj.d.ts:8223` + `.d.mts`, site 2's unique marker ("any update-shaped context") is in `src` once and in **zero** dist files, and neither `fires once` nor `fires ONCE` survives anywhere in `dist/`. Because site 1 moves published text, this carries a **`patch`** changeset, matching the four in-repo precedents for prose repairs inside published packages and the sibling PR objectstack-ai#18625. It adds no exported symbol, no key and no accepted value — the accept set was widened by PR objectstack-ai#17249 in `packages/spec`, not here — hence `Clause-②: no`. ## What the second site says instead, and why it does not name a dispatch The natural replacement for site 2 was to name the one `before*` context that still carries no row — the opt-in `dispatchUnscopedMultiWrite` dispatch, fired once for a `multi: true` write with no caller predicate at all. That draft was **withdrawn before pushing**: `wrapDeclarativeHook` is applied at registration (`hook-binder.ts:221`), but the engine's own note records that no metadata binding can set that flag today, so naming it as an absence case *this function observes* would be a claim whose reachability cannot be shown. The sentence now states what is true by construction — any update-shaped context whose prior row is not in hand — and then says plainly that a predicate bulk write is no longer one of those. Vocabulary is deliberately aligned with what PR objectstack-ai#18625 landed in this package (per matched row, on the single-record shape, ADR-0058 Addendum II D1/D2) rather than inventing a second spelling. objectstack-ai#18625's own `ROW-INVARIANT-IN-EFFECT` wording is about which *rewrites* are admitted, which neither of these two sites discusses, so it is not copied in as padding. ## Tests and gates, on final head `0cd458243a` - `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2` — **299 test files, 4995 tests, all passed**, lock verdict `command-exit 0`. - `pnpm --filter @objectstack/objectql typecheck` — green, including `check:test-typecheck` (40 files / 234 errors / 65 pinned signatures held in the shrink-only ledger, unchanged). - Dependency-closure build — `turbo run build --filter=@objectstack/objectql` 14/14 successful. - `eslint . --no-inline-config` over the **whole** repo: 6811 files, **0 errors, 0 warnings**. No narrowing was needed, so no narrowing is claimed. - Gate families derived on that same sha with `scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack`: **54 derived, 54 run, 0 unrun** (reconciled with `--ran`, each line carrying its exit code captured before any pipe). 51 green; the three non-zero ones are below. | gate | exit | reading | |:--|:--|:--| | `pnpm check:cross-package-test-inputs` | 1 | **Known gate defect, already filed as objectstack-ai#18621** — it reds whenever `packages/spec/dist/` exists, which the dependency-closure build creates. The flag is rooted in `packages/cli/test/init-created-files-summary.e2e.test.ts` descending `packages/spec/dist/`; neither path is in this diff. Named, not chased, and no twin card filed. | | `pnpm check:dual-build-cjs-loads` | 3 | **PREREQUISITE NOT MET**, in the gate's own words: it reads built output and most packages have no `dist/` in this worktree. Nothing was measured — recorded as NOT MEASURED, not as a pass and not as a red. CI's `Build Core` covers it. | | `node scripts/check-engine-split-ratio.mjs --days 90` | 2 | The gate **refuses to answer** because this container's clone is shallow and its oldest visible commit sits inside the 90-day window. NOT MEASURED. It is an ADR-0076 D7 repo-history metric, orthogonal to a comment-only diff; CI checks out with the history it needs. | ## Acceptance notes **R3, judged occurrence by occurrence — both extra name-matches are false positives, and no edit is owed to either.** Sweeping `fires once` / `fires ONCE` over `packages/objectql/src` returns 4 hits: the 2 repaired here, plus - `hook-input-shape-contract.test.ts:136` — "`before*` **now fires once PER MATCHED ROW** (D1), so the count is the row count". This states the **current** contract, correctly, and the assertion under it (`expect(seen).toHaveLength(2)` for a two-row batch) pins the per-row model. Correct as written. - `plugin-metadata-event-outage.test.ts:33` — "It would also fire once per event during an outage, where the boot **line fires once per process**". A statement about a degradation log line's emission frequency; unrelated to hook dispatch in every respect. Correct as written. Neither file was touched. **No follow-up card is owed for either** on this measurement; the seat decides. **Under-count check inside the declared surface.** The rest of `hook-wrappers.ts` mentions the batch model in five more places, and every one of them is already correct: the `HookConditionError` note ("the `before*` phase **was** dispatched ONCE for the whole batch") and the retired-helpers block are **past-tense history**, which is correct as history; "The batch-scoped `hookContext` still exists inside `update()`/`delete()`, but it is never handed to `triggerHooks`" is a true present-tense statement; and the `unevaluableConditionError` note already says "a predicate write dispatches `before*` once per matched row now". Only the two cited docblocks were defective, so the card's count is exact for this file. **Noted, not filed.** The detached docblock shape at site 1 — a block describing `pickRecordPayload` left sitting above `hookRecordState`, which the dts bundler then publishes as if it were part of `hookRecordState`'s own documentation — is an observation about layout, not a defect in any of the three filing classes: the text is accurate after this change and the emit is TypeScript behaving as documented. Successor that would meet it: any PR re-documenting these helpers. Recording it because it is why site 1 ships at all, which was not obvious from the source. **Out of surface, untouched:** `packages/objectql/CHANGELOG.md` still carries the retired phrasing in released entries. That is a dated record of what was true then and is correctly left alone. --- _Generated by [Claude Code](https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17975
Clause-②: no
What moved
Two sites still stated the RETIRED per-row
previousrule in the present tense. Both now state the #16074 ruling as@objectstack/specstates it (PR #17249:hook.zod.ts,bulk-write-hook-conformance.ts):packages/objectql/src/engine.ts— the docblock ofprivate async dispatchPerRowBeforeHooks, clause D3.packages/objectql/src/bulk-write-per-row-hooks.test.ts— the comment ona 'previous'-reading GUARD can now refuse a bulk write per row.The ruled contract as written: a per-row
previous-conditioned rewrite is ADMITTED when its written KEY SET is the same on every matched row AND is assigned IN PLACE; key-set divergence is REFUSED byMULTI_UPDATE_HOOK_KEY_DIVERGENCE(#14099); a rewrite AIMED at one row stays outside the contract. Both sites were located by SYMBOL, not by the line numbers on the card.⛔ No behaviour change. The runtime already follows the new rule — the three provenance stamps (
email-template-provenance.ts,sharing-rule-provenance.ts,webhook-provenance.ts) all assign in place. ⛔packages/objectql/CHANGELOG.mdis untouched: a changelog is a dated record of what was true then.R2, re-measured here: the docblock IS published surface, and the repair reaches it
The card's load-bearing claim, verified against the real tarball rather than taken on trust:
npmtarball@objectstack/objectql@17.4.0unpacked: the retired sentence is present in 6 published files —dist/core.js,dist/core.mjs,dist/index.js,dist/index.mjs,dist/util-Dw5ZTIII.d.mtsanddist/util-Dw5ZTIII.d.tsline 3554 — the exact reading the card recorded. It sits on a member ofdeclare class ObjectQL, which both the.and./coreentrypoints export, so every consumer's editor surfaces it on hover.pnpm --filter '@objectstack/objectql...' buildon this branch: the retired sentence is absent fromdist/entirely, and the corrected text is atdist/util-B8LXqiG6.d.ts:3613(plus.d.mtsand the four JS bundles) — the same six files, corrected.⇒ the grading stands: this is a contract-face edit on published text, and
'@objectstack/objectql'owes a changeset.R3, the broader sweep: still exactly two live restatements
The card's instrument (
not so a rewrite) was 8 hits, 2 live. The sweep here covered the IDEA, not one string — 15 further spellings, includingrewrite can be aimed,aimed at one row,widens to every matched row,is supplied so,conditioned on the row,row-invariant/ROW-INVARIANT,out of contract,outside the contract,guard can REFUSE,can REFUSE,not to aim,never so a rewrite,only a guard,cannot rewrite,must not rewrite— overpackages/**,docs/**,content/docs/**,scripts/**andskills/**.Result: no third live restatement. Everything else is one of (a) already stating the ruled contract —
packages/spec/src/data/hook.zod.ts,packages/spec/src/data/bulk-write-hook-conformance.ts,content/docs/data-modeling/formulas.mdx:405,scripts/adr-anchors/packages__spec__src__data__bulk-write-hook-conformance.ts.json; or (b) a dated record — the four CHANGELOG lines,docs/adr/0058-*.md:236(which carries its own "Amended in Amendment II.3 below" pointer), and.changeset/hook-previous-row-invariant-rewrite.mdquoting the old text on purpose.packages/objectql/src/multi-update-hook-key-divergence.ts:12-15quotes the addendum's widening MECHANISM, which #16074 did not retire — swept and left alone.The changeset level — asked, answered,
patchGraded
patch, and this PR declares no clause ②.It moves published PROSE: no exported symbol, no key, no accepted value. The accept set was widened by PR #17249 in
@objectstack/spec, not here. The dispatching seat had graded the cardClause-②: yesas the conservative call, re-measured it, and overturned its own grading — the correction is on the card as comment 5712379730, in the fixed spelling (Clause-②-correction: 5711903408, thenClause-②: no). The originalyesleaned on #16872, which moved exported SYMBOLS through a barrel; this moves prose, and the four in-repo precedents for a prose repair inside a published package are allpatch(abc4b83ce8,484484041a,7851fa3146,2d5945a00f).With no clause ② declared,
check-changeset-no-major's level axis stands down. Measured on this head through the gate's ownjudgeLevelwith this PR's real inputs (packagesTouchednames@objectstack/objectql):patchplusnoreturnsnot-declared, 0 offenders. Theminorthis PR carried for one commit existed only to satisfy that axis under the declaration since corrected.Verification
Two runs, each named with the sha it was taken at. The full sweep below ran at
cd22004048; the regrade commitc3de704e18is this PR head, and the families the changeset path derives were re-run and re-reconciled there.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsagainst the real change set (3 paths) → 63 families. All 63 ran, each exit code written to disk before any was read, then reconciled:dispatch-gates --ranreports63 derived, 61 run, 2 NOT-MEASURED, 0 UNRUN.c3de704e18.dispatch-gates --commands .changeset/17975-objectql-per-row-previous-docblock.mdderives 18 families for the changeset path; all 18 ran and all 18 exited 0. One reconciliation, one count:18 derived, 18 run, 0 NOT-MEASURED, 0 UNRUN. ⛔ The other 60 were not re-run — a changeset level is outside what they read.cd22004048. Includingcheck:nul-bytes,check:engine-double-contract,check:dts-closure,check:published-files,check:test-source-alias,check:type-check-coverage,check:comment-mask-adoption,check:comment-mask-corpus,check:changeset-gate-self-tests,check-empty-changeset --base origin/main,check-changeset-no-major --base origin/main(locallyno-pull-request: the axis has no PR body to read outside CI, which is why the table above was measured throughjudgeLeveldirectly).pnpm check:dual-build-cjs-loads— exit 3,PREREQUISITE NOT MET: reads built output for 66 packages, needs a whole-repopnpm build. CI's.pnpm check:type-check-debt— exit 3, same class: 18 workspace dependencies have no built type entry point here, and the gate refuses rather than measuring a different world.node scripts/check-engine-split-ratio.mjs --days 90— exit 2, REFUSED: this clone is shallow and its oldest visible commit sits inside the 90-day window. The--self-testleg is green.pnpm check:cross-package-test-inputsexits 1 here, onpackages/cli/test/init-created-files-summary.e2e.test.tsdescendingpackages/spec/dist/. It is gated onstatSync(packages/spec/dist).isDirectory(), so it fires only in a tree where spec has been BUILT. Control: the same gate run in a fresh, unbuiltorigin/mainworktree exits 0. None of this PR's three paths appears in the finding.pnpm --filter @objectstack/objectql test→ 298 files, 4990 tests, all passing.pnpm --filter @objectstack/objectql typecheck→ exit 0. Targeted re-run of the two files that carry this contract (bulk-write-per-row-hooks.test.ts,multi-update-hook-key-divergence.test.ts): 77 passing. Every heavy run went throughscripts/pm/os-verify-lock.sh; all three reportVERDICT command-exit 0.multi-update-hook-key-divergence.test.tspins both directions (a row-invariant rewrite is never refused, however its values differ, and the divergence refusal). The repaired test comment now points a reader at it.pnpm exec eslint --no-inline-config --format jsonover the two changed source files: 2 files, 0 errors, 0 warnings. Population:eslint.config.mjs:971lints**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}minusNEVER_LINTED. Why the narrowing excludes nothing: that config, in its own words ateslint.config.mjs:328, "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file" — with no cross-file type information in play, a comment-only diff cannot move the verdict on a file it does not contain. The repo-wide sweep is CI's run.Authored by the
domain:enginedev seat, sessionsession_01CqmCgU5RGDoJYhHUMVp2af(https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af).Acceptance notes
scripts/check-cross-package-test-inputs.mjschanges verdict on build state: the walked-root leg is guarded bystatSync(ROOT).isDirectory(), so a clean checkout passes and the same commit fails oncepackages/spec/dist/exists. CI lints before building, somainstays green and only local runs see it. Not fixed here (out of scope, and it is a gate, not this contract) — reported to the dispatching seat with dedupe words.dispatch-gatesreports this branch as STALE TREE —origin/mainmoved 2 commits during the run and one derived file (scripts/pm/check-harness-current.mjs) changed across that range. The derivation above is against this PR's own tree; no path of this diff is in that file.Generated by Claude Code