Skip to content

made pending#48

Merged
AviPeltz merged 1 commit intomainfrom
improve-diff-view
Nov 9, 2025
Merged

made pending#48
AviPeltz merged 1 commit intomainfrom
improve-diff-view

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Nov 9, 2025

Description

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

Release Notes

  • New Features
    • Added optimistic UI updates for worktree creation—new worktrees now appear immediately in the interface while being created in the background.
    • Added loading indicators to provide visual feedback during worktree processing.
    • Implemented safeguards to prevent interaction with pending worktrees during creation, ensuring UI consistency.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 9, 2025

Walkthrough

The changes implement optimistic updates for worktree creation by introducing a PendingWorktree model and isPending flag. When users create or select worktrees for tasks, pending entries are immediately added to state, merged into the worktree list via enrichWorktreesWithTasks, and displayed with spinner indicators. Pending entries are removed upon completion or failure.

Changes

Cohort / File(s) Summary
Core optimistic update logic
apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx
Introduces PendingWorktree model and pendingWorktrees state. Updates enrichWorktreesWithTasks signature to accept and merge pending worktrees with real ones. Modifies task/worktree creation flows to add optimistic entries immediately and clean them up on success or failure. UI guards filter out pending IDs to prevent selection.
UI pending state handling
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx
Adds isPending optional field to WorktreeWithTask interface. Imports Loader2 spinner component. Disables worktree selection button during pending state with reduced opacity and wait cursor. Replaces status indicators with spinner when isPending is true. Displays dedicated pending UI block in hover card instead of normal task details.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as TaskTabs UI
    participant Logic as NewLayoutMain
    participant State as Worktree State
    participant Backend

    User->>Logic: Select/Create Task
    Logic->>State: Add PendingWorktree (optimistic)
    Logic->>Logic: enrichWorktreesWithTasks(worktrees, pendingWorktrees)
    Logic->>UI: Pass enriched list with isPending=true
    UI->>UI: Render Loader2 spinner, disable button
    
    par Async Operation
        Backend->>Backend: Process worktree creation
    end
    
    alt Success
        Backend-->>Logic: Worktree created
        Logic->>State: Remove PendingWorktree
        Logic->>UI: Update with real worktree
        UI->>UI: Hide spinner, enable button
    else Failure
        Backend-->>Logic: Creation failed
        Logic->>State: Remove PendingWorktree
        UI->>UI: Return to normal state
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • enrichWorktreesWithTasks signature change: Verify all call sites are updated and pending-to-real worktree merging logic is correct
  • Optimistic state management: Review PendingWorktree model definition, state lifecycle (creation/cleanup), and edge cases where pending entries may not be properly removed
  • UI guard logic: Confirm pending ID filtering (pending-*) is consistently applied across all worktree selection paths
  • isPending flag propagation: Ensure the flag flows correctly from enriched worktrees through UI components and doesn't cause stale pending states

Poem

🐰 A rabbit hops through worktrees bright,
Pending states shown with spinner's light,
Optimistic leaps before they're real,
Fast feedback loops—what a deal!
✨ Hop on, hop on, with grace so fleet!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is completely empty except for the template structure; all required sections lack substantive content and no issues, tests, or change type are documented. Fill in all template sections with details: describe the optimistic update implementation, link related issues, check the change type, and document testing steps performed.
Title check ❓ Inconclusive The title 'made pending' is vague and generic, failing to clearly communicate the main change despite implementing optimistic updates for worktrees. Use a more descriptive title like 'Add optimistic updates for worktree creation' or 'Implement pending worktree UI state' to clearly convey the primary change.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-diff-view

Comment @coderabbitai help to get the list of available commands and usage tips.

@AviPeltz AviPeltz merged commit c898e5e into main Nov 9, 2025
1 of 5 checks passed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (1)

592-676: Prevent duplicate pending worktrees on repeated selections.

If a user re-triggers task selection/creation while the first optimistic request is still pending, we enqueue another PendingWorktree for the same branch and fire a second worktree-create. That leads to duplicate entries, redundant IPC traffic, and confusing UI updates. Guard both flows by short-circuiting when a pending entry already exists for the branch before appending to state.

 	if (existingWorktree) {
 		// Worktree already exists - switch to it
 		…
 	} else {
+		if (pendingWorktrees.some((wt) => wt.branch === task.branch)) {
+			handleCloseAddTaskModal();
+			return;
+		}
 		// Worktree doesn't exist - create it with optimistic update
 		…
 	}
 	if (!currentWorkspace) return;

+	if (pendingWorktrees.some((wt) => wt.branch === taskData.branch)) {
+		handleCloseAddTaskModal();
+		return;
+	}
 	// Create pending worktree for optimistic update
 	const pendingId = `pending-${Date.now()}`;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b587cc and 44a6805.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (10 hunks)
  • apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (2)
apps/desktop/src/shared/types.ts (1)
  • Worktree (58-67)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
  • StatusIndicator (53-113)
apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (3)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
  • TaskStatus (3-9)
apps/desktop/src/shared/types.ts (1)
  • Worktree (58-67)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (1)
  • WorktreeWithTask (15-29)
🪛 GitHub Actions: CI
apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx

[error] 234-234: TS2322: Type '{ id: string; branch: string; path: string; tabs: never[]; isPending: true; task: { id: string; slug: string; title: string; status: TaskStatus; description: string; } | undefined; }[]' is not assignable to type 'WorktreeWithTask[]'. Property 'createdAt' is missing in type '{ id: string; branch: string; path: string; tabs: never[]; isPending: true; task: { id: string; slug: string; title: string; status: TaskStatus; description: string; } | undefined; }' but required in type 'WorktreeWithTask'.

Comment on lines +234 to +253
const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map(
(pending) => ({
id: pending.id,
branch: pending.branch,
path: "", // Pending worktrees don't have a path yet
tabs: [],
isPending: true, // Mark as pending for UI
task: pending.taskData
? {
id: pending.id,
slug: pending.taskData.slug,
title: pending.taskData.name,
status: pending.taskData.status,
description: pending.description || "",
}
: undefined,
}),
);

// Then, enrich real worktrees with task metadata
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Build breaks: populate required Worktree fields for pending entries.

WorktreeWithTask extends Worktree, so every pending object you create here must satisfy the base interface. Right now the build fails with TS2322 because createdAt is missing, and title isn’t a known property on Worktree. Please supply the required fields and drop the extra property so pending entries compile like the real ones.

 const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map(
 	(pending) => ({
 		id: pending.id,
 		branch: pending.branch,
-		path: "", // Pending worktrees don't have a path yet
-		tabs: [],
-		isPending: true, // Mark as pending for UI
-		task: pending.taskData
+		path: "", // Pending worktrees don't have a path yet
+		createdAt: new Date().toISOString(),
+		tabs: [],
+		description: pending.description,
+		isPending: true, // Mark as pending for UI
+		task: pending.taskData
 			? {
 					id: pending.id,
 					slug: pending.taskData.slug,
 					title: pending.taskData.name,
 					status: pending.taskData.status,
 					description: pending.description || "",
 				}
 			: undefined,
 	}),
 );
📝 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.

Suggested change
const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map(
(pending) => ({
id: pending.id,
branch: pending.branch,
path: "", // Pending worktrees don't have a path yet
tabs: [],
isPending: true, // Mark as pending for UI
task: pending.taskData
? {
id: pending.id,
slug: pending.taskData.slug,
title: pending.taskData.name,
status: pending.taskData.status,
description: pending.description || "",
}
: undefined,
}),
);
// Then, enrich real worktrees with task metadata
const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map(
(pending) => ({
id: pending.id,
branch: pending.branch,
path: "", // Pending worktrees don't have a path yet
createdAt: new Date().toISOString(),
tabs: [],
description: pending.description,
isPending: true, // Mark as pending for UI
task: pending.taskData
? {
id: pending.id,
slug: pending.taskData.slug,
title: pending.taskData.name,
status: pending.taskData.status,
description: pending.description || "",
}
: undefined,
}),
);
🧰 Tools
🪛 GitHub Actions: CI

[error] 234-234: TS2322: Type '{ id: string; branch: string; path: string; tabs: never[]; isPending: true; task: { id: string; slug: string; title: string; status: TaskStatus; description: string; } | undefined; }[]' is not assignable to type 'WorktreeWithTask[]'. Property 'createdAt' is missing in type '{ id: string; branch: string; path: string; tabs: never[]; isPending: true; task: { id: string; slug: string; title: string; status: TaskStatus; description: string; } | undefined; }' but required in type 'WorktreeWithTask'.

🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx
around lines 234 to 253, the temporary pending worktree objects are missing
required Worktree fields and include an extra property that breaks typing; add a
createdAt field on each pending worktree (e.g. use pending.createdAt if present
or set to a current ISO timestamp) so the object satisfies the Worktree base
interface, and remove the non-existent title property from the nested task
object (keep slug/name/status/description as before) so the pending entries
compile like the real ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant