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
This commit is contained in:
Shawn 2025-09-01 09:18:26 -04:00 committed by Hannes Rudolph
parent 3048471ae1
commit bb2ae8d7dd
4 changed files with 11 additions and 40 deletions

View file

@ -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) {

View file

@ -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
}

View file

@ -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
}

View file

@ -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",
})