✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

📈 Frontend coverage change detected

The frontend unit test (vitest) coverage has increased by 0.0300%

• Current PR: 86.3300% (12518 lines, 1711 missed)
• Latest develop: 86.3000% (12497 lines, 1711 missed)

🎉 Great job on improving test coverage!

📊 View detailed coverage comparison

@cursor review

Summary

This PR fixes issue #13280 where user-selected theme preferences (Light/Dark) from the Settings menu were not persisting across page reloads when custom light/dark themes were configured. The fix introduces a bidirectional theme mapping system that intelligently maps between preset themes (Light/Dark) and custom theme equivalents (Custom Theme Light/Custom Theme Dark), ensuring user preferences are preserved across page reloads and theme configuration changes.

Key changes:

• Added mapCachedThemeToAvailableTheme() utility function for theme preference mapping
• Updated getCachedTheme() to properly restore custom light/dark themes with displayName
• Modified processThemeInput() in App.tsx to use the mapping when processing theme configurations
• Added comprehensive unit tests (143 lines in utils.test.ts, 394 lines in App.test.tsx)
• Added E2E test specifically validating theme persistence across page reloads

Code Quality

The code quality is excellent and follows Streamlit's best practices:

TypeScript Best Practices:

• ✅ Static THEME_MAPPING constant is defined at module level (not recreated per call) - follows the static data structure pattern in .cursor/rules/typescript.mdc
• ✅ mapCachedThemeToAvailableTheme has explicit return type (ThemeConfig | null)
• ✅ Proper JSDoc documentation with parameter descriptions
• ✅ Clean null handling with early returns
• ✅ Proper use of ?? nullish coalescing

Code Organization:

• The mapping logic is cleanly separated in utils.ts and properly exported through index.ts
• The App.tsx changes are minimal and focused, using the new utility function appropriately in both branches (when custom themes are added and when removed)

Minor Issue:

• In e2e_playwright/theming/custom_light_theme_test.py line 55, there's a minor formatting inconsistency: # [ theme.dark] config should be # [theme.dark] config (extra space after bracket)

Test Coverage

Test coverage is comprehensive and well-structured:

Unit Tests (frontend/lib/src/theme/utils.test.ts):

• Tests for mapCachedThemeToAvailableTheme covering:
  • Null cached theme handling
  • Exact match when theme exists
  • Bidirectional mapping (preset ↔ custom)
  • Edge cases (single custom theme, no suitable match)
• Tests for getDefaultTheme with custom light/dark themes

Integration Tests (frontend/app/src/App.test.tsx):

• Systematic 3×4 matrix covering:
  • Server configs: No custom theme, Single custom theme, Light/Dark custom themes
  • Cache states: No cache, "Light" preset, "Custom Theme", "Custom Theme Light"
• Tests use proper RTL patterns with expect.objectContaining()
• Proper cleanup of localStorage after each test

E2E Test (e2e_playwright/theming/custom_light_theme_test.py):

• ✅ Uses expect_no_skeletons() for stability before assertions
• ✅ Uses stable locators (get_by_test_id, get_by_role, get_by_text)
• ✅ Uses expect for auto-wait assertions (not bare assert)
• ✅ Has descriptive test name and docstring referencing the issue
• ✅ Tests the actual user flow (select Dark theme → reload → verify Dark is still selected)
• ✅ Uses assert_snapshot with appropriate image_threshold
• ✅ Verifies both visual appearance and dropdown selection state

Backwards Compatibility

The changes are fully backwards compatible:

1. Existing cached themes: Users with existing cached preset themes ("Light"/"Dark") will continue to work. The mapping function returns the exact match first.

2. No custom themes: When no custom themes are configured, behavior is unchanged - the code falls through to the existing host-specified theme logic.

3. Graceful degradation: When mapCachedThemeToAvailableTheme returns null, the code falls back to existing default behavior (Custom Theme Auto for light/dark configs, or singular Custom Theme).

4. Theme removal: When custom themes are removed from config, the mapping correctly converts Custom Theme Light → Light and Custom Theme Dark → Dark, preserving user intent.

Security & Risk

No security concerns identified:

• All changes involve theme preference handling in localStorage (client-side only)
• No user input is processed without proper type handling
• No external API calls or data fetching involved

Regression Risk: Low

• The mapping function is pure and well-tested
• Fallback behavior ensures the app works even if mapping fails
• Existing tests for theme handling are updated to reflect new behavior

Recommendations

1. Minor (Optional): Fix the typo in the comment at e2e_playwright/theming/custom_light_theme_test.py:55:

   # [theme.dark] config to test theme persistence on reload

   instead of:

   # [ theme.dark] config to test theme persistence on reload

2. The test coverage is excellent. The systematic 3×4 matrix approach in App.test.tsx is particularly well-designed and provides confidence in the fix.

Verdict

APPROVED: This is a well-implemented fix for a user-facing issue. The code is clean, follows best practices, and has excellent test coverage including both unit tests and an E2E test that directly validates the reported issue. The backwards compatibility is preserved, and there are no security or regression risks.

This is an automated AI review. Please verify the feedback and use your judgment.