From 01d0080a6ba47c974c824ffc5b5b04318c57c815 Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:02:06 +0000 Subject: [PATCH 1/3] Stamp lateness on duly_task: late_after at dispatch, completed_late at completion Two write-once stamps make lateness a plain date comparison everywhere, so objectstack#14104 stops being a blocker rather than being resolved. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- AGENTS.md | 34 ++ src/dashboards/duty-health.dashboard.ts | 106 +++--- src/data/demo-history.ts | 21 ++ src/data/task.seed.ts | 11 + src/datasets/duty-health.dataset.ts | 128 ++++--- src/hooks/task.hook.ts | 189 +++++++++- src/jobs/dispatch.plan.ts | 32 ++ src/objects/task.object.ts | 83 ++++- src/views/task.view.ts | 63 +++- test/dashboard.test.ts | 90 ++++- test/datasets.test.ts | 40 ++- test/dispatch.test.ts | 72 ++++ test/task-hook.test.ts | 447 ++++++++++++++++++++++++ 13 files changed, 1197 insertions(+), 119 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6dd7368..c9931b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -138,6 +138,40 @@ rejects it. Use `position` (distribution), `permission_set` (capability), `is_open`. A stored flag needs a writer that runs every midnight; a formula field is virtual and a filter naming one silently matches nothing. Ask `status` and `due_date` directly — they are stored and indexed. + + **The one exception, and its exact boundary (#52).** A value may be stored + when it is **written once, at the instant it becomes knowable, and never + recomputed** — a historical fact, not a maintained flag. `duly_task` carries + four: `completed_at` and `completed_late` (stamped at completion), + `visible_from` and `late_after` (`due_date + duty.grace_days`, stamped at + dispatch). + + The test is not "is it derivable" — every one of these is derivable. It is + **what happens the day nobody writes it**: + + | | maintained flag (`is_late`) | write-once fact (`completed_late`) | + |:--|:--|:--| + | needs a writer at midnight | yes — and lies the day it does not run | no | + | changes when config changes | yes, retroactively and silently | no, by design | + | wrong answer looks like | a stale flag nobody can see is stale | nothing: it is what was true then | + + So the question to ask of a candidate column is: **would a second write ever + have to happen?** If yes — if it tracks the clock, or has to be refreshed + when a related record is edited — it is the banned shape, whatever it is + called. If no, and it records what was true at a moment that has passed, it + is the same category as `completed_at` and it may be stored. + + Two obligations come with using this exception, both load-bearing: + + - **`readonly: true` and ONE writer**, which is the hook or the planner. A + column a caller can set is a column that drifts. + - **Say what the write-once cost is, where the reader will hit it.** For + `late_after`: editing a duty's `grace_days` does not move the deadline on + tasks already dispatched. That surprises whoever has just corrected a + misconfiguration, so the `late` view's comment names `duly_catalog_sync` + — the action that already exists to replay duty edits — as the place a + recompute would belong. An undocumented denormalisation is the drift this + rule is really about. 6. **`sharingModel` is mandatory and fail-closed.** Unset means private, and the publish linter errors on it (ADR-0090 D1/D7). State it deliberately. 7. **A hierarchy scope REQUIRES `requires: ['hierarchy-security']`. Omitting it diff --git a/src/dashboards/duty-health.dashboard.ts b/src/dashboards/duty-health.dashboard.ts index c810524..16b7538 100644 --- a/src/dashboards/duty-health.dashboard.ts +++ b/src/dashboards/duty-health.dashboard.ts @@ -11,45 +11,37 @@ import { Dashboard } from '@objectstack/spec/ui'; * rather than on this file. Nothing on this screen writes — managers read * here, and assigning is their only write in the product. * - * ── What is NOT on this dashboard, and why that is deliberate ───────────── + * ── The on-time rate, and what it is a rate OF ─────────────────────────── * - * The card asked for five things in reading order. Two of them — *"Late"* and - * *"On-time rate, current period"* — are the SAME missing comparison, not two - * separate omissions: + * The card asked for five things in reading order, and two of them — *"Late"* + * and *"On-time rate"* — used to be one missing comparison, + * `completed_at <= due_date + duty.grace_days`, which no filter grammar can + * express (no date arithmetic, no column-to-column comparison on the SQL path, + * and the offset is itself a column: objectstack#14104). * - * completed_at <= due_date + duty.grace_days (on-time) - * now > due_date + duty.grace_days (late) + * #52 answered it by moving the comparison off the query and onto the row: + * `late_after` is stamped at dispatch, `completed_late` at completion, both + * write-once. So the tile below binds `on_time_rate`, a derived ratio of two + * plain counts, and needs no platform change — #14104 stopped being a blocker + * rather than getting resolved. `src/datasets/duty-health.dataset.ts` carries + * the reasoning. * - * Both need `due_date + duty.grace_days`, and the filter grammar cannot say - * it: there is no date arithmetic in `FILTER_OPERATORS`, the `{N_days_ago}` - * macro vocabulary is relative to NOW and never to another column, and the - * offset here is itself a column. Column-to-column comparison (`$field`) is - * refused by `driver-sql` while the in-memory evaluator resolves it, so even - * the half that parses would be a per-deployment answer. Filed upstream as - * **objectstack-ai/objectstack#14104**; `src/datasets/duty-health.dataset.ts` - * carries the full measurement, and it is why no dataset in this app declares - * an on-time or a late measure for a widget to bind. + * **It is a rate over COMPLETED work** — `tasks_done_on_time / tasks_done` — + * and the description below says so on the screen. That is the number a + * compliance manager is asked for, and it is not the same as "how much of what + * was owed got done": still-open work is not in it. Stagnation is what answers + * for work that has not finished, which is why the not-moving tile keeps the + * headline position rather than being displaced by this one. * - * A grace-free approximation IS expressible here — a widget `filter` of - * `due_date < {today}` over `tasks_due` needs no platform change at all — and - * it is deliberately not built. It marks late every task still inside the - * grace window its own duty grants, so a customer who configured 7 days of - * grace gets its people listed as late the morning after the due date. That - * is not a rounding error; it is wrong in exactly the direction the customer - * configured against, and it would be wrong invisibly. The product decision is - * open on **#52** (with **#48** on the `late` LIST view, which does carry the - * grace-free definition today); when it lands, the tile lands with it. + * A grace-free approximation is STILL not built here, and is still the thing + * to refuse: a widget `filter` of `due_date < {today}` over `tasks_due` is a + * "late" count in two lines, and it marks late every task inside the grace its + * own duty grants. `test/dashboard.test.ts` pins that shape out. * - * The reordering is an improvement rather than a loss: the card already wanted - * the not-moving tile to be the visual focus, and with lateness deferred it is - * unambiguously the headline. Stagnation is also the EARLIER signal — a task - * untouched for three weeks and not due for another two is already a problem, - * and no lateness measure can see it until it is too late to act. - * - * The absence is stated on the screen itself, in `description` below, for the - * same reason the caliber note is: a manager reading "not moving: 3" on a - * dashboard that says nothing about lateness will conclude there are three - * problems. An unexplained absence is a wrong number with no digits. + * The ordering rule stands for the same reason it always did: stagnation is + * the EARLIER signal — a task untouched for three weeks and not due for + * another two is already a problem, and no lateness measure can see it until + * it is too late to act. * * ── Shapes this file is authored around ────────────────────────────────── * @@ -117,9 +109,9 @@ export const DutyHealthDashboard = Dashboard.create({ */ description: 'Governed duties only — role-catalog and manager-assigned work. Self-declared duties are ' - + 'excluded from every number here. Lateness is not shown yet: it depends on each duty\'s ' - + 'grace period, which this view cannot apply — the Team → Late list is the interim answer ' - + 'and it does not account for grace.', + + 'excluded from every number here. On-time is measured against each task\'s own grace ' + + 'period, as it stood when the task was dispatched, and counts completed work only — ' + + 'open work is what the not-moving tiles answer for.', header: { showTitle: true, @@ -188,6 +180,42 @@ export const DutyHealthDashboard = Dashboard.create({ layout: { x: 9, y: 0, w: 3, h: 4 }, }, + /** + * 3b. The on-time rate — the number the product is asked for by name. + * + * `on_time_rate` is `tasks_done_on_time / tasks_done`, both counts over + * `completed_late`, the verdict stamped at completion against the grace in + * force then. It is a RATE OVER COMPLETED WORK; the tile description says + * so, because a rate whose denominator a reader has to guess is a number + * they will guess wrong. + * + * `default` rather than `warning` / `danger`: this is the one number on the + * screen that is good when it is high, and colouring it as an alert would + * read as a problem at 100%. The attention colours stay on the two + * not-moving tiles, which is where action is actually needed. + * + * No `owner` dimension, here or anywhere on this screen: an on-time rate + * split by person is a performance score, and this product does not have + * one. `test/dashboard.test.ts` pins that as a property of the barrel. + */ + { + id: 'on_time_rate', + title: 'On-time rate', + description: + 'Governed tasks completed within their own grace period, as a share of governed tasks ' + + 'completed. Open work is not counted here.', + type: 'metric', + dataset: 'duly_duty_health', + values: ['on_time_rate'], + colorVariant: 'default', + // Directly under the headline tile and the same width as it — the second + // number a manager reads — but SHORTER, and that is a rule rather than a + // taste call: no other number on this screen may out-area the not-moving + // tile, because stagnation is the signal that arrives early enough to act + // on. `test/dashboard.test.ts` pins it. + layout: { x: 0, y: 4, w: 6, h: 3 }, + }, + /** * 4. By unit. Ordered by the unit DIMENSION, never by the count — * `sortBy` names a selected dimension, so the bar order is a property of @@ -230,7 +258,7 @@ export const DutyHealthDashboard = Dashboard.create({ showDataLabels: false, }, options: { sortBy: 'business_unit', sortOrder: 'asc' }, - layout: { x: 0, y: 4, w: 7, h: 6 }, + layout: { x: 0, y: 7, w: 7, h: 6 }, }, /** @@ -268,7 +296,7 @@ export const DutyHealthDashboard = Dashboard.create({ }, // Chronological, and — like the chart above — independent of the count. options: { sortBy: 'due_week', sortOrder: 'asc' }, - layout: { x: 7, y: 4, w: 5, h: 6 }, + layout: { x: 7, y: 7, w: 5, h: 6 }, }, ], }); diff --git a/src/data/demo-history.ts b/src/data/demo-history.ts index c3394c7..792fbfe 100644 --- a/src/data/demo-history.ts +++ b/src/data/demo-history.ts @@ -93,6 +93,11 @@ export const DISPATCH_DUTIES: readonly DispatchDuty[] = DUTIES.map((duty) => { due_anchor: cadence.due_anchor ?? null, due_offset_days: cadence.due_offset_days ?? null, lead_days: cadence.lead_days ?? null, + // The grace the catalog authored, so `late_after` on every seeded row is + // the deadline the real dispatcher would have stamped. Omit it and the + // whole fixture reads as zero grace: the demo's Late list would show tasks + // still inside a 7-day window, which is the exact defect #52 removed. + grace_days: cadence.grace_days ?? null, timezone: timezoneOf(unit), // The same window `duty.seed.ts` writes onto `duly_duty.effective_from`. // Stated HERE too, not only there: the planner clips every period whose @@ -217,6 +222,18 @@ const NOTES: Readonly> = { export interface SeededTask extends Omit { status: 'open' | 'in_progress' | 'done' | 'skipped' | 'cancelled'; completed_at?: string; + /** + * `completed_late` is deliberately NOT on this interface. + * + * The verdict is `completed_at` past `late_after`, and both of those are on + * the row this fixture emits — so `task.hook.ts` stamps it on the way in, + * from the same comparison it makes on a live completion. Computing it here + * as well would be a second spelling of one rule, in a file whose whole + * point is that it re-uses the dispatcher's own arithmetic instead of + * re-deriving it. The drift each row completes with (below) is what decides + * which way the verdict falls; the grace it is judged against comes from the + * catalog, through `late_after`. + */ skip_reason?: string; note?: string; /** Written by a SECOND seed pass — an insert can never carry it. See `task.seed.ts`. */ @@ -332,6 +349,10 @@ const resolveDraft = (draft: TaskDraft, index: number): SeededTask => { const completed = iso( new Date(Math.max(dueInstant.getTime() + drift * DAY + 14 * HOUR, dispatched.getTime())), ); + // The `+1` drift lands a day past the due date, so whether a row is + // stamped late depends on the grace its own duty granted — which is what + // makes the seeded on-time rate a number worth looking at rather than 100% + // by construction. The stamp itself is the hook's, on the way in. return withNote({ ...draft, status: 'done', completed_at: completed, last_update_at: completed }); } diff --git a/src/data/task.seed.ts b/src/data/task.seed.ts index 6ba8751..9e854de 100644 --- a/src/data/task.seed.ts +++ b/src/data/task.seed.ts @@ -86,12 +86,23 @@ export const taskHistorySeed = defineSeed(Task, { period_key: task.period_key, due_date: task.due_date, visible_from: task.visible_from, + // From the planner, like `due_date` and `visible_from` beside it — the + // deadline the real dispatcher would have stamped, with the duty's own + // grace applied. Readonly to callers; carried here on the same + // system-context leg as `completed_at` below. + late_after: task.late_after, status: task.status, // Carried on the INSERT, from the seed loader's system context — the leg // that is exempt from the readonly strip. A caller's identical write is // still refused by `completed_at_required_when_done`, and // `test/seed-history.test.ts` pins both halves. completed_at: task.completed_at, + // `completed_late` is NOT written here. A `done` row inserted this way + // makes no completion transition, so the hook's `beforeUpdate` leg never + // sees it — its `beforeInsert` leg does, and stamps the verdict from the + // two values above. The seed states the facts; the engine draws the + // conclusion, which is the only way the demo and a live completion cannot + // disagree. skip_reason: task.skip_reason, note: task.note, })), diff --git a/src/datasets/duty-health.dataset.ts b/src/datasets/duty-health.dataset.ts index 1a2c722..f191e36 100644 --- a/src/datasets/duty-health.dataset.ts +++ b/src/datasets/duty-health.dataset.ts @@ -11,62 +11,55 @@ import { governed } from './governed.js'; * dimension or measure this file does not declare renders an empty chart and * reports success, so the names below are a contract. `#10` binds them. * - * ── What is NOT here, and why it is upstream rather than worked around ──── + * ── How the on-time measures became expressible ────────────────────────── * - * The card asks for three more measures — `done on time`, `late`, and the - * `on-time rate` derived from them. All three reduce to ONE comparison: + * The card's three measures — `done on time`, `late`, and the `on-time rate` + * over them — all reduce to ONE comparison: * * completed_at <= due_date + duty.grace_days * - * That comparison cannot be expressed by a dataset measure filter, and the - * reason is worth stating precisely, because one third of it DOES work and - * the other two thirds are separate platform gaps: + * A dataset measure filter cannot say that, and still cannot: `FILTER_OPERATORS` + * has no date arithmetic and its `{N_days_ago}` macros are relative to NOW + * rather than to a column, so "+ grace_days" has no spelling; and a + * column-to-column reference (`{ completed_at: { $lte: { $field: 'due_date' } } }`) + * is resolved by the in-memory evaluator and REFUSED by driver-sql with + * `INVALID_FILTER` / 400 (objectstack#5222), which for a dataset is worse than + * a uniform gap — the same measure would answer on one deployment and 400 on + * another. Both halves are objectstack#14104. * - * 1. **Reaching the related field works.** `include: ['duty']` plus a - * `duty.grace_days` path is exactly what the semantic layer is for — joins - * are compiled from `include` (ADR-0071, ≤3 hops) and the author writes no - * ON clause. `frequency` below is that same reach, in production, so this - * half is proven rather than assumed. + * What changed in #52 is not the grammar: it is WHEN the comparison is made. + * Both operands are knowable at a definite instant, so each is resolved there + * and stored on `duly_task` — * - * 2. **Comparing two COLUMNS is refused on the SQL path.** The filter grammar - * declares a field reference — `{ completed_at: { $lte: { $field: - * 'due_date' } } }` — and `@objectstack/spec`'s own `filter.zod.ts` records - * that `driver-sql` (and `driver-sqlite-wasm`, which inherits its compiler) - * reject it with `INVALID_FILTER` / HTTP 400, while the in-memory evaluator - * resolves it. Tracked upstream as objectstack#5222. That split is worse - * than a uniform gap for a DATASET in particular: the same declaration - * would answer on a memory driver and 400 on a SQL one, so the measure's - * correctness would depend on the deployment. + * late_after = due_date + grace_days stamped at DISPATCH + * completed_late = completed_at > late_after stamped at COMPLETION * - * 3. **There is no date arithmetic in the grammar at all.** `FILTER_OPERATORS` - * is closed — equality, ordering, set, range, string, null/exists — and - * nothing adds an interval to a column. The `{N_days_ago}` macro - * vocabulary (`DATE_MACRO_PARAM_RE`) is relative to NOW, never to another - * column, so it cannot express "+ grace_days" either. And here the interval - * is itself a column, which is strictly harder than a literal offset. So - * even if #5222 landed tomorrow, `due_date + duty.grace_days` would still - * have no spelling. + * — and what is left at query time is a count over a boolean, which this + * grammar has always been able to express. **objectstack#14104 stops being a + * blocker rather than being resolved**: with `late_after` on the row there is + * no column-to-column comparison left to make. If it ever lands, nothing here + * needs to change — and the stamps would still be right, because they answer + * "was this late" with the grace that was in force at the time, which a + * query-time comparison against today's `duty.grace_days` cannot do. * - * The two workarounds were both considered and both rejected on the card's own - * terms. Denormalising `grace_days` (or a pre-computed `grace_deadline`) onto - * `duly_task` is a second writer that drifts the day a duty's grace is edited — - * `AGENTS.md` rule 5 forbids it outright. Reducing the rate in TypeScript over - * query results is the hand-written aggregation the metadata-first instruction - * on this card exists to prevent, and it would also put the number outside the - * semantic layer where no dashboard could bind it. + * That is also why this is not the denormalisation `AGENTS.md` rule 5 forbids. + * Rule 5 is about a MAINTAINED flag — one that needs a writer every midnight + * and lies on the day it does not run. These are written once, at the moment + * they become true, and never recomputed; the boundary is stated under rule 5 + * itself. The consequence is deliberate: editing a duty's `grace_days` does not + * move the verdict on work already completed. `duly_catalog_sync` is where a + * replay onto open tasks would belong, and it does not do that today. * - * So the measures are absent rather than approximated. An `on_time_rate` that - * silently ignored grace would be wrong in the direction that matters — it - * would mark late every task completed inside the grace its own duty grants — - * and it would be wrong invisibly, which is how a number nobody trusts becomes - * the number everybody reports. Note what the gap currently costs: `grace_days` - * is authored on `duly_catalog_item`, propagated to `duly_duty` at - * instantiation, and read by NOTHING. This dataset was its only intended - * consumer. + * The rejected alternative has not changed either: reducing the rate in + * TypeScript over query results is the hand-written aggregation the + * metadata-first rule exists to prevent, and it would put the number outside + * the semantic layer where no dashboard could bind it. * - * Filed upstream as **objectstack-ai/objectstack#14104**, with parts 2 and 3 above - * as the two independent halves. Do not close this hole locally: an approximation - * here is exactly how a platform gap becomes permanent and invisible. + * ⛔ What must still never be built here is the GRACE-FREE approximation — + * a `due_date < {today}` window standing in for lateness. It marks late every + * task completed inside the grace its own duty grants, which is wrong in + * exactly the direction a customer configures grace against, and wrong + * invisibly. `test/dashboard.test.ts` and `test/datasets.test.ts` both pin it. * * ── `tasks_due` excludes cancelled, in every dataset that uses the name ─── * A cancelled task was withdrawn: it was never owed, so it is neither load nor @@ -124,5 +117,48 @@ export const DutyHealth = defineDataset({ aggregate: 'count', filter: governed({ status: 'skipped' }), }, + { + /** + * Completed inside the grace its duty granted. `completed_late` is the + * verdict the completion hook stamped against the `late_after` the task + * was dispatched with — so this counts what was on time AT THE TIME, + * which is the only reading an audit accepts. + * + * `status: 'done'` is carried as well as the flag, rather than trusted + * to imply it: the verdict is cleared when a task is reopened, but a + * measure that leaned on that would silently start counting skipped and + * cancelled rows the day the clearing leg changed. + */ + name: 'tasks_done_on_time', + label: 'Done on time', + aggregate: 'count', + filter: governed({ status: 'done', completed_late: false }), + }, + { + // The other half of the same population — never a separate question, and + // never a person's score. `tasks_done_on_time + tasks_completed_late` + // is `tasks_done` exactly, because every done row carries a definite + // verdict (a task with no due date has no deadline to miss and is + // stamped `false`). + name: 'tasks_completed_late', + label: 'Completed late', + aggregate: 'count', + filter: governed({ status: 'done', completed_late: true }), + }, + { + /** + * The product's headline number, at last expressible. + * + * A DERIVED measure (ADR-0021 Q1) — it names other measures and nothing + * else, so the caliber gate it inherits is theirs and cannot drift from + * them. Deliberately over `tasks_done` rather than `tasks_due`: this + * answers "of the work that was completed, how much was on time", and + * folding still-open or skipped work into the denominator would answer a + * different question under the same name. + */ + name: 'on_time_rate', + label: 'On-time rate', + derived: { op: 'ratio', of: ['tasks_done_on_time', 'tasks_done'] }, + }, ], }); diff --git a/src/hooks/task.hook.ts b/src/hooks/task.hook.ts index 4dc7374..910fa11 100644 --- a/src/hooks/task.hook.ts +++ b/src/hooks/task.hook.ts @@ -3,12 +3,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; /** - * `duly_task` lifecycle stamps — the two server-owned timestamps. + * `duly_task` lifecycle stamps — the server-owned columns. * - * Both `completed_at` and `last_update_at` are `readonly: true`, so a caller's - * value is stripped at the API boundary and this hook is their ONE writer. A - * value a `beforeUpdate` hook derives is not a caller write and survives that - * strip. + * `completed_at`, `last_update_at`, `late_after` and `completed_late` are all + * `readonly: true`, so a caller's value is stripped at the API boundary and + * this hook is their ONE writer. A value a `before*` hook derives is not a + * caller write and survives that strip. + * + * ── The two lateness stamps, and why they are stored at all ────────────── + * "Late" is `due_date + duty.grace_days` against a moment, and no filter or + * dataset grammar can say it: no date arithmetic, no column-to-column + * comparison on the SQL path, and here the offset is itself a column on a + * joined object (objectstack#14104). Both halves become knowable at a definite + * instant, so each is resolved at that instant and stored: + * + * `late_after` = `due_date + grace_days` · at DISPATCH, by the planner + * `completed_late` = `completed_at > late_after` · at COMPLETION, here + * + * Neither is a MAINTAINED flag — the shape `AGENTS.md` rule 5 forbids, which + * needs a writer running every midnight and lies on the day it does not run. + * Each is written once, at the moment it becomes true, and is never recomputed: + * the same category as `completed_at` beside them. The boundary is written out + * under rule 5 itself. + * + * **Write-once is the whole design.** Change a duty's `grace_days` and tasks + * already completed keep their verdict, and already-dispatched open tasks keep + * the `late_after` they were born with. A compliance record that rewrites + * itself when configuration changes is worth nothing in front of an auditor. + * If a replay is ever wanted, `duly_catalog_sync` is where it belongs — it + * already exists to push duty edits onto instantiated records — and it + * deliberately does not do this today. + * + * Two consequences of THIS hook holding the pen, both deliberate: + * + * - `late_after` is never touched on an update except to fill a blank. There + * is no leg that recomputes it, so no duty edit and no re-date can move it. + * - The verdict compares CIVIL DAYS in UTC, not instants in the duty's zone. + * Grace is granted in whole days, and the overdue escalation already judges + * lateness on `daysBetween(due_date, today())` — the same UTC day boundary. + * Agreeing with it is the point of this card: one system, one answer. A + * per-zone boundary would need the duty read this handler deliberately does + * not do (see "one self-contained function" below), and it would put the two + * surfaces back into disagreement by up to a day. * * ── Why this hook must stay in the barrel ──────────────────────────────── * Hooks are read from `defineStack({ hooks })` only. A `*.hook.ts` that is not @@ -74,6 +110,46 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; * The single-record path (`dispatch.mode === 'record'`) has a payload of its * own, so the row-conditional stamp is sound there and is unchanged. * + * ── `completed_late` on that path: refuse the LATE direction only ──────── + * The verdict is read off THIS row's `late_after`, so it is the same + * out-of-contract rewrite `completed_at` is — worse, in fact, because two rows + * in one batch can legitimately disagree: tick five tasks together and one of + * them is past its grace, and whichever dispatch runs last decides the + * compliance record for all five. + * + * Refusing every bulk completion would answer that, and it would delete a + * feature this product is built around — a week of ticks in one gesture is the + * difference between a Monday habit and a chore. Writing NOTHING, the answer + * `last_update_at` gets below, is not available either: that one is safe only + * because nothing reads it on this path, and here the on-time rate reads + * exactly these rows. A silent null verdict on the most common completion + * gesture in the product is the defect this card exists to remove. + * + * So the guard is asymmetric, and the asymmetry is what makes the write sound: + * + * - A row that would be stamped LATE refuses the write + * (`DULY_TASK_BULK_LATE_COMPLETION`, 409), naming the task and the day its + * grace ran out. + * - Every row that survives its own guard computes `false`, so the value that + * reaches the shared payload is the one every matched row would have + * written. It is row-INVARIANT by construction, not by luck. + * + * A batch is therefore either wholly on time and stamped correctly, or refused + * — decided from each row alone, in any dispatch order, with no accumulator. + * The cost is stated rather than hidden: a selection that includes late work + * cannot be completed in one gesture, and the caller is told which row and why. + * Bulk SKIP is untouched (`skipped` is not a completion), and so is the + * clearing direction: `completed_late = null` alongside `completed_at = null` + * is correct for every row leaving `done`. + * + * Not extended to the view's `visible` predicate the way the already-done + * guard is. That predicate is a client-side hide, and this condition depends on + * the completion instant against a stored date — a boundary that moves at + * midnight, between the render and the write. A predicate that evaluated it + * differently from the server would hide a working action instead of + * preventing a refused one, and a silently missing bulk action is worse than an + * error that names its cause. + * * ── The same path and `last_update_at`: write nothing, do not refuse ───── * The stagnation stamp is row-conditional too, in the other direction: it * fires when THIS row's `status`, `note` or `skip_reason` differs from THIS @@ -131,9 +207,50 @@ const stampTaskLifecycle = (ctx: HookContext): void => { const input = ctx.input as Record; const now = new Date().toISOString(); + // The CIVIL DAY an instant falls on. `late_after` is a calendar date and + // `completed_at` is an instant, so the verdict has to be day-against-day: + // grace is granted in whole days, and a task completed at 14:00 on the last + // day of its grace is inside it. Both spellings are ISO, so a lexical `>` is + // chronological. Accepts a `Date` because a caller's payload may carry one. + const dayOf = (value: unknown): string => { + if (value instanceof Date) return value.toISOString().slice(0, 10); + return typeof value === 'string' ? value.slice(0, 10) : ''; + }; + + // The verdict. `false` when there is no deadline to miss — see the header: + // a definite "not late" rather than a missing answer, so `done` always + // splits into on-time + late. + const completedLate = (completedAt: unknown, lateAfter: unknown): boolean => { + const completedDay = dayOf(completedAt); + const deadline = dayOf(lateAfter); + if (completedDay === '' || deadline === '') return false; + return completedDay > deadline; + }; + if (ctx.event === 'beforeInsert') { // A brand-new task has just been touched, by definition. input.last_update_at = now; + + // ── late_after — the zero-grace fallback, for the paths with no duty ── + // The dispatcher stamps this itself, with the duty's own grace + // (`dispatch.plan.ts`). Every other producer has no duty to read: the + // assignment fan-out creates tasks with `duty` unset, and a member + // hand-creating their own task has none either. Zero grace is what "no + // duty governs this row" already means to the overdue escalation, so the + // two surfaces agree rather than one of them silently never firing. + // + // A task with no `due_date` keeps a blank stamp: there is no deadline to + // derive, and a task with no due date genuinely cannot be late. + const dueDay = dayOf(input.due_date); + if (dueDay !== '' && dayOf(input.late_after) === '') input.late_after = dueDay; + + // A row inserted ALREADY done — a seeded history, an import — carries its + // verdict from the same beat, because both halves are on the payload. The + // dispatch contract has no predicate INSERT (it covers update and delete + // only), so this is a payload of one row and there is no batch to leak on. + if (input.status === 'done') { + input.completed_late = completedLate(input.completed_at, input.late_after); + } return; } @@ -173,13 +290,58 @@ const stampTaskLifecycle = (ctx: HookContext): void => { ); } + // ── late_after — FILLED once if it is blank, never rewritten ─────────── + // Write-once is the design (see the header), so this leg only ever turns a + // blank into a value: a task that acquires a due date after it was created + // would otherwise be one that can never be late anywhere. A row that already + // carries a stamp keeps it, whatever happens to its due date afterwards. + // + // Not on the shared-payload path. The CONDITION here is row-conditional — + // "this row has no stamp" — so on a batch the fill computed for a blank row + // would land on every matched row and overwrite a stamp another row was born + // with. Nothing is written instead: an unstamped row stays unstamped, which + // is where it already was, rather than a stamped row being corrupted. An + // administrative bulk re-date is left alone, as it is for `last_update_at`. + if (ctx.dispatch?.mode !== 'per-row' && dayOf(previous.late_after) === '') { + const dueDay = dayOf('due_date' in input ? input.due_date : previous.due_date); + if (dueDay !== '') input.late_after = dueDay; + } + + const lateAfter = 'late_after' in input ? input.late_after : previous.late_after; + if (!wasDone && isDone) { + // ── The on-time verdict, and the second shared-payload guard ───────── + // The verdict is read off THIS row's own stamp, so it is exactly the + // rewrite D3 puts outside the contract: one row's answer would be written + // to every matched row. The sanctioned route is to REFUSE — and refusing + // only the LATE direction is what keeps bulk complete working: every row + // that survives its own guard computes `false`, so the value that reaches + // the shared payload is the same one every row in the batch would have + // written. A batch is either all on time, or it is refused. + const late = completedLate(now, lateAfter); + if (ctx.dispatch?.mode === 'per-row' && late) { + throw Object.assign( + new Error( + `Task ${String(input.id ?? previous.id ?? '')} is being completed after ${String(lateAfter ?? '')}, ` + + 'the day its grace ran out. A bulk status write carries one payload for every matched row, so ' + + 'this task\'s on-time verdict would be recorded against the whole batch. Complete a late task ' + + 'on its own row.', + ), + { code: 'DULY_TASK_BULK_LATE_COMPLETION', status: 409 }, + ); + } input.completed_at = now; + input.completed_late = late; } else if (wasDone && !isDone) { // Reopened, skipped or cancelled — the completion is undone, so the // timestamp goes with it, or the record keeps a completion that no longer - // happened. + // happened. The verdict goes with it for the same reason: a completion + // that did not happen has nothing to be late about. Both values are + // row-invariant — `null` is correct for every row being moved out of done, + // including one that was never completed — so this direction stays + // allowed on the shared-payload path. input.completed_at = null; + input.completed_late = null; } // ── last_update_at — only when a human moved the work ─────────────────── @@ -236,12 +398,15 @@ export const TaskLifecycleHook: Hook = { object: 'duly_task', events: ['beforeInsert', 'beforeUpdate'], description: - 'Server-owned timestamps on duly_task: completed_at on the transition into and out of ' - + 'done, and last_update_at only when status, note or skip_reason actually changed — ' - + 'never on an administrative write, which would reset the stagnation signal. On a ' - + 'predicate (bulk) write both row-conditional stamps are handled by ADR-0058 ' - + 'Addendum II D3: one payload for the whole batch, so a re-stamp of an already-done ' - + 'row is refused outright and last_update_at is not stamped at all.', + 'Server-owned columns on duly_task: completed_at and the completed_late verdict on the ' + + 'transition into and out of done, late_after filled at insert for the paths the ' + + 'dispatcher does not stamp, and last_update_at only when status, note or skip_reason ' + + 'actually changed — never on an administrative write, which would reset the stagnation ' + + 'signal. Both lateness stamps are write-once: a later change to the duty\'s grace never ' + + 'moves them. On a predicate (bulk) write the row-conditional stamps are handled by ' + + 'ADR-0058 Addendum II D3: one payload for the whole batch, so a re-stamp of an ' + + 'already-done row and a completion that would be stamped late are both refused, and ' + + 'last_update_at is not stamped at all.', // Explicit because it is load-bearing rather than a default worth inheriting: // if this handler throws, the write MUST be refused. Committing a task whose // stamps were not applied is the exact silent corruption the diff --git a/src/jobs/dispatch.plan.ts b/src/jobs/dispatch.plan.ts index de31712..bd31266 100644 --- a/src/jobs/dispatch.plan.ts +++ b/src/jobs/dispatch.plan.ts @@ -63,6 +63,20 @@ export const DEFAULT_TIMEZONE = 'UTC'; export const DEFAULT_DUE_ANCHOR: DueAnchor = 'period_start'; export const DEFAULT_DUE_OFFSET_DAYS = 0; export const DEFAULT_LEAD_DAYS = 7; +/** + * Zero grace, and it is the DECLARED default like the others — `duly_duty` + * writes `record.form == "standing" ? null : 0`. + * + * It is also the reading the rest of the product already gives an absent + * grace: the overdue escalation's CEL gate is + * `has(grace_days) && !isBlank(grace_days) ? grace_days : 0` + * (`src/flows/reminders.flow.ts`), so a task whose duty grants no grace + * escalates the day after its due date. `late_after` agreeing with that is what + * keeps the two surfaces from giving one person two answers — which is the + * whole defect this stamp closes. A null grace producing a null `late_after` + * would instead produce a task that can never be late anywhere. + */ +export const DEFAULT_GRACE_DAYS = 0; /** The status a duty must hold to dispatch. */ export const DISPATCHABLE_STATUS = 'active'; @@ -90,6 +104,10 @@ export const DISPATCH_DUTY_FIELDS = [ 'due_anchor', 'due_offset_days', 'lead_days', + // Read for `late_after` only. The planner does not otherwise care about + // grace: a task is dispatched the same way whether its duty grants 0 days + // or 30. + 'grace_days', 'timezone', 'effective_from', 'effective_to', @@ -113,6 +131,7 @@ export interface DispatchDuty { due_anchor?: string | null; due_offset_days?: number | null; lead_days?: number | null; + grace_days?: number | null; timezone?: string | null; effective_from?: string | null; effective_to?: string | null; @@ -131,6 +150,14 @@ export interface TaskDraft { period_key: string; due_date: string; visible_from: string; + /** + * `due_date + duty.grace_days`, resolved at dispatch and carried on the row. + * + * The duty's grace is knowable exactly once — now — and a task is a record of + * what was owed WHEN IT WAS OWED. Editing the duty afterwards does not reach + * back through this column; `duly_catalog_sync` is where a replay would live. + */ + late_after: string; status: 'open'; } @@ -329,6 +356,7 @@ function planForDuty(duty: DispatchDuty, now: Date, window: BackfillWindow | nul const dueAnchor = (duty.due_anchor ?? DEFAULT_DUE_ANCHOR) as DueAnchor; const dueOffsetDays = duty.due_offset_days ?? DEFAULT_DUE_OFFSET_DAYS; const leadDays = duty.lead_days ?? DEFAULT_LEAD_DAYS; + const graceDays = duty.grace_days ?? DEFAULT_GRACE_DAYS; try { // "Today" is resolved in the DUTY's zone, not the server's. A single UTC @@ -383,6 +411,10 @@ function planForDuty(duty: DispatchDuty, now: Date, window: BackfillWindow | nul period_key: periodKey, due_date: dueDate, visible_from: visibleFrom, + // Civil-date arithmetic through the period engine, like `visible_from` + // beside it — a second `addDays` is a second thing that can be wrong + // about February. + late_after: addCalendarDays(dueDate, graceDays), status: 'open', }); } diff --git a/src/objects/task.object.ts b/src/objects/task.object.ts index 73a5cb6..2d80d78 100644 --- a/src/objects/task.object.ts +++ b/src/objects/task.object.ts @@ -13,10 +13,17 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; * plain idempotent job instead of a distributed lock. * * ── What is deliberately NOT a field here ──────────────────────────────── - * `is_late` / `is_open` / `is_overdue`: derivable from `due_date`, `grace_days` - * and `status`, which are stored and indexed. A stored copy is a second writer - * that drifts, and a formula field is virtual — a filter naming one silently - * matches nothing. Consumers ask `status` and `due_date` directly. + * `is_late` / `is_open` / `is_overdue`: a MAINTAINED flag, whose truth changes + * with the clock rather than with the record. It needs a writer that runs every + * midnight, and the day it does not run the flag lies without erroring; a + * formula field is virtual instead, so a filter naming one silently matches + * nothing. Consumers ask `status` and `due_date` directly. + * + * `late_after` and `completed_late` below are NOT that shape, and the boundary + * is written out under `AGENTS.md` rule 5. Each is stamped ONCE, at the moment + * it becomes knowable — dispatch and completion — and is never recomputed, so + * no writer has to keep running for them to stay true. They are the same + * category as `completed_at` and `visible_from`, which sit beside them. * * `progress_percent`: a number nobody can verify, which becomes the number * everyone reports on. Progress lives in `status` and in `last_update_at`. @@ -99,6 +106,47 @@ export const Task = ObjectSchema.create({ description: 'due_date minus the duty lead time. Before this the task exists but stays out of the way.', }), + /** + * `due_date + duty.grace_days`, stamped ONCE at dispatch. The last day a + * task may still be open, or be completed, without being late. + * + * ── Why it is stored and not asked ─────────────────────────────────── + * "Late" is `due_date + duty.grace_days` compared against a moment, and no + * filter grammar can say it: `FILTER_OPERATORS` has no date arithmetic, the + * `{N_days_ago}` macros are relative to now and never to a column, and here + * the offset is itself a column on a JOINED object (objectstack#14104). The + * offset is knowable at dispatch, so it is applied at dispatch and what + * lands on the row is a plain date. Every surface that asks about lateness + * — the `late` view, the on-time measures — is then an ordinary date filter + * with nothing to compute at read time. + * + * ── Write-once, and what that costs ────────────────────────────────── + * The stamp carries the grace the duty granted AT DISPATCH. Change a duty's + * `grace_days` afterwards and already-dispatched tasks keep the deadline + * they were born with — deliberately, for the same reason `subject` is + * copied rather than joined: a task is a record of what was owed, and a + * compliance record that rewrites itself when configuration changes is + * worth nothing in front of an auditor. + * + * The cost is real and belongs to somebody: an admin who has just corrected + * a misconfigured grace will expect it to apply to open work. The path for + * that is `duly_catalog_sync`, which already exists to replay duty edits + * onto instantiated records; it deliberately does not do this yet. + * + * Blank only when the task has no `due_date` at all — nothing to be late + * against, so it never appears in a lateness surface. Every task that HAS a + * due date gets one: the planner stamps it with the duty's grace, and + * `task.hook.ts` stamps `late_after = due_date` (zero grace) for the paths + * that have no duty to read — the assignment fan-out and a hand-created + * task. That is the same reading of "no duty governs this row" the overdue + * escalation already uses (`src/flows/reminders.flow.ts`). + */ + late_after: Field.date({ + label: 'Late after', + readonly: true, + description: 'The due date plus the grace the duty granted when this task was dispatched. Open past this day, or completed after it, is late. Stamped once, at dispatch — editing the duty\'s grace afterwards does not move it.', + }), + status: Field.select({ label: 'Status', required: true, @@ -128,6 +176,30 @@ export const Task = ObjectSchema.create({ readonly: true, }), + /** + * The verdict: was this completion late? `completed_at` past `late_after`. + * + * Written ONCE, by `task.hook.ts`, in the same beat as `completed_at` — + * which is the first moment both halves of the comparison exist. It is a + * historical fact from then on, exactly like the timestamp beside it, and + * nothing recomputes it. That is what makes the on-time rate a count over a + * boolean instead of the column-to-column comparison the query grammar + * cannot express. + * + * Cleared with `completed_at` when a task is reopened: a completion that no + * longer happened has no verdict. + * + * `false` when the task carries no `late_after` — a task with no due date + * has no deadline to miss, so "not late" is the answer, not a missing one. + * Keeping it a definite answer is what makes `done = on time + late` an + * identity a dashboard reader can rely on. + */ + completed_late: Field.boolean({ + label: 'Completed late', + readonly: true, + description: 'True when the task was completed after its late-after date. Stamped once, at completion, against the grace in force then — a later change to the duty\'s grace never moves it.', + }), + /** * The stagnation signal, and the most useful number in the product. * @@ -163,6 +235,9 @@ export const Task = ObjectSchema.create({ { fields: ['owner', 'status'] }, { fields: ['business_unit', 'due_date'] }, { fields: ['due_date'] }, + // The `late` lens filters on this column and sorts by it, the same shape + // `due_date` above is indexed for. + { fields: ['late_after'] }, { fields: ['last_update_at'] }, { fields: ['assignment'] }, ], diff --git a/src/views/task.view.ts b/src/views/task.view.ts index 7a9a06d..c0f2efd 100644 --- a/src/views/task.view.ts +++ b/src/views/task.view.ts @@ -44,6 +44,21 @@ const columns = [ * route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a * batch-scoped payload. * + * ── A second refusal on the same route: a LATE completion ──────────────── + * Since #52 a completion also stamps `completed_late`, and that verdict is read + * off the row's own `late_after` — so one batch cannot carry two answers. The + * hook refuses a bulk completion whose row would be stamped late + * (`DULY_TASK_BULK_LATE_COMPLETION`, 409) and stamps `false` otherwise, which + * every row that survived its own guard would have written. Bulk complete keeps + * working for on-time work; a selection containing late work is refused, by + * name, and those rows are ticked individually. + * + * That refusal deliberately has NO predicate half here. This one turns on the + * completion instant against a stored date — a boundary that moves at midnight, + * between the render and the write — so a client-side copy would sooner or + * later hide an action the server would have accepted. A missing bulk action + * with no explanation is worse than a refusal that names its cause. + * * This predicate is kept because it is still the right UX: it stops the * console from assembling a batch the server would refuse, so a user gets an * unavailable action rather than an error they did not cause. @@ -126,20 +141,56 @@ export const TaskViews = defineView({ sort: [{ field: 'due_date', order: 'asc' }], }, - // Late = past due and still open. Read from stored columns, never from a - // flag: a stored `is_late` needs a writer that runs every midnight, and the - // day it does not run the view lies without erroring. + /** + * Late = past the GRACE the duty granted, and still open. + * + * `late_after` is `due_date + duty.grace_days`, stamped on the row at + * dispatch (`src/jobs/dispatch.plan.ts`), so the filter here is an ordinary + * date comparison against a stored, indexed column — no column-to-column + * comparison, no date arithmetic, nothing this grammar cannot say. + * + * It reads a stored column, and that is still not a stored FLAG. An + * `is_late` boolean would need a writer running every midnight and would + * lie on the day it did not run; `late_after` is a date the row was born + * with, and the clock moving past it is what makes the row late. The + * comparison still happens at read time — it just has both operands now. + * + * ── What changed, and why the old comment is gone ──────────────────── + * This view used to filter `due_date < {today}` and say, in its own + * comment, that reading stored columns was the point — which was right — + * without mentioning that it was also dropping grace. A duty granting 7 + * days had its people listed as late the morning after the due date, six + * days early, while the overdue escalation (which DOES read `grace_days`) + * correctly stayed silent: one system, two answers about the same person on + * the same day. That was #48, and both surfaces now answer from the same + * stamp. + * + * ── The write-once consequence, named so the next reader finds it ──── + * `late_after` carries the grace in force AT DISPATCH. Edit a duty's + * `grace_days` and tasks already dispatched keep the deadline they were + * born with, so this list does not move for them. That is deliberate — a + * task records what was owed when it was owed — and the place a replay + * belongs is `duly_catalog_sync`, which already exists to push duty edits + * onto instantiated records. It does not do this today, and this card did + * not build it. + * + * A task with no `due_date` has no `late_after` and never appears here. + * That is the honest answer: nothing was owed by any particular day. + */ late: { label: 'Late', type: 'grid', data, - columns, + // `late_after` is carried as a column, not just filtered on: the whole + // complaint behind #48 was a screen that would not show you why it + // thought a task was late. + columns: [...columns, { field: 'late_after' }], filter: [ - { field: 'due_date', operator: 'less_than', value: '{today}' }, + { field: 'late_after', operator: 'less_than', value: '{today}' }, { field: 'status', operator: 'in', value: ['open', 'in_progress'] }, ], bulkActionDefs: bulkActions, - sort: [{ field: 'due_date', order: 'asc' }], + sort: [{ field: 'late_after', order: 'asc' }], }, // Stagnation: open, and untouched for a fortnight. This is the earliest diff --git a/test/dashboard.test.ts b/test/dashboard.test.ts index c839b95..f4c0310 100644 --- a/test/dashboard.test.ts +++ b/test/dashboard.test.ts @@ -458,18 +458,67 @@ describe('no ranking of people', () => { }); }); -describe('the numbers that must stay absent until #52 decides what "late" means', () => { - it('no widget binds an on-time or lateness measure', () => { - const offenders = allWidgets.flatMap((entry) => - (entry.widget.values ?? []) - .filter((name) => /on_?time|late|overdue/i.test(name)) - .map((name) => `${site(entry)} binds '${name}'`), +describe('lateness is on the screen, and only in the one shape that respects grace', () => { + /** + * #52 decided what "late" means: `late_after` (`due_date + grace_days`) is + * stamped at dispatch and `completed_late` at completion, both write-once, so + * every lateness number is a count over a stamp rather than a comparison the + * query grammar cannot make. + * + * The assertion that used to live here — "no widget binds a lateness measure" + * — was the right rule while the only expressible version ignored grace. What + * replaces it is not weaker: any lateness number on this screen must trace + * back to those stamps, and the grace-free window remains banned outright by + * the test below it, which is the one that would actually be reintroduced by + * accident. + */ + it('every lateness number a widget binds is built on the stamps, not on a due-date window', () => { + const measuresByDataset = new Map( + dulyDatasets.map((ds) => [ds.name, new Map(ds.measures.map((m) => [m.name, m]))]), ); - expect( - offenders, - 'no dataset can express `completed_at <= due_date + duty.grace_days` (objectstack#14104), so ' - + 'a measure with one of these names is an approximation that ignores grace — see #52', - ).toEqual([]); + + /** The measure text, following a derived measure through to its operands. */ + const sourcesOf = (dataset: string, name: string, seen = new Set()): string[] => { + if (seen.has(name)) return []; + seen.add(name); + const measure = measuresByDataset.get(dataset)?.get(name); + if (!measure) return []; + const derived = (measure as { derived?: { of: string[] } }).derived; + if (derived) return derived.of.flatMap((operand) => sourcesOf(dataset, operand, seen)); + return [JSON.stringify(measure)]; + }; + + let checked = 0; + for (const entry of allWidgets) { + for (const name of entry.widget.values ?? []) { + if (!/on_?time|late|overdue/i.test(name)) continue; + checked += 1; + const sources = sourcesOf(String(entry.widget.dataset ?? ''), name); + expect(sources.length, `${site(entry)} binds '${name}', which resolves to no measure`) + .toBeGreaterThan(0); + // At least one operand must READ a stamp. Not every one: a rate's + // denominator is an honest plain count (`tasks_done`), and demanding + // the stamp there would only teach the next author to bolt a redundant + // condition onto it. + expect( + sources.some((source) => source.includes('completed_late') || source.includes('late_after')), + `${site(entry)} binds '${name}', which is computed without the lateness stamps — a ` + + 'lateness number that does not read them is one that ignores grace', + ).toBe(true); + // And none of them may reach for a due-date window instead. This is the + // MEASURE-side half of the widget-filter rule below: the grace-free + // approximation is two lines wherever it is written, and a measure is + // the place a reviewer is least likely to look for it. + for (const source of sources) { + expect( + source.includes('due_date'), + `${site(entry)} binds '${name}', whose measures read due_date — lateness is the stamp, ` + + 'and a due-date window is the grace-free approximation under another name', + ).toBe(false); + } + } + } + expect(checked, 'the on-time number is gone from the screen entirely').toBeGreaterThan(0); }); it('no widget rebuilds a grace-free lateness out of a due-date window', () => { @@ -504,6 +553,25 @@ describe('the numbers that must stay absent until #52 decides what "late" means' } }); + it('says what the on-time rate is a rate OF, so nobody has to guess the denominator', () => { + // The rate counts COMPLETED work, not everything that was owed. A reader + // who assumes the denominator is `tasks_due` reads a materially different + // number off the same tile, so the screen says which it is. + for (const dashboard of dashboards) { + const binds = (dashboard.widgets ?? []).some((w) => + (w.values ?? []).some((name) => /on_?time/i.test(name)), + ); + if (!binds) continue; + const text = `${dashboard.description ?? ''} ${(dashboard.widgets ?? []) + .map((w) => `${w.title ?? ''} ${w.description ?? ''}`) + .join(' ')}`.toLowerCase(); + expect(text, `${dashboard.name} shows an on-time rate without saying what it is a rate of`) + .toContain('completed'); + expect(text, `${dashboard.name} does not say the rate respects each duty's grace`) + .toContain('grace'); + } + }); + it('explains the absence on the screen, for as long as the number is missing', () => { // An unexplained absence is a wrong number with no digits: a manager // reading "not moving: 3" on a screen silent about lateness concludes diff --git a/test/datasets.test.ts b/test/datasets.test.ts index 7343893..6e1e2eb 100644 --- a/test/datasets.test.ts +++ b/test/datasets.test.ts @@ -88,8 +88,38 @@ describe('dataset protocol', () => { describe('caliber — self-declared work is surfaced, never scored', () => { it('EVERY measure is filtered to source IN (catalog, assigned)', () => { + /** + * A DERIVED measure (ADR-0021 Q1) carries no filter of its own — it names + * other measures and nothing else — so the gate reaches it through its + * operands instead of exempting it. Both halves matter: an operand that + * does not exist would be an empty chart reporting success, and an operand + * outside this dataset would be a gate this walk cannot see. `on_time_rate` + * is governed exactly because `tasks_done_on_time` and `tasks_done` are. + */ expect(allMeasures.length).toBeGreaterThan(0); for (const { dataset, measure } of allMeasures) { + const derived = (measure as { derived?: { op: string; of: string[] } }).derived; + if (derived) { + const siblings = new Set( + dulyDatasets.find((ds) => ds.name === dataset)!.measures.map((m) => m.name), + ); + expect(derived.of.length, `${dataset}.${measure.name} derives from nothing`).toBeGreaterThan(0); + for (const operand of derived.of) { + expect( + siblings.has(operand), + `${dataset}.${measure.name} derives from '${operand}', which this dataset does not ` + + 'declare — a widget binding it renders empty and reports success', + ).toBe(true); + const source = dulyDatasets + .find((ds) => ds.name === dataset)! + .measures.find((m) => m.name === operand)!; + expect( + (source.filter as Record | undefined)?.source, + `${dataset}.${measure.name} inherits its caliber from '${operand}', which is ungoverned`, + ).toEqual({ $in: [...GOVERNED_SOURCES] }); + } + continue; + } const filter = measure.filter as Record | undefined; expect(filter, `${dataset}.${measure.name} has no filter at all`).toBeDefined(); expect( @@ -182,7 +212,15 @@ describe('the absences that are the product', () => { } }); - it('lateness is never stored — no measure reads a derived flag', () => { + it('lateness is never stored — no measure reads a MAINTAINED flag', () => { + /** + * The banned shape is a flag whose truth changes with the clock: it needs a + * writer every midnight and lies on the day it does not run. `completed_late` + * is deliberately not one of these and is deliberately not in the list — it + * is written once, at completion, and never recomputed (`AGENTS.md` rule 5 + * carries the boundary). The difference is not the name: it is whether a + * second write ever has to happen. + */ for (const { dataset, measure } of allMeasures) { const text = deepText(measure).join(' '); for (const flag of ['is_late', 'is_overdue', 'is_open', 'is_completed']) { diff --git a/test/dispatch.test.ts b/test/dispatch.test.ts index 95d66d6..a6622f5 100644 --- a/test/dispatch.test.ts +++ b/test/dispatch.test.ts @@ -20,6 +20,7 @@ import { type DispatchEngine, } from '../src/jobs/dispatch.job.js'; import { + DEFAULT_GRACE_DAYS, DEFAULT_TIMEZONE, DISPATCH_DUTY_FIELDS, nextDispatchedPeriod, @@ -152,6 +153,11 @@ describe('the duty projection covers every field the planner reads', () => { 'due_anchor', 'due_offset_days', 'lead_days', + // Read for `late_after`. Omitted, it comes back undefined — which reads + // as "this duty grants no grace", so every task would be dispatched with + // its deadline on the due date and the Late list would be the grace-free + // one #48 was filed against, with nothing erroring. + 'grace_days', 'timezone', 'effective_from', 'effective_to', @@ -355,6 +361,72 @@ describe('the copied fields', () => { // that wrote either would be a second writer on the stagnation clock. expect(Object.keys(draft ?? {})).not.toContain('completed_at'); expect(Object.keys(draft ?? {})).not.toContain('last_update_at'); + // `late_after` is the exception, and it is not a second writer: it is + // knowable ONLY here, from a duty field the task does not carry, and it is + // never written again (#52). + expect(Object.keys(draft ?? {})).toContain('late_after'); + }); +}); + +/** + * `late_after` — the lateness deadline, resolved once, at dispatch (#52). + * + * The planner is where the duty's `grace_days` is in hand, and it is the only + * place it is in hand: `duly_task` carries no duty grace of its own, and the + * task's own filter grammar cannot add an interval held in a joined column + * (objectstack#14104). So the stamp is what turns "late" into a plain date + * comparison everywhere downstream. + */ +describe('late_after — due date plus the grace the duty granted', () => { + const now = new Date('2026-08-15T09:00:00Z'); + const only = (over: Partial) => planDispatch({ duties: [duty(over)], now }).drafts[0]; + + it('adds the duty grace to the due date', () => { + const draft = only({ grace_days: 7 }); + expect(draft?.due_date).toBe('2026-08-05'); + expect(draft?.late_after).toBe('2026-08-12'); + }); + + it('a duty granting no grace is late the day after the due date, not on it', () => { + // `late_after` IS the last day still inside the window, so a zero-grace + // duty stamps the due date itself — the `late` view then asks + // `late_after < today`, which fires the following morning. That is the same + // day-one the overdue escalation fires on (`due_date + grace + 1`), which + // is the disagreement #52 existed to end. + expect(only({ grace_days: 0 })?.late_after).toBe('2026-08-05'); + }); + + it('an ABSENT grace reads as zero, never as "no deadline"', () => { + // The trap: a null grace producing a null `late_after` would produce a task + // that can never be late on any surface — silently, and only for the duties + // whose grace nobody filled in. Zero is also how the overdue escalation + // already reads an absent grace. + expect(only({ grace_days: null })?.late_after).toBe('2026-08-05'); + expect(only({ grace_days: undefined })?.late_after).toBe('2026-08-05'); + expect(DEFAULT_GRACE_DAYS).toBe(0); + }); + + it('crosses a month end by the calendar, not by 24-hour arithmetic', () => { + // Through the period engine's own civil-date shift, like `visible_from`. + const draft = planDispatch({ + duties: [duty({ due_anchor: 'period_end', due_offset_days: 0, grace_days: 5 })], + now: new Date('2026-08-15T09:00:00Z'), + }).drafts[0]; + expect(draft?.due_date).toBe('2026-08-31'); + expect(draft?.late_after).toBe('2026-09-05'); + }); + + it('the grace the DUTY held is the one stamped — nothing reads it again later', () => { + // Two duties, two graces, one run: the deadline travels on the row from + // here, so a later edit to either duty cannot reach the tasks it produced. + const drafts = planDispatch({ + duties: [duty({ id: 'strict', grace_days: 0 }), duty({ id: 'lenient', grace_days: 14 })], + now, + }).drafts; + expect(drafts.map((d) => `${d.duty}:${d.late_after}`)).toEqual([ + 'strict:2026-08-05', + 'lenient:2026-08-19', + ]); }); }); diff --git a/test/task-hook.test.ts b/test/task-hook.test.ts index e7e7552..f5c6f8f 100644 --- a/test/task-hook.test.ts +++ b/test/task-hook.test.ts @@ -5,6 +5,7 @@ import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/run import stack from '../objectstack.config.js'; import { dulyHooks } from '../src/hooks/index.js'; +import { planDispatch } from '../src/jobs/dispatch.plan.js'; /** * `duly_task` lifecycle stamps. @@ -635,3 +636,449 @@ describe('last_update_at on a predicate write — one payload, N rows', () => { expect(edited.last_update_at as string > before, 'the by-id path is unchanged').toBe(true); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// The lateness stamps (#52) +// ───────────────────────────────────────────────────────────────────────── + +/** A civil date `days` from today, in UTC — the boundary the stamps use. */ +const dayFromToday = (days: number): string => + new Date(Date.now() + days * 86_400_000).toISOString().slice(0, 10); + +describe('late_after — the deadline the row is born with', () => { + it('is filled from the due date when nothing supplies one — zero grace, not "no deadline"', async () => { + // The dispatcher stamps this itself, with the duty's grace. Every other + // producer — the assignment fan-out, a member creating their own task — + // has no duty to read, and zero grace is what "no duty governs this row" + // already means to the overdue escalation. A blank here would be a task + // that can never be late on any surface. + const task = await newTask({ due_date: '2026-05-04' }); + expect(task.late_after).toBe('2026-05-04'); + }); + + it('keeps the deadline the dispatcher stamped, grace and all', async () => { + // The planner's value must survive the insert leg untouched, or every task + // in the system would silently be judged at zero grace. + const task = await newTask({ due_date: '2026-05-04', late_after: '2026-05-11' }); + expect(task.late_after).toBe('2026-05-11'); + }); + + it('a task with no due date has no deadline, and no lateness filter can match it', async () => { + // The honest answer rather than a convenient one: nothing was owed by any + // particular day. The `late` view is `late_after < {today}`, so this row + // never appears there — asserted through a real query rather than by + // reading the column, because "a blank does not match a date filter" is the + // claim the view's comment actually makes. + const task = await newTask({ subject: 'no due date at all' }); + expect(task.late_after ?? null).toBeNull(); + + const late = await data.find('duly_task', { + where: { late_after: { $lt: dayFromToday(3650) }, id: task.id }, + }); + expect(late, 'a task with no deadline must not surface in the Late lens').toEqual([]); + }); + + it('fills a blank deadline if a due date arrives later, and only then', async () => { + const task = await newTask({ subject: 'due date added afterwards' }); + expect(task.late_after ?? null).toBeNull(); + + const dated = await data.update('duly_task', { id: task.id, due_date: '2026-06-30' }); + expect(dated.late_after, 'a dated task must become answerable to the Late lens').toBe('2026-06-30'); + }); + + it('a re-date NEVER moves a deadline the row already carries', async () => { + // Write-once, in its smallest form: `late_after` is what the row was born + // with. Anything else and a task's compliance deadline could be moved by + // an ordinary edit, with no trace. + const task = await newTask({ due_date: '2026-05-04', late_after: '2026-05-11' }); + await data.update('duly_task', { id: task.id, due_date: '2026-09-30' }); + expect((await read(task.id)).late_after).toBe('2026-05-11'); + }); + + it('is not writable by a caller', async () => { + const task = await newTask({ due_date: '2026-05-04', late_after: '2026-05-11' }); + await data.update('duly_task', { id: task.id, late_after: '2099-01-01' }); + expect((await read(task.id)).late_after, 'readonly, and the hook is its only writer') + .toBe('2026-05-11'); + }); +}); + +describe('WRITE-ONCE — editing a duty\'s grace never rewrites history', () => { + /** + * The assertion this whole card turns on. + * + * An admin who widens a duty's grace from 3 days to 14 is correcting a + * configuration. They are NOT re-adjudicating last quarter's compliance + * record — and a system that let them do it silently would be a system whose + * on-time rate changes when nobody completed anything. The stamps are the + * mechanism: `late_after` is resolved at dispatch and `completed_late` at + * completion, and no leg of this hook recomputes either. + * + * The duty here is a real record, edited through the engine, and the task is + * produced by the real planner from that duty's own fields — so this walks + * the actual path rather than asserting on hand-made values. + */ + const GRACE_AT_DISPATCH = 3; + + const dispatchedTaskFor = async (dutyId: string, grace: number | null) => { + const duty = await data.findOne('duly_duty', { where: { id: dutyId } }); + const [draft] = planDispatch({ + duties: [{ + id: duty.id, + name: duty.name, + form: 'recurring', + status: 'active', + owner: 'user_alice', + business_unit: null, + source: 'catalog', + frequency: 'monthly', + due_anchor: 'period_start', + due_offset_days: 4, + lead_days: 0, + grace_days: grace, + timezone: 'UTC', + }], + now: new Date('2026-08-15T09:00:00Z'), + window: null, + }).drafts; + return { draft, duty }; + }; + + const newDuty = async (subject: string, grace: number) => + data.insert('duly_duty', { + name: subject, + owner: 'user_alice', + form: 'recurring', + status: 'active', + source: 'catalog', + frequency: 'monthly', + due_anchor: 'period_start', + due_offset_days: 4, + lead_days: 0, + grace_days: grace, + timezone: 'UTC', + }); + + it('an open task keeps the deadline it was dispatched with', async () => { + const duty = await newDuty('Grace widened after dispatch', GRACE_AT_DISPATCH); + const { draft } = await dispatchedTaskFor(duty.id, GRACE_AT_DISPATCH); + expect(draft!.late_after).toBe('2026-08-08'); + + const task = await data.insert('duly_task', { ...draft, subject: draft!.subject, owner: 'user_alice' }); + + // The correction an admin makes on Monday morning. + const widened = await data.update('duly_duty', { id: duty.id, grace_days: 21 }); + expect(widened.grace_days).toBe(21); + + expect( + (await read(task.id)).late_after, + 'an already-dispatched task must keep the deadline it was born with — duly_catalog_sync is ' + + 'where a replay would belong, and it does not do this today', + ).toBe('2026-08-08'); + }); + + it('a completed task keeps its verdict — the on-time rate does not move when nobody completed anything', async () => { + const duty = await newDuty('Grace widened after completion', GRACE_AT_DISPATCH); + const { draft } = await dispatchedTaskFor(duty.id, GRACE_AT_DISPATCH); + + // Dispatched with a deadline that is already past, so completing it NOW is + // late under the grace that was in force. + const task = await data.insert('duly_task', { + ...draft, + subject: 'completed after its grace ran out', + owner: 'user_alice', + late_after: dayFromToday(-2), + }); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_late, 'completed two days past its grace').toBe(true); + + // Widen the grace so that, recomputed today, the same completion would be + // on time. Nothing may recompute it. + await data.update('duly_duty', { id: duty.id, grace_days: 30 }); + + const after = await read(task.id); + expect(after.completed_late, 'a compliance verdict that rewrites itself is worth nothing').toBe(true); + expect(after.late_after, 'and the deadline it was judged against stands too').toBe(dayFromToday(-2)); + }); +}); + +describe('completed_late — the verdict, stamped with completed_at', () => { + it('is false for a completion inside the grace window', async () => { + const task = await newTask({ due_date: dayFromToday(-3), late_after: dayFromToday(2) }); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_late, 'past due but inside its grace — the whole point of grace').toBe(false); + }); + + it('is false on the LAST day of the window, not true', async () => { + // `late_after` is the last day still inside the window: grace is granted in + // whole days, so a task completed at any hour of that day is on time. Off + // by one here and every duty grants a day less grace than it says. + const task = await newTask({ due_date: dayFromToday(-5), late_after: dayFromToday(0) }); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_late).toBe(false); + }); + + it('is true the day after the window closes', async () => { + const task = await newTask({ due_date: dayFromToday(-9), late_after: dayFromToday(-1) }); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_late).toBe(true); + }); + + it('a task with no deadline is completed ON TIME, never "unknown"', async () => { + // A definite answer, so `done` always splits into on-time + late and the + // dashboard's two counts add up to the third. + const task = await newTask({ subject: 'nothing was owed by any day' }); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_late).toBe(false); + }); + + it('every row that reaches done carries a definite verdict', async () => { + // The invariant behind `tasks_done_on_time + tasks_completed_late = + // tasks_done`. A null verdict is not a third state; it is a done row + // missing from the metric that this card exists to make computable. + for (const over of [ + { subject: 'verdict: no dates at all' }, + { subject: 'verdict: due, inside grace', due_date: dayFromToday(-1), late_after: dayFromToday(1) }, + { subject: 'verdict: due, past grace', due_date: dayFromToday(-9), late_after: dayFromToday(-4) }, + ]) { + const task = await newTask(over); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(typeof done.completed_late, `${over.subject}`).toBe('boolean'); + } + }); + + it('is stamped on an ALREADY-done insert — a seeded history or an import', async () => { + // These rows never make a completion transition, so the beforeUpdate leg + // never sees them. Without the insert leg the whole seeded six months would + // read as verdict-less and the demo's on-time rate would be empty. + const late = await newTask({ + subject: 'seeded, and late', + status: 'done', + due_date: '2026-05-01', + late_after: '2026-05-04', + completed_at: '2026-05-09T09:00:00.000Z', + }); + expect(late.completed_late).toBe(true); + + const onTime = await newTask({ + subject: 'seeded, and on time', + status: 'done', + due_date: '2026-05-01', + late_after: '2026-05-04', + completed_at: '2026-05-04T23:30:00.000Z', + }); + expect(onTime.completed_late).toBe(false); + }); + + it('is cleared when a task is reopened — a completion that did not happen has no verdict', async () => { + const task = await newTask({ due_date: dayFromToday(-9), late_after: dayFromToday(-4) }); + await data.update('duly_task', { id: task.id, status: 'done' }); + expect((await read(task.id)).completed_late).toBe(true); + + await data.update('duly_task', { id: task.id, status: 'in_progress' }); + const reopened = await read(task.id); + expect(reopened.completed_at ?? null).toBeNull(); + expect(reopened.completed_late ?? null, 'the verdict goes with the timestamp').toBeNull(); + }); + + it('is not re-judged when an already-done task is saved again', async () => { + // Same reason `completed_at` is not re-stamped: a re-save is a state, not a + // transition. Re-judging here would silently turn every on-time record late + // the moment someone edits a note after the deadline. + const task = await newTask({ due_date: dayFromToday(-2), late_after: dayFromToday(0) }); + await data.update('duly_task', { id: task.id, status: 'done' }); + expect((await read(task.id)).completed_late).toBe(false); + + await data.update('duly_task', { + id: task.id, + status: 'done', + note: 'a note added long after the fact', + late_after: dayFromToday(-10), + }); + expect((await read(task.id)).completed_late, 'a saved record is not a new completion').toBe(false); + }); + + it('is not writable by a caller', async () => { + const task = await newTask({ due_date: dayFromToday(-9), late_after: dayFromToday(-4) }); + const done = await data.update('duly_task', { id: task.id, status: 'done', completed_late: false }); + expect(done.completed_late, 'the caller does not get to choose this either').toBe(true); + }); +}); + +// ── The verdict on the shared-payload path ───────────────────────────────── +// +// `completed_late` is read off THIS row's `late_after`, so it is the rewrite +// ADR-0058 Addendum II D3 puts outside the contract — and worse than +// `completed_at`, because two rows in one batch can legitimately disagree. +// +// The response is asymmetric, and the asymmetry is what makes the write sound: +// a row that would be stamped LATE refuses the write, and every row that +// survives its own guard computes `false`, which is row-invariant by +// construction. So a batch is either wholly on time and stamped correctly, or +// refused — and bulk complete, which this product is built around, keeps +// working for the case it is actually used for. +describe('completed_late on a predicate write — one payload, N rows', () => { + const refusal = async (promise: Promise) => { + try { + await promise; + } catch (error: any) { + return { code: error?.code, status: error?.status, message: String(error?.message ?? '') }; + } + throw new Error('expected the predicate write to be refused, but it resolved'); + }; + + const openTask = (subject: string, lateAfterDays: number) => + newTask({ subject, due_date: dayFromToday(lateAfterDays - 1), late_after: dayFromToday(lateAfterDays) }); + + it('refuses a bulk completion that contains a late row, naming it', async () => { + // THE assertion. Without it, whichever dispatch runs last decides the + // compliance verdict for every row in the selection — five tasks ticked + // together, one of them late, and the answer is either "all late" or "all + // on time" depending on an order the caller cannot see or control. + const onTime = (await openTask('bulk: inside its grace', 4)).id; + const late = (await openTask('bulk: past its grace', -2)).id; + + const { code, status, message } = await refusal( + data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [onTime, late] } } }), + ); + + expect(code).toBe('DULY_TASK_BULK_LATE_COMPLETION'); + expect(status).toBe(409); + expect(message, 'the refusal must name the row a caller has to remove').toContain(late); + expect(message, 'and the day its grace ran out, which is why it was refused') + .toContain(dayFromToday(-2)); + }); + + it('writes nothing at all — the refusal is not a partial batch', async () => { + const onTime = (await openTask('bulk partial: on time', 4)).id; + const late = (await openTask('bulk partial: late', -3)).id; + + await refusal( + data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [onTime, late] } } }), + ); + + for (const id of [onTime, late]) { + const row = await read(id); + expect(row.status, 'no row in a refused batch may commit').toBe('open'); + expect(row.completed_at ?? null).toBeNull(); + expect(row.completed_late ?? null).toBeNull(); + } + }); + + it('refuses whichever dispatch order the batch arrives in', async () => { + // Decided from the ROW alone. An accumulator would only catch the orders in + // which the late row happens to be dispatched second. + for (const lateFirst of [true, false]) { + const onTime = (await openTask(`order on-time ${lateFirst}`, 5)).id; + const late = (await openTask(`order late ${lateFirst}`, -1)).id; + const ids = lateFirst ? [late, onTime] : [onTime, late]; + + const { code } = await refusal( + data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }), + ); + expect(code, `late-first=${lateFirst} must refuse`).toBe('DULY_TASK_BULK_LATE_COMPLETION'); + } + }); + + it('refuses an all-late batch too, even though nothing would leak', async () => { + // The hook cannot see the batch — `dispatch.index` is a position, not a + // total — so the rule is stated on the ROW: one a caller can predict and a + // test can pin. Same boundary the already-done guard draws. + const ids = [ + (await openTask('all late a', -2)).id, + (await openTask('all late b', -6)).id, + ]; + const { code } = await refusal( + data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }), + ); + expect(code).toBe('DULY_TASK_BULK_LATE_COMPLETION'); + }); + + it('still completes an on-time batch — every row stamped false, in one write', async () => { + // The negative control, and the feature this guard must not cost: a week of + // ticks in one gesture. Every row that survives its own guard computes the + // SAME value, so what reaches the shared payload is row-invariant. + const ids: string[] = []; + for (let i = 0; i < 5; i += 1) ids.push((await openTask(`on-time batch ${i}`, 2 + i)).id); + + const affected = await data.update('duly_task', { status: 'done' }, { + multi: true, + where: { id: { $in: ids } }, + }); + expect(affected).toBe(5); + + for (const id of ids) { + const row = await read(id); + expect(row.status).toBe('done'); + expect(row.completed_at, `${id} must be stamped like any other completion`).toBeTruthy(); + expect(row.completed_late, `${id} must carry a verdict, not a blank`).toBe(false); + } + }); + + it('completes a batch of deadline-less rows — no deadline is not a refusal', async () => { + // The shape the existing bulk-complete tests use, and the one a hand-created + // task arrives in. "No deadline to miss" is `false` for every row, so the + // batch is uniform and allowed. + const ids: string[] = []; + for (let i = 0; i < 3; i += 1) ids.push((await newTask({ subject: `undated batch ${i}` })).id); + + await data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }); + for (const id of ids) expect((await read(id)).completed_late).toBe(false); + }); + + it('leaves bulk SKIP alone — skipped is not a completion', async () => { + const ids = [ + (await openTask('skip late a', -4)).id, + (await openTask('skip on-time b', 4)).id, + ]; + const affected = await data.update( + 'duly_task', + { status: 'skipped', skip_reason: 'the plant was down for the whole period' }, + { multi: true, where: { id: { $in: ids } } }, + ); + expect(affected, 'a late row must not block a bulk skip — nothing is being judged').toBe(2); + for (const id of ids) { + expect((await read(id)).status).toBe('skipped'); + expect((await read(id)).completed_late ?? null).toBeNull(); + } + }); + + it('a predicate write clearing done is NOT refused — null is right for every row', async () => { + const late = (await openTask('cleared late', -3)).id; + const onTime = (await openTask('cleared on-time', 3)).id; + await data.update('duly_task', { id: late, status: 'done' }); + await data.update('duly_task', { id: onTime, status: 'done' }); + expect((await read(late)).completed_late).toBe(true); + + const affected = await data.update('duly_task', { status: 'in_progress' }, { + multi: true, + where: { id: { $in: [late, onTime] } }, + }); + expect(affected).toBe(2); + for (const id of [late, onTime]) { + expect((await read(id)).completed_at ?? null).toBeNull(); + expect((await read(id)).completed_late ?? null, 'the verdict is cleared with the timestamp') + .toBeNull(); + } + }); + + it('does not fill a blank late_after on the shared-payload path', async () => { + // The fill leg is row-conditional in the other direction — "this row has no + // stamp" — so on a batch it would write one row's due date onto every + // matched row, overwriting a deadline another row was born with. Nothing is + // written instead: an unstamped row stays where it already was. + const blank = (await newTask({ subject: 'bulk re-date: no deadline yet' })).id; + const stamped = (await openTask('bulk re-date: already stamped', 6)).id; + const original = (await read(stamped)).late_after; + + await data.update('duly_task', { due_date: '2026-12-24' }, { + multi: true, + where: { id: { $in: [blank, stamped] } }, + }); + + expect((await read(stamped)).late_after, 'a stamped row must not be re-dated by another row') + .toBe(original); + expect((await read(blank)).late_after ?? null, 'and the blank row is left blank, not filled from a batch') + .toBeNull(); + }); +}); From de6ce90898123ded4308545c15ab4db62c7dde97 Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:25:03 +0000 Subject: [PATCH 2/3] Record the measured console bulk path and the tile's number format The console's bulk complete posts one payload per record (updateMany), so the shared-payload guard costs the UI gesture nothing; and a measure `format` is a numeral pattern, not a keyword. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- docs/product/data-model.md | 28 +++++++++++++--- src/datasets/duty-health.dataset.ts | 27 ++++++++++++++++ src/hooks/task.hook.ts | 32 ++++++++++++++++-- src/views/task.view.ts | 25 ++++++++++----- test/invariants.test.ts | 15 ++++++++- test/seed.test.ts | 50 ++++++++++++++++++++++++++--- test/views.test.ts | 9 ++++-- 7 files changed, 163 insertions(+), 23 deletions(-) diff --git a/docs/product/data-model.md b/docs/product/data-model.md index 340db9e..186635d 100644 --- a/docs/product/data-model.md +++ b/docs/product/data-model.md @@ -80,14 +80,32 @@ percentage describes work that already finished. `last_update_at` describes work that has quietly stopped, and it says so weeks before the due date does. It is server-owned, stamped on every status change and note edit. +**`late_after` and `completed_late` are the lateness pair, and they are stamped +once each.** `late_after` is `due_date + duty.grace_days`, written by the +dispatcher; `completed_late` is `completed_at > late_after`, written by the +completion hook. Both are server-owned and neither is ever recomputed — so the +Late list is a plain date filter, the on-time rate is a count over a boolean, +and the platform gap that made `completed_at <= due_date + duty.grace_days` +unaskable (objectstack#14104) stops being a blocker rather than being resolved. + +The consequence is deliberate and worth stating plainly: **editing a duty's +grace does not re-adjudicate work already dispatched or already completed.** A +task records what was owed when it was owed, and a compliance record that +rewrites itself when configuration changes is worth nothing in front of an +auditor. The place a replay onto open tasks would belong is `duly_catalog_sync`, +which already exists to push duty edits onto instantiated records; it does not +do this today. + **Deliberately absent.** `is_late`, `is_overdue`, `is_open`, `is_completed`, `progress_percent`. -The first four are derivable from `due_date`, `grace_days` and `status`, all of -which are stored and indexed. A stored copy needs a writer that runs every -midnight, and the night it does not run, every view lies without erroring. A -formula field is worse: it is virtual, so a *filter* naming one silently matches -nothing at all. Consumers ask the stored columns. +The first four are the *maintained-flag* shape, which is a different thing from +the two stamps above: their truth changes with the clock rather than with the +record, so each needs a writer that runs every midnight, and the night it does +not run every view lies without erroring. A formula field is worse: it is +virtual, so a *filter* naming one silently matches nothing at all. The test is +not "is it derivable" — everything here is derivable — but "would a second write +ever have to happen". `AGENTS.md` rule 5 carries the boundary. `progress_percent` is a number nobody can verify, which is exactly why it becomes the number everyone reports. Progress lives in `status` and `last_update_at`. diff --git a/src/datasets/duty-health.dataset.ts b/src/datasets/duty-health.dataset.ts index f191e36..ccb0d38 100644 --- a/src/datasets/duty-health.dataset.ts +++ b/src/datasets/duty-health.dataset.ts @@ -159,6 +159,33 @@ export const DutyHealth = defineDataset({ name: 'on_time_rate', label: 'On-time rate', derived: { op: 'ratio', of: ['tasks_done_on_time', 'tasks_done'] }, + /** + * A fraction rendered to two decimals, and NOT a percent — measured + * against the console, not chosen by taste. + * + * `format` here is a numeral PATTERN, not a keyword: the renderer takes + * the decimals from the digits after the point (`format.split('.')[1]`) + * and switches to percent only on a literal `%`. Measured against this + * demo's own 0.94, in the browser: + * + * (none) `0.94` + * 'percent' `1` — no `%` in the pattern, so this is not a percent at + * all, just zero decimals. Silently wrong on the one tile + * the product is judged by, which is why the obvious + * spelling is written down here as refuted. + * '0.00' `0.94`. What ships. + * + * `'0.0%'` is the spelling that would print `94.0%`, and it is + * deliberately NOT used. Read off the renderer rather than measured, + * because the demo cannot currently produce the value it goes wrong on: + * a percent is scaled by a HEURISTIC — `value > -1 && value < 1 ? value + * * 100 : value` — because a measure, unlike a field, cannot declare its + * scale. A rate of exactly 1 (100% on time, the number a customer most + * wants to see) falls outside that window and renders as `1.0%`. A tile + * that reports a perfect month as one percent is worse than one that + * says `1.00`. Filed as #101. + */ + format: '0.00', }, ], }); diff --git a/src/hooks/task.hook.ts b/src/hooks/task.hook.ts index 910fa11..4299013 100644 --- a/src/hooks/task.hook.ts +++ b/src/hooks/task.hook.ts @@ -136,12 +136,31 @@ import type { Hook, HookContext } from '@objectstack/spec/data'; * * A batch is therefore either wholly on time and stamped correctly, or refused * — decided from each row alone, in any dispatch order, with no accumulator. - * The cost is stated rather than hidden: a selection that includes late work - * cannot be completed in one gesture, and the caller is told which row and why. * Bulk SKIP is untouched (`skipped` is not a completion), and so is the * clearing direction: `completed_late = null` alongside `completed_at = null` * is correct for every row leaving `done`. * + * ── What this costs the console: nothing. Measured, not assumed ────────── + * The obvious fear is that ticking a week of work in one gesture now fails + * whenever one row is late. It does not, because the console's bulk action is + * NOT a predicate write. Measured against a live `pnpm demo` on + * `@objectstack/rest` 17.2.0 by recording the requests the toolbar issues: + * + * POST /api/v1/data/duly_task/updateMany + * { "records": [ { "id": …, "data": { "status": "done" } }, + * { "id": …, "data": { "status": "done" } } ], … } + * + * One payload PER RECORD, so each row is dispatched with its own — and a + * two-row selection of one late and one on-time task was completed in a single + * gesture with `completed_late: true` and `false` landing correctly on the two + * rows. The row-conditional stamp is sound there for the same reason it is + * sound on `mode: 'record'`. + * + * The shared payload this guard is about is the OTHER shape — `multi: true` + * with a `where`, which is what an import, a backfill, an MCP caller or a + * filtered REST update assembles. That is precisely where no view predicate can + * reach and why the authority has to live at the write. + * * Not extended to the view's `visible` predicate the way the already-done * guard is. That predicate is a client-side hide, and this condition depends on * the completion instant against a stored date — a boundary that moves at @@ -239,6 +258,15 @@ const stampTaskLifecycle = (ctx: HookContext): void => { // duty governs this row" already means to the overdue escalation, so the // two surfaces agree rather than one of them silently never firing. // + // ⚠ The one row this is WRONG for is a task created by hand ON A DUTY + // that grants grace — today, any `one_off` duty (#61 keeps `grace_days` + // on that form deliberately). The escalation reads the duty and waits; + // this fallback does not and stamps the due date. Filed as #100 with the + // options, because the fix is a producer this handler cannot be: a + // lowered `body` ships without its module scope and has no engine to read + // `duly_duty` with. It is not a regression — before the stamps existed the + // `late` view was grace-free for every task alike. + // // A task with no `due_date` keeps a blank stamp: there is no deadline to // derive, and a task with no due date genuinely cannot be late. const dueDay = dayOf(input.due_date); diff --git a/src/views/task.view.ts b/src/views/task.view.ts index c0f2efd..4c77ddc 100644 --- a/src/views/task.view.ts +++ b/src/views/task.view.ts @@ -49,15 +49,24 @@ const columns = [ * off the row's own `late_after` — so one batch cannot carry two answers. The * hook refuses a bulk completion whose row would be stamped late * (`DULY_TASK_BULK_LATE_COMPLETION`, 409) and stamps `false` otherwise, which - * every row that survived its own guard would have written. Bulk complete keeps - * working for on-time work; a selection containing late work is refused, by - * name, and those rows are ticked individually. + * every row that survived its own guard would have written. * - * That refusal deliberately has NO predicate half here. This one turns on the - * completion instant against a stored date — a boundary that moves at midnight, - * between the render and the write — so a client-side copy would sooner or - * later hide an action the server would have accepted. A missing bulk action - * with no explanation is worse than a refusal that names its cause. + * **The action below is unaffected, and that is measured rather than hoped + * for.** The toolbar does not issue a predicate write at all: recorded against + * a live `pnpm demo`, selecting two rows and pressing Complete sends + * `POST /api/v1/data/duly_task/updateMany` with `records: [{ id, data }, …]` — + * one payload PER RECORD. A selection of one late and one on-time task + * completed in one gesture, with `completed_late` landing `true` and `false` on + * the right rows. The shared payload the hook guards is the `multi: true` + + * `where` shape an import, a backfill or an MCP caller assembles, which is + * exactly the caller that never reads this file. + * + * So that refusal deliberately has NO predicate half here — and it would be + * the wrong place for one anyway. It turns on the completion instant against a + * stored date, a boundary that moves at midnight between the render and the + * write, so a client-side copy would sooner or later hide an action the server + * would have accepted. A missing bulk action with no explanation is worse than + * a refusal that names its cause. * * This predicate is kept because it is still the right UX: it stops the * console from assembling a batch the server would refuse, so a user gets an diff --git a/test/invariants.test.ts b/test/invariants.test.ts index 72dad55..38022ee 100644 --- a/test/invariants.test.ts +++ b/test/invariants.test.ts @@ -46,10 +46,23 @@ describe('product invariants', () => { expect(Task.fields.note.required).not.toBe(true); }); - it('lateness is derived from stored columns, never stored', () => { + it('no MAINTAINED lateness flag exists on the task', () => { + // The banned shape is a flag whose truth changes with the clock: it needs a + // writer every midnight and lies the night it does not run. `late_after` + // and `completed_late` (#52) are deliberately not in this list — each is + // written once, at the instant it becomes knowable, and never recomputed, + // which is the same category as `completed_at` beside them. `AGENTS.md` + // rule 5 carries the boundary; the difference is not the name but whether a + // second write ever has to happen. for (const flag of ['is_late', 'is_overdue', 'is_open', 'is_completed']) { expect(Object.keys(Task.fields)).not.toContain(flag); } + for (const stamp of ['late_after', 'completed_late']) { + expect( + (Task.fields as Record)[stamp]?.readonly, + `${stamp} must be readonly — a write-once column a caller can set is a column that drifts`, + ).toBe(true); + } }); it('caliber is a stored column on both duty and task', () => { diff --git a/test/seed.test.ts b/test/seed.test.ts index be027a9..e2fc187 100644 --- a/test/seed.test.ts +++ b/test/seed.test.ts @@ -199,14 +199,55 @@ describe('view populations — what an evaluator actually opens', () => { }); it('Late has 3-4 rows, as the card asks', async () => { - // `late`: due_date < {today} AND status in (open, in_progress). + // `late`: late_after < {today} AND status in (open, in_progress). Since #52 + // the lens reads the stamped deadline, not the raw due date — a task inside + // the grace its own duty granted is not late yet. const late = (await all('duly_task')).filter( - (task) => task.due_date && String(task.due_date) < TODAY && ['open', 'in_progress'].includes(task.status), + (task) => task.late_after && String(task.late_after) < TODAY && ['open', 'in_progress'].includes(task.status), ); expect(late.length).toBeGreaterThanOrEqual(3); expect(late.length).toBeLessThanOrEqual(4); }); + it('every seeded task carries the deadline it was dispatched with, and grace is really in it', async () => { + // Item 6 of #52: the demo has to carry the stamps, and they have to come + // from the DUTY's grace rather than from the due date. Drop `grace_days` + // out of `DISPATCH_DUTIES` and every value here collapses onto `due_date` + // — the whole fixture silently reverts to the grace-free reading, with + // nothing erroring and the counts above unchanged. + const tasks = (await all('duly_task')).filter((task) => task.due_date); + expect(tasks.length).toBeGreaterThan(100); + for (const task of tasks) { + expect(task.late_after, `${task.subject} has a due date and no deadline`).toBeTruthy(); + expect( + String(task.late_after) >= String(task.due_date), + `${task.subject}: a deadline before its own due date`, + ).toBe(true); + } + const withGrace = tasks.filter((task) => String(task.late_after) > String(task.due_date)); + expect( + withGrace.length, + 'no seeded task has any grace at all — the duty projection is not reading grace_days', + ).toBeGreaterThan(20); + }); + + it('the on-time rate the dashboard reads is a real number, not 100%', async () => { + // `duly_duty_health` counts `completed_late` on the governed population. + // Every done row must carry a definite verdict or the two counts stop + // adding up to `tasks_done`, and the split has to be non-trivial or the + // tile reads as fabricated. + const done = (await all('duly_task')).filter((task) => task.status === 'done'); + expect(done.length).toBeGreaterThan(50); + for (const task of done) { + expect(typeof task.completed_late, `${task.subject} completed with no verdict`).toBe('boolean'); + } + const late = done.filter((task) => task.completed_late === true); + expect(late.length, 'a history with no late completion makes the measure decorative') + .toBeGreaterThan(0); + expect(late.length / done.length, 'and a mostly-late history is not a plausible demo either') + .toBeLessThan(0.5); + }); + it('Not moving has 2-3 rows — and the second seed pass is what puts them there', async () => { // `stalled`: status in (open, in_progress) AND last_update_at < {14_days_ago}. // @@ -225,7 +266,7 @@ describe('view populations — what an evaluator actually opens', () => { expect(stalled.length).toBeLessThanOrEqual(3); }); - it('stagnation is NOT lateness — at least one stalled row is not yet due', async () => { + it('stagnation is NOT lateness — at least one stalled row is not yet late', async () => { // The product's central claim: stagnation fires while intervening is still // cheap, weeks before a due date makes the failure obvious. If every // stalled row were also late, the two views would be one view with a @@ -236,7 +277,8 @@ describe('view populations — what an evaluator actually opens', () => { ['open', 'in_progress'].includes(task.status) && task.last_update_at && new Date(task.last_update_at as string).getTime() < threshold && - String(task.due_date) >= TODAY, + // Against the stamped deadline, which is what the Late lens reads. + String(task.late_after) >= TODAY, ); expect(stalledNotLate.length).toBeGreaterThanOrEqual(1); }); diff --git a/test/views.test.ts b/test/views.test.ts index edaaff2..7edb420 100644 --- a/test/views.test.ts +++ b/test/views.test.ts @@ -200,9 +200,12 @@ describe('the lenses say what the product means', () => { }); /** - * Lateness and stagnation are asked of stored, indexed columns. A stored - * flag needs a writer that runs every midnight; a formula field is virtual - * and a filter naming one silently matches nothing. + * Lateness and stagnation are asked of stored, indexed columns — since #52 + * that includes `late_after`, the deadline stamped on the row at dispatch. + * What stays banned is the MAINTAINED flag: one whose truth changes with the + * clock, needing a writer every midnight and lying the night it does not run. + * A formula field is worse again — virtual, so a filter naming one silently + * matches nothing. */ it('no filter reaches for a derived flag', () => { for (const { where, view } of allViews) { From 5c4d04ac8f8599786d3feeedd082c5d4bf002c00 Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 1 Sep 2026 16:31:53 +0000 Subject: [PATCH 3/3] Carry the two lateness stamps into the zh-CN bundle and the i18n walk #103's coverage gate went red on merge, exactly as it is built to: the two new field labels and the new widget had no bundle key, three dataset slots had no verdict, and the zh-CN dashboard description still explained why lateness was missing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- src/translations/authored-text.ts | 8 ++++++++ src/translations/zh-CN.ts | 19 ++++++++++++++++++- test/i18n-coverage.test.ts | 7 ++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/translations/authored-text.ts b/src/translations/authored-text.ts index ca2cc07..3993035 100644 --- a/src/translations/authored-text.ts +++ b/src/translations/authored-text.ts @@ -371,6 +371,14 @@ const VERDICTS: Readonly> = { 'dataset.measures[].name': machine('measure name — bound by dashboards'), 'dataset.measures[].field': machine('field path'), 'dataset.measures[].aggregate': machine('aggregation function'), + 'dataset.measures[].derived.op': machine('derived-measure operator'), + // Other MEASURE names, by ADR-0021 Q1 — a derived measure references + // measures and nothing else, which is what keeps it enumerable. Same + // category as `measures[].name` above, seen from the other end. + 'dataset.measures[].derived.of[]': machine('operand measure names'), + // A numeral PATTERN (`0.00`), not a word: it says how many decimals and + // whether to render a percent. Nothing in it is language. + 'dataset.measures[].format': machine('number format pattern'), // ── flow — no translator, no bundle group ───────────────────────────── 'flow.name': machine('flow name'), diff --git a/src/translations/zh-CN.ts b/src/translations/zh-CN.ts index 0636454..cf2b1b9 100644 --- a/src/translations/zh-CN.ts +++ b/src/translations/zh-CN.ts @@ -239,6 +239,17 @@ export const dulyChinese = defineTranslationBundle({ help: '跳过是一种正当结果——“当时装置停车，没有任何东西需要申报”。记下原因，才不会让“跳过”变成“完成”的同义词。', }, completed_at: { label: '完成时间' }, + // 逾期判定的两个「盖章」字段（#52）。用「宽限期」与职责上的 + // grace_days 保持同一个词，读者才能把两处联系起来；用「派发时」 + // 而不是「当时」，是因为这里要说清楚盖的是哪一刻的章。 + late_after: { + label: '逾期起算日', + help: '到期日加上派发这条任务时职责给出的宽限期。过了这一天仍未完成，或在这一天之后才完成，即为逾期。此值在派发时写入一次——之后修改职责的宽限期不会改动它。', + }, + completed_late: { + label: '逾期完成', + help: '完成时间晚于「逾期起算日」时为是。此值在完成的那一刻写入一次，按当时生效的宽限期判定——之后修改职责的宽限期不会改动它。', + }, last_update_at: { label: '最后更新' }, note: { label: '备注', @@ -470,7 +481,7 @@ export const dulyChinese = defineTranslationBundle({ dashboards: { duly_duty_health: { label: '职责健康度', - description: '仅统计组织认定的职责——来自岗位职责库与主管指派的工作；自行申报的职责不计入这里的任何数字。逾期暂未展示：它取决于每条职责各自的宽限期，而本视图无法应用宽限期——“团队 → 逾期”列表是目前的替代答案，它不考虑宽限期。', + description: '仅统计组织认定的职责——来自岗位职责库与主管指派的工作；自行申报的职责不计入这里的任何数字。按期与否，按每条任务派发当时自身的宽限期判定；分母只含已完成的工作——未完成的工作由「停滞」几块指标回答。', widgets: { not_moving_14d: { title: '停滞', @@ -484,6 +495,12 @@ export const dulyChinese = defineTranslationBundle({ title: '最久未动的任务', description: '最停滞的那个待办任务上一次有动静的时间——是一个日期，不是一个分数。', }, + // 「按期率」用 #52 的原词。分母写清楚是「已完成」，是因为读者若 + // 默认分母是「应完成」，同一块指标会读出完全不同的数。 + on_time_rate: { + title: '按期率', + description: '在各自宽限期内完成的、组织认定的任务，占已完成的组织认定任务的比例。未完成的工作不计入这里。', + }, not_moving_by_unit: { title: '停滞情况（按部门）', description: '各部门超过 14 天没有动静的、组织认定的待办任务。部门按名称排序，绝不按数量排序。', diff --git a/test/i18n-coverage.test.ts b/test/i18n-coverage.test.ts index 34e8363..256a053 100644 --- a/test/i18n-coverage.test.ts +++ b/test/i18n-coverage.test.ts @@ -295,7 +295,12 @@ describe('untranslatable display text is declared rather than dropped', () => { walk.untranslatable.filter((entry) => entry.path.startsWith(prefix)).length; expect(count('view.bulkActionDefs'), 'bulk-action toolbar copy').toBe(35); expect(count('object.validations'), 'custom validation messages').toBe(11); - expect(count('dataset.'), 'dataset labels behind chart axes').toBe(26); + // Was 26 before #52 added the three on-time measures — `Done on time`, + // `Completed late` and the `On-time rate` derived from them. A measure + // label still has no bundle key anywhere in the platform's schema, so each + // new one enlarges the same declared gap rather than opening a new kind of + // one; the number moves with the code because that is what this pin is for. + expect(count('dataset.'), 'dataset labels behind chart axes').toBe(29); // Was 6 before #99 (#69) landed: three `notify` nodes' inline title and // message. They now reference an email template, whose per-locale rows are // checked below — so this is a gap that CLOSED, pinned at zero so it