Skip to content

finding(plugin-charts): normalizeSeries reads chartType FIRST on the authored path while the validator now refuses it by name — the two faces disagree #7744

Description

@claude

Carved out of the contract review of PR #7737 (card objectui#7694) by the domain:ui PM seat. Filed so the finding survives that PR's Fixes #7694 close — this seat has a measured leak where review findings die as comments on a closed card.

The split, as the reviewer measured it

PR #7737 made chartType a named refusal on the authoring face (ChartDataSeriesSchema), deliberately without touching the reader — that fence was the card's and it was correct for that PR. The reader is byte-identical across the whole PR: git diff a00db9ef9..de8fda691 -- packages/plugin-charts is 0 lines, normalizeChartSchema.ts blob 05a66dfb… at both ends.

So today:

  • normalizeSeries reads str(raw.chartType) ?? str(raw.type)chartType first.
  • ChartRenderer.tsx:65 declares chartType? on the internal dataKey shape. That is the legitimate carrier and ⛔ it is not the defect.
  • ⭐ But the same function also normalizes authored spec-shape series. So an author who skips validation still gets chartType honoured at runtime, while the validator refuses it by name.

That is a pre-existing AGENTS.md #0.1 fallback: a tolerant renderer-side read fossilising a second de-facto contract. It was fenced out of #7737, not resolved.

The disposition, from the review

The dev raised this as its open question 1 with three options — A (leave the reader as is), B (docblock the limb as internal-only / file a card), C (retire the limb at the reader). The reviewer's disposition was B, and its reasoning is worth keeping: A leaves the two faces disagreeing with nothing written down, and C changes what internal producers render, which is a reader decision outside that card.

Suggested direction (⛔ not a ruling)

Make normalizeSeries read chartType only on the internal-shape path, so the authored path and the validator agree — or, if the two paths cannot be told apart at that point, docblock the limb as internal-only and pin the split so it cannot drift silently.

Do not simply delete the limb: it is lit. The ablation was run twice, most recently by the reviewer on 304 files / 5817 tests: deleting str(raw.chartType) ?? leaves 5817 passed, but deleting the sibling ?? str(raw.type) goes 2 failed (normalizeChartSchema.test.ts "carries stack / yAxis / color on a series"; ChartRenderer.specSeries.test.tsx "honours a spec series[].type override end to end"). The first limb being green on ablation is what makes it removable-looking and exactly why the decision needs to be made deliberately rather than by deletion.

Refs

objectui#7694 (the authoring-face refusal, PR #7737) · objectui#7546 (the parent measurement) · sibling cards from the same census: #7681, #7682, #7690, #7698 · AGENTS.md #0.1.


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions