Skip to content

Commit cbcffa8

Browse files
claudeclaude
committed
fix(objectql): name the bulk-write limitation instead of blaming the author, and record the per-row contract (#5037)
The rc-window stopgap half of the 2026-08-04 ruling on #4800 / #4862. A hook `condition` reading `previous` on a predicate (`multi: true`) write is unevaluable — the engine matches N rows, fires the hook once and binds no prior record — and since #4775 that rejects the write. #4861 already gave the case its own sentence, but that sentence predates the ruling and led with "rewrite the condition without `previous`", which is advice to silently turn a transition into a state test. The contract is now the opposite: on a bulk write, after hooks and record-change flow triggers evaluate and fire PER ROW. - `docs/adr/0058`: addendum recording the contract, today's measured behaviour, the stopgap and its expiry (#5038), and the consequences #5038 must price. - `hook-wrappers.ts`: the rejection states it is a CURRENT-VERSION limitation, cites the contract and #5038, leads with the single-record route, and prices the rewrite. `HookConditionError.limitation` (`bulk_write_previous_unbound` | `bulk_write_stored_state_unavailable`) is the machine-readable discriminator — deliberately not `code`, since ADR-0112 makes `error.code` a closed wire vocabulary and rest-server promotes a thrown error's `.code` onto the envelope. - "does this condition read `previous`" now comes from the parsed CEL AST (`collectCelRootIdentifiers`), computed once at wrap time, with the fault-text check as fallback — so the diagnosis no longer rides on cel-js's wording, and `record.previous_status` is correctly NOT a `previous` reference. - `scripts/adr-anchors.json`: anchors ADR-0058 + ADR-0112 to hook-wrappers.ts. Single-record writes, bulk writes whose conditions do not name `previous`, and the undeclared-key typo report are unchanged; fail loud takes no exception. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
1 parent 94f7b6a commit cbcffa8

7 files changed

Lines changed: 692 additions & 23 deletions

File tree

‎.changeset/bulk-write-previous-hook-condition-diagnostic.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): a bulk write blocked by a `previous` hook condition is told it is a VERSION limit, not an authoring mistake (#5037)
6+
7+
#4775 made an unevaluable hook `condition` abort the operation, and #4861 gave
8+
the predicate-bulk-write case its own sentence instead of a raw
9+
`Unknown variable: previous`. What that sentence still said was
10+
*"rewrite the condition without `previous`, or target the write at one record"* —
11+
written before the maintainer's 2026-08-04 ruling on #4800/#4862, and wrong in
12+
its most important claim. The ruling settled the contract: **on a bulk write,
13+
after-hooks and record-change flow triggers evaluate and fire per row** (recorded
14+
as an ADR-0058 addendum, implemented by #5038). The author's transition condition
15+
is legitimate; the engine is what is behind. Telling them to drop `previous` was
16+
advising a silent semantic change — a transition ("just became done") becomes a
17+
state test ("is done"), which fires on every row that was already done.
18+
19+
The rejection now says what is actually true:
20+
21+
- it names the batch and why there is no single prior record to bind, as before;
22+
- it states this is a **current-version limitation**, cites the per-row contract
23+
(ADR-0058 addendum, #4800/#4862) and the issue that retires the rejection
24+
(#5038);
25+
- it leads with the route that works today — target the write at one record, and
26+
the same condition evaluates as authored — and prices the rewrite instead of
27+
recommending it;
28+
- it still refuses to point at a record-change flow trigger as a way out, which
29+
remains verified rather than assumed: that trigger binds the same lifecycle
30+
hooks and receives the same unbound `previous` on a bulk write (#4862).
31+
32+
**Machine-readable, so a caller never parses the prose.** `HookConditionError`
33+
gains `limitation?: 'bulk_write_previous_unbound' |
34+
'bulk_write_stored_state_unavailable'` (exported as `HookConditionLimitation`)
35+
alongside the existing `predicateBulkWrite` flag. It is deliberately *not* named
36+
`code`: ADR-0112 makes `error.code` a closed wire vocabulary
37+
(`StandardErrorCode` ∪ `ERROR_CODE_LEDGER`) and `rest-server.ts` promotes a
38+
thrown error's `.code` onto the response envelope, so a `.code` here would mint
39+
an unregistered wire code as a side effect. A code that needs to travel goes
40+
through the ledger as a decision.
41+
42+
**"Does this condition read `previous`" is now read off the parsed CEL AST**
43+
(`collectCelRootIdentifiers`, the utility #4972's build gate already uses),
44+
computed once at wrap time, with the old fault-text check kept as a fallback.
45+
The diagnosis no longer depends on cel-js's wording, and it stays correct when
46+
the evaluator faults on some other key the same condition reads.
47+
`record.previous_status` is not a `previous` reference — the AST reports roots,
48+
not member names — so it keeps the declared-field diagnosis, whose remedy is the
49+
right one there.
50+
51+
Unchanged, and pinned by tests: single-record writes (bound `previous`, condition
52+
evaluates, handler runs), bulk writes whose conditions do not name `previous`,
53+
the plain undeclared-key typo report on a bulk write, and fail-loud itself — the
54+
write still fails. Nothing here is an exemption; it is the same rejection with a
55+
diagnosis attached.

‎docs/adr/0058-expression-and-predicate-surface.md‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,60 @@
5656
5757
---
5858

59+
> **Addendum (2026-08, #4800 / #4862 / #5037) — BULK-WRITE SCOPE: on a predicate
60+
> (`multi: true`) write, after-hooks and record-change flow triggers evaluate and
61+
> fire PER ROW.** _Contract recorded here; implementation tracked by #5038; the
62+
> rc window ships a named diagnostic in its place._
63+
>
64+
> The addendum above settles what happens when a write-path predicate cannot be
65+
> evaluated. It does not settle **what the evaluation is even over** when one
66+
> write touches N rows — and that is a scope question this ADR owns, because
67+
> "the same CEL means the same thing on every surface" is D6's premise.
68+
>
69+
> **The decision.** A bulk write is N record changes, so every record-scoped
70+
> declaration on it is evaluated **per row**, with `record` = that row's state
71+
> and `previous` = that row's pre-write state. This is not a new idea on the
72+
> platform: validation predicates have worked this way on bulk writes since
73+
> #3106 (`rulesNeedRows` fetches the matched rows and `evaluateValidationRules`
74+
> runs once per row). Hook `condition`s — and the record-change flow triggers
75+
> that ride the same lifecycle hooks — join them. An author writes one
76+
> transition condition (`previous.done != true && record.done == true`) and it
77+
> means the same thing whether the write carries an id or a predicate.
78+
>
79+
> **What the engine does today, measured (#4862).** A `multi: true` update
80+
> reaches `driver.updateMany`, which resolves an affected COUNT; the lifecycle
81+
> hook fires **once**, `hookContext.previous` is never assigned (only the
82+
> single-id branch fetches a prior row), and `record` degrades to the write's
83+
> bare payload. So a condition naming `previous` is unevaluable and — since the
84+
> #4775 row above — **rejects the write**.
85+
>
86+
> **The rc-window stopgap (#5037).** The rejection stands: fail loud takes no
87+
> exception here (the alternatives — logging an error and skipping the hook, or
88+
> skipping it silently — were considered and refused on #4800, because a missing
89+
> audit row is the one failure nobody goes looking for). What changed is that it
90+
> must no longer read as an author's mistake. `HookConditionError` carries a
91+
> machine-readable `limitation` (`bulk_write_previous_unbound`,
92+
> `bulk_write_stored_state_unavailable`) and a message that names the batch, says
93+
> the CURRENT VERSION is what cannot bind the row's prior state, points at the
94+
> contract above, and gives the route that works today (target the write at one
95+
> record). It is a stopgap with an expiry: when #5038 lands per-row evaluation
96+
> the condition evaluates as authored and this rejection has nothing left to
97+
> report.
98+
>
99+
> **Deliberately not written into that message:** "use a record-change flow
100+
> trigger instead". Verified, not assumed — that trigger subscribes to these very
101+
> lifecycle hooks, so on a bulk write it fires once with the same unbound
102+
> `previous` (#4862). Naming it would have made the error that fixes a
103+
> `declared ≠ delivered` into another one.
104+
>
105+
> **Consequences to price when #5038 implements this**: an after-hook that fires
106+
> once per batch today fires N times (notification hooks send N messages,
107+
> cache-invalidation hooks run N times), so the shape of `ctx.result` per row,
108+
> the per-row meaning of `onError`, and a ceiling on very large matched sets are
109+
> part of that implementation, not free riders on it.
110+
111+
---
112+
59113
## TL;DR
60114

61115
ObjectStack exposes **~50 authorable declarations** that hold an expression — formulas, visibility/required/readonly predicates, validation rules, hook conditions, flow/edge conditions, sharing-rule conditions, RLS `using`/`check`, action/view/app visibility, notification/ETL/export/sync/connector conditions — and they all funnel through **one authoring primitive** (`ExpressionInputSchema` → `{ dialect: 'cel', source }`, helpers `cel`/`F`/`P`). The authoring surface is already unified and clean.

‎packages/objectql/src/core.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export { applyInMemoryAggregation, bucketDateValue } from './in-memory-aggregati
5757
export { bindHooksToEngine } from './hook-binder.js';
5858
export type { BindHooksOptions, BindHooksResult } from './hook-binder.js';
5959
export { wrapDeclarativeHook, HookConditionError } from './hook-wrappers.js';
60-
export type { WrapDeclarativeOptions } from './hook-wrappers.js';
60+
export type { WrapDeclarativeOptions, HookConditionLimitation } from './hook-wrappers.js';
6161

6262
// Validation
6363
export { ValidationError, validateRecord } from './validation/record-validator.js';

0 commit comments

Comments
 (0)