From d2c05e3a34d180aea3cf5022f7f160e82bbff6eb Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Mon, 28 Sep 2026 16:44:27 +0300 Subject: [PATCH] refactor(code-index): extract scan execution without behavior changes --- .../code-index-scan-executor.spec.ts | 149 ++++++++++++++++++ .../code-index/__tests__/orchestrator.spec.ts | 66 ++++++++ .../code-index/code-index-scan-executor.ts | 101 ++++++++++++ src/services/code-index/orchestrator.ts | 141 ++--------------- 4 files changed, 325 insertions(+), 132 deletions(-) create mode 100644 src/services/code-index/__tests__/code-index-scan-executor.spec.ts create mode 100644 src/services/code-index/code-index-scan-executor.ts diff --git a/src/services/code-index/__tests__/code-index-scan-executor.spec.ts b/src/services/code-index/__tests__/code-index-scan-executor.spec.ts new file mode 100644 index 0000000000..33b71584ff --- /dev/null +++ b/src/services/code-index/__tests__/code-index-scan-executor.spec.ts @@ -0,0 +1,149 @@ +import { CodeIndexScanExecutor } from "../code-index-scan-executor" +import type { IDirectoryScanner } from "../interfaces" + +vi.mock("../../../i18n", () => ({ t: (key: string) => key })) + +function setup() { + const scanner = { scanDirectory: vi.fn() } + const vectorStore = { markIndexingIncomplete: vi.fn().mockResolvedValue(undefined) } + const stateManager = { setSystemState: vi.fn(), reportBlockIndexingProgress: vi.fn() } + return { + scanner, + vectorStore, + stateManager, + executor: new CodeIndexScanExecutor("/workspace", scanner, vectorStore, stateManager), + } +} + +describe("CodeIndexScanExecutor", () => { + it.each(["runFullScan", "runIncrementalScan"] as const)( + "%s skips failure validation when cancellation occurs during the scan", + async (method) => { + const { executor, scanner } = setup() + const controller = new AbortController() + scanner.scanDirectory.mockImplementation(async (_path, onError, _onIndexed, onParsed) => { + onParsed?.(3) + onError?.(new Error("batch failure")) + controller.abort() + return { stats: { processed: 0, skipped: 0 }, totalBlockCount: 3 } + }) + await expect(executor[method](controller.signal)).resolves.toBe(false) + }, + ) + + it("completes an incremental scan with no changed files", async () => { + const { executor, scanner } = setup() + scanner.scanDirectory.mockResolvedValue({ stats: { processed: 0, skipped: 2 }, totalBlockCount: 0 }) + await expect(executor.runIncrementalScan(new AbortController().signal)).resolves.toBe(true) + }) + + it.each(["runFullScan", "runIncrementalScan"] as const)( + "%s preserves progress and signal wiring", + async (method) => { + const { executor, scanner, vectorStore, stateManager } = setup() + const signal = new AbortController().signal + scanner.scanDirectory.mockImplementation(async (path, _onError, onIndexed, onParsed, receivedSignal) => { + expect(path).toBe("/workspace") + expect(receivedSignal).toBe(signal) + expect(vectorStore.markIndexingIncomplete).toHaveBeenCalledOnce() + onParsed?.(2) + onIndexed?.(1) + onParsed?.(1) + onIndexed?.(2) + return { stats: { processed: 2, skipped: 0 }, totalBlockCount: 3 } + }) + await expect(executor[method](signal)).resolves.toBe(true) + expect(stateManager.reportBlockIndexingProgress.mock.calls).toEqual([ + [0, 2], + [1, 2], + [1, 3], + [3, 3], + ]) + expect(stateManager.setSystemState).toHaveBeenCalledExactlyOnceWith( + "Indexing", + method === "runFullScan" + ? "Services ready. Starting workspace scan..." + : "Checking for new or modified files...", + ) + }, + ) + + it.each(["runFullScan", "runIncrementalScan"] as const)("%s rejects missing results", async (method) => { + const { executor } = setup() + // An unconfigured mock simulates a scanner violating its return contract. + await expect(executor[method](new AbortController().signal)).rejects.toThrow( + method === "runFullScan" + ? "Scan failed, is scanner initialized?" + : "Incremental scan failed, is scanner initialized?", + ) + }) + + it.each(["runFullScan", "runIncrementalScan"] as const)( + "%s returns cancellation before validating results", + async (method) => { + const { executor, scanner } = setup() + const controller = new AbortController() + // Abort before the unconfigured scanner returns a missing result. + controller.abort() + await expect(executor[method](controller.signal)).resolves.toBe(false) + expect(scanner.scanDirectory).toHaveBeenCalledOnce() + }, + ) + + it.each(["runFullScan", "runIncrementalScan"] as const)( + "%s propagates scanner and marker failures unchanged", + async (method) => { + const { executor, scanner, vectorStore } = setup() + const signal = new AbortController().signal + const failure = new Error("scan failed") + scanner.scanDirectory.mockRejectedValueOnce(failure) + await expect(executor[method](signal)).rejects.toBe(failure) + const abort = new DOMException("stopped", "AbortError") + scanner.scanDirectory.mockRejectedValueOnce(abort) + await expect(executor[method](signal)).rejects.toBe(abort) + vectorStore.markIndexingIncomplete.mockRejectedValueOnce(failure) + await expect(executor[method](signal)).rejects.toBe(failure) + expect(scanner.scanDirectory).toHaveBeenCalledTimes(2) + }, + ) + + it.each([ + { found: 0, indexed: 0, error: false, message: undefined }, + { found: 3, indexed: 0, error: false, message: "embeddings:orchestrator.indexingFailedNoBlocks" }, + { found: 3, indexed: 0, error: true, message: "Indexing failed: batch failure" }, + { found: 0, indexed: 0, error: true, message: "Indexing failed completely: batch failure" }, + { found: 10, indexed: 9, error: true, message: undefined }, + { + found: 10, + indexed: 8, + error: true, + message: "Indexing partially failed: Only 8 of 10 blocks were indexed. batch failure", + }, + { found: 10, indexed: 8, error: false, message: undefined }, + ])( + "preserves full-scan policy for $indexed/$found blocks, error=$error", + async ({ found, indexed, error, message }) => { + const { executor, scanner } = setup() + scanner.scanDirectory.mockImplementation(async (_path, onError, onIndexed, onParsed) => { + onParsed?.(found) + onIndexed?.(indexed) + if (error) onError?.(new Error("batch failure")) + return { stats: { processed: 1, skipped: 0 }, totalBlockCount: found } + }) + const result = executor.runFullScan(new AbortController().signal) + if (message) await expect(result).rejects.toThrow(message) + else await expect(result).resolves.toBe(true) + }, + ) + + it.each([0, 2])("preserves incremental batch-error tolerance with %s indexed blocks", async (indexed) => { + const { executor, scanner } = setup() + scanner.scanDirectory.mockImplementation(async (_path, onError, onIndexed, onParsed) => { + onParsed?.(3) + onIndexed?.(indexed) + onError?.(new Error("batch failure")) + return { stats: { processed: 1, skipped: 0 }, totalBlockCount: 3 } + }) + await expect(executor.runIncrementalScan(new AbortController().signal)).resolves.toBe(true) + }) +}) diff --git a/src/services/code-index/__tests__/orchestrator.spec.ts b/src/services/code-index/__tests__/orchestrator.spec.ts index 86b0f94808..bad3d69be3 100644 --- a/src/services/code-index/__tests__/orchestrator.spec.ts +++ b/src/services/code-index/__tests__/orchestrator.spec.ts @@ -107,6 +107,72 @@ describe("CodeIndexOrchestrator - error path cleanup gating", () => { } }) + it.each([false, true])( + "keeps watcher startup and completion after scanning (incremental: %s)", + async (incremental) => { + const events: string[] = [] + vectorStore.initialize.mockResolvedValue(false) + vectorStore.hasIndexedData.mockResolvedValue(incremental) + vectorStore.markIndexingIncomplete.mockImplementation(async () => { + events.push("incomplete") + }) + scanner.scanDirectory.mockImplementation(async () => { + events.push("scan") + return { stats: { processed: 0, skipped: 0 }, totalBlockCount: 0 } + }) + fileWatcher.initialize.mockImplementation(async () => { + events.push("watcher") + }) + vectorStore.markIndexingComplete.mockImplementation(async () => { + events.push("complete") + }) + const orchestrator = new CodeIndexOrchestrator( + configManager, + stateManager, + workspacePath, + cacheManager, + vectorStore, + scanner, + fileWatcher, + ) + await orchestrator.startIndexing() + expect(events).toEqual(["incomplete", "scan", "watcher", "complete"]) + expect(orchestrator.state).toBe("Indexed") + expect(cacheManager.clearCacheFile).not.toHaveBeenCalled() + expect(vectorStore.clearCollection).not.toHaveBeenCalled() + }, + ) + + it.each([false, true])( + "keeps returned cancellation cleanup in the owner (incremental: %s)", + async (incremental) => { + vectorStore.initialize.mockResolvedValue(false) + vectorStore.hasIndexedData.mockResolvedValue(incremental) + const orchestrator = new CodeIndexOrchestrator( + configManager, + stateManager, + workspacePath, + cacheManager, + vectorStore, + scanner, + fileWatcher, + ) + scanner.scanDirectory.mockImplementation(async () => { + orchestrator.stopIndexing() + return { stats: { processed: 0, skipped: 0 }, totalBlockCount: 0 } + }) + // Preserve the original normal-return path: a failed flush is retried by the catch handler. + cacheManager.flush.mockRejectedValueOnce(new Error("flush failed")) + await orchestrator.startIndexing() + expect(cacheManager.flush).toHaveBeenCalledTimes(2) + expect(orchestrator.state).toBe("Standby") + expect(fileWatcher.initialize).not.toHaveBeenCalled() + expect(vectorStore.markIndexingComplete).not.toHaveBeenCalled() + expect(vectorStore.clearCollection).not.toHaveBeenCalled() + expect(cacheManager.clearCacheFile).not.toHaveBeenCalled() + }, + ) + it("should not call clearCollection() or clear cache when initialize() fails (indexing not started)", async () => { // Arrange: fail at initialize() vectorStore.initialize.mockRejectedValue(new Error("Qdrant unreachable")) diff --git a/src/services/code-index/code-index-scan-executor.ts b/src/services/code-index/code-index-scan-executor.ts new file mode 100644 index 0000000000..2ff55454d9 --- /dev/null +++ b/src/services/code-index/code-index-scan-executor.ts @@ -0,0 +1,101 @@ +import type { IDirectoryScanner, IVectorStore } from "./interfaces" +import type { CodeIndexStateManager } from "./state-manager" +import { t } from "../../i18n" + +/** Executes scans; cancellation cleanup, watcher startup and completion remain with the orchestrator. */ +export class CodeIndexScanExecutor { + constructor( + private readonly workspacePath: string, + private readonly scanner: IDirectoryScanner, + private readonly vectorStore: Pick, + private readonly stateManager: Pick, + ) {} + + /** Returns false when the scanner returns after cancellation, preserving the owner's normal-return path. */ + async runIncrementalScan(signal: AbortSignal): Promise { + console.log( + "[CodeIndexOrchestrator] Collection already has indexed data. Running incremental scan for new/changed files...", + ) + this.stateManager.setSystemState("Indexing", "Checking for new or modified files...") + const summary = await this.scanWorkspace(signal, "incremental") + if (!summary) return false + + // Preserve the existing incremental policy: reported batch errors do not prevent completion. + if (summary.found > 0) { + console.log( + `[CodeIndexOrchestrator] Incremental scan completed: ${summary.indexed} blocks indexed from new/changed files`, + ) + } else { + console.log("[CodeIndexOrchestrator] No new or changed files found") + } + return true + } + + /** Returns false on returned cancellation; scanner rejections propagate unchanged. */ + async runFullScan(signal: AbortSignal): Promise { + this.stateManager.setSystemState("Indexing", "Services ready. Starting workspace scan...") + const summary = await this.scanWorkspace(signal, "full") + if (!summary) return false + this.validateFullScan(summary.indexed, summary.found, summary.batchErrors) + return true + } + + private async scanWorkspace( + signal: AbortSignal, + kind: "full" | "incremental", + ): Promise<{ indexed: number; found: number; batchErrors: Error[] } | undefined> { + await this.vectorStore.markIndexingIncomplete() + let indexed = 0 + let found = 0 + const batchErrors: Error[] = [] + const result = await this.scanner.scanDirectory( + this.workspacePath, + (batchError: Error) => { + console.error( + `[CodeIndexOrchestrator] Error during ${kind === "full" ? "initial" : "incremental"} scan batch: ${batchError.message}`, + batchError, + ) + batchErrors.push(batchError) + }, + (count: number) => { + indexed += count + this.stateManager.reportBlockIndexingProgress(indexed, found) + }, + (count: number) => { + found += count + this.stateManager.reportBlockIndexingProgress(indexed, found) + }, + signal, + ) + + // Cancellation precedes missing-result checks and validation, just as in the orchestrator. + if (signal.aborted) return undefined + if (!result) { + throw new Error( + kind === "full" + ? "Scan failed, is scanner initialized?" + : "Incremental scan failed, is scanner initialized?", + ) + } + return { indexed, found, batchErrors } + } + + private validateFullScan(indexed: number, found: number, batchErrors: Error[]): void { + const firstError = batchErrors[0] + if (indexed === 0 && found > 0) { + throw new Error( + firstError + ? `Indexing failed: ${firstError.message}` + : t("embeddings:orchestrator.indexingFailedNoBlocks"), + ) + } + if (firstError && (found - indexed) / found > 0.1) { + throw new Error( + `Indexing partially failed: Only ${indexed} of ${found} blocks were indexed. ${firstError.message}`, + ) + } + if (firstError && indexed === 0) { + throw new Error(`Indexing failed completely: ${firstError.message}`) + } + } +} diff --git a/src/services/code-index/orchestrator.ts b/src/services/code-index/orchestrator.ts index 1efe647be9..c055fff370 100644 --- a/src/services/code-index/orchestrator.ts +++ b/src/services/code-index/orchestrator.ts @@ -5,6 +5,7 @@ import { CodeIndexStateManager, IndexingState } from "./state-manager" import { IFileWatcher, IVectorStore, BatchProcessingSummary } from "./interfaces" import { DirectoryScanner } from "./processors" import { CacheManager } from "./cache-manager" +import { CodeIndexScanExecutor } from "./code-index-scan-executor" import { TelemetryService } from "@roo-code/telemetry" import { TelemetryEventName } from "@roo-code/types" import { t } from "../../i18n" @@ -16,16 +17,19 @@ export class CodeIndexOrchestrator { private _fileWatcherSubscriptions: vscode.Disposable[] = [] private _isProcessing: boolean = false private _abortController: AbortController | null = null + private readonly scanExecutor: CodeIndexScanExecutor constructor( private readonly configManager: CodeIndexConfigManager, private readonly stateManager: CodeIndexStateManager, - private readonly workspacePath: string, + workspacePath: string, private readonly cacheManager: CacheManager, private readonly vectorStore: IVectorStore, - private readonly scanner: DirectoryScanner, + scanner: DirectoryScanner, private readonly fileWatcher: IFileWatcher, - ) {} + ) { + this.scanExecutor = new CodeIndexScanExecutor(workspacePath, scanner, vectorStore, stateManager) + } /** * Starts the file watcher if not already running. @@ -145,65 +149,13 @@ export class CodeIndexOrchestrator { const hasExistingData = await this.vectorStore.hasIndexedData() if (hasExistingData && !collectionCreated) { - // Collection exists with data - run incremental scan to catch any new/changed files - // This handles files added while workspace was closed or Qdrant was inactive - console.log( - "[CodeIndexOrchestrator] Collection already has indexed data. Running incremental scan for new/changed files...", - ) - this.stateManager.setSystemState("Indexing", "Checking for new or modified files...") - - // Mark as incomplete at the start of incremental scan - await this.vectorStore.markIndexingIncomplete() - - let cumulativeBlocksIndexed = 0 - let cumulativeBlocksFoundSoFar = 0 - const batchErrors: Error[] = [] - - const handleFileParsed = (fileBlockCount: number) => { - cumulativeBlocksFoundSoFar += fileBlockCount - this.stateManager.reportBlockIndexingProgress(cumulativeBlocksIndexed, cumulativeBlocksFoundSoFar) - } - - const handleBlocksIndexed = (indexedCount: number) => { - cumulativeBlocksIndexed += indexedCount - this.stateManager.reportBlockIndexingProgress(cumulativeBlocksIndexed, cumulativeBlocksFoundSoFar) - } - - // Run incremental scan - scanner will skip unchanged files using cache - const result = await this.scanner.scanDirectory( - this.workspacePath, - (batchError: Error) => { - console.error( - `[CodeIndexOrchestrator] Error during incremental scan batch: ${batchError.message}`, - batchError, - ) - batchErrors.push(batchError) - }, - handleBlocksIndexed, - handleFileParsed, - signal, - ) - - if (signal.aborted) { + if (!(await this.scanExecutor.runIncrementalScan(signal))) { await this.cacheManager.flush() this.stopWatcher() this.stateManager.setSystemState("Standby", t("embeddings:orchestrator.indexingStopped")) return } - if (!result) { - throw new Error("Incremental scan failed, is scanner initialized?") - } - - // If new files were found and indexed, log the results - if (cumulativeBlocksFoundSoFar > 0) { - console.log( - `[CodeIndexOrchestrator] Incremental scan completed: ${cumulativeBlocksIndexed} blocks indexed from new/changed files`, - ) - } else { - console.log("[CodeIndexOrchestrator] No new or changed files found") - } - await this._startWatcher() // Mark indexing as complete after successful incremental scan @@ -211,88 +163,13 @@ export class CodeIndexOrchestrator { this.stateManager.setSystemState("Indexed", t("embeddings:orchestrator.fileWatcherStarted")) } else { - // No existing data or collection was just created - do a full scan - this.stateManager.setSystemState("Indexing", "Services ready. Starting workspace scan...") - - // Mark as incomplete at the start of full scan - await this.vectorStore.markIndexingIncomplete() - - let cumulativeBlocksIndexed = 0 - let cumulativeBlocksFoundSoFar = 0 - const batchErrors: Error[] = [] - - const handleFileParsed = (fileBlockCount: number) => { - cumulativeBlocksFoundSoFar += fileBlockCount - this.stateManager.reportBlockIndexingProgress(cumulativeBlocksIndexed, cumulativeBlocksFoundSoFar) - } - - const handleBlocksIndexed = (indexedCount: number) => { - cumulativeBlocksIndexed += indexedCount - this.stateManager.reportBlockIndexingProgress(cumulativeBlocksIndexed, cumulativeBlocksFoundSoFar) - } - - const result = await this.scanner.scanDirectory( - this.workspacePath, - (batchError: Error) => { - console.error( - `[CodeIndexOrchestrator] Error during initial scan batch: ${batchError.message}`, - batchError, - ) - batchErrors.push(batchError) - }, - handleBlocksIndexed, - handleFileParsed, - signal, - ) - - if (signal.aborted) { + if (!(await this.scanExecutor.runFullScan(signal))) { await this.cacheManager.flush() this.stopWatcher() this.stateManager.setSystemState("Standby", t("embeddings:orchestrator.indexingStopped")) return } - if (!result) { - throw new Error("Scan failed, is scanner initialized?") - } - - const { stats } = result - - // Check if any blocks were actually indexed successfully - // If no blocks were indexed but blocks were found, it means all batches failed - if (cumulativeBlocksIndexed === 0 && cumulativeBlocksFoundSoFar > 0) { - if (batchErrors.length > 0) { - // Use the first batch error as it's likely representative of the main issue - const firstError = batchErrors[0] - throw new Error(`Indexing failed: ${firstError.message}`) - } else { - throw new Error(t("embeddings:orchestrator.indexingFailedNoBlocks")) - } - } - - // Check for partial failures - if a significant portion of blocks failed - const failureRate = (cumulativeBlocksFoundSoFar - cumulativeBlocksIndexed) / cumulativeBlocksFoundSoFar - if (batchErrors.length > 0 && failureRate > 0.1) { - // More than 10% of blocks failed to index - const firstError = batchErrors[0] - throw new Error( - `Indexing partially failed: Only ${cumulativeBlocksIndexed} of ${cumulativeBlocksFoundSoFar} blocks were indexed. ${firstError.message}`, - ) - } - - // CRITICAL: If there were ANY batch errors and NO blocks were successfully indexed, - // this is a complete failure regardless of the failure rate calculation - if (batchErrors.length > 0 && cumulativeBlocksIndexed === 0) { - const firstError = batchErrors[0] - throw new Error(`Indexing failed completely: ${firstError.message}`) - } - - // Final sanity check: If we found blocks but indexed none and somehow no errors were reported, - // this is still a failure - if (cumulativeBlocksFoundSoFar > 0 && cumulativeBlocksIndexed === 0) { - throw new Error(t("embeddings:orchestrator.indexingFailedCritical")) - } - await this._startWatcher() // Mark indexing as complete after successful full scan