From fd410bca84914e2542664d16c310944d891d0edf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 12:46:51 +0000 Subject: [PATCH] feat(expense): make total_amount a live summary roll-up (#84) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that nested cross-object writes are safe (framework#1867 fixed) and the runtime computes the `summary` field type, replace the hand-maintained denormalized `expense_report.total_amount` with a native live roll-up: total_amount: Field.summary({ summaryOperations: { object: 'expense_line', field: 'amount', function: 'sum', relationshipField: 'expense_report' }, }) The engine recomputes it on every expense_line insert/update/delete — no hook, no client-maintained header. Supporting changes: - expense_report.hook.ts: drop the `total_amount = 0` insert default (computed now). - data/index.ts: stop seeding `total_amount` on reports — the summary derives it from the seeded lines as they load. - The "submitted needs total > 0" validation now fires on the draft→submitted TRANSITION (previous.status != "submitted"), when the lines already exist, instead of on every write — so a report seeded directly in its final state isn't gated by rollup timing. - CHARTER.md: rewrite the "Rollup vs. lifecycle hooks" section for the live summary. Verified at runtime (objectstack dev --fresh, real SQLite): seed loads with zero errors and a report's total_amount (2223) equals the sum of its four seeded lines (2223), recomputed live over the expense_line→expense_report lookup FK. Gate: pnpm typecheck ✓ · pnpm build ✓ · pnpm format:check ✓. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BZguyAaQbyUpwMZ2gMLaAP --- packages/expense/CHARTER.md | 67 ++++++++++--------- packages/expense/src/data/index.ts | 13 ++-- .../src/objects/expense_report.hook.ts | 3 +- .../src/objects/expense_report.object.ts | 18 +++-- 4 files changed, 55 insertions(+), 46 deletions(-) diff --git a/packages/expense/CHARTER.md b/packages/expense/CHARTER.md index 5f168e0..66e2d3f 100644 --- a/packages/expense/CHARTER.md +++ b/packages/expense/CHARTER.md @@ -36,37 +36,42 @@ fork points. | Internationalization | English + 简体中文 ship out of the box | | Realistic seed data | 6 categories, 5 reports across every state, 13 lines | -## Rollup vs. lifecycle hooks (an important runtime constraint) - -`expense_report.total_amount` is the sum of its lines. The obvious way to keep -it current is a child hook that, on every line change, recomputes the sum and -writes it back to the parent. **This template deliberately does *not* do that**, -and the reason is worth understanding before you fork: - -- Hook bodies run inside a single shared **QuickJS (asyncify) sandbox** that - allows only **one** suspended async call at a time. A hook that performs a - *nested* engine write — `ctx.api.object('expense_report').update(...)` from - an `expense_line` hook — re-enters that sandbox while the line hook is still - suspended, corrupting asyncify state and **crashing the process** - (`memory access out of bounds`). This is a hard platform limit in the - standalone runtime, not a bug in the hook. -- Several reference templates appear to roll up via `ctx.services.data`, but - that service is **undefined inside the sandbox**, so those rollups silently - no-op — they neither update the parent nor crash. Don't copy that pattern - expecting it to work. - -So in this template `total_amount` is treated as a stored header field: -defaulted on insert, shipped correct in the seed, and meant to be maintained by -the client (or a server-side aggregation in your fork). What a hook *can* safely -do is mutate **its own** incoming payload — which is exactly what -`expense_report.hook.ts` does for the auto-number and the lifecycle timestamps -(`submitted_at` / `approved_at` / `reimbursed_at`), with no nested writes. - -**Fork point:** to make the total self-maintaining, compute it outside the -sandbox — e.g. a native aggregate/summary field once the runtime computes the -`summary` field type, a database trigger/view, or an external worker that -listens for line changes and patches the parent at top level (not nested in a -hook). +## Rollup: `total_amount` is a live summary + +`expense_report.total_amount` is the sum of its lines, kept current +automatically by a native **`summary` field** — the engine recomputes it +whenever an `expense_line` is inserted, updated, or deleted: + +```ts +total_amount: Field.summary({ + summaryOperations: { + object: 'expense_line', + field: 'amount', + function: 'sum', + relationshipField: 'expense_report', + }, +}); +``` + +No hook, no denormalization, no client-maintained header. Two consequences worth +knowing before you fork: + +- `total_amount` is **not seeded** — it is computed, so the summary derives it + from the seeded lines as they load. +- The "submitted reports need a positive total" rule fires on the + draft→submitted **transition** (`previous.status != "submitted"`), when the + lines already exist — not on every write — so a report seeded directly in its + final state isn't gated by rollup timing. + +**History:** earlier versions of this template hand-maintained `total_amount` as +a stored header field, because a child hook doing a nested parent write +(`ctx.api.object('expense_report').update(...)`) crashed the sandbox +(`memory access out of bounds`, framework#1867). That is now fixed — nested +writes from hooks are safe. For a plain aggregate the `summary` field above is +the delete-safe, declarative tool; for a **non-aggregate** cross-object rollup +(e.g. copying a child's latest status onto the parent) an `afterInsert` / +`afterUpdate` hook doing `ctx.api.object(parent).update(...)` is now the +sanctioned pattern. ## Limits (LOC budget) diff --git a/packages/expense/src/data/index.ts b/packages/expense/src/data/index.ts index 363b42d..55148f5 100644 --- a/packages/expense/src/data/index.ts +++ b/packages/expense/src/data/index.ts @@ -10,11 +10,11 @@ import { ExpenseLine } from '../objects/expense_line.object'; * Seed data — covers the full template surface: * • 8 categories with soft per-transaction limits (incl. Mileage, Telecom) * • 5 reports spanning every lifecycle state + the approval threshold - * • 13 lines whose amounts match each report's total_amount + * • 13 lines whose amounts roll up into each report's total_amount * - * Report totals are set explicitly because `total_amount` is a stored header - * field maintained at the top level — there is no line rollup hook (a nested - * parent write from a line hook crashes the sandbox; see CHARTER.md). + * Report totals are NOT seeded: `total_amount` is a live `summary` roll-up that + * the engine computes from the seeded lines (sum of `expense_line.amount`) as + * they load. See CHARTER.md ("Rollup vs. lifecycle hooks"). */ const categories = defineSeed(ExpenseCategory, { @@ -75,7 +75,6 @@ const reports = defineSeed(ExpenseReport, { period_end: cel`daysAgo(33)`, cost_center: 'SALES', currency: 'usd', - total_amount: 2223, submitted_at: cel`daysAgo(32)`, approved_at: cel`daysAgo(30)`, reimbursed_at: cel`daysAgo(25)`, @@ -91,7 +90,6 @@ const reports = defineSeed(ExpenseReport, { period_end: cel`daysAgo(6)`, cost_center: 'SALES', currency: 'usd', - total_amount: 247, submitted_at: cel`daysAgo(3)`, }, { @@ -103,7 +101,6 @@ const reports = defineSeed(ExpenseReport, { period_end: cel`daysAgo(14)`, cost_center: 'ENG', currency: 'usd', - total_amount: 550, submitted_at: cel`daysAgo(10)`, approved_at: cel`daysAgo(7)`, payment_method: 'payroll', @@ -117,7 +114,6 @@ const reports = defineSeed(ExpenseReport, { period_end: cel`daysFromNow(13)`, cost_center: 'MKT', currency: 'usd', - total_amount: 850, }, { title: 'Misc taxi receipts', @@ -128,7 +124,6 @@ const reports = defineSeed(ExpenseReport, { period_end: cel`daysAgo(12)`, cost_center: 'OPS', currency: 'usd', - total_amount: 69, submitted_at: cel`daysAgo(9)`, notes: 'Rejected — split personal and business rides, then resubmit.', }, diff --git a/packages/expense/src/objects/expense_report.hook.ts b/packages/expense/src/objects/expense_report.hook.ts index 2a68ac2..869337a 100644 --- a/packages/expense/src/objects/expense_report.hook.ts +++ b/packages/expense/src/objects/expense_report.hook.ts @@ -31,7 +31,8 @@ const reportHook: Hook = { if (event === 'beforeInsert') { if (!input.status) input.status = 'draft'; - if (input.total_amount == null) input.total_amount = 0; + // total_amount is a live summary roll-up now — the engine computes it from + // the report's lines; do not seed a header default here. if (!input.report_number) { const ts = new Date(); const date = `${ts.getUTCFullYear()}${String(ts.getUTCMonth() + 1).padStart(2, '0')}${String( diff --git a/packages/expense/src/objects/expense_report.object.ts b/packages/expense/src/objects/expense_report.object.ts index 46dc187..2a13fb6 100644 --- a/packages/expense/src/objects/expense_report.object.ts +++ b/packages/expense/src/objects/expense_report.object.ts @@ -83,13 +83,17 @@ export const ExpenseReport = ObjectSchema.create({ ], description: 'Single currency per report. Multi-currency FX is a fork point.', }), - total_amount: Field.currency({ + total_amount: Field.summary({ label: 'Total Amount', group: 'financial', - min: 0, - defaultValue: 0, + summaryOperations: { + object: 'expense_line', + field: 'amount', + function: 'sum', + relationshipField: 'expense_report', + }, description: - 'Sum of line amounts. Stored header field maintained client-side/seed (not a line rollup hook — that crashes the sandbox; see object docstring).', + 'Sum of the report’s line amounts — a live roll-up the engine recomputes whenever a line is inserted/updated/deleted. Previously a hand-maintained stored field because a nested-write rollup hook crashed the sandbox (framework#1867, now fixed).', }), approval_required: Field.formula({ label: 'Approval Required', @@ -162,7 +166,11 @@ export const ExpenseReport = ObjectSchema.create({ type: 'script', severity: 'error', message: 'Submitted reports must have at least one expense line (total > 0).', - condition: P`record.status == "submitted" && (record.total_amount == null || record.total_amount <= 0)`, + // total_amount is a live summary now (0 until lines roll up), so this fires + // only on the draft→submitted TRANSITION — when the lines already exist — + // not on every write. The seed inserts reports in their final state + // directly (previous == null), so it is not gated by the rollup timing. + condition: P`previous != null && previous.status != "submitted" && record.status == "submitted" && (record.total_amount == null || record.total_amount <= 0)`, }, { name: 'submitted_requires_purpose',