refactor(cards): give every registered card and variant its own error boundary - #301
Merged
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughRegistered cards and variants now use ChangesCard error-boundary standardization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CardRegistry
participant CardExport
participant withCardErrorBoundary
participant EntityErrorBoundary
participant ErrorDisplay
CardRegistry->>CardExport: select registered card or variant
CardExport->>withCardErrorBoundary: render wrapped memoized card
withCardErrorBoundary->>EntityErrorBoundary: pass entityId and tier
EntityErrorBoundary->>ErrorDisplay: render recoverable fallback on render error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Enforces the repo’s “per-card/per-variant error boundary” convention by introducing a shared withCardErrorBoundary helper and applying it across all registered entity cards and variants, with a registry-driven test guard and corresponding spec/change-doc updates (including closing change 0041).
Changes:
- Add
withCardErrorBoundary(wrapping cards inEntityErrorBoundary, outsidememo) and apply it to all registry-dispatched cards and variants. - Extend
EntityErrorBoundaryto accept an optionaltierand forward it toErrorDisplayso fallbacks can degrade by layout tier when available. - Add a registry-enumerating test to prevent new cards/variants from being added without boundaries; update entity-cards spec + docs indexes and close change 0041.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/cardErrorBoundary.tsx | New shared helper that wraps cards/variants in EntityErrorBoundary and sets a useful displayName. |
| src/components/ErrorBoundary.tsx | Adds optional tier to EntityErrorBoundary and forwards it to ErrorDisplay. |
| src/components/tests/cardErrorBoundary.test.tsx | New registry-driven guard test asserting every dispatched card/variant contains its own boundary and uses the tile fallback. |
| src/components/WeatherCard/index.tsx | Wraps the dispatcher component itself with withCardErrorBoundary (in addition to per-variant boundaries). |
| src/components/WeatherCard/WeatherCardDefault.tsx | Switches variant export to use withCardErrorBoundary. |
| src/components/WeatherCard/WeatherCardDetailed.tsx | Switches variant export to use withCardErrorBoundary. |
| src/components/WeatherCard/WeatherCardMinimal.tsx | Switches variant export to use withCardErrorBoundary. |
| src/components/WeatherCard/WeatherCardModern.tsx | Switches variant export to use withCardErrorBoundary. |
| src/components/ClimateCard/index.tsx | Wraps both the default climate presentation and the dial variant with withCardErrorBoundary. |
| src/components/MediaPlayerCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/VacuumCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/PersonCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/SensorCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/LightCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/BinarySensorCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/ButtonCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/ActionCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/AlarmCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/FanCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/CoverCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/LockCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/CameraCard/index.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/InputBooleanCard.tsx | Switches card export to use withCardErrorBoundary. |
| src/components/InputNumberCard.tsx | Refactors export to wrap the memoized component via withCardErrorBoundary. |
| src/components/InputSelectCard.tsx | Refactors export to wrap the memoized component via withCardErrorBoundary. |
| src/components/InputTextCard.tsx | Refactors export to wrap the memoized component via withCardErrorBoundary. |
| src/components/InputDateTimeCard.tsx | Refactors export to wrap the memoized component via withCardErrorBoundary. |
| docs/specs/entity-cards/index.md | Records the containment contract and documents the two-boundary model (renderer + per-card). |
| docs/changes/0041-card-conventions-and-reference.md | Marks PR 1 task complete and closes the change document (status complete). |
| docs/index.yml | Flips change 0041 status to complete. |
| docs/index.md | Flips change 0041 row to complete. |
| AGENTS.md | Updates the “Entity Card Registration” instructions to require withCardErrorBoundary outside memo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #301 +/- ##
=======================================
Coverage ? 87.29%
=======================================
Files ? 226
Lines ? 8235
Branches ? 2789
=======================================
Hits ? 7189
Misses ? 823
Partials ? 223 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/__tests__/cardErrorBoundary.test.tsx`:
- Around line 125-137: The test case around the glance-tier Card recovery path
should verify the intended retry behavior rather than only confirming the inline
button is hidden. If glance tiles support recovery, open the fallback tile,
assert the modal exposes Retry, and verify activating it triggers the boundary
reset; otherwise narrow the assertion to the documented read-only behavior.
- Around line 146-164: Add a test alongside the existing withCardErrorBoundary
displayName cases that rerenders a wrapped memoized card with changed props and
verifies the inner card updates. Ensure the assertion distinguishes the boundary
being outside memo, while preserving the existing naming coverage.
In `@src/components/cardErrorBoundary.tsx`:
- Around line 77-90: Update withCardErrorBoundary in
src/components/cardErrorBoundary.tsx (lines 77-90) to accept a fallback/default
tier and pass props.tier ?? defaultTier to EntityErrorBoundary; update the
ActionCard usage in src/components/ActionCard/index.tsx (lines 351-353) to
provide glance as that default tier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6d1e671-f37e-4b7a-9b36-68b797c46fec
📒 Files selected for processing (32)
AGENTS.mddocs/changes/0041-card-conventions-and-reference.mddocs/index.mddocs/index.ymldocs/specs/entity-cards/index.mdsrc/components/ActionCard/index.tsxsrc/components/AlarmCard/index.tsxsrc/components/BinarySensorCard/index.tsxsrc/components/ButtonCard/index.tsxsrc/components/CameraCard/index.tsxsrc/components/ClimateCard/index.tsxsrc/components/CoverCard/index.tsxsrc/components/ErrorBoundary.tsxsrc/components/FanCard/index.tsxsrc/components/InputBooleanCard.tsxsrc/components/InputDateTimeCard.tsxsrc/components/InputNumberCard.tsxsrc/components/InputSelectCard.tsxsrc/components/InputTextCard.tsxsrc/components/LightCard/index.tsxsrc/components/LockCard/index.tsxsrc/components/MediaPlayerCard/index.tsxsrc/components/PersonCard/index.tsxsrc/components/SensorCard/index.tsxsrc/components/VacuumCard/index.tsxsrc/components/WeatherCard/WeatherCardDefault.tsxsrc/components/WeatherCard/WeatherCardDetailed.tsxsrc/components/WeatherCard/WeatherCardMinimal.tsxsrc/components/WeatherCard/WeatherCardModern.tsxsrc/components/WeatherCard/index.tsxsrc/components/__tests__/cardErrorBoundary.test.tsxsrc/components/cardErrorBoundary.tsx
fx
force-pushed
the
refactor/0041-errorboundary-decision
branch
from
July 30, 2026 10:01
9baef35 to
a2ebbe9
Compare
… boundary Settles the enforce-or-retire question the change document leaves open, in favour of enforcing. Verified against the tree first: the registry maps 23 domains onto 20 distinct components — the action family's four domains share one card — of which four wrapped and sixteen did not, the count the change document claims and still accurate. Enforcing rather than retiring, because the paths GridView does not cover are concrete. CardConfig.tsx renders LightCard and BinarySensorCard directly in its preview, both of them among the sixteen, and every family has a story. On the preview path the nearest boundary above a throwing card was the Dashboard's, so one card's render error blanked the whole dashboard rather than one tile; in a story there was none at all. The established shape is a nine-line rationale comment plus a four-line wrapper, so repeating it twenty times would have been twenty copies of one contract — and the copies were already drifting, VacuumCard's citing a count of nine families. withCardErrorBoundary holds the rationale once and each card spends one call; the four that already wrapped are converted to it so one convention has one shape. The helper wraps in EntityErrorBoundary rather than the base ErrorBoundary. On the dashboard the card's own boundary is now the inner one, so it — not GridView's — decides what a failed tile looks like, and the base fallback is a 600px-wide panel with a 400px floor that would overflow a one-cell tile and cover its neighbours. The card knows its tier, which GridView's wrapper does not, so the fallback can also degrade with the cell. Two holes the per-component count did not show. ClimateCard's dial variant had no boundary and is what the loader pins every climate card placed before change 0017 onto. WeatherCard counted as wrapping because each of its four variants does, but those sit below the dispatcher and cannot catch a throw in its option read, its store subscriptions or the CardConfig.Modal subtree it renders as the variant's sibling. The test builds its table from the registry and each entry's variants rather than naming cards, so a card or variant added without a boundary fails on the commit that adds it. It asserts the fallback rendered, not merely that nothing threw: "did not throw" is also what a card that quietly rendered looks like. Completes PR 1 of docs/changes/0041-card-conventions-and-reference.md and closes the change.
fx
force-pushed
the
refactor/0041-errorboundary-decision
branch
from
July 30, 2026 10:17
a2ebbe9 to
7b3d5e1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
docs/specs/entity-cards/index.md:425
- In this spec line, “
GridView's wrapper does not know” is inaccurate:GridViewcomputestierfor each item, it just doesn’t pass it into the outerEntityErrorBoundary. Rewording avoids implyingGridViewcan’t access the tier value.
Two boundaries stand over a card, at different altitudes, and both render the same thing. `EntityErrorBoundary` (`ErrorBoundary.tsx`) is what `GridView` wraps each tile in, rendering an `ErrorDisplay` card with a retry that resets it. Beneath that, `withCardErrorBoundary` (`cardErrorBoundary.tsx`) puts one inside each card, outside the card's `memo` — the only boundary on the paths `GridView` does not own, and on the dashboard the inner of the two, which is why it renders the tile-shaped fallback rather than the base `ErrorBoundary`'s dialog. It also passes the `tier`, which `GridView`'s wrapper does not know. The base `ErrorBoundary` underneath both supports a custom fallback and collapsible stack details.
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.
Summary
Completes PR 1 — ErrorBoundary decision of
docs/changes/0041-card-conventions-and-reference.md, which is that change's last open task, so this closes the change:**Status:** completein the change document,status: completeindocs/index.yml, and the row indocs/index.md.The decision the change document left open — enforce the per-card boundary convention or retire it — is settled in favour of enforcing, with the reasoning recorded in the change document's Design Decisions and in the entity-cards spec.
The count, verified rather than taken
The change document claims sixteen registered components lack a boundary. That is still accurate: the registry maps 23 domains onto 20 distinct components (the action family's four domains share
ActionCard), of which four wrapped —WeatherCard,MediaPlayerCard,PersonCard,VacuumCard— and sixteen did not.Two holes the per-component count does not show, both closed here.
ClimateCard'sdialvariant had no boundary and is what the loader pins every climate card placed before change 0017 onto, so it is the presentation most climate cards actually render. AndWeatherCardcounted as wrapping only because each of its four variants does, but those sit below the dispatcher and cannot catch a throw in its option read, its store subscriptions, or theCardConfig.Modalsubtree it renders as the variant's sibling. Counting registered components and the variants registered against them, 17 of 27 render targets were unprotected.Why enforce rather than retire
The paths
GridView'sEntityErrorBoundarydoes not cover are concrete, not hypothetical.CardConfig.tsxrendersLightCardandBinarySensorCarddirectly in its configuration preview — two of the sixteen — and all 20 families have a Storybook story. On the preview path the nearest boundary above a throwing card wasDashboard's, so one card's render error blanked the entire dashboard rather than one tile; in a story there was no boundary at all. Retiring would have traded a real safeguard for consistency with the majority that happened to be unprotected.One helper, not sixteen copies
The established shape is a nine-line rationale comment plus a four-line wrapper function. Repeating it twenty times would be twenty copies of one contract — the duplication
REVIEW.mdcalls this repo's largest defect class — and the copies were already drifting:VacuumCard's cited a count of nine families and an issue this change supersedes.withCardErrorBoundary(src/components/cardErrorBoundary.tsx) holds the rationale once and each card spends one call. The four that already wrapped are converted to it in the same PR, because leaving two shapes for one convention is a smaller version of the state this change exists to end.The helper wraps in
EntityErrorBoundaryrather than the baseErrorBoundary. On the dashboard the card's own boundary is now the inner one, so it — notGridView's — decides what a failed tile looks like, and the base fallback is a 600px-wide panel with a 400px minimum height that would overflow a 1×1 cell and cover its neighbours. The card also knows itstier, whichGridView's wrapper does not, soEntityErrorBoundarygained an optionaltierand the fallback now degrades with the cell. This one was caught by the localcodex reviewpass, not by me.Tests
src/components/__tests__/cardErrorBoundary.test.tsxbuilds its table fromdomainToCardand each entry'svariantsrather than naming cards, so a card or variant added without a boundary fails on the commit that adds it — the same reasoning as the reference-path check in PR 2, applied to the convention instead of to the citations. Every case renders the card bare, because a boundary asserted underneath another boundary proves nothing about the case this one exists for. It asserts the fallback rendered rather than merely that nothing threw: "did not throw" is also what a card that quietly rendered looks like.27 containment cases, 27 tile-shape cases (the
glancetier has no room for the message or Retry, so a fallback ignoring the tier renders both), the registry-enumeration guard, and 2 display-name cases.The memo-placement requirement is pinned by the existing
cardSpanMemo.test.tsx, whose three cards (LightCard,BinarySensorCard,ButtonCard) are all newly wrapped and whose load-bearing by-valuespancomparator still behaves identically through the boundary.Gates
npm testnpm run lintMigrationCutoffswarningnpm run typechecknpm run test:coveragesrc/coverednpm run build-storybookMutation probes
Each probe verified the mutation applied (
git diff --quiet), read the result, restored, and re-grepped for content.LightCardloses its boundaryClimateCard+diallose theirsWeatherCarddispatcher loses its ownweathercasesInputTextCardloses its boundarytiertypehopPre-PR review
codex review --base main— 2 findings, both fixed: the 400px fallback overflowing a tile (above), and spec over-constraint, which is why the spec requirement now states observable containment rather than prescribing the wrapper and the memo arrangement. Those live inAGENTS.mdand the helper's doc comment instead, perREVIEW.md→ Specification Altitude.Local CodeRabbit (
cr --agent --base main --committed) — 1 finding, addressed:innerName's metadata type is now an explicitNameCarrierinterface naming the memo exotic shape, which also removed a cast.Out of scope
GridView's own boundary is untouched, and no flat card file was restructured into a folder — both explicitly out of scope in the change document.Summary by CodeRabbit
New Features
Tests