Repository navigation
[codex] Add diff pane file viewer action - #3715
Merged
Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughDiff components now accept an injected Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Header as DiffFileHeader
participant Entry as DiffFileEntry
participant Pane as DiffPane
participant Registry as PaneRegistry
participant Page as Page
User->>Header: click open (may include modifier)
Header->>Entry: call onOpenFile(path, openInNewTab?) or onOpenInExternalEditor(path)
Entry->>Pane: forward onOpenFile / onOpenInExternalEditor
Pane->>Registry: invoke injected onOpenFile(path, openInNewTab?)
Registry->>Page: registry's onOpenFile called
Page->>Page: normalize to absolute workspace path
Page->>Page: recordView / create or pin pane using normalized path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR adds a built-in file viewer action to the v2 diff pane's file path chip, separating it from the external editor action which gets a new dedicated Confidence Score: 5/5Safe to merge — the path normalization bug fix is correct and the new UI interactions are well-structured. All changes are P2 or lower. The path normalization fix properly handles already-absolute paths (toAbsoluteWorkspacePath is idempotent for them), and the UI split between file-viewer and external-editor is clean and consistent across the component tree. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx | Bug fix: normalizes filePath to absoluteFilePath before passing to pane data and comparison, so relative diff paths work correctly with the shared file document store. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx | Adds a dedicated ExternalLink button for opening files in the external editor; file path chip now triggers built-in file viewer with Shift-click support for new-tab opening. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileEntry/DiffFileEntry.tsx | Splits handleOpenFile (file viewer) from handleOpenInExternalEditor; extracts showDeletedFileToast to avoid duplication; threads openInNewTab param correctly. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx | Adds onOpenFile prop to DiffPane and passes it down to DiffFileEntry alongside the existing onOpenInExternalEditor. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx | Threads new onOpenInExternalEditor prop through to DiffFileHeader alongside existing onOpenFile. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx | Passes onOpenFile from registry options into DiffPane, connecting the workspace-level file opener to the diff pane. |
Sequence Diagram
sequenceDiagram
participant User
participant DiffFileHeader
participant DiffFileEntry
participant openFilePane (page.tsx)
participant FileDocumentStore
User->>DiffFileHeader: Click file chip
DiffFileHeader->>DiffFileEntry: onOpenFile(event.shiftKey)
DiffFileEntry->>DiffFileEntry: handleOpenFile(openInNewTab)
alt file.status === deleted
DiffFileEntry->>User: toast.error(File no longer exists)
else
DiffFileEntry->>openFilePane (page.tsx): onOpenFile(file.path, openInNewTab)
openFilePane (page.tsx)->>openFilePane (page.tsx): toAbsoluteWorkspacePath(worktreePath, filePath)
openFilePane (page.tsx)->>FileDocumentStore: openPane / addTab with absoluteFilePath
end
User->>DiffFileHeader: Click ExternalLink button
DiffFileHeader->>DiffFileEntry: onOpenInExternalEditor()
DiffFileEntry->>DiffFileEntry: handleOpenInExternalEditor()
alt file.status === deleted
DiffFileEntry->>User: toast.error(File no longer exists)
else
DiffFileEntry->>openFilePane (page.tsx): openInExternalEditor(file.path)
end
Reviews (1): Last reviewed commit: "Add diff pane file viewer action" | Re-trigger Greptile
Kitenite
force-pushed
the
diff-pane-viewer-button
branch
from
April 25, 2026 00:53
dbc3cd7 to
4f3f9e6
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx (1)
65-96: Optional: also honor Cmd/Ctrl-click for "open in new tab".Currently only
event.shiftKeytriggers a new tab. PR objective explicitly preserves Shift-click, so this is by design — but on macOS/Windows the most conventional new-tab modifiers for clicking a "link-like" target are ⌘-click / Ctrl-click. Worth considering accepting either modifier so users don't have to learn a chip-specific convention.♻️ Possible tweak
- onClick={(event) => onOpenFile?.(event.shiftKey)} + onClick={(event) => + onOpenFile?.( + event.shiftKey || event.metaKey || event.ctrlKey, + ) + }Note: middle-click is another common new-tab gesture; if you want to support it, add an
onAuxClickhandler that checksevent.button === 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx around lines 65 - 96, The click handler currently only treats event.shiftKey as "open in new tab"; update the button handlers around onOpenFile to also treat event.metaKey (Cmd on macOS) and event.ctrlKey (Ctrl on Windows/Linux) as new-tab modifiers and optionally support middle-click by adding an onAuxClick that calls onOpenFile when event.button === 1; specifically modify the onClick callback that calls onOpenFile?.(event.shiftKey) to compute a boolean like openInNewTab = event.shiftKey || event.metaKey || event.ctrlKey and pass that to onOpenFile, and add an onAuxClick handler that checks event.button === 1 and calls onOpenFile(true) (guarded by existence of onOpenFile).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx:
- Around line 99-114: The tooltip on the Open in editor button doesn't show when
the button is disabled because the class disabled:pointer-events-none prevents
hover events; update DiffFileHeader's Open-in-editor button handling so the
TooltipTrigger can receive pointer events even when onOpenInExternalEditor is
falsy — wrap the button in a non-disabled wrapper element (e.g., a <span>) when
disabled and apply TooltipTrigger asChild to that wrapper, keep the button
disabled and styled the same, and ensure onOpenInExternalEditor remains the
click handler on the button (refer to the ExternalLink button within the
DiffFileHeader component and Tooltip / TooltipTrigger / TooltipContent usage).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx:
- Around line 65-96: The click handler currently only treats event.shiftKey as
"open in new tab"; update the button handlers around onOpenFile to also treat
event.metaKey (Cmd on macOS) and event.ctrlKey (Ctrl on Windows/Linux) as
new-tab modifiers and optionally support middle-click by adding an onAuxClick
that calls onOpenFile when event.button === 1; specifically modify the onClick
callback that calls onOpenFile?.(event.shiftKey) to compute a boolean like
openInNewTab = event.shiftKey || event.metaKey || event.ctrlKey and pass that to
onOpenFile, and add an onAuxClick handler that checks event.button === 1 and
calls onOpenFile(true) (guarded by existence of onOpenFile).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1205e466-1669-4dc1-b364-cc8e0566e5b8
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileEntry/DiffFileEntry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
Kitenite
force-pushed
the
diff-pane-viewer-button
branch
from
April 25, 2026 00:55
4f3f9e6 to
a1788a7
Compare
Kitenite
force-pushed
the
diff-pane-viewer-button
branch
from
April 25, 2026 01:07
a1788a7 to
457516a
Compare
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
3 of 12 tasks
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 25, 2026
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 25, 2026
upstream の 10 commits は #426 と #427 で fork 固有の差分を保ちながら個別に cherry-pick 取り込み済み。本 merge は ours strategy で **記録だけ** マージ済みに することで behind=0 を達成し、git 履歴上の追跡を正しくする。 Cherry-pick 取り込み済 (PR #426): - 5aab22a fix closed picker filters (ParadevAI#3702) → cdb52f9 - 99db5be [codex] simplify workspace controls (ParadevAI#3714) → f079606 - 186078a fix(chat): prevent ask_user question from shadowing sandbox access prompt (ParadevAI#3662) → 09d6b57 - 47893c2 fix desktop workspace creation title clamp (ParadevAI#3718) → 6a8c4ae - 09323ff Add diff pane file viewer action (ParadevAI#3715) → 817ed8d - a5891c6 remove pending launch log (ParadevAI#3721) → 0764d03 - c83de0c Add Tiptap table support (ParadevAI#3719) → e67a885 - 486b621 [codex] Fix v2 terminal lifecycle after sleep (ParadevAI#3711) → b71fbbb (+ #426 内 review fixups) Cherry-pick 取り込み済 (PR #427): - e07aef6 feat(desktop): play v2 notification hooks client-side (ParadevAI#3675) → 27ac18a - eae6008 [codex] Port v2 terminal hotkeys to v1 (ParadevAI#3724) → 05a77b8 (+ #427 内 Windows .ps1 v2 化) Fork 固有領域は変更ゼロで保持: 19 tRPC procedures (workspaces.githubExtended)、 AudioScheduler / Aivis TTS / notification-manager、terminal suggestion handler (新 terminalKeyboardHandler.ts に移植)、TERMINAL_OPTIONS、SUPERSET_WORKSPACE_NAME、 MainWindowEffects、INCEPTION_AUTH_PROVIDER_ID、v1MigrationState、TiptapPromptEditor、 electron-builder.ts (dmg.size="4g", fileAssociations)、Service Status Dashboard、 Linux daemon systemd、Worktree auto-sync、Windows support、DnD scratch route 他。
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
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
Validation
bunx @biomejs/biome@2.4.2 check <touched files>bun run --cwd apps/desktop typecheckgit diff --checkSummary by cubic
Clicking a file in the v2 Diff pane now opens it in the built‑in file viewer, with Shift‑click opening in a new tab. An external‑link button and modifier‑click open the file in your editor; opens use absolute paths for consistent view history, deleted files show a clear toast, and reopening an already‑active file pins its pane.
Written for commit 457516a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes