Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Dec 22, 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 worksrequestImmediate() bypasses pause, hidden, and min-interval checks
  • Loop prevention — 500ms minimum interval between refreshes (scheduled events only, not manual)
  • Trigger trackingLastRefreshInfo 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

Generated with mux • Model: anthropic:claude-opus-4-5 • Thinking: high

@ammar-agent ammar-agent changed the title 🤖 perf: 1s debounce for active workspace git status 🤖 refactor: consolidate refresh logic into RefreshController Dec 23, 2025
@ammar-agent ammar-agent force-pushed the git-debounce-5w4e branch 12 times, most recently from 167e1ce to f605635 Compare December 23, 2025 02:53
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.
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
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.
@ammar-agent ammar-agent changed the title 🤖 refactor: consolidate refresh logic into RefreshController 🤖 fix: make code review diff refresh robust and predictable Dec 23, 2025
@ammario ammario enabled auto-merge December 23, 2025 17:57
@ammario ammario added this pull request to the merge queue Dec 23, 2025
Merged via the queue into main with commit 4a3de01 Dec 23, 2025
20 checks passed
@ammario ammario deleted the git-debounce-5w4e branch December 23, 2025 18:22
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 <tk@coder.com>

---
_Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking:
`high`_
ammario added a commit that referenced this pull request Dec 23, 2025
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.

2 participants