fix(types): ChartDataSeriesSchema declares the six series keys the renderer reads (objectui#7546) - #7684
Merged
Merged
Conversation
…nderer reads (objectui#7546) `ChartDataSeriesSchema` is a non-strict `z.object`, so `label`, `variant`, `opacity`, `dashArray`, `stack` and `yAxis` — all read by `normalizeSeries` and all doing real work in `AdvancedChartImpl` — were stripped in silence while `safeParse` reported success. Reproduced red on a472b07: the card's fixture parsed to `{ name }`. Both faces (TS twin and Zod mirror) now declare the six with the read's own value domains: `label` as the spec's `I18nLabel`, `variant` as the three values the normalizer honours, `opacity` finite, `yAxis` left/right. Measured per key with lit controls, per the PM ruling. `chartType` — the seventh key the review found — is deliberately NOT declared: it is the renderer's internal spelling of `type`, the spec lists it as an alias of `type` and refuses it by name, and nothing on this authoring face writes it. Reported for its own card; the pin test holds that gap visible. Docs: the schema-reference `series` row and a per-series options section in the plugin-charts guide. Changeset graded `minor` — the mirror now refuses a malformed value the renderer used to ignore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
os-sam
added
needs:contract-review
and removed
package: types
tests
labels
Claude
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 5, 2026
Contributor
Contract review —
|
| # | Change | Verdict | Deciding measurement |
|---|---|---|---|
| 1 | label: I18nLabelSchema.optional() / TS label?: string | I18nLabel |
RIGHT | Spec member (probe: locale map {en, zh-CN} → OK). Producers: internal-shape { dataKey, label } at plugin-dashboard/src/DashboardRenderer.tsx:648-652, app-shell/src/views/ObjectView.tsx, plugin-list/src/ListView.tsx, plugin-view/src/ObjectView.tsx (census src label 5 in 4 files; control dataKey 13). Consumers: normalizeSeries label() (normalizeChartSchema.ts:242), ChartRenderer.tsx s.label || s.dataKey, AdvancedChartImpl.tsx:1364, 1592. Value domain = the reader's (string, or first string of a map). |
| 2 | variant: z.enum(['primary','comparison','current']) / TS three-value union |
WRONG on the third member (primary/comparison RIGHT) |
Installed spec refuses it: variant: Invalid option: expected one of "primary"|"comparison". Authoring-face producers of current: docs 0 at base (git show a472b0716 — both doc pages), fixtures 0, designer 0 (the chart registration's series is one code input, plugin-charts/src/index.tsx:114). The only two producers — ObjectChart.tsx:852 s.variant || 'current', DatasetWidget.tsx:1450 s.variant ?? 'current' — write onto internal-shape (dataKey) arrays handed straight to ChartRenderer (ObjectChart.tsx:1118) or to SchemaRenderer as any (DatasetWidget.tsx:1519), whose dev-only validator is core's hand-written validateSchema (schema-validator.ts imports no zod) — they never meet ChartDataSeriesSchema. Core's own authored-series reader seriesPresentation (core/utils/chart-presentation.ts:125) keeps primary | comparison and drops current; ChartRenderer.tsx:65 types current on the internal arm only. current is therefore the renderer's internal default, produced by nothing an author writes — the identical evidence profile the PR uses to exclude chartType. Declaring it on the published Zod face and teaching it in two doc pages while #7682 (the author's own drift card on this exact synonym) is open is the AGENTS.md #0.1 fossilisation: a renderer-side tolerance becoming a second de-facto contract. Narrowing to the spec pair refuses zero live producers (census: the only variant value ever written inside a series literal, in any file class, is 'comparison'). |
| 3 | opacity: z.number() / TS number |
RIGHT (one reservation, non-blocking) | The read's domain: num() = finite number; zod 4.4.3 z.number() refuses NaN and Infinity (probed: Infinity → false, 7 → true). Producer chart-presentation.ts:131 (dataset/report path, exercised by DatasetReportRenderer.chartChrome.test.tsx); consumer AdvancedChartImpl.tsx:94-95. Reservation: wider than the spec's .min(0).max(1) (probe opacity: 7 → REFUSE Too big) and than the PR's own doc row ("0–1"), so the doc claims a bound the mirror does not enforce. Consistent with the card's "declare the read's domain" rule; noted, not refused. |
| 4 | dashArray: z.string() |
RIGHT | Spec member; producer chart-presentation.ts:130; consumer AdvancedChartImpl.tsx:96 (strokeDasharray). Control lit (name/dataKey counted by the same query). |
| 5 | stack: z.string() |
RIGHT | Spec member; producer chart-presentation.ts:128; consumer AdvancedChartImpl.tsx:1893, 2023 (stackId); tests 5. |
| 6 | yAxis: z.enum(['left','right']) |
RIGHT | Spec union identical (probe yAxis: 'top' → REFUSE). Every series-level yAxis literal in the repo: 'left' 1, 'right' 9 — nothing else; the 'total'/'amount'/'n'/'deals' hits are chart-level bare-string report axes (e.g. normalizeChartSchema.test.ts:42), not series. Producers chart-presentation.ts:124, DatasetWidget.tsx:1446; consumers AdvancedChartImpl.tsx:1887, 2019, 2025. No live producer refused. |
| 7 | chartType left undeclared, reported |
RIGHT as a liveness disposition (carrier defect in ③ Q1) | Installed spec refuses it by name: Unrecognized key(s) on this chart series: chartType. Did you mean chartType → type?. Core translates authored type → internal chartType (chart-presentation.ts:122-123), confirming it is the internal spelling. Census of series-level chartType: docs 0, fixtures 0, src literals 0, tests 9 — all nine internal dataKey-shaped, which take ChartRenderer's raw fast path and never reach the normalizer's first limb. Ablation re-measured on 197 files / 2380 tests: B0 baseline 2380 passed; B1 (str(raw.chartType) ?? deleted, blob 05a66dfb → bd9e640f) predicted 0 red → 2380 passed; B2 control (?? str(raw.type) deleted, blob → 1cc8e75a) predicted ≥2 red → 2 failed in normalizeChartSchema.test.ts and ChartRenderer.specSeries.test.tsx. Restore proven (blob = HEAD, diff empty). The author's reading holds on a larger population. |
| 8 | TS twin in lockstep; ts-expect-error ratchet on chartType |
RIGHT | A′ (six TS lines deleted, Zod kept, blob 8abbf7fe → cc7c6bfd): the parity guard is silent by design (its header says that direction is "a different class"), but chart-series-keys-7546.test.ts fires under tsc — TS2353 at 174:7, TS2339 at 185:39 and 186:37. Covered. |
| 9 | Object stays non-strict; data tombstone and #6939 refinement untouched |
RIGHT | Pin (e) + chart-inline-data-retired.test.ts; baseline 3 files / 54 tests passed. |
| 10 | Docs: schema-reference.md series row; plugin-charts.mdx "Per-series options" table + example |
WRONG insofar as they teach current (rest RIGHT) |
Both pages publish variant as primary / comparison / current; base had no such teaching anywhere (git grep at a472b0716: 0). See row 2. |
Parity ratchet (brief item 4): not dark. Ablation A (six Zod declarations deleted, blob f6769b2c → db9ef916, anchored counts 0/0): tsc -p tsconfig.test.json exit 2, zod-mirror-parity.test.ts(1478,14): error TS2322: Type '"data-display.zod.ts#ChartDataSeriesSchema"' is not assignable to type 'never' plus chart-series-keys-7546.test.ts(163,38): TS2339. The channel is real in CI: ci.yml Type Check → pnpm type-check → turbo → @object-ui/types type-check = tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json; this head's job log shows ##[group]@object-ui/types:type-check / cache miss, executing a834f9af5a52ccd5 (02:25:10Z), job success. The vitest "miss" is exactly what the parity file's own header predicts ("its Tests 12 passed (12) line does not move when this half reddens"). Author's disclosure is accurate. Also closed: check:doc-snippets, NOT MEASURED locally — CI Doc Snippet Type Check (job 101232202884) success; all 32 check runs green.
② Semver grading
- Declared:
'@object-ui/types': minor. Correct level:minor. AGENTS.md §版本号策略: "objectui 自身的破坏性变更也标minor（在正文里写清 breaking 语义即可）";majoris mechanically refused byscripts/check-changeset-no-major.mjs(CI "Changeset Bump Policy" green). The precedent the body cites (finding(types/plugin-charts): the staticChartSchemanode's two data keys are declared-but-unread —ChartDataSeries.datais required and never read, andcategoriesis read as a SERIES list, not axis labels #6896, [Decision]ChartSchema's real data model — chart-leveldataandxAxisKey— is undeclared, and #6896's retirement now points authors at it by name #7113, same file) is the same transition. - Narrowing stated plainly: yes. The body leads with "
⚠️ Shipped asminor, notpatch… one document class that validated before now refuses" and enumerates the refused values (variant: 'bogus',yAxis: 'top',opacity: '0.4'/Infinity, non-stringstack/dashArray, non-string non-maplabel), then the widening, then a FROM→TO block. Both halves are present. - Consequence of ① row 2: the FROM→TO block and the paragraph "
variantis one value wider than the spec's…" must be rewritten whencurrentcomes out; the grade staysminor.
③ Boundary-flag disposition
Q1 — chartType shape (A alias-refusal / B fold / C declare). A. It is the contract of record's own posture (the installed spec's alias map already says chartType → type), it makes the strip loud without minting a second writable name, and its narrowing is graded on its own card. Not for this PR. However, it currently has no carrier: Fixes #7546 closes the only open card that names it; #7690 carries only the axis-config half; #7682 carries only variant. After merge the gap survives only as a comment on a closed card and a pin-test comment. That is measured-and-lost. A card must exist and be named in the PR's "Out of scope — filed" list before merge (a PM write; the PR is blocked until the reference exists).
Q2 — variant three-value union (A keep / B narrow now). B, narrow now. (i) No authoring-face producer writes current — docs 0, fixtures 0, designer 0 — and the two internal producers never meet the mirror, so narrowing refuses nothing live; (ii) it is the same instrument reading the PR applied to chartType, and consistency is the finding; (iii) AGENTS.md #0.1; (iv) #7682 remains the right place to decide the renderer's tolerance — the published contract does not need to pre-empt it in the internal direction, and option A would force a second narrowing changeset later.
Q3 — axis-config object dialect (A file / B leave). A — already done. #7690 (02:28:08Z, pm:queue, domain:ui, package: types, priority:p3) quotes the card body's measurement verbatim and names #7684's fence. The closing keyword is correct for the six-key half. Half-state noted in the report — the card lacking needs:contract-review — is resolved: #7546 now carries the label.
Out-of-scope carriers verified: #7681 (per-series type dropped on the dataKey fast path) and #7682 (variant drift) both exist and are open.
VERDICT: REFUSE
What must change:
- Narrow
variantto the spec pair on both faces —packages/types/src/zod/data-display.zod.tsz.enum(['primary', 'comparison']);packages/types/src/data-display.tsvariant?: 'primary' | 'comparison'with the docblock's "one wider than the spec" paragraph removed; dropcurrentfromcontent/docs/api/schema-reference.md(series row) andcontent/docs/plugins/plugin-charts.mdx(table); update the pin test'sit.eachin block (b) and theEq<>pin in block (c); rewrite the changeset's FROM→TO block and its "one value wider" paragraph. No reader changes — the normalizer's tolerance forcurrentstays for finding(plugin-charts/core/types): seriesvariantcarries three different unions across its declarations and readers —currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops #7682 to resolve. - Give the
chartTypegap a carrier before merge — file the card (option A shape: named alias refusal pointing attype, its own narrowing grade) and reference it from the PR body's "Out of scope — filed, not fixed here" list, soFixes #7546does not sink a measured finding.
Not blocking, recorded: the plugin-charts.mdx opacity row says "0–1" while the mirror is z.number() (any finite); either state "any finite number (the spec bounds 0–1)" or declare .min(0).max(1).
Generated by Claude Code
Resolves the one conflict, content/docs/api/schema-reference.md: #7679 rewrote the chart node's property table (categories / data / xAxisKey rows) while this branch rewrote the series row to list the declared per-series keys. Kept main's table structure and folded the declared keys into its series row. No lockfile or generated file moved on main since the merge-base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
…t is renderer-internal (objectui#7546) Contract-review remediation on PR #7684 (comment 5548861339). - variant: z.enum(['primary', 'comparison']) on the Zod mirror and the matching TS union. The third value, 'current', is the renderer's internal default: written only by ObjectChart.tsx:852 and DatasetWidget.tsx:1450 onto dataKey-shaped arrays handed straight to ChartRenderer / SchemaRenderer (hand-written validateSchema, no zod), so it never meets this mirror; docs, fixtures and designer inputs write it 0 times (controls lit); the installed spec 17.2.0 refuses it. Declaring it would fossilise a renderer-side tolerance into a second contract (AGENTS.md #0.1) — the same reading that excluded chartType. No reader changed: the normalizer's tolerance stays for objectui#7682. - Pin test: it.each over the pair, a new refusal pin for 'current', the Eq pin narrowed; both doc pages and the changeset drop 'current'. - opacity stays z.number() (the read's own finite domain; SVG clamps, nothing is dropped); the doc row and describe now say "any finite number (the spec bounds it to 0–1)" instead of claiming a bound the mirror does not enforce. - Docs no longer teach opacity on a primary series: the renderer reads opacity / dashArray only inside comparisonStyle (objectui#7698, filed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
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
|
Contributor
Contract re-review (round 2) —
|
| # | Change | Verdict | Deciding measurement |
|---|---|---|---|
| 1 | label: I18nLabelSchema.optional() / TS label?: string | I18nLabel |
RIGHT | Spec member (same schema object imported). Census: src label 5 (the four internal-shape producers), docs 3, tests 16; control dataKey 12/7/22. Card fixture round-trips a locale map; refuses 42 at label. |
| 2 | variant: z.enum(['primary', 'comparison']) / TS 'primary' | 'comparison' |
RIGHT | Installed spec refuses 'current' (Invalid option: expected one of "primary"|"comparison") — the mirror now answers identically at path variant. Producers of current on the authoring face: docs 0, fixtures 0 (both spellings, bracket census; controls lit), designer 0 (plugin-charts/src/index.tsx:114 — series is one code input). The only writers remain ObjectChart.tsx:852 and DatasetWidget.tsx:1450 (internal-shape, never through this mirror). Narrowing refuses zero live producers. DOM: variant: 'current' through the spec path draws exactly as primary — nothing an author loses. |
| 3 | opacity: z.number() / TS number |
RIGHT (reservation, non-blocking — see ② and Q4 below) | Read's domain is num() = Number.isFinite; probe: 0.5 OK, 7 OK, -1 OK, Infinity / NaN / '0.4' refused at opacity. Spec refuses 7 (Too big) and -1 (Too small). Census of series-level literals in every file class: 0.4, 0.5, 0.6 only — the out-of-range class is empty. |
| 4 | dashArray: z.string() |
RIGHT as a contract member | Spec member, unconditional ("SVG stroke-dasharray override"). Read at normalizeChartSchema.ts:250; honoured on a comparison line/area mark — DOM: comparison line with dashArray: '2 2' renders stroke-dasharray="2 2", with no keys "4 4". |
| 5 | stack: z.string() |
RIGHT | stackProps at AdvancedChartImpl.tsx:1893, 2023; docs 2, tests 3; control lit. |
| 6 | yAxis: z.enum(['left', 'right']) |
RIGHT | Spec union identical; census of every series-level yAxis value in the repo: right 7, left 1, nothing else. |
| 7 | chartType left undeclared, reported |
RIGHT | Not re-litigated — the prior review upheld the ablation on 5× the population. Re-confirmed the two facts that ground it: spec refuses it by name (Unrecognized key(s) … Did you mean chartType → type?), and the mirror still strips it ({ name, chartType } parses to { name }). Carrier now exists: #7694 (open, pm:queue, priority:p3, filed by the PM 02:51:20Z), named in the PR body's out-of-scope list. |
| 8 | TS twin in lockstep; Eq pin narrowed; ts-expect-error ratchet on chartType |
RIGHT | tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json exit 0 on the head — the Eq<ChartDataSeries['variant'], 'primary' | 'comparison' | undefined> = true pin compiles, so the TS union is the pair. Pin file 27/27. |
| 9 | Narrowing complete on every carrier; no reader changed | RIGHT | Zod line 409, TS line 1443, docblock's "one wider than the spec" paragraph gone, both doc rows are the pair, it.each over the pair + a current refusal pin, changeset FROM→TO shows the pair. Residual current in the six files is explanatory only; git grep current -- content skills | grep variant = 0. normalizeChartSchema.ts blob 05a66dfb on both sides; git diff 6eebc54b6..HEAD -- packages/plugin-charts packages/core empty. Reverse verification: restoring the merge commit's three-value enum (blob f5c89fe4 → f6769b2c, anchored pair 0 / triple 1), predicted exactly 1 red / 26 green — observed 1 failed | 26 passed, the current pin; restored to f5c89fe4, git diff HEAD 0 bytes. |
| 10 | Merge commit 71f0c1941 |
RIGHT | git diff --stat 6eebc54b6 71f0c1941 lists exactly the PR's six files — the merged tree is main's tree plus this PR, nothing else. Branch-side file set is byte-for-byte the set of a472b0716..6eebc54b6 (30 files, 1186+/280−). merge-tree reproduces the single conflict in schema-reference.md; the resolution differs from main's file in one row only — #7679's categories / data / xAxisKey rows, its ChartDataSeries[] type name and its example rewrite are all intact. Lockfile unmoved between merge-base and main. |
| 11 | Docs: schema-reference.md series row |
RIGHT | Pair, keys listed, #7679's row semantics preserved. |
| 12 | Docs: plugin-charts.mdx "Per-series options" table + example |
WRONG on dashArray (rest RIGHT) |
DOM probe of the example's own shape — chartType: 'bar', comparison series { opacity: 0.6, dashArray: '4 4' }: the comparison rects carry fill-opacity="0.6" (the instrument is lit on the same entry) and the surface contains no stroke-dasharray at all. Source: Bar marks pass only fillOpacity (AdvancedChartImpl.tsx:2037, combo :1911), Scatter likewise (:1832), and comparisonStyle itself sets strokeDasharray to undefined for those kinds (:96). So the example teaches dashArray in a position where it is inert — the exact defect class the author removed for opacity in this remediation ("my own doc example had taught … a no-op") — and the dashArray row's stated condition (comparison-only) is incomplete: it is comparison and line/area only. The dashArray TS docblock carries the same incomplete condition. |
| 13 | Changeset '@object-ui/types': minor |
RIGHT on level (one wording gap, ②) | See ②. |
The sharpest question (Q3), judged. Declaring opacity / dashArray unconditionally is the right application of "declare the read's domain". The two evidence profiles are not the same: chartType was excluded because the name is one the contract of record refuses (an alias hazard, AGENTS.md #0.1) and its read is not load-bearing on any reachable path (0 red on limb ablation). opacity / dashArray are spec members declared unconditional, produced (chart-presentation.ts:130-131, the report test, the docs), and honoured on a reachable authoring path — lit in the DOM: { name, variant: 'comparison', opacity: 0.3, dashArray: '2 2' } through the spec-shape path yields stroke-opacity="0.3" stroke-dasharray="2 2". The conditionality is on a sibling key's value inside the renderer, not on the key's domain — the read's domain (num(), str()) is identical on primary and comparison series. Narrowing the mirror to comparison-only would (a) refuse a document the spec accepts ({ name, opacity: 0.5 }), (b) fossilise a partial renderer implementation into the published contract — the direction #0.1 forbids, and the PM's triage note on #7698 says the same — and (c) have to be undone when #7698 lands option A. The declaration also does not create the silent no-op: the render path never runs the mirror (the prior review confirmed SchemaRenderer uses core's hand-written validator), so { name, opacity } on a primary series drew identically before this PR; what the declaration changes is that parse-output consumers keep the key, and the docs can state the condition. That mitigation is exactly why row 12 is blocking: the PR's answer to "does this publish a surface that silently no-ops?" is "the docs say so" — and for dashArray the docs say it incompletely and the example demonstrates the no-op.
Q4 — opacity domain, graded. The reasoning is consistent with the rule the PR applies to every other key: the mirror mirrors normalizeSeries's read (Number.isFinite), not SVG's clamp and not the spec's bound. The distinction from variant: 'current' holds — an out-of-range number is not a second name for one intent (the N-dialects hazard), the docs do not teach it, and the .describe and both rows state the spec's bound. The weak leg is "would refuse a document class that renders today": that class is measured empty (every series-level literal in the repo is 0.4 / 0.5 / 0.6), which is the same reading that made narrowing variant free. So .min(0).max(1) would refuse zero live producers and would give mirror/spec range parity — a free narrowing available to a follow-up, not owed by this card, and the prior review already ruled it non-blocking. Recorded, not refused.
Q5 — the disclosed control miss. The zero is a reading. "variant": "comparison" over content/examples/apps = 0; the bracket census shows the fixtures population (examples/, apps/) writes no series-level variant in either spelling while name 2 / dataKey 6 / color 2 are counted inside the same series literals by the same instrument; the JSON-spelling "variant": hits in that population (222) are all outside series arrays (button/badge variants). The docs population's variant = 1 (plugin-charts.mdx:208, 'comparison'), matching the author's re-lit control.
② Semver grading
- Declared:
'@object-ui/types': minor. Correct level:minor. AGENTS.md §版本号策略: "objectui 自身的破坏性变更也标minor（在正文里写清 breaking 语义即可）";majoris mechanically refused byscripts/check-changeset-no-major.mjs(CI "Changeset Bump Policy" green on the head). Same transition, same file, as finding(types/plugin-charts): the staticChartSchemanode's two data keys are declared-but-unread —ChartDataSeries.datais required and never read, andcategoriesis read as a SERIES list, not axis labels #6896 / [Decision]ChartSchema's real data model — chart-leveldataandxAxisKey— is undeclared, and #6896's retirement now points authors at it by name #7113. - Narrowing stated plainly: yes. The body leads with "
⚠️ Shipped asminor, notpatch… one document class that validated before now refuses", enumerates the refused values, gives the FROM→TO block with the pair, and a paragraph sayingcurrent"is not a member here". One precision gap: the⚠️ paragraph defines the refused class as "a value the renderer drops in silence" —currentis not dropped (the normalizer keeps it,normalizeChartSchema.ts:247; DOM: it draws as primary), so the one refused value that the renderer honours is carried only by "not a member". Non-blocking — no authored document writes it (census 0, both spellings) — but the⚠️ list should namevariant: 'current'so the record is exact.
③ Boundary-flag disposition
- Q1 —
chartTypeshape. A (named alias refusal pointing attype), the spec's own posture; not for this PR. Carrier defect closed: finding(types): serieschartTypeis undeclared and SILENTLY STRIPPED - the normalizer reads it first, measured not live, alias-refusal owed #7694 exists (open,pm:queue, p3), is written in the option-A shape, and is named in the PR's out-of-scope list.Fixes #7546no longer sinks it. - Q2 —
variantunion. B, narrow now — done, on every face, refusing zero live producers, reader untouched (row 9). finding(plugin-charts/core/types): seriesvariantcarries three different unions across its declarations and readers —currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops #7682 keeps the renderer-side decision. - Q3 — axis-config object dialect. A — finding(types): the ChartSchema axis-config object dialect is undeclared and passes through unchecked - liveness read owed before declare-or-retire #7690 (open, p3) carries it with the card's own measurement.
- Q4 (new) —
opacity/dashArraycomparison-only. Declare unconditionally, state the condition in docs, carry the renderer gap — the right split, and finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698 (open, triaged p3 by the PM 07:05:06Z with "⛔ Do not open this as 'narrow the mirror'") is the carrier. One sub-finding from my probe belongs to it and must not be lost: finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698's body says "all five call sites applycmp?.fillOpacity/cmp?.strokeOpacity/cmp?.strokeDasharray" — measured, Bar (:1911,:2037) and Scatter (:1832) applyfillOpacityonly, sodashArrayis line/area-only even on a comparison series. Inside finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698's scope (its option-A implementer hits it), but the card body should be corrected — a PM comment on finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698, not a change to this PR. - Q5 — the control miss. A reading, not a dark instrument (above).
- Measured-and-lost check across the five carriers when
Fixes #7546closes the card: six keys stripped → this PR;chartType→ finding(types): serieschartTypeis undeclared and SILENTLY STRIPPED - the normalizer reads it first, measured not live, alias-refusal owed #7694; axis-config dialect → finding(types): the ChartSchema axis-config object dialect is undeclared and passes through unchecked - liveness read owed before declare-or-retire #7690;variantdrift /current→ finding(plugin-charts/core/types): seriesvariantcarries three different unions across its declarations and readers —currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops #7682; per-seriestypedropped on thedataKeyfast path → bug(plugin-charts): a declared per-seriestypeoverride is DROPPED when every series entry is written withdataKey— the internal-shape fast path forwards the array raw and the renderer reads onlychartType#7681 (open, p2);opacity/dashArraycomparison-only → finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698. Every measured finding has an open carrier; the only item without one is the docs defect in row 12, which is this PR's to fix.needs:contract-reviewrides on both the card and the PR; the PR is draft.mergeable_state: behindis fix(components,plugin-grid,app-shell): one authority for ComboboxOption, NamedActionDef and OrgTranslate (objectui#6349 batch 3) #7691 landing after the merge — the dispatcher's, not a finding.
VERDICT: REFUSE
What must change:
content/docs/plugins/plugin-charts.mdx— stop teachingdashArraywhere it is inert, and state its full condition. The example is achartType: 'bar'chart whose comparison series carriesdashArray: '4 4'; on a Bar mark that key reaches nothing (measured:fill-opacity="0.6"present from the same entry, nostroke-dasharrayin the rendered surface;AdvancedChartImpl.tsx:2037,:1911,:1832,:96). Either move the example to a family whose comparison mark takes a dash —areakeeps every key in the example live (stackProps,fillOpacity,strokeOpacity,strokeDasharrayat:2056) — or dropdashArrayfrom the bar example. In the same edit, thedashArrayrow and theChartDataSeries.dashArraydocblock inpackages/types/src/data-display.tsmust say the condition in full: applied today only on avariant: 'comparison'series and only online/areamarks (Bar and Scatter takefillOpacityonly). No contract change, no reader change, no new changeset semantics.
Not blocking, recorded: (a) name variant: 'current' in the changeset's opacity's range, and .min(0).max(1) would refuse zero live producers — a free follow-up for mirror/spec range parity if the maintainer wants it; (c) #7698's "all five call sites apply all three" needs the Bar/Scatter correction above, by PM comment on the card.
Generated by Claude Code
…rt-series-silent-strip
…mparison AND a line/area mark (objectui#7546) Second contract review (PR comment 5550238043) refused on one item: the "Per-series options" example in plugin-charts.mdx was a chartType 'bar' chart whose comparison series carried dashArray '4 4' — a position where the key reaches nothing. Re-measured through the real ChartRenderer under happy-dom, predictions before probes, a lit control on every probe: - bar, the old example verbatim: the comparison rects carry fill-opacity="0.6" (lit, same entry); the surface has zero stroke-dasharray attributes. - area, the new example: stroke-dasharray="8 4", stroke-opacity="0.6" and fill-opacity="0.6" on the comparison mark (both its area and curve paths); the primaries keep fill-opacity 1 and no dash; stack is live (the stacked cost area's lower edge is the revenue area's upper edge; unstacked both sit on the baseline). - line: stroke-dasharray="2 6" + stroke-opacity="0.6"; scatter: fill-opacity="0.6" on every symbol, no dash; a comparison line INSIDE a bar chart (per-series type) takes the dash — the condition is the mark, not chartType. Primary series of any family: nothing reaches the DOM. Mechanism (AdvancedChartImpl.tsx, blob 63a5e34 on main and here): comparisonStyle :96 returns the AUTHORED dashArray for every family — the undefined half at :96 is only the fallback — and the mark drops it: Scatter :1832, combo Bar :1911 and Bar :2037 pass fillOpacity only; Line :1898/:2048 pass strokeOpacity + strokeDasharray; Area :1905/:2056 pass all three. So opacity is live on every family (fill on bar/scatter, stroke on line, both on area) while dashArray is live on line/area only — the two keys split. Changes, wording only — no contract, reader or changeset-level change: - plugin-charts.mdx: the example moves to chartType 'area' (the one family where stack, variant, dashArray and opacity are ALL live), with a non-default dash '8 4' so the override visibly does work; the dashArray row states comparison AND line/area (bar/scatter take opacity only); the opacity row says every mark family, fill vs stroke per family. - data-display.ts: the dashArray docblock carries the full condition with the mark sites; the opacity docblock says it reaches every family. - changeset: the refusal list names variant 'current' — refused, but NOT a value the renderer drops (the normalizer keeps it and draws it as primary). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
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
|
Contributor
Contract re-review (round 3) —
|
| # | Item at 5f7e032a8 |
Verdict | Deciding measurement |
|---|---|---|---|
| 1 | Zod mirror data-display.zod.ts (the six declarations, variant pair, opacity finite) |
RIGHT — unchanged | Blob f5c89fe40 at 493b52eb3, 51f2a9634, 5f7e032a8; 3796 tokens, 0 differing. Suite on the head 107 files / 1769 tests green (the author's numbers reproduced exactly); tsc ×3 exit 0. |
| 2 | TS twin data-display.ts — contract |
RIGHT — unchanged | 2136 tokens at both 493b52eb3 and 5f7e032a8, 0 differing. Lit control: one keyword mutated on a copy → 1 differing token at index 1558; a comment-only mutation → 0 (the instrument is blind to comments by design, so the 0 above is a reading of code, not of prose). |
| 3 | Pin test chart-series-keys-7546.test.ts |
RIGHT — unchanged | Blob 01a1a24a5 on all three; 1474 tokens, 0 differing; 27/27 inside the 1769. |
| 4 | Renderer / normalizer — no reader changed | RIGHT | AdvancedChartImpl.tsx blob 63a5e34c9 on origin/main, 493b52eb3, 5f7e032a8 — 2902 tokens, 0 differing vs main; normalizeChartSchema.ts 05a66dfb5, 2416 tokens, 0; ChartRenderer.tsx b148a5dd4 identical. |
| 5 | schema-reference.md series row |
RIGHT — unchanged | Blob 1be46b438 on all three. |
| 6 | chartType left undeclared, reported |
RIGHT — unchanged | Mirror unchanged (row 1); carrier #7694 open. |
| 7 | Merge commit 51f2a9634 = main's tree + this PR and nothing else |
RIGHT | Parents 493b52eb3 + bc640ec56. Tree entries: merged 6356 = main 6354 + 2 added. Entries in the merged tree not in main's = exactly the six PR blobs, every one present in the PR tree; entries in main's tree not in the merged = exactly the four modified files' main blobs. git diff 493b52eb3 51f2a9634 is byte-identical (md5 bbb761c4…) to git diff 6eebc54b6 bc640ec56: 16 files from #7691 (6e8863093), #7649 (5b5d3ed77), #7662 (bc640ec56). The six PR blobs are identical on both sides of the merge — no conflict, nothing folded. |
| 8 | Head against the current main 8ad218d58 (#7700) |
RIGHT | git merge-tree --write-tree 8ad218d58 5f7e032a8 → clean, tree 289de6878; that tree differs from main by exactly the six files; #7700's eight files overlap none of them. On that merged tree: packages/types/ 108 files / 1811 tests green, type-check 0 errors — #7700's zod-mirror-parity.test.ts ledger edit and this PR's six declared keys coexist in the same package. No re-merge is owed; AGENTS.md says the queue rebuilds on current main. |
| 9 | Wording-only scope of 493b52eb3..5f7e032a8 |
RIGHT | Files changed in the wording commit: exactly three (.changeset/…7546…md, plugin-charts.mdx, data-display.ts). The mdx has 10 tsx fences; 9 are token-identical and fence #5 differs in exactly two tokens: 'bar'→'area' at [10] and '4 4'→'8 4' at [102]. Changeset frontmatter identical ('@object-ui/types': minor). |
| 10 | The blocking item — the example (chartType: 'area', stack ×2, comparison { dashArray: '8 4', opacity: 0.6 }) |
RIGHT — fixed | Rendered verbatim (R1): 3 .recharts-area layers (name binds); legend text CostRevenueRevenue (previous period) and the raw key revenue_prev absent (label reaches the legend and wins over the key); the comparison mark's area-curve and area-area paths carry stroke-dasharray="8 4", stroke-opacity="0.6", fill-opacity="0.6"; the only dash attributes on the whole surface are ["8 4","8 4"] — the default 4 4 appears nowhere; both primaries fill-opacity="1", no dash, no stroke-opacity. stack: the cost area's bottom edge (475,180),(53,145) is the revenue area's top edge (53,145),(475,180). Gate control (R6): the identical third series without variant → no dash, no stroke-opacity, fill-opacity="1" — so variant is live, and it is the gate. Every taught key reaches the surface. |
| 11 | The '8 4' choice (⭐) |
RIGHT | Default verified at source :96 (s.dashArray ?? (kind === 'line' || kind === 'area' ? '4 4' : undefined)) and in the DOM: R3 (comparison area, no keys) → stroke-dasharray="4 4", stroke-opacity="0.6", fill-opacity="0.2"; R4 (same series with dashArray: '4 4' written) → the comparison curve's outerHTML is byte-identical to R3's. An example writing '4 4' on a comparison line/area would render indistinguishably from omitting the key — inert in exactly the quieter way the author says. The example's inline comment ('8 4' overrides … '4 4'; 0.6 lifts its 0.2 fill default (its stroke default is already 0.6)) matches R3's three defaults to the digit. |
| 12 | dashArray row (comparison and a line/area mark, chart-level or per-series type; bar/scatter take opacity only) |
RIGHT — complete and correct | Line takes an authored dash (R5: 2 6, stroke-opacity 0.3, no fill-opacity); area takes it (R1); bar drops it — old example verbatim (R2): 3 bars, 0 stroke-dasharray on the surface, 2 rects at fill-opacity 0.6; comparison bar with an authored dash and no opacity (R7): 0 dash, rects at the 0.4 default; scatter (S1, numeric x, one series — the only shape the scatter branch mounts): 3 symbols all fill-opacity="0.6", 0 dash, no stroke-opacity; control without variant: no fill-opacity; per-series type override (S2: chartType: 'bar', comparison series type: 'line', dashArray: '2 6'): 1 bar + 1 line, the line curve carries 2 6 and 0.6 — the condition is the mark, not the chart. |
| 13 | opacity row (every mark family under comparison — fill on bar/scatter, stroke on line, both on area; unused on primary) |
RIGHT | Bar fill (R2/R7), scatter fill (S1), line stroke only (R5: fo null), area both (R1), primary unused (R6, S1-control). |
| 14 | data-display.ts docblocks for opacity / dashArray |
RIGHT | Every cited site checked against the head: comparisonStyle :92-97 with the authored value first at :96; Scatter :1832 and Bar :1911/:2037 pass fillOpacity only; Line :1898/:2048 strokeOpacity+strokeDasharray; Area :1905/:2056 all three. "comparisonStyle returns the authored value for every family, but a Bar or Scatter mark passes fillOpacity only and drops strokeDasharray (and strokeOpacity)" is the mechanism as the source reads. Old phrasings on disk: applied at …:96 0, Same condition as 0, every mark reads 0. |
| 15 | Changeset variant: 'current' (the non-blocking item taken) |
RIGHT — exact | Text: "and, separately, variant: 'current', which the renderer does NOT drop: the normalizer keeps that renderer-internal spelling and draws it exactly as primary, but it is not a member of the published pair, so it now refuses at variant". Source: normalizeChartSchema.ts:247 keeps current; AdvancedChartImpl.tsx:2010 tests only === 'comparison'; the pin refuses at path ['variant']. The "value the renderer drops" class and the honoured-but-refused value are now stated as two classes, which is the distinction round 2 asked for. |
| 16 | .min(0).max(1) declined (the non-blocking item not taken) |
RIGHT — still defensible | Spec 17.2.0: opacity: z.number().min(0).max(1).optional() (dist/ui/index.js ≈2399); mirror z.number(); the row, the describe and the docblock all state the bound and that the mirror does not enforce it. Census on the merged tree of every series-level opacity literal: 0.4 (pin), 0.5 (plugin-report chartChrome test), 0.6 (docs) — the out-of-range class is still empty; main's window added no opacity/dashArray producer (diff grep empty). A free narrowing, not owed by this card; round 2 already ruled it non-blocking; nothing on the current tree changes that. |
| 17 | PR-body sentence "the lockfile moved on main, so pnpm install --offline --frozen-lockfile was re-run" |
WRONG (record), non-blocking | pnpm-lock.yaml blob df8fbcd77 at 6eebc54b6, bc640ec56, 493b52eb3, 51f2a9634, 5f7e032a8; git log 6eebc54b6..bc640ec56 -- pnpm-lock.yaml is empty (lit control: registry.ts moved once in the same window). The last lockfile commit on main is 2f61238b9 (2026-09-04T17:10Z, #7670), which predates the PR's original base a472b0716 (git diff a472b0716 origin/main -- pnpm-lock.yaml empty). The lockfile has not moved at any point in this PR's life; the re-install was a no-op. No effect on the tree; a false process statement in the PR body and in dev report 5550321911, and the dispatcher's brief repeated it. |
The disclosed control miss (item 7 of the brief), judged. My unstacked control (R1b) puts revenue's top edge at (53,5),(475,75); stacked (R1) it is (53,145),(475,180) — the revenue path does move under stacking, because the y-domain rescales from max 400 to 650 (revenue+cost). The author's first prediction was wrong for exactly the reason stated. The recast control is geometric and domain-independent: unstacked, both primaries' bottom edges sit on one baseline (y=285, one value for both) and cost's bottom ≠ revenue's top; stacked, cost's bottom equals revenue's top point-for-point. That distinguishes the two states without assuming any path is fixed, so it is sound, and the stacking reading is real.
The disclosed instrument fault (item 8), judged. Scratch tree at the head, git status empty, on-disk blobs equal to the commit blobs (9b356dc97, bf2680f6b, 123b480a0). Anchored counts on disk: Narrower condition than 1, EVERY mark family 1, fillOpacity. only and drops 1, read at \AdvancedChartImpl.tsx:96`1,chartType: 'area'2,dashArray: '8 4'1,dashArray: '4 4'0,only on a mark that has a stroke to dash1,on every mark family1,keeps every key in this example live1,variant: 'current'1,renderer does NOT drop1 — every claimed edit is present, every superseded phrasing is 0. The commit carries exactly the three files its message names; the untracked probe is not in it; the Zod.describe` strings are untouched as claimed (row 1). The PR body, read back, carries the round-3 section. No second instance of the "believed applied, not applied" shape exists in this round. The one false statement found (row 17) has the opposite shape — an action reported as necessary that was not — and it changed no bytes.
One instrument note of my own, reported as such. My first scatter leg mounted nothing: the shape used a categorical x and two series, and the scatter branch requires a numeric x and exactly one series (scatter-multi-series refusal otherwise). Re-run in the mounting shape (S1) with a lit control; the null was mine, not the renderer's.
② Semver grading
- Declared:
'@object-ui/types': minor— frontmatter byte-identical since493b52eb3. Correct level:minor. AGENTS.md §版本号策略: objectui's own breaking changes are shipped asminorwith the breaking semantics written in the body ("objectui 自身的破坏性变更也标minor（在正文里写清 breaking 语义即可）");majoris mechanically refused byscripts/check-changeset-no-major.mjs, and Changeset Bump Policy is green on the head. Same transition, same file, as finding(types/plugin-charts): the staticChartSchemanode's two data keys are declared-but-unread —ChartDataSeries.datais required and never read, andcategoriesis read as a SERIES list, not axis labels #6896 / [Decision]ChartSchema's real data model — chart-leveldataandxAxisKey— is undeclared, and #6896's retirement now points authors at it by name #7113. - Both halves stated: yes. Widening — "the accept set widens toward what already renders"; narrowing — "one document class that validated before now refuses", the refused values enumerated, and now
variant: 'current'named separately as refused-but-honoured (row 15). The FROM→TO block shows the pair and the finiteopacitywith the unenforced spec bound. Nothing in ② moved this round except the precision gap round 2 recorded, which is closed.
③ Boundary-flag disposition
Every card and PR below was read in this session; state and updated_at are from those reads.
- Q1 —
chartTypeshape. Option A (named alias refusal pointing attype); not this PR's. Carrier finding(types): serieschartTypeis undeclared and SILENTLY STRIPPED - the normalizer reads it first, measured not live, alias-refusal owed #7694 open (pm:queue,finding,priority:p3,domain:ui; updated 02:51:20Z), named in the PR's out-of-scope list. Not sunk byFixes #7546. - Q2 —
variantunion. Narrowed to the spec pair at493b52eb3, unchanged since (rows 1–3). The normalizer's tolerance forcurrentis untouched and is finding(plugin-charts/core/types): seriesvariantcarries three different unions across its declarations and readers —currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops #7682's (open,finding,pm:queue, p3; updated 02:50:16Z). - Q3 — axis-config object dialect. finding(types): the ChartSchema axis-config object dialect is undeclared and passes through unchecked - liveness read owed before declare-or-retire #7690 open (
package: types,pm:queue,finding, p3; updated 02:28:08Z), filed from the card's own body. - Q4 —
opacity/dashArrayconditional in the renderer. Declared unconditionally (the spec's posture), the condition stated in docs, the renderer gap carried by finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698 open (plugin: charts,pm:queue,finding, p3; PM triage 07:05:06Z: "Do not open this as narrow the mirror"). Round 2's non-blocking (c) — correct finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698's "all five call sites apply all three" — is done: PM comment 5550238143 (07:18:29Z) states the Bar/ScatterfillOpacity-only split and the??mechanism; it matches my source reading and DOM readings exactly. The triage comment also already instructs finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698's implementer to re-read fix(types): ChartDataSeriesSchema declares the six series keys the renderer reads (objectui#7546) #7684's landed state and remove the interim docs wording in the same change. - bug(plugin-charts): a declared per-series
typeoverride is DROPPED when every series entry is written withdataKey— the internal-shape fast path forwards the array raw and the renderer reads onlychartType#7681 open (bug,pm:queue,finding,priority:p2; updated 02:50:48Z) — the per-seriestypedropped on thedataKeyfast path; unrelated to this round's change, still carried. - Round-2 non-blocking (a) — taken, exact (row 15). (b)
.min(0).max(1)— declined, defensible (row 16). It is deliberately carrier-less: it is not a measured defect (no document renders wrong; the mirror is looser than the spec by the "read's own domain" rule this PR applies to every key), and the fact is recorded durably in the docblock, both doc rows and the.describe. If the maintainer wants mirror/spec range parity, it is a one-line card of its own — the PM's call, not a condition here. - The control misses. Round 2's census zero was a reading (upheld there); this round's stacking prediction was a wrong prediction corrected by a sound geometric control (above). Neither is a dark instrument.
Fixes #7546closes the card — measured-and-lost check: six keys stripped → this PR;chartType→ finding(types): serieschartTypeis undeclared and SILENTLY STRIPPED - the normalizer reads it first, measured not live, alias-refusal owed #7694; axis-config dialect → finding(types): the ChartSchema axis-config object dialect is undeclared and passes through unchecked - liveness read owed before declare-or-retire #7690;variantdrift /current→ finding(plugin-charts/core/types): seriesvariantcarries three different unions across its declarations and readers —currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops #7682; per-seriestypeon the fast path → bug(plugin-charts): a declared per-seriestypeoverride is DROPPED when every series entry is written withdataKey— the internal-shape fast path forwards the array raw and the renderer reads onlychartType#7681;opacity/dashArraycomparison-only plus the bar/scatter split → finding(plugin-charts): series-levelopacityanddashArrayare honoured only on avariant: 'comparison'series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698 and its correction comment. Every measured finding has an open carrier. The card and the PR both carryneeds:contract-review; the PR is draft; not enqueued.- Tense corrections to the record as it reached me. fix(components,plugin-grid,app-shell): one authority for ComboboxOption, NamedActionDef and OrgTranslate (objectui#6349 batch 3) #7691, docs(plugin-charts, plugin-form, plugin-list, app-shell): census of the object-schema snake lookup reads — all seven legs keep, with the evidence recorded at each site #7649, fix(types): SchemaRegistry's kanban entry stops describing a component it cannot name #7662 are merged (06:57:50Z, 07:13:38Z, 07:14:56Z) and are the three first-parent commits in the merge window — the PR body names exactly those three, correctly. fix(types): declare the four alert-dialog keys the renderer reads (objectui#7104) #7700 was open when this review started and merged at 07:40:44Z as
8ad218d58, after the merge commit (07:22:39Z) and the head (07:29:47Z); it is not in the merged tree and the PR body does not claim it is. The brief's "fix(types): declare the four alert-dialog keys the renderer reads (objectui#7104) #7700 has been landing throughout" is therefore right only as of 07:40Z; the PR is now one commit behind, clean, and green on the dry-run merged tree (row 8). The brief's "the lockfile moved on main" is false (row 17).
VERDICT: PASS
Recorded, not blocking — no commit needed for either:
- PR body, one sentence (round-3 remediation, last paragraph): "the lockfile moved on
main, sopnpm install --offline --frozen-lockfilewas re-run in the worktree" is false —pnpm-lock.yamlis blobdf8fbcd77at every commit in this PR's history and last moved on main at2f61238b9(feat(scripts,components): gate unreferenced source files, and delete the one standing orphan #7670), before the PR's original base. Strike or correct it when the dispatcher flips the PR ready, so the record does not teach a lockfile movement that did not happen. Dev report 5550321911 carries the same sentence and cannot be edited; this report is its correction. - finding(types): ChartDataSeriesSchema SILENTLY STRIPS six series keys the renderer reads — label, variant, opacity, dashArray, stack, yAxis #7546's and the PR's
needs:contract-reviewlabels are the dispatcher's to clear on adoption; the PR is one commit behindmain(fix(types): declare the four alert-dialog keys the renderer reads (objectui#7104) #7700) with a clean dry-run merge and a green types suite on the merged tree — no re-merge owed before enqueue.
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 #7546
What
ChartDataSeriesSchemais a non-strictz.object— not.passthrough()likeBaseSchema— so an undeclared key is stripped in silence whilesafeParsereports success. Six keys thatnormalizeSeriesreads (normalizeChartSchema.ts:242-255) andAdvancedChartImpldoes real work with were undeclared:label(legend name,:1364),variant(comparison overlay,:2010-2033),opacity/dashArray(stroke and fill,:94-96),stack(stackId,:1893),yAxis(axis binding,:1887). Both faces — the TS twinChartDataSeriesand the Zod mirror — now declare them, each with its own measured value domain. No reader changed. The object stays non-strict; a truly undeclared key is still stripped, aschart-inline-data-retired.test.tspins.chartType, the seventh key the prior review found, is measured not live on this authoring face and is deliberately not declared — reported below with its evidence, per the ruling (comment 5548523375: declare what is live, report the rest, retire nothing here). Its carrier is objectui#7694 (see "Out of scope").Remediation after the second contract review (comment 5550238043, REFUSE) — head
5f7e032a8The one blocking item: the "Per-series options" example taught
dashArrayon achartType: 'bar'chart, where it reaches nothing. Re-measured before rewriting — neither the verdict's reading (comparisonStylesetsstrokeDasharraytoundefinedfor bar) nor the dispatcher's taken on trust — through the realChartRendererunder happy-dom (Recharts'ResponsiveContainersized as inChartRenderer.specSeries.test.tsx), on this branch withmainbc640ec56merged. Predictions stated first; a lit control on every probe; readings persisted to disk because vitest hides passing tests' stdout.bar, comparison{ dashArray: '4 4', opacity: 0.6 }fill-opacity="0.6"on the comparison rects (lit), zerostroke-dasharrayrecharts-rectanglepaths at0.6;stroke-dasharrayhits: 0area, comparison{ dashArray: '8 4', opacity: 0.6 }stroke-dasharray="8 4",stroke-opacity="0.6",fill-opacity="0.6"on the comparison mark; primaries untouched8 4on both the comparison'sarea-areaandarea-curvepaths,0.6stroke and fill on the same two; the four primary paths keepfill-opacity="1", no dash, and the default4 4appears nowherearea, two series withstack: 'money'vs withoutcostarea's lower edge IS therevenuearea's upper edge; control: unstacked both sit on the baselinecostpath…L475,180L53,145ZagainstrevenueM53,145L475,180…; unstacked both end on285line, comparison{ dashArray: '2 6', opacity: 0.6 }2 6+stroke-opacity 0.6on the line curve; nofill-opacityfrom itscatter, one comparison series with both keysfill-opacity="0.6"on every symbol, zerostroke-dasharrayrecharts-symbolsat0.6; dash hits 0dashArray; comparison bar with NOopacity4 4/0.5and0.4(instrument sees the attributes)4 4+0.5;0.4opacity: 0.6, dashArray: '2 6'on bar and on linedash: [] fillOp: [] strokeOp: []on bothbarchart, comparison series with per-seriestype: 'line'anddashArray: '2 6'chartType2 6+0.6on the line curve9/9 on the final run. One control miss, reported: P2b's first control predicted the bottom (
revenue) area path unchanged by stacking — wrong, stacking rescales the y-domain so both paths move (M53,5…→M53,145…); the readings themselves already showed the stacking (the lower edge coincidence above), so the control was recast geometrically and the probe re-run: 9/9.The mechanism is one step lower than the verdict puts it (renderer blob
63a5e34c9, identical onmainand here — no reader changed):comparisonStyle(AdvancedChartImpl.tsx:96) returns the AUTHOREDdashArrayfor every family — theundefinedat:96is only the fallback half. What drops it is the mark: Scatter:1832, combo Bar:1911and Bar:2037passfillOpacityonly; Line:1898/:2048passstrokeOpacity+strokeDasharray; Area:1905/:2056pass all three. So the two keys split:opacityis live on every family undercomparison(fill on bar/scatter, stroke on line, both on area);dashArray— andstrokeOpacitywith it — only on a line/area mark.What changed (wording plus one example's
chartType; no contract, reader or changeset-level change):plugin-charts.mdx— the example moves tochartType: 'area', the one family wherestack,variant,dashArrayandopacityare ALL live (P2 + P2b); droppingdashArrayfrom the bar example was the alternative, rejected because the row would then have no live demonstration at all. The dash is'8 4', not'4 4': on a comparison line/area'4 4'IS the default, so an example writing it would be inert in a second, quieter way. A one-line lead-in says whyarea. ThedashArrayrow states the full condition — comparison and a line/area mark (chartchartTypeor per-seriestype), bar/scatter takeopacityonly; theopacityrow now says every mark family, fill vs stroke per family (it neither over- nor understated before, it was silent on the split).data-display.ts— thedashArraydocblock carries the full condition with the mark sites; theopacitydocblock says it reaches every family. The Zod.describestrings are deliberately untouched: on all six keys they state the value domain, not the renderer's current coverage, and the coverage lives in the rows and docblocks.variant: 'current'as refused-but-honoured: the normalizer keeps it and draws it asprimary; it is refused because it is not a member of the published pair..min(0).max(1)for mirror/spec range parity onopacity— the maintainer's follow-up, not this card's.mainbc640ec56merged again (merge commit51f2a9634; #7691, #7649 and #7662 landed after the previous merge) — clean, no conflicts; the lockfile did NOT move — corrected 2026-09-05 by thedomain:uiPM seat after the round-3 contract review measured it:pnpm-lock.yamlis blobdf8fbcd77at6eebc54b6,bc640ec56,493b52eb3and5f7e032a8alike, 0 commits touched it in the merge window (lit control:registry.tsmoved once in the same window), and its last movement onmainwas2f61238b9(#7670), before this PR's original base. The re-install was a no-op in the worktree;git diff --stat origin/main..HEADis still exactly this PR's six files.Remediation after the contract review (comment 5548861339, REFUSE) — head
493b52eb3variantnarrowed to the spec pair on both faces —z.enum(['primary', 'comparison'])andvariant?: 'primary' | 'comparison'. Re-measured before rewriting, onorigin/main6eebc54b6(neither the PM's summary nor the reviewer's numbers taken on trust): the only writers of'current'are the two internal-shape producersObjectChart.tsx:852(s.variant || 'current') andDatasetWidget.tsx:1450(s.variant ?? 'current') plus their fourdataKey-shaped test assertions; docs 0, fixtures 0, designer 0 —git grepovercontent/**,examples/**,apps/**in both the TS and the JSON spelling, controls lit (variant: 'comparison'1 hit incontent/**on this branch; the same query overapps/**finds the button-actionvariant: 'primary'sites). The installed@objectstack/spec@17.2.0declaresvariant: z.enum(["primary", "comparison"])(dist/ui/index.js:2395). Core's own authored-series readerseriesPresentationkeeps the pair and dropscurrent(chart-presentation.ts:125). Socurrentis the renderer's internal default and nothing an author writes — the same reading that excludedchartType— and declaring it would have fossilised a renderer-side tolerance into a second contract (AGENTS.md #0.1). The narrowing refuses zero live producers. Pin test:it.eachover the pair, a new refusal pin forvariant: 'current'at its own path, the type-levelEqpin narrowed. Both doc pages, the TS docblock (the "one wider than the spec" paragraph removed) and the changeset's FROM→TO block and paragraph rewritten. No reader changed — the normalizer's tolerance forcurrent(normalizeChartSchema.ts:247) is untouched; objectui#7682 owns it.chartTypecarrier — objectui#7694 exists (filed by the PM seat; re-read 2026-09-05: open,pm:queue) and is named in the out-of-scope list below, soFixes #7546no longer sinks a measured finding.opacity(the non-blocking item) — took the wording option, not.min(0).max(1): the mirror staysz.number(); the doc row and the.describenow read "any finite number (the spec bounds it to 0–1)" instead of claiming a bound the mirror does not enforce. Why: every domain in this change is the read's own (num()=Number.isFinite), and the renderer never drops an out-of-range opacity (SVG clamps at paint), so.min(0).max(1)would refuse a document class that renders today — a second narrowing nothing in this PR measured; its own card if triage wants it. Census: every series-levelopacityliteral in the repo is within 0–1 (0.4,0.6; the same grep is the control).mainmerged into the branch (merge commit71f0c1941— no rebase, no amend, no force): one conflict,content/docs/api/schema-reference.md, where docs(charts): rewrite the inline-series chart examples into the model the renderer implements #7679 rewrote the chart node's property table (categories/data/xAxisKeyrows) while this branch rewrote theseriesrow. Resolution keeps docs(charts): rewrite the inline-series chart examples into the model the renderer implements #7679's table and folds the declared keys into itsseriesrow. No lockfile or generated file moved onmainsince the merge-base (git diff --stat a472b0716..origin/main -- pnpm-lock.yamlis empty;pnpm installnot re-run). After the mergegit diff --stat origin/main..HEADlists exactly this PR's six files.Found while re-measuring — filed, left to its own card: objectui#7698
The renderer's ONLY read of series-level
opacity/dashArrayis insidecomparisonStyle(AdvancedChartImpl.tsx:92-97), which returnsnullunlessvariant === 'comparison'; every mark reads throughcmp?.…, so on a primary series both keys are read and unused (and, measured in the second remediation above, the marks split further: Bar:1911/:2037and Scatter:1832passfillOpacityonly, sodashArrayis line/area-only even on a comparison series — the PM seat has already posted that correction on #7698 as comment 5550238143; not this PR's change) — while the spec declares them as unconditional overrides. The declarations stay (spec members, real work on comparison series). My own doc example had taughtopacity: 0.6on a plain stacked series, a no-op: the example now puts it on the comparison series, both rows say so, and the TS docblocks carry the condition. The renderer gap is objectui#7698's (duplicate check: 119 open issues read via the repo-scoped list endpoint — every open issue from #7380 up plus everyfinding-labelled one — control #7694 present, no prior card).Red first, on the untouched base
a472b071The pin file
packages/types/src/__tests__/chart-series-keys-7546.test.tswas written and run before any schema edit. Predicted 14 red / 13 green; observedTests 14 failed | 13 passed (27). The reproduction assertion is the card's measurement verbatim:expected { name: 'Revenue' } to deeply equal { name: 'Revenue', …(6) }onsuccess: true. The lit control — declaredname/type/colorsurviving the same query — was green in that run. After the change:Tests 27 passed (27).Per-key measurement (the ruling's three axes, lit controls on every count)
Producer census = key counts INSIDE
seriesarray literals, by file class, with the declared keys measured by the identical query. Controls lit: docsname6 /dataKey10 /color2; fixturesname2 /dataKey6 /color2.type: 'chart'nodeChartSeriesSchemalabelDashboardRenderer.tsx:648-652,ObjectView.tsx,ListView.tsx,plugin-view/ObjectView.tsx(4 sites)label()string or locale map; legend nameI18nLabelSchemastring | I18nLabelvariantObjectChart.tsx:852,856,DatasetWidget.tsx:1444,1450— internal-shape callers that never meet this mirrorcomparisonselects the overlayprimary | comparisonprimary | comparison— the spec pair;currentis the renderer's internal default, written by nothing an author writes (objectui#7682 owns the normalizer's tolerance)opacitycore/utils/chart-presentation.ts:131(dataset path)num(); stroke/fill opacity — today only on acomparisonseries (objectui#7698)number0–1number(the spec's bound stated in docs, not enforced — remediation item 3)dashArraychart-presentation.ts:130str();strokeDasharray— same condition (objectui#7698)stringstackchart-presentation.ts:128str(); RechartsstackIdstringyAxischart-presentation.ts:124,DatasetWidget.tsx:1446left | right; axis bindingleft | rightchartType?? type, narrowed to 3aliasesas a spelling oftype, refused by name (chart.zod.ts:231)Designer: the
chartregistration'sseriesinput is a singlecode(JSON) field with no per-key inputs, anddefaultProps.seriesauthorsdataKeyonly (plugin-charts/src/index.tsx). The six are the spec's canonical members under the same names; this node'sseriesaccepts the spec shape by design (ChartRenderer.tsx:55-66, "both shapes"). Declaring them widens the accept set only toward what already renders.chartType— the seventh, measured and reportedChartRenderer, which forwards an all-dataKeyarray raw (:126-131), so the normalizer's read never carries their value.type, and this package's ownChartDataSeries.typedocblock (objectui#6121) callstypethe author spelling andchartTypethe internal one.normalizeChartSchema.ts:244, restored under trap, blob05a66dfbmoved tobd9e640fand back): withstr(raw.chartType) ??deleted,packages/plugin-charts/plus the four dashboard/report series tests read49 files / 470 tests passed— predicted 0 red, observed 0 red. Control with the sibling?? str(raw.type)deleted instead: predicted at least 2 red in 2 files, observed2 failedinnormalizeChartSchema.test.tsandChartRenderer.specSeries.test.tsx. The review re-measured the same legs on 197 files / 2380 tests with the same outcome.The shape it should take — a named alias refusal pointing at
type, graded as its own narrowing — is objectui#7694's decision. The pin test holds the gap visible and the TS face carries ats-expect-errorratchet so it cannot be added by hand without re-reading this.Clause-② — my own determination: yes
The accept set moves both ways on a published mirror: six keys now survive with their values (widening toward what renders), and a series carrying one of them with a value the renderer silently drops —
variant: 'bogus'or the renderer-internalvariant: 'current',yAxis: 'top',opacity: '0.4'orInfinity, a non-stringstack/dashArray, a non-string non-maplabel— now refuses at its own path. Hence the changeset isminor, the level objectui#6896 and objectui#7113 set for the same transition in this file; the grade did not move with the narrowing. Draft; not flipped ready; not enqueued;needs:contract-reviewrides on both carriers (re-read 2026-09-05: the card and this PR both carry it).Ablation A — the six Zod declarations deleted (measured on
593be2ee9; restored under trap, absolute paths)Mutation proven on disk: anchored counts
yAxis: z.enum([left,right])0 andlabel: I18nLabelSchema0; blobf6769b2cmoved todb9ef916. Restore proven: blob equals the HEAD blob andgit diff HEADon the path is empty.14 failed | 13 passed. Matched.zod-mirror-parityunder vitest: predicted red — observed12 passed (12). A miss. The instrument was wrong, not the ratchet: the declared-but-unmirrored half (UnmirroredDeclaredKeys) is a compile-time assertion, and vitest does not typecheck. Re-run as leg A2 withtsc -p tsconfig.test.jsonunder the same mutation (blob movement re-proven): predicted red naming the pair and the six keys — observed exit 2 withzod-mirror-parity.test.ts(1478,14): error TS2322: Type '"data-display.zod.ts#ChartDataSeriesSchema"' is not assignable to type 'never'pluschart-series-keys-7546.test.ts(163,38): TS2339: Property 'label' does not exist. Matched on instrument and pair; partial miss on wording — the error names the pair, not the keys. The review confirmed the channel is live in CI (@object-ui/types:type-check, cache miss, job success).Reverse verification B — the narrowing reverted (on
493b52eb3; restored under trap, absolute paths)Mutation:
git checkout 71f0c1941 -- packages/types/src/zod/data-display.zod.ts(the merge commit's three-value enum), proven on disk: blobf5c89fe4moved tof6769b2c; anchored counts'comparison', 'current'0 to 1 andz.enum(['primary', 'comparison'])1 to 0. Predicted exactly 1 red / 26 green, the red being the newcurrentrefusal pin — observedTests 1 failed | 26 passed (27), that test. Restore proven:git hash-objectequals the HEAD blobf5c89fe4,git diff HEADempty,git statusempty. Nodist/is involved — the pin imports../zod/data-display.zodfrom source — so there is no rebuild leg to skip.Gates on the pushed head
5f7e032a8(after the final commit)Through
os-verify-lock.sh(slotobjectui-7546), joined with&&, the head echoed inside the hold (union-head=5f7e032a8) — verdict lineos-verify-lock: VERDICT command-exit 0(shared-box seconds):pnpm exec vitest run packages/types/ --maxWorkers=2—Test Files 107 passed (107),Tests 1769 passed (1769)(was 106 / 1768 on493b52eb3; the one new file ismain's fix(types): SchemaRegistry's kanban entry stops describing a component it cannot name #7662 pinschema-registry-kanban-honesty-7645.test.ts)pnpm --filter @object-ui/types type-check(three tsc projects, the tests included) — 0 errorspnpm --filter @object-ui/types lint— 0 errors, 269 pre-existingno-explicit-anywarningsOutside the lock, exit codes captured before any pipe, on the same head:
check:doc-fences✅ (227 documents),check:doc-types✅ (188 doc files, 895typeliterals, every documented type registered),check:control-bytes✅ (6271 tracked files, plus a direct control-byte grep of the three edited files = 0),check-changeset-presence✅ ("3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)"),check-changeset-no-major✅,check-changeset-overwrite✅ ("1 changeset(s) added, 0 modified" — the edited changeset is this branch's own).The DOM probe itself (9 tests, one untracked file under
packages/plugin-charts/src/, removed before the commit) ran through the same lock:Tests 9 passed (9), verdictcommand-exit 0; it is an instrument, not a pin — pinning "dashArray is dropped on a comparison bar" would fossilise the very gap #7698 exists to close.NOT MEASURED locally:
check:doc-snippets— the same declared narrowing as before (it needs all 26 packages built): populationcontent/docsplus the package READMEs; my one edited fence compiles standalone (tsc --noEmit --strict --ignoreConfig, exit 0); CI'sDoc Snippet Type Checkis the authority and was green on593be2ee9. Dependency build: not owed —pnpm --filter '@object-ui/types^...' buildanswersERR_PNPM_RECURSIVE_RUN_NO_SCRIPT(the only workspace dependency,@object-ui/test-support, has no build script); the package's runtime dependencies are@objectstack/specandzodfromnode_modules. Consumer sweep: no structural consumer ofChartSchema/ChartDataSeriesexists outsidepackages/types(ChartRendererPropstypes its schema inline), so no downstream type-check is owed.Docs
content/docs/api/schema-reference.md— theseriesrow, now inside #7679's rewritten chart table (which teaches the rows model and thedatatombstone), lists the declared keys withvariantas the pair.content/docs/plugins/plugin-charts.mdx— the "Per-series options" table and example:variantis the pair,opacitysays any finite number with the spec's bound and that it reaches every mark family on a comparison series, thedashArrayrow states the full condition (comparison AND a line/area mark), and the example is anareachart on which every key it writes is live (second remediation).Out of scope — filed, not fixed here
typeoverride is dropped when every entry is written withdataKey: the fast path forwards the array raw and the renderer reads onlychartType. Open (pm:queue).variantcarries three different unions across four sites;currentis an undocumented synonym ofprimarythat core'sseriesPresentationdrops. Open (finding,pm:queue); it owns the normalizer's tolerance this PR leaves untouched.xAxis/yAxisobjects) rides throughChartSchemaunvalidated; filed by the PM seat from the card's own body. Open (pm:queue).chartTypeis undeclared and silently stripped; the carrier for the named-alias-refusal shape, graded as its own narrowing. Open (pm:queue).opacity/dashArrayare honoured only on avariant: 'comparison'series while the spec declares them unconditional; filed from the first remediation. Open (pm:queue, p3, triaged 07:05Z with "do not open this as narrow the mirror").Dev seat: Claude Code session
session_01KbJQ1y1J12nZxYzFWhP8Q3, dispatched by thedomain:uiPM seat; contract-review remediation by the same seat.🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code