Review — system appearance theme selection

Solid, well-structured feature. CI is green and it's cleanly mergeable. The migration logic is careful and the per-slot model is implemented consistently across SettingsContext and SettingsTab. No crashes or blocking bugs found, but two real behavioral issues and a couple of nits are worth addressing before merge.

What it does well

• Clean state model. appearanceMode: 'system' | 'dark' | 'light' + darkTheme/lightTheme slots, with the legacy theme key retained as the effective theme for backward compat. New keys are correctly added to VALID_SETTINGS_KEYS (the silent-save trap is avoided).
• Migration matches the stated behavior and is mirrored in both the localStorage initializer (getInitialThemePreferences) and the server-load path: new users → system/mocha/latte; mocha users → same; non-mocha users → explicit dark with their theme in both slots.
• System listener is correct — matchMedia('(prefers-color-scheme: dark)') with proper addEventListener/cleanup, and a separate effect recomputes the effective theme. No render loop (the apply-effect depends on appearanceMode/darkTheme/lightTheme/systemIsDark, not on theme, which is what applyTheme mutates).
• SettingsTab wiring is complete — local state, the handleSave dependency array, and reset-to-defaults are all updated; the dark/light selects share renderThemeOptions().

Issues

1. Theme gallery (ThemeDocumentation.tsx) silently breaks the new model — main concern.
It still calls the legacy setTheme(id), which this PR redefined to force appearanceMode='dark' and overwrite both slots:

const setTheme = (newTheme) => {
  setAppearanceModeState('dark');
  setDarkThemeState(newTheme);
  setLightThemeState(newTheme);  // clobbers the user's light slot
  ...
}

So a user in system mode who clicks any card in the theme gallery is silently kicked into explicit dark mode and loses their separate light theme; picking a light theme there lands it in the dark slot. This contradicts the PR's own design principle for the custom-theme cards ("buttons update only the corresponding slot and do not change appearance mode"). Suggest giving the gallery Apply-as-Dark/Light affordances, or at minimum not flipping the mode — or explicitly scoping it out.

2. onThemeChange is now a dead prop. SettingsTab removed it from destructuring and replaced it with direct context setters, but it's still declared in SettingsTabProps (line 84) and still passed by App.tsx:4976 and GlobalSettingsPage.tsx:132. Harmless, but worth removing to avoid confusion.

Nits

• Migration asymmetry: the localStorage path derives appearanceMode/lightTheme without the !hasNewThemePreferences guard the server-load path uses. Only reachable with partial state (slot keys present but appearanceMode missing), which never happens since they're written together — defensive, not a real bug, but the two paths would ideally share a helper.
• getInitialThemePreferences() writes to localStorage inside a useState initializer (a side effect in render). It's idempotent and matches the existing pattern in this file, so acceptable.
• Good test coverage on the custom-theme-card slot behavior; nothing covers the gallery/setTheme mode-flip in issue #1.

Recommendation

Approve pending #1 (genuine UX regression) and ideally #2 cleanup. Everything else is polish.

🤖 Generated with Claude Code