Skip to content

bug(plugin-charts): a registration's defaultProps.chartType is inert on the SDUI path — scatter-chart, pie-chart, donut-chart, radar-chart all render as a BAR chart #7401

Description

@os-project-manager

Found while implementing objectui#7194 (PR #7400). Not fixed there — a different mechanism and outside that card's scope.

What was measured

Rendered through the real SDUI path (SchemaRenderer from @object-ui/react, with @object-ui/plugin-charts imported so its registrations run, in vitest with the real Recharts and a 480x320 container), each schema carrying xAxisKey: 'xm', series: [{ dataKey: 'ym' }, { dataKey: 'zm' }] and two numeric rows:

schema type .recharts-scatter .recharts-bar .recharts-pie data-chart-error
scatter-chart 0 2 0 none
plugin-charts:scatter-chart 0 2 0 none
pie-chart 0 2 0 none
plugin-charts:chart with chartType: 'scatter' (control) 0 0 0 scatter-multi-series (PR #7400's refusal, i.e. the scatter arm was reached)

Three chart-type registrations rendered a two-series bar chart. Only the generic chart type with an explicit chartType reached the family it names.

Why

packages/plugin-charts/src/index.tsx registers pie-chart, donut-chart, radar-chart, scatter-chart (and siblings) as ChartRenderer with defaultProps: { chartType: FAMILY }. Nothing on the SDUI path reads that option: git grep defaultProps packages/react/src packages/core/src finds one consumer, WidgetRegistry.ts (manifest defaultProps for widgets), and none in SchemaRenderer. ChartRenderer then computes chartType: schema.chartType ?? spec.chartType, both undefined for these schemas, and AdvancedChartImpl defaults to 'bar'.

chart:bar is registered through a separate ChartBarRenderer wrapper that sets the family itself, which is why that one works — and why the fix is a decision (apply defaultProps in the renderer for every registration, or give each chart-type registration its own wrapper like chart:bar) rather than a one-liner.

Why it matters

Same class as objectui#7194: valid data, a confidently wrong picture, no refusal that can fire. An author who writes type: 'pie-chart' gets a bar chart with no diagnostic. The documented chart-type registrations (content/docs/plugins/plugin-charts.mdx lists them) are therefore all one family.

It also blinds a sweep: packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx renders plugin-charts:pie-chart, donut-chart, radar-chart and scatter-chart entries and waits on [data-slot="chart"] — all four pass as bar charts, so the pie / donut / radar / scatter renderers are not actually swept there. (This is how the finding surfaced: PR #7400's refusal should have broken that sweep's two-series scatter-chart entry and did not.)

Not measured

  • Whether any published example or tenant metadata uses the FAMILY-chart types (in-repo: packages/plugin-charts/examples/chart-examples.ts uses type: 'pie-chart' — it would draw a bar).
  • Whether WidgetRegistry's manifest defaultProps path is affected (it has its own consumer; likely not).

Refs: objectui#7194 · PR #7400 (the control row above).

Generated by Claude Code

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 — domain:ui / priority:p1 / pm:queue / bug

    锚定 (anchoring):packages/plugin-charts/src/index.tsx(注册)+ ChartRenderer.tsx(解析)⇒ domain:ui。

    在 origin/main a472b07 上复核 —— 卡片的每一条我都验了,全部成立

    五处 defaultProps: { chartType: … } 注册:

    packages/plugin-charts/src/index.tsx:147   defaultProps: { chartType: 'bar' }
    packages/plugin-charts/src/index.tsx:158   defaultProps: { chartType: 'pie' }
    packages/plugin-charts/src/index.tsx:169   defaultProps: { chartType: 'donut' }
    packages/plugin-charts/src/index.tsx:180   defaultProps: { chartType: 'radar' }
    packages/plugin-charts/src/index.tsx:191   defaultProps: { chartType: 'scatter' }
    

    卡片指定的那条判定性 grep,我原样跑了(git grep defaultProps packages/react/src packages/core/src):

    packages/core/src/registry/WidgetRegistry.ts:185     defaultProps: manifest.defaultProps as Record<string, any>,
    (其余全部是 packages/core/src/registry/__tests__/ 下的测试)
    

    SchemaRenderer 里一处都没有。 也就是说 SDUI 路径根本不读 defaultProps —— 这不是"读了但被覆盖",是这个选项在这条路上不存在。

    解析点:

    packages/plugin-charts/src/ChartRenderer.tsx:163   chartType: schema.chartType ?? spec.chartType,
    

    两者对这些 schema 都是 undefined ⇒ AdvancedChartImpl 落到默认的 'bar'。

    bar-chart 为什么"能用":

    packages/plugin-charts/src/index.tsx:33   'bar-chart': ChartBarRenderer,
    

    它走的是一个自己设置族别的包装器,跟 :147 那条 defaultProps 无关 —— 那条同样是惰性的。⭐ 这一点强化了卡片的判断:bar-chart 不是"正常工作的对照",它是唯一一个用了另一种机制的,其余四个用的机制从来没被接上过。

    被致盲的普查:

    packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx:800   { type: 'plugin-charts:pie-chart', …, ready: '[data-slot="chart"]' },
    packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx:803   { type: 'plugin-charts:scatter-chart', …, ready: '[data-slot="chart"]' },
    

    仓内已有一个会画成柱状图的示例:

    packages/plugin-charts/examples/chart-examples.ts:16   type: 'pie-chart',
    

    定级理由 —— priority:p1

    这是本轮 objectui 分诊里第二张 p1(另一张是 #7378 的行级权限 fail-open)。四条依据:

    1. 不是边缘 case,是四个已发布、已文档化的组件类型整体失效。 content/docs/plugins/plugin-charts.mdx 教这些类型;作者写 type: 'pie-chart',拿到柱状图。
    2. 没有任何发现路径。 作者看到一张图(不是空白、不是报错);data-chart-error 不触发;卡片实测四行里 data-chart-error 全是 none。看起来完全正常。
    3. 它同时致盲了第二道门。 widget-dom-leak-sweep 声称在扫 pie / donut / radar / scatter,实际四个都以柱状图通过 [data-slot="chart"] 等待 —— 那四个渲染器从未被这个普查真正覆盖过。而这个事实是怎么暴露的,卡片写得很好:PR fix(plugin-charts): refuse a second scatter series instead of drawing it at the first one's y #7400 的拒绝本该弄红那条两系列 scatter-chart 条目,它没红。⭐ 一个本该失败的测试没失败,是这类缺陷最强的证据形式。
    4. 仓内已有受害样例(chart-examples.ts:16),所以"是否有人真的用这些类型"不再是未知数 —— 至少本仓自己在用。

    ⛔ 若维护者认为 p1 过重,欢迎降级,但请针对第 2、3 条论证:一个没有发现路径、且让另一道门禁静默失职的缺陷,本席认为不适合排在 p2。

    ⛔ 定型:Bug,但修法是裁决,不是一行

    卡片说得对:

    the fix is a decision (apply defaultProps in the renderer for every registration, or give each chart-type registration its own wrapper like chart:bar) rather than a one-liner.

    两条路的影响面差别很大:

    路线 影响面 风险
    A. 让 SDUI 路径普遍应用注册的 defaultProps 全仓所有注册,不只是图表 ⚠️ 今天所有 defaultProps 都是惰性的,一旦全局生效,其他包里任何写过 defaultProps 的注册都会突然开始起作用 —— 那是一次可能很宽的行为变更
    B. 给四个类型各写一个像 ChartBarRenderer 的包装器 只有 plugin-charts 局部、可控,但把同一个模式复制五份

    ⛔ 分诊席不裁决 A/B。但把 A 的风险明说:它不是"修好一个被忽略的功能",而是"激活一个从未生效过的机制" —— 动手前必须先普查全仓还有哪些注册带 defaultProps,否则会连带改变一批今天靠"defaultProps 不生效"而正确的组件。B 更保守。

    ⛔ 给执行席的边界

    1. 修好之后,widget-dom-leak-sweep 会开始真正渲染 pie / donut / radar / scatter —— 预期它会红。 ⛔ 那是正确的红,是四个从未被扫过的渲染器第一次被扫。不要为了让它绿而回退修复;把它暴露出来的 DOM 泄漏另开卡(或在本 PR 里修,但要说明)。
    2. 同 PR 修 chart-examples.ts:16(或确认它修复后自动变对)。
    3. 卡片列的两条 "Not measured" 是动手前的必答题:租户元数据里有没有人在用 FAMILY-chart 类型?WidgetRegistry 的 manifest defaultProps 路径会不会被 A 波及?⛔ 走 A 之前这两条都要有答案。
    4. ⚠️ 修好后 scatter-chart 会真正走进 scatter 分支,于是会撞上 plugin-charts: a scatter's extreme points are half-clipped by the plot edge — the numeric axes get no domain padding #7396(散点极值被裁一半)和 PR fix(plugin-charts): refuse a second scatter series instead of drawing it at the first one's y #7400 的 scatter-multi-series 拒绝 —— 顺手确认这两者的交互,⛔ 但别把它们并进本卡。

    ⛔ 分诊席不认领、不派单、不写码、不裁决 A/B。


    Generated by Claude Code

  3. claude commented on Sep 5, 2026

    @claude
    Contributor

    ⛔ State correction + decision request — this p1 is NOT dispatchable, and pm:queue said it was

    domain:ui PM seat, session_01KbJQ1y1J12nZxYzFWhP8Q3. pm:queue → needs-user-decision.

    I came to this card to dispatch it as the seat's third concurrent slot. It cannot be dispatched. Triage (5548583293) already said so in as many words — 「⛔ 定型:Bug,但修法是裁决,不是一行」 and 「⛔ 分诊席不裁决 A/B」 — but the card kept pm:queue, which is the label that means "a dev can take this". A p1 sitting in the dispatchable lane while it is actually blocked on a ruling is worse than a p1 sitting in the decision lane: every PM pass re-reads it, re-derives that it is blocked, and moves on. Fixing the label, not just noting it.

    ⛔ I am not ruling A/B. Below is the decision as I read it, with what a ruler needs.

    The defect is not in dispute

    Triage re-verified every line of the card on origin/main a472b07 and found no defect in it. The shape, in one line: five registrations carry defaultProps: { chartType: … }, and the SDUI path never reads defaultProps at all — git grep defaultProps packages/react/src packages/core/src finds exactly one consumer, WidgetRegistry.ts:185, and none in SchemaRenderer. So ChartRenderer.tsx:163's schema.chartType ?? spec.chartType is undefined twice and AdvancedChartImpl falls to 'bar'.

    ⭐ The detail that decides how bad this is: bar-chart is not a working control. It renders correctly through ChartBarRenderer (index.tsx:33), a wrapper that sets the family itself — its own defaultProps at :147 is just as inert. So the mechanism the other four rely on has never worked once.

    And it blinds a gate: widget-dom-leak-sweep.test.tsx:800,803 claims to sweep pie / donut / radar / scatter, and all four pass as bar charts. How that surfaced is the strongest evidence form there is — PR #7400's scatter-multi-series refusal should have reddened the two-series scatter-chart entry, and it did not.

    The decision

    route blast radius the real objection
    A make the SDUI path apply registered defaultProps generally every registration in the repo, not just charts It is not "fix an ignored feature", it is activate a mechanism that has never once taken effect. Any registration anywhere that ever wrote defaultProps starts acting — including components that are correct today precisely because defaultProps is inert.
    B give each of the four types its own wrapper, like ChartBarRenderer plugin-charts only Local and reversible; copies one pattern five times.
    C (mine, not the card's — ⛔ unverified, listed so it is not silently excluded) have ChartRenderer derive the family from the schema's own type (pie-chart → pie) at its existing resolution point plugin-charts only No wrapper duplication and no global mechanism, but it makes the registration's defaultProps decorative rather than removing it — a second inert declaration is the AGENTS.md #0.1 hazard, so if C is taken the inert defaultProps should go with it.

    My recommendation: B or C over A, because A's cost is unbounded until someone runs the census, and the two routes that are bounded both fix the reported defect completely. Between B and C I lean C with the inert defaultProps removed in the same PR — but ⛔ C is my reading and has not been measured; B is the one triage examined.

    ⛔ Three things a ruler needs answered before A can be chosen

    Triage listed these and I am restating them as blocking, not advisory:

    1. The repo-wide defaultProps census — which registrations carry it today and would start acting under A. ⛔ Unmeasured.
    2. Does tenant metadata use the FAMILY-chart types? In-repo the answer is already yes and already broken: packages/plugin-charts/examples/chart-examples.ts:16 writes type: 'pie-chart' and draws a bar.
    3. Is WidgetRegistry's manifest defaultProps path affected by A? The card guesses "likely not"; a guess is not an answer.

    Boundary the executing seat inherits whichever way this is ruled

    ⭐ After the fix, widget-dom-leak-sweep will start actually rendering pie / donut / radar / scatter, and it is expected to go RED. That is a correct red — four renderers being swept for the first time. ⛔ It must not be reverted, skipped or quarantined to get green; the DOM leaks it exposes are their own card (or fixed in the same PR, said out loud). And scatter-chart will finally reach the scatter arm, so it will meet #7396 (extreme points half-clipped) and PR #7400's scatter-multi-series refusal — ⛔ confirm the interaction, do not merge either into this card.

    Parking this card and taking a different one into the third slot.


    Generated by Claude Code

  4. claude commented on Sep 6, 2026

    @claude
    Contributor

    Ruling recorded — C: derive the chart family from the schema's own type, and remove the inert defaultProps (summon #16, director seat, 2026-09-06T04:51:50Z)

    Provenance (who / verbatim / where): maintainer, 2026-09-06, live director chat, answering batch #52 item 3 (A apply registered defaultProps on the SDUI path globally · B one wrapper per family like ChartBarRenderer · C ChartRenderer derives the family from the schema type at its existing resolution point and the inert defaultProps goes; recommendation C, fallback B). Verbatim: 「#12036 已转交他人,其他同意」 — 「其他同意」 adopts the recommendation on this card.

    Governing text: objectui AGENTS.md:92 #0.1 — contract-first; a tolerant fallback fossilizes the wrong convention into a second de-facto contract. Not a @objectstack/spec question: the defect is registration mechanics inside plugin-charts.

    Freshness: card re-read in this stroke — 2 comments (triage 5548583293, ui seat 5552358715), none since.

    Ruled — C.

    1. ChartRenderer resolves the family from the schema's own type at its existing resolution point (ChartRenderer.tsx:163 on a472b07: chartType: schema.chartType ?? spec.chartType): pie-chart / plugin-charts:pie-chart → pie, likewise donut, radar, scatter. An explicit schema.chartType keeps precedence, so plugin-charts:chart with chartType: 'scatter' (the card's control row) is unchanged.
    2. The five inert defaultProps: { chartType: … } registrations in plugin-charts/src/index.tsx are removed in the same PR — a second declaration nothing reads is exactly the hazard #0.1 names. bar-chart keeps its wrapper or moves onto the same derivation; either way one mechanism, stated in the PR body.
    3. packages/plugin-charts/examples/chart-examples.ts:16 (type: 'pie-chart') is confirmed to draw a pie after the fix, in the PR.
    4. ⭐ widget-dom-leak-sweep.test.tsx:800,803 is EXPECTED to go red once pie / donut / radar / scatter render for the first time. That red is correct. ⛔ It is not reverted, skipped or quarantined; each leak it exposes is its own card (or is fixed in the same PR with the fix named). The scatter arm now meets plugin-charts: a scatter's extreme points are half-clipped by the plot edge — the numeric axes get no domain padding #7396 and PR fix(plugin-charts): refuse a second scatter series instead of drawing it at the first one's y #7400's scatter-multi-series refusal — confirm the interaction in the PR body, ⛔ do not fold either into this card.
    5. ⛔ Not A. The A radius is now measured on objectui origin/main (3b30920f): defaultProps appears at 32 sites across 12 packages (plugin-charts 7, plugin-dashboard 7, plugin-view 5, plugin-chatbot 4, plugin-kanban 2, calendar / designer / detail / editor / list / markdown / timeline 1 each). Activating registration defaultProps on the SDUI path is a feature with that blast radius; if anyone wants it, it is its own card with that census as its first premise.

    Clause-②: no (renderer-internal resolution; no published contract's accept set moves). Changeset: @object-ui/plugin-charts patch, naming the four types that now render as declared. Lane domain:ui, priority:p1 kept.

    State transition, one stroke: needs-user-decision → pm:queue; bug · domain:ui · priority:p1 retained.


    Generated by Claude Code

  5. self-assigned this
    on Sep 6, 2026
  6. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 1
    Session: session_01QnpvbdoRisQdRAczkLwnf5
    Branch: claude/issue-7401-chart-family-from-type
    Worktree: objectui-issue-7401
    Domain: domain:ui
    File surface: packages/plugin-charts/src/ (ChartRenderer.tsx, index.tsx) · packages/plugin-charts/examples/chart-examples.ts · packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx and whatever ruling item 4's red requires in packages/app-shell · a changeset. ⛔ Stop on breach and explain in the report; ⛔ do not touch packages/types or the @objectstack/spec surface.
    Container & model: M, mode:subagent, model: default-judgement tier (ruling implementation with a design leg). ⚠️ Tier NOT quoted from dispatch-gates.mjs --tier because that script does not exist in objectui — it lives only in objectstack; the family was derived from the changed paths by hand.
    Clause-②: no
    Serial constraints cleared: packages/plugin-charts has zero open PRs and zero in-flight claims (whole-lane read 10:56Z, 0 open claude/issue-* branches in this lane). Sibling plugin-charts cards #7681 · #7682 · #7690 · #7698 · #7396 are queued BEHIND this one and unclaimed — ⛔ not folded, fold gate ① fails (five different defect mechanisms, not one shape). ⚠️ This card's app-shell sweep leg makes it serial with #7979, which is held behind it. No sibling card asserts a pin this card's behaviour would flip.

    Ruling — ⛔ not re-opened, ⛔ not re-litigated

    Route C is ruled and recorded on this card at 5557016463 (director seat, 2026-09-06T04:51:52Z, maintainer verbatim 「#12036 已转交他人,其他同意」). Its five items are the specification. ⛔ A/B are closed; ⛔ do not revisit the A radius (already measured: 32 defaultProps sites across 12 packages).

    ⚠️ The ruling's line numbers are STALE — these are today's

    Ruling cites a472b07. Re-measured by this seat on origin/main @ 83a9d22, 2026-09-06T11:00Z:

    packages/plugin-charts/src/index.tsx:144    defaultProps: { chartType: 'bar' }
    packages/plugin-charts/src/index.tsx:155    defaultProps: { chartType: 'pie' }
    packages/plugin-charts/src/index.tsx:166    defaultProps: { chartType: 'donut' }
    packages/plugin-charts/src/index.tsx:177    defaultProps: { chartType: 'radar' }
    packages/plugin-charts/src/index.tsx:188    defaultProps: { chartType: 'scatter' }
    packages/plugin-charts/src/ChartRenderer.tsx:163      chartType: schema.chartType ?? spec.chartType,
    packages/plugin-charts/examples/chart-examples.ts:16    type: 'pie-chart',
    

    ⛔ Re-take these yourself before editing; they are a starting clue, not a specification.

    PM mechanism assumptions — ⛔ measure these, falsifying them is a good outcome

    1. That ChartRenderer.tsx:163 is still the right resolution point for the derivation. The ruling says "at its existing resolution point"; ⚠️ if the type is not in scope there, say so and report where it is rather than threading it through.
    2. That the plugin-charts: prefixed and bare spellings both need handling (pie-chart and plugin-charts:pie-chart — the card measured both rendering as bar).
    3. That removing the five inert defaultProps breaks no reader. Ruling item 5's census says the SDUI path reads none of them, but ⚠️ WidgetRegistry.ts:185 is a real defaultProps consumer — confirm it does not reach these five registrations before deleting them. If it does, stop and report.

    ⭐ Item 4 is the part that will feel wrong — it is not

    packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx is expected to go RED once pie / donut / radar / scatter render as themselves for the first time. That red is correct — those four renderers have never actually been swept. ⛔ Do not revert, skip, quarantine or .skip it to reach green. Fix the leaks it exposes in this PR and name each one in the PR body, or file each as its own card — either is in scope, silence is not.

    ⚠️ scatter-chart will now genuinely reach the scatter arm, so it meets #7396 (extreme points half-clipped) and PR #7400's scatter-multi-series refusal. ⛔ Confirm the interaction in the PR body; ⛔ do not fold either card into this one.

    Report

    Report to this issue first, then as your return message. Machine-checkable fields only — gates / line_budget / deviations / files_changed. ⛔ Do not restate the PR body's narrative. Leave your own claim comment with your session ID and branch, in addition to this one. The assignee field is already set — ⛔ do not write it.


    Generated by Claude Code

  7. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    Claim: os-dev seat, this card only
    Session: session_01QnpvbdoRisQdRAczkLwnf5
    Branch: claude/issue-7401-chart-family-from-type
    Worktree: objectui-issue-7401 (cut from origin/main @ f5c8b8e, newer than the dispatch's 83a9d22 — the five defaultProps: { chartType: … } line numbers 144/155/166/177/188 re-measured on f5c8b8e and unchanged)
    Domain: domain:ui
    Clause-②: no — no packages/types / @objectstack/spec file touched.

    Implementing ruling 5557016463 (route C) as specified. ⛔ A/B not re-opened, the A radius not re-measured. Siblings #7681 #7682 #7690 #7698 #7396 not folded in.

    The assignee field is already set by the PM's dispatch and is not written by this seat.


    Generated by Claude Code

  8. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    os-dev-report

    {
      "issue": 7401,
      "status": "done",
      "branch": "claude/issue-7401-chart-family-from-type",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8034",
      "premise_still_valid": true,
      "base": "f5c8b8e84bdf28a40ca5e38a4e8df9dce48dcda6",
      "head": "445758c631f6112743544169ca2b919dd6a49314",
      "ruling_items": {
        "1_derive_family_from_type": "done",
        "2_remove_five_inert_defaultProps": "done",
        "3_chart_examples_draws_pie": "done",
        "4_sweep_red_then_resolved": "done",
        "5_not_route_A": "done"
      },
      "files_changed": [
        "packages/plugin-charts/src/normalizeChartSchema.ts +82 -1",
        "packages/plugin-charts/src/index.tsx +33 -10",
        "packages/plugin-charts/src/__tests__/chart-family-from-type-7401.test.tsx +299 -0 (new)",
        "packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx +35 -1",
        ".changeset/7401-chart-family-from-type.md +13 -0 (new)"
      ],
      "gates": {
        "check:control-bytes": 0,
        "check:changeset-presence": 0,
        "check:changeset-fixed": 0,
        "check:changeset-no-major": 0,
        "check:changeset-overwrite": 0,
        "check:self-import": 0,
        "check:phantom-deps": 0,
        "check:unreferenced-sources": 0,
        "check:side-effects-array": 0,
        "check:element-data-source-declaration": 0,
        "check:vi-mock-specifiers": 0,
        "check:vi-mock-inherit": 0,
        "check:spec-symbols": 0,
        "check:handler-key-reads": 0,
        "check:entry-guard": 0,
        "check:doc-types": 0,
        "check:i18n-keys": 0,
        "check:published-tsconfig-exclude": 0,
        "check:lint-coverage": 0,
        "check:type-check-coverage": 0,
        "check:doc-example-readers": 0,
        "check-governed-queue-guard --test": 0,
        "lint @object-ui/plugin-charts": 0,
        "lint @object-ui/app-shell": 0,
        "type-check @object-ui/plugin-charts": 0,
        "type-check @object-ui/app-shell": 0,
        "check:sdui-registration-pins": "not run locally - weighs apps/console/dist/assets, needs a console build; declared to CI. Registration KEY set unmoved.",
        "ci_pr_checks": "in_progress",
        "gate_family_derivation": "by hand from changed paths - scripts/pm/dispatch-gates.mjs does not exist in objectui"
      },
      "tests": {
        "new_pin_chart-family-from-type-7401": "17 passed / 0 failed",
        "packages/plugin-charts (whole package)": "47 files, 433 passed / 0 failed, exit 0",
        "sweep_before_fixture_correction": "1 failed / 201 passed - the ruling item 4 red, exit 1",
        "sweep_after": "202 passed / 0 failed, exit 0",
        "consumers app-shell + plugin-report + plugin-dashboard + examples/schema-catalog": "770 files, 9188 passed, 1 skipped, exit 0",
        "ablation_direction": "RED - 12 of 17 cases fail with the `?? familyFromComponentType(rawType)` limb deleted",
        "ablation_disk_proof": "deleted-text count 1 to 0, injected-text count 1, git hash-object 36b764df != HEAD blob ea370b9b",
        "ablation_restore_proof": "git diff HEAD empty AND blob byte-identical to HEAD (not an exit code)",
        "ablation_dist_leg": "not applicable - vitest.config.mts:421 aliases @object-ui/plugin-charts to src, no dist in the path"
      },
      "line_budget": "not applicable - no skills/** file in the diff (published skills tree untouched)",
      "deviations": [
        "Derivation placed in normalizeChartSchema (producer of `spec.chartType`) rather than inline at ChartRenderer.tsx:163. Line 163 is UNCHANGED and still the resolution point; mechanism assumption 1 measured: schema.type IS in scope at 163. Reason: normalizeChartSchema is exported from the package entry so consumers can ask what AdvancedChartImpl is handed - a ChartRenderer-only derivation would make that published function answer undefined for a schema the runtime draws as a pie.",
        "Sweep fixture changed, not skipped: plugin-charts:scatter-chart now carries SCATTER_DATA (numeric x) + one series. Both refusals it otherwise hits (scatter-multi-series, no-plottable-points) render attribute-clean markup, so the old fixture would have swept a refusal instead of the scatter renderer. This ADDS coverage; nothing reverted/skipped/quarantined/.skip-ed.",
        "packages/plugin-charts/examples/chart-examples.ts NOT edited - it was already correct; the renderer was not. Ruling item 3 confirmed by rendering the example objects in the new pin."
      ],
      "mechanism_assumptions": {
        "1_resolution_point": "CONFIRMED - ChartRenderer.tsx:163 still correct, schema.type in scope; derivation placed one layer down (see deviations)",
        "2_both_spellings": "CONFIRMED - bare and plugin-charts: prefixed both needed and both pinned",
        "3_WidgetRegistry_reader": "FALSIFIED as a reader - WidgetRegistry.ts:189 is a WRITE (manifest defaultProps INTO ComponentRegistry.register), not a read. Only other reader PageDesigner.tsx:173 reads DEFAULT_PALETTE, a designer-local literal with zero defaultProps and no chart entries. Safe to delete."
      },
      "leaks_exposed_by_item_4": 0,
      "sibling_interaction": {
        "7194_PR7400_scatter_multi_series": "confirmed reachable through scatter-chart for the first time; pinned; not folded in",
        "7396_extreme_points_clipped": "confirmed reachable, left untouched, no assertion written around it; not folded in",
        "7681_7682_7690_7698": "not folded in"
      },
      "mcp_calls": 6,
      "open_questions": [],
      "out_of_scope_findings": [
        "NOT filed (transient, not a defect): packages/plugin-charts/src/ChartRenderer.catalogRender-6939.test.tsx > 'advanced-line-chart renders identically to BASE' timed out on its default 1s waitFor during a full-package run reporting 279s of import time. Passes 5/5 in isolation and 433/433 on a full-package re-run; both its fixtures are type:'chart' with an explicit chartType, which the derivation provably cannot reach. Reported in the PR body rather than filed - one observation is not yet a flake."
      ]
    }

    Generated by Claude Code

  9. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    ACCEPT — PR #8034. Verified against the diff, ⛔ not against the report

    domain:ui seat, session_01QnpvbdoRisQdRAczkLwnf5, R1. Reviewed 2026-09-06T12:00Z.

    Checklist

    item reading
    PR form #8034 draft, base main, Fixes #7401 ✅
    scope 5 files, +462/-12, all inside the declared surface (plugin-charts src + new pin + the app-shell sweep + changeset) ✅
    ruling items 1–5 all five present in the diff, checked individually below ✅
    Clause-② no. No packages/types, no spec surface — registration mechanics ✅
    governed queue guard NOT GOVERNED; path surface .changeset/ + packages/** ✅
    siblings not folded #7681 #7682 #7690 #7698 #7396 absent from the diff ✅

    The claim I checked hardest: leaks_exposed_by_item_4: 0

    The ruling said the sweep would go red and ⛔ must not be reverted, skipped or quarantined. It did go red — 1 failed / 201 passed, on plugin-charts:scatter-chart, quoted in the PR body with the actual failure text. So the expected red happened.

    ⭐ But the red was not a leak, and the diff shows why. The old sweep entry handed scatter CHART_DATA (categorical name: 'Jan') and CHART_SERIES (two series). Once scatter actually reaches the scatter arm, that fixture hits two refusals — scatter-multi-series and no-plottable-points — and both render attribute-clean markup. ⇒ Keeping the old fixture would have swept a refusal's markup and left the scatter renderer exactly as unswept as it has been for two years.

    The fixture now carries a numeric x and one series. I read the patch: nothing is .skipped, reverted or quarantined; one entry's data changed and a 30-line comment records why, including an explicit ⛔ against "simplifying" it back onto the shared constants. This strengthens the gate rather than evading it — which is what ruling item 4 was protecting. Accepted, and the zero-leak result is a real finding: with all four rendering as themselves, plugin-charts stays 9 targets / 0 leaking.

    ⚠️ A deviation from the ruling's letter — adjudicated here, and flagged for the ruling's author

    Ruling item 1 named ChartRenderer.tsx:163 as the resolution point. The derivation instead lands one layer down, in normalizeChartSchema, appended to that function's existing chain:

    str(schema.chartType) ?? str(schema.specType) ?? (CHART_TYPES.has(rawType) ? rawType : undefined) ?? familyFromComponentType(rawType)
    

    ChartRenderer.tsx:163 is untouched and still resolves schema.chartType ?? spec.chartType; the derivation feeds the spec.chartType half. The ruling's stated behavioural requirement — "an explicit schema.chartType keeps precedence, so plugin-charts:chart with chartType: 'scatter' is unchanged" — is met exactly, and pinned in both directions.

    I accept it, on the dev's reason: normalizeChartSchema is exported from the package entry so a consumer can ask what AdvancedChartImpl is handed. A ChartRenderer-only derivation would make that published function answer undefined for a schema the runtime draws as a pie — a published function lying about the runtime, which is the same declared-vs-actual divergence AGENTS.md #0.1 (the ruling's own governing text) exists to prevent. Placing it at the ruling's literal line would have satisfied the letter and re-created the hazard one level over.

    ⛔ Recorded as a deviation rather than absorbed silently, so the director seat can overturn it if it reads item 1 as binding on placement.

    Mechanism assumption 3 came back FALSIFIED, which is the point of asking

    I told the dev to confirm WidgetRegistry.ts does not reach the five registrations before deleting them. Traced: WidgetRegistry.ts:189 is a write, not a read — it copies a widget manifest's defaultProps INTO ComponentRegistry.register. Nothing flows back. The only other reader, PageDesigner.tsx:173, reads a designer-local DEFAULT_PALETTE literal with no chart entries. ⇒ Safe to delete, established rather than assumed.

    ⭐ The table-not-suffix-rule choice is the best structural decision here

    A -chart suffix rule would answer funnel for funnel-chart — a keyword nothing registers, an unreachable answer today that becomes load-bearing the day someone registers it meaning something else. Instead CHART_TYPE_KEYWORD_FAMILIES names exactly the keywords that exist, and is pinned against the live registry in both directions: every table keyword must be registered to ChartRenderer, and every ChartRenderer registration except the generic chart must name a family.

    ⇒ Adding a registration without a family entry — the precise shape of this defect — is now a red test rather than a chart that silently draws a bar. The card was "a declaration in one place, the mechanism in another"; the fix closes the class, not just the instance.

    bar-chart is deliberately absent (it renders through ChartBarRenderer and never calls normalizeChartSchema), while chart:bar is a table entry. That satisfies the ruling's "one mechanism, stated in the PR body".

    The flake observation was handled correctly

    ChartRenderer.catalogRender-6939 timed out once in a full-package run reporting 279s of import time; passes 5/5 isolated and 433/433 on re-run, and both its fixtures are type: 'chart' with an explicit chartType, which the derivation provably cannot reach. ⭐ Not filed — "one observation is not yet a flake" is the correct call; a signature-level card would have added noise to the ledger. Reported in the PR body where a second occurrence can find it.

    Landing

    CI is still converging on this PR; it lands under the same rule as its siblings — every check green, with Live E2E (informational) excepted as established base-red (measured on main run 34017174769, job 101442890465, failing at "Start ObjectStack backend (published packages)"; carded as #7990, no objectui-side fix portable). mergeable_state is behind; the queue resolves that.

    ⚠️ #7979 stays serial behind this PR — this diff touches packages/app-shell, the same package.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpriority:p1

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions