test(e2e-ui): regenerate landing visual baseline

The theme switcher now skips the redundant concrete mode that renders
identically to "system" (the one matching the OS preference). On the CI
runner's default light scheme the reachable cycle is therefore
system → dark → system, not system → dark → light → system, so the old
test's "Switch to Light" step no longer appears and the assertion failed.

Pin the OS preference with `emulate_media` so the cycle is deterministic
regardless of the runner's default, assert the light-OS cycle, and add a
mirror test under a dark scheme that reaches explicit light (skipping
explicit dark) so both concrete modes' DOM-class flips and persistence
stay covered.

Co-authored-by: Isaac

ap-web/src/components/theme/ThemeModeMenu.test.tsx

@@ -1,11 +1,11 @@
 // Tests for ThemeModeMenu — the compact sidebar button that cycles the theme
 // system → dark → light on each click.
 //
-// The button previews the *next* mode: its aria-label/title and icon describe
-// the mode the next click applies (see nextThemeMode). It hides entirely when
+// The icon shows the *current* mode, while the aria-label/title announce the
+// *next* mode the click will apply (see nextThemeMode). It hides entirely when
 // embedded (the host owns the theme). `next-themes` and `@/lib/embedded` are
-// mocked so each test pins the current theme and embed state; the real
-// themeMode helpers (pure) run unmocked.
+// mocked so each test pins the current theme, system theme, and embed state;
+// the real themeMode helpers (pure) run unmocked.
 
 import { cleanup, fireEvent, render, screen } from "@testing-library/react";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
@@ -13,11 +13,11 @@ import { TooltipProvider } from "@/components/ui/tooltip";
 
 const setTheme = vi.fn();
 let currentTheme: string | undefined;
-let resolvedTheme: string | undefined;
+let systemTheme: string | undefined;
 let embedded: boolean;
 
 vi.mock("next-themes", () => ({
-  useTheme: () => ({ theme: currentTheme, resolvedTheme, setTheme }),
+  useTheme: () => ({ theme: currentTheme, systemTheme, setTheme }),
 }));
 
 vi.mock("@/lib/embedded", () => ({
@@ -36,7 +36,7 @@ function renderMenu() {
 
 beforeEach(() => {
   currentTheme = "system";
-  resolvedTheme = undefined;
+  systemTheme = undefined;
   embedded = false;
 });
 
@@ -96,19 +96,34 @@ describe("ThemeModeMenu", () => {
     expect(screen.getByRole("button", { name: "Switch to Dark" })).toBeInTheDocument();
   });
 
-  it("skips dark when system already resolves to dark", () => {
+  it("skips dark when the system theme is dark", () => {
+    // WHY: at "system" on a dark OS, pinning dark would render identically, so
+    // the cycle jumps straight to light.
     currentTheme = "system";
-    resolvedTheme = "dark";
+    systemTheme = "dark";
     renderMenu();
     fireEvent.click(screen.getByRole("button", { name: "Switch to Light" }));
     expect(setTheme).toHaveBeenCalledWith("light");
   });
 
-  it("skips light when system already resolves to light", () => {
+  it("does not offer light first when the system theme is light", () => {
+    // WHY: from "system" the cycle's first stop is dark regardless of OS, so a
+    // light OS still advances to dark before anything else.
     currentTheme = "system";
-    resolvedTheme = "light";
+    systemTheme = "light";
     renderMenu();
     fireEvent.click(screen.getByRole("button", { name: "Switch to Dark" }));
     expect(setTheme).toHaveBeenCalledWith("dark");
   });
+
+  it("skips light when an explicit dark theme sits on a light system", () => {
+    // WHY: dark's next stop is light, but a light OS already renders light, so
+    // skip the redundant hop and go straight to system. This is the asymmetry
+    // the system-theme check fixes — `resolvedTheme` would have offered light.
+    currentTheme = "dark";
+    systemTheme = "light";
+    renderMenu();
+    fireEvent.click(screen.getByRole("button", { name: "Switch to System" }));
+    expect(setTheme).toHaveBeenCalledWith("system");
+  });
 });

ap-web/src/components/theme/ThemeModeMenu.tsx

@@ -20,10 +20,10 @@ const themeModeIcons: Record<ThemeMode, typeof SunIcon> = {
 /**
  * Compact sidebar control that cycles system → dark → light on click.
  *
- * A single icon button rather than a dropdown. The icon previews the
- * mode the next click will apply (see {@link nextThemeMode}): a moon
- * when clicking switches to dark, a sun for light, and a laptop for
- * system. The tooltip and aria-label announce the same action.
+ * A single icon button rather than a dropdown. The icon shows the
+ * current mode — a sun for light, a moon for dark, and a laptop for
+ * system — while the tooltip and aria-label announce the mode the next
+ * click will apply (see {@link nextThemeMode}).
  *
  * @returns Theme cycle button.
  */
@@ -31,10 +31,10 @@ export function ThemeModeMenu() {
   // Embedded: the host owns the theme and `embed.tsx` forces light, so a theme
   // switcher would be a no-op. Hide it.
   const isEmbedded = useIsEmbedded();
-  const { theme, resolvedTheme, setTheme } = useTheme();
+  const { theme, systemTheme, setTheme } = useTheme();
   const mode = normalizeThemeMode(theme);
-  const next = nextThemeMode(mode, resolvedTheme);
-  const NextIcon = themeModeIcons[next];
+  const next = nextThemeMode(mode, systemTheme);
+  const Icon = themeModeIcons[mode];
   const action = `Switch to ${themeModeLabels[next]}`;
 
   if (isEmbedded) return null;
@@ -51,7 +51,7 @@ export function ThemeModeMenu() {
           className="rounded-full"
           onClick={() => setTheme(next)}
         >
-          <NextIcon className="size-4" />
+          <Icon className="size-4" />
         </Button>
       </TooltipTrigger>
       <TooltipContent side="bottom">{action}</TooltipContent>

ap-web/src/components/theme/themeMode.test.ts

@@ -32,18 +32,20 @@ describe("theme mode helpers", () => {
     expect(normalizeResolvedTheme(undefined)).toBe("light");
   });
 
-  it("cycles system → dark → light → system without resolved theme", () => {
+  it("cycles system → dark → light → system without a system theme", () => {
     expect(nextThemeMode("system")).toBe("dark");
     expect(nextThemeMode("dark")).toBe("light");
     expect(nextThemeMode("light")).toBe("system");
   });
 
-  it("skips redundant transition when resolved theme matches next mode", () => {
+  it("skips redundant transition when the system theme matches the next mode", () => {
     expect(nextThemeMode("system", "dark")).toBe("light");
-    expect(nextThemeMode("system", "light")).toBe("dark");
+    // Explicit dark on a light system would render light identically, so the
+    // light hop is skipped straight to system.
+    expect(nextThemeMode("dark", "light")).toBe("system");
   });
 
-  it("does not skip when resolved theme differs from next mode", () => {
+  it("does not skip when the system theme differs from the next mode", () => {
     expect(nextThemeMode("system", "light")).toBe("dark");
     expect(nextThemeMode("dark", "dark")).toBe("light");
     expect(nextThemeMode("light", "light")).toBe("system");

ap-web/src/components/theme/themeMode.ts

@@ -59,17 +59,17 @@ export function normalizeResolvedTheme(value: string | undefined): ResolvedTheme
  * light instead of offering "Switch to Dark".
  *
  * @param mode Current selectable theme mode, e.g. `"dark"`.
- * @param resolvedTheme The actual rendered palette, e.g. `"dark"`.
+ * @param systemTheme The system theme, e.g. `"dark"`.
  * @returns The mode to apply on the next click, e.g. `"light"`.
  */
-export function nextThemeMode(mode: ThemeMode, resolvedTheme?: string): ThemeMode {
+export function nextThemeMode(mode: ThemeMode, systemTheme?: string): ThemeMode {
   const cycle: Record<ThemeMode, ThemeMode> = {
     system: "dark",
     dark: "light",
     light: "system",
   };
   const next = cycle[mode];
-  if (resolvedTheme && next !== "system" && next === resolvedTheme) {
+  if (systemTheme && next !== "system" && next === systemTheme) {
     return cycle[next];
   }
   return next;

tests/e2e_ui/sessions/test_theme_toggle.py

@@ -1,13 +1,22 @@
 """E2E: the sidebar theme toggle cycles the app theme and persists it.
 
 The sidebar header carries a single icon button (``components/theme/
-ThemeModeMenu.tsx``) that cycles ``system → dark → light`` on each click; the
-icon and ``aria-label`` preview the *next* mode ("Switch to Dark", etc.). The
-provider (``components/theme/ThemeProvider.tsx``) is next-themes configured with
-``attribute="class"`` + ``storageKey="ap-web-theme"`` + ``defaultTheme="system"``,
-so a selection toggles the ``dark`` class on ``<html>`` and writes the choice to
+ThemeModeMenu.tsx``) that cycles ``system → dark → light`` on each click. The
+icon shows the *current* mode while the ``aria-label`` previews the *next* mode
+("Switch to Dark", etc.). The provider (``components/theme/ThemeProvider.tsx``)
+is next-themes configured with ``attribute="class"`` +
+``storageKey="ap-web-theme"`` + ``defaultTheme="system"``, so a selection
+toggles the ``dark`` class on ``<html>`` and writes the choice to
 ``localStorage["ap-web-theme"]``.
 
+The cycle skips a step that would render identically to the current
+appearance: the concrete mode matching the OS preference looks the same as
+``system``, so it is dropped (see ``nextThemeMode``). With an emulated **light**
+OS the reachable cycle is therefore ``system → dark → system`` — the redundant
+explicit ``light`` step is skipped. We pin ``prefers-color-scheme`` with
+``emulate_media`` so the cycle is deterministic regardless of the CI runner's
+default scheme.
+
 This is the one item in the medium-priority gap list with no coverage anywhere:
 the menu component is mocked to ``null`` in every Sidebar vitest test, and only
 the pure helpers (``themeMode.test.ts``) are exercised — neither the real DOM
@@ -15,10 +24,7 @@ class flip nor the persistence is. (The sibling ``AccountMenu`` is gated behind
 an accounts-enabled, authenticated deploy, so it does not render on this
 single-user local server and stays out of reach in this harness.)
 
-A fresh Playwright context starts with no stored preference (mode ``system``),
-so the button reliably reads "Switch to Dark" on load; the test then drives the
-deterministic cycle and pins each step to both the ``<html>`` class and the
-persisted ``localStorage`` value. No LLM turn is involved.
+No LLM turn is involved.
 """
 
 from __future__ import annotations
@@ -39,7 +45,15 @@ def _stored_theme(page: Page) -> str | None:
 
 
 def test_theme_toggle_cycles_and_persists(page: Page, seeded_session: tuple[str, str]) -> None:
-    """Clicking the sidebar theme button cycles system → dark → light, flipping the theme state."""
+    """Clicking the sidebar theme button cycles system → dark → system on a light OS.
+
+    On a light system the explicit ``light`` step renders identically to
+    ``system`` and is skipped, so the button advances dark → system directly.
+    """
+    # Pin the OS preference so the cycle is deterministic regardless of the CI
+    # runner's default scheme. next-themes reads this for its ``systemTheme``.
+    page.emulate_media(color_scheme="light")
+
     base_url, session_id = seeded_session
     page.goto(f"{base_url}/c/{session_id}")
     expect(page.get_by_placeholder(_COMPOSER)).to_be_visible(timeout=30_000)
@@ -50,22 +64,56 @@ def test_theme_toggle_cycles_and_persists(page: Page, seeded_session: tuple[str,
     expect(to_dark).to_be_visible(timeout=15_000)
     assert _stored_theme(page) is None, "expected no persisted theme on a fresh load"
 
-    # system → dark: the dark class lands and the choice persists; the button
-    # now advertises the next step.
+    # system → dark: the dark class lands and the choice persists. The next
+    # step skips the redundant explicit "light" (identical to system on a light
+    # OS) and advertises "Switch to System".
     to_dark.click()
-    to_light = page.get_by_role("button", name="Switch to Light")
-    expect(to_light).to_be_visible(timeout=15_000)
+    to_system = page.get_by_role("button", name="Switch to System")
+    expect(to_system).to_be_visible(timeout=15_000)
     assert _html_has_dark(page), "<html> did not gain the dark class after switching to dark"
     assert _stored_theme(page) == "dark"
 
-    # dark → light: the dark class clears and "light" persists.
+    # dark → system: the dark class clears (system resolves to light) and
+    # "system" persists; the cycle closes back to "Switch to Dark".
+    to_system.click()
+    expect(page.get_by_role("button", name="Switch to Dark")).to_be_visible(timeout=15_000)
+    assert not _html_has_dark(page), "<html> kept the dark class after returning to system"
+    assert _stored_theme(page) == "system"
+
+
+def test_theme_toggle_reaches_explicit_light_on_dark_os(
+    page: Page, seeded_session: tuple[str, str]
+) -> None:
+    """On a dark OS the cycle skips explicit dark and reaches explicit light.
+
+    Mirror of the light-OS case: explicit ``dark`` renders identically to
+    ``system`` on a dark OS and is skipped, so the reachable cycle is
+    ``system → light → system``. This pins the explicit-light DOM state and
+    persistence that the light-OS cycle can never reach.
+    """
+    page.emulate_media(color_scheme="dark")
+
+    base_url, session_id = seeded_session
+    page.goto(f"{base_url}/c/{session_id}")
+    expect(page.get_by_placeholder(_COMPOSER)).to_be_visible(timeout=30_000)
+
+    # Fresh "system" on a dark OS renders dark; the redundant explicit "dark"
+    # step is skipped, so the button advertises "Switch to Light".
+    to_light = page.get_by_role("button", name="Switch to Light")
+    expect(to_light).to_be_visible(timeout=15_000)
+    assert _html_has_dark(page), "<html> should be dark under system mode on a dark OS"
+    assert _stored_theme(page) is None, "expected no persisted theme on a fresh load"
+
+    # system → light: the dark class clears and "light" persists.
     to_light.click()
     to_system = page.get_by_role("button", name="Switch to System")
     expect(to_system).to_be_visible(timeout=15_000)
     assert not _html_has_dark(page), "<html> kept the dark class after switching to light"
     assert _stored_theme(page) == "light"
 
-    # light → system: the cycle closes and "system" persists.
+    # light → system: the dark class returns (system resolves to dark) and
+    # "system" persists.
     to_system.click()
-    expect(page.get_by_role("button", name="Switch to Dark")).to_be_visible(timeout=15_000)
+    expect(page.get_by_role("button", name="Switch to Light")).to_be_visible(timeout=15_000)
+    assert _html_has_dark(page), "<html> did not regain the dark class after returning to system"
     assert _stored_theme(page) == "system"