Skip to content

Commit 6117f7b

Browse files
baozhoutaoclaudeos-zhuang
authored
fix(spec,service-analytics): carry a percentage measure's SCALE, and report an empty filtered group as zero (objectui#3136) (#4442)
* fix(spec,service-analytics): carry a percentage measure's SCALE on the result column (objectui#3136) 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 <noreply@anthropic.com> * fix(spec,service-analytics): an empty filtered group is a measured zero, not missing data (objectui#3136) 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 <noreply@anthropic.com> * chore(spec): regenerate the api-surface baseline for the percent-scale exports (#4523) 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. * fix(showcase): translate the two percent-scale widget titles at birth `check-i18n-coverage` failed the TypeScript Type Check job: showcase's untranslated declared strings grew 452 → 454. The two new strings are the Revenue Pulse widgets this branch adds — `kpi_paid_rate` ("Paid Rate") and `table_rate_by_status` ("Paid Rate by Status") — declared with English titles and no zh-CN, a locale the example claims to support. This defect was always here; it was simply unreachable. The job used to die one step earlier on `check:api-surface`, so it never got as far as the i18n gate. Fixing the baseline uncovered it. Translated rather than baselined. The ratchet tolerates the debt that predates it and refuses growth, so the honest remedy for a string this branch introduces is to translate it — and the convention is already written down two lines above, where `showcase_chart_gallery`'s newest widget is translated at birth while its older siblings stay frozen. Revenue Pulse's other eight widget titles predate the ratchet the same way and are left alone; cleaning those up is not this branch's business. Terminology follows the existing zh-CN invoice bundle (发票 / 状态 / 付款). Count returns to exactly 452, so `scripts/i18n-coverage-baseline.json` needs no edit — no ratchet-down, no baseline churn. Verified: `check-i18n-coverage` OK (12 configs, none new); `check-i18n-bundles` all in sync; showcase `tsc --noEmit` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9TVFwXGsXqR3SD5e2qAU8 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: os-zhuang <jack@objectstack.ai>
1 parent 435415e commit 6117f7b

17 files changed

Lines changed: 394 additions & 22 deletions
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-analytics": minor
4+
---
5+
6+
fix(spec,service-analytics): a percentage measure carries its SCALE, so a ratio of 1 is 100% (objectui#3136)
7+
8+
A `%` format string says how to PRINT a number, not what scale that number is
9+
on — and the two readings collide at exactly `1`, which is both "100%" (a 0–1
10+
ratio at full compliance) and "1%" (a single percentage point). With nothing on
11+
the wire to tell them apart, renderers guessed from the value's magnitude and
12+
resolved the collision the wrong way: an SLA / pass-rate dashboard reporting
13+
`sla_rate = 1` displayed **"1.0%"** — "everything met the SLA" read as "1% met
14+
the SLA" — on both the KPI card and the dataset table.
15+
16+
The scale was never actually unknowable; it just never left the server. A
17+
measure declaring `derived: { op: 'ratio' }` is a 0–1 fraction *by definition*,
18+
and a measure aggregating a `percent` field has whatever scale that field
19+
stores. Both facts sit in metadata the enrichment pass already reads for the
20+
ADR-0053 currency chain — which walks back to the source field, checks
21+
`type === 'currency'`, and rides the resolved code onto the result column.
22+
Percentages got no such treatment. They do now, through the same seam.
23+
24+
**`percentScaleOf(field)` (`@objectstack/spec/data`)** is the one place the
25+
question is answered. A `percent` field stores a FRACTION unless it declares
26+
`max > 1` (e.g. `min: 0, max: 100`), which marks whole-percent storage — the
27+
same rule the percent edit widget already writes by, so a value round-trips.
28+
Non-`percent` fields get no opinion: a plain `number` an author formatted with
29+
a `%` keeps meaning exactly what their format string says.
30+
31+
**`AnalyticsResult.fields[].percentScale`** carries the answer: `'fraction'`
32+
(`1` ⇒ "100%") or `'whole'` (`1` ⇒ "1%"), absent when the column is not a
33+
percentage. `queryDataset` sets it from the measure's `derived.op === 'ratio'`
34+
first, then the source field's scale. `currency` — emitted since ADR-0053 but
35+
only ever written through a cast — is now declared on the same interface.
36+
37+
The config seam `measureCurrency` is renamed **`sourceFieldMeta`** and returns
38+
`max` alongside `type`/`defaultCurrency`. The old name had already outgrown
39+
itself: the date-bucketing path reads `type` through it to tell a `date`
40+
dimension from a `datetime` one, and the percent chain is its third consumer.
41+
42+
Renderers that receive `percentScale` must scale by it rather than inferring
43+
from the value; one that does not receive it (an older server) keeps whatever
44+
fallback it has, so this is additive on the wire.
45+
46+
**Same widget family, second fix: an empty filtered group is a measured zero.**
47+
A measure-scoped filter can exclude every row of a group the grid still lists,
48+
and the database reports that by omitting the group from the supplementary
49+
result — after the merge, indistinguishable from "not measured". For a COUNT or
50+
a SUM it *is* measured: the answer is 0. `emptyGroupValueFor(aggregate)`
51+
(`spec/data/aggregation-policy`) states which aggregates have an identity over
52+
the empty set, and `queryDataset` fills it in once all supplementary merges are
53+
done (a later measure's merge can append rows no earlier query saw). So
54+
"0 of 12 paid" now reports `0` instead of blank, and a ratio built on it
55+
computes to `0` instead of going null — the difference between a dashboard
56+
saying "0% met the SLA" and saying nothing at all. `avg`/`min`/`max` keep their
57+
null: there is nothing to average over an empty group, and flattening that to
58+
zero would invent a measurement.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
---
3+
4+
Regenerates `packages/spec/api-surface.json` so the committed baseline records the
5+
four `./data` exports the percent-scale chain adds — `PercentScale`,
6+
`PercentScaleFieldMeta`, `percentScaleOf` and `emptyGroupValueFor`.
7+
8+
Deliberately empty: this releases nothing. `api-surface.json` is a build-time
9+
snapshot the `check:api-surface` gate diffs against, not shipped code, and the
10+
exports it now records are already described by the changeset for the change that
11+
introduced them. Declaring a bump here would double-count that release.

examples/app-showcase/src/system/translations/index.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,12 @@ export const ShowcaseTranslationBundle = {
160160
combo_count_vs_progress: { title: 'Task Count vs Avg Progress' },
161161
},
162162
},
163+
showcase_revenue_pulse: {
164+
widgets: {
165+
kpi_paid_rate: { title: 'Paid Rate' },
166+
table_rate_by_status: { title: 'Paid Rate by Status' },
167+
},
168+
},
163169
},
164170
},
165171
'zh-CN': {
@@ -376,6 +382,16 @@ export const ShowcaseTranslationBundle = {
376382
combo_count_vs_progress: { title: '任务数与平均进度' },
377383
},
378384
},
385+
// Same rule as the gallery above: these two widgets are born with the
386+
// percent-scale fix, so they are translated at birth. Revenue Pulse's
387+
// other widget titles predate the ratchet and stay in the frozen
388+
// baseline. objectui#3136.
389+
showcase_revenue_pulse: {
390+
widgets: {
391+
kpi_paid_rate: { title: '已付比例' },
392+
table_rate_by_status: { title: '各状态已付比例' },
393+
},
394+
},
379395
},
380396
},
381397
};

examples/app-showcase/src/ui/dashboards/revenue-pulse.dashboard.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,14 @@ export const RevenuePulseDashboard: Dashboard = {
6464
{ 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 } },
6565
{ 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 } },
6666
{ 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 } },
67+
68+
// ── Percent scale (objectui#3136) ────────────────────────────────────
69+
// A ratio measure rendered on the two surfaces that disagreed: a KPI card
70+
// and a grouped table. Grouping by status pins the Paid row's rate at
71+
// exactly 1 — the boundary where "is this a 0–1 ratio or a percentage
72+
// point?" cannot be answered from the number, and where guessing printed
73+
// "1.0%". The column now carries its declared scale, so it reads 100.0%.
74+
{ 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 } },
75+
{ 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 } },
6776
],
6877
};

examples/app-showcase/src/ui/datasets/revenue-pulse.dataset.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ export const ShowcaseInvoiceDataset = defineDataset({
2424
measures: [
2525
{ name: 'invoice_count', label: 'Invoices', aggregate: 'count' },
2626
{ name: 'subtotal_sum', label: 'Subtotal', aggregate: 'sum', field: 'total', format: '0,0' },
27+
// A RATE — the percent-scale case (objectui#3136). `paid_rate` is a 0–1
28+
// ratio by construction, and grouping by `status` makes the Paid bucket
29+
// exactly 1: the value a magnitude-guessing renderer printed as "1.0%"
30+
// instead of "100.0%". The server annotates the column's scale from the
31+
// `ratio` operator, so display no longer has to infer it.
32+
{ name: 'paid_count', label: 'Paid Invoices', aggregate: 'count', filter: { status: 'paid' } },
33+
{
34+
name: 'paid_rate',
35+
label: 'Paid Rate',
36+
derived: { op: 'ratio', of: ['paid_count', 'invoice_count'] },
37+
format: '0.0%',
38+
},
2739
],
2840
});
2941

packages/services/service-analytics/src/__tests__/dataset-granularity-postprocess.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ function service(bucketByGranularity: Record<string, string>) {
5858
return [{ created_at: bucketByGranularity[g ?? 'none'], task_count: 10, account_count: 10 }];
5959
},
6060
// `created_at` is a tz-naive calendar date → ranges are exact under any tz.
61-
measureCurrency: (_o, f) => (f === 'created_at' ? { type: 'date' } : undefined),
61+
sourceFieldMeta: (_o, f) => (f === 'created_at' ? { type: 'date' } : undefined),
6262
});
6363
return { svc, seen };
6464
}

packages/services/service-analytics/src/__tests__/query-dataset.test.ts

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ describe('AnalyticsService.queryDataset', () => {
123123
});
124124

125125
// ── ADR-0053 currency chain (measure → field currencyConfig → tenant ctx) ──
126-
function pricedSvc(rows: Array<Record<string, unknown>>, measureCurrency?: (o: string, f: string) => { type?: string; defaultCurrency?: string } | undefined) {
126+
function pricedSvc(rows: Array<Record<string, unknown>>, sourceFieldMeta?: (o: string, f: string) => { type?: string; defaultCurrency?: string; max?: number } | undefined) {
127127
return new AnalyticsService({
128128
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
129129
executeRawSql: async () => rows,
130130
getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined),
131-
...(measureCurrency ? { measureCurrency } : {}),
131+
...(sourceFieldMeta ? { sourceFieldMeta } : {}),
132132
});
133133
}
134134
const moneyDataset = (measure: Record<string, unknown>) => DatasetSchema.parse({
@@ -161,6 +161,95 @@ describe('AnalyticsService.queryDataset', () => {
161161
expect(r.fields.find((f: any) => f.name === 'revenue')?.currency).toBeUndefined();
162162
});
163163

164+
// ── percent scale chain (objectui#3136) ───────────────────────────────────
165+
// A "%" format says how to PRINT a number, not what scale it is on, and the
166+
// two readings collide at exactly 1 ("100%" vs "1%"). The scale is answerable
167+
// from metadata, so it rides onto the result column next to `currency`.
168+
const rateDataset = (measures: Array<Record<string, unknown>>) => DatasetSchema.parse({
169+
name: 'sla', label: 'SLA', object: 'ticket', include: [],
170+
dimensions: [{ name: 'status', field: 'status', type: 'string' }],
171+
measures,
172+
});
173+
174+
it('marks a derived RATIO as fraction-scaled — the 1.0 = 100% case', async () => {
175+
// Two of two met: the ratio is exactly 1, the value that renders as "1.0%"
176+
// when a renderer guesses the scale from the number's magnitude.
177+
const svc = pricedSvc([{ status: 'met', met_count: 2, base_count: 2 }]);
178+
const r = await svc.queryDataset(
179+
rateDataset([
180+
{ name: 'base_count', aggregate: 'count', label: 'Applicable' },
181+
{ name: 'met_count', aggregate: 'count', field: 'met', label: 'Met' },
182+
{ name: 'sla_rate', label: 'SLA rate', derived: { op: 'ratio', of: ['met_count', 'base_count'] }, format: '0.0%' },
183+
]),
184+
{ dimensions: ['status'], measures: ['base_count', 'met_count', 'sla_rate'] },
185+
{ tenantId: 'o' } as ExecutionContext,
186+
) as any;
187+
expect(r.rows[0].sla_rate).toBe(1);
188+
expect(r.fields.find((f: any) => f.name === 'sla_rate')?.percentScale).toBe('fraction');
189+
// A count is not a percentage — annotating it would be a lie about the scale.
190+
expect(r.fields.find((f: any) => f.name === 'base_count')?.percentScale).toBeUndefined();
191+
});
192+
193+
it('a measure-scoped COUNT over a group with no matching rows is 0, not blank', async () => {
194+
// The supplementary query for `met_count` returns only the groups that had
195+
// a matching row; the database reports "none matched" by omitting the group
196+
// entirely. That omission is a measured ZERO for a count — reporting it as
197+
// missing blanked the cell and left the ratio null, hiding "0% of breached
198+
// tickets met the SLA" on the one dashboard that exists to show it.
199+
const svc = new AnalyticsService({
200+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
201+
// The base pass sees both groups; the filtered pass only "met".
202+
executeRawSql: async (_o, sql) => sql.includes('met_count')
203+
? [{ status: 'met', met_count: 2 }]
204+
: [{ status: 'met', base_count: 2 }, { status: 'breached', base_count: 3 }],
205+
getReadScope: () => undefined,
206+
});
207+
const r = await svc.queryDataset(
208+
rateDataset([
209+
{ name: 'base_count', aggregate: 'count', label: 'Applicable' },
210+
{ name: 'met_count', aggregate: 'count', field: 'met', label: 'Met', filter: { sla_met: true } },
211+
{ name: 'sla_rate', label: 'SLA rate', derived: { op: 'ratio', of: ['met_count', 'base_count'] }, format: '0.0%' },
212+
]),
213+
{ dimensions: ['status'], measures: ['base_count', 'met_count', 'sla_rate'] },
214+
{ tenantId: 'o' } as ExecutionContext,
215+
) as any;
216+
const breached = r.rows.find((x: any) => x.status === 'breached');
217+
expect(breached.met_count).toBe(0);
218+
expect(breached.sla_rate).toBe(0);
219+
// The group that DID match is untouched — and is the 1.0 = 100% case.
220+
expect(r.rows.find((x: any) => x.status === 'met').sla_rate).toBe(1);
221+
});
222+
223+
it('inherits the SOURCE FIELD scale: a `max: 100` percent field is whole-scaled', async () => {
224+
const svc = pricedSvc([{ status: 'open', allocation: 80 }], (_o, f) => f === 'allocation_percent' ? { type: 'percent', max: 100 } : undefined);
225+
const r = await svc.queryDataset(
226+
rateDataset([{ name: 'allocation', aggregate: 'avg', field: 'allocation_percent', label: 'Allocation', format: '0.0%' }]),
227+
{ dimensions: ['status'], measures: ['allocation'] },
228+
{ tenantId: 'o' } as ExecutionContext,
229+
) as any;
230+
expect(r.fields.find((f: any) => f.name === 'allocation')?.percentScale).toBe('whole');
231+
});
232+
233+
it('inherits the SOURCE FIELD scale: a bare percent field is fraction-scaled', async () => {
234+
const svc = pricedSvc([{ status: 'open', win: 0.75 }], (_o, f) => f === 'win_probability' ? { type: 'percent' } : undefined);
235+
const r = await svc.queryDataset(
236+
rateDataset([{ name: 'win', aggregate: 'avg', field: 'win_probability', label: 'Win', format: '0.0%' }]),
237+
{ dimensions: ['status'], measures: ['win'] },
238+
{ tenantId: 'o' } as ExecutionContext,
239+
) as any;
240+
expect(r.fields.find((f: any) => f.name === 'win')?.percentScale).toBe('fraction');
241+
});
242+
243+
it('leaves a plain-number measure unannotated — its format string stays the only word', async () => {
244+
const svc = pricedSvc([{ status: 'open', tax: 7 }], (_o, f) => f === 'tax_rate' ? { type: 'number', max: 100 } : undefined);
245+
const r = await svc.queryDataset(
246+
rateDataset([{ name: 'tax', aggregate: 'avg', field: 'tax_rate', label: 'Tax', format: '0.0%' }]),
247+
{ dimensions: ['status'], measures: ['tax'] },
248+
{ tenantId: 'o' } as ExecutionContext,
249+
) as any;
250+
expect(r.fields.find((f: any) => f.name === 'tax')?.percentScale).toBeUndefined();
251+
});
252+
164253
it('enriches dimension columns with their dataset display label', async () => {
165254
const labeled = DatasetSchema.parse({
166255
name: 'sales2', label: 'Sales', object: 'opportunity', include: ['account'],
@@ -259,7 +348,7 @@ describe('AnalyticsService.queryDataset', () => {
259348
// closed_at is a datetime instant → its month bucket boundary is that tz's
260349
// MIDNIGHT INSTANT. June/July 2026 in New York are EDT (−04), so local
261350
// midnight is 04:00 UTC.
262-
measureCurrency: (_o, f) => (f === 'closed_at' ? { type: 'datetime' } : undefined),
351+
sourceFieldMeta: (_o, f) => (f === 'closed_at' ? { type: 'datetime' } : undefined),
263352
getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined),
264353
});
265354
const result = await svc.queryDataset(
@@ -282,7 +371,7 @@ describe('AnalyticsService.queryDataset', () => {
282371
const svc = new AnalyticsService({
283372
queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
284373
executeAggregate: async () => [{ close_date: '2026-06', revenue: 100 }],
285-
measureCurrency: (_o, f) => (f === 'close_date' ? { type: 'date' } : undefined),
374+
sourceFieldMeta: (_o, f) => (f === 'close_date' ? { type: 'date' } : undefined),
286375
getReadScope: (_o, ctx?: ExecutionContext) => (ctx?.tenantId ? { organization_id: ctx.tenantId } : undefined),
287376
});
288377
const result = await svc.queryDataset(

0 commit comments

Comments
 (0)