Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .changeset/dataset-percent-scale.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
"@object-ui/core": patch
"@object-ui/plugin-dashboard": patch
"@object-ui/plugin-report": patch
"@object-ui/app-shell": patch
---

Honor the server's declared percent scale, so a ratio of exactly 1 renders as 100.0% (#3136)

A dataset measure declared `format: '0.0%'` rendered every ratio below 1
correctly and got the single most consequential one wrong: a rate of exactly
`1` printed as **`1.0%`**. On an SLA / pass-rate dashboard that turns
"everything met the SLA" into "1% met the SLA", on both surfaces the issue
names — the KPI card and the dataset-bound table (they share `formatMeasure`).

The cause was never a bad multiplier; it was a missing fact. `formatMeasure`
scaled by magnitude — `percentDisplayValue` multiplies by 100 only strictly
inside `(-1, 1)` — because the column arrived with a `%` format string and
nothing saying what scale its numbers were on. That guess is undecidable at
exactly 1, which is both a full-compliance ratio ("100%") and one percentage
point ("1%"), and it resolved to the reading almost nobody means.

The server now answers the question instead (framework: `percentScaleOf` +
`AnalyticsResult.fields[].percentScale`, the sibling of the ADR-0053 currency
chain): a `derived: { op: 'ratio' }` measure is a `fraction` by definition, and
a measure over a `percent` field inherits that field's scale. `formatMeasure`
takes the declared scale as a fourth argument and, when present, scales by it —
`fraction` ×100, `whole` verbatim — instead of inspecting the value. Every
dataset-bound call site passes the column's `percentScale`: the dashboard
metric/table/pivot cells, the report renderer's cells, totals and KPI, and the
dataset preview.

`percentDisplayValue` is untouched and still the fallback for a column that
arrives without the annotation (an older server, or a non-dataset percent cell
in a list view), so nothing that renders correctly today changes.
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ function MeasureFormatField({ measure, onPatch, disabled }: { measure: Measure;
const { kind, decimals } = parseMeasureFormat(measure.format, measure.currency);
const currency = measure.currency || 'USD';
const apply = (k: string, d: number, c: string) => onPatch(buildMeasureFormat(k, d, c));
const sample = formatMeasure(kind === 'percent' ? 0.1234 : 1234.5, measure.format, measure.currency);
// The percent sample is a hand-picked 0–1 FRACTION, so it says so rather than
// leaving the formatter to infer a scale from the sample's magnitude.
const sample = formatMeasure(kind === 'percent' ? 0.1234 : 1234.5, measure.format, measure.currency, kind === 'percent' ? 'fraction' : undefined);
return (
<div className="space-y-1.5">
<div className="grid grid-cols-2 gap-1.5">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export function DatasetPreview({ draft }: MetadataPreviewProps) {
{columns.map((c) => (
<td key={c} className="px-2 py-1 tabular-nums whitespace-nowrap">
{measureNames.includes(c)
? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency)
? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency, measureField(c)?.percentScale)
: formatDimensionValue(row[c])}
</td>
))}
Expand Down
26 changes: 23 additions & 3 deletions packages/core/src/utils/__tests__/dataset-format.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,33 @@ describe('formatMeasure', () => {
// metric card shows "0.6%" for an avg of 0.608 instead of "60.8%" (the bug).
expect(formatMeasure(0.75, '0%')).toBe('75%');
expect(formatMeasure(0.608_333_333, '0.0%')).toBe('60.8%');
// Boundary: exactly 0 and exactly 1 (100% stored as 1.0) — 1.0 passes
// through, mirroring the list renderer's strict `< 1` heuristic so the two
// surfaces stay in lockstep (a known shared limitation, not a new one).
// Boundary: exactly 0 and exactly 1 (100% stored as 1.0) — with no declared
// scale, 1.0 passes through, mirroring the list renderer's strict `< 1`
// heuristic so the two unannotated surfaces stay in lockstep. This is the
// ambiguity `percentScale` exists to remove (see below).
expect(formatMeasure(0, '0%')).toBe('0%');
expect(formatMeasure(1, '0%')).toBe('1%');
});

it('honors a DECLARED percent scale over the value-magnitude heuristic (#3136)', () => {
// The bug: a ratio of exactly 1 (full compliance) is indistinguishable from
// 1 percentage point by magnitude alone, and the heuristic resolves it the
// wrong way — "everything met the SLA" reads as "1% met the SLA". The
// server now says which scale the column is on, and that wins.
expect(formatMeasure(1, '0.0%', undefined, 'fraction')).toBe('100.0%');
expect(formatMeasure(0.6667, '0.0%', undefined, 'fraction')).toBe('66.7%');
expect(formatMeasure(0, '0.0%', undefined, 'fraction')).toBe('0.0%');
// Whole-percent storage is the other half of the same ambiguity: a declared
// `whole` column renders 1 as "1%" and is NOT scaled up, even though the
// heuristic would have multiplied a sub-1 value by 100.
expect(formatMeasure(1, '0.0%', undefined, 'whole')).toBe('1.0%');
expect(formatMeasure(0.5, '0.0%', undefined, 'whole')).toBe('0.5%');
expect(formatMeasure(80, '0.0%', undefined, 'whole')).toBe('80.0%');
// Non-percent formats ignore the annotation entirely — it describes the
// percentage scale, not a general multiplier.
expect(formatMeasure(1, '0.0', undefined, 'fraction')).toBe('1.0');
});

it('falls back to plain formatting for an unknown currency code', () => {
expect(formatMeasure(1234, '0,0', 'NOTACODE')).toBe('1,234');
});
Expand Down
25 changes: 23 additions & 2 deletions packages/core/src/utils/dataset-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,21 @@ export interface DatasetResultField {
label?: string;
format?: string;
currency?: string;
percentScale?: PercentScale;
}

/**
* How a percentage column's stored number relates to its displayed percentage —
* the server's answer to a question a `%` format string cannot express:
* `fraction` is a 0–1 ratio (`1` ⇒ "100%"), `whole` is already percentage
* points (`1` ⇒ "1%"). Resolved from metadata server-side (a `ratio` measure is
* a fraction by definition; a measure over a `percent` field inherits that
* field's scale) and carried on the result column, so display never has to
* infer it from the value's magnitude — the inference that printed a ratio of
* exactly 1 as "1.0%" (#3136).
*/
export type PercentScale = 'fraction' | 'whole';

/**
* Scale a stored `percent`-field value to its DISPLAY magnitude.
*
Expand All @@ -55,7 +68,7 @@ export function percentDisplayValue(value: number): number {
* controls grouping / decimals / percent; it can't be baked into the row value
* server-side (the same number feeds charts), so it is applied here.
*/
export function formatMeasure(v: unknown, format?: string, currency?: string): string {
export function formatMeasure(v: unknown, format?: string, currency?: string, percentScale?: PercentScale): string {
if (v == null) return '—';
if (typeof v !== 'number') return String(v);

Expand Down Expand Up @@ -86,7 +99,15 @@ export function formatMeasure(v: unknown, format?: string, currency?: string): s
// (0.75 ⇒ 75%). Scale to display magnitude the SAME way the list-view cell
// renderer does — otherwise an avg of 0.608 renders as "0.6%" instead of
// "60.8%", disagreeing with the per-row "75%" the list already shows.
const display = isPercent ? percentDisplayValue(v) : v;
//
// A DECLARED `percentScale` from the server wins outright: the value-magnitude
// heuristic below cannot tell a 0–1 ratio of exactly 1 from 1 percentage point
// (both are the number 1) and resolves it as "1%", so an SLA rate of full
// compliance rendered as "1.0%" instead of "100.0%" (#3136). The heuristic
// stays as the fallback for columns that arrive without the annotation.
const display = isPercent
? (percentScale ? (percentScale === 'fraction' ? v * 100 : v) : percentDisplayValue(v))
: v;
const body = display.toLocaleString(undefined, { minimumFractionDigits: decimals ?? 0, maximumFractionDigits: decimals ?? 0 });
return `${legacyDollar}${body}${isPercent ? '%' : ''}`;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-dashboard/src/DatasetWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource:
const value = state.rows[0]?.[values[0]] ?? 0;
return (
<div className="flex h-full w-full flex-col items-start justify-center gap-1 p-2">
<span className="text-2xl font-semibold tabular-nums">{formatMeasure(value, f?.format, f?.currency)}</span>
<span className="text-2xl font-semibold tabular-nums">{formatMeasure(value, f?.format, f?.currency, f?.percentScale)}</span>
<span className="text-xs text-muted-foreground">{headerLabel(values[0])}</span>
</div>
);
Expand Down Expand Up @@ -447,7 +447,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource:
const cellCols = pivot.colHeaders.flatMap((col) =>
values.map((m) => ({ col, measure: m, header: values.length === 1 ? col.label : `${col.label} · ${headerLabel(m)}` })),
);
const fmtMeasure = (v: unknown, m: string) => formatMeasure(v, measureField(m)?.format, measureField(m)?.currency);
const fmtMeasure = (v: unknown, m: string) => formatMeasure(v, measureField(m)?.format, measureField(m)?.currency, measureField(m)?.percentScale);
// Server-supplied marginal totals (ADR-0021): match each grouping by its
// dimension array, then its rows to the pivot headers via the same bucket
// ids. Absent (older server) → maps stay empty and no totals UI renders.
Expand Down Expand Up @@ -554,7 +554,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource:
>
{columns.map((c) => (
<td key={c} className="px-2 py-1 whitespace-nowrap tabular-nums">
{values.includes(c) ? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency) : formatDimensionValue(row[c])}
{values.includes(c) ? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency, measureField(c)?.percentScale) : formatDimensionValue(row[c])}
</td>
))}
</tr>
Expand Down
26 changes: 26 additions & 0 deletions packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,32 @@ describe('DatasetWidget', () => {
expect(screen.queryByText('0.6%')).not.toBeInTheDocument();
});

it('renders a declared-fraction ratio of exactly 1 as "100.0%" on BOTH the metric card and the table (#3136)', async () => {
// A ratio of exactly 1 is the one value the magnitude heuristic cannot
// resolve — 1 is both "100%" (a full-compliance ratio) and "1%" (a single
// percentage point) — and it resolved it as "1.0%": "everything met the
// SLA" reported to management as "1% met the SLA". The server now declares
// the column's scale, and both dataset-bound surfaces honour it.
const fields = [
{ name: 'status', type: 'string', label: 'Status' },
{ name: 'sla_rate', type: 'number', label: 'SLA rate', format: '0.0%', percentScale: 'fraction' as const },
];
const metricSrc = { queryDataset: vi.fn(async () => ({ rows: [{ sla_rate: 1 }], fields })) };
render(<DatasetWidget widget={{ type: 'metric', dataset: 'sla_ds', values: ['sla_rate'] }} dataSource={metricSrc} />);
expect(await screen.findByText('100.0%')).toBeInTheDocument();
expect(screen.queryByText('1.0%')).not.toBeInTheDocument();

// Same column in the grouped table: the "met" bucket is 100%, and a
// measured ZERO stays "0.0%" — a real 0% is not missing data.
const tableSrc = { queryDataset: vi.fn(async () => ({
rows: [{ status: 'met', sla_rate: 1 }, { status: 'breached', sla_rate: 0 }],
fields,
})) };
render(<DatasetWidget widget={{ type: 'table', dataset: 'sla_ds', dimensions: ['status'], values: ['sla_rate'] }} dataSource={tableSrc} />);
await waitFor(() => expect(screen.getAllByText('100.0%').length).toBe(2));
expect(screen.getByText('0.0%')).toBeInTheDocument();
});

it('runs the dataset query for a dimensioned (chart) widget — rows→dimensions, values→measures', async () => {
const src = makeSource(async () => ({ rows: [{ stage: 'won', revenue: 100 }, { stage: 'lost', revenue: 20 }] }));
render(<DatasetWidget widget={{ type: 'bar', dataset: 'sales', dimensions: ['stage'], values: ['revenue'] }} dataSource={src} />);
Expand Down
14 changes: 7 additions & 7 deletions packages/plugin-report/src/DatasetReportRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ function DatasetReportTable({
{columns.map((c) => (
<td key={c} className="px-2 py-1 tabular-nums whitespace-nowrap">
{values.includes(c)
? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency)
? formatMeasure(row[c], measureField(c)?.format, measureField(c)?.currency, measureField(c)?.percentScale)
: formatDimensionValue(row[c])}
</td>
))}
Expand All @@ -480,7 +480,7 @@ function DatasetReportTable({
)}
{values.map((measure) => (
<td key={measure} className="px-2 py-1 tabular-nums whitespace-nowrap">
{formatMeasure(grandTotal[measure], measureField(measure)?.format, measureField(measure)?.currency)}
{formatMeasure(grandTotal[measure], measureField(measure)?.format, measureField(measure)?.currency, measureField(measure)?.percentScale)}
</td>
))}
</tr>
Expand Down Expand Up @@ -699,7 +699,7 @@ function DatasetReportChart({
{title ? <h3 className="mb-2 text-sm font-semibold">{title}</h3> : null}
<div className="flex flex-col gap-0.5 py-2">
<span className="text-2xl font-semibold tabular-nums">
{formatMeasure(state.rows[0]?.[yAxis], mf?.format, mf?.currency)}
{formatMeasure(state.rows[0]?.[yAxis], mf?.format, mf?.currency, mf?.percentScale)}
</span>
<span className="text-xs text-muted-foreground">{headerLabel(yAxis)}</span>
</div>
Expand Down Expand Up @@ -907,7 +907,7 @@ function DatasetMatrixTable({
data-testid={clickable ? 'dataset-drill-cell' : undefined}
onClick={clickable ? () => drillCell(rh.key, cc.col.key, entry!.index) : undefined}
>
{formatMeasure(value, measureField(cc.measure)?.format, measureField(cc.measure)?.currency)}
{formatMeasure(value, measureField(cc.measure)?.format, measureField(cc.measure)?.currency, measureField(cc.measure)?.percentScale)}
</td>
);
})}
Expand All @@ -918,7 +918,7 @@ function DatasetMatrixTable({
className="px-2 py-1 text-right tabular-nums whitespace-nowrap font-medium"
data-testid="matrix-row-total"
>
{formatMeasure(rowTotalById.get(rh.id)?.[measure], measureField(measure)?.format, measureField(measure)?.currency)}
{formatMeasure(rowTotalById.get(rh.id)?.[measure], measureField(measure)?.format, measureField(measure)?.currency, measureField(measure)?.percentScale)}
</td>
))}
</tr>
Expand All @@ -932,7 +932,7 @@ function DatasetMatrixTable({
)}
{cellCols.map((cc) => (
<td key={`${cc.col.id}-${cc.measure}`} className="px-2 py-1 text-right tabular-nums whitespace-nowrap">
{formatMeasure(colTotalById.get(cc.col.id)?.[cc.measure], measureField(cc.measure)?.format, measureField(cc.measure)?.currency)}
{formatMeasure(colTotalById.get(cc.col.id)?.[cc.measure], measureField(cc.measure)?.format, measureField(cc.measure)?.currency, measureField(cc.measure)?.percentScale)}
</td>
))}
{showTotalCol &&
Expand All @@ -942,7 +942,7 @@ function DatasetMatrixTable({
className="px-2 py-1 text-right tabular-nums whitespace-nowrap"
data-testid="matrix-grand-total"
>
{formatMeasure(grandTotal?.[measure], measureField(measure)?.format, measureField(measure)?.currency)}
{formatMeasure(grandTotal?.[measure], measureField(measure)?.format, measureField(measure)?.currency, measureField(measure)?.percentScale)}
</td>
))}
</tr>
Expand Down
Loading