From dcaef4c11f4eb431d7ba35e6a8563abb0725efef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Sat, 1 Aug 2026 02:53:26 -0700 Subject: [PATCH 1/3] fix(spec,service-analytics): carry a percentage measure's SCALE on the result column (objectui#3136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `%` format string says how to print a number, not what scale it is on, and the two readings collide at exactly 1 — both "100%" (a 0-1 ratio) and "1%" (one percentage point). Renderers guessed from the value's magnitude and resolved it the wrong way, so an SLA rate of full compliance displayed as "1.0%". The scale was answerable from metadata all along; it just never left the server. `derived: { op: 'ratio' }` is a 0-1 fraction by definition, and a measure over a `percent` field has that field's scale. Both are now resolved in the measure-column enrichment pass, next to the ADR-0053 currency chain that already walks back to the source field for exactly this kind of display fact. - `percentScaleOf(field)` (spec/data) — the one rule: a `percent` field stores a fraction unless it declares `max > 1`, matching what the edit widget writes. Non-percent fields get no opinion. - `AnalyticsResult.fields[].percentScale` — 'fraction' | 'whole', absent when the column is not a percentage. `currency` (emitted since ADR-0053 through a cast) is declared on the same interface. - `measureCurrency` → `sourceFieldMeta`, now returning `max`. The old name had outgrown itself: date bucketing already read `type` through it, and the percent chain is its third consumer. - Showcase: a `paid_rate` ratio measure + KPI/table widgets on Revenue Pulse. Grouping by status pins the Paid bucket at exactly 1 — the repro value. Co-Authored-By: Claude Opus 5 --- .changeset/dataset-percent-scale-chain.md | 44 ++++++++++++ .../ui/dashboards/revenue-pulse.dashboard.ts | 9 +++ .../src/ui/datasets/revenue-pulse.dataset.ts | 12 ++++ .../dataset-granularity-postprocess.test.ts | 2 +- .../src/__tests__/query-dataset.test.ts | 67 +++++++++++++++++-- .../src/analytics-service.ts | 39 ++++++++--- .../services/service-analytics/src/plugin.ts | 10 +-- .../spec/src/contracts/analytics-service.ts | 18 +++++ packages/spec/src/data/index.ts | 4 ++ packages/spec/src/data/percent-scale.test.ts | 27 ++++++++ packages/spec/src/data/percent-scale.ts | 53 +++++++++++++++ 11 files changed, 265 insertions(+), 20 deletions(-) create mode 100644 .changeset/dataset-percent-scale-chain.md create mode 100644 packages/spec/src/data/percent-scale.test.ts create mode 100644 packages/spec/src/data/percent-scale.ts diff --git a/.changeset/dataset-percent-scale-chain.md b/.changeset/dataset-percent-scale-chain.md new file mode 100644 index 0000000000..41ff0f2586 --- /dev/null +++ b/.changeset/dataset-percent-scale-chain.md @@ -0,0 +1,44 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-analytics": minor +--- + +fix(spec,service-analytics): a percentage measure carries its SCALE, so a ratio of 1 is 100% (objectui#3136) + +A `%` format string says how to PRINT a number, not what scale that number is +on — and the two readings collide at exactly `1`, which is both "100%" (a 0–1 +ratio at full compliance) and "1%" (a single percentage point). With nothing on +the wire to tell them apart, renderers guessed from the value's magnitude and +resolved the collision the wrong way: an SLA / pass-rate dashboard reporting +`sla_rate = 1` displayed **"1.0%"** — "everything met the SLA" read as "1% met +the SLA" — on both the KPI card and the dataset table. + +The scale was never actually unknowable; it just never left the server. A +measure declaring `derived: { op: 'ratio' }` is a 0–1 fraction *by definition*, +and a measure aggregating a `percent` field has whatever scale that field +stores. Both facts sit in metadata the enrichment pass already reads for the +ADR-0053 currency chain — which walks back to the source field, checks +`type === 'currency'`, and rides the resolved code onto the result column. +Percentages got no such treatment. They do now, through the same seam. + +**`percentScaleOf(field)` (`@objectstack/spec/data`)** is the one place the +question is answered. A `percent` field stores a FRACTION unless it declares +`max > 1` (e.g. `min: 0, max: 100`), which marks whole-percent storage — the +same rule the percent edit widget already writes by, so a value round-trips. +Non-`percent` fields get no opinion: a plain `number` an author formatted with +a `%` keeps meaning exactly what their format string says. + +**`AnalyticsResult.fields[].percentScale`** carries the answer: `'fraction'` +(`1` ⇒ "100%") or `'whole'` (`1` ⇒ "1%"), absent when the column is not a +percentage. `queryDataset` sets it from the measure's `derived.op === 'ratio'` +first, then the source field's scale. `currency` — emitted since ADR-0053 but +only ever written through a cast — is now declared on the same interface. + +The config seam `measureCurrency` is renamed **`sourceFieldMeta`** and returns +`max` alongside `type`/`defaultCurrency`. The old name had already outgrown +itself: the date-bucketing path reads `type` through it to tell a `date` +dimension from a `datetime` one, and the percent chain is its third consumer. + +Renderers that receive `percentScale` must scale by it rather than inferring +from the value; one that does not receive it (an older server) keeps whatever +fallback it has, so this is additive on the wire. diff --git a/examples/app-showcase/src/ui/dashboards/revenue-pulse.dashboard.ts b/examples/app-showcase/src/ui/dashboards/revenue-pulse.dashboard.ts index 87a40d8014..c55b499365 100644 --- a/examples/app-showcase/src/ui/dashboards/revenue-pulse.dashboard.ts +++ b/examples/app-showcase/src/ui/dashboards/revenue-pulse.dashboard.ts @@ -64,5 +64,14 @@ export const RevenuePulseDashboard: Dashboard = { { id: 'col_accounts_by_month', type: 'column', title: 'Accounts Signed by Month', dataset: accountDs, dimensions: ['signed_on'], values: ['account_count'], chartConfig: cfg('column', 'signed_on', 'account_count'), filterBindings: { dateRange: 'signed_on', region: 'sales_region' }, layout: { x: 6, y: 2, w: 6, h: 4 } }, { id: 'donut_invoices_by_status', type: 'donut', title: 'Invoices by Status', dataset: invoiceDs, dimensions: ['status'], values: ['invoice_count'], chartConfig: cfg('donut', 'status', 'invoice_count'), layout: { x: 0, y: 6, w: 6, h: 4 } }, { id: 'bar_accounts_by_industry', type: 'bar', title: 'Accounts by Industry', dataset: accountDs, dimensions: ['industry'], values: ['account_count'], chartConfig: cfg('bar', 'industry', 'account_count'), filterBindings: { dateRange: 'signed_on', region: 'sales_region' }, layout: { x: 6, y: 6, w: 6, h: 4 } }, + + // ── Percent scale (objectui#3136) ──────────────────────────────────── + // A ratio measure rendered on the two surfaces that disagreed: a KPI card + // and a grouped table. Grouping by status pins the Paid row's rate at + // exactly 1 — the boundary where "is this a 0–1 ratio or a percentage + // point?" cannot be answered from the number, and where guessing printed + // "1.0%". The column now carries its declared scale, so it reads 100.0%. + { id: 'kpi_paid_rate', type: 'metric', title: 'Paid Rate', dataset: invoiceDs, values: ['paid_rate'], colorVariant: 'orange', layout: { x: 0, y: 10, w: 3, h: 2 } }, + { id: 'table_rate_by_status', type: 'table', title: 'Paid Rate by Status', dataset: invoiceDs, dimensions: ['status'], values: ['invoice_count', 'paid_count', 'paid_rate'], layout: { x: 3, y: 10, w: 9, h: 4 } }, ], }; diff --git a/examples/app-showcase/src/ui/datasets/revenue-pulse.dataset.ts b/examples/app-showcase/src/ui/datasets/revenue-pulse.dataset.ts index 7a4ca552c9..b584003143 100644 --- a/examples/app-showcase/src/ui/datasets/revenue-pulse.dataset.ts +++ b/examples/app-showcase/src/ui/datasets/revenue-pulse.dataset.ts @@ -24,6 +24,18 @@ export const ShowcaseInvoiceDataset = defineDataset({ measures: [ { name: 'invoice_count', label: 'Invoices', aggregate: 'count' }, { name: 'subtotal_sum', label: 'Subtotal', aggregate: 'sum', field: 'total', format: '0,0' }, + // A RATE — the percent-scale case (objectui#3136). `paid_rate` is a 0–1 + // ratio by construction, and grouping by `status` makes the Paid bucket + // exactly 1: the value a magnitude-guessing renderer printed as "1.0%" + // instead of "100.0%". The server annotates the column's scale from the + // `ratio` operator, so display no longer has to infer it. + { name: 'paid_count', label: 'Paid Invoices', aggregate: 'count', filter: { status: 'paid' } }, + { + name: 'paid_rate', + label: 'Paid Rate', + derived: { op: 'ratio', of: ['paid_count', 'invoice_count'] }, + format: '0.0%', + }, ], }); diff --git a/packages/services/service-analytics/src/__tests__/dataset-granularity-postprocess.test.ts b/packages/services/service-analytics/src/__tests__/dataset-granularity-postprocess.test.ts index 80b3fe9629..5daf762e92 100644 --- a/packages/services/service-analytics/src/__tests__/dataset-granularity-postprocess.test.ts +++ b/packages/services/service-analytics/src/__tests__/dataset-granularity-postprocess.test.ts @@ -58,7 +58,7 @@ function service(bucketByGranularity: Record) { return [{ created_at: bucketByGranularity[g ?? 'none'], task_count: 10, account_count: 10 }]; }, // `created_at` is a tz-naive calendar date → ranges are exact under any tz. - measureCurrency: (_o, f) => (f === 'created_at' ? { type: 'date' } : undefined), + sourceFieldMeta: (_o, f) => (f === 'created_at' ? { type: 'date' } : undefined), }); return { svc, seen }; } diff --git a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts index 46ca3894ff..d25d091aad 100644 --- a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts +++ b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts @@ -123,12 +123,12 @@ describe('AnalyticsService.queryDataset', () => { }); // ── ADR-0053 currency chain (measure → field currencyConfig → tenant ctx) ── - function pricedSvc(rows: Array>, measureCurrency?: (o: string, f: string) => { type?: string; defaultCurrency?: string } | undefined) { + function pricedSvc(rows: Array>, sourceFieldMeta?: (o: string, f: string) => { type?: string; defaultCurrency?: string; max?: number } | undefined) { return new AnalyticsService({ queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), executeRawSql: async () => rows, getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined), - ...(measureCurrency ? { measureCurrency } : {}), + ...(sourceFieldMeta ? { sourceFieldMeta } : {}), }); } const moneyDataset = (measure: Record) => DatasetSchema.parse({ @@ -161,6 +161,65 @@ describe('AnalyticsService.queryDataset', () => { expect(r.fields.find((f: any) => f.name === 'revenue')?.currency).toBeUndefined(); }); + // ── percent scale chain (objectui#3136) ─────────────────────────────────── + // A "%" format says how to PRINT a number, not what scale it is on, and the + // two readings collide at exactly 1 ("100%" vs "1%"). The scale is answerable + // from metadata, so it rides onto the result column next to `currency`. + const rateDataset = (measures: Array>) => DatasetSchema.parse({ + name: 'sla', label: 'SLA', object: 'ticket', include: [], + dimensions: [{ name: 'status', field: 'status', type: 'string' }], + measures, + }); + + it('marks a derived RATIO as fraction-scaled — the 1.0 = 100% case', async () => { + // Two of two met: the ratio is exactly 1, the value that renders as "1.0%" + // when a renderer guesses the scale from the number's magnitude. + const svc = pricedSvc([{ status: 'met', met_count: 2, base_count: 2 }]); + const r = await svc.queryDataset( + rateDataset([ + { name: 'base_count', aggregate: 'count', label: 'Applicable' }, + { name: 'met_count', aggregate: 'count', field: 'met', label: 'Met' }, + { name: 'sla_rate', label: 'SLA rate', derived: { op: 'ratio', of: ['met_count', 'base_count'] }, format: '0.0%' }, + ]), + { dimensions: ['status'], measures: ['base_count', 'met_count', 'sla_rate'] }, + { tenantId: 'o' } as ExecutionContext, + ) as any; + expect(r.rows[0].sla_rate).toBe(1); + expect(r.fields.find((f: any) => f.name === 'sla_rate')?.percentScale).toBe('fraction'); + // A count is not a percentage — annotating it would be a lie about the scale. + expect(r.fields.find((f: any) => f.name === 'base_count')?.percentScale).toBeUndefined(); + }); + + it('inherits the SOURCE FIELD scale: a `max: 100` percent field is whole-scaled', async () => { + const svc = pricedSvc([{ status: 'open', allocation: 80 }], (_o, f) => f === 'allocation_percent' ? { type: 'percent', max: 100 } : undefined); + const r = await svc.queryDataset( + rateDataset([{ name: 'allocation', aggregate: 'avg', field: 'allocation_percent', label: 'Allocation', format: '0.0%' }]), + { dimensions: ['status'], measures: ['allocation'] }, + { tenantId: 'o' } as ExecutionContext, + ) as any; + expect(r.fields.find((f: any) => f.name === 'allocation')?.percentScale).toBe('whole'); + }); + + it('inherits the SOURCE FIELD scale: a bare percent field is fraction-scaled', async () => { + const svc = pricedSvc([{ status: 'open', win: 0.75 }], (_o, f) => f === 'win_probability' ? { type: 'percent' } : undefined); + const r = await svc.queryDataset( + rateDataset([{ name: 'win', aggregate: 'avg', field: 'win_probability', label: 'Win', format: '0.0%' }]), + { dimensions: ['status'], measures: ['win'] }, + { tenantId: 'o' } as ExecutionContext, + ) as any; + expect(r.fields.find((f: any) => f.name === 'win')?.percentScale).toBe('fraction'); + }); + + it('leaves a plain-number measure unannotated — its format string stays the only word', async () => { + const svc = pricedSvc([{ status: 'open', tax: 7 }], (_o, f) => f === 'tax_rate' ? { type: 'number', max: 100 } : undefined); + const r = await svc.queryDataset( + rateDataset([{ name: 'tax', aggregate: 'avg', field: 'tax_rate', label: 'Tax', format: '0.0%' }]), + { dimensions: ['status'], measures: ['tax'] }, + { tenantId: 'o' } as ExecutionContext, + ) as any; + expect(r.fields.find((f: any) => f.name === 'tax')?.percentScale).toBeUndefined(); + }); + it('enriches dimension columns with their dataset display label', async () => { const labeled = DatasetSchema.parse({ name: 'sales2', label: 'Sales', object: 'opportunity', include: ['account'], @@ -259,7 +318,7 @@ describe('AnalyticsService.queryDataset', () => { // closed_at is a datetime instant → its month bucket boundary is that tz's // MIDNIGHT INSTANT. June/July 2026 in New York are EDT (−04), so local // midnight is 04:00 UTC. - measureCurrency: (_o, f) => (f === 'closed_at' ? { type: 'datetime' } : undefined), + sourceFieldMeta: (_o, f) => (f === 'closed_at' ? { type: 'datetime' } : undefined), getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined), }); const result = await svc.queryDataset( @@ -282,7 +341,7 @@ describe('AnalyticsService.queryDataset', () => { const svc = new AnalyticsService({ queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), executeAggregate: async () => [{ close_date: '2026-06', revenue: 100 }], - measureCurrency: (_o, f) => (f === 'close_date' ? { type: 'date' } : undefined), + sourceFieldMeta: (_o, f) => (f === 'close_date' ? { type: 'date' } : undefined), getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined), }); const result = await svc.queryDataset( diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 45c1fb26db..c88beb60e3 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -7,7 +7,7 @@ import type { CubeMeta, DatasetSelection, } from '@objectstack/spec/contracts'; -import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import { percentScaleOf, type Cube, type FilterCondition } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { Dataset } from '@objectstack/spec/ui'; import type { Logger } from '@objectstack/spec/contracts'; @@ -216,13 +216,20 @@ export interface AnalyticsServiceConfig { */ relationshipResolver?: RelationshipResolver; /** - * ADR-0053 currency chain — resolve a measure's SOURCE FIELD currency - * metadata so a monetary measure that omits an explicit `currency` falls back - * to the field's declared currency, then the tenant default (`ctx.currency`). - * Returns the source field's `type` and (fixed-mode) `defaultCurrency`; - * `undefined` for an unknown field. Non-`currency` fields never get a code. + * Resolve the metadata of a dimension's or measure's SOURCE FIELD on the + * dataset's base object — the one seam through which display semantics that + * live on the field reach the result columns. `undefined` for an unknown + * field. Feeds three chains: + * + * - ADR-0053 currency: a monetary measure that omits an explicit `currency` + * falls back to the field's declared currency, then the tenant default + * (`ctx.currency`). Non-`currency` fields never get a code. + * - Percent scale (objectui#3136): a measure over a `percent` field inherits + * 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. */ - measureCurrency?: (object: string, field: string) => { type?: string; defaultCurrency?: string } | undefined; + sourceFieldMeta?: (object: string, field: string) => { type?: string; defaultCurrency?: string; max?: number } | undefined; /** Pre-defined datasets to compile + register at construction (ADR-0021). */ datasets?: Dataset[]; /** @@ -286,7 +293,7 @@ export class AnalyticsService implements IAnalyticsService { private readonly datasetRegistry = new Map(); /** Optional object-graph resolver used when compiling datasets. */ private readonly relationshipResolver?: RelationshipResolver; - private readonly measureCurrency?: AnalyticsServiceConfig['measureCurrency']; + private readonly sourceFieldMeta?: AnalyticsServiceConfig['sourceFieldMeta']; /** Optional dimension display-label resolver (select options / lookup names). */ private readonly labelResolver?: DimensionLabelDeps; /** ADR-0037 P3: pending-seed row resolver for draft data preview. */ @@ -309,7 +316,7 @@ export class AnalyticsService implements IAnalyticsService { this.readScopeProvider = config.getReadScope; this.relationshipResolver = config.relationshipResolver; - this.measureCurrency = config.measureCurrency; + this.sourceFieldMeta = config.sourceFieldMeta; this.labelResolver = config.labelResolver; this.draftRowsResolver = config.draftRowsResolver; this.isRegisteredObject = config.isRegisteredObject; @@ -657,7 +664,7 @@ export class AnalyticsService implements IAnalyticsService { if (!d.field || d.type !== 'date') continue; const granularity = resolveDimensionGranularity(selection, d.name, d.dateGranularity); if (!granularity) continue; - const ftype = this.measureCurrency?.(dataset.object, d.field as string)?.type; + const ftype = this.sourceFieldMeta?.(dataset.object, d.field as string)?.type; if (ftype === 'datetime') rangeDims.push({ d, granularity, instant: true }); else if (ftype === 'date') rangeDims.push({ d, granularity, instant: false }); else if (rangeTz === 'UTC') rangeDims.push({ d, granularity, instant: false }); @@ -746,14 +753,24 @@ export class AnalyticsService implements IAnalyticsService { // number) never receive a currency code. const fc = f as { currency?: string }; const mc = m as { currency?: string }; + const meta = m.field ? this.sourceFieldMeta?.(dataset.object, m.field) : undefined; if (fc.currency == null) { - const meta = m.field ? this.measureCurrency?.(dataset.object, m.field) : undefined; const monetary = !!mc.currency || meta?.type === 'currency'; if (monetary) { const resolved = mc.currency ?? meta?.defaultCurrency ?? context?.currency; if (resolved) fc.currency = resolved; } } + // Percent scale chain (objectui#3136) — the currency chain's sibling. + // A `%` format says how to PRINT a number, not what scale it is on, and + // the two readings collide at exactly 1 ("100%" vs "1%"). Both answers + // are in metadata, so answer here instead of leaving the renderer to + // guess from the value's magnitude: a `ratio` is a 0–1 fraction by + // definition, and any aggregate of a `percent` field (avg/min/max — + // sum is already rejected as incoherent) keeps that field's own scale. + if (f.percentScale == null) { + f.percentScale = m.derived?.op === 'ratio' ? 'fraction' : percentScaleOf(meta); + } } } diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 904cd1e4b5..1d62ad0ded 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -518,12 +518,14 @@ export class AnalyticsServicePlugin implements Plugin { coerceTemporalFilterColumn, relationshipResolver, labelResolver, - // ADR-0053 — source-field currency metadata for the measure currency chain. - measureCurrency: (object: string, field: string) => { + // 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). + sourceFieldMeta: (object: string, field: string) => { const f = dataEngine()?.getObject?.(object)?.fields?.[field] as - | { type?: string; currencyConfig?: { defaultCurrency?: string } } + | { type?: string; max?: number; currencyConfig?: { defaultCurrency?: string } } | undefined; - return f ? { type: f.type, defaultCurrency: f.currencyConfig?.defaultCurrency } : undefined; + return f ? { type: f.type, max: f.max, defaultCurrency: f.currencyConfig?.defaultCurrency } : undefined; }, // ADR-0062 D6 — a federated object carries an `external` block (ADR-0015). // Reported so NativeSQLStrategy declines it (its hand-compiled FROM would diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index 2db04ffb91..7d426533c7 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -2,6 +2,7 @@ import type { Cube } from '../data/analytics.zod.js'; import type { FilterCondition } from '../data/filter.zod.js'; +import type { PercentScale } from '../data/percent-scale.js'; import type { ExecutionContext } from '../kernel/execution-context.zod.js'; import type { Dataset } from '../ui/dataset.zod.js'; @@ -72,6 +73,23 @@ export interface AnalyticsResult { label?: string; /** Display format hint (e.g. measure `format` like "$0,0", "0.0%"). */ format?: string; + /** + * ADR-0053 currency chain — the resolved ISO 4217 code for a MONETARY + * measure (explicit measure `currency` → source-field default → tenant + * default). Absent on non-monetary columns, which must never render a + * symbol. + */ + currency?: string; + /** + * The column's percent SCALE, when it is a percentage: `fraction` for a + * 0–1 ratio (`1` ⇒ "100%"), `whole` for percentage points (`1` ⇒ "1%"). + * Resolved from metadata — a `derived: { op: 'ratio' }` measure is a + * fraction by definition, and a measure over a `percent` field inherits + * that field's scale (see `percentScaleOf` in `spec/data`). Absent when + * the column is not a percentage; renderers that receive it must scale + * by it instead of guessing from the value (objectui#3136). + */ + percentScale?: PercentScale; }>; /** Generated SQL (if available) */ sql?: string; diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index 08c39bc149..a1b776087a 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -71,6 +71,10 @@ export * from './analytics.zod'; // and build-time coherence validation. export * from './aggregation-policy'; +// Percent storage scale (0–1 fraction vs whole percentage points) — resolved +// from field metadata so renderers never guess it from the value's magnitude. +export * from './percent-scale'; + // Record display-name contract (ADR-0079) — title eligibility, primary-field // resolution/derivation, record display-name rendering, primary provisioning, // and title-completeness classification. Shared by authoring, display diff --git a/packages/spec/src/data/percent-scale.test.ts b/packages/spec/src/data/percent-scale.test.ts new file mode 100644 index 0000000000..41c61c6170 --- /dev/null +++ b/packages/spec/src/data/percent-scale.test.ts @@ -0,0 +1,27 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { percentScaleOf } from './percent-scale'; + +describe('percentScaleOf', () => { + it('reads a bare percent field as fraction-stored (0–1)', () => { + // No `max` → the edit widget divides typed input by 100, so 1 means 100%. + expect(percentScaleOf({ type: 'percent' })).toBe('fraction'); + expect(percentScaleOf({ type: 'percent', max: 1 })).toBe('fraction'); + }); + + it('reads `max` above 1 as whole-percent storage (0–100)', () => { + // `min: 0, max: 100` is the declaration the edit widget keys off to store + // the typed number unscaled — so 1 means 1%, not 100%. + expect(percentScaleOf({ type: 'percent', max: 100 })).toBe('whole'); + }); + + it('has no opinion about a non-percent field', () => { + // A plain number carries no percent semantics even when an author formats + // it with a "%" — annotating it would override their format string. + expect(percentScaleOf({ type: 'number', max: 100 })).toBeUndefined(); + expect(percentScaleOf({ type: 'currency' })).toBeUndefined(); + expect(percentScaleOf(undefined)).toBeUndefined(); + expect(percentScaleOf({})).toBeUndefined(); + }); +}); diff --git a/packages/spec/src/data/percent-scale.ts b/packages/spec/src/data/percent-scale.ts new file mode 100644 index 0000000000..6ae0c9e794 --- /dev/null +++ b/packages/spec/src/data/percent-scale.ts @@ -0,0 +1,53 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Percent SCALE — the single source of truth for "what magnitude is this + * percentage stored at". A `%` display format says *how* to print a number, not + * *what scale it is on*: `1` is "100%" when the value is a 0–1 ratio and "1%" + * when it is already in percentage points. Nothing in the number itself tells + * the two apart, so renderers were guessing from the value's magnitude — and a + * ratio of exactly `1` (full compliance, the one value that matters most on an + * SLA / pass-rate dashboard) guessed wrong and printed "1%" (objectui#3136). + * + * The scale is answerable from METADATA, so it is answered here and carried on + * the wire (see `AnalyticsResult.fields[].percentScale`) rather than re-guessed + * per surface. This mirrors the ADR-0053 currency chain: a monetary measure + * resolves its display currency from the source field's metadata instead of a + * "$" baked into the format string. + */ + +/** + * How a percentage's stored number relates to its displayed percentage: + * + * - `fraction` — a 0–1 ratio. Multiply by 100 to display: `0.75` ⇒ "75%", + * `1` ⇒ "100%". Every `derived: { op: 'ratio' }` measure is this by + * definition, as is a `percent` field written by the standard edit widget. + * - `whole` — already in percentage points. Display verbatim: `75` ⇒ "75%", + * `1` ⇒ "1%". A `percent` field declaring a `max` above 1 (e.g. `max: 100`) + * is this — that bound is what the edit widget keys off when it stores the + * typed number unscaled. + */ +export type PercentScale = 'fraction' | 'whole'; + +/** The subset of field metadata the scale is resolvable from. */ +export interface PercentScaleFieldMeta { + type?: string; + /** Declared upper bound; `> 1` marks whole-percent storage (see below). */ + max?: number; +} + +/** + * Resolve the storage scale of a `percent` field, or `undefined` when the field + * is not a percentage (a plain `number` carries no percent semantics — a + * measure over one keeps whatever the author's format string implies). + * + * A percent field stores a FRACTION unless it declares `max > 1`. That is not a + * new convention invented here: it is the rule the percent EDIT widget already + * writes by (it divides typed input by 100 unless `max > 1`), so reading it back + * this way is what makes a value round-trip. Declaring `min: 0, max: 100` is + * therefore a load-bearing statement about storage, not just validation. + */ +export function percentScaleOf(field: PercentScaleFieldMeta | undefined): PercentScale | undefined { + if (field?.type !== 'percent') return undefined; + return typeof field.max === 'number' && field.max > 1 ? 'whole' : 'fraction'; +} From 8da4887b4d44bdd9a03f67f1b540550ec024ede2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Sat, 1 Aug 2026 03:35:37 -0700 Subject: [PATCH 2/3] fix(spec,service-analytics): an empty filtered group is a measured zero, not missing data (objectui#3136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A measure-scoped filter can exclude every row of a group the grid still lists, and the database reports that by omitting the group from the supplementary result — after the merge, indistinguishable from "never measured". For a COUNT or a SUM it IS measured: the answer is 0. So "0 of 12 paid" rendered as a blank cell and every ratio built on it went null — a compliance dashboard silently dropping the row it exists to show. On the showcase's Paid-Rate table the Sent bucket read "—/—" where the truth is "0 / 0.0%". - `emptyGroupValueFor(aggregate)` (spec/data/aggregation-policy) states which aggregates have an identity over the empty set. avg/min/max keep their null: there is nothing to average, and a zero there would invent a measurement. - `queryDataset` fills it in after ALL supplementary merges, not inside the loop — a later measure's merge can append rows for dimension keys no earlier query saw, and those rows need the same fill. Co-Authored-By: Claude Opus 5 --- .changeset/dataset-percent-scale-chain.md | 14 +++++++++ .../src/__tests__/query-dataset.test.ts | 30 +++++++++++++++++++ .../service-analytics/src/dataset-executor.ts | 19 +++++++++++- .../spec/src/data/aggregation-policy.test.ts | 15 +++++++++- packages/spec/src/data/aggregation-policy.ts | 22 ++++++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/.changeset/dataset-percent-scale-chain.md b/.changeset/dataset-percent-scale-chain.md index 41ff0f2586..3482491b95 100644 --- a/.changeset/dataset-percent-scale-chain.md +++ b/.changeset/dataset-percent-scale-chain.md @@ -42,3 +42,17 @@ dimension from a `datetime` one, and the percent chain is its third consumer. Renderers that receive `percentScale` must scale by it rather than inferring from the value; one that does not receive it (an older server) keeps whatever fallback it has, so this is additive on the wire. + +**Same widget family, second fix: an empty filtered group is a measured zero.** +A measure-scoped filter can exclude every row of a group the grid still lists, +and the database reports that by omitting the group from the supplementary +result — after the merge, indistinguishable from "not measured". For a COUNT or +a SUM it *is* measured: the answer is 0. `emptyGroupValueFor(aggregate)` +(`spec/data/aggregation-policy`) states which aggregates have an identity over +the empty set, and `queryDataset` fills it in once all supplementary merges are +done (a later measure's merge can append rows no earlier query saw). So +"0 of 12 paid" now reports `0` instead of blank, and a ratio built on it +computes to `0` instead of going null — the difference between a dashboard +saying "0% met the SLA" and saying nothing at all. `avg`/`min`/`max` keep their +null: there is nothing to average over an empty group, and flattening that to +zero would invent a measurement. diff --git a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts index d25d091aad..653a1e4e20 100644 --- a/packages/services/service-analytics/src/__tests__/query-dataset.test.ts +++ b/packages/services/service-analytics/src/__tests__/query-dataset.test.ts @@ -190,6 +190,36 @@ describe('AnalyticsService.queryDataset', () => { expect(r.fields.find((f: any) => f.name === 'base_count')?.percentScale).toBeUndefined(); }); + it('a measure-scoped COUNT over a group with no matching rows is 0, not blank', async () => { + // The supplementary query for `met_count` returns only the groups that had + // a matching row; the database reports "none matched" by omitting the group + // entirely. That omission is a measured ZERO for a count — reporting it as + // missing blanked the cell and left the ratio null, hiding "0% of breached + // tickets met the SLA" on the one dashboard that exists to show it. + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + // The base pass sees both groups; the filtered pass only "met". + executeRawSql: async (_o, sql) => sql.includes('met_count') + ? [{ status: 'met', met_count: 2 }] + : [{ status: 'met', base_count: 2 }, { status: 'breached', base_count: 3 }], + getReadScope: () => undefined, + }); + const r = await svc.queryDataset( + rateDataset([ + { name: 'base_count', aggregate: 'count', label: 'Applicable' }, + { name: 'met_count', aggregate: 'count', field: 'met', label: 'Met', filter: { sla_met: true } }, + { name: 'sla_rate', label: 'SLA rate', derived: { op: 'ratio', of: ['met_count', 'base_count'] }, format: '0.0%' }, + ]), + { dimensions: ['status'], measures: ['base_count', 'met_count', 'sla_rate'] }, + { tenantId: 'o' } as ExecutionContext, + ) as any; + const breached = r.rows.find((x: any) => x.status === 'breached'); + expect(breached.met_count).toBe(0); + expect(breached.sla_rate).toBe(0); + // The group that DID match is untouched — and is the 1.0 = 100% case. + expect(r.rows.find((x: any) => x.status === 'met').sla_rate).toBe(1); + }); + it('inherits the SOURCE FIELD scale: a `max: 100` percent field is whole-scaled', async () => { const svc = pricedSvc([{ status: 'open', allocation: 80 }], (_o, f) => f === 'allocation_percent' ? { type: 'percent', max: 100 } : undefined); const r = await svc.queryDataset( diff --git a/packages/services/service-analytics/src/dataset-executor.ts b/packages/services/service-analytics/src/dataset-executor.ts index 8d2e06fae7..4bbd296e68 100644 --- a/packages/services/service-analytics/src/dataset-executor.ts +++ b/packages/services/service-analytics/src/dataset-executor.ts @@ -7,7 +7,7 @@ import type { DatasetSelection, DatasetCompareTo, } from '@objectstack/spec/contracts'; -import type { FilterCondition } from '@objectstack/spec/data'; +import { emptyGroupValueFor, type FilterCondition } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { filterTokenContextFrom, resolveFilterTokens } from '@objectstack/core'; import type { CompiledDataset, DerivedMeasureSpec } from './dataset-compiler.js'; @@ -554,6 +554,23 @@ export class DatasetExecutor { result.fields.push({ name: m, type: 'number' }); } + // A measure-scoped filter can exclude EVERY row of a group the grid still + // lists, and the database reports that by omitting the group from the + // sub-result — indistinguishable, after the merge, from "not measured". + // For a count or a sum it IS measured: the answer is 0. Fill it in, so a + // "0 of 12 paid" group reads as 0 rather than blank and any ratio built on + // it computes instead of going null (objectui#3136). avg/min/max keep their + // null — there is nothing to average over an empty group. + // + // Runs after ALL supplementary merges, not inside the loop: a later + // measure's merge can append rows for dimension keys no earlier query saw, + // and those rows need the same fill. + for (const m of filtered) { + const empty = emptyGroupValueFor(compiled.cube.measures?.[m]?.type); + if (empty === undefined) continue; + for (const row of result.rows) if (row[m] == null) row[m] = empty; + } + // compareTo — run a shifted query over the same base measures and attach. if (selection.compareTo) { const compareRows = await this.runCompare(compiled, selection, [...baseMeasures], dimensions, baseFilter, context); diff --git a/packages/spec/src/data/aggregation-policy.test.ts b/packages/spec/src/data/aggregation-policy.test.ts index 054eacdef1..b9770d727c 100644 --- a/packages/spec/src/data/aggregation-policy.test.ts +++ b/packages/spec/src/data/aggregation-policy.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { defaultAggregateFor, isIncoherentAggregate, MEASURE_FIELD_TYPES } from './aggregation-policy'; +import { defaultAggregateFor, emptyGroupValueFor, isIncoherentAggregate, MEASURE_FIELD_TYPES } from './aggregation-policy'; describe('defaultAggregateFor', () => { it('SUMs additive amounts', () => { @@ -20,6 +20,19 @@ describe('defaultAggregateFor', () => { }); describe('isIncoherentAggregate', () => { + it('counts and sums over an empty group are 0; avg/min/max stay undefined', () => { + // A group every row of which a measure filter excluded is a MEASURED zero + // for a count or a sum — not missing data. Averaging nothing is genuinely + // unanswerable and must not be flattened to a zero that reads as real. + expect(emptyGroupValueFor('count')).toBe(0); + expect(emptyGroupValueFor('count_distinct')).toBe(0); + expect(emptyGroupValueFor('sum')).toBe(0); + expect(emptyGroupValueFor('avg')).toBeUndefined(); + expect(emptyGroupValueFor('min')).toBeUndefined(); + expect(emptyGroupValueFor('max')).toBeUndefined(); + expect(emptyGroupValueFor(undefined)).toBeUndefined(); + }); + it('flags SUM and count_distinct of a percentage', () => { expect(isIncoherentAggregate('sum', 'percent')).toBe(true); expect(isIncoherentAggregate('count_distinct', 'percent')).toBe(true); diff --git a/packages/spec/src/data/aggregation-policy.ts b/packages/spec/src/data/aggregation-policy.ts index 27ebe29f85..f6d120c994 100644 --- a/packages/spec/src/data/aggregation-policy.ts +++ b/packages/spec/src/data/aggregation-policy.ts @@ -32,3 +32,25 @@ export function isIncoherentAggregate(aggregate: string, fieldType: string | und if (fieldType === 'percent' && (aggregate === 'sum' || aggregate === 'count_distinct')) return true; return false; } + +/** + * What this aggregate evaluates to over an EMPTY set of rows — the identity + * element, or `undefined` when the aggregate genuinely has no answer. + * + * Counting no rows is `0` and summing them is `0`: those are measured facts, + * not missing data. Averaging, minimising or maximising no rows is undefined — + * there is nothing to average — and must stay null rather than be flattened to + * a zero that reads as a real measurement. + * + * This distinction only surfaces for a measure the runtime evaluates per + * GROUP: a measure-scoped filter can exclude every row of a group the grid + * still lists, and a database reports that group by omitting the row entirely. + * Treating that omission as "no data" reported "0 of 12 paid" as blank and + * left any ratio built on it null (objectui#3136) — hiding the very number a + * compliance dashboard exists to show. + */ +export function emptyGroupValueFor(aggregate: string | undefined): 0 | undefined { + return aggregate === 'count' || aggregate === 'count_distinct' || aggregate === 'sum' + ? 0 + : undefined; +} From bb79c309144b8f54c4e08a275ede1ae5aa4c6639 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sun, 2 Aug 2026 00:17:55 +0800 Subject: [PATCH 3/3] chore(spec): regenerate the api-surface baseline for the percent-scale exports (#4523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regenerates `packages/spec/api-surface.json` so the committed baseline records the four `./data` exports the percent-scale chain adds — `PercentScale`, `PercentScaleFieldMeta`, `percentScaleOf` and `emptyGroupValueFor` — plus an empty-frontmatter changeset declaring that this releases nothing. The api-surface gate is a snapshot check: it fails whenever the built public surface and the committed list disagree, in either direction, which is what makes an unintended public-API change impossible to land silently. #4442 added the exports without refreshing the baseline, so `check:api-surface` reported 4 unrecorded additions and failed the TypeScript Type Check job. Nothing was removed or narrowed — 0 breaking. The changeset is empty on purpose: `api-surface.json` is a build-time snapshot, not shipped code, and the exports it records already ship under `dataset-percent-scale-chain.md` on this branch. A bump here would double-count that release. --- .changeset/spec-api-surface-baseline-percent-scale.md | 11 +++++++++++ packages/spec/api-surface.json | 4 ++++ 2 files changed, 15 insertions(+) create mode 100644 .changeset/spec-api-surface-baseline-percent-scale.md diff --git a/.changeset/spec-api-surface-baseline-percent-scale.md b/.changeset/spec-api-surface-baseline-percent-scale.md new file mode 100644 index 0000000000..1b07342ca6 --- /dev/null +++ b/.changeset/spec-api-surface-baseline-percent-scale.md @@ -0,0 +1,11 @@ +--- +--- + +Regenerates `packages/spec/api-surface.json` so the committed baseline records the +four `./data` exports the percent-scale chain adds — `PercentScale`, +`PercentScaleFieldMeta`, `percentScaleOf` and `emptyGroupValueFor`. + +Deliberately empty: this releases nothing. `api-surface.json` is a build-time +snapshot the `check:api-surface` gate diffs against, not shipped code, and the +exports it now records are already described by the changeset for the change that +introduced them. Declaring a bump here would double-count that release. diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index c24fc55e53..47971d0d1a 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -473,6 +473,8 @@ "PaginationConformanceRow (interface)", "PerOperationRequiredPermissions (type)", "PerOperationRequiredPermissionsSchema (const)", + "PercentScale (type)", + "PercentScaleFieldMeta (interface)", "PoolConfig (type)", "PoolConfigSchema (const)", "ProvisionPrimaryOptions (interface)", @@ -612,6 +614,7 @@ "deriveRecordFlowSurface (function)", "deriveRecordSurface (function)", "effectiveOperationsArray (function)", + "emptyGroupValueFor (function)", "fieldForm (const)", "foldQueryAliasSlots (function)", "formatUnknownAuthoringKey (function)", @@ -641,6 +644,7 @@ "parseAutonumberFormat (function)", "parseDateMacroParam (function)", "parseFilterAST (function)", + "percentScaleOf (function)", "provisionPrimary (function)", "referencedFields (function)", "renderAutonumber (function)",