From ad1b5dde32011aa7ad827a1de6150c68facc12e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:34:17 +0000 Subject: [PATCH 1/4] feat(service-analytics)!: refuse an aggregate a measure's field type cannot carry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WIP — compile-leg refusal + the four reconciled datetime-storage annotations. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...aggregate-datetime-measure-refusal.test.ts | 459 ++++++++++++++++++ .../native-sql-datetime-filter-column.test.ts | 37 +- .../native-sql-datetime-filter.test.ts | 79 ++- .../src/analytics-service.ts | 69 ++- .../service-analytics/src/dataset-compiler.ts | 116 ++++- .../services/service-analytics/src/plugin.ts | 31 +- .../src/strategies/native-sql-strategy.ts | 33 +- .../src/strategies/objectql-strategy.ts | 13 +- 8 files changed, 785 insertions(+), 52 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts diff --git a/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts new file mode 100644 index 0000000000..ada19e658a --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts @@ -0,0 +1,459 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16737 — an aggregate a `Field.datetime` measure cannot carry is REFUSED at + * compile time, and `derived` can no longer be handed its output. + * + * ## The shape, re-driven here rather than quoted + * + * `{ aggregate: 'avg', field: 'submitted_at' }` over a `Field.datetime` + * compiled to `AVG(submitted_at)` and reached the backend. What came back was + * decided by the dialect, not by the data — and the SQLite half is the + * dangerous one, because it SUCCEEDS: + * + * ``` + * -- better-sqlite3, and re-driven live in the first suite below via sql.js + * select typeof(submitted_at), submitted_at from clm_contract limit 1; + * text|2026-05-19T00:00:00.000Z -- ONE canonical storage form (#3912) + * select avg(submitted_at) from clm_contract; + * 2025.5 -- text->numeric coercion: the average YEAR + * + * -- PostgreSQL 16.13, measured on this card + * select avg(submitted_at) from t; + * ERROR: function avg(timestamp with time zone) does not exist -- SQLSTATE 42883 + * ``` + * + * ⭐ **The danger is the shape, not the magnitude.** `2025.5` fed to + * `derived: { op: 'difference', of: [avg_a, avg_b] }` renders as `-0.85` on a + * tile labelled "average cycle time delta" — indistinguishable from a correct + * answer. Which half is dialect-specific: the SILENT half is SQLite's (a + * text→numeric coercion no other dialect performs on a temporal column); the + * MEANINGLESS half is not — there is no backend on which the mean of a set of + * instants is a duration. + * + * ## Why a refusal and not a definition + * + * The director ruling (decision batch #59, 2026-09-06, on #16099) settled it as + * a contract rather than an implementation choice: ONE compatibility table in + * `@objectstack/spec` (`AGGREGATE_FIELD_TYPE_COMPATIBILITY`, #16353), consumed + * by two refusal legs. This file pins the COMPILE leg. ⛔ Nothing here restates + * the table's rows — the pairs asserted below are read from the shipped table, + * so a row changed upstream changes these expectations with it rather than + * leaving a second, drifting account of the contract. + * + * ## Dissolution verification — direction predicted BEFORE running + * + * Deleting the `assertAggregateFieldTypeCompatible` call in + * `dataset-compiler.ts`'s measure loop must turn the REFUSAL cases red in the + * ordinary direction: each asserts the ADR-0112 envelope (`code` + `status`), + * the offending pair in the message, AND that nothing reached the driver + * (`sqls` empty) — with the gate gone the compile succeeds, SQL IS emitted, so + * no case can pass vacuously. Every NEGATIVE control (numeric `avg`, the + * datetime DIMENSION, `min`/`max`/`count` over a datetime, the three + * cannot-answer tiers) is predicted to stay GREEN in both directions: none of + * them reaches the refusing branch. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { + AGGREGATE_FIELD_TYPE_COMPATIBILITY, + isAggregateCompatibleWithFieldType, +} from '@objectstack/spec/data'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { AnalyticsService } from '../analytics-service.js'; + +// ───────────────────────────────────────────────────────────────────────────── +// The storage reality, re-driven — not quoted from the card (#16737 item 1) +// ───────────────────────────────────────────────────────────────────────────── + +/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */ +async function locateWasm(): Promise<((file: string) => string) | undefined> { + try { + const { createRequire } = await import('node:module'); + const require = createRequire(import.meta.url); + const pkgJsonPath = require.resolve('sql.js/package.json'); + const { dirname, join } = await import('node:path'); + const dir = dirname(pkgJsonPath); + return (file: string) => join(dir, 'dist', file); + } catch { + return undefined; + } +} + +describe('#16737 — what SQLite actually does with an aggregate over a datetime column', () => { + let db: any; + + beforeAll(async () => { + const mod: any = await import('sql.js'); + const initSqlJs = mod.default ?? mod; + const locateFile = await locateWasm(); + const SQL = await initSqlJs(locateFile ? { locateFile } : undefined); + db = new SQL.Database(); + // The CANONICAL storage form a `Field.datetime` column holds on SQLite + // since #3912 — `YYYY-MM-DDTHH:MM:SS.sssZ` text, one form for every write + // path. `cycle_days` is the numeric control. + db.run(`CREATE TABLE "clm_contract" ("id" TEXT PRIMARY KEY, "submitted_at" TEXT, "cycle_days" REAL);`); + const insert = db.prepare(`INSERT INTO "clm_contract" VALUES (?,?,?)`); + insert.run(['c1', '2026-05-19T00:00:00.000Z', 30]); + insert.run(['c2', '2025-01-19T00:00:00.000Z', 10]); + insert.free(); + }); + + afterAll(() => db?.close()); + + const scalar = (sql: string): unknown => { + const stmt = db.prepare(sql); + stmt.step(); + const row = stmt.getAsObject(); + stmt.free(); + return Object.values(row)[0]; + }; + + it('stores the canonical UTC TEXT form — not an INTEGER epoch', () => { + expect(scalar(`select typeof(submitted_at) from clm_contract limit 1`)).toBe('text'); + expect(scalar(`select submitted_at from clm_contract where id = 'c1'`)) + .toBe('2026-05-19T00:00:00.000Z'); + }); + + it('⭐ AVG over that column returns the average YEAR, silently', () => { + // This is the defect, driven rather than recalled: SQLite coerces the text + // to a number by reading its leading digits, so the "average submission + // time" of 2026-05 and 2025-01 is the mean of 2026 and 2025. + const avg = scalar(`select avg(submitted_at) from clm_contract`); + expect(typeof avg).toBe('number'); + expect(avg).toBeCloseTo(2025.5, 10); + // …and the difference of two such numbers is the "clean plausible number" + // the card names: nothing about `0.5` says it is a difference of years. + expect(Number(avg) - 2025).toBeCloseTo(0.5, 10); + }); + + it('a genuine numeric column averages correctly — the control that keeps the above meaningful', () => { + expect(scalar(`select avg(cycle_days) from clm_contract`)).toBe(20); + }); + + it('min/max over the same column return real instants — which is why they stay accepted', () => { + expect(scalar(`select min(submitted_at) from clm_contract`)).toBe('2025-01-19T00:00:00.000Z'); + expect(scalar(`select max(submitted_at) from clm_contract`)).toBe('2026-05-19T00:00:00.000Z'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// The refusal, through the real service +// ───────────────────────────────────────────────────────────────────────────── + +const FIELD_TYPES: Record = { + submitted_at: 'datetime', + approved_at: 'datetime', + close_date: 'date', + cycle_days: 'number', + amount: 'currency', +}; + +/** + * A service wired the way a host wires it: `sourceFieldMeta` answers the + * declared type, and every emitted statement is recorded so a refusal can be + * shown to have happened BEFORE the driver — the assertion that stops a case + * from passing on a query that merely returned nothing. + */ +function makeService(rows: Array> = [], opts?: { noFieldMeta?: boolean }) { + const sqls: string[] = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async (_object: string, sql: string) => { + sqls.push(sql); + return rows; + }, + ...(opts?.noFieldMeta + ? {} + : { sourceFieldMeta: (_o: string, f: string) => (FIELD_TYPES[f] ? { type: FIELD_TYPES[f] } : undefined) }), + }); + return { svc, sqls }; +} + +/** + * The ObjectQL profile, for the two DIMENSION controls. `NativeSQLStrategy` + * declines any query carrying a `granularity`, so a date-BUCKETED dimension is + * served by the driver-independent path on every driver — which is exactly the + * shape a "new contracts by month" chart takes, and therefore the shape the + * dimension controls have to exercise. + */ +function makeBucketService(rows: Array> = []) { + const calls: unknown[] = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (...args: unknown[]) => { + calls.push(args); + return rows; + }, + sourceFieldMeta: (_o: string, f: string) => (FIELD_TYPES[f] ? { type: FIELD_TYPES[f] } : undefined), + } as never); + return { svc, calls }; +} + +const dataset = (measures: unknown[], dimensions: unknown[] = [{ name: 'status', field: 'status', type: 'string' }]) => + DatasetSchema.parse({ + name: 'contract_cycle', + label: 'Contract cycle', + object: 'clm_contract', + include: [], + dimensions, + measures, + }); + +/** The ADR-0112 envelope a caller-shaped dataset refusal must carry. */ +async function refusalOf(fn: () => Promise): Promise { + try { + await fn(); + } catch (e) { + return e as Error & { code?: string; status?: number }; + } + throw new Error('expected a refusal, none was thrown'); +} + +describe('#16737 — the compile leg refuses an aggregate the field type cannot carry', () => { + it('the contract this executes says so: `avg` × `datetime` is not an accepted pair', () => { + // Read from the shipped table rather than restated — if the ruling changes + // this row, this file changes with it instead of contradicting it. + expect(isAggregateCompatibleWithFieldType('avg', 'datetime')).toBe(false); + expect(isAggregateCompatibleWithFieldType('sum', 'datetime')).toBe(false); + expect(isAggregateCompatibleWithFieldType('avg', 'number')).toBe(true); + expect(isAggregateCompatibleWithFieldType('min', 'datetime')).toBe(true); + expect(AGGREGATE_FIELD_TYPE_COMPATIBILITY.avg).not.toContain('datetime'); + }); + + it('⭐ AVG over a datetime measure is refused — 400 DATASET_INVALID, before any SQL', async () => { + const { svc, sqls } = makeService(); + const err = await refusalOf(() => + svc.queryDataset( + dataset([{ name: 'avg_submitted', aggregate: 'avg', field: 'submitted_at', label: 'Avg submitted' }]), + { dimensions: ['status'], measures: ['avg_submitted'] }, + ), + ); + expect(err.code).toBe('DATASET_INVALID'); + expect(err.status).toBe(400); + expect(err.message).toContain('avg_submitted'); + expect(err.message).toContain('submitted_at'); + expect(err.message).toContain('datetime'); + // The refusal is a COMPILE-time verdict: the driver was never asked. + expect(sqls).toEqual([]); + }); + + it('SUM over a datetime measure is refused on the same envelope', async () => { + const { svc, sqls } = makeService(); + const err = await refusalOf(() => + svc.queryDataset( + dataset([{ name: 'sum_submitted', aggregate: 'sum', field: 'submitted_at' }]), + { dimensions: ['status'], measures: ['sum_submitted'] }, + ), + ); + expect(err.code).toBe('DATASET_INVALID'); + expect(err.status).toBe(400); + expect(sqls).toEqual([]); + }); + + it('a `date` field is refused for the same reason — the table, not a datetime special case', async () => { + const { svc } = makeService(); + const err = await refusalOf(() => + svc.queryDataset( + dataset([{ name: 'avg_close', aggregate: 'avg', field: 'close_date' }]), + { dimensions: ['status'], measures: ['avg_close'] }, + ), + ); + expect(err.code).toBe('DATASET_INVALID'); + expect(err.message).toContain('date'); + }); + + it('the message names the accepted set, read off the table rather than hand-written', async () => { + const { svc } = makeService(); + const err = await refusalOf(() => + svc.queryDataset( + dataset([{ name: 'avg_submitted', aggregate: 'avg', field: 'submitted_at' }]), + { dimensions: ['status'], measures: ['avg_submitted'] }, + ), + ); + for (const accepted of AGGREGATE_FIELD_TYPE_COMPATIBILITY.avg) { + expect(err.message).toContain(accepted); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// `derived` compounding — the half that made the nonsense presentable (item 4) +// ───────────────────────────────────────────────────────────────────────────── + +describe('#16737 — `derived` can no longer be handed a refused aggregate', () => { + const derivedDataset = () => + dataset([ + { name: 'avg_a', aggregate: 'avg', field: 'submitted_at' }, + { name: 'avg_b', aggregate: 'avg', field: 'approved_at' }, + { name: 'cycle_delta', label: 'Average cycle time delta', derived: { op: 'difference', of: ['avg_a', 'avg_b'] } }, + ]); + + it('⭐ selecting ONLY the derived measure is refused — the operands are pulled in, so hiding them does not help', async () => { + // This is the filer's exact shape: the two averages are never named by the + // selection, only the difference is. Before this card that difference came + // back as `-0.849999999999909` and rendered. + const { svc, sqls } = makeService(); + const err = await refusalOf(() => + svc.queryDataset(derivedDataset(), { dimensions: ['status'], measures: ['cycle_delta'] }), + ); + expect(err.code).toBe('DATASET_INVALID'); + expect(err.status).toBe(400); + // Named by the OPERAND that is wrong, not by the derived measure — the + // author fixes `avg_a`, and `cycle_delta` is not itself malformed. + expect(err.message).toContain('avg_a'); + expect(err.message).toContain('submitted_at'); + expect(sqls).toEqual([]); + }); + + it('and refused identically when the operands ARE selected', async () => { + const { svc, sqls } = makeService(); + const err = await refusalOf(() => + svc.queryDataset(derivedDataset(), { dimensions: ['status'], measures: ['avg_a', 'avg_b', 'cycle_delta'] }), + ); + expect(err.code).toBe('DATASET_INVALID'); + expect(sqls).toEqual([]); + }); + + it('a derived measure over NUMERIC operands is untouched — the refusal is about the operand, not about `derived`', async () => { + const { svc } = makeService([{ status: 'open', fast: 10, slow: 30 }]); + const result: any = await svc.queryDataset( + dataset([ + { name: 'fast', aggregate: 'avg', field: 'cycle_days' }, + { name: 'slow', aggregate: 'avg', field: 'amount' }, + { name: 'gap', derived: { op: 'difference', of: ['fast', 'slow'] } }, + ]), + { dimensions: ['status'], measures: ['gap'] }, + ); + expect(result.rows[0].gap).toBe(-20); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Negative controls (item 6) — what this card must NOT have touched +// ───────────────────────────────────────────────────────────────────────────── + +describe('#16737 negative controls — aggregation only, and only the incoherent pairs', () => { + it('AVG over a genuine numeric measure still works end to end', async () => { + const { svc, sqls } = makeService([{ status: 'open', avg_cycle: 20 }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'avg_cycle', aggregate: 'avg', field: 'cycle_days' }]), + { dimensions: ['status'], measures: ['avg_cycle'] }, + ); + expect(result.rows).toEqual([{ status: 'open', avg_cycle: 20 }]); + expect(sqls.length).toBe(1); + expect(sqls[0]).toContain('AVG'); + }); + + it('AVG over a `currency` measure still works — the numeric class, not just `number`', async () => { + const { svc } = makeService([{ status: 'open', avg_amount: 500 }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'avg_amount', aggregate: 'avg', field: 'amount' }]), + { dimensions: ['status'], measures: ['avg_amount'] }, + ); + expect(result.rows[0].avg_amount).toBe(500); + }); + + it('⭐ a `datetime` used as a DIMENSION is untouched — grouping', async () => { + const { svc, calls } = makeBucketService([{ submitted: '2026-05', row_count: 3 }]); + const result: any = await svc.queryDataset( + dataset( + [{ name: 'row_count', aggregate: 'count' }], + [{ name: 'submitted', field: 'submitted_at', type: 'date', dateGranularity: 'month' }], + ), + { dimensions: ['submitted'], measures: ['row_count'] }, + ); + expect(result.rows.length).toBe(1); + expect(result.rows[0].row_count).toBe(3); + // The grouping reached the engine keyed on the DATETIME column with its + // month bucket — the dimension path, untouched by this card's refusal. + expect(calls.length).toBe(1); + expect(JSON.stringify(calls[0])).toContain('submitted_at'); + expect(JSON.stringify(calls[0])).toContain('month'); + }); + + it('⭐ a `datetime` used as a DIMENSION is untouched — date-range filtering', async () => { + const { svc, calls } = makeBucketService([{ submitted: '2026-05', row_count: 3 }]); + const result: any = await svc.queryDataset( + dataset( + [{ name: 'row_count', aggregate: 'count' }], + [{ name: 'submitted', field: 'submitted_at', type: 'date', dateGranularity: 'month' }], + ), + { + dimensions: ['submitted'], + measures: ['row_count'], + timeDimensions: [{ dimension: 'submitted', dateRange: ['2026-01-01', '2026-12-31'] }], + }, + ); + expect(result.rows.length).toBe(1); + // The window reached the engine as a resolved bound on the datetime column + // — the dimension path this card must not have touched. + expect(JSON.stringify(calls[0])).toContain('2026-01-01'); + }); + + it('MIN and MAX over a datetime are still accepted — they return a value of the field’s own type', async () => { + const { svc, sqls } = makeService([ + { status: 'open', first_submitted: '2025-01-19T00:00:00.000Z', last_submitted: '2026-05-19T00:00:00.000Z' }, + ]); + const result: any = await svc.queryDataset( + dataset([ + { name: 'first_submitted', aggregate: 'min', field: 'submitted_at' }, + { name: 'last_submitted', aggregate: 'max', field: 'submitted_at' }, + ]), + { dimensions: ['status'], measures: ['first_submitted', 'last_submitted'] }, + ); + expect(result.rows[0].first_submitted).toBe('2025-01-19T00:00:00.000Z'); + expect(sqls.length).toBe(1); + }); + + it('COUNT over a datetime is still accepted — counting instants is still counting', async () => { + const { svc } = makeService([{ status: 'open', submitted_count: 2 }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'submitted_count', aggregate: 'count', field: 'submitted_at' }]), + { dimensions: ['status'], measures: ['submitted_count'] }, + ); + expect(result.rows[0].submitted_count).toBe(2); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// "Cannot answer, do not block" — the three tiers this gate stands down on +// ───────────────────────────────────────────────────────────────────────────── + +describe('#16737 — the gate stands down rather than guessing', () => { + it('no `sourceFieldMeta` wired (no data engine) → the pair is not judged', async () => { + const { svc, sqls } = makeService([{ status: 'open', avg_submitted: 2025.5 }], { noFieldMeta: true }); + const result: any = await svc.queryDataset( + dataset([{ name: 'avg_submitted', aggregate: 'avg', field: 'submitted_at' }]), + { dimensions: ['status'], measures: ['avg_submitted'] }, + ); + expect(result.rows.length).toBe(1); + expect(sqls.length).toBe(1); + }); + + it('a field the hook cannot resolve → not judged', async () => { + const { svc } = makeService([{ status: 'open', avg_mystery: 1 }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'avg_mystery', aggregate: 'avg', field: 'mystery_column' }]), + { dimensions: ['status'], measures: ['avg_mystery'] }, + ); + expect(result.rows.length).toBe(1); + }); + + it('a RELATIONSHIP-PATH field → not judged, because the hook answers about the base object', async () => { + // `account.submitted_at` is a column on `account`, not on `clm_contract`. + // Judging it from `sourceFieldMeta('clm_contract', …)` would be answering + // about a different column that happens to share a name. + const { svc } = makeService([{ status: 'open', avg_acct: 1 }]); + const rel = DatasetSchema.parse({ + name: 'contract_cycle_rel', + label: 'Contract cycle', + object: 'clm_contract', + include: ['account'], + dimensions: [{ name: 'status', field: 'status', type: 'string' }], + measures: [{ name: 'avg_acct', aggregate: 'avg', field: 'account.submitted_at' }], + }); + const result: any = await svc.queryDataset(rel, { dimensions: ['status'], measures: ['avg_acct'] }); + expect(result.rows.length).toBe(1); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts index 8ba11903c5..d3917e2bff 100644 --- a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts +++ b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts @@ -3,17 +3,26 @@ /** * Regression #3912 — the COLUMN half of the datetime storage-form fix. * - * `native-sql-datetime-filter.test.ts` covers coercing the comparand to epoch ms. - * That alone is not enough: a SQLite `Field.datetime` column holds an INTEGER - * epoch (a `Date` write) and ISO TEXT (a REST/JSON write, a `NOW()` default) at - * the same time, so an epoch comparand matches the INTEGER rows and misses every - * TEXT one — a dashboard `dateRange: last_30_days` reading 0 with rows in range. + * `native-sql-datetime-filter.test.ts` covers coercing the COMPARAND. That alone + * is not enough on the tier this hook exists for: a SQLite `Field.datetime` + * column written before the canonical convention, and not yet backfilled, holds + * an INTEGER epoch (a `Date` write) next to text (a REST/JSON write, a `NOW()` + * default) at the same time, so one comparand matches one half of the rows and + * misses the other — a dashboard `dateRange: last_30_days` reading 0 with rows + * in range. * - * The fix threads a companion `StrategyContext.coerceTemporalFilterColumn` hook - * that lets the driver normalise the column reference. These tests assert the - * strategy applies it to exactly the value comparisons, leaves the null and LIKE - * predicates on the raw column, and emits byte-identical SQL when the hook is - * absent (Postgres, non-SQL drivers, legacy wiring). + * ⛔ That mixed column is the TRANSITIONAL state, not what a write produces + * today; the storage reality is stated once, on + * `AnalyticsServiceConfig.coerceTemporalFilterValue` in `analytics-service.ts` + * (#16737). What these tests pin is the STRATEGY's half and is independent of + * it: whatever expression the driver hands back, it is applied to exactly the + * value comparisons, the null and LIKE predicates keep the raw column, and the + * SQL is byte-identical when the hook is absent or answers with the bare column + * (a converged SQLite column, Postgres, non-SQL drivers, legacy wiring). + * + * `EPOCH_MS(...)` below is therefore a MARKER, not a claim about emitted SQL: + * a hook return that is visibly different from the input column is what makes + * "was the hook applied here" decidable in an assertion. */ import { describe, it, expect } from 'vitest'; @@ -35,7 +44,13 @@ const cube: Cube = { public: false, }; -/** Stand-in for `SqlDriver.temporalFilterColumnSql` under better-sqlite3. */ +/** + * Stand-in for `SqlDriver.temporalFilterColumnSql` on an UN-BACKFILLED SQLite + * column — the one tier that still answers with a repair expression. The real + * driver emits a `case typeof(...)` CASE; `EPOCH_MS(...)` is a stand-in marker + * (see the module header) so the assertions read as "hook applied / not applied" + * rather than pinning a driver's SQL text from another package. + */ function sqliteColumnHook(object: string, field: string, columnSql: string): string { if (object === 'compliance_assessment' && field === 'assessed_at') { return `EPOCH_MS(${columnSql})`; diff --git a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts index e067ca555b..a3c8d68618 100644 --- a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts +++ b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts @@ -5,20 +5,37 @@ * `Field.datetime` dimension must NOT silently return zero rows. * * Root cause (confirmed): the analytics layer expands relative-date tokens like - * `{12_months_ago}` to ISO date strings (`"2025-06-18"`). Under better-sqlite3 a - * `Field.datetime` column is stored as an INTEGER epoch (ms), so the compiled - * `WHERE col >= '2025-06-18'` is a TEXT-vs-INTEGER affinity compare that is - * ALWAYS false → empty result, even though the data exists. `Field.date` columns - * store ISO TEXT and compare fine. + * `{12_months_ago}` to ISO date strings (`"2025-06-18"`), and the raw-SQL + * strategy binds them OUTSIDE the driver's builder, so nothing canonicalises + * them to the column's storage form. `WHERE col >= '2025-06-18'` then compares + * an unnormalised comparand against the stored value and is always false → + * empty result, even though the data exists. * * The fix threads the driver's storage-form coercion into NativeSQLStrategy via * `StrategyContext.coerceTemporalFilterValue`. These tests assert the strategy: - * 1. binds the epoch-ms value when the hook reports a datetime column (SQLite), - * 2. leaves the ISO string untouched when the hook reports no coercion + * 1. binds whatever the hook returns, VERBATIM and at the hook's own type, + * when the hook reports a datetime column, + * 2. leaves the comparand untouched when the hook reports no coercion * (a `Field.date` text column, OR a native-timestamp dialect like Postgres), * proving no Postgres regression, * 3. applies the same handling to `gte`/`lte`/`gt`/`lt`/`equals`, `in`, and the * `dateRange` (timeDimension) path. + * + * ## ⛔ Two storage forms appear below, and only one of them is current (#16737) + * + * This file's original narrative said "under better-sqlite3 a `Field.datetime` + * column is stored as an INTEGER epoch (ms)". #3912 retired that: a SQLite + * `Field.datetime` now has ONE storage form, canonical UTC TEXT, and the epoch + * survives only in a database written before the convention and not yet + * backfilled. The storage reality is stated once, on + * `AnalyticsServiceConfig.coerceTemporalFilterValue` in `analytics-service.ts`. + * + * The epoch-ms fixture is KEPT rather than re-spelled, because the property + * under test is that the strategy binds the hook's return verbatim — and an + * epoch hook is the only one that changes both the VALUE and its JS TYPE, which + * is what makes "verbatim" decidable. `canonicalTextHook` below adds today's + * real driver behaviour beside it, so the suite covers the live form as well as + * the one it was written against. */ import { describe, it, expect } from 'vitest'; @@ -44,8 +61,10 @@ const cube: Cube = { const EPOCH_2025_06_18 = Date.parse('2025-06-18T00:00:00.000Z'); /** - * A hook that mimics the SqlDriver-under-SQLite behaviour: ISO → epoch ms for the - * datetime column, value untouched for everything else. + * A LEGACY-form hook: ISO → epoch ms for the datetime column, value untouched + * for everything else. It is not what `SqlDriver` answers today (see the module + * header) — it is the type-changing return that makes "bound verbatim" + * observable. */ function sqliteHook(object: string, field: string, value: unknown): unknown { if (object === 'compliance_assessment' && field === 'assessed_at' && typeof value === 'string') { @@ -55,6 +74,20 @@ function sqliteHook(object: string, field: string, value: unknown): unknown { return value; // date text / non-temporal / native timestamp → unchanged } +/** + * [#16737] Today's `SqlDriver`-under-SQLite behaviour: a `Field.datetime` + * comparand is canonicalised to the ONE stored form, canonical UTC text + * (`YYYY-MM-DDTHH:MM:SS.sssZ`, #3912) — a bare calendar day becoming UTC + * midnight. Same contract as {@link sqliteHook}, current spelling. + */ +function canonicalTextHook(object: string, field: string, value: unknown): unknown { + if (object === 'compliance_assessment' && field === 'assessed_at' && typeof value === 'string') { + const ms = Date.parse(/^\d{4}-\d{2}-\d{2}$/.test(value) ? `${value}T00:00:00.000Z` : value); + return Number.isFinite(ms) ? new Date(ms).toISOString() : value; + } + return value; +} + function ctxWith(overrides: Partial): StrategyContext { return { getCube: (name) => (name === 'compliance' ? cube : undefined), @@ -77,12 +110,36 @@ describe('NativeSQLStrategy — datetime filter storage coercion', () => { const { sql, params } = await strategy.generateSql(query, ctx); expect(sql).toContain('assessed_at >= $1'); - // The ISO string was converted to its INTEGER epoch storage form — this is - // the exact value that matches the stored datetime and fixes "No rows". + // The hook's return is bound verbatim, at the hook's own JS type — the + // strategy adds no interpretation of its own. (This assertion used to be + // described as "the value that matches the stored datetime"; that is the + // DRIVER's claim to make, not this suite's — see the module header.) expect(params).toEqual([EPOCH_2025_06_18]); expect(typeof params[0]).toBe('number'); }); + it('binds the CANONICAL UTC text a modern SQLite driver returns (#16737 — the live storage form)', async () => { + // The sibling of the epoch case above, against what `SqlDriver` actually + // answers on `origin/main`. Both prove the same property — the hook's + // return is bound verbatim — so the strategy is correct for the storage + // form the driver has TODAY, not only for the one this file was written + // against. A bare calendar day arrives as UTC midnight, in full canonical + // spelling, and stays a string. + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: canonicalTextHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18' } }, + }; + + const { sql, params } = await strategy.generateSql(query, ctx); + + expect(sql).toContain('assessed_at >= $1'); + expect(params).toEqual(['2025-06-18T00:00:00.000Z']); + expect(typeof params[0]).toBe('string'); + }); + it('leaves the ISO string untouched when the hook reports no coercion (Postgres / date text — no regression)', async () => { const strategy = new NativeSQLStrategy(); // Hook present but returns the value unchanged for this column — the contract diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index a51e7f0fb4..0a90642c0f 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -480,19 +480,65 @@ export interface AnalyticsServiceConfig { getAllowedRelationships?: (cubeName: string) => Set | undefined; /** * Coerce a filter comparand to a temporal column's storage form so a - * relative-date / ISO-string value compares correctly on the active driver - * (SQLite `Field.datetime` → epoch ms; `Field.date` / native timestamp → - * unchanged). Threaded into the StrategyContext and consulted by - * `NativeSQLStrategy` when binding filter values. See the contract docs on + * relative-date / ISO-string value compares correctly on the active driver. + * Threaded into the StrategyContext and consulted by `NativeSQLStrategy` when + * binding filter values. See the contract docs on * `StrategyContext.coerceTemporalFilterValue` for the full rationale. + * + * # ⭐ The storage reality this package coerces against — ONE statement (#16737) + * + * ⛔ This block is the SINGLE place in `service-analytics` that states what a + * `Field.datetime` column physically holds. Every other site that needs the + * fact links here instead of restating it — {@link + * AnalyticsServiceConfig.coerceTemporalFilterColumn} below, the two hook + * bridges in `plugin.ts`, `NativeSQLStrategy.temporalColumn` / + * `buildFilterClause`, and `ObjectQLStrategy.dateRangeBounds`. Four of those + * carried three mutually incompatible accounts of it before this card, which + * is what #16737 was filed to end. + * + * Measured on `origin/main` (`driver-sql`, better-sqlite3), not recalled: + * + * - **SQLite `Field.datetime` has ONE storage form: canonical UTC TEXT**, + * `YYYY-MM-DDTHH:MM:SS.sssZ` (#3912/#3928). `SqlDriver.storageDatetimeValue` + * canonicalises on the WRITE path, so every accepted input shape folds onto + * the same stored string — a `Date`, an ISO `…Z`, an ISO with an offset, a + * naive wall clock (ADR-0074), an epoch number, an epoch string, a bare + * calendar day — and the `NOW()` column default writes the same + * `strftime('%Y-%m-%dT%H:%M:%fZ', 'now')` bytes. A value the driver cannot + * interpret is preserved verbatim rather than nulled. + * - **⛔ The INTEGER epoch is a LEGACY form, not a live write path.** It is + * what a database written BEFORE the convention still holds. `initObjects` + * runs `backfillCanonicalDatetimes` at schema sync to converge such a + * database; `SqlDriver.needsLegacyDatetimeRepair` is the ONE predicate for + * "might this column still hold a pre-canonical value", and it is the only + * thing that makes the column hook below emit anything but the bare column. + * Two cases keep it true: a table not yet backfilled, and an EXTERNAL / + * unmanaged object (`registerExternalObject` never marks its datetime + * columns canonical). + * - **Postgres and MySQL never enter that question at all**: the DDL gives + * them a real temporal type (`timestamptz`, `DATETIME(3)`), so there is one + * on-disk shape by construction and nothing to repair. + * + * ⇒ The mixed INTEGER/TEXT column #3912 fixed is TRANSITIONAL, not the steady + * state, and the flat "a SQLite `Field.datetime` IS an INTEGER epoch" (#2034) + * has been wrong since #3912 landed. Both hooks stay necessary regardless: + * the comparand still has to be canonicalised (an author writes + * `'2025-06-18'`, storage holds `'2025-06-18T00:00:00.000Z'`), and the column + * still has to be repaired on the un-migrated and external tiers. */ coerceTemporalFilterValue?: (objectName: string, fieldName: string, value: unknown) => unknown; /** * Normalise the COLUMN side of the same comparison to that storage form — the - * other half of the fix, needed because a SQLite `Field.datetime` holds both an - * INTEGER epoch (a `Date` write) and ISO TEXT (a REST/JSON write, a `NOW()` - * default) at once, so coercing only the comparand matches one of them and - * misses the other (#3912). See `StrategyContext.coerceTemporalFilterColumn`. + * other half of #3912, and the half that is now conditional. + * + * The driver answers with the bare column on a converged SQLite column and on + * every non-SQLite dialect, and with a repair expression only while + * `needsLegacyDatetimeRepair` holds — see the storage-reality block on + * {@link AnalyticsServiceConfig.coerceTemporalFilterValue} above, which is + * where that fact is stated once. Coercing the comparand alone is therefore + * still not sufficient: on an un-migrated or external table it matches + * whichever half the writer produced and empties the other. + * See `StrategyContext.coerceTemporalFilterColumn`. */ coerceTemporalFilterColumn?: (objectName: string, fieldName: string, columnSql: string) => string; /** @@ -1084,6 +1130,13 @@ export class AnalyticsService implements IAnalyticsService { const compiled = compileDataset(dataset, this.relationshipResolver, { getObjectDatasource: this.getObjectDatasource, isExternalObject: this.isExternalObject, + // [#16737 / #16099] …and the aggregate × field-type compatibility table + // (`@objectstack/spec`, #16353) is decidable from the same declared type + // the result-column enrichment already reads. Same source, one call + // shape, so a host that wired `sourceFieldMeta` gets the compile-time + // refusal with no second hook to remember. + declaredFieldType: (object: string, field: string) => + this.sourceFieldMeta?.(object, field)?.type, }); this.cubeRegistry.register(compiled.cube); this.datasetRegistry.set(dataset.name, compiled); diff --git a/packages/services/service-analytics/src/dataset-compiler.ts b/packages/services/service-analytics/src/dataset-compiler.ts index 9e9f3fc74d..88d2266a4d 100644 --- a/packages/services/service-analytics/src/dataset-compiler.ts +++ b/packages/services/service-analytics/src/dataset-compiler.ts @@ -1,7 +1,11 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import type { Cube, Metric, Dimension as CubeDimension, CubeJoin } from '@objectstack/spec/data'; -import { AggregationFunction } from '@objectstack/spec/data'; +import { + AGGREGATE_FIELD_TYPE_COMPATIBILITY, + AggregationFunction, + isAggregateCompatibleWithFieldType, +} from '@objectstack/spec/data'; import type { Dataset, DatasetMeasure, DatasetDimension } from '@objectstack/spec/ui'; import { resolveI18nLabel } from '@objectstack/spec/ui'; import type { FilterCondition } from '@objectstack/spec/data'; @@ -153,6 +157,20 @@ export interface DatasetCompileOptions { * construction. Rejecting it here would break a path that works today. */ isExternalObject?: (objectName: string) => boolean; + /** + * [#16737 / #16099] The DECLARED `FieldType` of `field` on `objectName`, or + * `undefined` when nothing authoritative can answer. Supplied by the host + * from the same `AnalyticsServiceConfig.sourceFieldMeta` the result-column + * enrichment reads. + * + * It is what makes {@link assertAggregateFieldTypeCompatible} decidable at + * COMPILE time — the compile leg of the director ruling (decision batch #59, + * 2026-09-06: "both legs, table in spec"), whose table is + * `AGGREGATE_FIELD_TYPE_COMPATIBILITY` in `@objectstack/spec`. Absent hook, + * unknown object, unknown field → the pair is not judged, matching every + * other probe on this interface. + */ + declaredFieldType?: (objectName: string, field: string) => string | undefined; } /** Map a dataset measure's aggregate to the Cube metric `type`. */ @@ -180,6 +198,97 @@ function aggregateToMetricType(m: DatasetMeasure): Metric['type'] { return m.aggregate as Metric['type']; } +/** + * [#16737 / #16099] Refuse a measure whose AGGREGATE cannot meaningfully consume + * its field's declared TYPE — the compile leg of the director ruling (decision + * batch #59, 2026-09-06: "both legs, table in spec"; the table is + * `AGGREGATE_FIELD_TYPE_COMPATIBILITY` in `@objectstack/spec`, #16353). + * + * ## The shape this closes + * + * `{ aggregate: 'avg', field: 'submitted_at' }` over a `Field.datetime` + * compiled to `AVG(submitted_at)` and reached the backend, where the ANSWER is + * a property of the dialect rather than of the data. Both halves were measured + * on this card: + * + * ``` + * -- SQLite (better-sqlite3), the canonical storage form (#3912) + * select typeof(submitted_at), submitted_at from clm_contract limit 1; + * text|2026-05-19T00:00:00.000Z + * select avg(submitted_at) from clm_contract; + * 2025.5 -- text->numeric coercion: the average YEAR + * + * -- PostgreSQL 16.13 + * select avg(submitted_at) from clm_contract; + * ERROR: function avg(timestamp with time zone) does not exist -- 42883 + * ``` + * + * ⭐ The SQLite half is the dangerous one, and the reason this refusal is a + * refusal rather than a definition: `2025.5` is not a number a reader can tell + * is wrong. Fed to `derived: { op: 'difference', of: [avg_a, avg_b] }` it + * became `-0.85` on a tile labelled "average cycle time delta" — exactly what a + * correct answer looks like. Postgres at least fails loudly; the DEV datasource + * in this platform's default flow is SQLite, so the plausible number is what + * ships (Prime Directive #12). + * + * ## Why compile time, and why that also covers `derived` + * + * `derived` measures reference other measures BY NAME, and the executor expands + * a selected derived measure into its `of` dependencies before querying. Both + * are downstream of THIS loop: a dataset carrying an incompatible base measure + * never finishes compiling, so no `derived` op can be handed its output. That + * is the whole of `derived` coverage — there is no second gate to keep in step, + * which is why the refusal is placed on the measure and not on the consumer. + * + * ## Tiering — "cannot answer, do not block", the same as every sibling probe + * + * - No `declaredFieldType` hook (no data engine wired) → not judged. + * - A field the hook cannot resolve → not judged. + * - A RELATIONSHIP-PATH field (`account.closed_at`) → not judged. The hook + * resolves a column on the BASE object, so it would answer about a different + * column of the same name, or about nothing; the spec module says exactly + * this ("a consumer that cannot resolve a field's type must NOT call the + * predicate with a guess"). + * - `count` with no field → nothing to judge. + * + * ⛔ The accepted set is NEVER restated here. It is read off the exported table + * so the message cannot drift from the contract it enforces, and so a row + * changed in the spec changes this refusal in the same commit. + */ +function assertAggregateFieldTypeCompatible( + datasetName: string, + objectName: string, + measure: DatasetMeasure, + declaredFieldType?: (objectName: string, field: string) => string | undefined, +): void { + if (!declaredFieldType) return; + const aggregate = measure.aggregate; + const field = measure.field; + if (!aggregate || !field) return; + // A dotted reference resolves on a JOINED object; this hook answers for the + // base one. Not judged rather than judged wrongly. + if (field.includes('.')) return; + const fieldType = declaredFieldType(objectName, field); + if (!fieldType) return; + if (isAggregateCompatibleWithFieldType(aggregate, fieldType)) return; + + const accepted = AGGREGATE_FIELD_TYPE_COMPATIBILITY[aggregate]; + // [#5716] `DATASET_INVALID` / 400 — a verdict about the dataset DOCUMENT, + // decided from metadata alone before any query runs, and fixable only by the + // author who wrote the pair. + throw datasetInvalidError( + `[dataset-compiler] dataset "${datasetName}" measure "${measure.name}" applies aggregate ` + + `"${aggregate}" to field "${field}", which object "${objectName}" declares as ` + + `\`${fieldType}\`. That pair is not accepted: the answer would be decided by the SQL ` + + `dialect rather than by the data (SQLite coerces the stored text to a number — an ` + + `AVG over a datetime returns the average YEAR — while Postgres has no such function ` + + `and fails at query time), so one dataset would mean two things on two deployments. ` + + `"${aggregate}" accepts: ${accepted.join(', ')}. ` + + `For a temporal field, \`min\`/\`max\` return a real instant; a DURATION has to be ` + + `stored as a number (a computed "days open" field) and aggregated as one.`, + ); +} + /** Map a dataset dimension type to the Cube dimension `type`. */ function dimensionType(d: DatasetDimension): CubeDimension['type'] { switch (d.type) { @@ -450,6 +559,11 @@ export function compileDataset( continue; } if (m.field) assertDeclared(m.field, 'measure', m.name); + // [#16737 / #16099] …and the aggregate ITSELF must be one the field's + // declared type can carry. Placed after the join-declaration check so a + // dotted field is refused for the reason it is actually wrong (an + // undeclared relationship) before this gate stands down on it. + assertAggregateFieldTypeCompatible(dataset.name, dataset.object, m, options?.declaredFieldType); const metric: Metric = { name: m.name, // [#6761] Same as the dimension label above — see {@link REGISTRY_LOCALE}. diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 8074e13470..9dc01f9aee 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -637,9 +637,16 @@ export class AnalyticsServicePlugin implements Plugin { // The raw-SQL strategy binds dashboard relative-date tokens (already expanded // to ISO strings) directly, bypassing the driver's CRUD coercion. Delegate to // the driver — the single source of truth for the on-disk storage convention — - // so a `Field.datetime` ISO comparand becomes epoch ms on SQLite, while - // `Field.date` text and native-timestamp (Postgres) columns pass through - // unchanged. Resolved at call time so plugin-init order does not matter. + // so a `Field.datetime` comparand is canonicalised to the SAME form the write + // path stores, while `Field.date` text and native-timestamp (Postgres) + // columns pass through unchanged. Resolved at call time so plugin-init order + // does not matter. + // + // ⛔ What that form IS is stated in exactly one place — + // `AnalyticsServiceConfig.coerceTemporalFilterValue`'s storage-reality block + // in `analytics-service.ts` (#16737). Do not restate it here; this comment + // used to say "becomes epoch ms on SQLite", which stopped being true when + // #3912 made canonical UTC text the one stored form. const coerceTemporalFilterValue = ( objectName: string, fieldName: string, @@ -658,11 +665,19 @@ export class AnalyticsServicePlugin implements Plugin { return value; }; - // The column half of the same fix (#3912). A SQLite `Field.datetime` column - // holds BOTH storage forms — INTEGER epoch from a `Date` write, ISO TEXT from - // a REST/JSON write or a `NOW()` default — so coercing the comparand alone - // matched whichever half the writer produced and returned an empty window for - // the other. Ask the driver for the column expression that normalises both. + // The column half of the same fix (#3912), and the half that is CONDITIONAL. + // A SQLite `Field.datetime` column written before the canonical convention + // can still hold a mix — INTEGER epoch from a `Date` write next to text from + // a REST/JSON write — so coercing the comparand alone matched whichever half + // the writer produced and returned an empty window for the other. Ask the + // driver for the column expression that normalises both; on a converged + // column, and on every dialect with a real temporal type, it answers with the + // bare column and the comparison stays indexable. + // + // ⛔ Same rule as the hook above: the storage reality is stated once, on + // `AnalyticsServiceConfig.coerceTemporalFilterValue` (#16737). This comment + // used to assert the mixed form as the steady state; it is the transitional + // one. const coerceTemporalFilterColumn = ( objectName: string, fieldName: string, diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 1078b75b92..909ba6c63e 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -960,12 +960,20 @@ export class NativeSQLStrategy implements AnalyticsStrategy { * The column side of {@link coerceTemporal}: normalise the reference so it * reads in the storage form the comparand was coerced into. * - * A SQLite `Field.datetime` column carries an INTEGER epoch (a `Date` write) - * and ISO TEXT (a REST/JSON write, a `NOW()` default — including the platform's - * own `created_at`) at the SAME time, so coercing the value alone fixes one half - * and empties the other. That is #3912: a `dateRange: last_30_days` on - * `created_date` read 0 with 29 rows in range. Every other column and dialect - * gets its reference back verbatim. + * ⛔ What that form is, on which dialect, is stated in ONE place — + * `AnalyticsServiceConfig.coerceTemporalFilterValue`'s storage-reality block + * in `analytics-service.ts` (#16737) — and this docblock deliberately does not + * restate it. It used to, in a form that has been wrong since #3912: "a SQLite + * `Field.datetime` column carries an INTEGER epoch and ISO TEXT at the SAME + * time" describes a database written before the canonical convention and not + * yet backfilled, not the steady state. + * + * What is true of THIS method either way: the mixed column is the case it + * exists for (#3912 — a `dateRange: last_30_days` on `created_date` read 0 + * with 29 rows in range, because coercing the value alone fixes one half and + * empties the other), and every column the driver reports as converged — plus + * every dialect with a real temporal type — gets its reference back verbatim, + * so the comparison stays indexable. */ private temporalColumn( ctx: StrategyContext, @@ -1186,11 +1194,14 @@ export class NativeSQLStrategy implements AnalyticsStrategy { } // Coerce so booleans/numbers bind as their native SQL types AND so a - // relative-date / ISO-string comparand on a SQLite `Field.datetime` - // column is converted to its INTEGER epoch storage form. Without this a - // dashboard filter like `assessed_at >= '2025-06-18'` compiles to a - // TEXT-vs-INTEGER affinity compare that is always false → "No rows", - // even though the rows exist (the confirmed time-series chart bug). + // relative-date / ISO-string comparand on a SQLite `Field.datetime` column + // is converted to that column's storage form (#16737: the ONE statement of + // what that form is lives on `AnalyticsServiceConfig.coerceTemporalFilterValue` + // — this comment used to name the INTEGER epoch, which #3912 retired as a + // live write path). Without the coercion a dashboard filter like + // `assessed_at >= '2025-06-18'` compares an unnormalised comparand against + // the stored form and is always false → "No rows", even though the rows + // exist (the confirmed time-series chart bug). params.push(this.coerceTemporal(ctx, target, values[0])); return `${this.temporalColumn(ctx, target, rawCol)} ${sqlOp} $${params.length}`; } diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index b3773cff25..a1ca088874 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -1665,12 +1665,21 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * epoch-ms window would have to be declared, not here). An author who wants an * instant window writes it as one; a declared `string` binds as a string. No * STORAGE coercion happens here either, deliberately: `NativeSQLStrategy` needs - * `coerceTemporal` because it binds into raw SQL and had to learn that a - * SQLite `Field.datetime` is an INTEGER epoch (#2034); this path goes through + * `coerceTemporal` because it binds into raw SQL and so has to canonicalise + * the comparand itself (#2034, then #3912); this path goes through * `engine.aggregate()`, where the driver's own CRUD filter coercion applies — * the very coercion that already makes a `where` bound on that same column * work today. * + * ⛔ This sentence used to end "…had to learn that a SQLite `Field.datetime` + * IS an INTEGER epoch (#2034)". That flat claim has been wrong since #3912 + * gave the column one canonical UTC-text storage form; the epoch survives only + * in a database not yet backfilled. The reason `NativeSQLStrategy` needs the + * coercion is unchanged — it binds outside the driver's builder — and the + * storage fact itself is stated in ONE place, on + * `AnalyticsServiceConfig.coerceTemporalFilterValue` in `analytics-service.ts` + * (#16737). + * * A bare-string `dateRange` degenerates to the single point `[s, s]`, matching * `NativeSQLStrategy`. Relative phrases ("Last 7 days") are NOT resolved here; * neither SQL path resolves them, and inventing a second interpretation on the From 1505f4548c2fad28fe79657eb97d6ae4bf85e42e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:37:03 +0000 Subject: [PATCH 2/4] wip: changeset + adr-0087 ledger entry --- ...et-measure-aggregate-field-type-refused.md | 89 +++++++++++++++++++ ...et-measure-aggregate-field-type-refused.ts | 47 ++++++++++ packages/spec/src/migrations/registry.ts | 43 +++++++++ 3 files changed, 179 insertions(+) create mode 100644 .changeset/dataset-measure-aggregate-field-type-refused.md create mode 100644 packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts diff --git a/.changeset/dataset-measure-aggregate-field-type-refused.md b/.changeset/dataset-measure-aggregate-field-type-refused.md new file mode 100644 index 0000000000..7f6a0895e7 --- /dev/null +++ b/.changeset/dataset-measure-aggregate-field-type-refused.md @@ -0,0 +1,89 @@ +--- +"@objectstack/service-analytics": minor +"@objectstack/spec": minor +--- + +feat(service-analytics)!: a dataset measure whose `aggregate` its `field`'s declared type cannot carry is refused at compile time with `400 DATASET_INVALID` (#16737, compile leg of #16099) + + + +**BREAKING** — an accept-set narrowing on a published authoring surface. A dataset +measure pairing `aggregate: 'avg'` with a `Field.datetime` used to compile to +`AVG(col)` and reach the backend; it is now refused by `compileDataset` before any +query is built. Shipped as `minor` under the repo's launch-window convention for +accept-set narrowings; the hand-migration prescription is registered under protocol +major 18 as `dataset-measure-aggregate-field-type-refused`. + +The pair is judged against `AGGREGATE_FIELD_TYPE_COMPATIBILITY` — the one table +`@objectstack/spec` declared in #16353 under the director ruling of decision batch +#59 (2026-09-06, "both legs, table in spec"). ⛔ This changeset adds no rows and +restates none: the refusal reads the shipped predicate, so the contract has exactly +one statement. + +## What was wrong + +The answer to `AVG` over a temporal column was decided by the SQL dialect rather +than by the data. Both halves measured on this card: + +``` +-- SQLite (better-sqlite3), the canonical UTC-text storage form (#3912) +select typeof(submitted_at), submitted_at from clm_contract limit 1; + text|2026-05-19T00:00:00.000Z +select avg(submitted_at) from clm_contract; + 2025.5 <- text->numeric coercion: the average YEAR + +-- PostgreSQL 16.13 +select avg(submitted_at) from t; + ERROR: function avg(timestamp with time zone) does not exist -- SQLSTATE 42883 +``` + +The silent half is the dangerous one, and SQLite is the default dev datasource: +`derived: { op: 'difference', of: [avg_a, avg_b] }` over two such averages returned +`-0.85` and rendered on a tile labelled "average cycle time delta" — a number +indistinguishable from a correct one. Nothing refused it at any layer: not the +schema, not `os validate` / `os lint`, not the analytics service, not the renderer. + +## What it does now + +- `compileDataset` refuses an incompatible `aggregate` × `field` pair with + `DATASET_INVALID` / **400**, naming the measure, the field, its declared type and + the accepted set (read off the table, never restated). Nothing reaches the driver. +- It reads the declared type from the `sourceFieldMeta` a host already wires, via a + new optional `DatasetCompileOptions.declaredFieldType` probe. +- **`derived` is covered by construction.** A derived measure's `of` operands are + base measures of the same dataset, so a dataset carrying a refused base measure + never finishes compiling and no `derived` op can be handed its output — including + when the selection names only the derived measure. +- Tiered "cannot answer, do not block" like every sibling probe: no + `sourceFieldMeta`, an unresolvable field, or a `relationship.field` path (whose + column lives on a joined object) leaves the pair unjudged. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `{ aggregate: 'avg', field: }` | `{ aggregate: 'min' \| 'max', field: }` — a real instant of the field's own type | +| `{ aggregate: 'sum', field: }` | store the duration as a number (a computed "days open" field) and `sum`/`avg` that | +| `{ aggregate: 'avg', field: }` | unchanged — `avg` accepts `percent` | +| `{ aggregate: 'sum', field: }` | `avg`, or sum the underlying amounts — a rate does not add | +| `derived: { op: 'difference', of: ['avg_a', 'avg_b'] }` over temporal averages | fix the two operand measures; the `derived` spec itself is unchanged | + +⭐ A duration is not recoverable from an aggregate over instants on any backend. +Where an "average cycle time" is wanted, the cycle length has to exist as a number +before it can be averaged. + +## What is deliberately untouched + +`date` / `datetime` used as a **dimension** — grouping, bucketing, date-range +filtering — is unchanged; this is about aggregation only. `avg` over a genuine +numeric measure, `min` / `max` over a temporal one, and `count` / `count_distinct` +over anything all behave exactly as before. + +Alongside the refusal, `service-analytics`' four contradictory annotations about +what a SQLite `Field.datetime` column physically holds are reconciled to one +statement. Two said it holds an INTEGER epoch and ISO TEXT at once; one said flatly +that it IS an INTEGER epoch. Neither is current: since #3912 the column has ONE +storage form, canonical UTC text, with the epoch surviving only in a database not +yet converged by `backfillCanonicalDatetimes`. The fact is now stated once, on +`AnalyticsServiceConfig.coerceTemporalFilterValue`, and the other sites link to it. +No behaviour changes from that half. diff --git a/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts b/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts new file mode 100644 index 0000000000..b2dafc12fc --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts @@ -0,0 +1,47 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'dataset-measure-aggregate-field-type-refused', + surface: 'dataset measure `aggregate` × `field` pairs (`DatasetMeasureSchema`, the rows ' + + 'inside `Dataset.measures[]`) whose aggregate the field\'s declared `FieldType` ' + + 'cannot carry — notably `avg` / `sum` over a `date` / `datetime` / `time` field, ' + + 'and `sum` over a `percent`', + replacement: 'an aggregate the field\'s type accepts, per ' + + '`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`, #16353): ' + + '`min` / `max` for a temporal field — both return a real instant of the field\'s own ' + + 'type — `avg` for a `percent`, `count` / `count_distinct` for anything. A DURATION ' + + 'is not recoverable from an aggregate over instants: store it as a number (a ' + + 'computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + + 'names a refused measure is fixed by fixing that measure, not the `derived` one', + reason: + '#16737 / #16099. Nothing between the author and the driver correlated a measure\'s ' + + 'aggregate with its field type, so `avg` over a `Field.datetime` compiled to ' + + '`AVG(col)` and reached the backend — where the ANSWER was decided by the dialect ' + + 'rather than by the data. Measured on both halves: SQLite coerces the column\'s ' + + 'canonical UTC text to a number by reading its leading digits, so ' + + '`avg(submitted_at)` over 2026-05 and 2025-01 returns `2025.5` — the average YEAR, ' + + 'no error, no log; PostgreSQL 16 answers `function avg(timestamp with time zone) ' + + 'does not exist` (SQLSTATE 42883). ⭐ The silent half is the dangerous one, and it ' + + 'is the DEV default: `derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + + 'such averages rendered `-0.85` on a tile labelled "average cycle time delta" — ' + + 'indistinguishable from a correct answer, which is the shape Prime Directive #12 ' + + 'exists to remove. Which pairs are accepted is therefore a contract, declared once ' + + 'in `@objectstack/spec` under the director ruling of decision batch #59 ' + + '(2026-09-06, "both legs, table in spec") and executed by the consumer legs; the ' + + 'compile-time leg (`dataset-compiler`, `service-analytics`) refuses the pair with ' + + '`DATASET_INVALID` / 400 before any query is built, using the declared type the ' + + 'host already supplies through `AnalyticsServiceConfig.sourceFieldMeta`. ' + + '⚠️ A `date` / `datetime` used as a DIMENSION — grouping, bucketing, date-range ' + + 'filtering — is untouched: this is about aggregation only.', + acceptanceCriteria: + 'Every dataset measure pairs an `aggregate` with a `field` whose declared type that ' + + 'aggregate accepts. Accepted pairs compile and execute byte-identically to before ' + + '(`avg` over `number` / `currency`, `min` / `max` over `datetime`, `count` over ' + + 'anything); a refused pair answers `400 DATASET_INVALID` naming the measure, the ' + + 'field, its declared type and the accepted set, with no SQL emitted. The refusal ' + + 'stands down rather than guessing wherever the type cannot be resolved: no ' + + '`sourceFieldMeta` wired, an unknown field, or a `relationship.field` path whose ' + + 'column lives on a joined object.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 1aa751853c..b2e6d9fe05 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6412,6 +6412,49 @@ const step18: MigrationStep = { + 'and a sweep that renamed either has over-applied the rule: batchSize is a COUNT of ' + 'documents, not a duration, and consistency / projection / hint are not numbers at all.', }, + { + id: 'dataset-measure-aggregate-field-type-refused', + surface: 'dataset measure `aggregate` × `field` pairs (`DatasetMeasureSchema`, the rows ' + + 'inside `Dataset.measures[]`) whose aggregate the field\'s declared `FieldType` ' + + 'cannot carry — notably `avg` / `sum` over a `date` / `datetime` / `time` field, ' + + 'and `sum` over a `percent`', + replacement: 'an aggregate the field\'s type accepts, per ' + + '`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`, #16353): ' + + '`min` / `max` for a temporal field — both return a real instant of the field\'s own ' + + 'type — `avg` for a `percent`, `count` / `count_distinct` for anything. A DURATION ' + + 'is not recoverable from an aggregate over instants: store it as a number (a ' + + 'computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + + 'names a refused measure is fixed by fixing that measure, not the `derived` one', + reason: + '#16737 / #16099. Nothing between the author and the driver correlated a measure\'s ' + + 'aggregate with its field type, so `avg` over a `Field.datetime` compiled to ' + + '`AVG(col)` and reached the backend — where the ANSWER was decided by the dialect ' + + 'rather than by the data. Measured on both halves: SQLite coerces the column\'s ' + + 'canonical UTC text to a number by reading its leading digits, so ' + + '`avg(submitted_at)` over 2026-05 and 2025-01 returns `2025.5` — the average YEAR, ' + + 'no error, no log; PostgreSQL 16 answers `function avg(timestamp with time zone) ' + + 'does not exist` (SQLSTATE 42883). ⭐ The silent half is the dangerous one, and it ' + + 'is the DEV default: `derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + + 'such averages rendered `-0.85` on a tile labelled "average cycle time delta" — ' + + 'indistinguishable from a correct answer, which is the shape Prime Directive #12 ' + + 'exists to remove. Which pairs are accepted is therefore a contract, declared once ' + + 'in `@objectstack/spec` under the director ruling of decision batch #59 ' + + '(2026-09-06, "both legs, table in spec") and executed by the consumer legs; the ' + + 'compile-time leg (`dataset-compiler`, `service-analytics`) refuses the pair with ' + + '`DATASET_INVALID` / 400 before any query is built, using the declared type the ' + + 'host already supplies through `AnalyticsServiceConfig.sourceFieldMeta`. ' + + '⚠️ A `date` / `datetime` used as a DIMENSION — grouping, bucketing, date-range ' + + 'filtering — is untouched: this is about aggregation only.', + acceptanceCriteria: + 'Every dataset measure pairs an `aggregate` with a `field` whose declared type that ' + + 'aggregate accepts. Accepted pairs compile and execute byte-identically to before ' + + '(`avg` over `number` / `currency`, `min` / `max` over `datetime`, `count` over ' + + 'anything); a refused pair answers `400 DATASET_INVALID` naming the measure, the ' + + 'field, its declared type and the accepted set, with no SQL emitted. The refusal ' + + 'stands down rather than guessing wherever the type cannot be resolved: no ' + + '`sourceFieldMeta` wired, an unknown field, or a `relationship.field` path whose ' + + 'column lives on a joined object.', + }, { id: 'datasource-config-mongo-options-credential-refused', surface: 'datasource.config.options.auth.password (mongodb) — a login credential written ' + From 80ec9f2b6a7e076b9c9683334224f62234979ac1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 05:03:09 +0000 Subject: [PATCH 3/4] fix(service-analytics): scope the compile-leg refusal to temporal source fields The full table refuses `min`/`max` over the string classes and the boolean rows, both of which this platform answers on purpose and pins with tests (#15768, maintainer ruling #11152). Executing those is a product judgement that belongs to #16099; the temporal rows carry no such collision and are the ones this card is about. Re-points the two `measure-result-type.test.ts` fixture measures that aggregated a datetime column, and corrects the module header that recorded the missing refusal as an open finding. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...aggregate-datetime-measure-refusal.test.ts | 52 +++++++++++++++++++ .../src/__tests__/measure-result-type.test.ts | 21 ++++++-- .../service-analytics/src/dataset-compiler.ts | 37 +++++++++++++ .../src/measure-result-type.ts | 24 +++++---- 4 files changed, 119 insertions(+), 15 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts index ada19e658a..c351452daf 100644 --- a/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts @@ -41,6 +41,21 @@ * so a row changed upstream changes these expectations with it rather than * leaving a second, drifting account of the contract. * + * ## ⚠️ The compile leg lands SCOPED to temporal source fields, and says why + * + * The verdict is the table's; what is scoped is which FIELDS the gate judges. + * Executing every row today refuses two families this platform answers on + * purpose, with tests: `min` / `max` over the STRING classes (typed `'string'` + * by `measureResultType`, #15768, and pinned end to end in + * `measure-result-type.test.ts`), and the boolean rows (maintainer ruling + * #11152 has booleans aggregate as numbers on every backend). The spec module's + * own header records both as OVERRIDES of existing opinions rather than + * agreement, and refers the boolean one back to the maintainer. Refusing them + * would break uses that work today — a product judgement, and #16099's + * full-table leg is where it belongs. The temporal rows carry no such + * collision, which is why they are the ones executed here. The last suite in + * this file pins that boundary so it cannot widen by accident. + * * ## Dissolution verification — direction predicted BEFORE running * * Deleting the `assertAggregateFieldTypeCompatible` call in @@ -145,8 +160,10 @@ const FIELD_TYPES: Record = { submitted_at: 'datetime', approved_at: 'datetime', close_date: 'date', + shift_start: 'time', cycle_days: 'number', amount: 'currency', + note: 'text', }; /** @@ -457,3 +474,38 @@ describe('#16737 — the gate stands down rather than guessing', () => { expect(result.rows.length).toBe(1); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// The scope boundary, pinned — so it cannot widen (or narrow) unnoticed +// ───────────────────────────────────────────────────────────────────────────── + +describe('#16737 — the compile leg is scoped to temporal source fields, on purpose', () => { + it('a `min` over a TEXT field still compiles here, though the table refuses the pair', async () => { + // ⚠️ Not an endorsement of the pair — a statement about WHO refuses it. + // `measureResultType` (#15768) types this result as `'string'` and + // `measure-result-type.test.ts` pins it end to end, so enforcing the + // table's string rows is a product judgement that belongs to #16099, not a + // side effect of this card. The table's verdict is asserted directly, so + // this case reads as "the contract says no, this gate does not act on it". + expect(isAggregateCompatibleWithFieldType('min', 'text')).toBe(false); + const { svc } = makeService([{ status: 'open', first_note: 'Archive the backlog' }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'first_note', aggregate: 'min', field: 'note' }]), + { dimensions: ['status'], measures: ['first_note'] }, + ); + expect(result.rows[0].first_note).toBe('Archive the backlog'); + }); + + it('every temporal member IS judged — the scope is the class, not the one type the card named', async () => { + for (const [field, label] of [['submitted_at', 'datetime'], ['close_date', 'date'], ['shift_start', 'time']] as const) { + const { svc } = makeService(); + const err = await refusalOf(() => + svc.queryDataset( + dataset([{ name: 'avg_temporal', aggregate: 'avg', field }]), + { dimensions: ['status'], measures: ['avg_temporal'] }, + ), + ); + expect(err.code, `avg over a ${label} field`).toBe('DATASET_INVALID'); + } + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/measure-result-type.test.ts b/packages/services/service-analytics/src/__tests__/measure-result-type.test.ts index 20d6b965bb..0a36d78328 100644 --- a/packages/services/service-analytics/src/__tests__/measure-result-type.test.ts +++ b/packages/services/service-analytics/src/__tests__/measure-result-type.test.ts @@ -312,8 +312,17 @@ const dataset = DatasetSchema.parse({ { name: 'task_count', aggregate: 'count', label: 'Tasks' }, { name: 'counted_touches', aggregate: 'count', field: 'last_update_at', label: 'Touched' }, { name: 'counted_subjects', aggregate: 'count', field: 'subject', label: 'Subjects' }, - { name: 'summed_touches', aggregate: 'sum', field: 'last_update_at', label: 'Summed touches' }, - { name: 'avg_touch', aggregate: 'avg', field: 'last_update_at', label: 'Average touch' }, + // ⚠️ [#16737] These two used to aggregate `last_update_at`, and section D's + // comment on them read "nothing refuses the pair". That is no longer true: + // `sum` / `avg` over a temporal field is now refused at COMPILE time + // (`dataset-compiler`, the #16099 leg), so a dataset declaring the pair + // cannot exist to be queried. What these two are here to pin is unchanged — + // that `sum` / `avg` keep saying `number` — so they moved to the numeric + // column and keep pinning it. ⛔ Do not point them back at a temporal field: + // that pins a shape the platform refuses, and the suite would be asserting + // the absence of this card's fix. + { name: 'summed_touches', aggregate: 'sum', field: 'estimate_hours', label: 'Summed estimates' }, + { name: 'avg_touch', aggregate: 'avg', field: 'estimate_hours', label: 'Average estimate' }, { name: 'min_estimate', aggregate: 'min', field: 'estimate_hours', label: 'Smallest estimate' }, { name: 'min_flag', aggregate: 'min', field: 'is_urgent', label: 'Min urgency flag' }, { name: 'min_payload', aggregate: 'min', field: 'payload', label: 'Min payload' }, @@ -487,7 +496,7 @@ describe('C) both strategy producers move together — the correction is downstr // ───────────────────────────────────────────────────────────────────────────── describe('D) the columns that are genuinely numeric keep saying number', () => { - it('count / count_distinct / sum / avg over the SAME datetime column, and a derived measure', async () => { + it('count / count_distinct over the SAME datetime column, sum / avg over a numeric one, and a derived measure', async () => { const result = await objectqlService().queryDataset( dataset, { @@ -500,8 +509,10 @@ describe('D) the columns that are genuinely numeric keep saying number', () => { // otherwise would be a new bug, so this is a load-bearing control. expect(typeOf(result.fields, 'task_count')).toBe('number'); expect(typeOf(result.fields, 'counted_touches')).toBe('number'); - // `sum`/`avg` over a temporal column: nothing refuses the pair and the value - // is backend-decided, so no type is invented for it. + // `sum`/`avg` keep saying `number`, which is correct for them over the + // numeric column they now aggregate. [#16737] Over a TEMPORAL column the + // pair no longer reaches a type at all — it is refused at compile time, and + // `aggregate-datetime-measure-refusal.test.ts` is where that is pinned. expect(typeOf(result.fields, 'summed_touches')).toBe('number'); expect(typeOf(result.fields, 'avg_touch')).toBe('number'); // A derived measure has no aggregate and is numeric by construction. diff --git a/packages/services/service-analytics/src/dataset-compiler.ts b/packages/services/service-analytics/src/dataset-compiler.ts index 88d2266a4d..d4570fabe2 100644 --- a/packages/services/service-analytics/src/dataset-compiler.ts +++ b/packages/services/service-analytics/src/dataset-compiler.ts @@ -10,6 +10,7 @@ import type { Dataset, DatasetMeasure, DatasetDimension } from '@objectstack/spe import { resolveI18nLabel } from '@objectstack/spec/ui'; import type { FilterCondition } from '@objectstack/spec/data'; import { datasetInvalidError } from './dataset-refusal.js'; +import { TEMPORAL_SOURCE_FIELD_TYPES } from './measure-result-type.js'; /** * Dataset → Cube compiler (ADR-0021 D-A=(c), WS2). @@ -240,10 +241,43 @@ function aggregateToMetricType(m: DatasetMeasure): Metric['type'] { * is the whole of `derived` coverage — there is no second gate to keep in step, * which is why the refusal is placed on the measure and not on the consumer. * + * ## ⚠️ Scope: TEMPORAL source fields only, and why the rest of the table waits + * + * The verdict is the spec predicate's — ⛔ no row is restated here, and + * `min`/`max` over a temporal field stay ACCEPTED because the table accepts + * them. What is scoped is which FIELDS this gate judges at all: the temporal + * class (`TEMPORAL_SOURCE_FIELD_TYPES` — this package's own shipped statement + * of it, the set `measureResultType` already reads), and no other. + * + * ⛔ That is a deliberate stop, not an oversight, and it is not a rule invented + * ahead of the table. Executing every row of the table today REFUSES pairs this + * platform currently answers, on purpose, with tests: + * + * - `min` / `max` over the STRING classes (`text`, `select`, `lookup`, + * `autonumber`, …). `measureResultType` (#15768) types exactly those results + * as `'string'`, and `__tests__/measure-result-type.test.ts` pins them end to + * end through `queryDataset` — 15 cases that go red the moment those rows are + * enforced. The spec module's own header records this as an OVERRIDE of an + * existing opinion rather than agreement with it. + * - `sum` / `avg` / `min` / `max` over `boolean` / `toggle`. Maintainer ruling + * #11152 pins booleans aggregating as numbers on every backend + * (`AGGREGATION_CASES`), and the spec header refers the collision between + * that ruling and batch #59 back to the maintainer as its own decision. + * + * Refusing those would break uses that work today, which is a product judgement + * and not a dev's to take mid-flight. The temporal rows carry no such + * collision, and were measured on both dialects before this gate was written: + * SQLite answers a silent average YEAR, Postgres refuses at 42883, no shipped + * dataset in this repo pairs them, and there is no reading on which the mean of + * a set of instants is a duration. So the temporal rows are executed and the + * remainder stays with #16099, whose full-table leg is blocked on those two + * collisions being ruled. + * * ## Tiering — "cannot answer, do not block", the same as every sibling probe * * - No `declaredFieldType` hook (no data engine wired) → not judged. * - A field the hook cannot resolve → not judged. + * - A field outside the temporal class → not judged HERE (see the scope note). * - A RELATIONSHIP-PATH field (`account.closed_at`) → not judged. The hook * resolves a column on the BASE object, so it would answer about a different * column of the same name, or about nothing; the spec module says exactly @@ -270,6 +304,9 @@ function assertAggregateFieldTypeCompatible( if (field.includes('.')) return; const fieldType = declaredFieldType(objectName, field); if (!fieldType) return; + // Scoped to the temporal class — see the scope note above. The VERDICT still + // comes from the spec table, never from this condition. + if (!TEMPORAL_SOURCE_FIELD_TYPES.has(fieldType)) return; if (isAggregateCompatibleWithFieldType(aggregate, fieldType)) return; const accepted = AGGREGATE_FIELD_TYPE_COMPATIBILITY[aggregate]; diff --git a/packages/services/service-analytics/src/measure-result-type.ts b/packages/services/service-analytics/src/measure-result-type.ts index 806b5a3614..16894f5439 100644 --- a/packages/services/service-analytics/src/measure-result-type.ts +++ b/packages/services/service-analytics/src/measure-result-type.ts @@ -50,16 +50,20 @@ import { * operands were — it is answered here as "no correction", which leaves the * `number` its producer minted. * - * **`sum`/`avg` over a temporal field is NOT a case a type is invented for.** - * Nothing in the shipped stack refuses the pair: `dataset-compiler`'s - * `aggregateToMetricType` checks only membership of the vocabulary, the three - * source-field gates check only that the column EXISTS, and no lint rule pairs - * an aggregate with a field type. It therefore reaches the driver, where what - * comes back is decided by the backend and the storage form — a mean of epoch - * integers on SQLite, a refusal from Postgres, which has no `avg(timestamptz)`. - * There is no one value for a type to describe, so this rule leaves both alone - * and the missing refusal is reported as its own finding rather than papered - * over with a type that would be wrong on at least one backend. + * **`sum`/`avg` over a temporal field is NOT a case a type is invented for — + * and since #16737 it is not a case that reaches a type at all.** This rule + * left the pair alone because what came back was decided by the backend and the + * storage form (SQLite coerces the column's canonical UTC text and answers the + * average YEAR; Postgres has no `avg(timestamptz)` and refuses at 42883), so + * there was no one value for a type to describe. The missing refusal was + * reported as its own finding rather than papered over with a type that would + * be wrong on at least one backend — and that finding is now closed: + * `dataset-compiler` refuses the pair at COMPILE time against + * `AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec`, #16353; the + * compile leg of #16099), so no such measure can be queried and no column + * descriptor is ever minted for one. The rows below are unchanged: `sum`/`avg` + * still keep the `number` their producer minted, which is correct for the + * numeric fields they may now be applied to. * * ## The field-type axis: every `FieldType` member, by MEASURED value class * From 181d3cc8a7727c3c80f7cef470ecf0339b46924b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 12:36:23 +0000 Subject: [PATCH 4/4] docs(changeset,spec): scope the breaking declaration to the temporal class it actually withdraws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compile leg was scoped to TEMPORAL source fields in 80ec9f2b6, but the changeset and the ADR-0087 ledger entry still described the pre-scoping full-table gate. A breaking-change record that overstates what changed tells every consumer reading the release notes that behaviour was withdrawn which was not. F1 — the breaking declaration: - changeset FROM/TO: the two `percent` rows dropped (`sum` x `percent` is a table row this leg does not execute; `avg` x `percent` was never a migration at all), and the surviving `avg` row widened to name all three temporal members it does refuse. - changeset: a new scope section states the temporal scope outright, and that the string rows sit under #16785 (ruled C - the table is to be AMENDED to accept them), the boolean rows were settled as ACCEPT by #16685 / #16750, and `sum` x `percent` is not executed here. - ledger `surface`: scoped to the temporal class; "sum over a percent" removed. - ledger `replacement`: the "`avg` for a `percent`" prescription dropped with the surface rows it belonged to. - ledger `acceptanceCriteria`: qualified to a measure over a `date` / `datetime` / `time` field, and says outright that a field of any other class is neither refused nor certified by this leg. - `registry.ts` REGENERATED with `pnpm --filter @objectstack/spec gen:migration-registry`, never hand-edited; two consecutive runs are byte-identical and the diff is confined to this entry's block. - also corrected: the changeset said "four contradictory annotations"; the sweep reconciled seven source sites plus two test narratives. F2 — the boolean collision is settled, so stop narrating it as live. #16685 was ruled A and #16750 added `boolean` / `toggle` to the four arithmetic / order rows, so the table ACCEPTS them. Reworded in `dataset-compiler.ts`'s scope docblock, the refusal suite's module header, and `measure-result-type.ts` (whose boolean paragraph still referred a missing refusal onward). All three now also record #16785 C for the string rows. F3 — the ledger `reason` presented both dialect halves as measured alike. The SQLite half is pinned by a live `sql.js` suite; the Postgres 42883 half was measured in-session and is pinned by nothing. Said so where it is stated. F4 — the scope-boundary test asserted `isAggregateCompatibleWithFieldType( 'min', 'text') === false`, a verdict #16785 C is about to amend. Dropped: the case now pins only what this PR owns - a non-temporal field is not judged, so the measure compiles and SQL is emitted. Refutability is carried by a second case on `sum` x `text`, a row no ruling is moving, plus a non-vacuity assertion that SQL reached the driver in both. F5 — the changeset now names the two uncovered faces: `/analytics/query` and any `compileDataset` caller wiring no `declaredFieldType` probe. Refs #16737. Review: PR #16778 contract review, comment 5580295870. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...et-measure-aggregate-field-type-refused.md | 42 ++++++++++--- ...aggregate-datetime-measure-refusal.test.ts | 61 +++++++++++++------ .../service-analytics/src/dataset-compiler.ts | 39 ++++++------ .../src/measure-result-type.ts | 13 +++- ...et-measure-aggregate-field-type-refused.ts | 33 +++++++--- packages/spec/src/migrations/registry.ts | 33 +++++++--- 6 files changed, 154 insertions(+), 67 deletions(-) diff --git a/.changeset/dataset-measure-aggregate-field-type-refused.md b/.changeset/dataset-measure-aggregate-field-type-refused.md index 7f6a0895e7..c6826dbf69 100644 --- a/.changeset/dataset-measure-aggregate-field-type-refused.md +++ b/.changeset/dataset-measure-aggregate-field-type-refused.md @@ -58,14 +58,34 @@ schema, not `os validate` / `os lint`, not the analytics service, not the render `sourceFieldMeta`, an unresolvable field, or a `relationship.field` path (whose column lives on a joined object) leaves the pair unjudged. +## ⚠️ Scope: the compile leg executes the TEMPORAL rows only + +The gate judges only a measure whose field is declared `date` / `datetime` / +`time`; a field of any other class is never handed to the predicate. The +verdict for the pairs it does judge is the table's — no row is restated — but +which FIELDS are judged is narrower than the table, on purpose: + +- **String rows** (`min` / `max` over `text`, `select`, `lookup`, + `autonumber`, …) are **not enforced here**. They are under #16785, **ruled + C**: the table itself is to be amended to accept them, because + `measureResultType` (#15768) already types those results as `'string'` and + pins them end to end. Enforcing them from this card would pre-empt that + ruling. +- **Boolean rows** are not a refusal at all any more: #16685 was ruled A and + #16750 added `boolean` / `toggle` to `sum` / `avg` / `min` / `max`, so the + table ACCEPTS them and this gate never judged them. +- The table's `sum` × `percent` row is likewise **not** executed by this leg; + `sum` over a `percent` compiles exactly as it did before. + +⇒ The only pairs whose behaviour changes in this release are `avg` / `sum` +over a `date` / `datetime` / `time` field. The full-table leg remains #16099's. + ## FROM → TO | you wrote | write instead | |:--|:--| -| `{ aggregate: 'avg', field: }` | `{ aggregate: 'min' \| 'max', field: }` — a real instant of the field's own type | +| `{ aggregate: 'avg', field: }` | `{ aggregate: 'min' \| 'max', field: }` — a real instant of the field's own type | | `{ aggregate: 'sum', field: }` | store the duration as a number (a computed "days open" field) and `sum`/`avg` that | -| `{ aggregate: 'avg', field: }` | unchanged — `avg` accepts `percent` | -| `{ aggregate: 'sum', field: }` | `avg`, or sum the underlying amounts — a rate does not add | | `derived: { op: 'difference', of: ['avg_a', 'avg_b'] }` over temporal averages | fix the two operand measures; the `derived` spec itself is unchanged | ⭐ A duration is not recoverable from an aggregate over instants on any backend. @@ -79,10 +99,18 @@ filtering — is unchanged; this is about aggregation only. `avg` over a genuine numeric measure, `min` / `max` over a temporal one, and `count` / `count_distinct` over anything all behave exactly as before. -Alongside the refusal, `service-analytics`' four contradictory annotations about -what a SQLite `Field.datetime` column physically holds are reconciled to one -statement. Two said it holds an INTEGER epoch and ISO TEXT at once; one said flatly -that it IS an INTEGER epoch. Neither is current: since #3912 the column has ONE +⚠️ **Two faces stay uncovered, deliberately.** The refusal lives in +`compileDataset` and reads a `declaredFieldType` probe, so it applies only where +a host wires one: `/analytics/query` — the non-dataset face, whose measures a +Cube infers rather than an author declaring them — is NOT covered, and neither +is any other `compileDataset` caller that passes no probe (those stand down +unjudged rather than guessing). Closing those is #16099's, not this card's. + +Alongside the refusal, `service-analytics`' contradictory annotations about what a +SQLite `Field.datetime` column physically holds are reconciled to one statement — +**seven** source sites plus two test narratives, not the four the card quoted. Some +said the column holds an INTEGER epoch and ISO TEXT at once; one said flatly that it +IS an INTEGER epoch. Neither is current: since #3912 the column has ONE storage form, canonical UTC text, with the epoch surviving only in a database not yet converged by `backfillCanonicalDatetimes`. The fact is now stated once, on `AnalyticsServiceConfig.coerceTemporalFilterValue`, and the other sites link to it. diff --git a/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts index c351452daf..a4ba274f76 100644 --- a/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts @@ -44,17 +44,19 @@ * ## ⚠️ The compile leg lands SCOPED to temporal source fields, and says why * * The verdict is the table's; what is scoped is which FIELDS the gate judges. - * Executing every row today refuses two families this platform answers on - * purpose, with tests: `min` / `max` over the STRING classes (typed `'string'` - * by `measureResultType`, #15768, and pinned end to end in - * `measure-result-type.test.ts`), and the boolean rows (maintainer ruling - * #11152 has booleans aggregate as numbers on every backend). The spec module's - * own header records both as OVERRIDES of existing opinions rather than - * agreement, and refers the boolean one back to the maintainer. Refusing them - * would break uses that work today — a product judgement, and #16099's - * full-table leg is where it belongs. The temporal rows carry no such - * collision, which is why they are the ones executed here. The last suite in - * this file pins that boundary so it cannot widen by accident. + * Executing every row of the table today would refuse `min` / `max` over the + * STRING classes — typed `'string'` by `measureResultType` (#15768) and pinned + * end to end in `measure-result-type.test.ts` — which this platform answers on + * purpose. #16785 has since **ruled C** on those rows: the TABLE is to be + * amended to accept them, so enforcing them from here would pre-empt a ruling + * that goes the other way. The BOOLEAN rows are settled and are not a collision + * any more: #16685 was ruled A and #16750 added `boolean` / `toggle` to + * `sum` / `avg` / `min` / `max` (maintainer ruling #11152 — booleans aggregate + * as numbers on every backend), so the table ACCEPTS them and nothing refuses + * them anywhere. The temporal rows carry no such counter-evidence, which is why + * they are the ones executed here; the full-table leg stays with #16099. The + * last suite in this file pins that boundary so it cannot widen by accident — + * ⛔ without pinning any verdict the table itself is still under ruling for. * * ## Dissolution verification — direction predicted BEFORE running * @@ -480,20 +482,39 @@ describe('#16737 — the gate stands down rather than guessing', () => { // ───────────────────────────────────────────────────────────────────────────── describe('#16737 — the compile leg is scoped to temporal source fields, on purpose', () => { - it('a `min` over a TEXT field still compiles here, though the table refuses the pair', async () => { - // ⚠️ Not an endorsement of the pair — a statement about WHO refuses it. - // `measureResultType` (#15768) types this result as `'string'` and - // `measure-result-type.test.ts` pins it end to end, so enforcing the - // table's string rows is a product judgement that belongs to #16099, not a - // side effect of this card. The table's verdict is asserted directly, so - // this case reads as "the contract says no, this gate does not act on it". - expect(isAggregateCompatibleWithFieldType('min', 'text')).toBe(false); - const { svc } = makeService([{ status: 'open', first_note: 'Archive the backlog' }]); + it('a `min` over a TEXT field still compiles here — WHATEVER the table says about that pair', async () => { + // ⚠️ Not an endorsement of the pair, and ⛔ deliberately NOT a pin on the + // table's verdict for it. `min` × `text` is the row #16785 is ruled C on — + // the table is to be AMENDED to accept it — so asserting today's `false` + // here would make this file go red when that ruling lands, and would make a + // test of this card the thing standing in the way of a decision this card + // does not own. What this case owns is one fact, true on either side of + // that amendment: a NON-TEMPORAL field is not judged by this gate, so the + // measure compiles and reaches the driver. + const { svc, sqls } = makeService([{ status: 'open', first_note: 'Archive the backlog' }]); const result: any = await svc.queryDataset( dataset([{ name: 'first_note', aggregate: 'min', field: 'note' }]), { dimensions: ['status'], measures: ['first_note'] }, ); expect(result.rows[0].first_note).toBe('Archive the backlog'); + // Not vacuous: the gate refuses BEFORE any SQL, so a widened gate would + // leave `sqls` empty and this assertion is what would catch it. + expect(sqls.length).toBe(1); + }); + + it('a `sum` over a TEXT field also compiles here — the boundary, on a row no ruling is moving', async () => { + // The refutable half of the boundary. #16785 C amends only `min` / `max` + // for the string classes, so `sum` × `text` stays a pair the TABLE refuses + // and this GATE does not act on — before that amendment and after it. If + // the gate ever widened past the temporal class, this case goes red. + expect(isAggregateCompatibleWithFieldType('sum', 'text')).toBe(false); + const { svc, sqls } = makeService([{ status: 'open', note_sum: 0 }]); + const result: any = await svc.queryDataset( + dataset([{ name: 'note_sum', aggregate: 'sum', field: 'note' }]), + { dimensions: ['status'], measures: ['note_sum'] }, + ); + expect(result.rows.length).toBe(1); + expect(sqls.length).toBe(1); }); it('every temporal member IS judged — the scope is the class, not the one type the card named', async () => { diff --git a/packages/services/service-analytics/src/dataset-compiler.ts b/packages/services/service-analytics/src/dataset-compiler.ts index d4570fabe2..2f7b2c6faf 100644 --- a/packages/services/service-analytics/src/dataset-compiler.ts +++ b/packages/services/service-analytics/src/dataset-compiler.ts @@ -254,24 +254,29 @@ function aggregateToMetricType(m: DatasetMeasure): Metric['type'] { * platform currently answers, on purpose, with tests: * * - `min` / `max` over the STRING classes (`text`, `select`, `lookup`, - * `autonumber`, …). `measureResultType` (#15768) types exactly those results - * as `'string'`, and `__tests__/measure-result-type.test.ts` pins them end to - * end through `queryDataset` — 15 cases that go red the moment those rows are - * enforced. The spec module's own header records this as an OVERRIDE of an - * existing opinion rather than agreement with it. - * - `sum` / `avg` / `min` / `max` over `boolean` / `toggle`. Maintainer ruling - * #11152 pins booleans aggregating as numbers on every backend - * (`AGGREGATION_CASES`), and the spec header refers the collision between - * that ruling and batch #59 back to the maintainer as its own decision. + * `autonumber`, …) — refused by the table, and ⛔ NOT enforced here. + * `measureResultType` (#15768) types exactly those results as `'string'`, and + * `__tests__/measure-result-type.test.ts` pins them end to end through + * `queryDataset` — 15 cases that would go red the moment those rows were + * enforced. #16785 has since **ruled C** on exactly this: the TABLE is to be + * amended to accept `min` / `max` over the string classes. Enforcing them + * from here would pre-empt a ruling that goes the other way. + * - `boolean` / `toggle` are no longer a collision at all, and this note no + * longer refers one to the maintainer. #16685 was ruled A and #16750 added + * both members to the `sum` / `avg` / `min` / `max` rows, on the authority of + * maintainer ruling #11152 (booleans aggregate as NUMBERS on every backend, + * pinned by `AGGREGATION_CASES`). The table ACCEPTS them — so there is + * nothing here to refuse, and this gate never judged them either way: they + * are outside the temporal class. * - * Refusing those would break uses that work today, which is a product judgement - * and not a dev's to take mid-flight. The temporal rows carry no such - * collision, and were measured on both dialects before this gate was written: - * SQLite answers a silent average YEAR, Postgres refuses at 42883, no shipped - * dataset in this repo pairs them, and there is no reading on which the mean of - * a set of instants is a duration. So the temporal rows are executed and the - * remainder stays with #16099, whose full-table leg is blocked on those two - * collisions being ruled. + * Enforcing the string rows would break uses that work today, and #16785 ruled + * that they be AMENDED rather than executed. The temporal rows carry no such + * counter-evidence, and were measured on both dialects before this gate was + * written: SQLite answers a silent average YEAR, Postgres refuses at 42883, no + * shipped dataset in this repo pairs them, and there is no reading on which the + * mean of a set of instants is a duration. So the temporal rows are executed + * here and the full-table leg stays with #16099 — now waiting on #16785's + * amendment landing, not on two unruled collisions. * * ## Tiering — "cannot answer, do not block", the same as every sibling probe * diff --git a/packages/services/service-analytics/src/measure-result-type.ts b/packages/services/service-analytics/src/measure-result-type.ts index 16894f5439..b53ada35ef 100644 --- a/packages/services/service-analytics/src/measure-result-type.ts +++ b/packages/services/service-analytics/src/measure-result-type.ts @@ -126,9 +126,16 @@ import { * `DimensionType` does carry a `boolean` word, so a correction is SPELLABLE * here — which is exactly why it is not made: spelling it would ship one of * three disagreeing readings as a published declaration. The column keeps the - * `number` it has (the accurate word for the raw SQLite value), and the - * missing refusal is owned by the `needs-user-decision` card for "no layer - * refuses an incoherent aggregate / field-type pair". + * `number` it has (the accurate word for the raw SQLite value). + * + * ⚠️ ⛔ This is NOT a missing refusal, and it is no longer referred onward. + * `AGGREGATE_FIELD_TYPE_COMPATIBILITY` ACCEPTS `sum` / `avg` / `min` / `max` + * over `boolean` / `toggle` — #16685 ruled A, landed as #16750, on the + * authority of maintainer ruling #11152 — so the pair is deliberately allowed + * and the compile leg (#16737) never judges it. What stays open here is only + * the RESULT-TYPE question above: three readings that disagree about what the + * one answering backend reports. A refusal would not settle it, and none is + * owed. * * ### The JSON-column classes: array- and object-valued types * diff --git a/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts b/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts index b2dafc12fc..bef4ca069f 100644 --- a/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.dataset-measure-aggregate-field-type-refused.ts @@ -5,15 +5,18 @@ import type { SemanticMigration } from '../../types.js'; export const entry: SemanticMigration = { id: 'dataset-measure-aggregate-field-type-refused', surface: 'dataset measure `aggregate` × `field` pairs (`DatasetMeasureSchema`, the rows ' - + 'inside `Dataset.measures[]`) whose aggregate the field\'s declared `FieldType` ' - + 'cannot carry — notably `avg` / `sum` over a `date` / `datetime` / `time` field, ' - + 'and `sum` over a `percent`', + + 'inside `Dataset.measures[]`) over a TEMPORAL field — `date`, `datetime`, `time` — ' + + 'whose aggregate that declared `FieldType` cannot carry: `avg` and `sum` over any of ' + + 'the three. ⛔ The compile leg is scoped to that class and to nothing else: the ' + + 'table\'s string rows are under #16785 (ruled C — the table itself is to be amended ' + + 'to accept `min` / `max` over them) and its `sum` × `percent` row is not executed ' + + 'here either, so no non-temporal pair changes behaviour', replacement: 'an aggregate the field\'s type accepts, per ' + '`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`, #16353): ' + '`min` / `max` for a temporal field — both return a real instant of the field\'s own ' - + 'type — `avg` for a `percent`, `count` / `count_distinct` for anything. A DURATION ' - + 'is not recoverable from an aggregate over instants: store it as a number (a ' - + 'computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + + 'type — or `count` / `count_distinct`, which read no arithmetic off the value. ' + + 'A DURATION is not recoverable from an aggregate over instants: store it as a ' + + 'number (a computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + 'names a refused measure is fixed by fixing that measure, not the `derived` one', reason: '#16737 / #16099. Nothing between the author and the driver correlated a measure\'s ' @@ -23,8 +26,14 @@ export const entry: SemanticMigration = { + 'canonical UTC text to a number by reading its leading digits, so ' + '`avg(submitted_at)` over 2026-05 and 2025-01 returns `2025.5` — the average YEAR, ' + 'no error, no log; PostgreSQL 16 answers `function avg(timestamp with time zone) ' - + 'does not exist` (SQLSTATE 42883). ⭐ The silent half is the dangerous one, and it ' - + 'is the DEV default: `derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + + 'does not exist` (SQLSTATE 42883). ⚠️ The two halves are not evidenced alike: the ' + + 'SQLite half is PINNED by a live `sql.js` suite in ' + + '`__tests__/aggregate-datetime-measure-refusal.test.ts`, while the Postgres half was ' + + 'MEASURED IN-SESSION on PostgreSQL 16.13 and is not pinned by any test — the live PG ' + + 'conformance job carries no cell for it. Nothing depends on it: the refusal is ' + + 'decided from declared metadata before a driver is reached. ⭐ The silent half is ' + + 'the dangerous one, and it is the DEV default: ' + + '`derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + 'such averages rendered `-0.85` on a tile labelled "average cycle time delta" — ' + 'indistinguishable from a correct answer, which is the shape Prime Directive #12 ' + 'exists to remove. Which pairs are accepted is therefore a contract, declared once ' @@ -36,8 +45,12 @@ export const entry: SemanticMigration = { + '⚠️ A `date` / `datetime` used as a DIMENSION — grouping, bucketing, date-range ' + 'filtering — is untouched: this is about aggregation only.', acceptanceCriteria: - 'Every dataset measure pairs an `aggregate` with a `field` whose declared type that ' - + 'aggregate accepts. Accepted pairs compile and execute byte-identically to before ' + 'Every dataset measure over a `date` / `datetime` / `time` field pairs that field with ' + + 'an `aggregate` the temporal class accepts — `min`, `max`, `count`, `count_distinct` ' + + '— and none pairs it with `avg` or `sum`. ⛔ The criterion reaches no further: a ' + + 'measure over a field of any OTHER class is not judged by this leg at all, so a ' + + 'string, boolean, percent or numeric pair is neither refused nor certified here. ' + + 'Accepted pairs compile and execute byte-identically to before ' + '(`avg` over `number` / `currency`, `min` / `max` over `datetime`, `count` over ' + 'anything); a refused pair answers `400 DATASET_INVALID` naming the measure, the ' + 'field, its declared type and the accepted set, with no SQL emitted. The refusal ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index b2e6d9fe05..73e619e069 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6415,15 +6415,18 @@ const step18: MigrationStep = { { id: 'dataset-measure-aggregate-field-type-refused', surface: 'dataset measure `aggregate` × `field` pairs (`DatasetMeasureSchema`, the rows ' - + 'inside `Dataset.measures[]`) whose aggregate the field\'s declared `FieldType` ' - + 'cannot carry — notably `avg` / `sum` over a `date` / `datetime` / `time` field, ' - + 'and `sum` over a `percent`', + + 'inside `Dataset.measures[]`) over a TEMPORAL field — `date`, `datetime`, `time` — ' + + 'whose aggregate that declared `FieldType` cannot carry: `avg` and `sum` over any of ' + + 'the three. ⛔ The compile leg is scoped to that class and to nothing else: the ' + + 'table\'s string rows are under #16785 (ruled C — the table itself is to be amended ' + + 'to accept `min` / `max` over them) and its `sum` × `percent` row is not executed ' + + 'here either, so no non-temporal pair changes behaviour', replacement: 'an aggregate the field\'s type accepts, per ' + '`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`, #16353): ' + '`min` / `max` for a temporal field — both return a real instant of the field\'s own ' - + 'type — `avg` for a `percent`, `count` / `count_distinct` for anything. A DURATION ' - + 'is not recoverable from an aggregate over instants: store it as a number (a ' - + 'computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + + 'type — or `count` / `count_distinct`, which read no arithmetic off the value. ' + + 'A DURATION is not recoverable from an aggregate over instants: store it as a ' + + 'number (a computed "days open" field) and aggregate that. A `derived` measure whose `of` ' + 'names a refused measure is fixed by fixing that measure, not the `derived` one', reason: '#16737 / #16099. Nothing between the author and the driver correlated a measure\'s ' @@ -6433,8 +6436,14 @@ const step18: MigrationStep = { + 'canonical UTC text to a number by reading its leading digits, so ' + '`avg(submitted_at)` over 2026-05 and 2025-01 returns `2025.5` — the average YEAR, ' + 'no error, no log; PostgreSQL 16 answers `function avg(timestamp with time zone) ' - + 'does not exist` (SQLSTATE 42883). ⭐ The silent half is the dangerous one, and it ' - + 'is the DEV default: `derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + + 'does not exist` (SQLSTATE 42883). ⚠️ The two halves are not evidenced alike: the ' + + 'SQLite half is PINNED by a live `sql.js` suite in ' + + '`__tests__/aggregate-datetime-measure-refusal.test.ts`, while the Postgres half was ' + + 'MEASURED IN-SESSION on PostgreSQL 16.13 and is not pinned by any test — the live PG ' + + 'conformance job carries no cell for it. Nothing depends on it: the refusal is ' + + 'decided from declared metadata before a driver is reached. ⭐ The silent half is ' + + 'the dangerous one, and it is the DEV default: ' + + '`derived: { op: \'difference\', of: [avg_a, avg_b] }` over two ' + 'such averages rendered `-0.85` on a tile labelled "average cycle time delta" — ' + 'indistinguishable from a correct answer, which is the shape Prime Directive #12 ' + 'exists to remove. Which pairs are accepted is therefore a contract, declared once ' @@ -6446,8 +6455,12 @@ const step18: MigrationStep = { + '⚠️ A `date` / `datetime` used as a DIMENSION — grouping, bucketing, date-range ' + 'filtering — is untouched: this is about aggregation only.', acceptanceCriteria: - 'Every dataset measure pairs an `aggregate` with a `field` whose declared type that ' - + 'aggregate accepts. Accepted pairs compile and execute byte-identically to before ' + 'Every dataset measure over a `date` / `datetime` / `time` field pairs that field with ' + + 'an `aggregate` the temporal class accepts — `min`, `max`, `count`, `count_distinct` ' + + '— and none pairs it with `avg` or `sum`. ⛔ The criterion reaches no further: a ' + + 'measure over a field of any OTHER class is not judged by this leg at all, so a ' + + 'string, boolean, percent or numeric pair is neither refused nor certified here. ' + + 'Accepted pairs compile and execute byte-identically to before ' + '(`avg` over `number` / `currency`, `min` / `max` over `datetime`, `count` over ' + 'anything); a refused pair answers `400 DATASET_INVALID` naming the measure, the ' + 'field, its declared type and the accepted set, with no SQL emitted. The refusal '