Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-charts): refuse a second scatter series instead of drawing it at the first one's y - #7400
Draft
os-project-manager wants to merge 3 commits into
Draft
Conversation
… it at the first one's y A scatter binds one measure: `series[0].dataKey` is the YAxis key and every `<Scatter>` reads the same rows through it, so a second series added a colour and a legend entry and nothing else. Measured: two series over two rows painted four symbols at two positions, each drawn twice, and the second measure's values appeared nowhere on the plot. The data was valid and the picture was confidently wrong — no existing refusal could see it. Ruled B (maintainer, 2026-09-02): a `chartType: 'scatter'` with two or more `series` now renders the file's refusal shell under `data-chart-error="scatter-multi-series"`, stating that a scatter plots one measure, naming the fix (keep exactly one series) and listing the series keys as data. A single-series scatter is unchanged. No multi-measure projection is built; render-time only (authoring-time rejection follows #7113). A `compareTo` overlay counts as a second series — it was painted at the primary's y by the same mechanism. The sentence resolves from `chart.scatterOneMeasure` in all ten locale packs via `useSafeTranslate`, the hook ObjectChart already uses (no module-scope defaults map). Measured against a control build of the base: +420 gzipped bytes in the console's eager `framework` chunk, 243 over its ceiling. The ceiling is NOT raised here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
os-project-manager
commented
Sep 2, 2026
CollaboratorAuthor
Standing down on |
| build | framework gzip | delta | gate |
|---|---|---|---|
control (a6e5f7050) | 523,823 B | — | exit 0 (headroom 177 B) |
| mechanism only — renderer + tests, packs at base | 523,823 B | 0 | exit 0 |
mechanism + ten-pack copy (eacc55913) | 524,243 B | +420 | exit 1 |
⇒ The mechanism costs the eager closure nothing — plugin-charts is lazy and not in the closure. The entire overage is the ten-pack copy, ~42 bytes per pack, for one sentence already at the ruling's floor (states one measure, names the fix).
Why I am not making it green
- ⛔ Raising
PER_CHUNK_GZIP_CEILINGS['framework']is 门禁削弱 — the maintainer's floor, not an execution lane's, however well-precedented. - ⛔ Shipping the mechanism with English only and deferring the nine translations delivers less than ruling B specifies. It is a real option and it is on the decision card as one — but it is the maintainer's to choose, not this lane's to take because it happens to be the one that compiles.
Where the decision lives
#7399 — one maintainer decision covering this PR and PR #7391 (#7210 / #7225), since both need ruled user-facing copy in the same ten locale packs and both meet the same 177 bytes. It now carries both forks' measurements and the asymmetry between them.
#7194 is pm:blocked with Blocked-by: #7399 and Unlock-action: re-check PR #7400. This PR stays draft, keeps needs:contract-review, and is not enqueued.
Separately, and not blocked by any of the above
Clause-② is yes on this card, so an in-seat contract review at CONTRACT_REVIEW_TIER is owed before needs:contract-review comes off. This seat runs opus and is not at tier, so a claude-fable-5 reviewer has been dispatched — as was done for #7182. That review runs now, in parallel with the budget decision, so this PR is ready the moment #7399 rules. compareTo consequence this PR's description raises: supportsCompareTo excludes only pie / donut / funnel, so a compare-to scatter is a two-series scatter in the product and now refuses. That was surfaced by the implementer rather than buried, which is the right instinct.
mergeable_state reads behind; the branch needs updating before any enqueue. Not done now — there is no point rebuilding against a moving base while the PR is parked.
Generated by Claude Code
This was referenced Sep 2, 2026
os-project-manager
commented
Sep 2, 2026
CollaboratorAuthor
Contract review complete — PASS WITH REQUIRED AMENDMENTS. |
| decision | about | |
|---|---|---|
| #7399 | ruled? no | the framework byte ceiling |
| #7402 | ruled? no | whether ruling B reaches a compareTo scatter |
Draft, needs:contract-review retained, not enqueued.
Generated by Claude Code
…nd name the pin's real guard Prose only — no behaviour, no pins added or removed, no locale copy touched. Two amendments required by the Clause-② contract review of this PR. R1 — the changeset and the `compareTo` pin comment both presented the overlay's refusal as settled by the maintainer ruling. The ruling's words are "a `chartType: 'scatter'` with two or more `series`"; it never mentions `compareTo` or `variant`, and "the ruling counts series, not variants" was the implementer's reading of it. Both now describe the behaviour as current and unruled, pointing at objectui#7402, which is open on whether refusing is right on that path. The pin stays — it records what ships — and its comment now says that shape (b) on that card would invert it as a decision landing, not as a regression. R2 — the locale pin carried "The gate `check:i18n-keys` enforces this equality too", which is false. `inlineDefaultValue` reads a `defaultValue` property off an options object and `interpolationOptions` skips a positional string, and this call site passes the English positionally. Re-measured on this head: with the inline fallback mutated to `Keep ONE series:`, all three i18n gates still exit 0 (`check:i18n-keys`, `check:i18n-drift`, `check:i18n-dead-keys`), and the mutation was proven on disk by blob hash before each run and the restore by blob equality plus an empty `git diff HEAD`. The comment now says the assertion is the sole guard and why no gate sees the pair. Verified: 13/13 pins in AdvancedChartImpl.scatterSeriesArity.test.tsx pass; check-changeset-presence and all three i18n gates exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
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 freeto 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#7194
Maintainer ruling B (comment 5507673384, 2026-09-02): a
chartType: 'scatter'with two or moreseriesrenders the file's existing refusal shell, stating that a scatter binds one measure and naming the authored fix; a single-series scatter is unchanged; no multi-measure projection (option A waits for a real caller); render-time only — the authoring-time (zod) rejection follows #7113 when that card rules. Clause-② yes ⇒ this PR is a draft; it is not flipped ready and not enqueued.What changed
packages/plugin-charts/src/AdvancedChartImpl.tsx— the scatter arm checksseries.length > 1first (a binding fault is true of the spec whatever the rows carry, so it outranks the finding(plugin-charts): scatter is NOT MEASURED for the degenerate-magnitude class — its all-positive CONTROL drew zero marks, so the sweep's zeros say nothing #7171 positional refusal the same way the missing-category-key guard outranks the series guard) and rendersSeriesArityRefusal: the existingChartRefusalshell underdata-chart-error="scatter-multi-series",role="status", no console warning (matching the three scatter/magnitude answers already in the file). The shell fits without modification. The sentence is read fromchart.scatterOneMeasurethroughuseSafeTranslate— the hookObjectChartalready imports from@object-ui/i18n— so no module-scopecreateSafeTranslationmap is added and no partial i18n mock is disturbed (census: 91 tests mock@object-ui/i18n; none of the partial ones reaches this renderer). The series keys are rendered as data after the sentence, as code elements, outside the translated copy.packages/i18n/src/locales/{en,zh,ja,ko,de,fr,es,pt,ru,ar}.ts— one key,chart.scatterOneMeasure. en:A scatter plots one measure. Keep exactly one series:— states one measure, names the fix; the keys follow as data.packages/plugin-charts/src/AdvancedChartImpl.scatterSeriesArity.test.tsx— 13 pins: single-series control draws 2 marks / no refusal / no note;series: []and noseriesstill reach the chart through thevaluefallback; two series ⇒ refusal, no chart container, 0 symbols, no legend; copy contains the sentence and lists both keys; three series refuse; avariant: 'comparison'second series refuses; the binding refusal wins overno-plottable-points; holds with zero rows; no console warning; a two-seriesbarstill draws both series;enpack value equals the inline fallback; every pack defines it and none copies the English; under anI18nProvider(zh) the pack value renders and the keys stay untranslated..changeset/7194-scatter-one-measure.md— patch for@object-ui/plugin-chartsand@object-ui/i18n.A consequence the contract review looked at:
compareToon scatterObjectChart.supportsCompareToexcludes only pie / donut / funnel, andDatasetWidgetsynthesises avariant: 'comparison'series for every compared measure — so a compare-to scatter is a two-series scatter in the product. Today its overlay is painted at the primary's y values (the overlay never reads its own__comparisoncolumn — the same mechanism this card fixes). Under the guard it now refuses, and the pin records that.series" and never mentionscompareToorvariant— and objectui#7402 is open on whether refusing is the right answer on this path. This PR does not pre-judge it: if that card excludes scatter fromcompareTo(its shape (b), as pie / donut / funnel already are), the pin inverts, and that is the decision landing rather than a regression. Exempting the overlay would be a one-line change to the predicate, and the picture it would restore is the wrong one.amount,amount__comparison" while the author wrote exactly one series and never wroteamount__comparison.Byte cost — measured, both halves, against a control build of the base
Control:
origin/mainata6e5f7050built in a second worktree (pnpm --filter @object-ui/console exec vite build, thennode scripts/check-eager-closure-budget.mjs), the same recipe for all three builds; per-chunk numbers read fromapps/console/dist/eager-closure.json.frameworkgzip bytescheck:eager-closurea6e5f7050)✅ framework 511.5 KB / 511.7 KB ceiling (headroom 0.2 KB)eacc55913, unchanged ate090fd866)❌ framework 512.0 KB / 511.7 KB ceiling (OVER by 0.2 KB)Ceiling
PER_CHUNK_GZIP_CEILINGS['framework']= 524,000; headroom on the control is 177 bytes; this branch is over by 243. The code costs the eager closure nothing (plugin-chartsis lazy and not in the closure); the ten-pack copy costs 420 bytes, ~42 per pack, for one sentence already at the ruling's floor (one measure + the fix). ⛔ The ceiling is not raised here and the copy is not weakened below the ruling. This is the fork the dispatch anticipated (PM comment 5513454764, zone 2b): a maintainer budget decision, filed as #7399, one card covering #7194 and #7210. This PR stays draft until it rules; landing order against #7391 is the PM's.Premise re-checks against the merged ref
origin/maina6e5f7050: yes —scatterYKey = series[0]?.dataKey || 'value'and the legend named offseries[0]. fix(plugin-charts): draw every categorical x-axis label on short axes #7385 (d6fe1e1f2) touched only the categorical x-axis path (xAxisCommonProps, label rotation and ellipsis); scatter's numeric axes and the refusal surface were untouched.examples/content/docs/apps/packages/*/examples(controlbar: 3 files). ⭐ The contract review widened this census to the dashboard-widget spellingtype: 'scatter' | 'bubble', which the regex above cannot see: still 0, with controltype: 'bar'firing at 10 files including fiveexamples/schema-catalog/**/plugin-dashboard/*.json. The zero holds on a stronger basis than first claimed. One two-series scatter exists in a test corpus:packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsxrendersplugin-charts:scatter-chartwith the shared two-seriesCHART_SERIES. The sweep stays green on this branch (202/202) because that entry never reaches the scatter arm — the SDUI path does not apply a registration'sdefaultProps, so it renders with nochartType(filed as bug(plugin-charts): a registration'sdefaultProps.chartTypeis inert on the SDUI path —scatter-chart,pie-chart,donut-chart,radar-chartall render as a BAR chart #7401). Not an authored product scatter; the ruling's premise holds. No file outside the declared surface was touched.Overlap
claude/issue-7248-scatter-y-domain, claims 5513102502 / 5513329051, file surfacepackages/plugin-charts/src/**) is live on the same scatter arm; its branch is empty as of this push. This diff adds 11 lines at the top of the arm and one component aboveunplottedPointsNote; the union is textual. That claim states the lane seat keeps precedence.useSettledSchemawas extracted and published with ONE adopter — the convergence #6482 asked for is 1 of 4, and the gantt's ungated double fetch is still live #7225) shares the eagerly-loaded locale packs and the sameframeworkceiling.Verification — mechanism at
eacc55913, amendments ate090fd866pnpm exec vitest run packages/plugin-charts/ packages/i18n/(root, under the shared verify lock):Test Files 100 passed (100)/Tests 1323 passed (1323), exit 0.pnpm exec vitest run packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx: 202 passed on this branch and on the control.AdvancedChartImpl.tsxchecked out over the commit, the new file readsTests 8 failed | 5 passed (13)— the 8 refusal/pack pins red, the 5 controls green; restored withgit checkout HEAD -- FILE,git diff HEADempty, blob hash624cba8…on both sides.pnpm --filter @object-ui/plugin-charts run type-checkexit 0 (the new test file is in--listFiles);pnpm --filter @object-ui/i18n run type-checkexit 0. Dependency closure built first.check:i18n-keys0 ·check:i18n-drift0 (1 key added, 0 en values changed) ·check:control-bytes0 ·check:vi-mock-specifiers0 ·check:vi-mock-inherit0 ·check:shell-escape-residue0 ·check-changeset-presence0 ·check:eager-closure1 (above, expected).eslinton the changed files: 0 errors, warnings are the pre-existingno-explicit-anypattern the sibling chart pins also carry.pnpm lint(repo-wide) was not run locally; CI owns it.Contract review (Clause-②) — PASS, amendments applied at
e090fd866Reviewed at
CONTRACT_REVIEW_TIER. Verdict PASS WITH REQUIRED AMENDMENTS; both are applied and verified, so the gate label is off.compareTopin comment no longer attribute that behaviour to ruling B; both now describe it as current and unruled, name [Decision] Does #7194's ruling B reach acompareToscatter — a published capability it never named? The refusal currently swallows it and names a fix the author cannot act on #7402, and state that shape (b) there inverts the pin as a decision landing, not a regression. The pin itself stays: it records what ships. This section of the PR body was corrected on the same grounds.check:i18n-keysalso enforces the inline-fallback /en-pack equality. ⭐ Verified false by mutation, twice — with the fallback changed toKeep ONE series:all three i18n gates still exit 0, becauseinlineDefaultValuereads only adefaultValueproperty off an options object andinterpolationOptionsskips a positional string. ThetoBe(EN_COPY)assertion is the sole guard, and the comment now says so. Mutation proven on disk by blob hash and anchored token counts; restore proven by state (git diff HEADempty, blob back to624cba845).Amendments are prose only — 2 files, 35 insertions, 8 deletions, no renderer and no locale packs. Eager-closure figures byte-identical across the amendment:
framework524,243 at botheacc55913ande090fd866.Not done here
ChartSchema's real data model — chart-leveldataandxAxisKey— is undeclared, and #6896's retirement now points authors at it by name #7113 (ChartSchemadata model) is not pre-empted: no authoring-time rejection is added.🤖 Generated with Claude Code
https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
Generated by Claude Code