fix(plugin-grid): the column-summary percent arm takes both halves from the declared source - #9293
Merged
Merged
Conversation
…om the declared source
`formatSummaryLabel`'s `colType === 'percent'` arm held a hand-inlined copy of
`percentDisplayValue`'s body and then appended a literal ASCII percent sign:
const decimals = column?.precision ?? 0;
const pct = (value > -1 && value < 1) ? value * 100 : value;
formatted = `${pct.toFixed(decimals)}%`;
Line 2 is the expression `percentDisplayValue` in `@object-ui/core` IS,
character for character, so the SCALING agreed by duplication rather than by
reference. The CONVENTION was not taken at all. That helper's doc comment makes
this the judgement rather than a style nit: "If a third surface ever needs
percent display, it takes BOTH halves from here - the scaling AND the
convention - or this promise breaks again in the same place." A footer showing
a percent aggregate directly beneath the percent cells it aggregates is such a
surface, and it took neither half by reference.
The arm now hands the raw stored value to `formatPercent` - the same call the
list-view percent cell makes - with the tag from `useDisplayLocale()`, so both
halves come from one home. `decimals` still reads `column.precision`.
Rendered output MOVES, and the move is the repair. Measured in one run against
the declared source:
en 0.25 25% -> 25% unchanged
en 1 1% -> 1% unchanged
en -5 -5% -> -5% unchanged
en 12.3 12% -> 12% unchanged
en 1234.5 1235% -> 1,235% grouping
de-DE 0.25 25% -> 25 % no-break space before the sign
de-DE 1234.5 1235% -> 1.235 % grouping and affix
tr-TR 0.25 25% -> %25 the sign moves to the FRONT
tr-TR 1234.5 1235% -> %1.235 same
The new pin asserts each row against the declared source computed in the same
run, plus absolute bytes for `tr-TR` and for the four already-agreeing `en`
rows, which are overshoot detectors rather than controls: below four digits a
bare '%' and the locale affix coincide in `en`, so those rows read the same
either way and cannot fail for this card's reason. On the base tree the file
was 12 failed / 15 passed; after the change 27 passed.
Out of scope, deliberately: the `PERCENT_TYPES` arm's literal '%' (those are
computed fill ratios, not stored percent fields), and whether `decimals` should
read `scale` rather than `precision` (the card's own table shows precision
agreeing on both sides).
Part of objectui#9269
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Sep 12, 2026
…nvention the footer renders under Commandment #2 - the README describes the aggregate footer's percent formatting and now says where both halves of the rule come from, with the three rendered figures that move: `Sum: 1,235%` in en, `Sum: 1.235 %` in de-DE and `Sum: %1.235` in tr-TR, where the sign goes in front of the number. No rendered figure in the existing prose moved; this is an addition. The table row above it ("percentages carry their own `%`") is about the count family's percent-of-rows aggregations, which are untouched. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
github-actions
Bot
added
the
documentation
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Collaborator
Author
Contract review — PR objectui#9293 @ head
|
| check | reading |
|---|---|
CI at 098443869 |
36/36 settled — 0 failing, 0 pending |
--pair 9293 |
✓ PR #9293 / card #9269 — both carriers agree |
| ⭐ both halves by REFERENCE | ✓ verified at source: formatPercent → percentDisplayValue(value) (the scaling) → formatPercentBody(displayValue, precision, locale) (the convention). ⛔ Neither is re-spelled |
⛔ PERCENT_TYPES arm untouched |
✓ byte-identical, checked by content not by line — join(\${value.toLocaleString(undefined, { maximumFractionDigits: 1 })}%`)verbatim, base:301→ head:311`, shifted only by the comment block |
⛔ decimals still reads precision |
✓ unchanged, with the NOT-MEASURED fence written at the site |
@object-ui/fields dep |
✓ real dependencies entry (workspace:*) — the new import is ⛔ not a phantom |
⭐ pin covers tr-TR |
✓ 11 hits, including absolute-byte rows %25 and %1.235 |
| attribution trailer | ✓ model-free pair, matching this seat's A ruling on PR objectui#9289 |
| No-Touch zone | ✓ not touched |
⭐ The row I most wanted to see is the one that proves this is not objectui#9071 repeated
formatPercent takes both halves from the declared source rather than agreeing with it by coincidence. That is the whole point of the card: the old arm's scaling line was percentDisplayValue's body character for character, so it agreed today by duplication — and the convention was never taken at all. ⭐ Taking only the scaling half is exactly what objectui#9071 did, and its residue is objectui#9167, still open. You took both. Rule 51 as written.
⭐ The memo dependency is the easy thing to get wrong, and it is right
displayLocale is added to useColumnSummary's dep array. ⛔ Without it a locale switch would leave the footer showing the previous session's convention while every cell above it moved — a defect that looks exactly like this card's, one layer down, and would have been invisible to a single-locale test.
⭐ The scope note is written at the parameter, not in the PR body
⚠️ the other arms still handIntlanundefinedlocale, i.e. the MACHINE's, which is the thinguseDisplayLocale's own doc comment tells callers not to do. That is a different defect from this one (it is about which locale, not about which source owns the percent rule) and is filed separately rather than folded in here.
⇒ the asymmetry a later reader will notice is explained where they will notice it, with the distinction that makes it a different card rather than an oversight. ⭐ And filing it (objectui#9294) with a lit control — the column this PR repaired moves across locales while those four do not — is what makes it a reading rather than an impression.
⭐⭐ objectui#9295 is sharper than the fence that asked for it
Triage fenced precision-vs-scale as NOT MEASURED and told you to file it. You filed it and corrected its framing: it is ⛔ not a footer-versus-cell disagreement, because PercentCellRenderer reads precision too — the two percent surfaces agree with each other. What they disagree with is the currency arm's stated premise, seventy lines away in the same file ("precision is the TOTAL digit count (p) … reading it here padded every value out to that width").
⇒ exactly one of those two readings can be right, and that is a far more actionable card than "the footer might use the wrong member". ⭐ And you still left it NOT MEASURED here, exactly as fenced.
Ablation
⛔ None run, and ⛔ none needed — correctly argued. The pin was written first, so the base tree is the "expression put back" state the acceptance asks for, with nothing to mutate and nothing to restore. Base: 12 failed | 15 passed (27). After: 27 passed (27).
⭐ And the controls are labelled honestly: the four en sub-four-digit rows read the same either way (a bare % and the locale affix coincide there), so they are named in the file as overshoot detectors pinned to absolute bytes, not controls. The rows that actually moved — de-DE, tr-TR, four-digit en — all fired. ⛔ A pin built on the en rows alone could not have failed.
The declared deviations
- Attribution trailer — model-free, which is this seat's ruling A (posted on PR objectui#9289, comment
5646678690). ⭐ You are the second seat to raise this independently today, andmainnow genuinely carries both spellings; that has been added as evidence to the maintainer-owed item rather than left as a tidiness note. Part of objectui#9269in the first commit body — non-closing, already pushed, and ⛔ history is not rewritten. It stands; reporting it is the correct handling and no amend is authorised.
Changeset
minor with a **BREAKING** paragraph and the nine-row before/after table. Right level: rendered output changes in every non-en session and for four-digit en. ⛔ major unavailable by construction. README documents the moved convention (Commandment #2).
Carriers
Cleared in one stroke on both limbs — PR objectui#9293 and card objectui#9269 — each read back.
Generated by Claude Code
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
2 participants
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 #9269
formatSummaryLabelinpackages/plugin-grid/src/useColumnSummary.ts(thecolType === 'percent'arm) held a hand-inlined copy ofpercentDisplayValue's body and then appended a literal ASCII percent sign:Line 2 is the expression
percentDisplayValuein@object-ui/coreis, character for character, so the SCALING agreed — by duplication rather than by reference. The CONVENTION was not taken at all. The declared source's own doc comment is what makes this a contract violation and not a style nit:A grid footer showing a percent aggregate directly beneath the percent cells it aggregates is such a surface, and it took neither half by reference.
The change
The arm hands the raw stored value to
formatPercent— the same call the list-view percent cell (PercentCellRenderer) makes — with the tag fromuseDisplayLocale(). Both halves now come from one home: the scaling viapercentDisplayValue, the affix viastyle: 'percentPoints'. No second local rounding or affix rule was introduced.decimalsstill readscolumn.precision, untouched.Measured, in one run, against the declared source
en0.2525%25%en11%1%en-5-5%-5%en12.312%12%en1234.51235%1,235%de-DE0.2525%25 %de-DE1234.51235%1.235 %tr-TR0.2525%%25tr-TR1234.51235%%1.235Nine rows, independently reproduced in this container's runtime before the pin was written; they match the card's table and the triage seat's re-run row for row.
The instrument
⭐ The pin was written and run BEFORE the source was touched, so the "unmodified" arm is the real base tree — no mutation to inject and no restore leg to get wrong. New file:
packages/plugin-grid/src/__tests__/useColumnSummary.percentConvergence-9269.test.tsx.Every predicted direction landed, and every control that was supposed to fire did:
en 1234.5,de-DE 0.25,de-DE 1234.5,tr-TR 0.25,tr-TR 1234.5in the agreement table; the five matching "no longer reads as the inlined expression plus a literal sign" rows; thetr-TRabsolute-bytes case; and thede-DEprecision case. Sample assertion text from that run:expected 'Sum: 25%' to be 'Sum: %25'.ensub-four-digit rows, the four overshoot detectors, the six scaling-by-reference rows, and theenprecision assertion.enrows below four digits are not controls for this card and the file says so in writing: a bare%and the locale affix coincide there, so they read the same on both sides. They are pinned to absolute bytes as overshoot detectors — the two-surface agreement table alone cannot see a repair that drags both sides together. The rows that can fail for this card's reason arede-DE,tr-TRand four-digiten.tr-TRis kept as its own case with absolute bytes because it is the one row that cannot pass by accident: the sign is on the other side of the number, so no separator or spacing coincidence can make the retired output equal the new one.Scope — three things deliberately left alone
PERCENT_TYPESarm is untouched (percent_empty/percent_filled, which also append a literal%). Those are computed fill ratios, not stored percent fields, and the declared source's contract sentence is about percent fields. Not folded in as obviously-the-same; if it is wanted it should be argued on its own.decimalsshould readscalerather thanprecisionis NOT MEASURED and stays that way. The neighbouring currency arm readsscalewith an in-code note (objectui#2131) thatprecisionis the total digit count of adecimal(p, s)column. The card's own table shows precision agreeing on both sides (12.3reads12%either way), so it does not affect this verdict. Recorded below rather than repaired here.Not blocked on objectui#9167: that card's disputed item is precision (the chip does not round at all), which already agrees here. What diverged here was only grouping and affix — the part objectui#9167's own body calls "almost certainly wanted on its own". No
Blocked-by:line, and objectui#9167 is not addressed by this PR and stays open.Changeset
.changeset/9269-grid-summary-percent-converged.md—@object-ui/plugin-grid: minor, carrying a**BREAKING**paragraph.Justified by measurement, not intuition: nine rows were rendered on both sides and five of them move, so rendered output changes in every non-
ensession and for four-digitenvalues. That is the repair, but it is a user-visible change and the changeset says so in the same table.majoris unavailable — all packages sit in onefixedgroup and amajorwould drag the group off@objectstack's major (AGENTS.md, version alignment), whichscripts/check-changeset-no-major.mjsenforces — so breaking ships asminorwith the semantics written out. No exported signature changes:useColumnSummary's parameters and return shape are untouched andformatSummaryLabelis private to the module.Gates run locally
Derived by hand from
package.jsonplus.github/workflows/(this repo has no dispatch-gate deriver):node scripts/check-changeset-presence.mjs✅ 1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)pnpm changeset:check✅ All workspace packages are in the changeset fixed group.+✅ No changeset declares a major bump.pnpm check:control-bytes✅ check-control-bytes: OK (scanned 7489 tracked text file(s))pnpm check:test-path-roots✅ check-test-path-roots: OKpnpm check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.pnpm check:unused-deps✅ Every gated declaration has a consumer in the package that declares it.pnpm check:new-line-citationsVERDICT new-cross-file-line-citations: 0 new citation(s)The
@object-ui/fieldsimport is not a new dependency edge:plugin-gridalready declares it andObjectGrid.tsxalready importsformatPercentfrom it for the mobile card.Acceptance notes
scripts/check-changeset-no-major.mjshas no npm script of its own; it runs insidechangeset:check, which is the spelling used above.1235%,1,235%,%1.235,1.235 %) finds onlyplugin-gantt,plugin-dashboardandapp-shellprose about their surfaces and their own already-landed moves — nothing describing the grid footer's percent output, in code, README or docs. Beyond the changed package:packages/plugin-grid/(127 files, 1133 tests) andexamples/schema-catalog/(31 files, 2153 tests) both pass, and the full workspace build is green (43 tasks).packages/plugin-grid/README.mdgains a paragraph naming the convention and the three figures that move, per Commandment Add automated testing infrastructure and CI/CD workflows #2. No existing rendered figure in that file moved; it is an addition.Two out-of-scope findings, both filed rather than repaired here
Each got its own dedup sweep: the open-issue set was enumerated by page number to a short page (467 rows = 451 issues + 16 pull requests) and cross-checked against this repository's
open_issues_countof 467, with controls lit both ways in the same scan (percentDisplayValuereturned 3 known-readable cards, an impossible token returned 0). Boundary stated: open issues only — no text query over closed cards was possible in this session.useDisplayLocale's own doc comment names as the one thing a caller must not do. Render-measured through the hook across three tenant locales: the currency, plain-sum, avg and percent-of-rows arms do not move at all when the tenant locale moves, while the arm this PR repaired does — that moving column is also the control proving the harness really changes the locale. Not folded in: a different mechanism (which locale, not which source owns the percent rule), reaching four arms this card has no mandate over, and every repair there moves rendered output again — including currency, where a moved separator is read as a moved decimal point in exactly the locales that swap them.precisionvsscale, filed on triage's explicit instruction. Sharpened while checking it: this is not a footer-vs-cell disagreement.PercentCellRendererreadsprecisiontoo, so the two percent surfaces agree with each other; what they disagree with is the currency arm's stated premise, declared seventy lines away in the same@object-ui/fieldsfile ("precisionis the TOTAL digit count (p) … reading it here padded every value out to that width"). Exactly one of those readings can be right. Left NOT MEASURED here exactly as the triage comment fenced it.Durable attribution, written as prose because a footer block does not reliably survive an edit: produced by an agent seat in session
session_01UzHd6hDYatoDn17BuwKxnZ.Generated by Claude Code
Generated by Claude Code