-
Notifications
You must be signed in to change notification settings - Fork 32
🤖 fix: make code review diff refresh robust and predictable #1296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+779
−266
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ebab3e7 to
2ec7a60
Compare
167e1ce to
f605635
Compare
Active workspace now uses 1s debounce for git status refresh after file-modifying tools complete, while background workspaces keep 3s. This makes the visible git indicators feel more responsive without adding load from background workspace refreshes.
Adds debugging visibility to ReviewPanel's refresh button tooltip: - Shows how long ago the last refresh occurred (e.g., '5s ago', '2m ago') - Shows what triggered the refresh (manual click, tool completion, focus, etc.) - Persisted per-workspace so it survives workspace switches RefreshController now uses rate-limiting instead of debouncing: - First event starts timer; subsequent events don't reset it - Guarantees refresh fires within N seconds of first trigger - Events during in-flight refresh queue a follow-up after completion - Better UX during rapid tool calls (refreshes periodically vs never) Fixes controller recreation bug: onRefresh callback was causing useMemo to recreate the controller on every render, disposing active timers. Now uses refs for callbacks to maintain stable dependencies. Debug logging improvements: - RefreshController accepts debugLabel for human-readable logs - WorkspaceStore.getWorkspaceName() helper for log-friendly workspace names - Logs now show workspace names instead of opaque IDs Adds tests for file-modifying tool subscription mechanism.
This reverts commit 43e6652.
Prevents refresh loops by construction - even if other guards fail, executeRefresh() refuses to run if less than 500ms since last start.
- Enforce a 500ms minimum interval between refresh starts without dropping requests - Manual refresh always bypasses pause/hidden by construction - Remove always-on debug log spam from review refresh hook - Update RefreshController tests for new behavior
- Manual refresh now bumps refreshTrigger immediately (no blocking fetch) - Move origin/* git fetch into diff/tree load with GIT_TERMINAL_PROMPT=0 - Share origin fetch across file tree + diff via a keyed promise ref
73c20b2 to
112544a
Compare
The MIN_REFRESH_INTERVAL_MS (500ms) guard was blocking the second requestImmediate() call in GitStatusStore's initialization flow: 1. syncWorkspaces() calls requestImmediate() - runs but no subscribers yet 2. Components subscribe via subscribeKey() which queues another requestImmediate() 3. Second requestImmediate() was delayed 500ms due to MIN_REFRESH_INTERVAL This caused storybook tests to fail since git status data wasn't fetched until after the 500ms cooldown, even though the mock returns instantly. Fix: Manual/immediate refresh now bypasses MIN_REFRESH_INTERVAL check, just like it already bypasses pause and hidden checks. The interval guard is meant to prevent loops from scheduled events, not explicit user/component requests.
ThomasK33
pushed a commit
that referenced
this pull request
Dec 23, 2025
Makes the Code Review "diff refresh" mechanism robust and predictable, driven by file-modifying tool-call events (`bash`, `file_edit_*`) rather than polling or filesystem watchers. ## Key Changes ### RefreshController improvements - **Coalescing rate-limit** instead of resetting debounce — prevents starvation when tool events are frequent - **Manual refresh always works** — `requestImmediate()` bypasses pause, hidden, and min-interval checks - **Loop prevention** — 500ms minimum interval between refreshes (scheduled events only, not manual) - **Trigger tracking** — `LastRefreshInfo` records what caused each refresh ### Review panel refresh - **Non-blocking manual refresh** — immediately bumps `refreshTrigger`, origin fetch runs in background - **Origin fetch deduplication** — file tree and diff loads share the same `git fetch` call - **Refresh tooltip** — shows "Last: 2m ago via manual" for debugging ### GitStatusStore - **Active workspace priority** — 1s debounce vs 3s for background workspaces - **File modification subscription** — refreshes on tool completion events ## Bug Fixes - Fixed storybook test failure: `requestImmediate()` was blocked by MIN_REFRESH_INTERVAL when components subscribed immediately after `syncWorkspaces()` — now bypasses interval check for manual/immediate requests Signed-off-by: Thomas Kosiewski <[email protected]> --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes the Code Review "diff refresh" mechanism robust and predictable, driven by file-modifying tool-call events (
bash,file_edit_*) rather than polling or filesystem watchers.Key Changes
RefreshController improvements
requestImmediate()bypasses pause, hidden, and min-interval checksLastRefreshInforecords what caused each refreshReview panel refresh
refreshTrigger, origin fetch runs in backgroundgit fetchcallGitStatusStore
Bug Fixes
requestImmediate()was blocked by MIN_REFRESH_INTERVAL when components subscribed immediately aftersyncWorkspaces()— now bypasses interval check for manual/immediate requestsGenerated with
mux• Model:anthropic:claude-opus-4-5• Thinking:high