Repository navigation
Commit c0803b0
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)
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_TIER` review 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 `formula` and declares `expression` is a stored
computed column: it has a column, so a `summary` can roll it up. The
engine evaluated `expression` for `formula` fields only, so the value a
client sent for such a field was stored as-is. Now
`@objectstack/objectql` computes it on every write, so a client-sent
value never stands (shape B).
- **Insert** (`insert`, batch insert, `insertMany`): computed over the
defaulted row before the `beforeInsert` hooks, 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.
- **Update by id**: computed over the stored row merged with the
caller's patch, before `beforeUpdate`, written onto the raw payload
(never through the hook-write recording) and never onto a `readonly` key
the #16344 hide pass withheld, so the hooks see the expression over the
pre-strip row (a `readonly` key 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 the `readonlyWhen` strip 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.
- **Predicate update** (`multi: true`): one patch reaches every matched
row (the D7 `readPriorRows` memo, 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 with
`VALIDATION_FAILED` before anything is written. The field error is
`invalid_value` and names the field. Its per-row `beforeUpdate` hooks
run before the settle and are handed the caller's patch.
- One evaluator: the read path's `applyFormulaPlan`, planned by the same
entry builder as the `formula` read plan (`formulaPlanEntry`, factored
out of `planFormulaProjection`). The scale rounding, the one hop, the
`can()` permission map and the once-per-field fault report are the
read's. An expression that does not evaluate stores `null`, as a
`formula` reads `null`.
- No new export, class member, spec key or error code. The new helpers
are module-level functions in `engine.ts`.
- `packages/spec/src/data/field.zod.ts`: the `expression` describe and
TSDoc now say what the key means on a `formula` field and on a stored
field. This is a `domain:spec` file, declared to that lane by the claim.
`content/docs/references/**` was regenerated (`gen:docs`).
- Both examples' comments (`showcase_invoice_line.amount`,
`crm_opportunity_line_item.amount`) now say what the server does. No
metadata changed. `amount` needs no `readonlyWhen` of its own: on a paid
invoice, the locked inputs keep it frozen.
- Changeset: `minor` for `@objectstack/objectql` and
`@objectstack/spec`, with a **BREAKING** banner, FROM / TO / Fix, and an
`adr-0087: not-required (no-migration-prescription)` marker.
## Measurement (M, a booted composition)
The showcase and the CRM were booted with `objectstack dev --seed-admin
--fresh` and driven over HTTP. Before: `origin/main` at `f89d29e382`.
After: this branch.
| Write (showcase, line of the PAID INV-1003, qty 6 × 49.99) | before:
line amount / invoice total | after: line amount / invoice total |
|:---|:---|:---|
| `POST /api/v1/batch` update `{quantity:60, amount:2999.40}`, atomic |
2999.4 / **2999.4** (quantity dropped `readonly_when`) | 299.94 /
**299.94** (quantity dropped `readonly_when`) |
| `PATCH /api/v1/data/showcase_invoice_line/ID {"amount":1}` | 1 / **1**
| 299.94 / **299.94** |
| Write (showcase, draft INV-1002, the unpaid control) | before | after
|
|:---|:---|:---|
| `PATCH {"amount":5}` | amount 5, total 5 | amount unchanged |
| `PATCH {quantity:5, unit_price:20, amount:100}` (the grid's consistent
save) | stored as sent | stored as sent: 5 / 20 / 100, total 100 |
| `PATCH {"quantity":7}` | amount stale | amount 140, total 140 |
| `POST` line `{quantity:2, unit_price:10, amount:999}` | amount 999 |
amount 20 |
| CRM (`crm_opportunity_line_item`, inputs not locked) | before | after
|
|:---|:---|:---|
| `POST` line `{quantity:2, unit_price:10, amount:999}` | amount 999,
`line_total` 999 | amount 20, `line_total` 20 |
| `PATCH {"quantity":3}` | amount stays 999, `line_total` 999 | amount
30, `line_total` 30 |
Seeds: all 5 seeded showcase lines already satisfy `amount == quantity *
unit_price`, so every invoice `total` is 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-`formula` fields declaring `expression`
Every `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 a `Field.formula`
/ `type: 'formula'` or not a field. There are exactly two stored ones:
| Field | Writers | Readers |
|:---|:---|:---|
| `showcase_invoice_line.amount` (`invoice.object.ts`) | the console
line-item grid (recomputes client-side and sends the computed value),
the seed (5 rows, all consistent), the REST / MCP API (any value, before
this PR). No hook, flow or import writes it. | `showcase_invoice.total`
summary, and through it `revenue-pulse.dataset.ts`,
`renewals-pipeline.page.ts` (chart + list), `invoice-document.page.ts` |
| `crm_opportunity_line_item.amount` (`opportunity-line-item.object.ts`)
| the console grid (also `inlineAmountField: 'amount'` for the grid
footer), the API. No seed, hook or flow. | `crm_opportunity.line_total`
summary |
Before this PR, nothing said what a non-`formula` field's `expression`
means on the server. The spec describe said only "Formula expression
(CEL)". The lint (`validate-expressions.ts` field-formula pass) judges
any field's `expression` as a CEL `value` expression, and no rule
distinguishes stored from `formula`.
`content/docs/data-modeling/validation-rules.mdx` lists `expression`
under `formula` only. The two example comments said the server stores
the client's value.
## The four-axis call (shape B over shape A)
- **Actual business need.** Measured: the card's paid-lock bypass, plus
the stale or wrong stored `amount` on 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 (a `readonlyWhen` on the
showcase `amount`) closes only the first. Shape B closes both, in both
apps, with no per-app metadata.
- **Long-term soundness.** B is platform-first: one engine rule instead
of every app remembering to copy its inputs' lock onto the derived
column. Under A, `expression` on a stored field would stay a declared
key the server does not honour.
- **Preventing AI authoring errors.** Under B, declaring an `expression`
is what enforces it. An author or agent who declares `expression` on a
stored field gets the computation it declared, and an agent that sends a
wrong or stale value cannot store it.
- **Startup-stage scope.** No new spec key, export, error code or gate.
B gives an existing, consumed key the meaning both producers already
assumed.
- **Overwrite, not refuse.** A whole-record round-trip (form save,
read-modify-write, the grid) echoes the stored `amount` back. 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 (the `formula` and read-only strips). The one refusal
is the predicate update that cannot be expressed as one statement. That
follows the bulk `readonlyWhen` strip's fail-safe rule, and uses the
existing `VALIDATION_FAILED` / `invalid_value` (checked first:
`FieldErrorCode` in `packages/spec/src/api/errors.zod.ts`, plus the
engine's `ReadonlyFieldRejectedError` / `readonly-strict-errors.ts`
codes, 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):
- the card's two writes on a paid line: the `/batch`-shaped update keeps
`quantity` 6, stores `amount` 299.94, reports exactly `[quantity]
readonly_when`, and leaves `total` 299.94. The `amount`-only PATCH
stores 299.94 and leaves `total` 299.94;
- the unpaid control: the grid's consistent save is stored as sent;
- insert and update, each with no amount, a wrong amount and the right
amount; batch insert per row; a patch that moves no input;
- `beforeUpdate` is handed the expression over the pre-strip row, and an
input that a hook changes is recomputed;
- predicate update: one value stored; untouched when no input moves;
refused (`VALIDATION_FAILED`, field `amount`, `invalid_value`) and
nothing written when the values would differ per row;
- a stored field that is `readonly` and declares `expression`, updated
by id with its value and a moved input: the caller's value is reported
as a `readonly` drop and refused under `strictReadonlyWrites`, 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.js` relatively, so it runs
source, not `dist`. With `node scripts/ablation-replace.mjs`, the
planner's guard (`if (def && typeof def === 'object' && def.type !==
'formula' && def.expression) {`) was replaced by `if (false && …`. The
mutation landed (anchor 1 → 0, blob `fd733c5e5f21` → `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 restore
`fd733c5e5f21` == HEAD, `git diff HEAD` empty.
**Pin sweep.** `git grep -n "expression" -- '*.object.ts'` and the tests
naming `showcase_invoice_line` / `crm_opportunity_line_item` were read.
No existing pin asserts a client-sent stored-expression value. The
dogfood persona matrix inserts a showcase line with no `unit_price`,
which now stores `amount` `null`, as it did before (absent). No
`readonly_when` pin moves: the drop set on the paid line is unchanged.
## Verification record
Head of record: `c22a069d3` (fix round 1; round 0 was `c01e06227`)
(`origin/main` `cc95f5e63` merged in; the incoming commits touch none of
`packages/objectql`, `packages/spec` or `examples/`).
- `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.
- typecheck, exit 0 each, re-run after the merge:
`@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 with `gen:docs`, then "All
14 generated artifacts are up to date", re-run after the merge.
- Fix round 1: the 116 families derived for this round's paths were
re-run on `c22a069d3`, all exit 0 (`--ran`: a DERIVED zero). The other
two of the 118 read only the two example objects, byte-identical to
`c01e06227`.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 118 families. All 118 were re-run on
the head of record `c01e06227`, 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)".
- On the first pass, three families did not pass. `check:skill-examples`
and `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET: unbuilt
`client-react` and 7 other packages); both passed after building those
packages. `check:objectql-double-limit` exited 1 because the new suite's
`find` double was limit-blind; the double now honours the bound and the
gate exits 0.
- The repository was deepened first (`--shallow-since=2026-07-13`) for
`check-engine-split-ratio --days 90` and `check-plugin-teardown-shape`.
## Acceptance notes
- **Not closed by either shape: adding a line to a paid invoice.** `POST
/api/v1/data/showcase_invoice_line` on the paid INV-1003 is accepted,
before and after this PR, and moves its `total`, because `readonlyWhen`
is 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 of `amount` is the caller's value.
- The fault log for a stored expression that does not evaluate reuses
the `formula` sentence ("formula field 'amount' … reads null").
- A stored `currency` field that declares no `scale` stores the
evaluator's double unrounded, as a `formula` does (`3 * 0.1` →
`0.30000000000000004`). Both examples' seeded values are exact.
- The console grid was not driven in a browser (no `../objectui` in 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.
- This PR touches no governed surface.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01J2MKvkwHvmoNaHpU2wuqJu)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent ecb7b44 commit c0803b0
9 files changed
Lines changed: 551 additions & 38 deletions
File tree
- .changeset
- content/docs/references
- data
- system
- examples
- app-crm/src/objects
- app-showcase/src/data/objects
- packages
- objectql/src
- spec/src/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
99 | | - | |
| 99 | + | |
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
262 | 262 | | |
263 | 263 | | |
264 | 264 | | |
265 | | - | |
| 265 | + | |
266 | 266 | | |
267 | 267 | | |
268 | 268 | | |
| |||
618 | 618 | | |
619 | 619 | | |
620 | 620 | | |
621 | | - | |
| 621 | + | |
622 | 622 | | |
623 | 623 | | |
624 | 624 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
100 | | - | |
| 100 | + | |
101 | 101 | | |
102 | 102 | | |
103 | 103 | | |
| |||
526 | 526 | | |
527 | 527 | | |
528 | 528 | | |
529 | | - | |
| 529 | + | |
530 | 530 | | |
531 | 531 | | |
532 | 532 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
59 | 60 | | |
60 | 61 | | |
61 | 62 | | |
| |||
0 commit comments