fix(tui): interaction improvements to diff viewer

packages/opencode/src/cli/cmd/tui/feature-plugins/system/diff-viewer-file-tree-utils.ts

@@ -157,6 +157,49 @@ export function moveFileTreeSelectionToFile(
   return next?.id ?? (offset < 0 ? fileRows[0]!.id : fileRows[fileRows.length - 1]!.id)
 }
 
+export function fileTreeFileSelection(tree: FileTree, fileIndex: number) {
+  const node = tree.nodes.find((item) => item.kind === "file" && item.fileIndex === fileIndex)
+  if (!node) return undefined
+  return {
+    highlightedNode: node.id,
+    expandedNodes: fileTreeParentDirectories(tree, node.id),
+  }
+}
+
+export function singlePatchFileIndex(
+  selected: number | undefined,
+  active: number | undefined,
+  current: number | undefined,
+  first: number | undefined,
+) {
+  return selected ?? active ?? current ?? first
+}
+
+export function orderedPatchFileIndexes(rows: readonly FileTreeRow[]) {
+  return rows.flatMap((row) => (row.fileIndex === undefined ? [] : [row.fileIndex]))
+}
+
+export function movePatchFileIndex(
+  fileIndexes: readonly number[],
+  current: number | undefined,
+  offset: number,
+) {
+  if (fileIndexes.length === 0) return undefined
+  const index = current === undefined ? -1 : fileIndexes.indexOf(current)
+  if (index === -1) return offset < 0 ? fileIndexes[fileIndexes.length - 1] : fileIndexes[0]
+  return fileIndexes[Math.max(0, Math.min(fileIndexes.length - 1, index + offset))]
+}
+
+export function relativePatchFileIndexFromViewport(
+  entries: readonly { readonly fileIndex: number; readonly titleContentY: number }[],
+  scrollTop: number,
+  offset: number,
+) {
+  const ordered = [...entries].sort((left, right) => left.titleContentY - right.titleContentY)
+  if (offset > 0) return ordered.find((entry) => entry.titleContentY > scrollTop)?.fileIndex
+  return ordered.findLast((entry) => entry.titleContentY < scrollTop)?.fileIndex
+}
+
 export function allExpandedFileTreeDirectories(tree: FileTree) {
   return new Set(tree.nodes.filter((node) => node.kind === "directory").map((node) => node.id))
 }
@@ -189,3 +232,11 @@ function addFileTreeNode(nodes: FileTreeNode[], roots: number[], input: Omit<Fil
   else nodes[input.parent]!.children.push(id)
   return id
 }
+
+function fileTreeParentDirectories(tree: FileTree, id: number) {
+  const result = new Set<number>()
+  for (let parent = tree.nodes[id]?.parent; parent !== undefined; parent = tree.nodes[parent]?.parent) {
+    result.add(parent)
+  }
+  return result
+}

packages/opencode/src/cli/cmd/tui/feature-plugins/system/diff-viewer.tsx

@@ -14,12 +14,17 @@ import { DialogSelect } from "@tui/ui/dialog-select"
 import {
   allExpandedFileTreeDirectories,
   buildFileTree,
+  fileTreeFileSelection,
   flattenFileTree,
   moveFileTreeSelection,
   moveFileTreeSelectionToFirstChild,
   moveFileTreeSelectionToFile,
   moveFileTreeSelectionToParent,
+  movePatchFileIndex,
+  orderedPatchFileIndexes,
+  relativePatchFileIndexFromViewport,
   setFileTreeDirectoryExpanded,
+  singlePatchFileIndex,
   toggleFileTreeDirectory,
 } from "./diff-viewer-file-tree-utils"
 
@@ -108,6 +113,7 @@ function DiffViewer(props: { api: TuiPluginApi }) {
   const [selectedFileIndex, setSelectedFileIndex] = createSignal<number | undefined>()
   const [reviewedFileNames, setReviewedFileNames] = createSignal<ReadonlySet<string>>(new Set())
   const fileRows = createMemo(() => flattenFileTree(fileTree(), expandedFileNodes()))
+  const patchFileIndexes = createMemo(() => orderedPatchFileIndexes(flattenFileTree(fileTree())))
   const focusRunner = (input: Record<DiffViewerFocus, () => void>) => () => input[focus()]()
   const switchFocusShortcut = useCommandShortcut("diff.switch_focus")
   const nextFileShortcut = useCommandShortcut("diff.next_file")
@@ -164,39 +170,67 @@ function DiffViewer(props: { api: TuiPluginApi }) {
   }
 
   const revealFileTreeFile = (fileIndex: number) => {
-    const node = fileTree().nodes.find((item) => item.kind === "file" && item.fileIndex === fileIndex)
-    if (!node) return
+    const selection = fileTreeFileSelection(fileTree(), fileIndex)
+    if (!selection) return
     setExpandedFileNodes((expanded) => {
       const next = new Set(expanded)
-      for (let parent = node.parent; parent !== undefined; parent = fileTree().nodes[parent]?.parent) {
-        next.add(parent)
-      }
+      selection.expandedNodes.forEach((node) => next.add(node))
       return next
     })
-    setHighlighted(node.id)
+    setHighlighted(selection.highlightedNode)
+  }
+
+  const selectPatchFile = (fileIndex: number) => {
+    revealFileTreeFile(fileIndex)
+    setActivePatchFileIndex(fileIndex)
+    setSelectedFileIndex(fileIndex)
   }
 
   const scrollToFileIndex = (fileIndex: number | undefined) => {
     if (fileIndex === undefined) return
-    setActivePatchFileIndex(fileIndex)
-    setSelectedFileIndex(fileIndex)
+    selectPatchFile(fileIndex)
     const patchNode = patchNodeByFileIndex.get(fileIndex)
     if (patchNode) scrollPatchNodeToTop(patchNode, fileIndex)
   }
 
   const jumpToFileIndex = (fileIndex: number | undefined) => {
     if (fileIndex === undefined) return
-    revealFileTreeFile(fileIndex)
     scrollToFileIndex(fileIndex)
   }
 
   const currentPatchFileIndex = () => {
     if (!scroll) return undefined
-    const entries = files()
-      .map((_, fileIndex) => ({ fileIndex, node: patchNodeByFileIndex.get(fileIndex) }))
+    const viewportContentY = scroll.scrollTop + 1
+    const entries = patchFileIndexes()
+      .map((fileIndex) => ({
+        fileIndex,
+        node: patchNodeByFileIndex.get(fileIndex),
+      }))
       .filter((entry): entry is { fileIndex: number; node: BoxRenderable } => Boolean(entry.node))
-      .sort((left, right) => left.node.y - right.node.y)
-    return entries.findLast((entry) => entry.node.y <= scroll!.viewport.y + 1)?.fileIndex ?? entries[0]?.fileIndex
+      .map((entry) => ({
+        ...entry,
+        contentY: scroll!.scrollTop + entry.node.y - scroll!.viewport.y,
+      }))
+      .sort((left, right) => left.contentY - right.contentY)
+    return entries.findLast((entry) => entry.contentY <= viewportContentY)?.fileIndex ?? entries[0]?.fileIndex
+  }
+
+  const nextPatchFileIndexFromViewport = (offset: number) => {
+    if (!scroll) return undefined
+    return relativePatchFileIndexFromViewport(
+      patchFileIndexes()
+        .map((fileIndex) => ({ fileIndex, node: patchNodeByFileIndex.get(fileIndex) }))
+        .filter((entry): entry is { fileIndex: number; node: BoxRenderable } => Boolean(entry.node))
+        .map((entry) => {
+          const contentY = scroll!.scrollTop + entry.node.y - scroll!.viewport.y
+          return {
+            fileIndex: entry.fileIndex,
+            titleContentY: contentY + (contentY === 0 ? 0 : 1),
+          }
+        }),
+      scroll.scrollTop,
+      offset,
+    )
   }
 
   const jumpRelativePatchFile = (offset: number) => {
@@ -207,32 +241,55 @@ function DiffViewer(props: { api: TuiPluginApi }) {
       jumpToFileIndex(fileRows().find((row) => row.id === nextFromSelection)?.fileIndex)
       return
     }
-    const currentFileIndex = activePatchFileIndex() ?? currentPatchFileIndex()
-    const currentRow = fileRows().find((row) => row.fileIndex === currentFileIndex)
     scrollToFileIndex(
-      fileRows().find((row) => row.id === moveFileTreeSelectionToFile(fileRows(), currentRow?.id, offset))?.fileIndex,
+      nextPatchFileIndexFromViewport(offset) ??
+        movePatchFileIndex(patchFileIndexes(), currentPatchFileIndex() ?? activePatchFileIndex(), offset),
     )
   }
 
   const highlightedPatchFileIndex = () => fileRows().find((row) => row.id === highlightedFileNode())?.fileIndex
   const firstPatchFileIndex = () => fileRows().find((row) => row.fileIndex !== undefined)?.fileIndex
   const visiblePatchFiles = createMemo(() => {
-    if (!singlePatch()) return files().map((file, fileIndex) => ({ file, fileIndex }))
-    const fileIndex = activePatchFileIndex() ?? currentPatchFileIndex() ?? firstPatchFileIndex()
+    if (!singlePatch()) {
+      return patchFileIndexes().flatMap((fileIndex) => {
+        const file = files()[fileIndex]
+        return file ? [{ file, fileIndex }] : []
+      })
+    }
+    const fileIndex = singlePatchFileIndex(
+      selectedFileIndex(),
+      activePatchFileIndex(),
+      currentPatchFileIndex(),
+      firstPatchFileIndex(),
+    )
     const file = fileIndex === undefined ? undefined : files()[fileIndex]
     return file && fileIndex !== undefined ? [{ file, fileIndex }] : []
   })
 
   const ensureHighlightedPatchFile = () => {
-    if (activePatchFileIndex() !== undefined) return
-    const fileIndex = currentPatchFileIndex() ?? firstPatchFileIndex()
-    if (fileIndex !== undefined) setActivePatchFileIndex(fileIndex)
+    const fileIndex = currentPatchFileIndex() ?? activePatchFileIndex() ?? firstPatchFileIndex()
+    if (fileIndex === undefined) return
+    selectPatchFile(fileIndex)
   }
 
-  const scrollToHighlightedPatchFile = () => {
-    const fileIndex = activePatchFileIndex()
-    if (fileIndex === undefined) return
+  const scrollToPatchFileIndexAfterRender = (fileIndex: number) => {
     setPendingPatchScrollFileIndex(fileIndex)
+    requestAnimationFrame(() => {
+      const patchNode = patchNodeByFileIndex.get(fileIndex)
+      if (patchNode) scrollPatchNodeToTop(patchNode, fileIndex)
+      requestAnimationFrame(() => {
+        const patchNode = patchNodeByFileIndex.get(fileIndex)
+        if (patchNode) scrollPatchNodeToTop(patchNode, fileIndex)
+        setPendingPatchScrollFileIndex(undefined)
+      })
+    })
+  }
+
+  const scrollSinglePatchToTop = () => {
+    requestAnimationFrame(() => {
+      scroll?.scrollTo(0)
+      requestAnimationFrame(() => scroll?.scrollTo(0))
+    })
   }
 
   const registerPatchNode = (fileIndex: number, element: BoxRenderable) => {
@@ -437,12 +494,18 @@ function DiffViewer(props: { api: TuiPluginApi }) {
       title: "Toggle single patch view",
       category: "VCS",
       run() {
-        setSinglePatch((value) => {
-          const next = !value
-          if (next) ensureHighlightedPatchFile()
-          else scrollToHighlightedPatchFile()
-          return next
-        })
+        if (!singlePatch()) {
+          ensureHighlightedPatchFile()
+          setSinglePatch(true)
+          scrollSinglePatchToTop()
+          return
+        }
+        const fileIndex =
+          visiblePatchFiles()[0]?.fileIndex ??
+          singlePatchFileIndex(selectedFileIndex(), activePatchFileIndex(), currentPatchFileIndex(), firstPatchFileIndex())
+        if (fileIndex !== undefined) selectPatchFile(fileIndex)
+        setSinglePatch(false)
+        if (fileIndex !== undefined) scrollToPatchFileIndexAfterRender(fileIndex)
       },
     },
     {
@@ -581,7 +644,7 @@ function DiffViewer(props: { api: TuiPluginApi }) {
                                   flexDirection="row"
                                   gap={1}
                                   flexShrink={0}
-                                  paddingLeft={2}
+                                  paddingLeft={1}
                                   paddingRight={1}
                                   border={["left"]}
                                   borderColor={theme().border}

packages/opencode/test/cli/tui/diff-viewer-file-tree-utils.test.ts

@@ -2,12 +2,17 @@ import { describe, expect, test } from "bun:test"
 import {
   allExpandedFileTreeDirectories,
   buildFileTree,
+  fileTreeFileSelection,
   flattenFileTree,
   moveFileTreeSelection,
   moveFileTreeSelectionToFirstChild,
   moveFileTreeSelectionToFile,
   moveFileTreeSelectionToParent,
+  movePatchFileIndex,
+  orderedPatchFileIndexes,
+  relativePatchFileIndexFromViewport,
   setFileTreeDirectoryExpanded,
+  singlePatchFileIndex,
   toggleFileTreeDirectory,
 } from "../../../src/cli/cmd/tui/feature-plugins/system/diff-viewer-file-tree-utils"
 
@@ -233,6 +238,100 @@ describe("diff viewer file tree utilities", () => {
     expect(moveFileTreeSelectionToFile(rows, readme.id, 1)).toBe(readme.id)
   })
 
+  test("selects a file tree node and expands its parents for a patch file", () => {
+    const tree = buildFileTree([
+      { file: "src/config/tui.ts" },
+      { file: "src/session/index.ts" },
+      { file: "README.md" },
+    ])
+    const selection = fileTreeFileSelection(tree, 1)
+
+    expect(selection?.highlightedNode).toBe(tree.nodes.find((node) => node.kind === "file" && node.name === "index.ts")?.id)
+    expect([...selection!.expandedNodes].map((id) => tree.nodes[id]!.name)).toEqual(["session", "src"])
+    expect(fileTreeFileSelection(tree, 99)).toBeUndefined()
+  })
+
+  test("prefers the selected file when choosing the single patch file", () => {
+    expect(singlePatchFileIndex(2, 1, 0, 3)).toBe(2)
+    expect(singlePatchFileIndex(undefined, 1, 0, 3)).toBe(1)
+    expect(singlePatchFileIndex(undefined, undefined, 0, 3)).toBe(0)
+    expect(singlePatchFileIndex(undefined, undefined, undefined, 3)).toBe(3)
+  })
+
+  test("orders patches by the flattened file tree order", () => {
+    const rows = flattenFileTree(
+      buildFileTree([
+        { file: "src/dir-8/juniper-4.ts" },
+        { file: "src/dir-8/harbor-94.ts" },
+        { file: "src/dir-8/cedar-16.ts" },
+      ]),
+    )
+
+    expect(orderedPatchFileIndexes(rows)).toEqual([2, 1, 0])
+  })
+
+  test("moves patch selection through the ordered patch file indexes", () => {
+    const fileIndexes = [2, 1, 0]
+
+    expect(movePatchFileIndex(fileIndexes, undefined, 1)).toBe(2)
+    expect(movePatchFileIndex(fileIndexes, undefined, -1)).toBe(0)
+    expect(movePatchFileIndex(fileIndexes, 2, 1)).toBe(1)
+    expect(movePatchFileIndex(fileIndexes, 1, -1)).toBe(2)
+    expect(movePatchFileIndex(fileIndexes, 0, 1)).toBe(0)
+    expect(movePatchFileIndex(fileIndexes, 99, 1)).toBe(2)
+    expect(movePatchFileIndex([], undefined, 1)).toBeUndefined()
+  })
+
+  test("moves to the next visible patch title below the viewport", () => {
+    expect(
+      relativePatchFileIndexFromViewport(
+        [
+          { fileIndex: 0, titleContentY: 0 },
+          { fileIndex: 1, titleContentY: 30 },
+          { fileIndex: 2, titleContentY: 60 },
+        ],
+        10,
+        1,
+      ),
+    ).toBe(1)
+    expect(
+      relativePatchFileIndexFromViewport(
+        [
+          { fileIndex: 0, titleContentY: 0 },
+          { fileIndex: 1, titleContentY: 30 },
+          { fileIndex: 2, titleContentY: 60 },
+        ],
+        30,
+        1,
+      ),
+    ).toBe(2)
+  })
+
+  test("moves to the previous visible patch title above the viewport", () => {
+    expect(
+      relativePatchFileIndexFromViewport(
+        [
+          { fileIndex: 0, titleContentY: 0 },
+          { fileIndex: 1, titleContentY: 30 },
+          { fileIndex: 2, titleContentY: 60 },
+        ],
+        50,
+        -1,
+      ),
+    ).toBe(1)
+    expect(
+      relativePatchFileIndexFromViewport(
+        [
+          { fileIndex: 0, titleContentY: 0 },
+          { fileIndex: 1, titleContentY: 30 },
+          { fileIndex: 2, titleContentY: 60 },
+        ],
+        30,
+        -1,
+      ),
+    ).toBe(0)
+  })
+
   test("toggles only selected directory expansion", () => {
     const tree = buildFileTree([{ file: "src/config/tui.ts" }, { file: "README.md" }])
     const src = tree.nodes.find((node) => node.kind === "directory" && node.name === "src")!