From bb2ae8d7ddd08d9810707610164022ad76912ca9 Mon Sep 17 00:00:00 2001 From: Shawn <5414767+playcations@users.noreply.github.com> Date: Mon, 1 Sep 2025 09:18:26 -0400 Subject: [PATCH] ix: code quality improvements for FCO feature and address more comments - Remove duplicate enableCheckpoints check in checkpoints/index.ts - Remove unused _CheckpointEventData interface from FilesChangedOverview.tsx - Fix message type naming consistency: checkpoint_created -> checkpointCreated, checkpoint_restored -> checkpointRestored - Remove debug console.log statements from checkpoints/index.ts --- src/core/checkpoints/index.ts | 33 +++---------------- src/shared/ExtensionMessage.ts | 6 ++-- .../file-changes/FilesChangedOverview.tsx | 8 +---- .../__tests__/FilesChangedOverview.spec.tsx | 4 +-- 4 files changed, 11 insertions(+), 40 deletions(-) diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index c60e803134..e143dc37f0 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -43,8 +43,6 @@ export async function getCheckpointService( } } - console.log("[Task#getCheckpointService] initializing checkpoints service") - try { const workspaceDir = task.cwd || getWorkspacePath() @@ -72,7 +70,6 @@ export async function getCheckpointService( if (task.checkpointServiceInitializing) { await pWaitFor( () => { - console.log("[Task#getCheckpointService] waiting for service to initialize") return !!task.checkpointService && !!task?.checkpointService?.isInitialized }, { interval, timeout }, @@ -133,22 +130,9 @@ async function checkGitInstallation( log("[Task#getCheckpointService] service initialized") try { - // Debug logging to understand checkpoint detection - console.log("[DEBUG] Checkpoint detection - total messages:", task.clineMessages.length) - console.log( - "[DEBUG] Checkpoint detection - message types:", - task.clineMessages.map((m) => ({ ts: m.ts, type: m.type, say: m.say, ask: m.ask })), - ) - const checkpointMessages = task.clineMessages.filter(({ say }) => say === "checkpoint_saved") - console.log( - "[DEBUG] Found checkpoint messages:", - checkpointMessages.length, - checkpointMessages.map((m) => ({ ts: m.ts, text: m.text })), - ) const isCheckpointNeeded = checkpointMessages.length === 0 - console.log("[DEBUG] isCheckpointNeeded result:", isCheckpointNeeded) task.checkpointService = service task.checkpointServiceInitializing = false @@ -375,7 +359,6 @@ export async function getInitializedCheckpointService( try { await pWaitFor( () => { - console.log("[Task#getCheckpointService] waiting for service to initialize") return service.isInitialized }, { interval, timeout }, @@ -418,20 +401,17 @@ export async function checkpointSave(task: Task, force = false, files?: vscode.U const provider = task.providerRef.deref() // Capture the previous checkpoint BEFORE saving the new one - const previousCheckpoint = service.baseHash - console.log(`[checkpointSave] Previous checkpoint: ${previousCheckpoint}`) + const previousCheckpoint = service.getCurrentCheckpoint() // Start the checkpoint process in the background and track it const savePromise = service .saveCheckpoint(`Task: ${task.taskId}, Time: ${Date.now()}`, { allowEmpty: force, files, suppressMessage }) .then(async (result: any) => { - console.log(`[checkpointSave] New checkpoint created: ${result?.commit}`) - // Notify FCO that checkpoint was created if (provider && result) { try { provider.postMessageToWebview({ - type: "checkpoint_created", + type: "checkpointCreated", checkpoint: result.commit, previousCheckpoint: previousCheckpoint, } as any) @@ -440,9 +420,6 @@ export async function checkpointSave(task: Task, force = false, files?: vscode.U // to avoid duplicate/conflicting messages that override cumulative tracking. // The checkpoint event handler calculates cumulative changes from the baseline // and sends the complete filesChanged message with all accumulated changes. - console.log( - `[checkpointSave] FCO update delegated to checkpoint event for cumulative tracking`, - ) } catch (error) { console.error("[Task#checkpointSave] Failed to notify FCO of checkpoint creation:", error) } @@ -509,8 +486,8 @@ export async function checkpointRestore( } // Calculate and send current changes with LLM-only filtering (should be empty immediately after restore) - if (cline.taskId && cline.fileContextTracker) { - const changes = await fileChangeManager.getLLMOnlyChanges(cline.taskId, cline.fileContextTracker) + if (task.taskId && task.fileContextTracker) { + const changes = await fileChangeManager.getLLMOnlyChanges(task.taskId, task.fileContextTracker) provider?.postMessageToWebview({ type: "filesChanged", filesChanged: changes.files.length > 0 ? changes : undefined, @@ -525,7 +502,7 @@ export async function checkpointRestore( // Notify FCO that checkpoint was restored try { await provider?.postMessageToWebview({ - type: "checkpoint_restored", + type: "checkpointRestored", checkpoint: commitHash, } as any) } catch (error) { diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index b90ab373da..4c7e35428c 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -129,8 +129,8 @@ export interface ExtensionMessage { | "commands" | "insertTextIntoTextarea" | "filesChanged" - | "checkpoint_created" - | "checkpoint_restored" + | "checkpointCreated" + | "checkpointRestored" | "say" text?: string payload?: any // Add a generic payload for now, can refine later @@ -209,7 +209,7 @@ export interface ExtensionMessage { commands?: Command[] queuedMessages?: QueuedMessage[] filesChanged?: FileChangeset // Added filesChanged property - checkpoint?: string // For checkpoint_created and checkpoint_restored messages + checkpoint?: string // For checkpointCreated and checkpointRestored messages previousCheckpoint?: string // For checkpoint_created message say?: ClineSay // Added say property } diff --git a/webview-ui/src/components/file-changes/FilesChangedOverview.tsx b/webview-ui/src/components/file-changes/FilesChangedOverview.tsx index 2414cdc17b..78a3a8e364 100644 --- a/webview-ui/src/components/file-changes/FilesChangedOverview.tsx +++ b/webview-ui/src/components/file-changes/FilesChangedOverview.tsx @@ -5,12 +5,6 @@ import { useExtensionState } from "@/context/ExtensionStateContext" import { vscode } from "@/utils/vscode" import { useDebouncedAction } from "@/components/ui/hooks/useDebouncedAction" -interface _CheckpointEventData { - type: "checkpoint_created" | "checkpoint_restored" - checkpoint: string - previousCheckpoint?: string -} - /** * FilesChangedOverview is a self-managing component that listens for checkpoint events * and displays file changes. It manages its own state and communicates with the backend @@ -149,7 +143,7 @@ const FilesChangedOverview: React.FC = () => { case "checkpoint_created": handleCheckpointCreated(message.checkpoint, message.previousCheckpoint) break - case "checkpoint_restored": + case "checkpointRestored": handleCheckpointRestored(message.checkpoint) break } diff --git a/webview-ui/src/components/file-changes/__tests__/FilesChangedOverview.spec.tsx b/webview-ui/src/components/file-changes/__tests__/FilesChangedOverview.spec.tsx index ab2b18ab9d..abadb7fd6e 100644 --- a/webview-ui/src/components/file-changes/__tests__/FilesChangedOverview.spec.tsx +++ b/webview-ui/src/components/file-changes/__tests__/FilesChangedOverview.spec.tsx @@ -233,7 +233,7 @@ describe("FilesChangedOverview (Self-Managing)", () => { }) }) - it("should handle checkpoint_restored message", async () => { + it("should handle checkpointRestored message", async () => { renderComponent() // First set up some files @@ -248,7 +248,7 @@ describe("FilesChangedOverview (Self-Managing)", () => { // Simulate checkpoint restore simulateMessage({ - type: "checkpoint_restored", + type: "checkpointRestored", checkpoint: "restored-checkpoint-hash", })