Add diff scope switching and provider update settings#3169
Open
juliusmarminge wants to merge 4 commits into
Open
Add diff scope switching and provider update settings#3169juliusmarminge wants to merge 4 commits into
juliusmarminge wants to merge 4 commits into
Conversation
- Replace the turn strip with a scope menu - Support unstaged diff routing and parsing - Update diff header skeleton and tests
|
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)
Comment |
github-actions
Bot
added
vouch:trusted
Contributor
|
🚀 Expo continuous deployment is ready!
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Whitespace toggle ignores git scopes
- Added ignoreWhitespace to ReviewDiffPreviewInput schema, passed -w flag in server git diff commands when enabled, and included the setting in the client's diffPreview query so the toggle triggers a refetch with whitespace filtering.
- ✅ Fixed: Unstaged label shows worktree diff
- Renamed all user-facing 'Unstaged' labels to 'Working tree' to accurately describe the scope which includes staged, unstaged, and untracked changes compared to HEAD.
- ✅ Fixed: Truncated git diffs unhandled
- Added extraction of the source's truncated flag, stripping of the [truncated] marker from diff text before parsing, and a visible warning banner when a truncated source is displayed.
- ✅ Fixed: Diff list drops virtualizer
- Re-added the Virtualizer import from @pierre/diffs/react and restored the Virtualizer wrapper with overscrollSize and intersectionObserverMargin config around the file diff list.
Or push these changes by commenting:
@cursor push 2fee8036e0
Preview (2fee8036e0)
diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts
--- a/apps/server/src/vcs/GitVcsDriverCore.ts
+++ b/apps/server/src/vcs/GitVcsDriverCore.ts
@@ -1814,10 +1814,12 @@
)
: null);
+ const ignoreWsArgs = input.ignoreWhitespace ? ["-w"] : [];
+
const dirtyTrackedResult = yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.dirtyTracked",
input.cwd,
- ["diff", "--patch", "--minimal", "HEAD", "--"],
+ ["diff", "--patch", "--minimal", ...ignoreWsArgs, "HEAD", "--"],
{
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
@@ -1843,7 +1845,7 @@
? yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.base",
input.cwd,
- ["diff", "--patch", "--minimal", `${baseRef}...HEAD`],
+ ["diff", "--patch", "--minimal", ...ignoreWsArgs, `${baseRef}...HEAD`],
{
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx
--- a/apps/web/src/components/DiffPanel.tsx
+++ b/apps/web/src/components/DiffPanel.tsx
@@ -1,4 +1,5 @@
import { useAtomValue } from "@effect/atom-react";
+import { Virtualizer } from "@pierre/diffs/react";
import { useNavigate, useParams, useSearch } from "@tanstack/react-router";
import { scopeThreadRef } from "@t3tools/client-runtime/environment";
import {
@@ -247,7 +248,7 @@
const selectedScopeLabel =
selectedTurnId === null
? selectedGitScope === "unstaged"
- ? "Unstaged"
+ ? "Working tree"
: "Branch changes"
: selectedTurn?.turnId === latestTurn?.turnId
? "Latest turn"
@@ -263,7 +264,7 @@
const reviewSectionTitle = selectedTurn
? `Turn ${selectedCheckpointTurnCount ?? "?"}`
: selectedGitScope === "unstaged"
- ? "Unstaged"
+ ? "Working tree"
: "Branch changes";
const selectedCheckpointRange = useMemo(
() =>
@@ -290,7 +291,7 @@
selectedTurnId === null && activeThread && activeCwd
? reviewEnvironment.diffPreview({
environmentId: activeThread.environmentId,
- input: { cwd: activeCwd },
+ input: { cwd: activeCwd, ignoreWhitespace: diffIgnoreWhitespace || undefined },
})
: null,
);
@@ -303,18 +304,24 @@
shouldRetryBranchDiffAtEnvironmentCwd && activeThread && serverConfig
? reviewEnvironment.diffPreview({
environmentId: activeThread.environmentId,
- input: { cwd: serverConfig.cwd },
+ input: { cwd: serverConfig.cwd, ignoreWhitespace: diffIgnoreWhitespace || undefined },
})
: null,
);
const branchDiffPreview = shouldRetryBranchDiffAtEnvironmentCwd
? fallbackBranchDiffPreview
: primaryBranchDiffPreview;
- const gitDiff = branchDiffPreview.data?.sources.find(
+ const gitDiffSource = branchDiffPreview.data?.sources.find(
(source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"),
- )?.diff;
+ );
+ const gitDiff = gitDiffSource?.diff;
+ const gitDiffTruncated = gitDiffSource?.truncated ?? false;
- const selectedPatch = selectedTurn ? activeCheckpointDiff.data?.diff : gitDiff;
+ const selectedPatch = selectedTurn
+ ? activeCheckpointDiff.data?.diff
+ : gitDiff != null
+ ? gitDiff.replace(/\n*\[truncated\]\s*$/, "")
+ : gitDiff;
const isLoadingSelectedPatch = selectedTurn
? activeCheckpointDiff.isPending
: branchDiffPreview.isPending;
@@ -424,7 +431,7 @@
</DropdownMenuTrigger>
<DropdownMenuContent align="start" className="w-60">
<DropdownMenuItem onClick={() => selectGitScope("unstaged")}>
- <span>Unstaged</span>
+ <span>Working tree</span>
{selectedTurnId === null && selectedGitScope === "unstaged" && (
<CheckIcon className="ml-auto" />
)}
@@ -564,6 +571,13 @@
<p className="mb-2 text-[11px] text-red-500/80">{selectedPatchError}</p>
</div>
)}
+ {gitDiffTruncated && !selectedTurn && renderablePatch && (
+ <div className="border-b border-amber-200 bg-amber-50 px-3 py-1.5 dark:border-amber-900/60 dark:bg-amber-950/40">
+ <p className="text-[11px] text-amber-700 dark:text-amber-300">
+ Diff output was truncated by the server. Showing available excerpt.
+ </p>
+ </div>
+ )}
{!renderablePatch ? (
isLoadingSelectedPatch ? (
<DiffPanelLoadingState
@@ -571,7 +585,7 @@
selectedTurn
? "Loading checkpoint diff..."
: selectedGitScope === "unstaged"
- ? "Loading unstaged diff..."
+ ? "Loading working tree diff..."
: "Loading branch diff..."
}
/>
@@ -585,9 +599,13 @@
</div>
)
) : renderablePatch.kind === "files" ? (
- <div
+ <Virtualizer
key={collapseScopeKey ?? reviewSectionId}
className="diff-render-surface h-full min-h-0 overflow-auto px-2 pb-2"
+ config={{
+ overscrollSize: 600,
+ intersectionObserverMargin: 1200,
+ }}
>
{renderableFiles.map((fileDiff) => {
const filePath = resolveFileDiffPath(fileDiff);
@@ -661,7 +679,7 @@
</div>
);
})}
- </div>
+ </Virtualizer>
) : (
<div className="h-full overflow-auto p-2">
<div className="space-y-2">
diff --git a/packages/contracts/src/review.ts b/packages/contracts/src/review.ts
--- a/packages/contracts/src/review.ts
+++ b/packages/contracts/src/review.ts
@@ -6,6 +6,7 @@
export const ReviewDiffPreviewInput = Schema.Struct({
cwd: TrimmedNonEmptyString,
baseRef: Schema.optional(TrimmedNonEmptyString),
+ ignoreWhitespace: Schema.optional(Schema.Boolean),
});
export type ReviewDiffPreviewInput = typeof ReviewDiffPreviewInput.Type;You can send follow-ups to the cloud agent here.
| : primaryBranchDiffPreview; | ||
| const gitDiff = branchDiffPreview.data?.sources.find( | ||
| (source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"), | ||
| )?.diff; |
Contributor
There was a problem hiding this comment.
Unstaged label shows worktree diff
Medium Severity
The “Unstaged” scope and diffScope=unstaged route state select the working-tree preview source, which elsewhere represents dirty worktree changes (staged, tracked, and untracked), not git unstaged-only diffs. Users can believe they are reviewing unstaged files while seeing a broader worktree patch.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d134318. Configure here.
Contributor
ApprovabilityVerdict: Needs human review This PR introduces significant new user-facing functionality (diff scope switching, base ref selection) with substantial UI and state management changes. Multiple unresolved review comments identify potential bugs including state mismatches and missing feature parity that should be addressed before merging. You can customize Macroscope's approvability policy. Learn more. |
- Add diff base-ref routing and parsing - Pair local and remote refs in the branch picker - Update git ref listing and diff rendering for the new selector
github-actions
Bot
added
size:XL
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 6 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Scope switch clears base ref
- The selectGitScope function now extracts the previous diffBaseRef via parseDiffRouteSearch before stripping params and re-adds it when scope is "branch".
- ✅ Fixed: Turn file scroll target missing
- The scroll useEffect now queries [data-title] elements and matches by textContent (consistent with the click handler) instead of the non-existent [data-diff-file-path] attribute.
- ✅ Fixed: Base ref search shows stale
- filteredBaseRefItems now applies a client-side .filter() on choice.label against the baseRefQuery so stale/unrelated refs are hidden while refetches are in flight.
Or push these changes by commenting:
@cursor push 6ea65f1c5b
Preview (6ea65f1c5b)
diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx
--- a/apps/web/src/components/DiffPanel.tsx
+++ b/apps/web/src/components/DiffPanel.tsx
@@ -380,7 +380,13 @@
const baseRefItems = [AUTOMATIC_BASE_REF, ...baseRefChoices.map(valueForBaseRefChoice)];
const filteredBaseRefItems = [
...(baseRefQuery.trim().length === 0 ? [AUTOMATIC_BASE_REF] : []),
- ...baseRefChoices.map(valueForBaseRefChoice),
+ ...baseRefChoices
+ .filter(
+ (choice) =>
+ baseRefQuery.trim().length === 0 ||
+ choice.label.toLowerCase().includes(baseRefQuery.trim().toLowerCase()),
+ )
+ .map(valueForBaseRefChoice),
];
const gitDiff = selectedGitSource?.diff;
@@ -436,8 +442,8 @@
return;
}
const target = Array.from(
- patchViewportRef.current.querySelectorAll<HTMLElement>("[data-diff-file-path]"),
- ).find((element) => element.dataset.diffFilePath === selectedFilePath);
+ patchViewportRef.current.querySelectorAll<HTMLElement>("[data-title]"),
+ ).find((element) => element.textContent?.trim() === selectedFilePath);
target?.scrollIntoView({ block: "nearest" });
}, [selectedFilePath, renderableFiles]);
@@ -491,8 +497,15 @@
to: "/$environmentId/$threadId",
params: buildThreadRouteParams(scopeThreadRef(activeThread.environmentId, activeThread.id)),
search: (previous) => {
+ const prevBaseRef =
+ scope === "branch" ? parseDiffRouteSearch(previous).diffBaseRef : undefined;
const rest = stripDiffSearchParams(previous);
- return { ...rest, diff: "1", ...(scope === "unstaged" ? { diffScope: scope } : {}) };
+ return {
+ ...rest,
+ diff: "1",
+ ...(scope === "unstaged" ? { diffScope: scope } : {}),
+ ...(prevBaseRef ? { diffBaseRef: prevBaseRef } : {}),
+ };
},
});
};You can send follow-ups to the cloud agent here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- Store diff turn, scope, and file selection per thread - Remove diff state from route search params - Keep diff file reveal behavior stable across panel updates
github-actions
Bot
added
size:XXL
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale turn id loads wrong diff
- Added a useEffect that detects when the persisted selectedTurnId is absent from orderedTurnDiffSummaries and updates the store to select the latest available turn, keeping internal state in sync with the displayed diff.
- ✅ Fixed: Branch scope clears base ref
- Changed selectGitScope to preserve the existing baseRef when the current selection is already 'branch' scope, instead of unconditionally resetting it to null via DEFAULT_SELECTION.
Or push these changes by commenting:
@cursor push ea8244b85f
Preview (ea8244b85f)
diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx
--- a/apps/web/src/components/DiffPanel.tsx
+++ b/apps/web/src/components/DiffPanel.tsx
@@ -256,6 +256,21 @@
? undefined
: (orderedTurnDiffSummaries.find((summary) => summary.turnId === selectedTurnId) ??
orderedTurnDiffSummaries[0]);
+
+ useEffect(() => {
+ if (
+ selectedTurnId !== null &&
+ orderedTurnDiffSummaries.length > 0 &&
+ !orderedTurnDiffSummaries.some((summary) => summary.turnId === selectedTurnId)
+ ) {
+ if (routeThreadRef) {
+ useDiffPanelStore
+ .getState()
+ .selectTurn(routeThreadRef, orderedTurnDiffSummaries[0]!.turnId);
+ }
+ }
+ }, [selectedTurnId, orderedTurnDiffSummaries, routeThreadRef]);
+
const selectedCheckpointTurnCount =
selectedTurn &&
(selectedTurn.checkpointTurnCount ?? inferredCheckpointTurnCountByTurnId[selectedTurn.turnId]);
diff --git a/apps/web/src/diffPanelStore.ts b/apps/web/src/diffPanelStore.ts
--- a/apps/web/src/diffPanelStore.ts
+++ b/apps/web/src/diffPanelStore.ts
@@ -30,12 +30,27 @@
(set) => ({
byThreadKey: {},
selectGitScope: (ref, scope) =>
- set((state) => ({
- byThreadKey: {
- ...state.byThreadKey,
- [scopedThreadKey(ref)]: scope === "branch" ? DEFAULT_SELECTION : { kind: "unstaged" },
- },
- })),
+ set((state) => {
+ const threadKey = scopedThreadKey(ref);
+ if (scope === "branch") {
+ const current = state.byThreadKey[threadKey];
+ return {
+ byThreadKey: {
+ ...state.byThreadKey,
+ [threadKey]: {
+ kind: "branch",
+ baseRef: current?.kind === "branch" ? current.baseRef : null,
+ },
+ },
+ };
+ }
+ return {
+ byThreadKey: {
+ ...state.byThreadKey,
+ [threadKey]: { kind: "unstaged" },
+ },
+ };
+ }),
selectBranchBaseRef: (ref, baseRef) =>
set((state) => ({
byThreadKey: {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit c966b5b. Configure here.
- Persist branch base refs per thread and restore them after scope changes - Reconcile missing turn selections to the latest available turn - Filter stale base-ref choices and honor ignore-whitespace diffs
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Option-based secret handling and Effect schema decoding for more explicit nullability.Testing
vp checkvp run typecheckvp testNote
Medium Risk
Large UI and state-model change (no URL deep links, new git diff data paths) touches review/VCS APIs, but behavior is covered by new integration and store tests.
Overview
Moves diff panel state off the URL (
?diff, turn/file search params) into a persisted per-thread store (diffPanelStore), so opening the panel and choosing a scope no longer updates route search. ChatView only toggles the right panel; turn diffs from the timeline write selection into that store instead of navigating.The diff panel adds a scope dropdown: Unstaged, Branch changes (via
reviewEnvironment.diffPreviewworking-tree / branch-range sources), Latest turn, or a specific turn (checkpoint diff unchanged). Branch view gets a searchable base ref combobox backed bylistRefswithincludeMatchingRemoteRefsand local/remoterefKindfilters, plusbuildBaseRefChoicespairing. Rendering switches from per-fileVirtualizertoAnnotatableCodeView(CodeView) with sticky headers; git scopes usecompactPartialHunkOffsetsso partial hunks virtualize correctly.Server:
getReviewDiffPreviewpasses--ignore-all-spacewhenignoreWhitespaceis set;listRefscan include matching remotes and filter by ref kind. Contracts extendReviewDiffPreviewInputandVcsListRefsInputaccordingly. RemovesdiffRouteSearchand thread-route search validation.Reviewed by Cursor Bugbot for commit 104bb7a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add diff scope switching and store-driven selection to the diff panel
AnnotatableCodeViewcomponent (AnnotatableFileDiff.tsx) that renders multiple diff files with inline comment annotations and supports sticky headers and programmatic file scrolling.getReviewDiffPreviewandlistRefshandlers to supportignoreWhitespaceand ref-kind/remote-ref filtering respectively.diffsearch param from the chat route; the diff panel open/close state is no longer reflected in the URL.?diff=1no longer opens the diff panel; bookmarks or shared links containing diff search params will not restore diff panel state.Macroscope summarized 104bb7a.