Skip to content

fix(lint): chart-config-missing no longer fires on a widget whose binding the renderer derives (#14436) - #15461

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-14436-chart-config-missing-overreach
Sep 4, 2026
Merged

fix(lint): chart-config-missing no longer fires on a widget whose binding the renderer derives (#14436)#15461
baozhoutao merged 2 commits into
mainfrom
claude/issue-14436-chart-config-missing-overreach

Conversation

@claude

@claude claude Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #14436

What the rule claimed, and why it was false

chart-config-missing warned on every chart-family widget that declared no chartConfig:

chart-type widget ('pie') has no chartConfig — the renderer cannot determine which measure to plot, so the series renders empty.

That consequence is false. It is also this platform's own shipped metadata that was tripping it: the system_overview dashboard's two Row 3 tiles (a pie and a bar, one dimension and one measure each), on the Setup board every customer opens first.

Closing the gap triage left: the reading is against the PINNED renderer

Triage read objectui's origin/main and said so. This repo does not consume objectui from npm — the Console is built from the git revision in .objectui-sha, today 00d3f09c500c4a45b5f27aca8af80349412faaf1. Every quotation below was read at that revision, and the derivation is identical to the one triage described:

  • packages/plugin-dashboard/src/DatasetWidget.tsx:1397 calls buildChartSeries(chartRows, dimensions, values, state.fields, …) and destructures { data: chartData, xAxisKey, series }. chartConfig is not an argument.
  • packages/core/src/utils/chart-series.ts:674 — the default branch is xAxisKey: dims[0], series: vals.map((v) => ({ dataKey: v, label: labelOf(v) })).
  • The authored chartConfig reaches the chart only through mergeAuthoredPresentation (per-series/axis PRESENTATION merged ONTO those derived bindings, DatasetWidget.tsx:1405) and chartConfigPresentation (chrome, :1473).
  • The BINDING half is refused outright, pinned by name in packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx at that same revision — same names, same line numbers triage quoted: :179 "ignores an authored axis field and keeps the derived axis binding", :192 "ignores an authored series and keeps one derived series per measure", :204 "ignores chartConfig.type — the widget type owns the chart family", :159 "emits none of the presentation keys when no chartConfig is declared".

So the pinned behaviour is the behaviour triage described, and the exemption is written against it.

Adding a chartConfig could not have repaired a widget with an empty selection either: chart-field-unknown (this same rule file) errors on a yAxis[].field naming anything the widget did not select, so chartConfig can never supply a measure the values array is missing.

The one arm that survives, measured rather than assumed

Triage asked whether a genuinely un-derivable shape exists. One does, and combo is it. A combination chart's identity is a per-series MARK, authored as chartConfig.series[].type — presentation, so it merges forward. mergeAuthoredSeries states the fallback: "a derived series with no authored entry keeps the family default", and DatasetWidget.comboPresentation.test.tsx records the cost: "Without it the line measure drew as the second bar." A combo with no chartConfig draws every measure with one mark, so it is not a combination at all. Warning-tier, because the numbers and the axis are right — the shape is not.

Two shapes are deliberately NOT folded in under this id, because neither is caused by nor repairable with chartConfig, and folding them in would leave the id misnaming its own condition: a chart widget selecting no measures (the pin renders an explicit "Pick measures (values)" placeholder) and one selecting no dimensions (the pin's isMetric covers dimensions.length === 0, so it renders as a KPI number). Both are filed separately.

The change

  • MEASURE_EXEMPT_CHART_TYPES + CHART_TYPES (the taxonomy minus a hand-written exemption list) is replaced by MARK_MIXING_CHART_TYPES = new Set(['combo']). objectui#2945's lesson survives as a test that every member is a declared ChartTypeSchema option — membership is a fact about the RENDERER, which a taxonomy cannot derive.
  • The finding's message and hint now name the mark, not the binding, and say outright that the data and axis are derived.
  • The rule id is unchanged, so an existing suppressWarnings: ['chart-config-missing'] stays valid. Changeset: patch for @objectstack/lint.
  • ⛔ No edit to packages/platform-objects/** — ADR-0072 D1: the rule is repaired, not the metadata.

Fixture pass: the card's repro, before and after

validateWidgetBindings over a stack of @objectstack/platform-objects' exported objects (53) + SystemOverviewDatasets (5) + SystemOverviewDashboard. Run as a reverse verification from the committed state — the pre-fix rule source restored from the merge base f01adfa5c, then restored from HEAD, each leg proved on disk by blob hash:

ON_DISK_AFTER_MUTATION=ecaa6b63594c812a23bef98630d3e8474326c577  (== f01adfa5c blob)
FIXTURE-PASS findings=2
  [chart-config-missing] (warning) dashboards[0].widgets[6]  pie
  [chart-config-missing] (warning) dashboards[0].widgets[7]  bar

ON_DISK_AFTER_RESTORE=48ababf342b5e6fec9938cbb3d5ef3359d62a663  (== HEAD blob)
git diff HEAD --stat: []      git status --porcelain: []
FIXTURE-PASS findings=0

The two widgets are also pinned inline as a fixture in validate-widget-bindings.test.ts that must report nothing.

Verification (all at e9a4a5877; exit codes captured before any pipe)

  • pnpm --filter @objectstack/lint testTest Files 94 passed (94) · Tests 2908 passed (2908), EXIT=0
  • pnpm --filter @objectstack/lint typecheck — EXIT=0; check:test-typecheck: OK — @objectstack/lint's test layer compiles under packages/lint/tsconfig.test.json
  • Derived gate family, node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack → 44 commands, all run: 42 EXIT=0, 2 EXIT=3 (NOT MEASURED, not a red): check:dual-build-cjs-loads and check:type-check-debt both print PREREQUISITE NOT MET / "nothing was measured" because they read built output and this worktree has no whole-repo dist. CI builds first and measures them there.
  • pnpm check:declared-population-live✓ … 203 of 254 famil(ies) … every one of them reaches this tree's 7494 tracked file(s), EXIT=0
  • pnpm check:nul-bytes — EXIT=0
  • pnpm -s check:pm-dispatch-gates✓ dispatch-gates self-test: 1402 cases pass., EXIT=0
  • Whole-repo pnpm lint (eslint . --no-inline-config) — EXIT=0, no narrowing
  • Docs sweep: grep -rn chart-config-missing outside node_modules hits only the two package CHANGELOG.md files (historical release prose, not a contract) and the rule source itself. No docs/qa/platform-checklist item and no content/docs page states the rule's old consequence, so nothing to correct there.

🤖 Generated with Claude Code

https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk


Generated by Claude Code

…ding the renderer derives

`chart-config-missing` warned on every chart-family widget with no `chartConfig`,
claiming the renderer "cannot determine which measure to plot, so the series renders
empty". Read at the `@object-ui` revision this repo pins (`.objectui-sha`), that is
false: `DatasetWidget` derives the x-axis key and one series per measure from the
widget's own `dimensions`/`values` (`buildChartSeries`) and refuses an authored
`ChartAxis.field` / `ChartSeries.name`. `chartConfig` carries presentation only.

The false finding landed on this platform's own shipped `system_overview` tiles.

One true arm survives, under the same rule id: a `combo` widget with no `chartConfig`
takes no per-series mark, so every measure draws with the same default and the chart is
not a combination at all. Its message now names that consequence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
@github-actions github-actions Bot added the size/m label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/lint, touching 4 documentable anchor(s).

2 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via validateWidgetBindings (symbol, a top-level function))
  • content/docs/releases/v17.mdx (via validateWidgetBindings (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6530cff5acfb304640e116a241203c716301ff28 — the merge of head e9a4a58770730bad70123897f4cc366c40270676 into base 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6530cff5acfb304640e116a241203c716301ff28 && git checkout 6530cff5acfb304640e116a241203c716301ff28
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3 e9a4a58770730bad70123897f4cc366c40270676 && git checkout -B drift-repro 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3 && git merge --no-ff e9a4a58770730bad70123897f4cc366c40270676

node scripts/docs-audit/affected-docs.mjs --json 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 0db29473cbc43d2c678b434fa5e4bcc0422f9fd3 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claude Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

ACCEPT — PM seat domain:devx @ objectstack (#6023), session session_012zGPuVVX3deAx9LdjK8jCk.

Reviewed against the three-dot diff origin/main...claude/issue-14436-chart-config-missing-overreach (merge base 6ed4b811a, head e9a4a5877), not the self-report. 3 files, +214/−42, matching the PR file list.

  • packages/lint/src/validate-widget-bindings.ts: CHART_TYPES / MEASURE_EXEMPT_CHART_TYPES (taxonomy minus a hand list) replaced by MARK_MIXING_CHART_TYPES = {combo}; the rule id, severity and suppressWarnings spelling are unchanged; the warning message now names the surviving consequence (uniform mark on a combo) instead of the false one ("series renders empty"); list(values) keeps the empty-selection case readable. The ChartTypeSchema import moved to the test as a membership check, so objectui#2945's lesson survives as a red test rather than a derivation. Docblock records the pinned-revision reading (.objectui-sha), which is the verification the claim note asked for.
  • Test file: the taxonomy loop is inverted to assert silence for the derived families, combo keeps positive + suppressible + declared-marks-clean cases, and the card's own repro (the two system_overview Row 3 tiles) is pinned as a fixture.
  • Changeset: @objectstack/lint patch — a warning rule narrows, no API removed (MARK_MIXING_CHART_TYPES is an additive export).
  • Hot-file scan: no other open PR touches validate-widget-bindings.*. Docs sweep on main: the id appears only in the rule source and CHANGELOGs, so no doc edit is owed.
  • CI on e9a4a5877 still in progress at review time; auto-merge is the arbiter (lint.yml step 31 check:pm-dispatch-gates, dev-reported 1402/1402 locally).

Out-of-scope findings #15462 and #15463 are graded separately on the cards. Flipping ready and arming auto-merge now; Fixes #14436 closes the card on merge.


Generated by Claude Code

@baozhoutao
baozhoutao marked this pull request as ready for review September 4, 2026 17:01
@baozhoutao
baozhoutao enabled auto-merge September 4, 2026 17:01
@baozhoutao
baozhoutao added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 347b777 Sep 4, 2026
35 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-14436-chart-config-missing-overreach branch September 4, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The shipped system_overview dashboard draws two chart widgets with no chartConfigchart-config-missing fires on our own metadata

2 participants