diff --git a/.changeset/analytics-measure-result-type-temporal.md b/.changeset/analytics-measure-result-type-temporal.md new file mode 100644 index 0000000000..8f3511e6f9 --- /dev/null +++ b/.changeset/analytics-measure-result-type-temporal.md @@ -0,0 +1,24 @@ +--- +"@objectstack/service-analytics": minor +"@objectstack/spec": patch +--- + +A dataset measure's `fields[].type` stops contradicting the value beside it: a `min`/`max` over a temporal field is described as `time`, not `number` (#15768) + +`POST /api/v1/analytics/dataset/query` described **every** measure column as `type: "number"`, including a `min`/`max` over a `date` / `datetime` / `time` field whose value in the same response is an ISO instant. Measured on a real boot (`@objectstack/cli` 17.3.0, SQLite dev datasource): + +```json +{"rows":[{"oldest_last_update_at":"2026-07-04T07:00:00.000Z"}], + "fields":[{"name":"oldest_last_update_at","type":"number","label":"Oldest touch","format":"relative"}]} +``` + +`min` and `max` return a value **of the aggregated field's own type**, so that column carries an instant and the metadata denied it — which is enough on its own to keep a formatter that branches on the declared type from ever reaching a temporal branch. + +What changed: + +- **The measure column's type is resolved from the authored measure plus the source field's declared type**, in `AnalyticsService.queryDataset`'s ADR-0021 result-column enrichment — the same block that already resolves `label` / `format` / `currency` / `percentScale`, and the one seam every producer of the shape passes through on the way to the route, which relays that method's return verbatim. The rule itself is `measureResultType` in the new `measure-result-type.ts`, so the per-aggregate verdict has one home instead of four copies. +- **The corrected spelling is `time`**, the `DimensionType` word a temporal DIMENSION column in the same response has always carried. A second temporal word in one wire position would have left every existing consumer branch unreached. +- **Only `min` and `max` move.** `count` and `count_distinct` are numeric however temporal the column they read is; `sum` / `avg` over a temporal column are refused by no layer and answered by the backend (an epoch mean on SQLite, an error on Postgres), so there is no single value for a type to describe and none is invented; a derived measure is numeric by construction, because `computeDerived` coerces its operands with `Number()`. Row values are untouched on every path. +- **Tiered "cannot answer, do not block".** A host with no source-field metadata wired, and a measure over a relationship PATH (which the source-field lookup resolves against the base object and therefore cannot answer), both leave the column exactly as the query layer produced it. + +`AnalyticsResult.fields[].type` and the `AnalyticsResultResponse` schema now state the vocabulary this position speaks and what each aggregate answers; neither declaration widens — the wire type was, and remains, a string. 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 new file mode 100644 index 0000000000..b4f65a2ee9 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/measure-result-type.test.ts @@ -0,0 +1,344 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15768 — a dataset measure's `fields[].type` must describe the value sitting + * beside it in the same response. + * + * Measured on a real boot (`@objectstack/cli` 17.3.0, SQLite dev datasource), + * `POST /api/v1/analytics/dataset/query` answered a `min` over a + * `Field.datetime` column with: + * + * ```json + * {"rows":[{"oldest_last_update_at":"2026-07-04T07:00:00.000Z","untouched_over_30d":3}], + * "fields":[{"name":"oldest_last_update_at","type":"number","label":"Oldest touch","format":"relative"}, + * {"name":"untouched_over_30d","type":"number","label":"Untouched > 30 days"}]} + * ``` + * + * The value is an ISO instant; the metadata beside it says `number`. Every + * producer of this shape minted a flat `'number'` for every measure, so a + * renderer that branches on the declared type could never reach a temporal + * branch for the column. + * + * ## Where the assembly point is, and how this file proves it is the real one + * + * The triage seat recorded that it could not find the production code behind + * `fields[].type` — its grep landed only on test constants. There are FOUR + * producers of the measure descriptor, not one: + * + * - `ObjectQLStrategy.buildFieldMeta` (`strategies/objectql-strategy.ts`) + * - `NativeSQLStrategy.buildFieldMeta` (`strategies/native-sql-strategy.ts`) + * - `evaluateAnalyticsQueryOverRows` (`preview-evaluator.ts`, draft preview) + * - `DatasetExecutor.runMeasurePass` + its compare / derived appends + * + * — each spelling `{ name: m, type: 'number' }` and none of them knowing the + * aggregated field's declared type. What they all pass through is + * `AnalyticsService.queryDataset`'s ADR-0021 result-column enrichment, the same + * block that already resolves `label` / `format` / `currency` / `percentScale` + * from the AUTHORED measure plus `sourceFieldMeta`; the REST face relays that + * method's return verbatim (`res.json(result)` in `rest-server.ts`, the + * `POST {basePath}/analytics/dataset/query` route). So the correction is made + * there, once. + * + * Section C is the CONTROL for that claim: the same selection is driven down + * the ObjectQL-aggregate path AND the native-SQL path — two different + * `buildFieldMeta` producers — and both move together, which is only possible + * if the value the wire carries is decided downstream of both. Section B drives + * the supplementary-sub-query producer (every base measure filter-scoped, the + * card's own shape) and the `__compare` producer for the same reason. + * + * ## Reverse verification, direction predicted BEFORE running + * + * Reverting ONLY the two-line call site in `analytics-service.ts` (leaving + * `measure-result-type.ts` in place) must turn RED every assertion that expects + * `'time'` — sections B and C — and leave section A (the rule in isolation) and + * section D (the columns the rule deliberately does not touch) GREEN. Ordinary + * direction: the change CORRECTS a value on existing entries, mints no column + * and removes no limb, so nothing downstream can gain or lose a finding. + * Predicted red: the four `'time'` cases. Measured: recorded in the PR body. + */ + +import { describe, it, expect } from 'vitest'; +import { AggregationFunction } from '@objectstack/spec/data'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { measureResultType, MEASURE_RESULT_TYPE_TEMPORAL } from '../measure-result-type.js'; + +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +// ───────────────────────────────────────────────────────────────────────────── +// A) the CLOSED aggregate vocabulary, enumerated rather than sampled +// ───────────────────────────────────────────────────────────────────────────── + +/** + * One row per member of `AggregationFunction`, with what the member answers and + * why. `overTemporal` is the verdict for a `date`/`datetime`/`time` source + * field; `undefined` means "this rule says nothing — the producer's `'number'` + * stands", which for four of the six members is the CORRECT answer rather than + * an omission. + */ +const VOCABULARY: ReadonlyArray<{ + fn: (typeof AggregationFunction.options)[number]; + overTemporal: string | undefined; + why: string; +}> = [ + { fn: 'count', overTemporal: undefined, why: 'a row count is a number however temporal the counted column is' }, + { fn: 'count_distinct', overTemporal: undefined, why: 'a cardinality is a number, same reason as count' }, + { fn: 'sum', overTemporal: undefined, why: 'unrefused and backend-decided over a temporal column — no single value to type' }, + { fn: 'avg', overTemporal: undefined, why: 'same as sum: an epoch mean on SQLite, a refusal on Postgres' }, + { fn: 'min', overTemporal: MEASURE_RESULT_TYPE_TEMPORAL, why: 'returns a value of the aggregated field own type' }, + { fn: 'max', overTemporal: MEASURE_RESULT_TYPE_TEMPORAL, why: 'returns a value of the aggregated field own type' }, +]; + +describe('A) measureResultType covers the whole closed AggregationFunction vocabulary', () => { + it('the table enumerates every declared member, and only declared members', () => { + // The exhaustiveness guard. A member ADDED to the spec enum lands here as a + // failure rather than silently falling through `measureResultType` as + // "nothing to say" — which is exactly how a new aggregate would inherit the + // flat `number` this card is about. + expect([...VOCABULARY.map((v) => v.fn)].sort()).toEqual([...AggregationFunction.options].sort()); + }); + + for (const { fn, overTemporal, why } of VOCABULARY) { + it(`${fn} over a temporal field → ${overTemporal ?? 'no correction'} (${why})`, () => { + expect(measureResultType(fn, 'datetime')).toBe(overTemporal); + expect(measureResultType(fn, 'date')).toBe(overTemporal); + expect(measureResultType(fn, 'time')).toBe(overTemporal); + }); + + it(`${fn} over a NUMBER field is never corrected`, () => { + expect(measureResultType(fn, 'number')).toBeUndefined(); + expect(measureResultType(fn, 'currency')).toBeUndefined(); + }); + } + + it('a derived measure (no aggregate) is never corrected — computeDerived coerces with Number()', () => { + expect(measureResultType(undefined, 'datetime')).toBeUndefined(); + }); + + it('an unanswerable source field is left alone ("cannot answer, do not block")', () => { + // No data engine wired, an object/field the host does not know, or a + // relationship-path measure `sourceFieldMeta` cannot resolve. + expect(measureResultType('min', undefined)).toBeUndefined(); + expect(measureResultType('max', undefined)).toBeUndefined(); + }); + + it('min/max over a NON-temporal field is deliberately out of this rule population', () => { + // Still described as `number`, and still wrong for a text column — reported + // as its own finding rather than absorbed here. + expect(measureResultType('min', 'text')).toBeUndefined(); + expect(measureResultType('max', 'select')).toBeUndefined(); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// the fixture — the card shape: a `min` over a `Field.datetime` +// ───────────────────────────────────────────────────────────────────────────── + +const dataset = DatasetSchema.parse({ + name: 'task_metrics', + label: 'Task Metrics', + object: 'duly_task', + dimensions: [ + { name: 'status', field: 'status', type: 'string', label: 'Status' }, + // The dated axis `compareTo` shifts. Its own descriptor is the control in + // section D: a temporal DIMENSION column has always said `time`. + { name: 'touched_on', field: 'last_update_at', type: 'date', label: 'Touched' }, + ], + measures: [ + // The card's measure, verbatim in shape: `min` over a datetime, carrying a + // measure-scoped filter (which is what routes it down the supplementary + // sub-query producer). + { name: 'oldest_last_update_at', aggregate: 'min', field: 'last_update_at', label: 'Oldest touch', format: 'relative', filter: { status: 'open' } }, + // `max` over the same column, unfiltered → the primary `buildFieldMeta` producer. + { name: 'newest_last_update_at', aggregate: 'max', field: 'last_update_at', label: 'Newest touch' }, + // The controls that must NOT move. + { name: 'task_count', aggregate: 'count', label: 'Tasks' }, + { name: 'counted_touches', aggregate: 'count', field: 'last_update_at', label: 'Touched' }, + { name: 'summed_touches', aggregate: 'sum', field: 'last_update_at', label: 'Summed touches' }, + { name: 'avg_touch', aggregate: 'avg', field: 'last_update_at', label: 'Average touch' }, + { name: 'min_estimate', aggregate: 'min', field: 'estimate_hours', label: 'Smallest estimate' }, + { name: 'touch_ratio', derived: { op: 'ratio', of: ['counted_touches', 'task_count'] }, label: 'Touch ratio' }, + ], +}); + +/** `duly_task.last_update_at` is `Field.datetime`; `estimate_hours` is a number. */ +const sourceFieldMeta = (_object: string, field: string) => + field === 'last_update_at' + ? { type: 'datetime' } + : field === 'estimate_hours' + ? { type: 'number' } + : undefined; + +const OLDEST = '2026-07-04T07:00:00.000Z'; +const NEWEST = '2026-08-30T09:15:00.000Z'; + +/** The grid every fake producer below answers with. */ +const GRID = [{ status: 'open', oldest_last_update_at: OLDEST, newest_last_update_at: NEWEST, task_count: 3, counted_touches: 3, summed_touches: 12, avg_touch: 4, min_estimate: 2 }]; + +/** The ObjectQL-aggregate path — one `buildFieldMeta` producer. */ +function objectqlService() { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + sourceFieldMeta, + executeAggregate: async () => GRID.map((r) => ({ ...r })), + }); +} + +/** The native-SQL path — the OTHER `buildFieldMeta` producer. */ +function nativeSqlService() { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + sourceFieldMeta, + executeRawSql: async () => GRID.map((r) => ({ ...r })), + }); +} + +/** `name → type` for the response's column metadata. */ +function typeOf(fields: Awaited>['fields'], name: string) { + return fields.find((f) => f.name === name)?.type; +} + +// ───────────────────────────────────────────────────────────────────────────── +// B) the card's own shape, end to end through queryDataset +// ───────────────────────────────────────────────────────────────────────────── + +describe('B) a min/max over a datetime is described as temporal, not number', () => { + it('the measured response: the ISO value and its metadata no longer contradict each other', async () => { + const result = await objectqlService().queryDataset( + dataset, + { dimensions: ['status'], measures: ['oldest_last_update_at'] }, + CTX, + ); + // The value beside the metadata — an ISO instant, exactly as the card recorded. + expect(result.rows[0]?.oldest_last_update_at).toBe(OLDEST); + // …and the metadata now says so. This is the assertion the card is about. + expect(result.fields.find((f) => f.name === 'oldest_last_update_at')).toMatchObject({ + name: 'oldest_last_update_at', + type: 'time', + label: 'Oldest touch', + format: 'relative', + }); + }); + + it('the SUPPLEMENTARY-sub-query producer is covered: every base measure filter-scoped', async () => { + // With `oldest_last_update_at` the only measure and it carrying a filter, + // `runMeasurePass` issues no primary query at all and appends the measure + // descriptor itself. Same corrected type. + const result = await objectqlService().queryDataset( + dataset, + { dimensions: [], measures: ['oldest_last_update_at'] }, + CTX, + ); + expect(typeOf(result.fields, 'oldest_last_update_at')).toBe('time'); + }); + + it('the PRIMARY buildFieldMeta producer is covered: an unfiltered max', async () => { + const result = await objectqlService().queryDataset( + dataset, + { dimensions: ['status'], measures: ['newest_last_update_at'] }, + CTX, + ); + expect(result.rows[0]?.newest_last_update_at).toBe(NEWEST); + expect(typeOf(result.fields, 'newest_last_update_at')).toBe('time'); + }); + + it('the __compare producer is covered: a period-over-period column of a temporal measure', async () => { + const result = await objectqlService().queryDataset( + dataset, + { + dimensions: ['status'], + measures: ['newest_last_update_at'], + timeDimensions: [{ dimension: 'touched_on', dateRange: ['2026-08-01', '2026-08-31'] }], + compareTo: { kind: 'previousPeriod' as const, dimension: 'touched_on' }, + }, + CTX, + ); + // The compare column exists and carries the same corrected type as the base + // column it is meant to be subtracted from. + expect(typeOf(result.fields, 'newest_last_update_at__compare')).toBe('time'); + expect(typeOf(result.fields, 'newest_last_update_at')).toBe('time'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// C) the control that identifies the assembly point +// ───────────────────────────────────────────────────────────────────────────── + +describe('C) both strategy producers move together — the correction is downstream of both', () => { + it('ObjectQL-aggregate and native-SQL answer the same column metadata', async () => { + const selection = { dimensions: ['status'], measures: ['newest_last_update_at', 'task_count'] }; + const viaObjectql = await objectqlService().queryDataset(dataset, selection, CTX); + const viaNativeSql = await nativeSqlService().queryDataset(dataset, selection, CTX); + + const shape = (r: Awaited>) => + r.fields.map((f) => ({ name: f.name, type: f.type })); + + expect(shape(viaObjectql)).toEqual(shape(viaNativeSql)); + expect(typeOf(viaObjectql.fields, 'newest_last_update_at')).toBe('time'); + expect(typeOf(viaNativeSql.fields, 'newest_last_update_at')).toBe('time'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// D) what the rule deliberately leaves alone +// ───────────────────────────────────────────────────────────────────────────── + +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 () => { + const result = await objectqlService().queryDataset( + dataset, + { + dimensions: ['status'], + measures: ['task_count', 'counted_touches', 'summed_touches', 'avg_touch', 'touch_ratio'], + }, + CTX, + ); + // `count` over a datetime genuinely IS a number — a rule that typed it + // 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. + 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. + expect(typeOf(result.fields, 'touch_ratio')).toBe('number'); + }); + + it('min over a NUMBER field is untouched', async () => { + const result = await objectqlService().queryDataset( + dataset, + { dimensions: ['status'], measures: ['min_estimate'] }, + CTX, + ); + expect(typeOf(result.fields, 'min_estimate')).toBe('number'); + }); + + it('a temporal DIMENSION column keeps the `time` it always carried — one word, not two', async () => { + // The reason the corrected measure spelling is `time` and not `datetime`: + // this position already says `time` for a date axis, and both words in one + // wire position would leave every existing `time` branch unreached. + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + sourceFieldMeta, + executeAggregate: async () => [{ touched_on: '2026-07-04', task_count: 3 }], + }); + const result = await svc.queryDataset(dataset, { dimensions: ['touched_on'], measures: ['task_count'] }, CTX); + expect(typeOf(result.fields, 'touched_on')).toBe('time'); + }); + + it('a host that cannot answer for the field leaves the column exactly as produced', async () => { + const blind = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + // No `sourceFieldMeta` at all — the "no data engine wired" tier. + executeAggregate: async () => GRID.map((r) => ({ ...r })), + }); + const result = await blind.queryDataset( + dataset, + { dimensions: ['status'], measures: ['newest_last_update_at'] }, + CTX, + ); + expect(typeOf(result.fields, 'newest_last_update_at')).toBe('number'); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 008dbddee2..978d82eaa1 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -33,6 +33,11 @@ import { // docblock for why the edge is acyclic and why it was worth adding. import { matchMissingColumnOfRelation } from '@objectstack/types'; import { CubeRegistry } from './cube-registry.js'; +// [#15768] The measure result-type rule — which aggregates return a value of +// the aggregated field's own type, and which are numeric whatever they read. +// Owned in its own module so the enumerated verdict per `AggregationFunction` +// member has one home rather than being inlined at the enrichment site. +import { measureResultType } from './measure-result-type.js'; import type { AnalyticsStrategy, AnalyticsDriverCapabilities, StrategyContext, DatasetScopedStrategyContext, DatasetScope } from './strategies/types.js'; import { NativeSQLStrategy } from './strategies/native-sql-strategy.js'; import { ObjectQLStrategy } from './strategies/objectql-strategy.js'; @@ -1426,6 +1431,23 @@ export class AnalyticsService implements IAnalyticsService { if (f.percentScale == null) { f.percentScale = m.derived?.op === 'ratio' ? 'fraction' : percentScaleOf(meta); } + // [#15768] The column's TYPE, which until now every producer of this + // shape minted as a flat `'number'` for every measure — so a `min`/`max` + // over a `date`/`datetime`/`time` field described an ISO instant as a + // number, in the same line that carried the instant. `measureResultType` + // owns the enumerated verdict per `AggregationFunction` member and + // answers `undefined` for every column it has nothing to say about, so + // the producer's own value stands unless the rule actively corrects it. + // + // Corrected HERE rather than in the four producers for the reason the + // display chains above are: this is the one seam every path to + // `POST /analytics/dataset/query` passes through — the route relays this + // method's return verbatim (`res.json(result)`) — and it is the only one + // holding both halves of the question, the AUTHORED measure (`aggregate` + // + `field`) and the source field's declared type. A per-producer copy + // would be four implementations of one rule, free to drift. + const resultType = measureResultType(m.aggregate, meta?.type); + if (resultType) f.type = resultType; } } diff --git a/packages/services/service-analytics/src/measure-result-type.ts b/packages/services/service-analytics/src/measure-result-type.ts new file mode 100644 index 0000000000..dbb5053da1 --- /dev/null +++ b/packages/services/service-analytics/src/measure-result-type.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { AggregationFunction } from '@objectstack/spec/data'; + +/** + * #15768 — what a dataset MEASURE column's `fields[].type` should say. + * + * Every producer of `AnalyticsResult.fields` mints `{ name, type: 'number' }` + * for a measure — `ObjectQLStrategy.buildFieldMeta`, `NativeSQLStrategy. + * buildFieldMeta`, `evaluateAnalyticsQueryOverRows` (draft preview) and + * `DatasetExecutor.runMeasurePass`'s supplementary/compare/derived appends. + * That is true for most of the aggregate vocabulary and false for exactly one + * corner of it, where the response then contradicts itself in one line: + * + * ```json + * {"rows":[{"oldest_last_update_at":"2026-07-04T07:00:00.000Z"}], + * "fields":[{"name":"oldest_last_update_at","type":"number", … }]} + * ``` + * + * `min`/`max` return a value OF THE AGGREGATED FIELD'S OWN TYPE. Over a + * `date` / `datetime` / `time` field that value is an instant, a calendar day + * or a clock time — never a number — so a renderer that branches on the + * declared type never reaches its temporal branch and falls through to a + * numeric default. + * + * ## The population, enumerated rather than sampled + * + * `AggregationFunction` (`spec/data/query.zod.ts`) is a CLOSED vocabulary, so + * "which aggregates does this rule speak about" has a finite answer, and every + * member is answered here — no member falls through unconsidered: + * + * | aggregate | result value | verdict | + * |:-----------------|:--------------------------------------|:--------| + * | `count` | a row count | `number` — unchanged. Counting `datetime`s is still counting. | + * | `count_distinct` | a cardinality | `number` — unchanged, same reason. | + * | `sum` | see below | `number` — unchanged. | + * | `avg` | see below | `number` — unchanged. | + * | `min` | a value of the aggregated field's type | temporal source ⇒ {@link MEASURE_RESULT_TYPE_TEMPORAL}. | + * | `max` | a value of the aggregated field's type | temporal source ⇒ {@link MEASURE_RESULT_TYPE_TEMPORAL}. | + * + * A measure with NO aggregate is a `derived` one (the two are mutually + * exclusive in `DatasetMeasureSchema`). `computeDerived` coerces every operand + * with `Number()`, so a derived measure is numeric BY CONSTRUCTION whatever its + * 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. + * + * ## The field-type axis, and what it deliberately excludes + * + * Only the TEMPORAL family is corrected: `date`, `datetime`, `time`. That is + * the population the card measured and the one the triage ruled on. `min`/`max` + * over a `text` / `select` / `lookup` field returns a string and is still + * described as `number` after this change — the same defect, a different + * population, reported separately rather than absorbed here. + * + * ## Why `'time'` and not `'date'` / `'datetime'` + * + * `fields[].type` is not a `FieldType` position. A temporal DIMENSION column in + * the very same response already carries `'time'` — `DatasetDimensionSchema`'s + * `type: 'date'` compiles to a cube dimension of `type: 'time'` + * (`dataset-compiler.dimensionType`), and both `buildFieldMeta`s copy that + * through. `'time'` is the `DimensionType` vocabulary this position already + * speaks (`string` / `number` / `boolean` / `time` / `geo`), so a consumer that + * can draw a date axis at all already has the branch. Spelling a temporal + * measure `'datetime'` would introduce a SECOND temporal word into one wire + * position and leave every existing consumer's `'time'` branch unreached. + */ + +/** + * The `DimensionType` word this position uses for a temporal column — the same + * one a `date` dimension column already carries in the same response. + */ +export const MEASURE_RESULT_TYPE_TEMPORAL = 'time'; + +/** + * Source-field types whose stored value is temporal (`FieldType`, `spec/data/ + * field.zod.ts` → "Date & Time"). `min`/`max` over one of these returns that + * same kind of value. + */ +export const TEMPORAL_SOURCE_FIELD_TYPES: ReadonlySet = new Set([ + 'date', + 'datetime', + 'time', +]); + +/** + * The corrected `fields[].type` for a measure column, or `undefined` for "this + * rule has nothing to say — keep whatever the producer minted". + * + * Tiered "cannot answer, do not block", the same way every other chain reading + * `sourceFieldMeta` is: an unknown field type, a host with no data engine + * wired, and a relationship-path measure (`account.closed_at`, which + * `sourceFieldMeta` cannot resolve because it looks a column up on the BASE + * object) all answer `undefined` and leave the column exactly as it was. + * + * @param aggregate - the measure's declared `aggregate`; absent on a `derived` + * measure. + * @param sourceFieldType - the DECLARED `FieldType` of the aggregated field, + * from `AnalyticsServiceConfig.sourceFieldMeta`. + */ +export function measureResultType( + aggregate: AggregationFunction | undefined, + sourceFieldType: string | undefined, +): string | undefined { + // `count` / `count_distinct` / `sum` / `avg` — and a derived measure's absent + // aggregate — all keep the `number` their producer minted. See the table above. + if (aggregate !== 'min' && aggregate !== 'max') return undefined; + if (sourceFieldType === undefined) return undefined; + return TEMPORAL_SOURCE_FIELD_TYPES.has(sourceFieldType) + ? MEASURE_RESULT_TYPE_TEMPORAL + : undefined; +} diff --git a/packages/spec/src/api/analytics.zod.ts b/packages/spec/src/api/analytics.zod.ts index 1194b2b5ad..6d18da7818 100644 --- a/packages/spec/src/api/analytics.zod.ts +++ b/packages/spec/src/api/analytics.zod.ts @@ -94,7 +94,14 @@ export const AnalyticsResultResponseSchema = lazySchema(() => BaseResponseSchema rows: z.array(z.record(z.string(), z.unknown())).describe('Result rows'), fields: z.array(z.object({ name: z.string(), - type: z.string(), + type: z.string().describe( + 'Column data type, in the `DimensionType` vocabulary (`string` / `number` / ' + + '`boolean` / `time` / `geo`). A dimension column carries its cube dimension\'s ' + + 'type; a measure column is `number` except for `min`/`max` over a ' + + '`date`/`datetime`/`time` field, which is `time` — those aggregates return a ' + + 'value of the aggregated field\'s own type. `count`/`count_distinct`, ' + + '`sum`/`avg` and derived measures stay numeric.', + ), label: z.string().optional() .describe('Human display label (e.g. measure `label`) — for legends/KPIs.'), format: z.string().optional() diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index 0b5ae561c1..ab1b7ed9a7 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -44,6 +44,32 @@ export interface AnalyticsResult { /** Column metadata */ fields: Array<{ name: string; + /** + * The column's data type, in the `DimensionType` vocabulary + * (`string` / `number` / `boolean` / `time` / `geo`) — one wire + * position, one vocabulary. + * + * A DIMENSION column carries its cube dimension's type, so a date axis + * has always said `time`. A MEASURE column says `number`, with ONE + * correction: `min`/`max` over a `date` / `datetime` / `time` field + * says `time` (#15768). Those two aggregates return a value OF THE + * AGGREGATED FIELD'S OWN TYPE, so the value beside the descriptor is an + * instant, a calendar day or a clock time — describing it as a number + * left a formatter that branches on this key unable to reach its + * temporal branch at all. + * + * Everything else in the closed `AggregationFunction` vocabulary is + * numeric whatever it reads and is unchanged: `count` / + * `count_distinct` count rows and values, `sum` / `avg` over a temporal + * column are refused by no layer and answered by the backend (an epoch + * mean on SQLite, an error on Postgres), so there is no one value for a + * type to describe. A derived measure is numeric by construction — + * `computeDerived` coerces its operands with `Number()`. + * + * The correction is tiered "cannot answer, do not block": a host with + * no source-field metadata, and a measure over a relationship PATH, + * both leave the column exactly as the query layer produced it. + */ type: string; /** Human display label (e.g. measure `label`) — for legends/KPIs. */ label?: string;