fix(app-shell): forward the authored chart block WHOLE to ListView, not a six-key projection (objectui#7823) - #7890
Merged
Conversation
…bjectui#7823) The object-view relay handed `ListView` a hand-listed projection of exactly six keys — `chartType`, `xAxisField`, `yAxisFields`, `aggregation`, `series`, `config` — which is the pre-ADR-0021 key set, frozen. The whole ADR-0021 (objectui#1890) authoring shape (`dataset` / `dimensions` / `values`) and the legacy `categoryField` / `valueField` spelling had no rung, so a view that declared them reached `ListView` with its binding stripped and `ListView` could not tell that from a view that declared nothing. Once objectui#7544 gave `ListView.availableViews` a chart capability check, the gate was handed six `undefined` keys and correctly answered "nothing declared" about a view whose author declared everything: ADR-0047 filtered the author's own whitelist down to `['grid']`, so no Chart toggle appeared and no diagnostic was emitted. The legacy `xAxisField` spelling did survive the projection and did resolve, so the two authoring shapes behaved differently on this route for reasons that lived entirely in that one object literal. The projection is replaced by a pointer, not widened from six keys to nine: a hand-listed key set is a copy, and copies rot silently — three more keys would buy this ADR's correctness while re-arming the identical trap for the next block key, with nothing to fire, because `viewDef` is `Record<string, any>` and a missing rung is invisible to tsc (objectui#7559 owns that mechanism and disclaims this census). Forwarding whole is safe because `ListView` reads the block BY NAME at both readers — `resolveListChartBinding` and `case 'chart'` — and never spreads it, so no key this relay stops dropping can collide downstream. It is the shape the sibling `gantt` / `timeline` / `tree` blocks in the same literal already use. An undeclared block now forwards `undefined` rather than the old permanently-truthy husk of six `undefined` keys, so the gate still offers no Chart toggle to a view that configured none. 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
|
This was referenced Sep 6, 2026
os-sam
marked this pull request as ready for review
September 6, 2026 02:03
os-sam
deleted the
claude/issue-7823-objectview-chart-relay-projection
branch
September 6, 2026 02:18
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
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 #7823
ObjectView'srenderListViewrelay handedListViewa hand-listed projection ofexactly six keys of the authored
chart:block. This replaces that copy with a pointer.What was measured, before any code was touched
The card was written at
900f8d995and flagged three things as unverified orcarried-over. All three were re-read line by line on
origin/main83fe6e741.1. The line numbers still hold. The dedicated chart-view branch is still at
:2098,the
fullSchemaliteral still opens at:2170, and the six-keychart:projection wasstill at
:2435.2.
InterfaceListPagedoes NOT carry the same projection — and the card's "maybe itdoes not relay it at all" is the closer half of its guess. It never names
chart, andit hand-lists nothing: it forwards the referenced view's
optionsbag WHOLE(
options: { ...(view.options ?? {}), ...(mapCfg ? { map: mapCfg } : {}) },:477), soa chart block authored under
optionspasses through untouched. It relays noview-level
chart:key at all — but it does relay the view-levelmapblock verbatimone line above (
...((view as any).map ? { map: (view as any).map } : {})), which is thein-repo precedent for the shape this PR writes. The six-key projection is unique to
ObjectView.tsx, so this fix is correctly scoped to one file and no second host needsthe same edit.
3. The spread order — and a CORRECTION to the card. The card asserts that
schema.chartat theListViewend "is the object-level list's block, never the activeview's". The first half of that is right as a mechanism and wrong as a reading:
fullSchemadoes spread...listSchemafirst (:2171) and declares no top-levelchart:rung, so nothing overrides whatever the spread carries.listSchemathis relay receives is built byplugin-view'sObjectView(
:1747), and that literal has nochartkey at all. Soschema.chartat theListViewend isundefinedon this route — not an object-level chart block. There isno object-level block to shadow the view's, because none is ever constructed.
options:(:2371), not at the top level offullSchema, so what the gate actually consulted wasschema.options.chart.Net effect on the gate, which resolves
schema.chart || schema.options?.chart || {}:undefined || sixKeyHusk— the husk won every time, because an object of sixundefinedvalues is truthy. The card's conclusion (the ADR-0021 shape never arrives)is correct; its route is not, and only the projection needed fixing.
The fix: a pointer, not a wider copy
Widening the list from six keys to nine would buy ADR-0021's correctness and re-arm the
identical trap for the next block key, with nothing to fire when it happens:
viewDefisan untyped record (
Recordofstringtoany), so a missing rung is invisible totsc(#7559 owns that mechanismand explicitly disclaims this census). A hand-listed key set is a copy of a schema's key
set, and copies rot silently.
The whole relay carries no key
ListViewcan trip over, and this was checked ratherthan assumed (the ruling's stop-and-report condition).
ListViewnever SPREADS thisblock:
resolveListChartBindingandcase 'chart'both read it BY NAME —dataset,dimensions,values,chartType,xAxisField,yAxisFields,categoryField,valueField,aggregation,series. Keys it does not consult are ignored exactly asthey already are for the
gantt/timeline/treeblocks in the same literal, whichare relayed whole today. No stop-and-report condition was met.
One shape change worth naming: an undeclared block now forwards
undefinedinstead of apermanently-truthy husk of six
undefinedkeys. Both resolve to "no binding" at the gate,so the observable answer is unchanged — and the negative controls below pin that.
Behaviour evidence — a reading BEFORE and AFTER
New test
packages/app-shell/src/views/ObjectView.chartRelay-7823.test.tsxmounts thereal
ListViewbehind the real relay (the sibling relay tests stub it; this card's claimis about the capability gate one seam further on, so the assertion is the Chart toggle in
the DOM, per the #6318 standard that it renders differently).
83fe6e741, unmodified)dataset/dimensions/values), whitelisting['grid','chart']expected false to be true)options.charthanded to the gate{chartType, xAxisField, yAxisFields, aggregation, series, config}, allundefinedtoEqualverbatimcategoryField/valueFieldxAxisField/yAxisFieldschart: {}chart: { chartType: 'pie' }, no bindingThe gate under test is the live one, not a stale build: the root vitest config aliases
@object-ui/plugin-listtopackages/plugin-list/src, so theListViewthat renderedhere is current source carrying #7544's capability check.
Measured run counts: before 5 failed / 3 passed; after 8 passed. Four of the five
failures are the
THE FIXarms; the fifth is the SCHEMA half of the first negativecontrol (
options.chartwas the husk, not absent) — its DOM half, the toggle, was greenbefore and stayed green after. Every negative control's behaviour assertion is green in
both worlds, which is what refuses a "fix" that merely widens the husk and lights the
toggle on views that configured no chart.
What this is not
Not #7559 (that card owns the mechanism and disclaims the census; this is a rung that
EXISTS and narrows, which a rung census cannot report because the key is present). Not
#7547 / #7029 / #7070 — those are invented binding names on the FORCED path; nothing here
adds or removes a floor. Not #7544 itself, which is
packages/plugin-listand lands thegate this relay now feeds.
packages/plugin-list/andpackages/types/are untouched.Gates, re-run on the pushed commit
4fed69accwithgit diff HEADemptynode scripts/check-changeset-presence.mjs— EXIT=0, verbatim:2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/7823-objectview-chart-relay-whole-block.md.Scoredpatch.node scripts/check-changeset-no-major.mjs— EXIT=0,No changeset declares a major bump.node scripts/check-governed-queue-guard.mjs --testover the three changed paths —EXIT=0, verbatim:
NOT GOVERNED - 3 path(s) checked against 5 governed surface(s); none matched. An ordinary pull request: the normal review and merge-queue route applies.pnpm check:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:governed-queue-guard— all EXIT=0.pnpm --filter @object-ui/app-shell type-check— EXIT=0. The new test file's presencein that program was confirmed with
--listFiles(1 hit) rather than assumed, since theemitting tsconfig excludes test directories.
pnpm exec vitest run packages/app-shell/src/views/(the changed file's wholedirectory, recursive) — EXIT=0,
Test Files 376 passed (376),Tests 3580 passed | 1 skipped (3581). No sibling relay or view test regressed.pnpm --filter @object-ui/app-shell lint— EXIT=0, 0 errors.node scripts/check-lint-coverage.mjs— EXIT=0,lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).(Its first run throughpnpm -sexited 254 withEMPTY output while the test suite held memory — read as "could not run", not as a
finding, and re-measured directly.)
pnpm check:unreferenced-sources,check:handler-key-reads,check:i18n-keys— allEXIT=0.
Exit codes were captured by redirecting first, never through a pipe.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code