feat: add review panel git, branch, and turn diff modes #344
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,8 @@ import * as Diff from "diff"; | |
| import type { FileDiff, FileNode } from "../sdk/client"; | ||
| import { useSDK } from "../context/sdk"; | ||
| import { useEvents } from "../context/events"; | ||
| import { useLayout, type ReviewMode } from "../context/layout"; | ||
| import { useSync } from "../context/sync"; | ||
|
|
||
| import { FileTree } from "./file-tree"; | ||
| import { FileViewer } from "./file-viewer"; | ||
|
|
@@ -41,63 +42,130 @@ interface ReviewPanelProps { | |
| sessionId: string; | ||
| } | ||
|
|
||
| const REVIEW_MODES: Array<{ value: ReviewMode; label: string }> = [ | ||
| { value: "session", label: "Session" }, | ||
| { value: "git", label: "Git" }, | ||
| { value: "branch", label: "Branch" }, | ||
| { value: "turn", label: "Turn" }, | ||
| ]; | ||
|
|
||
| function lastUserMessageID( | ||
| messages: Array<{ info: { id: string; role: string } }>, | ||
| ) { | ||
| for (let i = messages.length - 1; i >= 0; i--) { | ||
| if (messages[i].info.role === "user") return messages[i].info.id; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| export function ReviewPanel(props: ReviewPanelProps) { | ||
| const { client, directory } = useSDK(); | ||
| const events = useEvents(); | ||
| const layout = useLayout(); | ||
| const sync = useSync(); | ||
|
|
||
| const [diffs, setDiffs] = createSignal<FileDiff[]>([]); | ||
| const [selected, setSelected] = createSignal<string | null>(null); | ||
| const [loading, setLoading] = createSignal(false); | ||
| const [tab, setTab] = createSignal<"changes" | "all">("changes"); | ||
| const [isGitRepo, setIsGitRepo] = createSignal<boolean | null>(null); // null = unknown | ||
|
|
||
| const mode = () => layout.review.mode(); | ||
| const lastUserMessageId = createMemo(() => | ||
| lastUserMessageID(sync.messages(props.sessionId)), | ||
| ); | ||
|
|
||
| // Track the latest request to prevent race conditions | ||
| let version = 0; | ||
|
|
||
| async function checkGitRepo(current: number) { | ||
| try { | ||
| // Try to get VCS info - if it fails or returns no branch, it's not a git repo | ||
| const res = await client.vcs.get({ directory }); | ||
| if (current !== version) return; | ||
| setIsGitRepo(res.data?.branch !== undefined); | ||
| } catch { | ||
| if (current !== version) return; | ||
| setIsGitRepo(false); | ||
| } | ||
|
Comment on lines
+96
to
106
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. Fixed in b3234eb. I switched |
||
| } | ||
|
|
||
| function setFiles(files: FileDiff[]) { | ||
| setDiffs(files); | ||
| const paths = files.map((d) => d.file); | ||
| const sel = selected(); | ||
| if (!sel || !paths.includes(sel)) { | ||
| setSelected(files.length > 0 ? files[0].file : null); | ||
| } | ||
| } | ||
|
|
||
| async function loadDiffs() { | ||
| const current = ++version; | ||
| const currentMode = mode(); | ||
| setLoading(true); | ||
| try { | ||
| if (currentMode === "git" || currentMode === "branch") { | ||
| const res = await client.vcs.diff({ directory, mode: currentMode }); | ||
|
Comment on lines
119
to
+126
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. Fixed in 4b73df9. I now reset |
||
| if (current !== version) return; | ||
|
Comment on lines
+125
to
+127
There was a problem hiding this comment. This introduces a new dependency on the |
||
| if (res.data) { | ||
| setFiles(res.data); | ||
| setIsGitRepo(true); | ||
| return; | ||
| } | ||
| setFiles([]); | ||
| await checkGitRepo(current); | ||
| return; | ||
| } | ||
|
|
||
| if (currentMode === "turn") { | ||
| const messageID = lastUserMessageId(); | ||
| if (!messageID) { | ||
| if (current !== version) return; | ||
| setFiles([]); | ||
| setIsGitRepo(true); | ||
| return; | ||
| } | ||
| const res = await client.session.diff({ | ||
| sessionID: props.sessionId, | ||
| directory, | ||
| messageID, | ||
| }); | ||
| if (current !== version) return; | ||
| if (res.data) { | ||
| setFiles(res.data); | ||
| setIsGitRepo(true); | ||
| return; | ||
| } | ||
| setFiles([]); | ||
| await checkGitRepo(current); | ||
| return; | ||
| } | ||
|
|
||
| const res = await client.session.diff({ sessionID: props.sessionId, directory }); | ||
| if (current !== version) return; | ||
| if (res.data) { | ||
| setFiles(res.data); | ||
| setIsGitRepo(true); | ||
| return; | ||
| } | ||
| setFiles([]); | ||
| await checkGitRepo(current); | ||
| } catch (e) { | ||
| if (current !== version) return; | ||
| console.error("[ReviewPanel] Failed to load diffs:", e); | ||
| // Check if it's a git repo issue | ||
| setFiles([]); | ||
| await checkGitRepo(current); | ||
| } finally { | ||
| if (current === version) setLoading(false); | ||
| } | ||
| } | ||
|
|
||
| // Load diffs when session, mode, or relevant turn changes | ||
| createEffect(() => { | ||
| const id = props.sessionId; | ||
| const currentMode = mode(); | ||
| if (currentMode === "turn") lastUserMessageId(); | ||
| if (id) { | ||
| // Reset selection when session changes | ||
| setSelected(null); | ||
|
|
@@ -119,16 +187,9 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| sessionID?: string; | ||
| diff?: FileDiff[]; | ||
| }; | ||
| if (mode() === "session" && eventProps.sessionID === id && eventProps.diff) { | ||
| setFiles(eventProps.diff); | ||
| setIsGitRepo(true); | ||
| } | ||
| } | ||
| // Reload on session status idle to catch completed changes | ||
|
|
@@ -142,7 +203,7 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| } | ||
| } | ||
| if (event.type === "vcs.branch.updated") { | ||
| if (mode() === "git" || mode() === "branch") loadDiffs(); | ||
| } | ||
| }); | ||
|
|
||
|
|
@@ -195,6 +256,13 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| }); | ||
|
|
||
| const count = createMemo(() => diffs().length); | ||
| const isGitMode = createMemo(() => mode() === "git" || mode() === "branch"); | ||
| const emptyText = createMemo(() => { | ||
| if (mode() === "git") return "No uncommitted changes"; | ||
| if (mode() === "branch") return "No branch changes"; | ||
| if (mode() === "turn") return "No changes in the last turn"; | ||
| return "No changes in this session"; | ||
| }); | ||
|
|
||
| // List of changed file paths for filtering | ||
| const diffFiles = createMemo(() => diffs().map((d) => d.file)); | ||
|
|
@@ -330,6 +398,8 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| > | ||
| <div class="flex items-center gap-2"> | ||
| <button | ||
| type="button" | ||
| aria-label="Refresh diff list" | ||
| onClick={() => loadDiffs()} | ||
| class="p-1 rounded hover:bg-black/5 dark:hover:bg-white/5 transition-colors" | ||
| style={{ color: "var(--icon-weak)" }} | ||
|
|
@@ -342,6 +412,8 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| </button> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| aria-label="Close review panel" | ||
| onClick={() => layout.review.close()} | ||
| class="p-1 rounded hover:bg-black/5 dark:hover:bg-white/5" | ||
| style={{ color: "var(--icon-weak)" }} | ||
|
|
@@ -350,7 +422,7 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| </button> | ||
| </div> | ||
|
|
||
| {/* Review mode selector + Tabs for Changed Files / All Files */} | ||
| <Tabs | ||
| variant="pill" | ||
| value={tab()} | ||
|
|
@@ -370,6 +442,33 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| class="px-2 py-2" | ||
| style={{ "border-bottom": "1px solid var(--border-base)" }} | ||
| > | ||
| <div class="flex gap-1 mb-2" role="group" aria-label="Diff mode"> | ||
| <For each={REVIEW_MODES}> | ||
| {(item) => ( | ||
| <button | ||
| type="button" | ||
| onClick={() => layout.review.setMode(item.value)} | ||
| aria-pressed={mode() === item.value} | ||
| aria-label={`Switch to ${item.label.toLowerCase()} diff mode`} | ||
| class="flex-1 px-2 py-1 text-xs rounded transition-colors" | ||
| style={{ | ||
| background: | ||
| mode() === item.value | ||
| ? "var(--interactive-base)" | ||
| : "var(--surface-base)", | ||
| color: | ||
| mode() === item.value | ||
| ? "var(--text-on-interactive)" | ||
| : "var(--text-weak)", | ||
| border: "1px solid var(--border-base)", | ||
| }} | ||
| title={`Show ${item.label.toLowerCase()} changes`} | ||
| > | ||
| {item.label} | ||
| </button> | ||
| )} | ||
| </For> | ||
| </div> | ||
| <Tabs.List class="flex gap-1"> | ||
| <Tabs.Trigger | ||
| value="changes" | ||
|
|
@@ -418,7 +517,7 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| style={{ color: "var(--text-weak)" }} | ||
| > | ||
| <Show | ||
| when={!isGitMode() || isGitRepo() !== false} | ||
| fallback={ | ||
| <div class="flex flex-col items-center gap-2"> | ||
| <GitBranch | ||
|
|
@@ -435,7 +534,7 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| </div> | ||
| } | ||
| > | ||
| <span>{emptyText()}</span> | ||
| </Show> | ||
| </div> | ||
| } | ||
|
|
@@ -469,7 +568,7 @@ export function ReviewPanel(props: ReviewPanelProps) { | |
| > | ||
| {count() > 0 | ||
| ? "Select a file to view changes" | ||
| : emptyText()} | ||
| </span> | ||
| </div> | ||
| } | ||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment in
checkGitReposays "if it fails or returns no branch", butVcsInfo.branchis a requiredstringin the generated types. To avoid misleading future readers, consider updating the comment to reflect the actual check being performed (i.e., "if the request fails, treat it as not a Git repo"), or adjust the condition if the backend can legitimately return an empty/absent branch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6630c14. Updated
checkGitRepoto treat a successfulclient.vcs.getcall as definitive git-repo detection, and adjusted the comment accordingly (removed the branch-presence check sinceVcsInfo.branchis required by contract).