Skip to content

Commit 78adc8e

Browse files
authored
Merge branch 'main' into claude/issue-17883-generate-migration-file-column-width
2 parents c1e5ce2 + 60b9955 commit 78adc8e

53 files changed

Lines changed: 3628 additions & 1103 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.

.changeset/16236-formula-return-type-measure-column.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@
44

55
fix(service-analytics): a `min`/`max` over a `formula` field is typed from the formula's declared `returnType`, not described as `number` (#16236)
66

7+
> ⚠️ **Superseded within the same release window — ⛔ do not act on this entry.**
8+
> Everything below was accurate when it was written and is kept as the record of what
9+
> #16236 measured and built. It never reached a published version: **#17560** (director
10+
> ruling, decision batch #127, 2026-09-13) refuses `min` / `max` over a `formula` field
11+
> outright, on the compatibility table's own storage ground — a formula is VIRTUAL in SQL
12+
> storage, no column is emitted, so no aggregate can be lowered to it whatever
13+
> `returnType` says. At the version that compiles this entry such a measure answers
14+
> `DATASET_INVALID` / **400** at compile time instead of carrying any `fields[].type`, and
15+
> the `returnType?: string` member described at the foot of this entry is **not** on
16+
> `AnalyticsServiceConfig.sourceFieldMeta` — it was added and removed inside one release
17+
> window, so no published version ever carried it. ⇒ Read #17560's entry instead; the
18+
> FROM → TO below never became a shipped behaviour.
19+
720
**Behaviour change — read this if any dataset measure aggregates a `formula`
821
field.** `AnalyticsResult.fields[].type` for such a measure column was always
922
`number`, whatever the formula computes. It is now translated from the field's
@@ -47,3 +60,8 @@ a word outside the declared four: left alone, never guessed at.
4760
host that returns the three-member shape still satisfies the contract and gets
4861
exactly today's behaviour for every column. `AnalyticsServicePlugin` relays the
4962
key automatically, so a host on the plugin needs no change at all.
63+
64+
⚠️ **Superseded — see the banner at the top.** #17560 removed that member again in
65+
the same release window, so the shape a host writes against is the three-member one
66+
this paragraph calls today's. Nothing to do either way: a host that returns the
67+
fourth key is ignored, not refused.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
"@objectstack/spec": minor
4+
---
5+
6+
feat(service-analytics)!: `min` and `max` are judged by the aggregate × field-type table too — all 74 refused pairs answer `400 DATASET_INVALID` through one compile door (#17560)
7+
8+
<!-- adr-0087: registered dataset-measure-selecting-aggregate-field-type-refused -->
9+
10+
**BREAKING** — an accept-set narrowing on a published authoring surface, and the last
11+
one this table owed. A dataset measure pairing `aggregate: 'min'` (or `'max'`) with any
12+
of the **37** field types outside the numeric, temporal and boolean classes — for example
13+
`text`, `select`, `lookup`, `autonumber`, `json`, `multiselect`, `file`, `location`,
14+
`vector` or `formula`; the ADR-0087 entry registered below carries the full list — used to
15+
compile and reach the backend; it is now refused by
16+
`compileDataset` with `DATASET_INVALID` / **400** before any query is built. Shipped as
17+
`minor` under the repo's launch-window convention for accept-set narrowings.
18+
19+
⛔ This changeset adds no rows to any table and restates none. The verdict is
20+
`AGGREGATE_FIELD_TYPE_COMPATIBILITY`'s — the one table `@objectstack/spec` declared in
21+
#16353 under the director ruling of decision batch #59 ("both legs, table in spec") —
22+
read through `isAggregateCompatibleWithFieldType`.
23+
24+
## What was wrong
25+
26+
The table refused these 74 pairs from the day it was declared, and **four declarations
27+
gave three different answers about them**:
28+
29+
| declaration | what it said about `min` × `text` |
30+
|---|---|
31+
| `AGGREGATE_FIELD_TYPE_COMPATIBILITY` (spec) | refused |
32+
| `dataset-compiler`'s compile leg | never judged — `if (!DERIVING_AGGREGATES.has(aggregate)) return;` |
33+
| `measureResultType` (service-analytics, #15768) | a supported `'string'` result |
34+
| two shipped test files, in prose | "ruled C — the table is to be AMENDED to accept it" |
35+
36+
Driven through the real service door before anything was written, `min` / `max` over 13
37+
sampled refused pairs all compiled and emitted SQL, with `avg` × `datetime` as the
38+
firing control (refused, `DATASET_INVALID` / 400, no SQL) — so the zero was a reading of
39+
the tree rather than of a blind harness.
40+
41+
The fourth row had nothing behind it. The card it cited (#17513) is closed as a
42+
duplicate carrying zero rulings, and the one recorded ruling on this table says the
43+
opposite. ⇒ The director ruling of decision batch #127 (2026-09-13) settled all three
44+
sub-questions in one pass, because one shared fixture drove members of both halves:
45+
46+
1. **the string classes** (42 pairs) stay refused, as batch #59 ruled — ⛔ the table is
47+
not amended;
48+
2. **the non-string classes** (32 pairs) are refused **and enforced**;
49+
3. **`formula`** is refused on the table's own storage ground — it is VIRTUAL in SQL
50+
storage, no column is emitted, so no aggregate can be lowered to it whatever
51+
`returnType` says.
52+
53+
The divergence is real, and for these two aggregates it is the **ORDER** rather than the
54+
arithmetic: string order is collation-dependent, so two backends answer two different
55+
"smallest" values for one metadata document, and `min(jsonb)` does not exist on
56+
PostgreSQL at all.
57+
58+
## What changed
59+
60+
- **`dataset-compiler`**: the scope condition is gone. `assertAggregateFieldTypeCompatible`
61+
judges all six `AggregationFunction` members against the table, through the same
62+
`DATASET_INVALID` / 400 door. The refusal message names the divergence its own
63+
aggregate class really has (`min` / `max` SELECT a stored value and diverge on order;
64+
`sum` / `avg` DERIVE a number and diverge on arithmetic) and prescribes accordingly.
65+
- **`measureResultType`** asks `isAggregateCompatibleWithFieldType` before it answers, so
66+
the rule and the table agree **by construction**. Its `STRING_SOURCE_FIELD_TYPES`
67+
branch and its `formula` branch are retired with them; `min` / `max` over the temporal
68+
class still answers `'time'`, unchanged.
69+
- **`AnalyticsServiceConfig.sourceFieldMeta`** no longer declares `returnType`. It was
70+
carried (#16236) for one reader — the retired `formula` branch — and a declared input
71+
nobody consumes is the declared-not-enforced shape Prime Directive #10 refuses.
72+
73+
⚠️ **That key was never released, so against every published version this removal is a
74+
no-op.** #16236 is still a pending changeset in the same release window as this one;
75+
the last published entry (17.4.0) says in as many words that `FieldSchema.returnType`
76+
"is not on `AnalyticsServiceConfig.sourceFieldMeta`'s return shape". The key was
77+
therefore added and removed inside one window and no published tarball ever carried it.
78+
79+
**Host fix, one line:** drop `returnType` from whatever your `sourceFieldMeta` returns.
80+
You do not have to — the hook is a function RETURN position, so an extra key is not an
81+
excess-property error and is simply ignored at runtime — but keeping it declares an
82+
input nothing reads. Hosts on `AnalyticsServicePlugin` need no change at all: the plugin
83+
stopped relaying the key in this same change.
84+
85+
## FROM → TO, and the one-line fix
86+
87+
| you wrote | write instead |
88+
|---|---|
89+
| `{ aggregate: 'min' \| 'max', field: <a text/select/lookup/autonumber field> }` | `count` / `count_distinct` if you were counting; a **sort** on the list/report if you wanted the first or last RECORD |
90+
| `{ aggregate: 'min' \| 'max', field: <a json/multiselect/file/location/vector field> }` | store the quantity you meant as a numeric or temporal field and aggregate that |
91+
| `{ aggregate: 'min' \| 'max', field: <a formula field> }` | a formula emits no column; aggregate the stored field the formula reads, or persist the computed value |
92+
93+
⚠️ **Untouched:** those field types used as a **DIMENSION** (grouping, labelling,
94+
bucketing, filtering), `count` / `count_distinct` over any type, `min` / `max` over the
95+
numeric, temporal and boolean classes, and every `sum` / `avg` row #16778 and #16099
96+
already settled. The refusal also still stands down rather than guessing wherever the
97+
declared type cannot be resolved: no `sourceFieldMeta` wired, an unknown field, or a
98+
`relationship.field` path whose column lives on a joined object.
99+
100+
⚠️ The hand-migration prescription ships as the ADR-0087 semantic TODO registered above,
101+
which names the measure and the field type per affected pair — no lossless conversion
102+
exists, because nothing can compute "the smallest text value" in a way every backend
103+
agrees on.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec)!: the five `system/metrics.zod.ts` durations carry their unit in the key name (#17783, ruling A on #15939)
6+
7+
<!-- adr-0087: registered system-metrics-jsdoc-durations-unit-in-key -->
8+
9+
**BREAKING** — the five metrics durations whose unit was stated only in a source JSDoc now carry
10+
it in the key name, and each published `.describe()` states it too.
11+
12+
| def | before | after |
13+
|:--|:--|:--|
14+
| `MetricDefinition` | `summary.maxAge: 600` | `summary.maxAgeSeconds: 600` |
15+
| `ServiceLevelObjective` | `errorBudget.burnRateWindows[].window: 3600` | `errorBudget.burnRateWindows[].durationSeconds: 3600` |
16+
| `MetricExportConfig` | `interval: 60` | `intervalSeconds: 60` |
17+
| `MetricsConfig` | `collectionInterval: 15` | `collectionIntervalSeconds: 15` |
18+
| `MetricsConfig` | `retention.period: 604800` | `retention.durationSeconds: 604800` |
19+
20+
Every value is seconds, exactly as before, and every default (600, 3600 as authored, 60, 15,
21+
604800) is unchanged.
22+
23+
## Migration
24+
25+
```diff
26+
summary: {
27+
- maxAge: 600,
28+
+ maxAgeSeconds: 600,
29+
}
30+
31+
errorBudget: {
32+
- burnRateWindows: [{ window: 3600, threshold: 14.4 }],
33+
+ burnRateWindows: [{ durationSeconds: 3600, threshold: 14.4 }],
34+
}
35+
36+
exports: [{
37+
type: 'prometheus',
38+
- interval: 60,
39+
+ intervalSeconds: 60,
40+
}],
41+
- collectionInterval: 15,
42+
+ collectionIntervalSeconds: 15,
43+
retention: {
44+
- period: 604800,
45+
+ durationSeconds: 604800,
46+
},
47+
```
48+
49+
Rename the keys. Nothing else on these four defs moves, and the three same-named objects on this
50+
file — `MetricAggregationConfig.window`, `ServiceLevelIndicator.window` and
51+
`ServiceLevelObjective.period` — are untouched.
52+
53+
## Why
54+
55+
Each key named its unit in a source JSDoc — "Max age of observations in seconds", "Window size in
56+
seconds", "Export interval in seconds", "Collection interval in seconds", "Retention period in
57+
seconds" — and nowhere else. Four of the five carried no `.describe()` at all and the fifth read
58+
"Window size", so the text `content/docs/references/system/metrics.mdx` publishes named no unit:
59+
600, 3600, 60, 15 and 604800 are each a plausible number of seconds and a plausible number of
60+
milliseconds, and nothing on the page decided between them. Executes director-seat ruling A on
61+
#15939 (2026-09-11, maintainer 「同意」, decision batch #115), the per-file remediation of the
62+
#14478 rule — under that rule, moving the unit into the describe alone is itself a violation (unit
63+
in prose, none in the name), so each key is renamed and its describe corrected together.
64+
65+
Three of the five new names are deliberately **not** the mechanical suffix, and this file supplied
66+
the reason for each:
67+
68+
- `burnRateWindows[].window`**`durationSeconds`**, not `windowSeconds`. It is the fourth window
69+
length on this file, and #15679 already settled that a window length here reads `durationSeconds`
70+
so the measurements read alike. `windowSeconds` would stutter against the enclosing
71+
`burnRateWindows` array — the same objection #15679 recorded against `window.windowSeconds` — and
72+
on this tree `windowSeconds` is not an authorable key at all: its only key-position occurrence is
73+
an alias-map entry in `ServerRateLimitConfigSchema` that maps the spelling *away* to `windowMs`.
74+
- `retention.period`**`durationSeconds`**, not `periodSeconds`. `period` is calendar vocabulary
75+
elsewhere in this spec (`ServiceLevelObjective.period.type` selects rolling or calendar,
76+
`PluginRegistryEntry.pricing.billingPeriod` is monthly or yearly), so `periodSeconds` would have
77+
kept the ambiguous half of the name — the same objection #15679 raised against `sizeSeconds`.
78+
- `collectionInterval`**`collectionIntervalSeconds`**, keeping the qualifier, because
79+
`MetricExportConfig.intervalSeconds` is a different cadence one def over that this same change
80+
creates.
81+
82+
The two mechanical spellings are attested: `maxAgeSeconds` is the token
83+
`AccessControlConfig.maxAgeSeconds` already carries after this same rule renamed it on
84+
`system/object-storage.zod.ts`, and it keeps the `age` stem that the sibling `ageBuckets` counts
85+
buckets of; `intervalSeconds` is the token four seconds-valued cadences already carry. Counted in
86+
key position across `packages/spec/src` at `fc28c1d38`, the base of this change, the seconds
87+
suffixes run `Seconds` 40, `Sec` 1 (`maxExecutionTimeSec`) and `S` 0 — the two bare `*S` keys on
88+
that corpus, `maxCommitTimeMS` and `enableRLS`, are a millisecond spelling and a boolean. This
89+
change takes `Seconds` to 45 at `9b62f54671`.
90+
91+
## The kit
92+
93+
- a `retiredKey()` tombstone on each old spelling, so `tsc` types it `never` and a value reaching
94+
the parse raises the rename prescription instead of being silently stripped (none of the five
95+
enclosing shapes is `.strict()`)
96+
- the ADR-0087 D3 semantic entry `system-metrics-jsdoc-durations-unit-in-key` and five
97+
`RETIRED_KEYS_BY_MAJOR[18]` rows. No D2 conversion: `stack.zod.ts` declares no metrics collection
98+
and none of these defs is a stored metadata row — the reading
99+
`system-metrics-window-durations-unit-in-key` already recorded for this file
100+
- pin tests per key: the refusal carries the rename prescription and is not an `unrecognized_keys`
101+
issue, the suffixed key parses at the magnitude the retired one carried with the same default,
102+
and each describe publishes the unit
103+
- two authorable-surface rows move, three do not: that ratchet records **top-level** keys per def,
104+
so `MetricExportConfig:interval` and `MetricsConfig:collectionInterval` become `[RETIRED]` beside
105+
their suffixed rows (and their `authorable-defaults` rows move with them), while
106+
`summary.maxAge`, `burnRateWindows[].window` and `retention.period` are nested and move nothing

.changeset/dataset-measure-aggregate-field-type-refused.md

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,25 +60,45 @@ schema, not `os validate` / `os lint`, not the analytics service, not the render
6060

6161
## ⚠️ Scope: the compile leg executes the TEMPORAL rows only
6262

63+
> ⚠️ **Superseded within the same release window.** This section was accurate when it was
64+
> written and is kept as the record of where the compile leg stopped. Two later cards
65+
> widened it before any of the three entries shipped, so at the version that compiles this
66+
> entry the scope below is no longer the platform's: **#16099** judged `sum` / `avg` over
67+
> every remaining field class (including `sum` over a `percent`), and **#17560** (director
68+
> ruling, decision batch #127, 2026-09-13) judged `min` / `max` over every class the table
69+
> refuses. ⇒ Three sentences in this section are false at that version and are corrected
70+
> where they stand: the string rows are **not** awaiting a table amendment, `sum` over a
71+
> `percent` does **not** compile as it did before, and `avg` / `sum` over a temporal field
72+
> are **not** the only pairs whose behaviour changes. Read all three entries together.
73+
6374
The gate judges only a measure whose field is declared `date` / `datetime` /
6475
`time`; a field of any other class is never handed to the predicate. The
6576
verdict for the pairs it does judge is the table's — no row is restated — but
6677
which FIELDS are judged is narrower than the table, on purpose:
6778

6879
- **String rows** (`min` / `max` over `text`, `select`, `lookup`,
69-
`autonumber`, …) are **not enforced here**. They are under #16785, **ruled
70-
C**: the table itself is to be amended to accept them, because
71-
`measureResultType` (#15768) already types those results as `'string'` and
72-
pins them end to end. Enforcing them from this card would pre-empt that
73-
ruling.
80+
`autonumber`, …) are **not enforced here**. ⚠️ This card recorded them as
81+
「under #16785, **ruled C** — the table itself is to be amended to accept
82+
them」, because `measureResultType` (#15768) already typed those results as
83+
`'string'` and pinned them end to end, so enforcing them from here would
84+
pre-empt that ruling. **Both halves of that sentence turned out to be
85+
wrong.** `16785` resolves to no issue, and decision batch #127 (#17560,
86+
2026-09-13) found no ruling C anywhere behind the citation — the one recorded
87+
ruling on this table, decision batch #59, refuses the string rows. ⛔ The
88+
table is **not** amended; #17560 enforces those rows and retires the
89+
`measureResultType` opinion that disagreed with them.
7490
- **Boolean rows** are not a refusal at all any more: #16685 was ruled A and
7591
#16750 added `boolean` / `toggle` to `sum` / `avg` / `min` / `max`, so the
7692
table ACCEPTS them and this gate never judged them.
7793
- The table's `sum` × `percent` row is likewise **not** executed by this leg;
78-
`sum` over a `percent` compiles exactly as it did before.
79-
80-
⇒ The only pairs whose behaviour changes in this release are `avg` / `sum`
81-
over a `date` / `datetime` / `time` field. The full-table leg remains #16099's.
94+
`sum` over a `percent` compiles exactly as it did before. ⚠️ True of this
95+
card only — #16099 executes that row in the same release.
96+
97+
⇒ The only pairs whose behaviour changes **because of this card** are `avg` /
98+
`sum` over a `date` / `datetime` / `time` field. ⚠️ ⛔ Not a statement about the
99+
release: the full-table leg is #16099's and landed, and the `min` / `max` leg is
100+
#17560's and landed, so at the shipping version every pair the table refuses is
101+
refused at the compile door.
82102

83103
## FROM → TO
84104

.changeset/deriving-aggregate-nonnumeric-field-refused.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,22 @@ answer was a property of the dialect rather than of the data — the shape Prime
5656
measure never finishes compiling), and the three "cannot answer, do not block" tiers —
5757
no `sourceFieldMeta`, an unresolvable field, a `relationship.field` path.
5858

59-
## ⚠️ Scope: the DERIVING aggregates. `min` / `max` are still not judged here
59+
## ⚠️ Scope: the DERIVING aggregates — and see #17560, which closed the other half
60+
61+
> ⚠️ **Superseded within the same release window.** This section was accurate when it was
62+
> written and is kept as the record of why this change stopped where it did. #17560
63+
> (director ruling, decision batch #127, 2026-09-13) then judged `min` / `max` too, so at
64+
> the version that ships this entry **every** pair the table refuses is refused at the
65+
> compile door. Read that entry beside this one.
6066
6167
`min` / `max` SELECT one of the stored values; `sum` / `avg` DERIVE a number. This is the
6268
line this package already draws — `measureResultType` branches on exactly that pair of
6369
aggregates — and the defect is about a derived number, so the deriving aggregates are its
6470
population.
6571

66-
The `min` / `max` rows stay with **#17513**, and that is measured rather than assumed.
72+
The `min` / `max` rows stayed with the table-amendment card (then **#17513**, since closed
73+
as a duplicate of **#17560**, which ruled and landed them), and that is measured rather
74+
than assumed.
6775
Enforcing the residual whole was tried on this card: with `min` / `max` × the string
6876
classes subtracted, **15** cases in `measure-result-type.test.ts` still went red, every
6977
one of them on `min` × `json` — a pair the table refuses, in no ruling's scope, driven

0 commit comments

Comments
 (0)