refactor: pass git diff root path over IPC#32
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR removes module-level git-diff root state and threads Changes
|IPC Preload Bridge & Shared Types |Renderer - Editor Integration |Inline Diff Implementation Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: b90253ffa3
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/renderer/src/components/AppShell.tsx (1)
831-842:⚠️ Potential issue | 🟠 MajorSplit editor commands are missing the required
workspaceRootfield.The
onFileOpencallback in AppShell.tsx correctly includesworkspaceRootin the panel params. However, the split editor commands inpackages/renderer/src/lib/defaultCommands.ts(lines 22 and 40) still create panels with only{ filePath }. SinceEditorPaneParamsrequiresworkspaceRoot: string | null, split editors will fail to instantiate properly.Update both split commands to extract and pass
workspaceRootfrom the active panel's params:
- Line 22 (
editor.splitVertical): Changeparams: { filePath }toparams: { filePath, workspaceRoot: (active.params as Record<string, unknown>)?.workspaceRoot ?? null }- Line 40 (
editor.splitHorizontal): Apply the same fix🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/AppShell.tsx` around lines 831 - 842, Split editor commands (editor.splitVertical and editor.splitHorizontal in defaultCommands.ts) create panels with params missing required EditorPaneParams.workspaceRoot; update both to read workspaceRoot from the current active panel params and pass it into the new panel params. Specifically, replace params: { filePath } with params: { filePath, workspaceRoot: (active.params as Record<string, unknown>)?.workspaceRoot ?? null } for both editor.splitVertical and editor.splitHorizontal so the new split editor panels include workspaceRoot and can instantiate correctly.
🧹 Nitpick comments (1)
packages/renderer/src/components/panes/EditorPane.tsx (1)
149-158: Consider stale closure ifworkspaceRootchanges withoutfilePathchanging.The keybinding at line 152 captures
workspaceRootfrom the closure when the editor initializes. The effect only re-runs whenfilePathchanges (line 243), so ifworkspaceRootwere to change independently (e.g., worktree switch), the keybinding would use the stale value.In practice, workspace/worktree changes likely cause panel recreation, making this a low-risk edge case. If this is intentional, no changes needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/panes/EditorPane.tsx` around lines 149 - 158, The keybinding's run closure captures workspaceRoot at setup time which can become stale; update the handler so it always uses the current workspaceRoot — either by adding workspaceRoot to the dependency list that creates the keybinding (so the binding is re-created when workspaceRoot changes) or by reading workspaceRoot at invocation time via a stable ref/getter before calling toggleInlineDiff (identify the keybinding object and its run method, and the symbols toggleInlineDiff, workspaceRoot, filePath, setDiffActive, showToast). Ensure the change avoids introducing memory leaks (clean up/re-register the keybinding when dependencies change) and keeps the existing post-toggle behavior (setDiffActive and showToast).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/renderer/src/components/AppShell.tsx`:
- Around line 831-842: Split editor commands (editor.splitVertical and
editor.splitHorizontal in defaultCommands.ts) create panels with params missing
required EditorPaneParams.workspaceRoot; update both to read workspaceRoot from
the current active panel params and pass it into the new panel params.
Specifically, replace params: { filePath } with params: { filePath,
workspaceRoot: (active.params as Record<string, unknown>)?.workspaceRoot ?? null
} for both editor.splitVertical and editor.splitHorizontal so the new split
editor panels include workspaceRoot and can instantiate correctly.
---
Nitpick comments:
In `@packages/renderer/src/components/panes/EditorPane.tsx`:
- Around line 149-158: The keybinding's run closure captures workspaceRoot at
setup time which can become stale; update the handler so it always uses the
current workspaceRoot — either by adding workspaceRoot to the dependency list
that creates the keybinding (so the binding is re-created when workspaceRoot
changes) or by reading workspaceRoot at invocation time via a stable ref/getter
before calling toggleInlineDiff (identify the keybinding object and its run
method, and the symbols toggleInlineDiff, workspaceRoot, filePath,
setDiffActive, showToast). Ensure the change avoids introducing memory leaks
(clean up/re-register the keybinding when dependencies change) and keeps the
existing post-toggle behavior (setDiffActive and showToast).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d362dea9-c174-4773-a5c6-81fed32c5e76
📒 Files selected for processing (7)
packages/main/src/gitDiff.tspackages/main/src/index.tspackages/main/src/preload.tspackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/panes/EditorPane.tsxpackages/renderer/src/lib/editorInlineDiff.tspackages/shared/src/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{css,scss,tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode
Files:
packages/renderer/src/components/AppShell.tsxpackages/shared/src/index.tspackages/main/src/index.tspackages/main/src/preload.tspackages/main/src/gitDiff.tspackages/renderer/src/lib/editorInlineDiff.tspackages/renderer/src/components/panes/EditorPane.tsx
**/*editor*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CodeMirror 6 for editor implementation (not Monaco)
Files:
packages/renderer/src/lib/editorInlineDiff.ts
🧠 Learnings (1)
📚 Learning: 2026-03-25T11:00:59.388Z
Learnt from: Cheezeiii365
Repo: Cheezeiii365/aIDE PR: 4
File: packages/renderer/src/components/FileTree/ContextMenu.tsx:16-20
Timestamp: 2026-03-25T11:00:59.388Z
Learning: Since aIDE does not support Windows, path-handling utilities and related code should assume POSIX-style separators (`/`) (e.g., helpers like `dirname`/`basename` that split on `/`). During review, do not require Windows-style backslash (`\\`) support or `path.sep`/cross-platform normalization unless the project explicitly adds Windows support.
Applied to files:
packages/renderer/src/components/AppShell.tsxpackages/shared/src/index.tspackages/main/src/index.tspackages/main/src/preload.tspackages/main/src/gitDiff.tspackages/renderer/src/lib/editorInlineDiff.tspackages/renderer/src/components/panes/EditorPane.tsx
🔇 Additional comments (9)
packages/main/src/index.ts (1)
22-22: LGTM — clean removal ofsetDiffRootimport.The module-level state setter is correctly removed in favor of the explicit
rootPathparameter passed via IPC. The handler registration at line 932 remains intact.packages/shared/src/index.ts (1)
567-567: LGTM — API signature correctly updated.The
WindowApi.getGitFileOriginaltype now matches the updated IPC invocation pattern withrootPathas the first parameter.packages/main/src/preload.ts (1)
76-77: LGTM — preload bridge correctly forwardsrootPath.The IPC invocation now properly passes both
rootPathandfilePatharguments, matching the handler signature ingitDiff.tsand the type definition inWindowApi.packages/renderer/src/lib/editorInlineDiff.ts (1)
69-84: LGTM —toggleInlineDiffcorrectly threadsrootPathto the API.The function signature and API call are properly updated. The
result.content ?? ''fallback gracefully handles thenullcontent case when no git root is available or the file is untracked.packages/main/src/gitDiff.ts (1)
34-39: LGTM — IPC handler correctly refactored to accept explicitrootPath.The handler now:
- Accepts
rootPathas a parameter instead of relying on module state- Returns
{ content: null }early when no root is provided- Passes the root through to
getOriginalContentwhich validates the path stays within the repoThis makes git diff lookups deterministic and allows targeting any workspace.
packages/renderer/src/components/panes/EditorPane.tsx (4)
22-28: LGTM —EditorPaneParamsinterface correctly extended.The
workspaceRoot: string | nullfield is appropriately typed to handle workspaces without a filesystem root.
61-61: LGTM —workspaceRootcorrectly extracted from params.
354-367: LGTM — event listener correctly updated withworkspaceRootdependency.The dependency array now includes
workspaceRoot, ensuring the handler is refreshed when the workspace root changes.
397-404: LGTM — badge click handler correctly updated.Both the
toggleInlineDiffcall and theuseCallbackdependency array properly includeworkspaceRoot.
…orkspaceRoot Previously registerAppActions was called once with an empty dependency array, so the openFile handler captured the initial workspaceRoot closure. After a workspace switch, split-editor panels and Quick Open would pass the stale root, causing git-original lookups to fail (whole file shown as green/added). Co-authored-by: BT Odoy <Cheezeiii365@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/components/AppShell.tsx`:
- Around line 509-516: The useEffect that calls registerAppActions references
onFileOpen in its dependency array before onFileOpen is declared (causing a TDZ
ReferenceError); move the entire useEffect block that calls registerAppActions
(the block creating openFile/openUrl handlers) so it appears after the const
onFileOpen = useCallback(...) declaration, and keep onFileOpen in the dependency
array so the registration re-runs when onFileOpen changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14980774-54ac-475e-ac2e-fe8ffa839598
📒 Files selected for processing (3)
packages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/panes/EditorPane.tsxpackages/renderer/src/lib/defaultCommands.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{css,scss,tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode
Files:
packages/renderer/src/lib/defaultCommands.tspackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/panes/EditorPane.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-25T11:11:52.657Z
Learnt from: CR
Repo: Cheezeiii365/aIDE PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T11:11:52.657Z
Learning: Applies to **/*layout*.{ts,tsx} : Use Dockview 5.x for layout and tiling panes
Applied to files:
packages/renderer/src/lib/defaultCommands.ts
📚 Learning: 2026-03-25T11:00:59.388Z
Learnt from: Cheezeiii365
Repo: Cheezeiii365/aIDE PR: 4
File: packages/renderer/src/components/FileTree/ContextMenu.tsx:16-20
Timestamp: 2026-03-25T11:00:59.388Z
Learning: Since aIDE does not support Windows, path-handling utilities and related code should assume POSIX-style separators (`/`) (e.g., helpers like `dirname`/`basename` that split on `/`). During review, do not require Windows-style backslash (`\\`) support or `path.sep`/cross-platform normalization unless the project explicitly adds Windows support.
Applied to files:
packages/renderer/src/lib/defaultCommands.tspackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/panes/EditorPane.tsx
🔇 Additional comments (4)
packages/renderer/src/lib/defaultCommands.ts (1)
16-24: Good propagation ofworkspaceRootinto split editor panels.This keeps split-pane params aligned with the new inline-diff root-path flow and avoids relying on hidden global state.
Also applies to: 35-43
packages/renderer/src/components/AppShell.tsx (1)
832-833: Good update:workspaceRootis now captured and forwarded with editor panel params.Including
workspaceRootin both params and callback dependencies addresses stale-root behavior across workspace switches.Also applies to: 843-843
packages/renderer/src/components/panes/EditorPane.tsx (2)
24-25: Nice refactor: explicit diff-root wiring with a safe fallback.
workspaceRootin pane params plus fallback resolution keeps inline diff deterministic while still handling legacy/missing params.Also applies to: 61-68
159-160: Good consolidation of inline-diff toggling entry points.Routing hotkey, global command event, and badge click through one helper improves consistency and maintainability.
Also applies to: 364-368, 401-402
The useEffect dependency array [onFileOpen] was evaluated during render before the const declaration was reached, causing a Temporal Dead Zone access. Moved the effect to immediately after onFileOpen is defined. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Motivation
Description
currentRootandsetDiffRoot()frompackages/main/src/gitDiff.tsand changed theGIT_DIFF_ORIGINALIPC handler to accept(rootPath, filePath)and use that to compute the original content.WindowApisignature sogetGitFileOriginalnow accepts(rootPath: string | null, filePath: string)and forwards both args via IPC. (files:packages/main/src/preload.ts,packages/shared/src/index.ts).toggleInlineDiffnow takesrootPath,EditorPanepanel params includeworkspaceRootand pass it to inline-diff calls, andAppShellinjectsworkspaceRootwhen creating editor panels. (files:packages/renderer/src/lib/editorInlineDiff.ts,packages/renderer/src/components/panes/EditorPane.tsx,packages/renderer/src/components/AppShell.tsx).setDiffRoot()call from workspace activation inpackages/main/src/index.tsand adjusted imports accordingly.refactor: pass git diff root path over IPC.Testing
pnpm typecheckwhich failed due to existing, unrelated TypeScript errors inpackages/renderer/src/hooks/useSettings.ts, so type-checking did not fully pass (failure is unrelated to this change).Codex Task
Summary by CodeRabbit
Refactor
Bug Fix