Feat: Allow for reverting arbitrary branches #2971
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds branch-aware Git checkpoints and multi-branch restore flows: creates per-branch checkpoints from chat, persists branchId in checkpoint schema, replaces VersionsManager with a Sandbox-backed GitManager, adds a MultiBranchRevertModal UI, and refactors versions UI to operate per-branch. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UM as UserMessage UI
participant MR as MultiBranchRevertModal
participant SD as SandboxManager
participant GM as GitManager
participant EE as EditorEngine
U->>UM: open restore options
UM->>UM: render single vs multi-checkpoint UI
U->>MR: open multi-branch modal (passes checkpoints)
MR->>SD: request branch data for each selected checkpoint
SD->>GM: gitManager.createCommit(backup) -- create backup per branch
GM-->>SD: backup commit oid
MR->>GM: restoreToCommit(checkpoint.oid)
GM-->>MR: result (success / error)
MR-->>U: toast summary and close modal
sequenceDiagram
autonumber
actor U as User
participant CH as useChat Hook
participant SD as SandboxManager
participant GM as GitManager
participant API as chat.message.updateCheckpoints
U->>CH: send chat message
CH->>SD: enumerate sandbox branches
loop per branch
SD->>GM: createCommit("New Onlook backup")
GM-->>CH: { oid, createdAt, branchId }
end
CH->>API: updateCheckpoints([{ oid, branchId, createdAt }, ...])
API-->>CH: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| onOpenChange(false); | ||
| setSelectedBranchIds([]); | ||
| }} |
There was a problem hiding this comment.
The isRestoring state should be reset when the modal is closed via the Cancel button. Consider adding setIsRestoring(false) to ensure the state is properly cleaned up, preventing potential issues if the modal is reopened later.
onClick={() => {
onOpenChange(false);
setSelectedBranchIds([]);
setIsRestoring(false);
}}| onOpenChange(false); | |
| setSelectedBranchIds([]); | |
| }} | |
| onOpenChange(false); | |
| setSelectedBranchIds([]); | |
| setIsRestoring(false); | |
| }} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
saddlepaddle
force-pushed
the
fix-reverting-multiple-branches
branch
from
0d5ec35 to
806ea44
Compare
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| setFinishReason(null); | ||
|
|
||
| const applyCommit = async () => { | ||
| console.log('[CHECKPOINT] applyCommit started'); |
There was a problem hiding this comment.
There are multiple console.log statements (e.g. '[CHECKPOINT] ...') used for debugging. Consider removing or replacing them with a proper logging mechanism before production deployment.
| console.log('[CHECKPOINT] applyCommit started'); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx (1)
128-133: ResetisRestoringwhen closing the modal.If the modal is closed (Cancel button, ESC, or backdrop) while a restore is in progress,
isRestoringstaystrue. When reopened the primary action stays disabled and reads “Restoring…”. ResetisRestoringtofalsealong with clearing the selection when the modal closes.- onClick={() => { - onOpenChange(false); - setSelectedBranchIds([]); - }} + onClick={() => { + onOpenChange(false); + setSelectedBranchIds([]); + setIsRestoring(false); + }}apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
228-333: Remove the temporary checkpoint console loggingThe earlier review already called out the debug
console.log/console.warnstatements sprinkled through the checkpoint flow. Please strip them before merge (or move to a proper logger if we need structured traces).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(6 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.ts(1 hunks)apps/web/client/src/components/store/editor/engine.ts(0 hunks)apps/web/client/src/components/store/editor/sandbox/index.ts(2 hunks)apps/web/client/src/components/store/editor/version/git.ts(8 hunks)apps/web/client/src/components/store/editor/version/index.ts(0 hunks)apps/web/client/src/components/ui/settings-modal/versions/empty-state/saved.tsx(0 hunks)apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx(2 hunks)apps/web/client/src/components/ui/settings-modal/versions/index.tsx(0 hunks)apps/web/client/src/components/ui/settings-modal/versions/saved-version.tsx(0 hunks)apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx(7 hunks)apps/web/client/src/components/ui/settings-modal/versions/versions.tsx(4 hunks)apps/web/client/src/server/api/routers/chat/message.ts(1 hunks)packages/models/src/chat/message/checkpoint.ts(1 hunks)
💤 Files with no reviewable changes (5)
- apps/web/client/src/components/store/editor/engine.ts
- apps/web/client/src/components/ui/settings-modal/versions/saved-version.tsx
- apps/web/client/src/components/ui/settings-modal/versions/empty-state/saved.tsx
- apps/web/client/src/components/ui/settings-modal/versions/index.tsx
- apps/web/client/src/components/store/editor/version/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/message.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsxapps/web/client/src/components/store/editor/version/git.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/server/api/routers/chat/message.tspackages/models/src/chat/message/checkpoint.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsxapps/web/client/src/components/store/editor/version/git.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/server/api/routers/chat/message.tspackages/models/src/chat/message/checkpoint.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsxapps/web/client/src/components/store/editor/version/git.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
🧬 Code graph analysis (8)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (2)
packages/git/src/git.ts (2)
branch(116-123)status(89-91)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
packages/ui/src/components/select.tsx (5)
Select(158-158)SelectTrigger(166-166)SelectValue(167-167)SelectContent(159-159)SelectItem(161-161)packages/git/src/git.ts (1)
branch(116-123)
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx (2)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/ui/src/components/sonner.tsx (1)
toast(19-19)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx (2)
packages/git/src/git.ts (1)
updateCommitDisplayName(159-169)apps/web/client/src/components/store/editor/version/git.ts (1)
commit(169-177)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (2)
packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx (1)
MultiBranchRevertModal(18-150)
apps/web/client/src/components/store/editor/sandbox/index.ts (2)
apps/web/client/src/components/store/editor/sandbox/session.ts (1)
SessionManager(8-225)apps/web/client/src/components/store/editor/version/git.ts (1)
GitManager(22-412)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx (2)
packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)
apps/web/client/src/components/store/editor/version/git.ts (3)
packages/git/src/git.ts (1)
GitCommit(19-25)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)apps/web/client/src/utils/git/index.ts (2)
sanitizeCommitMessage(7-47)prepareCommitMessage(72-75)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
Show resolved
Hide resolved
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
Outdated
Show resolved
Hide resolved
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
Outdated
Show resolved
Hide resolved
| disabled={isRestoring || selectedBranchIds.length === 0} | ||
| className="order-1 sm:order-2" | ||
| > | ||
| {isRestoring ? 'Restoring...' : 'Revert Selected'} |
There was a problem hiding this comment.
UI text inconsistency: Update the button label from 'Revert Selected' to 'Restore Selected' to match the modal title.
| {isRestoring ? 'Restoring...' : 'Revert Selected'} | |
| {isRestoring ? 'Restoring...' : 'Restore Selected'} |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(7 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(2 hunks)apps/web/client/src/components/store/editor/git/git.ts(8 hunks)apps/web/client/src/components/store/editor/git/index.ts(1 hunks)apps/web/client/src/components/store/editor/git/utils.ts(1 hunks)apps/web/client/src/components/store/editor/sandbox/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/client/src/components/store/editor/git/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/store/editor/git/git.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/components/store/editor/git/utils.ts
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/store/editor/git/git.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/components/store/editor/git/utils.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/store/editor/git/git.tsapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/components/store/editor/git/utils.ts
🧬 Code graph analysis (4)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/components/store/editor/git/git.ts (3)
packages/git/src/git.ts (1)
GitCommit(19-25)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)apps/web/client/src/utils/git/index.ts (2)
sanitizeCommitMessage(7-47)prepareCommitMessage(72-75)
apps/web/client/src/components/store/editor/sandbox/index.ts (2)
apps/web/client/src/components/store/editor/sandbox/session.ts (1)
SessionManager(8-225)apps/web/client/src/components/store/editor/git/git.ts (1)
GitManager(22-412)
apps/web/client/src/components/store/editor/git/utils.ts (2)
packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)
| async (provider) => { | ||
| if (provider) { | ||
| this.initializeSyncEngine(provider); | ||
| await this.initializeSyncEngine(provider); | ||
| // Initialize git after provider is ready | ||
| await this.gitManager.init(); | ||
| } else if (this.sync) { |
There was a problem hiding this comment.
Handle errors in the async provider reaction.
Turning the reaction effect into an async function means any rejection from initializeSyncEngine or gitManager.init() now escapes without a catch, producing unhandled promise rejections and leaving the sandbox half‑initialized. Wrap the awaited calls in a try/catch (and surface the failure through errorManager or logs) so provider transitions remain resilient.
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/sandbox/index.ts around lines 38
to 43, the async reaction callback can throw from initializeSyncEngine or
gitManager.init causing unhandled promise rejections and leaving the sandbox
half-initialized; wrap the awaited calls in a try/catch, log or report the error
via errorManager (or console/error logger) and ensure you set the sandbox sync
state to a safe/rolled-back value on failure (e.g., clear any partial flags or
set this.sync = false) so provider transitions remain resilient.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx (1)
70-83: Guard activeBranchData before renaming.
editorEngine.branches.activeBranchDatacan beundefined(e.g. while branches load or after deletion). Accessing.sandbox.gitManagerwill throw and crash the modal. Bail out early when no active branch data (show a toast + telemetry, then return) before callingaddCommitNote.
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
147-147: Consider avoiding negative margin.The negative margin
mt-[-8px]can cause unexpected layout issues and makes the spacing less maintainable. Consider adjusting the parent container's padding or using a more explicit layout approach.Apply this diff to remove the negative margin:
<Textarea ref={textareaRef} value={editValue} onChange={(e) => setEditValue(e.target.value)} - className="text-small mt-[-8px] resize-none border-none px-0" + className="text-small resize-none border-none px-0" rows={2}Then adjust the parent container's padding at line 224 if needed to achieve the desired spacing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(6 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.ts(2 hunks)apps/web/client/src/components/store/editor/git/utils.ts(1 hunks)apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx(7 hunks)packages/models/src/chat/message/checkpoint.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/models/src/chat/message/checkpoint.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/git/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/git/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.tsapps/web/client/src/components/store/editor/git/utils.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/components/store/editor/git/utils.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/components/store/editor/git/utils.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/messages/** : Add or modify internationalized strings in apps/web/client/messages/* instead of hardcoding text
Applied to files:
apps/web/client/src/components/store/editor/git/utils.ts
🧬 Code graph analysis (6)
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.ts (2)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)
apps/web/client/src/components/store/editor/git/utils.ts (2)
packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (4)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
EditMessage(19-23)packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/components/store/editor/git/utils.ts (1)
restoreCheckpoint(19-60)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx (1)
MultiBranchRevertModal(28-176)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (3)
apps/web/client/src/app/project/[id]/_hooks/use-chat/utils.ts (1)
createCheckpointsForAllBranches(36-65)packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx (2)
packages/git/src/git.ts (1)
updateCommitDisplayName(159-169)apps/web/client/src/components/store/editor/git/utils.ts (1)
restoreCheckpoint(19-60)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/multi-branch-revert-modal.tsx (3)
packages/models/src/chat/message/checkpoint.ts (1)
GitMessageCheckpoint(10-14)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/store/editor/git/utils.ts (1)
restoreCheckpoint(19-60)
🔇 Additional comments (5)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (5)
33-42: LGTM!The refactored
getUserMessageContentfunction is cleaner and more functional in style. The chainedmap/joinapproach is more concise than the previous implementation.
54-59: LGTM!The checkpoint extraction and legacy detection logic is clear and correct. The approach of falling back to legacy UI when any checkpoint lacks
branchIdis a safe, conservative strategy that prevents confusion when dealing with mixed checkpoint types.
103-114: Verify intentional inconsistency in toast messages.The
handleRetryfunction only provides an error message in the toast promise (line 103-106), whilesendMessageprovides loading, success, and error messages (line 109-114). This inconsistency might be intentional to reduce UI noise for retries, but it's worth confirming.Is the absence of loading/success feedback in
handleRetryintentional? If so, consider adding a comment explaining the reasoning. If not, apply this diff to align the behavior:const handleRetry = async () => { - toast.promise(onEditMessage(message.id, getUserMessageContent(message), ChatType.EDIT), { + toast.promise(onEditMessage(message.id, getUserMessageContent(message), ChatType.EDIT), { + loading: 'Resubmitting message...', + success: 'Message resubmitted successfully', error: 'Failed to resubmit message', }); };
116-138: LGTM!The restoration handler functions are well-structured:
handleRestoreSingleBranchprovides per-checkpoint restoration for the modern flowhandleRestoreLegacysafely handles legacy checkpoints by falling back to the first checkpointgetBranchNameprovides appropriate fallbacks for missing or undefined branch dataState management with
isRestoringis properly handled to prevent concurrent operations.
1-26: No 'use client' directive required: The parentchat-messages/index.tsxalready declares'use client'.
| toast.success( | ||
| `Successfully restored ${successCount} branch${successCount > 1 ? 'es' : ''}`, | ||
| { | ||
| description: | ||
| failCount > 0 | ||
| ? `${failCount} branch${failCount > 1 ? 'es' : ''} failed to restore` | ||
| : undefined, | ||
| }, | ||
| ); | ||
| } else if (failCount > 0) { | ||
| toast.error('Failed to restore all selected branches'); | ||
| } | ||
|
|
||
| setIsRestoring(false); | ||
| onOpenChange(false); | ||
| setSelectedBranchIds([]); | ||
| }; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent className="max-w-md"> | ||
| <DialogHeader> | ||
| <DialogTitle>Restore Multiple Branches</DialogTitle> | ||
| <DialogDescription className="pt-2"> | ||
| Select the branches you want to restore to their previous state. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <div className="flex flex-col gap-2 py-4"> | ||
| <div className="mb-1 flex justify-end gap-1"> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={selectAll} | ||
| disabled={isRestoring} | ||
| > | ||
| Select All | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={selectNone} | ||
| disabled={isRestoring} | ||
| > | ||
| Select None | ||
| </Button> | ||
| </div> | ||
| <div className="flex flex-col gap-2"> | ||
| {checkpoints.map((checkpoint) => { | ||
| // Skip legacy checkpoints without branchId (shouldn't happen in multi-branch modal) | ||
| if (!checkpoint.branchId) return null; | ||
| const isSelected = selectedBranchIds.includes(checkpoint.branchId); | ||
| return ( | ||
| <button | ||
| key={checkpoint.branchId} | ||
| onClick={() => toggleBranch(checkpoint.branchId!)} | ||
| disabled={isRestoring} | ||
| className={cn( | ||
| 'flex items-center justify-between rounded-md border px-3 py-2.5 text-left transition-all', | ||
| 'hover:bg-background-secondary/50', | ||
| isSelected | ||
| ? 'border-primary/50 bg-primary/5' | ||
| : 'border-border/50', | ||
| isRestoring && 'cursor-not-allowed opacity-50', | ||
| )} | ||
| > | ||
| <span className="text-sm"> | ||
| {editorEngine.branches.getBranchById(checkpoint.branchId) | ||
| ?.name ?? checkpoint.branchId} | ||
| </span> | ||
| {isSelected && <Icons.Check className="text-primary h-4 w-4" />} | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| <DialogFooter className="flex-col gap-3 sm:flex-row sm:gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => { | ||
| onOpenChange(false); | ||
| setSelectedBranchIds([]); | ||
| }} | ||
| disabled={isRestoring} | ||
| className="order-2 sm:order-1" | ||
| > | ||
| Cancel | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| onClick={handleRevert} | ||
| disabled={isRestoring || selectedBranchIds.length === 0} | ||
| className="order-1 sm:order-2" | ||
| > | ||
| {isRestoring ? 'Restoring...' : 'Revert Selected'} | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Localize modal and toast copy.
Every string shown to users here (“Restore Multiple Branches”, “Please select at least one branch…”, button labels, success/failure toasts, etc.) is hardcoded. For files under apps/web/client/src/**, we must serve user text via next-intl message keys. Inject the translator and replace these literals with localized strings.
Based on learnings
.../client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
Show resolved
Hide resolved
| {gitCheckpoints.map((checkpoint) => ( | ||
| <DropdownMenuItem | ||
| key={checkpoint.branchId} | ||
| onClick={() => handleRestoreSingleBranch(checkpoint)} | ||
| > | ||
| {getBranchName(checkpoint.branchId)} | ||
| </DropdownMenuItem> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type guard for branch checkpoints in modern path.
Although the logic ensures all checkpoints have branchId when hasLegacyCheckpoints is false, TypeScript doesn't infer this. The key at line 300 uses checkpoint.branchId which is typed as potentially undefined, creating a type-safety concern.
Apply this diff to make the type guarantee explicit:
- {gitCheckpoints.map((checkpoint) => (
+ {gitCheckpoints
+ .filter((cp): cp is GitMessageCheckpoint & { branchId: string } => !!cp.branchId)
+ .map((checkpoint) => (
<DropdownMenuItem
key={checkpoint.branchId}
onClick={() => handleRestoreSingleBranch(checkpoint)}
>
{getBranchName(checkpoint.branchId)}
</DropdownMenuItem>
))}This ensures TypeScript knows checkpoint.branchId is defined and prevents potential runtime issues if the logic ever changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {gitCheckpoints.map((checkpoint) => ( | |
| <DropdownMenuItem | |
| key={checkpoint.branchId} | |
| onClick={() => handleRestoreSingleBranch(checkpoint)} | |
| > | |
| {getBranchName(checkpoint.branchId)} | |
| </DropdownMenuItem> | |
| ))} | |
| {gitCheckpoints | |
| .filter( | |
| (cp): cp is GitMessageCheckpoint & { branchId: string } => | |
| !!cp.branchId | |
| ) | |
| .map((checkpoint) => ( | |
| <DropdownMenuItem | |
| key={checkpoint.branchId} | |
| onClick={() => | |
| handleRestoreSingleBranch(checkpoint) | |
| } | |
| > | |
| {getBranchName(checkpoint.branchId)} | |
| </DropdownMenuItem> | |
| ))} |
| toast.error('Branch not found'); | ||
| return { success: false, error: 'Branch not found' }; | ||
| } | ||
|
|
||
| // Save current state before restoring | ||
| const saveResult = await branchData.sandbox.gitManager.createCommit(BACKUP_COMMIT_MESSAGE); | ||
| if (!saveResult.success) { | ||
| toast.warning('Failed to save before restoring backup'); | ||
| } | ||
|
|
||
| // Restore to the specified commit | ||
| const restoreResult = await branchData.sandbox.gitManager.restoreToCommit(checkpoint.oid); | ||
|
|
||
| if (!restoreResult.success) { | ||
| throw new Error(restoreResult.error || 'Failed to restore commit'); | ||
| } | ||
|
|
||
| const branchName = editorEngine.branches.getBranchById(targetBranchId)?.name || targetBranchId; | ||
| toast.success('Restored to backup!', { | ||
| description: `Branch "${branchName}" has been restored`, | ||
| }); | ||
|
|
||
| return { success: true }; | ||
| } catch (error) { | ||
| console.error('Failed to restore checkpoint:', error); | ||
| const errorMessage = error instanceof Error ? error.message : 'Unknown error'; | ||
| toast.error('Failed to restore checkpoint', { | ||
| description: errorMessage, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace hardcoded toast text with translations.
The new restore flow surfaces several literal strings (“Branch not found”, “Failed to save before restoring backup”, “Restored to backup!”, etc.). These must come from next-intl resources rather than inline English per our client UI localization guidelines. Wire up the translation hook/scoped messages and pass the translated copy into the toasts.
Based on learnings
| if (!result.success) { | ||
| toast.error('Failed to rename backup'); | ||
| editorEngine.posthog.capture('versions_rename_commit_failed', { | ||
| commit: commit.oid, | ||
| newName: name, | ||
| error: result.error, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| toast.success('Backup renamed successfully!', { | ||
| description: `Renamed to: "${name}"`, | ||
| }); | ||
|
|
||
| editorEngine.posthog.capture('versions_rename_commit_success', { | ||
| commit: commit.oid, | ||
| newName: name, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Localize toast copy instead of hardcoding.
All newly added toast messages (“Failed to rename backup”, “Backup renamed successfully!”, etc.) are literal strings. Per our client guidelines we must source user-visible text from next-intl/translations instead of hardcoding English. Refactor to use the appropriate t lookup with message keys.
Based on learnings
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
around lines 74-91, the toast messages are hardcoded English strings; replace
them with localized strings via the next-intl translations (use the component's
existing t/useTranslations hook or add one if missing). Swap toast.error('Failed
to rename backup') for toast.error(t('versions.rename_failed')) and
toast.success('Backup renamed successfully!', { description: `Renamed to:
"${name}"` }) for toast.success(t('versions.rename_success_title'), {
description: t('versions.rename_success_description', { name }) }) (or use your
agreed message keys), and pass any dynamic values through the translation
interpolation; ensure message keys are added to the locale files.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
54-77: HandlegitManager.createCommit()errors.Line 60 awaits
gitManager.createCommit()without catching rejections. If the promise throws, the handler bubbles an error, no toast displays, and the UI gives no feedback. Wrap the call intry/catch, keep the existing success/error result handling, and surface unexpected exceptions via a toast.const result = await gitManager.createCommit(); - if (!result.success) { - toast.error('Failed to create backup', { - description: result.error || 'Unknown error', - }); - return; - } - - toast.success('Backup created successfully!'); - editorEngine.posthog.capture('versions_create_commit_success'); + try { + if (!result.success) { + toast.error('Failed to create backup', { + description: result.error || 'Unknown error', + }); + return; + } + + toast.success('Backup created successfully!'); + editorEngine.posthog.capture('versions_create_commit_success'); + } catch (error) { + const description = error instanceof Error ? error.message : 'Unknown error'; + toast.error('Failed to create backup', { description }); + }
55-113: Replace hardcoded UI strings with translations.Lines 56 (“Git not initialized”), 62 (“Failed to create backup”), 68 (“Backup created successfully!”), 83 (“Versions”), 92 (“Select branch”), and 111 (“New backup”) all introduce literal English text. The client guideline for
apps/web/client/src/**/*requires UI copy and toasts to come fromnext-intl. Please wire upuseTranslations, move these strings into the locale resources, and reference the keys here (including toast descriptions). As per coding guidelines.
♻️ Duplicate comments (1)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx (1)
70-96: Localize rename toasts as previously requested.Lines 73-95 still emit literal English (“No active branch”, “Failed to rename backup”, “Backup renamed successfully!”). This repeats the earlier review note, and the guideline for
apps/web/client/src/**/*mandatesnext-intlmessages instead of hardcoded strings. Please hook upuseTranslations, move these strings into the locale files (with the renamed value passed as interpolation), and reuse them here. As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx(2 hunks)apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx(7 hunks)apps/web/client/src/components/ui/settings-modal/versions/versions.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsxapps/web/client/src/components/ui/settings-modal/versions/versions.tsxapps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx
🧬 Code graph analysis (3)
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx (2)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/ui/src/components/sonner.tsx (1)
toast(19-19)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/ui/src/components/select.tsx (5)
Select(158-158)SelectTrigger(166-166)SelectValue(167-167)SelectContent(159-159)SelectItem(161-161)
apps/web/client/src/components/ui/settings-modal/versions/version-row.tsx (2)
packages/git/src/git.ts (1)
updateCommitDisplayName(159-169)apps/web/client/src/components/store/editor/git/utils.ts (1)
restoreCheckpoint(19-60)
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx
Show resolved
Hide resolved
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx
Outdated
Show resolved
Hide resolved
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
Outdated
Show resolved
Hide resolved
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
🧬 Code graph analysis (1)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (1)
packages/ui/src/components/select.tsx (5)
Select(158-158)SelectTrigger(166-166)SelectValue(167-167)SelectContent(159-159)SelectItem(161-161)
🔇 Additional comments (2)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
54-86: Well-structured error handling and user feedback.The refactored
handleNewBackupfunction demonstrates solid error handling practices:
- Validates
gitManagerpresence before attempting operations- Handles both expected failures (result.success check) and unexpected errors (catch block)
- Provides clear user feedback via toast notifications
- Properly manages loading state in the finally block
98-98: Verified: branch switch is synchronous and triggers re-render.switchToBranchis a MobX action that setscurrentBranchIdimmediately, andVersionsis wrapped inobserver, soselectedBranchIdand downstream data (commits) update on the next render.
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (1)
91-101: Replace hardcoded UI strings with next‑intlConvert "Backup Versions", "Select branch", and "New backup" to translations.
- <h2 className="text-lg">Backup Versions</h2> + <h2 className="text-lg">{t('title')}</h2> ... - <SelectValue placeholder="Select branch" /> + <SelectValue placeholder={t('selectBranchPlaceholder')} /> ... - New backup + {t('newBackup')}Additionally (outside this hunk), ensure:
import { useTranslations } from 'next-intl'; // ... const t = useTranslations('versions');And add keys in apps/web/client/messages/*:
- versions.title
- versions.selectBranchPlaceholder
- versions.newBackup
Based on learnings.
Also applies to: 122-122
🧹 Nitpick comments (2)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
16-22: Guard against undefined activeBranch/sandbox to avoid crashesAccessing
editorEngine.branches.activeBranch.idcan throw if the branch store hasn’t initialized yet. Add a small guard to prevent a render‑time NPE.- const [isCreatingBackup, setIsCreatingBackup] = useState(false); - const selectedBranchId = editorEngine.branches.activeBranch.id; - const branchData = editorEngine.branches.getBranchDataById(selectedBranchId); - const gitManager = branchData?.sandbox.gitManager; + const [isCreatingBackup, setIsCreatingBackup] = useState(false); + const activeBranch = editorEngine.branches.activeBranch; + if (!activeBranch) { + // Optionally render a lightweight skeleton/spinner here + return null; + } + const selectedBranchId = activeBranch.id; + const branchData = editorEngine.branches.getBranchDataById(selectedBranchId); + const gitManager = branchData?.sandbox?.gitManager;
55-85: Internationalize toast messages and avoid racing on “latest commit” after create
- Replace hardcoded toast strings with next‑intl messages.
- Rely on the create API’s returned oid (if available) instead of
commits?.[0], which can race with async updates. Fallback to readingcommits?.[0]only if the API doesn’t return an oid.- if (!gitManager) { - toast.error('Git not initialized'); + if (!gitManager) { + toast.error(t('gitNotInitialized')); return; } - const result = await gitManager.createCommit(); + const result = await gitManager.createCommit(); if (!result.success) { - toast.error('Failed to create backup', { - description: result.error || 'Unknown error', + toast.error(t('createBackupFailed'), { + description: result.error || t('unknownError'), }); return; } - toast.success('Backup created successfully!'); + toast.success(t('backupCreated')); editorEngine.posthog.capture('versions_create_commit_success'); - const latestCommit = gitManager.commits?.[0]; - if (!latestCommit) { - console.error('No latest commit found'); - return; - } - setCommitToRename(latestCommit.oid); + // Prefer returned oid if the API exposes it + const createdOid = (result as any)?.commit?.oid ?? (result as any)?.oid; + if (createdOid) { + setCommitToRename(createdOid); + } else { + const latestCommit = gitManager.commits?.[0]; + if (!latestCommit) { + console.error('No latest commit found'); + return; + } + setCommitToRename(latestCommit.oid); + } } catch (error) { - toast.error('Failed to create backup', { - description: error instanceof Error ? error.message : 'Unknown error', + toast.error(t('createBackupFailed'), { + description: error instanceof Error ? error.message : t('unknownError'), });Also add (outside this hunk) at the top of the component:
import { useTranslations } from 'next-intl'; // ... const t = useTranslations('versions');As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx(2 hunks)apps/web/client/src/components/ui/settings-modal/versions/versions.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/components/ui/settings-modal/versions/empty-state/version.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/messages/** : Add or modify internationalized strings in apps/web/client/messages/* instead of hardcoding text
Applied to files:
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx
🧬 Code graph analysis (1)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (2)
packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/ui/src/components/select.tsx (5)
Select(158-158)SelectTrigger(166-166)SelectValue(167-167)SelectContent(159-159)SelectItem(161-161)
🔇 Additional comments (1)
apps/web/client/src/components/ui/settings-modal/versions/versions.tsx (1)
97-116: Branch switching and button gating look good
- Selector updates
editorEngine.branches.switchToBranch(value)— fixes prior desync.- “New backup” button gated on
gitManagerand disabled while loading/creating — prevents duplicate requests.
| const handleRevert = async () => { | ||
| try { | ||
| if (selectedBranchIds.length === 0) { | ||
| toast.error('Please select at least one branch to revert'); |
There was a problem hiding this comment.
Inconsistent terminology: The error message uses 'revert' while other parts of the UI (and function names) use 'restore'. Please consider using a single term for clarity.
| toast.error('Please select at least one branch to revert'); | |
| toast.error('Please select at least one branch to restore'); |
|
|
||
| const result = await restoreCheckpoint(checkpoint, editorEngine); | ||
|
|
||
| setIsCheckingOut(false); |
There was a problem hiding this comment.
Redundant state update: setIsCheckingOut(false) is called inside try and again in the finally block. Consider removing the inner call.
| setIsCheckingOut(false); |
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
This PR adds multi-branch revert functionality, refactors versioning to be branch-based, and updates the database schema for branch-specific checkpoints.
MultiBranchRevertModalinmulti-branch-revert-modal.tsxfor reverting multiple branches with UI support for selecting branches.user-message.tsxto handle legacy and multi-branch checkpoints with a dropdown menu for branch selection.createCheckpointsForAllBranchesinutils.tsto create checkpoints for all branches.VersionsManagerand integrates versioning intoGitManageringit.ts.engine.tsto removeversionsand use branch-based versioning.message.tsto includebranchIdin checkpoints and updates theupdateCheckpointsmutation.restoreCheckpointfunction ingit/utils.tsfor restoring branches to specific checkpoints.versions.tsxandversion-row.tsxto reflect new branch-based versioning.This description was created by
for 33cdbc8. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Refactor
Other