Refresh working tree diff after turns#3906
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces a new caching mechanism for diff previews that changes runtime behavior across server and client. The three open review comments raise valid design concerns about potential edge cases where the cache key may not properly invalidate stale diffs. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68f23c90ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| workingTree: gitStatusQuery.data?.workingTree ?? null, | ||
| }); |
There was a problem hiding this comment.
When the current ref changes while the working tree summary is unchanged, this cache key does not change even though getReviewDiffPreview recomputes the branch-range diff from the current HEAD/branch. For example, switching between two clean branches leaves workingTree as an empty summary and latestTurnId unchanged, so the branch diff panel can keep reusing the previous ${baseRef}...HEAD preview. Include ref/HEAD-relevant status (or a VCS generation) in the cache key, not just the working-tree summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 601d479ebe9a522c2bdff2ba0ba3f3e094b19044.
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| workingTree: gitStatusQuery.data?.workingTree ?? null, | ||
| }); |
There was a problem hiding this comment.
When an already-listed changed file is edited without changing its path or numstat summary, this key stays identical while the patch returned by getReviewDiffPreview changes. This is especially easy for untracked files because the status summary records them as the same path with 0 insertions/deletions, so editing the file contents after the preview is cached leaves the Working tree panel rendering the old patch until something else changes the key. Use a content/diff generation or refreshed diff hash rather than only the lossy workingTree summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 601d479ebe9a522c2bdff2ba0ba3f3e094b19044.
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate 601d479
github-actions
Bot
added
size:M
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 601d479ebe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const latestTurn = orderedTurnDiffSummaries[0]; | ||
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| localGeneration: gitStatusQuery.data?.localGeneration ?? null, |
There was a problem hiding this comment.
When the branch preview is based on a remote ref, a fetch can move that base ref while the working tree and HEAD stay unchanged; VcsStatusBroadcaster.updateCachedRemoteStatus publishes only remoteUpdated, and applyGitStatusStreamEvent preserves the old localGeneration, so this cache key stays identical even though getReviewDiffPreview recomputes ${baseRef}...HEAD against the new ref. Include a remote generation/fingerprint in the key for branch-range previews so the panel refreshes after remote status updates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in fe250edfc44c7333230992e613175367d91442d5.
github-actions
Bot
added
size:L
1 participant
What changed
Why
The Working tree diff preview could reuse a cached response after a later agent turn changed the repository. This was especially visible when a file created in one turn was deleted in the next: Git correctly omitted the deleted untracked file from a fresh preview, but the UI could continue rendering the earlier cached patch.
The updated cache identity selects a fresh preview after completed turns and working-tree status changes, while keeping the refresh declarative in the query layer.
Impact
Working tree and branch diff views now reflect the latest repository state after agent turns and Git status changes instead of briefly showing a stale patch.
Validation
vp test packages/client-runtime/src/state/runtime.test.ts apps/server/src/vcs/GitVcsDriverCore.test.tsvp checkvp run typecheckNote
Medium Risk
Touches VCS streaming semantics (more
localUpdatedevents) and review UI caching; behavior is well-tested but clients depending on deduplicated local updates may see extra refetches.Overview
Fixes stale working-tree and branch diff previews after agent turns or repo changes by tying client query cache identity to live VCS signals, not only RPC inputs.
localGenerationincrements on every local status refresh inVcsStatusBroadcasterand flows through stream events andVcsStatusResult;localUpdatedis published whenever a refresh is published, even if the local summary is unchanged.remoteRefHash(SHA-256 ofrefs/remotes) is added to remote status so fetches that move tracking refs invalidate cache without relying on ahead/behind alone.DiffPanelbuilds adiffPreviewCacheKeyfrom latest turn id,localGeneration, andremoteRefHashand passes it to environment diff preview queries.environmentRpcKeyaccepts an optional client-onlycacheKeyso the same RPC input can map to separate cached results.Regression tests cover untracked-file deletion in review previews, remote ref hash movement, unchanged local status still bumping generation, and cache key isolation.
Reviewed by Cursor Bugbot for commit fe250ed. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Refresh working tree diff after turns by tracking local and remote ref changes
localGenerationcounter to VCS status that increments on every local status update, replacing fingerprint-based deduplication inVcsStatusBroadcaster.remoteRefHashto remote status by hashing all remote-tracking refs viagit for-each-ref; the hash changes when any remote ref moves.diffPreviewCacheKeyinDiffPanelcomposed fromlatestTurnId,localGeneration, andremoteRefHash, so diff preview queries re-execute after each turn or remote ref change.environmentRpcKeyinruntime.tswith an optionalcacheKeyto isolate environment queries when the cache key differs.localUpdatedevents than before.Macroscope summarized fe250ed.