Preserve auto theme across watch refreshes#318
Conversation
Greptile SummaryThis PR fixes a watch-mode regression where
Confidence Score: 4/5Safe 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
Sequence DiagramsequenceDiagram
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) ✗
Prompt To Fix All With AIFix 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 |
Summary
--watchrefreshestheme: autofrom falling back to the default dark theme after watched file diffs reloadSolves 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
--watchmode, file changes trigger a soft reload: the diff bootstrap is replaced, but theAppcomponent is intentionally not remounted so view state like selected theme, layout, and scroll position can survive. That meantthemeIdstayed 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 lightPapertheme to darkGraphiteafter the first watched refresh.This fix snapshots the startup-detected theme mode for the lifetime of the mounted app, so soft reloads keep resolving
autoconsistently.