fix(spec,service-analytics): carry a percentage measure's SCALE, and report an empty filtered group as zero (objectui#3136) - #4442
Open
baozhoutao wants to merge 2 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…e 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>
…ro, 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the server half of objectstack-ai/objectui#3136. The Console half is objectstack-ai/objectui#3140 — merge this first; the renderer reads a field this PR adds.
The bug, precisely
A
%format string says how to print a number, not what scale it is on. The two readings collide at exactly1, which is both "100%" (a 0–1 ratio at full compliance) and "1%" (one percentage point). Nothing on the wire told them apart, so renderers guessed from the value's magnitude — and resolved the collision the way almost nobody means. An SLA dashboard reportingsla_rate = 1displayed "1.0%": "everything met the SLA" read as "1% met the SLA".The scale was never unknowable. It just never left the server:
derived: { op: 'ratio' }is a 0–1 fraction by definition;percentfield has whatever scale that field stores.Both facts sit in metadata the measure-column 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. Same seam, now they do.What lands
percentScaleOf(field)(spec/data/percent-scale.ts) — the one place the question is answered. Apercentfield stores a FRACTION unless it declaresmax > 1(e.g.min: 0, max: 100), which marks whole-percent storage. That is not a new convention: it is the rule the percent edit widget already writes by, so reading it back this way is what makes a value round-trip. Non-percentfields get no opinion — a plainnumberan author formatted with a%keeps meaning exactly what their format string says.AnalyticsResult.fields[].percentScale—'fraction'|'whole', absent when the column is not a percentage.queryDatasetsets it fromderived.op === 'ratio'first, then the source field.currency— emitted since ADR-0053 but only ever written through a cast — is now declared on the same interface.measureCurrency→sourceFieldMeta, returningmaxalongsidetype/defaultCurrency. The old name had already outgrown itself: the date-bucketing path readstypethrough it to tell adatedimension from adatetimeone, and percent scale is its third consumer.Second fix, same widget family: 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 — after the merge, indistinguishable from "never measured". For a COUNT or a SUM it is measured: the answer is 0.
emptyGroupValueFor(aggregate)states which aggregates have an identity over the empty set, and the executor fills it in after all supplementary merges (a later measure's merge can append rows no earlier query saw). So "0 of 12 paid" reports0instead of blank and a ratio on it computes to0instead of null.avg/min/maxkeep their null — there is nothing to average over an empty group, and a zero there would invent a measurement.Showcase: a
paid_rateratio measure plus KPI + table widgets on Revenue Pulse. Grouping bystatuspins the Paid bucket at exactly1— the repro value, now demonstrated instead of described.Verification
Real backend (
objectstack dev --fresh, showcase seed),POST /api/v1/analytics/dataset/query:{"status": "Paid", "invoice_count": 5, "paid_count": 5, "paid_rate": 1} {"status": "Sent", "invoice_count": 5, "paid_count": 0, "paid_rate": 0} {"name": "paid_rate", "format": "0.0%", "percentScale": "fraction"}Browser (Console dev server against that backend, Revenue Pulse dashboard), A/B in one running UI:
percentScalestripped (pre-fix server)The Sent row moved from
—/—to0/0.0%.Tests:
service-analytics470 passed (5 new — ratio→fraction,max:100source field→whole, bare percent field→fraction, plain number unannotated, empty filtered group→0);spec/data1234 passed.pnpm build71/71.spectypecheck clean,service-analyticssources clean. Fullpnpm test: 72/73 tasks green —service-datasourcefailed 5 assertions on 5s timeouts under load and passes 10/10 in isolation, unrelated to this change.🤖 Generated with Claude Code