Skip to content

Preserve auto theme across watch refreshes#318

Open
dive wants to merge 3 commits into
modem-dev:mainfrom
dive:fix/preserve-auto-theme-watch-refresh
Open

Preserve auto theme across watch refreshes#318
dive wants to merge 3 commits into
modem-dev:mainfrom
dive:fix/preserve-auto-theme-watch-refresh

Conversation

@dive
Copy link
Copy Markdown

@dive dive commented May 15, 2026

Summary

  • Preserve the startup-detected terminal theme mode across soft --watch refreshes
  • Keep theme: auto from falling back to the default dark theme after watched file diffs reload

Solves the bug

When Hunk started with --theme auto, startup detected the terminal background once & stored the resolved terminal theme mode on the initial app bootstrap.

In --watch mode, file changes trigger a soft reload: the diff bootstrap is replaced, but the App component is intentionally not remounted so view state like selected theme, layout, and scroll position can survive. That meant themeId stayed as "auto", but the replacement bootstrap no longer had the startup-only detected terminal theme mode.

As a result, resolveTheme("auto", null) fell back to the default dark theme, so users on light terminals could see Hunk switch from the light Paper theme to dark Graphite after the first watched refresh.

This fix snapshots the startup-detected theme mode for the lifetime of the mounted app, so soft reloads keep resolving auto consistently.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR fixes a watch-mode regression where --theme auto would fall back to the default dark theme after the first file-change refresh. The root cause was that soft reloads replace the bootstrap prop without remounting App, so bootstrap.initialThemeMode (set only at startup from terminal detection) was no longer available on re-renders.

  • App.tsx: snapshots bootstrap.initialThemeMode once at mount via useRef so subsequent re-renders caused by watch refreshes keep resolving \"auto\" to the correct light/dark mode.
  • AppHost.interactions.test.tsx: adds a regression test that writes a new file in watch mode, waits for the diff to reload, then verifies the Theme menu still shows Paper (the expected light theme) as active.

Confidence Score: 4/5

Safe to merge — the core fix is logically correct and the regression test covers the exact failure scenario.

The watch-mode theme snapshot works correctly: useRef returns the same object across re-renders so .current is permanently the mount-time value, and resolveTheme is called with the right mode after every soft reload. The two findings are minor — an unconventional React idiom for capturing initial state and a test retry loop that could misfire if F10 renders slowly — neither affects production behaviour.

The retry loop in openThemeMenu inside AppHost.interactions.test.tsx is worth a second look for potential flakiness.

Important Files Changed

Filename Overview
src/ui/App.tsx Snapshots bootstrap.initialThemeMode via useRef on mount so soft reloads can't clobber the resolved auto theme; fix is logically correct, though the idiom is non-standard.
src/ui/AppHost.interactions.test.tsx Adds a regression test for the watch-mode theme preservation; the openThemeMenu helper has a retry loop that can be counterproductive when F10 is a toggle key and the menu renders slower than the polling window.
CHANGELOG.md Adds a changelog entry for the auto-theme preservation fix.

Sequence Diagram

sequenceDiagram
    participant FS as File System
    participant Watch as Watch Loop
    participant AppHost as AppHost
    participant App as App (mounted once)
    participant Resolver as resolveTheme()

    Note over App: Mount — useRef snapshots bootstrap.initialThemeMode ("light")

    FS->>Watch: file change detected
    Watch->>AppHost: trigger soft reload
    AppHost->>App: "re-render with new bootstrap (initialThemeMode = undefined)"

    App->>App: "detectedThemeMode = ref.current ("light" — preserved from mount)"
    App->>Resolver: resolveTheme("auto", "light")
    Resolver-->>App: Paper (light theme) ✓

    Note over App: Before fix: resolveTheme("auto", null) → Graphite (dark theme fallback) ✗
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/ui/App.tsx:110-111
Using `useRef(x).current` to capture an initial prop value is non-idiomatic — `useRef` is designed for mutable boxes, and reading `.current` immediately drops the ref handle, leaving no way to update or reset it intentionally. `useState` expresses "initialise once, never change" more clearly and has the same runtime semantics here.

```suggestion
  // Soft reloads replace bootstrap without re-running startup terminal theme detection.
  const [detectedThemeMode] = useState(bootstrap.initialThemeMode);
```

### Issue 2 of 2
src/ui/AppHost.interactions.test.tsx:441-453
**Retry loop is counterproductive when F10 is a toggle key.** `waitForFrame` doesn't throw on timeout — it returns the last polled frame regardless of whether the predicate matched (default 8 attempts × 30 ms ≈ 240 ms). If the menu opened on the first F10 press but rendered just after the polling window closed, `menuFrame` won't contain `"Toggle files/filter focus"`, the loop iterates, and the second F10 press closes the menu that had just opened. `opened` stays `false`, and `expect(opened).toBe(true)` then fails. Passing a higher attempt count to the first `waitForFrame` call (like the 12 used for the final theme-label check) would remove the need for the outer retry entirely.

Reviews (1): Last reviewed commit: "fix(ui): preserve auto theme across watc..." | Re-trigger Greptile

Comment thread src/ui/App.tsx Outdated
Comment thread src/ui/AppHost.interactions.test.tsx Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant