fix(theme): tolerate blocked web storage

[JSONbored]

Motivation

• The theme provider previously read localStorage.getItem('hc-theme') without handling DOMException cases which can occur when Web Storage is blocked, causing client-side crashes because the provider is mounted globally.
• Make theme persistence best-effort and ensure the UI falls back to a safe default instead of throwing in restricted or sandboxed environments.

Description

• Add an isTheme type guard and a readStoredTheme helper that wraps window.localStorage.getItem in a try/catch and returns null on failure.
• Replace the direct localStorage read with readStoredTheme() and preserve existing best-effort writes using window.localStorage.setItem inside the existing guarded try/catch.
• Guard reduced-motion detection with optional chaining on window.matchMedia to avoid errors in environments where matchMedia is unavailable.
• Preserve the provider API (ThemeProvider and useTheme) and the default theme ("light") behavior.

Testing

• Ran pnpm type-check and the TypeScript checks completed successfully.
• Ran git diff --check and no diff-check issues were reported.

---

Codex Task

Summary by CodeRabbit

• Refactor
  • Enhanced theme system reliability with improved error handling for storage operations and reduced motion detection, ensuring more robust theme initialization and persistence.

[coderabbitai]

📝 Walkthrough

Walkthrough

The theme initialization logic in apps/web/src/lib/theme.tsx was refactored to safely handle localStorage operations. New type-safe helpers validate and read the stored theme with SSR guards and error handling, while ThemeProvider initialization and persistence effects now use these helpers and wrap writes in try/catch. The reduced-motion check was made more defensive with optional chaining on matchMedia.

Changes

Theme Storage Safety

Layer / File(s)

Summary

Theme storage helpers and provider integration apps/web/src/lib/theme.tsx

Added isTheme type guard and readStoredTheme() helper that safely read the hc-theme value from localStorage (returning null on SSR, invalid values, or errors). ThemeProvider initialization now reads from storage (defaulting to "light") instead of prefers-color-scheme, and theme persistence uses window.localStorage.setItem(...) wrapped in try/catch. The toggle function's reduced-motion check uses optional chaining on window.matchMedia for defensive API access.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through storage so bright,
Safe guards and helpers make things just right,
No SSR crashes, no motion surprise,
Try-catch wrapped wisdom before your eyes! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description includes motivation, implementation details, and testing evidence, but doesn't follow the provided template structure with its checkboxes and sections for platform/code PRs.	Consider reformatting the description to match the repository template, particularly the platform/code PR section with changed components, validation steps, and quality evidence subsections.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(theme): tolerate blocked web storage' directly and concisely describes the main change: making the theme system resilient to blocked web storage scenarios.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/fix-theme-toggle-client-crash-risk

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[gittensory]

Important

Gittensory found maintainer review notes

Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed.

Readiness score: 93/100

Signal	Result	Evidence	Action
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.	Explain no-issue PR.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.	No action.
Review load	✅ 20/20	Readiness component derived from cached public PR metadata and labels; size label size:S.	No action.
Validation evidence	✅ 25/25	PR body includes validation/test evidence.	No action.
Open PR queue	✅ 10/10	0 open PR(s), 0 likely reviewable.	No action.
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 62 PR(s), 347 issue(s).	No action.
Gate result	✅ Passing	No configured blocker found.	No action.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
• Open PR queue = repo-wide review pressure; it is not a PR quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 62 PR(s), 347 issue(s).
• PR-specific overlap: none found.

Maintainer notes

• No linked issue detected: The planned PR does not reference a closing issue or explicit linked issue number.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

• Re-run Gittensory review

---

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. Learn more about Gittensor contribution workflows.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@apps/web/src/lib/theme.tsx`:
- Around line 47-49: The optional chaining currently only protects the
matchMedia call but not the .matches access, causing a TypeError when
window.matchMedia is undefined; update the reduced computation to fully guard
both call and property access by checking that window.matchMedia exists and that
the result of window.matchMedia("(prefers-reduced-motion: reduce)") is truthy
before reading .matches (i.e., use a complete optional chain or intermediate
variable), ensuring the variable reduced safely falls back to false in
sandboxed/restricted environments where matchMedia is unavailable; locate and
change the reduced assignment in apps/web/src/lib/theme.tsx.

🪄 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: 620fcb5e-b7e7-40de-beb8-92f8be74bec7

📥 Commits

Reviewing files that changed from the base of the PR and between 8f302a4 and 8d125d9.

📒 Files selected for processing (1)

• apps/web/src/lib/theme.tsx

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Incomplete optional chaining will throw when matchMedia is unavailable.

The optional chaining on line 49 only guards the function call, not the .matches property access. If window.matchMedia is undefined, the expression window.matchMedia?.(...) evaluates to undefined, and then .matches attempts to read a property on undefined, throwing a TypeError.

This defeats the PR's goal to tolerate missing APIs in sandboxed or restricted environments, and will crash the provider (which wraps the entire app per the root component).

🔒 Proposed fix to complete the optional chain

         const reduced =
           typeof window !== "undefined" &&
-          window.matchMedia?.("(prefers-reduced-motion: reduce)").matches;
+          window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches;

📝 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

	const reduced =
	typeof window !== "undefined" &&
	window.matchMedia("(prefers-reduced-motion: reduce)").matches;
	window.matchMedia?.("(prefers-reduced-motion: reduce)").matches;
	const reduced =
	typeof window !== "undefined" &&
	window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches;

🤖 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/lib/theme.tsx` around lines 47 - 49, The optional chaining
currently only protects the matchMedia call but not the .matches access, causing
a TypeError when window.matchMedia is undefined; update the reduced computation
to fully guard both call and property access by checking that window.matchMedia
exists and that the result of window.matchMedia("(prefers-reduced-motion:
reduce)") is truthy before reading .matches (i.e., use a complete optional chain
or intermediate variable), ensuring the variable reduced safely falls back to
false in sandboxed/restricted environments where matchMedia is unavailable;
locate and change the reduced assignment in apps/web/src/lib/theme.tsx.

[reviewwed]

🔍 Maintainer advisory review

Reviewed 1 changed file(s) — two independent AI reviewers. This is an advisory review — it does not merge or close the PR.

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the items below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
The change improves robustness by handling blocked localStorage and adds type safety, but introduces a subtle bug with the optional chaining on window.matchMedia. The expression window.matchMedia?.("(prefers-reduced-motion: reduce)").matches will throw if matchMedia is undefined because .matches is accessed on undefined. This could break the UI in environments without matchMedia. No new security concerns are introduced, but the change lacks tests for the new helper functions and the blocked‑storage scenario.

Suggestions

• Fix the optional chaining to safely access .matches, e.g.: const reduced = window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches ?? false;
• Add unit tests for readStoredTheme to verify it returns null when localStorage throws (e.g., in private/incognito mode).
• Add tests for the theme toggle logic to ensure the class is applied correctly and the value is persisted.
• Consider exporting the isTheme helper if it might be useful elsewhere, or document its purpose.

Worth double-checking

• Runtime TypeError in browsers or environments lacking window.matchMedia, potentially preventing the app from rendering.
• Uncovered edge cases for blocked or unavailable localStorage could still cause silent failures if not tested.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The change adds defensive checks for blocked or unavailable web storage (localStorage) and uses optional chaining for matchMedia, making the theme provider more robust in environments where storage is disabled or throws. It does not alter the core logic, removes only dead code, and introduces no new external dependencies or side effects.

Suggestions

• Consider adding unit tests for readStoredTheme covering cases where localStorage throws, returns non‑Theme values, or is undefined.
• Ensure consistent use of window. prefix for all localStorage accesses elsewhere in the codebase for clarity.

Worth double-checking

• Very low risk of regressions; the only behavioral change is the removal of unused sys variable, which had no effect.
• Potential minor difference if a consumer relied on the void sys; line to suppress TypeScript unused‑variable warnings, but the variable is removed entirely.