Repository navigation
fix(objectql)!: a stored field's declared expression is computed on every write, so a paid invoice's line amount and total stay frozen - #24673
Conversation
…write A non-formula field that declares `expression` (a stored computed column a summary can roll up, such as an invoice line's `amount`) is now computed by the engine from the row the write stores - the stored row merged with the patch on an update, the defaulted row on an insert - after the hooks and both read-only strips. A client-sent value never stands. A predicate update that would compute different values on different matched rows is refused. Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
…pdate and predicate update Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
…be and both examples Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
…aller's bound Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
…rived-amount-lock
📓 Docs Drift CheckThis PR changes 2 package(s): 12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 144 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 78a567b29c467179c2960e187c8d189e99db5df8 && git checkout 78a567b29c467179c2960e187c8d189e99db5df8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 045dae8f23d21e7631e425bb851cad5ab2bc26fd c22a069d3dbc63130ecf78b590d5550ee2677d96 && git checkout -B drift-repro 045dae8f23d21e7631e425bb851cad5ab2bc26fd && git merge --no-ff c22a069d3dbc63130ecf78b590d5550ee2677d96
node scripts/docs-audit/affected-docs.mjs --json 045dae8f23d21e7631e425bb851cad5ab2bc26fd
|
Contract reviewServed-tier: ① Derived judgmentsThe computation point: two passes on insert and by-id update, one on predicate update.
The pre-hook pass on by-id writes inside the hook-write recording window, and that moves an undeclared accept-set edge — WRONG.
On update, the merged row is a read the write already makes; the cost and the authority. Overwrite, in every context. The predicate-update refusal. Null on fault. The paid-line pins. No public surface moves. ② Semver level
③ Boundary flags
Implemented-by: VERDICT: FAIL FAIL on ① alone: the by-id pre-hook pass writes through the hook-write recording and ahead of the read-only strips, so a stored field declaring Seat note ·
|
…hook-write recording It wrote through the recording proxy, so a stored expression field also declared readonly lost its readonly drop report and its strictReadonlyWrites refusal when the patch moved an input. The pass now writes onto the raw payload and skips the keys the hide pass withheld. Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
…ssion The describe and TSDoc, the changeset's TO and hooks bullets and its adr-0087 marker now state the predicate path (left alone when no input moves, refused when the values differ) and what each path hands its hooks. Claude-Session: https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: ① Derived judgmentsThe fix: by-id pass 1 now writes onto the recording's target and skips every withheld key — RIGHT.
The new pins have teeth against the baseline's defect — RIGHT. The test file gains exactly two Carried over unchanged from the baseline record, since the code they rest on is byte-identical at this head: the paid-line pins (the ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS PASS on the delta: the baseline's one FAIL ground is closed at this head — by-id pass 1 writes onto the recording's target and skips every withheld key, so a |
席位核对 ·
|
| Item | Reading |
|---|---|
| Shape | Draft PR → main. The body opens Refs #24619 (item a), with no closing keyword. 9 files, +551/−38. Not governed: check-governed-merges matched 0 of 9 paths. |
| Scope | Shape B, by the four axes in the dev's report. A stored, non-formula field's declared expression is computed by objectql on insert, on update by id (the stored row merged with the patch, after the before* hooks and both read-only strips), and on a predicate update (one value for every matched row, else refused VALIDATION_FAILED / invalid_value). A caller's value is overwritten, never stored. Measured on a booted showcase: before, the card's /batch update moved the paid INV-1003's total from 299.94 to 2999.4, and an amount-only PATCH set it to 1; after, neither moves it. Draft lines save with amount = quantity * unit_price. The CRM's line_total follows its lines. Every seed is unchanged. The census found two such fields, showcase_invoice_line.amount and crm_opportunity_line_item.amount, and both are accounted for. |
| Seat's own read | The record's FAIL ground was checked on the base head (seat note on 6279561155). On this head, the by-id pass 1 binds opCtx.data, the recording's target, and skips every key the hide pass withheld. git merge-tree --write-tree against today's origin/main (1c1144fcd8) is clean, and the PR reads mergeable: clean. The PR body was patched to this head with the dev's suggested lines, as the delta weighed them, plus the delta's one added clause. |
| Review | CONTRACT_REVIEW_TIER, narrowing face. Record 6279561155 was FAIL on c01e062278: by-id pass 1 wrote through the hook-write recording, so a readonly: true + expression field lost its readonly drop report and its strictReadonlyWrites refusal. The delta 6280886549 is PASS on this head. |
| Declaration | One changeset, @objectstack/objectql minor and @objectstack/spec minor, BREAKING (FROM / TO / Fix), Clause-②: no (narrowing) at a line start, with adr-0087: not-required (no-migration-prescription). The spec diff is the expression describe and its TSDoc only, so its accept set is byte-identical. No new export, no new ObjectQL member and no new error code. |
| Cross-lane | domain:spec, for packages/spec/src/data/field.zod.ts: 6279243125 and 6279251400, with the addenda naming this head 6280905054 and 6280912886. domain:devx, for the regenerated content/docs/references/**: 6279260461 and 6279265790, with the addenda 6280920064 and 6280927836. The hand-written validation-rules.mdx / field-types.mdx gap is noted there as devx's follow-up. No objection on the card or the seat posts. |
| Gates | Dev at c01e062278: 118 derived, 118 run, all exit 0. Fix round at c22a069d3: the 116 families derived for its 7 paths, all exit 0; check:generated 14 of 14 up to date. Full suites: objectql 449 files / 8461 passed, spec 702 files / 21349 passed, showcase 431 passed, CRM 46 passed, core test:repo 82 passed. |
| CI | At this read on c22a069d3d: ci-failure GREEN, all 41 check-runs completed, none failed. |
Disposition:
open_questions: 1–4 taken as the dev settled them, per record 6279561155 ③ and the delta: shape B; overwrite rather than refuse; the predicate update refused when the value differs per row; no rounding. Open question 5 (the fault sentence readsformula field) is the dev's deviation 1, a follow-up for the next touch ofreportFormulaFault.deviations: both accepted (delta 6280886549 ③): the shared fault wording kept, and the insert path's#16344comment kept.out_of_scope_findings:- finding 1, an insert of a line under a paid invoice moving its
total, was filed by the seat as showcase: a new line can be inserted on a paid invoice, so its total moves — readonlyWhen is insert-exempt by design, and nothing else locks a paid header against new lines #24684 (p2, this lane, dispatched); - finding 2,
validate()not computing stored expressions, stays noted; it is unmeasured on a public door.
- finding 1, an insert of a line under a paid invoice moving its
- The silent overwrite has no
onFieldsDroppedchannel. It is the seat's question to the spec lane (6279594471, 6279599508); reporting it would need a newDroppedFieldsEvent.reason.
At landing: a Release: line on the card. Row (a) is landed. Rows (b) and (c) were routed by triage 6278345889 to #24662 (domain:cli) and #24664 (domain:services). The card is released with pm:retriage, and the seat clears its pm:dispatched and assignee.
Landing: ordinary queue (pr_ready + automerge_enable through the relay). Refs closes nothing.
Generated by Claude Code
Refs #24619 (item a)
Clause-②: no (narrowing) (either shape narrows what a write of a derived stored value is accepted as: a locked value stays locked, or a declared computation is enforced; it owes a
CONTRACT_REVIEW_TIERreview on the narrowing face before the queue)Row (a) only. Rows (b) (
packages/rest,HookConditionError) and (c) (service-external-datasource,listRemoteTables) are not touched here; #24619 remains open for them.What changes
A field that is not a
formulaand declaresexpressionis a stored computed column: it has a column, so asummarycan roll it up. The engine evaluatedexpressionforformulafields only, so the value a client sent for such a field was stored as-is. Now@objectstack/objectqlcomputes it on every write, so a client-sent value never stands (shape B).insert, batch insert,insertMany): computed over the defaulted row before thebeforeInserthooks, so a hook is handed the stored value. It is computed again over the final row, after the hooks and the strips and before validation and the driver.beforeUpdate, written onto the raw payload (never through the hook-write recording) and never onto areadonlykey the Update-side: a readonly field is stripped from persistence but still reaches beforeUpdate, so hook-derived columns persist values computed from data the row never contains #16344 hide pass withheld, so the hooks see the expression over the pre-strip row (areadonlykey the caller sent stays withheld from the hooks) and the read-only strips still judge the caller's submission. It is computed again after the hooks and both read-only strips, so a locked input that thereadonlyWhenstrip took back out feeds the stored value, never the caller's. A key the caller sent is overwritten. A key the patch does not carry is added only when the computed value differs from the stored one.multi: true): one patch reaches every matched row (the D7readPriorRowsmemo, no second read). The computed value is stored when it is the same on every row. A patch that moves no input leaves the column alone and drops the caller's value. A patch that would compute different values on different rows is refused withVALIDATION_FAILEDbefore anything is written. The field error isinvalid_valueand names the field. Its per-rowbeforeUpdatehooks run before the settle and are handed the caller's patch.applyFormulaPlan, planned by the same entry builder as theformularead plan (formulaPlanEntry, factored out ofplanFormulaProjection). The scale rounding, the one hop, thecan()permission map and the once-per-field fault report are the read's. An expression that does not evaluate storesnull, as aformulareadsnull.engine.ts.packages/spec/src/data/field.zod.ts: theexpressiondescribe and TSDoc now say what the key means on aformulafield and on a stored field. This is adomain:specfile, declared to that lane by the claim.content/docs/references/**was regenerated (gen:docs).showcase_invoice_line.amount,crm_opportunity_line_item.amount) now say what the server does. No metadata changed.amountneeds noreadonlyWhenof its own: on a paid invoice, the locked inputs keep it frozen.minorfor@objectstack/objectqland@objectstack/spec, with a BREAKING banner, FROM / TO / Fix, and anadr-0087: not-required (no-migration-prescription)marker.Measurement (M, a booted composition)
The showcase and the CRM were booted with
objectstack dev --seed-admin --freshand driven over HTTP. Before:origin/mainatf89d29e382. After: this branch.POST /api/v1/batchupdate{quantity:60, amount:2999.40}, atomicreadonly_when)readonly_when)PATCH /api/v1/data/showcase_invoice_line/ID {"amount":1}PATCH {"amount":5}PATCH {quantity:5, unit_price:20, amount:100}(the grid's consistent save)PATCH {"quantity":7}POSTline{quantity:2, unit_price:10, amount:999}crm_opportunity_line_item, inputs not locked)POSTline{quantity:2, unit_price:10, amount:999}line_total999line_total20PATCH {"quantity":3}line_total999line_total30Seeds: all 5 seeded showcase lines already satisfy
amount == quantity * unit_price, so every invoicetotalis identical before and after the boot (INV-1001 1619.96, INV-1002 198, INV-1003 299.94, INV-1004 450). The CRM seeds no line items.Census: stored, non-
formulafields declaringexpressionEvery
expression:key in the tree was scanned (*.ts, tests excluded) and each one's owning field was resolved: examples,platform-objects, plugins, services,metadata-core. Every other hit is aField.formula/type: 'formula'or not a field. There are exactly two stored ones:showcase_invoice_line.amount(invoice.object.ts)showcase_invoice.totalsummary, and through itrevenue-pulse.dataset.ts,renewals-pipeline.page.ts(chart + list),invoice-document.page.tscrm_opportunity_line_item.amount(opportunity-line-item.object.ts)inlineAmountField: 'amount'for the grid footer), the API. No seed, hook or flow.crm_opportunity.line_totalsummaryBefore this PR, nothing said what a non-
formulafield'sexpressionmeans on the server. The spec describe said only "Formula expression (CEL)". The lint (validate-expressions.tsfield-formula pass) judges any field'sexpressionas a CELvalueexpression, and no rule distinguishes stored fromformula.content/docs/data-modeling/validation-rules.mdxlistsexpressionunderformulaonly. The two example comments said the server stores the client's value.The four-axis call (shape B over shape A)
amounton any unlocked line written through the API (showcase draft lines, every CRM line). An AI agent writing through MCP is exactly such a writer. Shape A (areadonlyWhenon the showcaseamount) closes only the first. Shape B closes both, in both apps, with no per-app metadata.expressionon a stored field would stay a declared key the server does not honour.expressionis what enforces it. An author or agent who declaresexpressionon a stored field gets the computation it declared, and an agent that sends a wrong or stale value cannot store it.amountback. Refusing a value that differs would turn those writes into 400s. The platform's precedent for a runtime-owned value is to accept the write and not store the caller's value (theformulaand read-only strips). The one refusal is the predicate update that cannot be expressed as one statement. That follows the bulkreadonlyWhenstrip's fail-safe rule, and uses the existingVALIDATION_FAILED/invalid_value(checked first:FieldErrorCodeinpackages/spec/src/api/errors.zod.ts, plus the engine'sReadonlyFieldRejectedError/readonly-strict-errors.tscodes, which describe dropped locks, not a per-row computation).Pins
packages/objectql/src/engine-stored-expression-write.test.ts(14 cases, the real engine over a real-shaped driver):/batch-shaped update keepsquantity6, storesamount299.94, reports exactly[quantity] readonly_when, and leavestotal299.94. Theamount-only PATCH stores 299.94 and leavestotal299.94;beforeUpdateis handed the expression over the pre-strip row, and an input that a hook changes is recomputed;VALIDATION_FAILED, fieldamount,invalid_value) and nothing written when the values would differ per row;readonlyand declaresexpression, updated by id with its value and a moved input: the caller's value is reported as areadonlydrop and refused understrictReadonlyWrites, and the stored value is the expression's (fix round 1; ablation with pass 1 on the recording payload: 2 failed | 12 passed).Ablation. The test imports
./engine.jsrelatively, so it runs source, notdist. Withnode scripts/ablation-replace.mjs, the planner's guard (if (def && typeof def === 'object' && def.type !== 'formula' && def.expression) {) was replaced byif (false && …. The mutation landed (anchor 1 → 0, blobfd733c5e5f21→c7fc76a3bea4): 10 failed | 2 passed. Every paid-line pin is red. The two that stay green are the consistent grid save and the no-input patch, whose behaviour is the same either way. Restored: blob after restorefd733c5e5f21== HEAD,git diff HEADempty.Pin sweep.
git grep -n "expression" -- '*.object.ts'and the tests namingshowcase_invoice_line/crm_opportunity_line_itemwere read. No existing pin asserts a client-sent stored-expression value. The dogfood persona matrix inserts a showcase line with nounit_price, which now storesamountnull, as it did before (absent). Noreadonly_whenpin moves: the drop set on the paid line is unchanged.Verification record
Head of record:
c22a069d3(fix round 1; round 0 wasc01e06227) (origin/maincc95f5e63merged in; the incoming commits touch none ofpackages/objectql,packages/specorexamples/).pnpm --filter @objectstack/objectql test: 449 files / 8461 tests passed.pnpm --filter @objectstack/spec test: 702 files / 21349 passed, 1 todo.pnpm --filter @objectstack/core test:repo: 6 files / 82 passed.pnpm --filter @objectstack/example-showcase test: 36 files / 431 passed.pnpm --filter @objectstack/example-crm test: 6 files / 46 passed.@objectstack/objectql(incl.check:test-typecheck),@objectstack/spec,@objectstack/example-showcase,@objectstack/example-crm.pnpm --filter @objectstack/spec check:generated: one stale artifact (content/docs/references/**), regenerated withgen:docs, then "All 14 generated artifacts are up to date", re-run after the merge.c22a069d3, all exit 0 (--ran: a DERIVED zero). The other two of the 118 read only the two example objects, byte-identical toc01e06227.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 118 families. All 118 were re-run on the head of recordc01e06227, and all exited 0.--ran: "118 derived famil(ies) accounted for — 118 run, 0 NOT-MEASURED (a DERIVED zero — all 118 recorded an exit code and none of them is 3)".check:skill-examplesandcheck:dual-build-cjs-loadsexited 3 (PREREQUISITE NOT MET: unbuiltclient-reactand 7 other packages); both passed after building those packages.check:objectql-double-limitexited 1 because the new suite'sfinddouble was limit-blind; the double now honours the bound and the gate exits 0.--shallow-since=2026-07-13) forcheck-engine-split-ratio --days 90andcheck-plugin-teardown-shape.Acceptance notes
POST /api/v1/data/showcase_invoice_lineon the paid INV-1003 is accepted, before and after this PR, and moves itstotal, becausereadonlyWhenis exempt on INSERT by design. Measured before the fix: total 1 → 8 with a 7.00 line. Reported to the seat as a finding; not fixed here.ObjectQL.validate()(the insert dry-run) does not compute stored expressions. Its preview ofamountis the caller's value.formulasentence ("formula field 'amount' … reads null").currencyfield that declares noscalestores the evaluator's double unrounded, as aformuladoes (3 * 0.1→0.30000000000000004). Both examples' seeded values are exact.../objectuiin this container). The grid recomputes the same product client-side, so its consistent saves are stored as sent, as the API measurement of the same payload shows.Generated by Claude Code