Skip to content

Commit 8b73784

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-15546-all-null-sum-zero
2 parents 2f5ff1e + 6ba0db4 commit 8b73784

46 files changed

Lines changed: 1846 additions & 329 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): declare the aggregate × field-type compatibility matrix a dataset measure is judged against — `AGGREGATE_FIELD_TYPE_COMPATIBILITY` and `isAggregateCompatibleWithFieldType` (#16353, spec half of #16099)
6+
7+
A dataset measure pairs an `aggregate` with a `field`, and nothing between author and driver correlated the two: `avg` over a `Field.datetime` compiled to `AVG(col)` and reached the backend, where one SQL family averages the column's storage form and another rejects the call — one metadata document, two answers. Which pairs are accepted is a contract, so it is now declared once in `@objectstack/spec/data`:
8+
9+
| Aggregate | Accepted field types |
10+
|---|---|
11+
| `count`, `count_distinct` | every `FieldType` |
12+
| `sum` | `number`, `currency`, `rating`, `slider`, `progress`, `summary` — the numeric class EXCEPT `percent` (a rate does not add; `isIncoherentAggregate` already says so) |
13+
| `avg` | the numeric class, `percent` included |
14+
| `min`, `max` | the numeric class plus `date`, `datetime`, `time` — both return a value of the field's own type |
15+
| every other pair | refused |
16+
17+
The ruling (director, decision batch #59, 2026-09-06) named its buckets by category; the table resolves them against the real `FieldType` membership through the `field-value.zod` semantic classes: "numeric" is `NUMERIC_VALUE_TYPES` (`integer` is a driver-internal column alias, not a `FieldType` — the integer-valued authorable members are `rating` / `slider` / `progress`); "temporal" is the three temporal classes, `time` included because its stored form is a dialect question exactly like `date` / `datetime` (native TIME on Postgres and MySQL, canonical `HH:MM:SS[.fff]` TEXT on SQLite), the canonical form orders chronologically on every dialect, and `AnalyticsResult.fields[].type` already describes `min` / `max` over it as temporal (#15768). `formula` is refused for arithmetic aggregates whatever its declared `returnType`: it is virtual in SQL storage, no column exists to aggregate.
18+
19+
Two refused rows override existing opinions and are recorded as such, not presented as agreement. **Booleans** are refused for `sum` / `avg` / `min` / `max` by the ruling's "every other pair: refused", while maintainer ruling #11152 already has every backend answer them as numbers (`sum(flag)=3`, `avg(flag)=0.5`, `min(flag)=0`, `max(flag)=1`, pinned in the spec's `AGGREGATION_CASES`; `driver-sql` casts the aggregand on Postgres to make it hold). That refusal is therefore not grounded in backend divergence; whether booleans belong in those rows is a collision between two rulings and is referred to the maintainer as its own decision — the row ships exactly as batch #59 stated it. **The string classes** are refused for `min` / `max` here, while `service-analytics` (#15768) already types `min` / `max` over them as a supported `'string'` result; the refusal is defensible (string order is collation-dependent) but it overrides that opinion.
20+
21+
**The narrowing, stated plainly.** Every pair outside the table — `avg` × `datetime`, `sum` × `boolean`, `min` × `text`, `sum` × `percent`, and so on — is an authoring shape `DatasetMeasureSchema` accepts today and will be REFUSED once the two consumer legs land: the compile-time refusal in the dataset compiler (#16099) and the authoring-time lint rule (its devx sub-card). A measure whose pair is refused is fixed by changing the aggregate to one the field's type supports (`min` / `max` for a temporal field; `avg` for a `percent`; `count` for anything), never by widening the table.
22+
23+
**Not breaking in this release, `minor` on purpose.** This changeset ships a table and a predicate that nothing yet enforces: `DatasetMeasureSchema` accepts byte-for-byte what it accepted before, no export is removed or narrowed, and no runtime path reads the table yet. It is an additive widening of the published surface — two new exports in `dist/*.d.ts` — which the maintainer ruling of 2026-09-04 (decision batch #35) puts at `minor`. The refusal itself arrives with the consumer legs, whose changesets carry the breaking declaration, its migration prescription and the ADR-0087 disposition; this one names the narrowing so an upgrading author can read the contract before it is executed.
24+
25+
`isIncoherentAggregate` is unchanged and stays the semantic opinion beside this table. The two diverge on exactly one pair: `count_distinct` × `percent` is flagged there and accepted here (the ruling reads `count_distinct` as "any type"). That divergence is pinned in the table's test and reported on #16353 rather than resolved silently.

.changeset/duration-unit-in-key-name.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"@objectstack/spec": minor
33
---
44

5-
feat(spec)!: a duration-shaped `z.number()` key carries its unit in the key name — `hook.timeout` / `job.timeout` / `DriverOptions.timeout``timeoutMs`, `MetadataManagerConfig.cache.ttl``ttlSeconds`, `cache.databaseLoader.ttl` `ttlMs`, tenant `idleTimeout` / `sessionTimeout``*Seconds`; new gate `check:duration-unit-keys` (#14478, #14519)
5+
feat(spec)!: a duration-shaped `z.number()` key carries its unit in the key name — `hook.timeout` / `job.timeout` / `DriverOptions.timeout``timeoutMs`, `MetadataManagerConfig.cache.databaseLoader.ttl``ttlMs` (the outer `cache.ttl` leaves outright under #15624 — nothing read it), tenant `idleTimeout` / `sessionTimeout``*Seconds`; new gate `check:duration-unit-keys` (#14478, #14519)
66

77
<!-- adr-0087: registered hook-timeout-to-timeout-ms, job-timeout-to-timeout-ms, metadata-manager-config-cache-ttl-unit-in-key, driver-options-timeout-to-timeout-ms, tenant-timeouts-unit-in-key -->
88

@@ -38,7 +38,7 @@ comment and published a bare `300` / `3600` to the reference page (#14519).
3838
| `HookSchema` (`hooks[]`) | `timeout` | `timeoutMs` | unchanged (ms) |
3939
| `JobSchema` (`jobs[]`) | `timeout` | `timeoutMs` | unchanged (ms) |
4040
| `DriverOptionsSchema` | `timeout` | `timeoutMs` | unchanged (ms) |
41-
| `MetadataManagerConfigSchema` | `cache.ttl` | `cache.ttlSeconds` | unchanged (s, default 3600) |
41+
| `MetadataManagerConfigSchema` | `cache.ttl` | *(deleted — its respelling `ttlSeconds` was retired before it shipped, #15624; the outer `cache` block was read by nothing, and the live TTL is `cache.databaseLoader.ttlMs`)* | |
4242
| `MetadataManagerConfigSchema` | `cache.databaseLoader.ttl` | `cache.databaseLoader.ttlMs` | unchanged (ms, default 60000) |
4343
| `DatabaseLevelIsolationStrategySchema` | `connectionPool.idleTimeout` | `connectionPool.idleTimeoutSeconds` | unchanged (s, default 300) |
4444
| `TenantSecurityPolicySchema` | `accessControl.sessionTimeout` | `accessControl.sessionTimeoutSeconds` | unchanged (s, default 3600) |
@@ -52,10 +52,13 @@ new MetadataManager({ cache: { ttl: 3600, databaseLoader: { ttl: 60_000 } } });
5252
// after — rename the key; the number is unchanged
5353
defineHook({ name: 'audit_order', object: 'order', events: ['afterInsert'], handler: 'auditOrder', timeoutMs: 5000 });
5454
defineJob({ name: 'nightly_sweep', schedule: { type: 'cron', expression: '0 1 * * *' }, handler: 'sweep', timeoutMs: 300000 });
55-
new MetadataManager({ cache: { ttlSeconds: 3600, databaseLoader: { ttlMs: 60_000 } } });
55+
new MetadataManager({ cache: { databaseLoader: { ttlMs: 60_000 } } }); // the outer `ttl` is deleted, not renamed (#15624)
5656
```
5757

58-
**Migration.** Rename each key; no value changes. Authoring an old spelling
58+
**Migration.** Rename each key; no value changes — with one exception: the outer
59+
`MetadataManagerConfig.cache.ttl` is DELETED, not renamed (its respelling `ttlSeconds`
60+
was retired before it shipped, #15624; nothing ever read the outer `cache` block, and
61+
the nested `cache.databaseLoader.ttl → ttlMs` rename above is unchanged). Authoring an old spelling
5962
fails to compile (`tsc`: the input type is `never`) and fails to parse with a
6063
prescription naming the new key. For `hooks[]` / `jobs[]` the rename is a
6164
mechanical D2 conversion (`hook-timeout-to-timeout-ms`,
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
feat(spec): `ExecutionStepMetrics` gains an optional `failures` slot, and `FlowRunSummary.failed` is declared as the fold INCLUDING what a delegating node rolled up from its child (maintainer ruling 2026-09-06 on #15617, spec half)
6+
7+
Additive. Nothing an author writes is renamed, retired or narrowed; no accept
8+
set shrinks. One optional key is declared on a runtime-produced schema and the
9+
prose of a published contract is reconciled with itself.
10+
11+
**What was wrong.** `FlowRunSummary` said two things about `failed`. Its
12+
header paragraph declared that a `subflow` node rolls its child run's totals
13+
up into the parent — "this summary answers *what did this run cause*" — while
14+
the field itself declared `failed = Σ nodes[].failures`, a fold over the
15+
parent's own node executions. For a parent that delegates its rows to a
16+
`subflow` (or a `map` item) those give different answers, and the engine could
17+
only satisfy the second one: `ExecutionStepMetrics` carried `selected` /
18+
`acted` / `unmeasuredEffect` and no failure slot, so a child's contained
19+
failures had no path into the parent's fold. Measured on the real engine by
20+
the services seat (#15617): parent `loop { subflow(child) }` → parent
21+
`failed=0` while the five child summaries carried `failed=[0,0,0,0,1]`
22+
`acted` rolled up, `failed` did not.
23+
24+
**What this declares.**
25+
26+
- `ExecutionStepMetrics.failures` (optional, integer ≥ 0): node executions
27+
that failed inside a child run this execution delegated to and went on from
28+
— a `subflow` child or a `map` item whose run COMPLETED while containing
29+
failures, i.e. the child's `summary.failed`, rolled up. It folds into the
30+
delegating node's `nodes[].failures` and so into the run-level `failed`
31+
the same fold shape `acted` has, but not the same rule at the failed-child
32+
boundary (next bullet). Absent means the step delegated nothing, or its
33+
child tracked no count, or the producer did not track it (every step the
34+
engine emits between this release and the engine half) — never zero.
35+
- It is NOT the step's own outcome. A step that failed is `status: 'failure'`
36+
and counts once through `nodes[].failures`, as before; a child that FAILED
37+
— whether or not it also contained failures before it failed — is
38+
precisely that step failure: its own `failed`, contained and fatal alike,
39+
stays on the child's run row and nothing rides up, so one failure is never
40+
counted twice. This is where the rule parts from `acted`, which does carry
41+
a failed child's writes up to the parent. The control the card measured (a
42+
failing child → parent `failed=1`) keeps counting exactly as today.
43+
- `FlowRunSummary.failed` is declared, at the field, as the fold of
44+
`nodes[].failures` INCLUDING what a delegating node rolled up; the
45+
`FlowRunNodeSummary.failures` describe names the roll-up path, and its
46+
`status` describe states that a delegating node whose child contained
47+
failures reads `success` beside `failures > 0` — status is judged on the
48+
node's own executions.
49+
50+
**What this does not do yet.** This is the contract half of a two-lane
51+
landing (contract first). No producer populates `failures` in this release:
52+
`subflow-node.ts` and the `map` node roll the child's contained failures into
53+
the slot in the services half, #16314, and only then does a parent's
54+
`failed` start counting them. Until that lands, every `ExecutionStepMetrics`
55+
the engine emits is byte-identical to today's, `failed` is numerically what it
56+
was, and the flow-run reference page keeps the narrowed wording PR #15609
57+
shipped ("node executions **of this run**") on purpose — it is widened when
58+
both halves are in.
59+
60+
**Consumers.** A reader of `ExecutionStepMetrics` sees one more optional
61+
number and nothing else changes shape; a consumer that already sums
62+
`nodes[].failures` to cross-check `failed` keeps agreeing with it, because the
63+
fold is unchanged — the roll-up enters the per-node array, not beside it. Two
64+
consequences of that placement are part of the contract from this release,
65+
even though no producer populates the slot yet: on a delegating node
66+
`nodes[].failures` may exceed `runs` (`runs: 5, failures: 15` is a legal
67+
shape — five subflow executions whose children each contained three), and it
68+
is no longer only that node's own failed executions, so a reader that derived
69+
"this node's executions that failed" or a failure RATE from `failures / runs`
70+
must read a delegating node's number as "failures this node caused, its
71+
child's contained ones included".
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata": patch
4+
---
5+
6+
feat(spec)!: retire the three inert outer keys of `MetadataManagerConfig.cache``enabled`, `ttlSeconds` (formerly `ttl`) and `maxSize` — read by nothing; `cache.databaseLoader` is the only live half (#15624, ADR-0049)
7+
8+
<!-- adr-0087: registered metadata-manager-config-inert-cache-keys-retired -->
9+
10+
**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep
11+
launch-window convention ships it as `minor`; the migration prescription is
12+
registered under protocol major 18, where `os migrate meta` users will look).
13+
ADR-0049 enforce-or-remove decides it: a declared-but-unenforced key with zero
14+
measured readers comes off, and the published reference page stops teaching it.
15+
16+
`MetadataManagerConfig.cache` declared three outer knobs — `enabled` (default
17+
`true`), `ttlSeconds` (default 3600; spelled `ttl` until #14478) and `maxSize`
18+
("Max cache size in bytes") — beside the nested `databaseLoader` block, and
19+
**nothing read the outer three**. The only runtime consumer of the block is
20+
`MetadataManager` (`packages/metadata`), which hands `cache.databaseLoader` and
21+
nothing else to `new DatabaseLoader({ cache })`; a reader census over
22+
`packages/**` (tests and changelogs excluded) found no runtime reader of any
23+
outer key, while the same grep shape found the nested `cache?.databaseLoader`
24+
read twice — the control that makes the zero a measurement. An author writing
25+
`cache: { enabled: false }` or `cache: { ttlSeconds: 60 }` got a clean parse
26+
and a cache that behaved exactly as before, with no error and no warning, and
27+
the published reference page (`references/kernel/metadata-loader`) documented
28+
all three as if they configured something.
29+
30+
**What is refused:** authoring `cache.enabled`, `cache.ttlSeconds`, `cache.ttl`
31+
or `cache.maxSize` on `MetadataManagerConfig`, with any value — directly, through
32+
`MetadataManagerOptions`, or through `MetadataPluginConfig.storage`. The nested
33+
object is not `.strict()`, so each key is a `retiredKey()` tombstone rather than
34+
a bare deletion (a deletion would have stripped it in silence — the same no-op
35+
one layer down): authoring it is a `tsc` error (`never`) and a parse error
36+
carrying the prescription, which names the live nested knob.
37+
38+
**What stays, byte-identical:** the DatabaseLoader read-through cache under
39+
`cache.databaseLoader``enabled` (default `true`), `maxSize` (an entry count,
40+
default 500) and `ttlMs` (milliseconds, default 60000) — and every runtime
41+
path. Parsed configs no longer carry the two former defaults (`enabled: true`,
42+
`ttlSeconds: 3600`) that were materialized and never consulted.
43+
44+
**The #14478 rename is folded in.** `cache.ttl``cache.ttlSeconds` was
45+
registered under this same unreleased major and never reached a published
46+
release, so it is absorbed by the removal: `cache.ttl`'s tombstone now
47+
prescribes deletion (naming `cache.databaseLoader.ttlMs`) instead of a rename to
48+
a key that is itself retired — an author upgrading from a published 17.x sees
49+
one hop. The nested `cache.databaseLoader.ttl``ttlMs` half of that rename is
50+
unchanged.
51+
52+
## FROM → TO
53+
54+
```ts
55+
// before — parsed green; no runtime ever read the three outer numbers
56+
new MetadataManager({
57+
datasource: 'default',
58+
cache: { enabled: true, ttlSeconds: 3600, maxSize: 10_485_760, databaseLoader: { ttlMs: 60_000 } },
59+
});
60+
61+
// after — delete the outer keys; the nested block is the cache that runs
62+
new MetadataManager({
63+
datasource: 'default',
64+
cache: { databaseLoader: { enabled: true, maxSize: 500, ttlMs: 60_000 } },
65+
});
66+
```
67+
68+
**Migration.** Delete `cache.enabled`, `cache.ttlSeconds` / `cache.ttl` and
69+
`cache.maxSize`; nothing replaces them, because nothing ever consumed them. If
70+
you meant to switch the cache off, cap it or set its TTL, write
71+
`cache.databaseLoader.enabled` / `.maxSize` (entries) / `.ttlMs` (milliseconds)
72+
— those are honoured. No `os migrate meta` conversion runs on this surface: a
73+
`MetadataManager` config is not a stack collection member and never a stored
74+
row, so the chain has no seam for it; the D3 semantic entry
75+
`metadata-manager-config-inert-cache-keys-retired` carries the prescription
76+
into `spec-changes.json`, the upgrade guide and the `spec_changes` MCP tool.
77+
78+
The retirement kit: `retiredKey()` tombstones on all three (and the absorbed
79+
`ttl`), `RETIRED_KEYS_BY_MAJOR[18]` entries for each, the D3 semantic entry
80+
above (the #14478 entry's outer half is re-worded from a rename to a deletion),
81+
negative pins asserting each prescription and a positive pin asserting the
82+
parse output no longer materializes the retired defaults, the published
83+
reference pages regenerated, and the hand-written docs page and this package's
84+
README (`@objectstack/metadata` ships `README.md`, hence its `patch`) no longer
85+
authoring `cache.enabled`.

0 commit comments

Comments
 (0)