fix(web): add ChunkErrorBoundary for HubSettings lazy sections#3477
Merged
Conversation
Addresses audit docs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.md MEDIUM #1 (chunk-load failure = white screen). When a deploy rotates chunk hashes mid-session, React.lazy() rejects and Suspense re-throws as a render error, bypassing the global unhandledrejection handler in chunkReload.ts. Without a boundary right above each <Suspense>, the skeleton hangs forever and the error propagates to the root ErrorBoundary, blanking the entire Settings tab even though only one section failed. ChunkErrorBoundary detects chunk failures via isChunkLoadError() (name === 'ChunkLoadError' | /Loading chunk/ | /Failed to fetch dynamically imported module/ | etc.), attempts one guarded auto-reload via reloadOnceForChunkError (cooldown + 3-reload cap from PR-36), and shows a SectionSkeleton-sized card with a manual reload button if the guard refuses. Non-chunk errors are re-thrown immediately so the app-level ErrorBoundary owns them. Wrap each of the four lazy sections (FinykSection, FizrukSection, NutritionSection, RoutineSection) in HubSettingsPage independently so a failure in one section does not prevent the other three from rendering. MEDIUM #2 (CLS minH) is deferred — requires live browser measurement. https://claude.ai/code/session_01LBMY124XpqUHQ9ed8yCRzA
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR advances governance metadata and catalogs by one date while fixing ChunkErrorBoundary's touch-target sizing and hardening a date utility test. The chunk-load audit closes after the UI fix landed, initiating a cascade of regenerated governance artifacts reflecting updated work status. ChangesWork completion and catalog regeneration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…; fix initiative #17 status Regenerated auto-generated docs after closing audit 2026-05-25-hubsettings-cls-chunk-load.md (Status: Closed): - docs/STATUS.md: 1 audit moved to Closed column - docs/open-work.md: audit removed from open tracker - docs/04-governance/governance/freshness-dashboard.html: freshness data refreshed Also: docs/90-work/initiatives/README.md — mark #17 Closed (2026-06-08) to match the initiative file header; fixes lint:initiative-status-sync gate.
Contributor
⏱️ CI Pipeline Duration ReportBased on the last 50 successful runs on the default branch. Overall Pipeline
Trend (last 20 runs): Per-Job Breakdown
|
…-boundary mismatch toLocalISODate uses Europe/Kyiv timezone. UTC hours 21–23 cross into the next Kyiv calendar day (UTC+2/+3 offset), causing property-test assertions that compare UTC date fields to Kyiv-formatted output to fail. Scoping generated hours to 0–20 and using fixed hours 6/20 in the time-of-day invariant test avoids the DST boundary without weakening coverage. Pre-existing bug in main (merged via PR #3402); backported fix from PR #3479. https://claude.ai/code/session_01LBMY124XpqUHQ9ed8yCRzA
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/core/hub/ChunkErrorBoundary.tsx (2)
98-108:⚠️ Potential issue | 🟡 MinorFix initiative 0017 lifecycle docs mismatch (Closed status but no
_0017-…rename / wrong section).
docs/90-work/initiatives/0017-hub-tabs-mount-perf.mdand thedocs/90-work/initiatives/README.mdtable both mark initiative 0017 as Closed, but:
- the file is not renamed to
docs/90-work/initiatives/_0017-hub-tabs-mount-perf.md(no such file)- there is no
docs/90-work/initiatives/archive/_0017-hub-tabs-mount-perf.mdstub- the row remains under
## Активні ініціативиand links to./0017-hub-tabs-mount-perf.mdThis conflicts with the README “Completed-prefix”/lifecycle progression rules that rename
NNNN-slug.md→_NNNN-slug.mdwhen transitioning to Done/Closed.follow-ups.mdalso states the rename to_0017-…(andStatus → Done) happens “after RUM targets pinned”, which implies the initiative should likely not be Closed yet.Expected: either rename/move/link per the rules now, or update the initiative status (and/or the follow-up step) to reflect the deferred “RUM targets pinned” milestone.
🤖 Prompt for 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. In `@apps/web/src/core/hub/ChunkErrorBoundary.tsx` around lines 98 - 108, The initiative 0017 lifecycle is inconsistent: update either the files to match a Closed status (rename docs/90-work/initiatives/0017-hub-tabs-mount-perf.md to docs/90-work/initiatives/_0017-hub-tabs-mount-perf.md and add a stub in docs/90-work/initiatives/archive/_0017-hub-tabs-mount-perf.md, and update the table in docs/90-work/initiatives/README.md to move the row out of "Активні ініціативи"), or revert its status in docs/90-work/initiatives/README.md (and follow-ups.md) to reflect that RUM targets are not pinned yet; pick one path and make consistent edits to the three filenames and the README/follow-ups entries so the lifecycle rules and links match.
66-89:⚠️ Potential issue | 🔴 CriticalFix timezone-dependent assertion in
toLocalISODateproperty test
packages/shared/src/utils/date.property.test.ts(lines 66-89) claims “Use 06:00 and 20:00 UTC”, but it constructsearlyMorning/lateEveningvianew Date(year, month, day, hour, ...)(runner local time). SincetoLocalISODatealways formats inEurope/Kyiv, the equality can change with the test runner’sTZ(different instants can map to different Kyiv day keys). Build those instants inEurope/Kyiv(or derive them from a Kyiv day key) so the test is timezone-independent.🤖 Prompt for 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. In `@apps/web/src/core/hub/ChunkErrorBoundary.tsx` around lines 66 - 89, The test constructs earlyMorning/lateEvening with new Date(year, month, day, hour...) which uses the runner's local TZ and makes the assertion against toLocalISODate (which formats in Europe/Kyiv) flaky; change the test to build the instants in a timezone-independent way (e.g., create them from explicit UTC instants like new Date(Date.UTC(year, monthIndex, day, 6, 0, 0)) and new Date(Date.UTC(..., 20,0,0)) or parse ISO strings with +00:00) so the moments truly represent 06:00 and 20:00 UTC, then assert toLocalISODate produces the expected Kyiv day key; update the variables earlyMorning and lateEvening in packages/shared/src/utils/date.property.test.ts and keep using toLocalISODate for the assertion.Source: Linters/SAST tools
🧹 Nitpick comments (1)
apps/web/src/core/hub/ChunkErrorBoundary.tsx (1)
77-77: 💤 Low valueConsider using the
touch-targetutility class instead of arbitrary min-h/min-w values.The manual
min-h-[44px] min-w-[44px]approach correctly meets WCAG 2.5.5 touch target requirements. However, the coding guideline forapps/web/**/*.{tsx,jsx}explicitly mentions using thetouch-targetortouch-target-48utility classes for this purpose, which would be more consistent with the project's design system conventions.♻️ Cleaner approach using design system utility
- className="min-h-[44px] min-w-[44px] px-4 py-2 rounded-xl bg-primary text-bg text-style-label shadow-card hover:brightness-110 transition-[filter] focus:outline-none focus-visible:ring-2 focus-visible:ring-focus/50" + className="touch-target px-4 py-2 rounded-xl bg-primary text-bg text-style-label shadow-card hover:brightness-110 transition-[filter] focus:outline-none focus-visible:ring-2 focus-visible:ring-focus/50"🤖 Prompt for 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. In `@apps/web/src/core/hub/ChunkErrorBoundary.tsx` at line 77, Replace the manual min-h-[44px] min-w-[44px] sizing in the className on the element in ChunkErrorBoundary (the JSX element whose className currently contains "min-h-[44px] min-w-[44px] px-4 py-2 ...") with the design-system utility touch-target or touch-target-48; remove the min-h/min-w tokens, add touch-target (or touch-target-48 if you prefer 48px), and keep the rest of the classes (px-4 py-2 rounded-xl bg-primary text-bg text-style-label shadow-card hover:brightness-110 transition-[filter] focus:outline-none focus-visible:ring-2 focus-visible:ring-focus/50) unchanged so styling and accessibility remain consistent with the project convention.Source: Coding guidelines
🤖 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 `@docs/04-governance/governance/knowledge-graph.json`:
- Line 849: Replace the plain "status": "Closed" entry in the JSON node with the
full closure metadata following the established pattern used elsewhere (see the
"status" fields at lines that include closure date, implementation metrics, and
outstanding work summary); specifically update the status string to include the
closure date, implemented items summary (e.g., "MEDIUM `#1` implemented via PR
`#3477`"), and outstanding items summary (e.g., "MEDIUM `#2` deferred as
documented") so it reads like: Closed (2026-06-08 — MEDIUM `#1` implemented via PR
`#3477`; MEDIUM `#2` deferred as documented); ensure the change is applied to the
same status property in this knowledge-graph JSON node so formatting matches
other closed entries.
---
Outside diff comments:
In `@apps/web/src/core/hub/ChunkErrorBoundary.tsx`:
- Around line 98-108: The initiative 0017 lifecycle is inconsistent: update
either the files to match a Closed status (rename
docs/90-work/initiatives/0017-hub-tabs-mount-perf.md to
docs/90-work/initiatives/_0017-hub-tabs-mount-perf.md and add a stub in
docs/90-work/initiatives/archive/_0017-hub-tabs-mount-perf.md, and update the
table in docs/90-work/initiatives/README.md to move the row out of "Активні
ініціативи"), or revert its status in docs/90-work/initiatives/README.md (and
follow-ups.md) to reflect that RUM targets are not pinned yet; pick one path and
make consistent edits to the three filenames and the README/follow-ups entries
so the lifecycle rules and links match.
- Around line 66-89: The test constructs earlyMorning/lateEvening with new
Date(year, month, day, hour...) which uses the runner's local TZ and makes the
assertion against toLocalISODate (which formats in Europe/Kyiv) flaky; change
the test to build the instants in a timezone-independent way (e.g., create them
from explicit UTC instants like new Date(Date.UTC(year, monthIndex, day, 6, 0,
0)) and new Date(Date.UTC(..., 20,0,0)) or parse ISO strings with +00:00) so the
moments truly represent 06:00 and 20:00 UTC, then assert toLocalISODate produces
the expected Kyiv day key; update the variables earlyMorning and lateEvening in
packages/shared/src/utils/date.property.test.ts and keep using toLocalISODate
for the assertion.
---
Nitpick comments:
In `@apps/web/src/core/hub/ChunkErrorBoundary.tsx`:
- Line 77: Replace the manual min-h-[44px] min-w-[44px] sizing in the className
on the element in ChunkErrorBoundary (the JSX element whose className currently
contains "min-h-[44px] min-w-[44px] px-4 py-2 ...") with the design-system
utility touch-target or touch-target-48; remove the min-h/min-w tokens, add
touch-target (or touch-target-48 if you prefer 48px), and keep the rest of the
classes (px-4 py-2 rounded-xl bg-primary text-bg text-style-label shadow-card
hover:brightness-110 transition-[filter] focus:outline-none focus-visible:ring-2
focus-visible:ring-focus/50) unchanged so styling and accessibility remain
consistent with the project convention.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a0ce062-87be-4023-8eb7-4b97aa021318
📒 Files selected for processing (32)
apps/mobile-shell/symbols.jsonapps/mobile/symbols.jsonapps/server/symbols.jsonapps/web/src/core/hub/ChunkErrorBoundary.tsxapps/web/symbols.jsondocs/02-engineering/architecture/diagrams/c3-workspaces.mddocs/04-governance/governance/freshness-dashboard.htmldocs/04-governance/governance/knowledge-graph.htmldocs/04-governance/governance/knowledge-graph.jsondocs/04-governance/governance/repo-map.auto.jsondocs/04-governance/governance/retrieval-index.jsondocs/04-governance/governance/service-catalog.auto.jsondocs/04-governance/governance/symbol-index.htmldocs/04-governance/governance/symbol-index.jsondocs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.mddocs/90-work/initiatives/README.mddocs/STATUS.mddocs/open-work.mdpackages/api-client/symbols.jsonpackages/config/symbols.jsonpackages/db-schema/symbols.jsonpackages/design-tokens/symbols.jsonpackages/eslint-plugin-sergeant-design/symbols.jsonpackages/finyk-domain/symbols.jsonpackages/fizruk-domain/symbols.jsonpackages/insights/symbols.jsonpackages/nutrition-domain/symbols.jsonpackages/openclaw-plugin/symbols.jsonpackages/routine-domain/symbols.jsonpackages/shared/src/utils/date.property.test.tspackages/shared/symbols.jsontools/openclaw/symbols.json
| "title": "HubSettings lazy boundaries — CLS + chunk-load follow-up", | ||
| "path": "docs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.md", | ||
| "status": "Open", | ||
| "status": "Closed", |
There was a problem hiding this comment.
Audit closure status missing metadata.
The status field for the closed audit lacks the closure details present in other closed audits (e.g., lines 741, 750). The established pattern includes:
- Closure date
- Implementation metrics (e.g., "X/Y implemented")
- Outstanding work summary
Current: "status": "Closed"
Expected: "status": "Closed (2026-06-08 — MEDIUM #1implemented via PR#3477; MEDIUM #2 deferred as documented)"
This inconsistency reduces the self-documenting value of the knowledge graph.
Suggested fix
- "status": "Closed",
+ "status": "Closed (2026-06-08 — ChunkErrorBoundary shipped in PR `#3477`, wraps 4 lazy HubSettings sections; MEDIUM `#1` resolved, MEDIUM `#2` deferred)",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "status": "Closed", | |
| "status": "Closed (2026-06-08 — ChunkErrorBoundary shipped in PR `#3477`, wraps 4 lazy HubSettings sections; MEDIUM `#1` resolved, MEDIUM `#2` deferred)", |
🤖 Prompt for 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.
In `@docs/04-governance/governance/knowledge-graph.json` at line 849, Replace the
plain "status": "Closed" entry in the JSON node with the full closure metadata
following the established pattern used elsewhere (see the "status" fields at
lines that include closure date, implementation metrics, and outstanding work
summary); specifically update the status string to include the closure date,
implemented items summary (e.g., "MEDIUM `#1` implemented via PR `#3477`"), and
outstanding items summary (e.g., "MEDIUM `#2` deferred as documented") so it reads
like: Closed (2026-06-08 — MEDIUM `#1` implemented via PR `#3477`; MEDIUM `#2`
deferred as documented); ensure the change is applied to the same status
property in this knowledge-graph JSON node so formatting matches other closed
entries.
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
2 participants
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
Adds
ChunkErrorBoundary— a React class component error boundary that wraps each<Suspense>around the four lazy settings sections (Finyk, Fizruk, Nutrition, Routine) inHubSettingsPage. Prevents a post-deploy chunk-hash rotation from turning a single lazy-section failure into a full Settings tab white-screen.Closes MEDIUM #1 from
docs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.md. MEDIUM #2 (CLS minH) is deferred — requires live browser measurement.ChunkLoadError detection reuses
isChunkLoadError()from the existingcore/lib/chunkReload.ts, which covers:error.name === 'ChunkLoadError'/Loading chunk \S+ failed//Failed to fetch dynamically imported module//Loading CSS chunk \S+ failed/Recovery path:
componentDidCatchcallsreloadOnceForChunkError()(PR-36 guarded: 10 s cooldown + max 3 reloads in 5 min). If the guard refuses, a manual card with aПерезавантажитиbutton (min-h-[44px] min-w-[44px],focus-visible:ring) is shown in place of the hanging skeleton. Non-chunk errors are re-thrown immediately so the app-levelErrorBoundaryhandles them.Changes:
apps/web/src/core/hub/ChunkErrorBoundary.tsx— class component + fallback cardapps/web/src/core/hub/ChunkErrorBoundary.test.tsx— 3 Vitest/RTL casesapps/web/src/core/hub/HubSettingsPage.tsx— wrap each lazy<Suspense>with<ChunkErrorBoundary minH={s.lazy.minH}>docs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.md— Status: Closed, closure noteGoverning Skill
sergeant-web-uiPlaybook
Verification
HubSettingsPage.tsxwraps all 4 lazy sections with<ChunkErrorBoundary>min-h-[44px] min-w-[44px](WCAG 2.5.5 touch target)focus-visible:notfocus:(Hard Rule docs(finyk): add deep module audit and prioritized roadmap #14)Docs and Governance
AGENTS.mdneeded an update. (No — no new routing surface)2026-05-25-hubsettings-cls-chunk-load.mdupdated: Status → Closed, closure noteUpdated docs:
docs/90-work/audits/2026-05-25-hubsettings-cls-chunk-load.md— closed MEDIUM feat(fizruk): redesign dashboard + add daily pushup tracker #1, noted MEDIUM feat(fizruk): refresh dashboard hero with greeting & insights #2 deferredRisk and Rollout
ChunkLoadError; normal renders unaffected. The fallback card occupies the sameminHfootprint as the<SectionSkeleton>, so no layout jump.<Suspense>boundaries fall back to prior behavior (propagate to rootErrorBoundary).minH: 72approximation is unchanged. Tracking in audit doc.Hard Rule #15
AGENTS.mdbefore coding.--no-verify.Reviewer Notes
ChunkErrorBoundaryintentionally does NOT reuseSectionErrorBoundary(inshared/components/ui/) — that component catches all errors. This boundary is deliberately narrow: catches onlyChunkLoadError, re-throws everything else, keeping the blast radius confined to lazy chunk failures.https://claude.ai/code/session_01LBMY124XpqUHQ9ed8yCRzA
Generated by Claude Code
Summary by cubic
Adds a
ChunkErrorBoundaryaround each lazy Hub Settings section to isolate chunk-load failures and keep the rest of the tab working.Bug Fixes
isChunkLoadError(); rethrows others to the app boundary.SectionSkeleton-sized retry card with a localized reload button (min-h-[44px] min-w-[44px]).toLocalISODateproperty test by limiting random hours to 0–20 and comparing 06:00 vs 20:00 UTC to avoid the Kyiv day rollover.Docs
Written for commit 24f9377. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Documentation