diff --git a/.changeset/16236-formula-return-type-measure-column.md b/.changeset/16236-formula-return-type-measure-column.md new file mode 100644 index 0000000000..f1430bdcdb --- /dev/null +++ b/.changeset/16236-formula-return-type-measure-column.md @@ -0,0 +1,49 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics): a `min`/`max` over a `formula` field is typed from the formula's declared `returnType`, not described as `number` (#16236) + +**Behaviour change — read this if any dataset measure aggregates a `formula` +field.** `AnalyticsResult.fields[].type` for such a measure column was always +`number`, whatever the formula computes. It is now translated from the field's +declared `FieldSchema.returnType`: + +``` +FROM {"rows":[{"first_label":"alpha","latest_due":"2026-06-01"}], + "fields":[{"name":"first_label","type":"number"}, + {"name":"latest_due","type":"number"}]} + +TO {"rows":[{"first_label":"alpha","latest_due":"2026-06-01"}], + "fields":[{"name":"first_label","type":"string"}, + {"name":"latest_due","type":"time"}]} +``` + +Both values were strings; both descriptors said `number`, so a renderer that +branches on the declared type never reached its textual or temporal branch. + +**The mapping is a TRANSLATION, not a pass-through.** `returnType` speaks the +authoring vocabulary (`number` / `text` / `boolean` / `date`); +`fields[].type` speaks `DimensionType` (`string` / `number` / `boolean` / +`time` / `geo`). Two of the four words do not exist on the wire at all: + +| declared `returnType` | `fields[].type` | +|:---|:---| +| `text` | `string` | +| `date` | `time` | +| `number` | unchanged — the producer's `number` is already correct | +| `boolean` | unchanged — three readings disagree on what `min`/`max` over a boolean returns | + +**A formula with no `returnType` is unchanged.** The key is optional — "absent +when the type can't be proven (an ambiguous/`dyn` expression)" — and an +unproven formula's measure column keeps the `number` it had. The absence is not +read as an answer. That tier is written down as a row in `measureResultType`'s +own table rather than left as an implied code path, and so is the treatment of +a word outside the declared four: left alone, never guessed at. + +**For hosts wiring `AnalyticsService` directly.** `AnalyticsServiceConfig`'s +`sourceFieldMeta` hook gains an optional fourth member on its return — +`returnType?: string` beside `type` / `defaultCurrency` / `max`. Additive: a +host that returns the three-member shape still satisfies the contract and gets +exactly today's behaviour for every column. `AnalyticsServicePlugin` relays the +key automatically, so a host on the plugin needs no change at all. diff --git a/packages/services/service-analytics/src/__tests__/formula-return-type-measure.test.ts b/packages/services/service-analytics/src/__tests__/formula-return-type-measure.test.ts new file mode 100644 index 0000000000..65ba426abd --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/formula-return-type-measure.test.ts @@ -0,0 +1,324 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16236 — a `min`/`max` over a `formula` field must be described by the type + * that formula was DECLARED to return. + * + * `FieldSchema.returnType` (`packages/spec/src/data/field.zod.ts`, located by + * text) declares the value type a formula computes, and its own JSDoc names + * this exact consumer FIRST: + * + * > Lets consumers — dataset measures, display formatting, validation — read a + * > declared type instead of re-parsing the expression. + * + * The dataset-measure side could not read it. The one channel from the host to + * `AnalyticsService` for a field's declared metadata is + * `AnalyticsServiceConfig.sourceFieldMeta`, whose return carried three members + * — `{ type?, defaultCurrency?, max? }` — and none of them was `returnType`. So + * `measureResultType` received the bare string `'formula'` and could say + * nothing further, and every formula measure column was described `number` + * whatever the formula computes. + * + * ## The reproduction, driven rather than read (section B) + * + * Two measures over two formula fields, one declared `returnType: 'text'` and + * one declared `returnType: 'date'`, driven end-to-end through + * `AnalyticsService.queryDataset` — the ADR-0021 result-column enrichment seam + * the REST face relays verbatim. Measured on `origin/main` @ `bea76c928` + * BEFORE this change: + * + * ```json + * {"rows":[{"first_label":"alpha","latest_due":"2026-06-01"}], + * "fields":[{"name":"first_label","type":"number","label":"First Label"}, + * {"name":"latest_due","type":"number","label":"Latest Due"}]} + * ``` + * + * Both values are strings; both descriptors say `number`. Same defect class as + * #15768/#16101, over the one `FieldType` member whose answer was already + * sitting in the metadata. + * + * ⚠️ **Why the pair is still reachable, re-measured rather than inherited.** + * The card's own "live control" (nothing between author and driver refuses the + * pair) was taken before the compile leg of #16099 landed, and + * `AGGREGATE_FIELD_TYPE_COMPATIBILITY` does NOT list `formula` under `min` or + * `max`. It is nonetheless not refused today: `dataset-compiler`'s + * `assertAggregateFieldTypeCompatible` executes only the TEMPORAL rows + * (`if (!TEMPORAL_SOURCE_FIELD_TYPES.has(fieldType)) return;`), by its own + * documented scope, and `formula` is outside that class. Section B is the + * standing control on that: it drives the pair through `queryDataset` and gets + * a response, so this rule's `formula` branch is reached by the tree as it + * ships. If the full-table leg is ever executed the branch becomes unreachable + * and inert — never wrong. + * + * ## ⚠️ The mapping is a TRANSLATION, and section A is the pin that keeps it one + * + * `returnType` speaks the AUTHORING vocabulary (`number` / `text` / `boolean` / + * `date`); `AnalyticsResult.fields[].type` speaks `DimensionType` (`string` / + * `number` / `boolean` / `time` / `geo`). Two of the four words are not wire + * words at all — `text` is `'string'` there, `date` is `'time'` — so relaying + * the literal would put a word into the response that no consumer branches on. + * + * The pin that stops a later edit degrading the table into a pass-through is + * NOT four hand-written expectations (a pass-through satisfies the two rows + * where the vocabularies happen to share a spelling). It is one invariant over + * the whole enum: **no member of `returnType` is answered by its own + * spelling.** All four members satisfy it today — `text`→`string`, + * `date`→`time`, and `number`/`boolean` answer "no correction" — so a relay + * reintroduced anywhere in the table fails it on every row it touches, + * including a fifth member added upstream later. + * + * The enum walked here is read off `FieldSchema` itself, so a member added to + * the spec lands as a failure of the exhaustiveness guard rather than silently + * falling through as "cannot answer". + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { FieldSchema } from '@objectstack/spec/data'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { + FORMULA_RETURN_TYPE_RESULT, + MEASURE_RESULT_TYPE_STRING, + MEASURE_RESULT_TYPE_TEMPORAL, + measureResultType, +} from '../measure-result-type.js'; + +/** + * The four members, read off the SPEC schema rather than restated here — the + * same construction the `FieldType` walk in `measure-result-type.test.ts` uses. + * A fifth member added upstream reds the exhaustiveness case below instead of + * quietly inheriting the "cannot answer" tier. + */ +const SPEC_RETURN_TYPES: readonly string[] = (() => { + const node = (FieldSchema as unknown as { shape: Record }).shape.returnType; + let inner = node as { options?: readonly string[]; unwrap?: () => unknown }; + for (let i = 0; i < 4 && inner && !inner.options; i++) { + inner = inner.unwrap?.() as typeof inner; + } + if (!inner?.options) throw new Error('could not read FieldSchema.returnType options from the spec schema'); + return inner.options; +})(); + +/** + * One row per member: the wire word this rule answers, and the MEASUREMENT + * behind it. `undefined` is a verdict here, never a gap — the two rows that + * carry it carry the reason the corresponding `FieldType` class already + * carries in `measure-result-type.ts`'s header. + */ +const RETURN_TYPE_VERDICTS: ReadonlyArray<{ returnType: string; expected: string | undefined; why: string }> = [ + { + returnType: 'text', + expected: MEASURE_RESULT_TYPE_STRING, + why: 'TRANSLATED — `text` is not a wire word; the DimensionType spelling for a string column is `string`', + }, + { + returnType: 'date', + expected: MEASURE_RESULT_TYPE_TEMPORAL, + why: 'TRANSLATED — `date` is not a wire word; the DimensionType spelling for a temporal column is `time`', + }, + { + returnType: 'number', + expected: undefined, + why: 'no correction — identical to the NUMERIC_VALUE_TYPES row: the producer already minted the correct word', + }, + { + returnType: 'boolean', + expected: undefined, + why: 'no correction — identical to the BOOLEAN_VALUE_TYPES row: three readings disagree on what min/max over a boolean returns', + }, +]; + +describe('A) the returnType → DimensionType table is a TRANSLATION, member by member', () => { + it('the table enumerates every member the spec declares, and only declared members', () => { + expect([...RETURN_TYPE_VERDICTS.map((v) => v.returnType)].sort()).toEqual([...SPEC_RETURN_TYPES].sort()); + expect([...Object.keys(FORMULA_RETURN_TYPE_RESULT)].sort()).toEqual([...SPEC_RETURN_TYPES].sort()); + }); + + it('⛔ NO member is answered by its own spelling — the anti-pass-through invariant', () => { + // The one assertion that a relayed literal cannot satisfy on ANY row, + // including a member added to the spec enum after this was written. + for (const member of SPEC_RETURN_TYPES) { + expect(measureResultType('min', 'formula', member), `returnType "${member}" relayed verbatim`).not.toBe(member); + expect(measureResultType('max', 'formula', member), `returnType "${member}" relayed verbatim`).not.toBe(member); + } + }); + + it('the two minted words are the DimensionType spellings, not the authoring ones', () => { + expect(MEASURE_RESULT_TYPE_STRING).toBe('string'); + expect(MEASURE_RESULT_TYPE_TEMPORAL).toBe('time'); + expect(measureResultType('min', 'formula', 'text')).not.toBe('text'); + expect(measureResultType('max', 'formula', 'date')).not.toBe('date'); + }); + + for (const { returnType, expected, why } of RETURN_TYPE_VERDICTS) { + it(`min/max over a formula declaring ${returnType} → ${expected ?? 'no correction'} (${why})`, () => { + expect(measureResultType('min', 'formula', returnType)).toBe(expected); + expect(measureResultType('max', 'formula', returnType)).toBe(expected); + expect(FORMULA_RETURN_TYPE_RESULT[returnType as keyof typeof FORMULA_RETURN_TYPE_RESULT]).toBe(expected); + }); + } + + it('⚠️ the ABSENT-returnType tier: no answer, the producer\'s word stands', () => { + // Written down as a row rather than left implied by a `?.` in the code + // path. `returnType` is optional — "absent when the type can't be proven + // (an ambiguous/`dyn` expression)" — and an unproven formula's measure + // column keeps the `number` its producer minted. The absence is NOT read + // as an answer. + expect(measureResultType('min', 'formula', undefined)).toBeUndefined(); + expect(measureResultType('max', 'formula', undefined)).toBeUndefined(); + expect(measureResultType('min', 'formula')).toBeUndefined(); + }); + + it('a returnType word outside the enum is the same "cannot answer" tier, never a guess', () => { + // A JS host, or an engine carrying a legacy spelling, can answer anything. + expect(measureResultType('min', 'formula', 'datetime')).toBeUndefined(); + expect(measureResultType('min', 'formula', 'string')).toBeUndefined(); + expect(measureResultType('min', 'formula', '')).toBeUndefined(); + }); + + it('returnType is consulted for `formula` ONLY — it is that member\'s declared key', () => { + // A host that answers `returnType` beside some other declared type changes + // nothing: the field-type tiers above decide first and this key is not a + // general override. + expect(measureResultType('min', 'text', 'date')).toBe(MEASURE_RESULT_TYPE_STRING); + expect(measureResultType('min', 'datetime', 'text')).toBe(MEASURE_RESULT_TYPE_TEMPORAL); + expect(measureResultType('min', 'number', 'text')).toBeUndefined(); + }); + + it('the non-min/max aggregates over a formula are never corrected', () => { + for (const fn of ['count', 'count_distinct', 'sum', 'avg'] as const) { + for (const member of SPEC_RETURN_TYPES) { + expect(measureResultType(fn, 'formula', member)).toBeUndefined(); + } + } + // …and a derived measure (no aggregate) likewise. + expect(measureResultType(undefined, 'formula', 'text')).toBeUndefined(); + }); +}); + +// ───────────────────────── end-to-end, through the real seam ───────────────── + +interface Task extends Record { + id: string; + label_calc: string; + due_calc: string; + amount: number; +} + +const ROWS: Task[] = [ + { id: 'r1', label_calc: 'alpha', due_calc: '2026-05-03', amount: 10 }, + { id: 'r2', label_calc: 'beta', due_calc: '2026-06-01', amount: 20 }, +]; + +const DATASET = DatasetSchema.parse({ + name: 'task_ds', + label: 'Task', + object: 'task', + dimensions: [], + measures: [ + { name: 'first_label', aggregate: 'min', field: 'label_calc', label: 'First Label' }, + { name: 'latest_due', aggregate: 'max', field: 'due_calc', label: 'Latest Due' }, + { name: 'total_amount', aggregate: 'sum', field: 'amount', label: 'Total Amount' }, + ], +}); + +const SELECTION = { dimensions: [], measures: ['first_label', 'latest_due', 'total_amount'] }; +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +/** Enough of an aggregate for this fixture — min/max compare as the rows store them. */ +function evaluateAggregate(opts: { aggregations?: unknown }) { + const aggs = (opts.aggregations ?? []) as Array<{ field: string; method: string; alias: string }>; + const row: Record = {}; + for (const a of aggs) { + const raw = ROWS.map((r) => r[a.field]); + const sorted = raw.map(String).sort(); + row[a.alias] = + a.method === 'sum' ? raw.reduce((s: number, v) => s + Number(v ?? 0), 0) + : a.method === 'max' ? sorted[sorted.length - 1] + : sorted[0]; + } + return [row]; +} + +/** + * A service wired the way a host wires it. `meta` is the ONLY thing that varies + * between the cases below, so a difference in the response is a difference the + * declared metadata caused. + */ +function svc(meta: (object: string, field: string) => Record | undefined) { + return new AnalyticsService({ + sourceFieldMeta: meta as never, + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (_o: string, options: Record) => evaluateAggregate(options), + }); +} + +const DECLARED = (object: string, field: string) => { + if (object !== 'task') return undefined; + if (field === 'label_calc') return { type: 'formula', returnType: 'text' }; + if (field === 'due_calc') return { type: 'formula', returnType: 'date' }; + if (field === 'amount') return { type: 'number' }; + return undefined; +}; + +/** The same two formula fields with the type left unproven at authoring. */ +const UNPROVEN = (object: string, field: string) => { + if (object !== 'task') return undefined; + if (field === 'label_calc' || field === 'due_calc') return { type: 'formula' }; + if (field === 'amount') return { type: 'number' }; + return undefined; +}; + +async function fieldsOf(meta: (o: string, f: string) => Record | undefined) { + const res = await svc(meta).queryDataset(DATASET, SELECTION, CTX); + return { + by: Object.fromEntries(res.fields.map((f) => [f.name, f])), + rows: res.rows, + }; +} + +describe('B) end-to-end — the card\'s own reproduction, through queryDataset', () => { + it('the pair is REACHABLE: the response exists and carries a value per measure', async () => { + // The standing control for the section-header note: the compile-time + // aggregate/field-type gate is scoped to the temporal class, so a formula + // measure is not refused and this rule's branch is really reached. + const { rows } = await fieldsOf(DECLARED); + expect(rows).toHaveLength(1); + expect(rows[0].first_label).toBe('alpha'); + expect(rows[0].latest_due).toBe('2026-06-01'); + }); + + it('a min over a formula declaring `text` is described `string`, beside a string value', async () => { + const { by, rows } = await fieldsOf(DECLARED); + expect(typeof rows[0].first_label).toBe('string'); + expect(by.first_label.type).toBe('string'); + // ⛔ and not the authoring word, and not the flat number it used to be. + expect(by.first_label.type).not.toBe('text'); + expect(by.first_label.type).not.toBe('number'); + }); + + it('a max over a formula declaring `date` is described `time`, beside a calendar day', async () => { + const { by, rows } = await fieldsOf(DECLARED); + expect(typeof rows[0].latest_due).toBe('string'); + expect(by.latest_due.type).toBe('time'); + expect(by.latest_due.type).not.toBe('date'); + expect(by.latest_due.type).not.toBe('number'); + }); + + it('a numeric measure beside them is untouched — the enrichment corrects, it does not repaint', async () => { + const { by } = await fieldsOf(DECLARED); + expect(by.total_amount.type).toBe('number'); + }); + + it('⚠️ an UNPROVEN formula keeps the producer\'s word — the absent tier, end to end', async () => { + const { by } = await fieldsOf(UNPROVEN); + expect(by.first_label.type).toBe('number'); + expect(by.latest_due.type).toBe('number'); + }); + + it('a host that wires no sourceFieldMeta at all is unchanged — "cannot answer, do not block"', async () => { + const res = await svc(() => undefined).queryDataset(DATASET, SELECTION, CTX); + for (const f of res.fields) expect(f.type).toBe('number'); + }); +}); 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 0a36d78328..ea2f2eeb15 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 @@ -118,23 +118,28 @@ const AGGREGATE_VOCABULARY: ReadonlyArray<{ * value (or on whether a value exists at all). Left uncorrected on purpose; * the missing refusal is owned by the `needs-user-decision` card for "no * layer refuses an incoherent aggregate / field-type pair". - * - `not-on-this-input` — an answer exists in the metadata but not on this - * rule's input (`formula.returnType`, which `sourceFieldMeta` does not - * carry). Filed rather than guessed. + * - `declared-elsewhere` — the answer is declared on a SECOND key rather than + * on the `FieldType` this axis walks. `formula` is the only member: its + * result type is `FieldSchema.returnType`, which `measureResultType` reads + * as a third input since #16236. On the declared `FieldType` ALONE — which + * is what this axis passes, and what a formula whose type could not be + * proven at authoring really presents — the verdict is no correction. The + * returnType axis has its own walk in + * `formula-return-type-measure.test.ts`. */ type FieldTypeBucket = | 'string' | 'temporal' | 'numeric-correct' | 'backend-dependent' - | 'not-on-this-input'; + | 'declared-elsewhere'; const EXPECTED_BY_BUCKET: Record = { string: MEASURE_RESULT_TYPE_STRING, temporal: MEASURE_RESULT_TYPE_TEMPORAL, 'numeric-correct': undefined, 'backend-dependent': undefined, - 'not-on-this-input': undefined, + 'declared-elsewhere': undefined, }; /** @@ -206,7 +211,7 @@ const FIELD_TYPE_VERDICTS: ReadonlyArray<{ { type: 'vector', bucket: 'backend-dependent', why: 'a number array in a JSON column' }, { type: 'json', bucket: 'backend-dependent', why: 'the untyped escape hatch — the value contract is explicitly open (z.unknown())' }, // ── answerable, but not from this rule's input ── - { type: 'formula', bucket: 'not-on-this-input', why: 'FieldSchema.returnType declares it, but sourceFieldMeta returns only { type, defaultCurrency, max } — and returnType is itself optional' }, + { type: 'formula', bucket: 'declared-elsewhere', why: 'the answer is FieldSchema.returnType, a SECOND key measureResultType takes as its third input (#16236); on the FieldType alone — an unproven formula — there is no correction' }, ]; describe('A) measureResultType covers both closed vocabularies, member by member', () => { @@ -229,7 +234,7 @@ describe('A) measureResultType covers both closed vocabularies, member by member it('every bucket is populated — the split is real, not three names for one branch', () => { const buckets = new Set(FIELD_TYPE_VERDICTS.map((v) => v.bucket)); expect([...buckets].sort()).toEqual([ - 'backend-dependent', 'not-on-this-input', 'numeric-correct', 'string', 'temporal', + 'backend-dependent', 'declared-elsewhere', 'numeric-correct', 'string', 'temporal', ]); }); @@ -346,7 +351,8 @@ const FIELD_TYPES: Record = { estimate_hours: 'number', // numeric-correct is_urgent: 'boolean', // backend-dependent payload: 'json', // backend-dependent - margin: 'formula', // not-on-this-input + margin: 'formula', // declared-elsewhere — and NO returnType here, so this + // fixture drives the absent tier end to end child_total: 'summary', // numeric-correct }; diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 4f3350d9c2..676a891858 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -690,8 +690,25 @@ export interface AnalyticsServiceConfig { * that field's storage scale via `percentScaleOf`, so a renderer scales by * declared metadata instead of guessing from the value. * - Date bucketing: a date vs datetime dimension drills by the right bound. + * - [#16236] Formula result type: `returnType` is what a `formula` field was + * DECLARED to compute, and the only channel it has to the measure + * result-column rule. See the key's own note below. + * + * ⚠️ [#16236] `returnType` is `FieldSchema.returnType` — the AUTHORING + * vocabulary `'number' | 'text' | 'boolean' | 'date'`, whose owner is + * `packages/spec/src/data/field.zod.ts`. It is declared `string` here for the + * reason its sibling `type` is: this shape is what a HOST answers at runtime, + * and a host can answer a word this contract does not accept. ⛔ The accepted + * set is NEVER restated at this seam — `measureResultType` reads it off + * {@link FORMULA_RETURN_TYPE_RESULT}, which is the one copy, and tiers an + * unrecognised word as "cannot answer, do not block". + * + * ⛔ It is NOT a wire word. `AnalyticsResult.fields[].type` speaks + * `DimensionType`, in which `text` is `'string'` and `date` is `'time'`; + * relaying this key into that position is the mistake the translation table + * exists to prevent. */ - sourceFieldMeta?: (object: string, field: string) => { type?: string; defaultCurrency?: string; max?: number } | undefined; + sourceFieldMeta?: (object: string, field: string) => { type?: string; defaultCurrency?: string; max?: number; returnType?: string } | undefined; /** * [#15684] The SQL dialect of the datasource backing `object` — `'sqlite'`, * `'postgres'`, `'mysql'`, or `undefined` when the host cannot answer. @@ -1818,7 +1835,13 @@ export class AnalyticsService implements IAnalyticsService { // 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); + // + // [#16236] The third input is the aggregated field's declared + // `returnType` — read off the SAME hook, in the same call, so a formula + // measure is typed from metadata the host already had rather than from + // a second probe. Absent (an unproven `dyn` expression) or unrecognised + // ⇒ the rule declines and the producer's `number` stands. + const resultType = measureResultType(m.aggregate, meta?.type, meta?.returnType); 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 index 5bb14398ce..9af67774a5 100644 --- a/packages/services/service-analytics/src/measure-result-type.ts +++ b/packages/services/service-analytics/src/measure-result-type.ts @@ -90,7 +90,8 @@ import { * | `MULTI_OPTION_TYPES` | multiselect, checkboxes, tags | no correction — an array in a JSON column (see below). | * | `FILE_REFERENCE_TYPES` | image, file, avatar, video, audio | no correction — the stored form is mid-migration (see below). | * | `STRUCTURED_JSON_TYPES` | json, composite, repeater, record, location, address, vector | no correction — an object in a JSON column (see below). | - * | `formula` | formula | no correction — the answer exists but is not on this rule's input (see below). | + * | `formula` | formula | {@link FORMULA_RETURN_TYPE_RESULT} — TRANSLATED from the field's own declared `returnType`. | + * | `formula`, `returnType` ABSENT | formula | no correction — do not answer, keep the word the producer minted (see below). | * * ### `autonumber` is a STRING, measured on three independent readings * @@ -152,18 +153,56 @@ import { * inline metadata object. Two shipped statements, two different stored forms; * correcting to either would describe half the deployments. * - * ### `formula`: answerable, but not from this rule's input + * ### `formula`: answered from the field's own DECLARED result type * * A formula field's result type IS declared — `FieldSchema.returnType` * (`number` / `text` / `boolean` / `date`), whose own JSDoc names "dataset - * measures" as its intended consumer. This rule cannot read it: its input is - * the declared `FieldType` alone, because that is all - * `AnalyticsServiceConfig.sourceFieldMeta` returns (`{ type?, - * defaultCurrency?, max? }`). `returnType` is also OPTIONAL — "absent when the - * type can't be proven (an ambiguous/`dyn` expression)" — so even with the - * plumbing the rule would answer for some formula fields and not others. - * Carrying it is a change to the host callback contract and its call site, not - * a row in this table; filed separately rather than guessed at here. + * measures" as its FIRST intended consumer. This rule could not read it: its + * input was the declared `FieldType` alone, because that was all + * `AnalyticsServiceConfig.sourceFieldMeta` returned (`{ type?, + * defaultCurrency?, max? }`), so `'formula'` arrived as a bare word with the + * answer sitting one key away in the metadata. [#16236] That hook now carries + * `returnType` and this rule takes it as a third input. + * + * ⚠️ **The mapping is a TRANSLATION, never a relay.** `returnType` speaks the + * AUTHORING vocabulary; `fields[].type` speaks `DimensionType`. Two of the four + * members are not wire words at all — `text` is `'string'` there and `date` is + * `'time'` — so passing the literal through would put a word into the response + * that no consumer branches on, which is the same mistake the section below + * ("Why `'string'` and `'time'`") already records for the `FieldType` axis. The + * table is {@link FORMULA_RETURN_TYPE_RESULT}; the pin that holds it a + * translation is in `__tests__/formula-return-type-measure.test.ts` and is one + * invariant over the whole enum rather than four expectations: NO member of + * `returnType` is answered by its own spelling. + * + * The two members whose spelling the wire vocabulary happens to share are not + * relayed either, and each for the reason its `FieldType` row already carries: + * + * - `returnType: 'number'` → **no correction**, identical to the + * `NUMERIC_VALUE_TYPES` row. The producer's `'number'` is already the correct + * word, so this rule has nothing to add. + * - `returnType: 'boolean'` → **no correction**, identical to the + * `BOOLEAN_VALUE_TYPES` row. Three readings disagree about what a `min`/`max` + * over a boolean returns, or whether it returns at all; minting `'boolean'` + * would ship one of them as a published declaration. + * + * ### `formula` with NO `returnType` — a ROW, not an implied code path + * + * `returnType` is OPTIONAL: "absent when the type can't be proven (an + * ambiguous/`dyn` expression)". So a formula field whose type could not be + * proven at authoring reaches this rule as `'formula'` with nothing behind it, + * and the verdict is: **do not answer — keep the word the producer minted.** + * An unproven formula's measure column stays `number`, and the ABSENCE is not + * itself read as an answer. + * + * That is the same "cannot answer, do not block" tier every other reader of + * `sourceFieldMeta` already uses (an unknown field type, a host with no data + * engine wired, a relationship-path measure), which is why it needed no new + * design — and it is written down HERE, as a row in this table, precisely so it + * does not become a behaviour that exists only inside a `?.` in the code path. + * A word outside the declared four is the same tier: a host answers at runtime + * and can answer anything, and an unrecognised one is left alone, never guessed + * at — the same treatment the `FieldType` axis gives an unrecognised `type`. * * ### `summary` is NUMERIC — the correction is not needed, not merely skipped * @@ -219,6 +258,43 @@ export const MEASURE_RESULT_TYPE_TEMPORAL = 'time'; */ export const MEASURE_RESULT_TYPE_STRING = 'string'; +/** + * The four members of `FieldSchema.returnType` (`packages/spec/src/data/ + * field.zod.ts`). Spelled here rather than imported because the spec exports + * the schema, not a named type for this key; `__tests__/formula-return-type- + * measure.test.ts` reads the enum off `FieldSchema` itself and reds if the two + * ever disagree, so the drift guard is mechanical rather than a promise. + */ +export type FormulaReturnType = 'number' | 'text' | 'boolean' | 'date'; + +/** + * [#16236] `FieldSchema.returnType` → the `DimensionType` word a `min`/`max` + * over that formula field's value should carry, or `undefined` for "no + * correction — keep the word the producer minted". + * + * ⚠️ **A TRANSLATION between two closed vocabularies, ⛔ never a relay.** The + * left column is the AUTHORING vocabulary a formula's result type is declared + * in; the right is the five-word wire vocabulary `AnalyticsResult.fields[].type` + * speaks. `text` and `date` do not exist on the right at all. + * + * | `returnType` | verdict | why | + * |:---|:---|:---| + * | `text` | `'string'` | the wire spelling for a string column, already carried by `lookup`/`string` dimension columns in the same response | + * | `date` | `'time'` | the wire spelling for a temporal column, already carried by a `date` dimension column in the same response | + * | `number` | no correction | identical to the `NUMERIC_VALUE_TYPES` row — the producer's `'number'` is already right | + * | `boolean` | no correction | identical to the `BOOLEAN_VALUE_TYPES` row — three readings disagree on what the aggregate even returns | + * + * ⭐ No member is answered by its own spelling, on ANY row — which is what makes + * a reintroduced pass-through detectable by one invariant instead of four + * hand-written expectations. See the module header for the long form. + */ +export const FORMULA_RETURN_TYPE_RESULT: Readonly> = { + text: MEASURE_RESULT_TYPE_STRING, + date: MEASURE_RESULT_TYPE_TEMPORAL, + number: undefined, + boolean: undefined, +}; + /** * Source-field types whose stored value is temporal (`FieldType`, `spec/data/ * field.zod.ts` → "Date & Time"). `min`/`max` over one of these returns that @@ -265,18 +341,28 @@ export const STRING_SOURCE_FIELD_TYPES: ReadonlySet = new Set([ * * `undefined` is also the answer for every field type whose `min`/`max` has no * single backend-independent value — booleans, the JSON-column classes, the - * mid-migration file types — and for `formula`, whose declared result type is - * not on this function's input. Those are VERDICTS, not gaps; the module - * header records the measurement behind each one. + * mid-migration file types — and for a `formula` field whose declared + * `returnType` is absent or unrecognised. Those are VERDICTS, not gaps; the + * module header records the measurement behind each one. * * @param aggregate - the measure's declared `aggregate`; absent on a `derived` * measure. * @param sourceFieldType - the DECLARED `FieldType` of the aggregated field, * from `AnalyticsServiceConfig.sourceFieldMeta`. + * @param formulaReturnType - [#16236] the aggregated field's declared + * `FieldSchema.returnType`, from the same hook. Read for `sourceFieldType === + * 'formula'` and for nothing else: it is that member's own declared key, not + * a general override of the table above. + * + * ⚠️ Typed `string`, not {@link FormulaReturnType}, for the reason its sibling + * `sourceFieldType` is: this is what a HOST answered at runtime, and a host + * can answer a word this contract does not accept. An unrecognised one lands + * in the "cannot answer" tier rather than being coerced into the table. */ export function measureResultType( aggregate: AggregationFunction | undefined, sourceFieldType: string | undefined, + formulaReturnType?: string, ): 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. @@ -284,5 +370,16 @@ export function measureResultType( if (sourceFieldType === undefined) return undefined; if (TEMPORAL_SOURCE_FIELD_TYPES.has(sourceFieldType)) return MEASURE_RESULT_TYPE_TEMPORAL; if (STRING_SOURCE_FIELD_TYPES.has(sourceFieldType)) return MEASURE_RESULT_TYPE_STRING; + // [#16236] The one member whose answer is declared on a SECOND key. ⛔ The + // literal is never relayed — {@link FORMULA_RETURN_TYPE_RESULT} translates it + // into the wire vocabulary, and answers `undefined` for the two members whose + // own `FieldType` rows already answer "no correction". + if (sourceFieldType === 'formula') { + if (formulaReturnType === undefined) return undefined; + // Widened on the RECORD, not the key: an out-of-contract word from a host + // reads back as `undefined`, which is this rule's "cannot answer" tier. + const table = FORMULA_RETURN_TYPE_RESULT as Readonly>; + return table[formulaReturnType]; + } return undefined; } diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 7287059bcb..247c320f27 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -1046,13 +1046,24 @@ export class AnalyticsServicePlugin implements Plugin { // of the two moves. debugSql: this.options.debugSql, // Source-field metadata behind the display chains on result columns: - // ADR-0053 currency (`currencyConfig.defaultCurrency`) and percent scale - // (`max`, which is what marks whole-percent storage — objectui#3136). + // ADR-0053 currency (`currencyConfig.defaultCurrency`), percent scale + // (`max`, which is what marks whole-percent storage — objectui#3136) and, + // since #16236, a formula field's declared `returnType`. + // + // [#16236] `returnType` is relayed exactly as the other three are — + // straight off the object's declared field, unvalidated and uncoerced. + // ⛔ Deliberately NOT narrowed to the accepted four here: this adapter's + // job is to carry what the engine declares, and a second copy of that + // vocabulary at this seam is one that can drift from + // `FieldSchema.returnType`. The one reader (`measureResultType`) holds + // the single copy and tiers anything else as "cannot answer". sourceFieldMeta: (object: string, field: string) => { const f = dataEngine()?.getObject?.(object)?.fields?.[field] as - | { type?: string; max?: number; currencyConfig?: { defaultCurrency?: string } } + | { type?: string; max?: number; returnType?: string; currencyConfig?: { defaultCurrency?: string } } | undefined; - return f ? { type: f.type, max: f.max, defaultCurrency: f.currencyConfig?.defaultCurrency } : undefined; + return f + ? { type: f.type, max: f.max, defaultCurrency: f.currencyConfig?.defaultCurrency, returnType: f.returnType } + : undefined; }, // #5033 — the datasource an object is bound to, used ONLY to name the // actual cause when a dataset's SQL references a table that is not on the