fix: address PR #5354 review feedback

- Fixed duplicate 'codeIndex' sections in all 17 non-English locale files
- Implemented memory management safeguards in WorkerPool:
  - Added queue size limits (default: 1000 tasks)
  - Added memory threshold monitoring (90% limit)
  - Added worker health checks with stale worker detection
  - Added status reporting method for monitoring
- Made throttling interval configurable in CodeIndexStateManager
- Added comprehensive worker message protocol documentation
- Updated all WorkerPool instantiations to use new options-based constructor
- Added tests for memory management features
- Fixed unhandled promise rejection in health check test
- Fixed ESLint warnings about Function type usage

Author: hannesrudolph

src/services/code-index/__tests__/state-manager-throttling.spec.ts

@@ -4,9 +4,9 @@ import { CodeIndexStateManager, IndexingState } from "../state-manager"
 // Mock vscode module
 vi.mock("vscode", () => ({
 	EventEmitter: class EventEmitter {
-		private listeners: Map<string, Function[]> = new Map()
+		private listeners: Map<string, Array<(...args: any[]) => void>> = new Map()
 
-		event = (listener: Function) => {
+		event = (listener: (...args: any[]) => void) => {
 			if (!this.listeners.has("event")) {
 				this.listeners.set("event", [])
 			}

src/services/code-index/__tests__/worker-pool-memory.spec.ts

@@ -0,0 +1,181 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
+import { WorkerPool } from "../workers/worker-pool"
+import { EventEmitter } from "events"
+
+// Mock worker_threads module
+vi.mock("worker_threads")
+
+// Mock os module
+vi.mock("os", () => ({
+	cpus: vi.fn(() => new Array(4)),
+}))
+
+describe("WorkerPool - Memory Management", () => {
+	let pool: WorkerPool | null = null
+	let mockWorkers: any[] = []
+	let Worker: any
+
+	beforeEach(async () => {
+		vi.clearAllTimers()
+		vi.clearAllMocks()
+		mockWorkers = []
+		pool = null
+
+		// Get the mocked Worker class
+		const workerThreads = await import("worker_threads")
+		Worker = workerThreads.Worker
+
+		// Reset and re-implement the Worker mock
+		vi.mocked(Worker).mockClear()
+		vi.mocked(Worker).mockImplementation((filename: string | URL) => {
+			const worker = new EventEmitter()
+			;(worker as any).terminate = vi.fn().mockResolvedValue(undefined)
+			;(worker as any).postMessage = vi.fn()
+			;(worker as any).scriptPath = filename
+			mockWorkers.push(worker)
+
+			// Simulate worker ready after a short delay
+			setTimeout(() => worker.emit("online"), 10)
+
+			return worker as any
+		})
+	})
+
+	afterEach(async () => {
+		if (pool) {
+			try {
+				await pool.shutdown()
+			} catch (e) {
+				// Ignore shutdown errors in tests
+			}
+			pool = null
+		}
+
+		vi.clearAllMocks()
+		vi.clearAllTimers()
+		mockWorkers = []
+	})
+
+	describe("queue size limits", () => {
+		it("should reject tasks when queue is full", async () => {
+			pool = new WorkerPool("/path/to/worker.js", {
+				maxWorkers: 1,
+				maxQueueSize: 2,
+			})
+
+			// Fill up the worker
+			const task1 = pool.execute({ type: "task1" })
+
+			// Fill up the queue
+			const task2 = pool.execute({ type: "task2" })
+			const task3 = pool.execute({ type: "task3" })
+
+			// This should be rejected
+			await expect(pool.execute({ type: "task4" })).rejects.toThrow("Worker pool queue is full")
+
+			// Complete tasks to avoid hanging
+			await new Promise((resolve) => setTimeout(resolve, 20))
+			mockWorkers[0].emit("message", { success: true, data: "result1" })
+			mockWorkers[0].emit("message", { success: true, data: "result2" })
+			mockWorkers[0].emit("message", { success: true, data: "result3" })
+
+			await Promise.all([task1, task2, task3])
+		})
+	})
+
+	describe("memory threshold", () => {
+		it("should reject tasks when memory usage is too high", async () => {
+			// Mock process.memoryUsage to return high memory
+			const originalMemoryUsage = process.memoryUsage
+			process.memoryUsage = vi.fn().mockReturnValue({
+				heapUsed: 600 * 1024 * 1024, // 600MB
+				heapTotal: 800 * 1024 * 1024,
+				external: 0,
+				arrayBuffers: 0,
+				rss: 900 * 1024 * 1024,
+			}) as any
+
+			pool = new WorkerPool("/path/to/worker.js", {
+				maxWorkers: 2,
+				memoryThresholdMB: 500, // 500MB threshold
+			})
+
+			await expect(pool.execute({ type: "test" })).rejects.toThrow(/Memory usage too high/)
+
+			// Restore original
+			process.memoryUsage = originalMemoryUsage
+		})
+	})
+
+	describe("health checks", () => {
+		it("should perform periodic health checks", async () => {
+			vi.useFakeTimers()
+			const consoleDebugSpy = vi.spyOn(console, "debug").mockImplementation(() => {})
+
+			pool = new WorkerPool("/path/to/worker.js", {
+				maxWorkers: 2,
+				healthCheckIntervalMs: 1000,
+			})
+
+			// Advance time to trigger health check
+			vi.advanceTimersByTime(1000)
+
+			// Health check should log status
+			expect(consoleDebugSpy).toHaveBeenCalledWith(expect.stringContaining("[WorkerPool] Status"))
+
+			consoleDebugSpy.mockRestore()
+			vi.useRealTimers()
+		})
+
+		it("should replace stale workers", async () => {
+			vi.useFakeTimers()
+			const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
+
+			pool = new WorkerPool("/path/to/worker.js", {
+				maxWorkers: 1,
+				healthCheckIntervalMs: 1000,
+			})
+
+			// Start a task and handle the rejection
+			const taskPromise = pool.execute({ type: "test" }).catch((e: Error) => e)
+
+			// Wait for task to be assigned
+			await vi.advanceTimersByTimeAsync(20)
+
+			// Advance time past stale threshold (1 minute)
+			await vi.advanceTimersByTimeAsync(61000)
+
+			// Task should be rejected due to timeout
+			const result = await taskPromise
+			expect(result).toBeInstanceOf(Error)
+			expect((result as Error).message).toBe("Worker task timed out")
+
+			// Should log warning about stale worker
+			expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Worker appears to be stale"))
+
+			consoleWarnSpy.mockRestore()
+			vi.useRealTimers()
+		})
+	})
+
+	describe("status reporting", () => {
+		it("should provide pool status", async () => {
+			pool = new WorkerPool("/path/to/worker.js", {
+				maxWorkers: 2,
+				maxQueueSize: 100,
+				memoryThresholdMB: 512,
+			})
+
+			const status = pool.getStatus()
+
+			expect(status).toMatchObject({
+				activeWorkers: 0,
+				availableWorkers: 2,
+				queueLength: 0,
+				maxQueueSize: 100,
+				memoryThresholdMB: 512,
+			})
+			expect(status.memoryUsageMB).toBeGreaterThan(0)
+		})
+	})
+})

src/services/code-index/__tests__/worker-pool.spec.ts

@@ -60,7 +60,7 @@ describe("WorkerPool", () => {
 
 	describe("initialization", () => {
 		it("should create workers with specified concurrency", () => {
-			pool = new WorkerPool("/path/to/worker.js", 4)
+			pool = new WorkerPool("/path/to/worker.js", { maxWorkers: 4 })
 			expect(Worker).toHaveBeenCalledTimes(4)
 			expect(Worker).toHaveBeenCalledWith("/path/to/worker.js")
 		})
@@ -82,7 +82,7 @@ describe("WorkerPool", () => {
 
 	describe("task execution", () => {
 		beforeEach(() => {
-			pool = new WorkerPool("/path/to/worker.js", 2)
+			pool = new WorkerPool("/path/to/worker.js", { maxWorkers: 2 })
 		})
 
 		it("should execute tasks and return results", async () => {
@@ -97,7 +97,7 @@ describe("WorkerPool", () => {
 
 			// Simulate worker response
 			const worker = mockWorkers[0]
-			const messageHandler = worker.listeners("message")[0] as Function
+			const messageHandler = worker.listeners("message")[0] as (...args: any[]) => void
 			messageHandler(expectedResult)
 
 			const result = await resultPromise
@@ -142,7 +142,7 @@ describe("WorkerPool", () => {
 			expect(mockWorkers[1].postMessage).toHaveBeenCalledTimes(1)
 
 			// Complete first task
-			const messageHandler0 = mockWorkers[0].listeners("message")[0] as Function
+			const messageHandler0 = mockWorkers[0].listeners("message")[0] as (...args: any[]) => void
 			messageHandler0({ success: true, data: "task1" })
 
 			// Wait for queue processing
@@ -154,7 +154,7 @@ describe("WorkerPool", () => {
 
 			// Complete remaining tasks
 			messageHandler0({ success: true, data: "task3" })
-			const messageHandler1 = mockWorkers[1].listeners("message")[0] as Function
+			const messageHandler1 = mockWorkers[1].listeners("message")[0] as (...args: any[]) => void
 			messageHandler1({ success: true, data: "task2" })
 
 			const results = await Promise.all(promises)
@@ -206,7 +206,7 @@ describe("WorkerPool", () => {
 
 	describe("shutdown", () => {
 		beforeEach(() => {
-			pool = new WorkerPool("/path/to/worker.js", 2)
+			pool = new WorkerPool("/path/to/worker.js", { maxWorkers: 2 })
 		})
 
 		it("should terminate all workers", async () => {
@@ -218,7 +218,7 @@ describe("WorkerPool", () => {
 
 		it("should reject pending tasks on shutdown", async () => {
 			// Create a new pool with limited workers
-			const testPool = new WorkerPool("/path/to/worker.js", 1)
+			const testPool = new WorkerPool("/path/to/worker.js", { maxWorkers: 1 })
 
 			// Wait for workers to be ready
 			await new Promise((resolve) => setTimeout(resolve, 20))
@@ -272,7 +272,7 @@ describe("WorkerPool", () => {
 
 	describe("error handling", () => {
 		beforeEach(() => {
-			pool = new WorkerPool("/path/to/worker.js", 1)
+			pool = new WorkerPool("/path/to/worker.js", { maxWorkers: 1 })
 		})
 
 		it("should handle worker initialization errors", async () => {
@@ -316,7 +316,7 @@ describe("WorkerPool", () => {
 
 	describe("performance", () => {
 		it("should process tasks concurrently", async () => {
-			pool = new WorkerPool("/path/to/worker.js", 3)
+			pool = new WorkerPool("/path/to/worker.js", { maxWorkers: 3 })
 
 			const tasks = Array(6)
 				.fill(null)
@@ -331,7 +331,7 @@ describe("WorkerPool", () => {
 			// Complete first batch of tasks
 			for (let i = 0; i < 3; i++) {
 				const worker = mockWorkers[i]
-				const handler = worker.listeners("message")[0] as Function
+				const handler = worker.listeners("message")[0] as (...args: any[]) => void
 				handler({ success: true, data: `result${i}` })
 			}
 
@@ -341,7 +341,7 @@ describe("WorkerPool", () => {
 			// Complete second batch
 			for (let i = 0; i < 3; i++) {
 				const worker = mockWorkers[i]
-				const handler = worker.listeners("message")[0] as Function
+				const handler = worker.listeners("message")[0] as (...args: any[]) => void
 				handler({ success: true, data: `result${i + 3}` })
 			}
 

src/services/code-index/manager.ts

@@ -52,6 +52,8 @@ export class CodeIndexManager {
 	private constructor(workspacePath: string, context: vscode.ExtensionContext) {
 		this.workspacePath = workspacePath
 		this.context = context
+		// Create state manager with default throttle interval (500ms)
+		// Can be customized by passing a different value to the constructor
 		this._stateManager = new CodeIndexStateManager()
 	}
 

src/services/code-index/processors/scanner.ts

@@ -42,10 +42,9 @@ export class DirectoryScanner implements IDirectoryScanner {
 	) {
 		// Initialize worker pool for file processing
 		try {
-			this.workerPool = new WorkerPool(
-				path.join(__dirname, "../workers/file-processor.worker.js"),
-				PARSING_CONCURRENCY,
-			)
+			this.workerPool = new WorkerPool(path.join(__dirname, "../workers/file-processor.worker.js"), {
+				maxWorkers: PARSING_CONCURRENCY,
+			})
 		} catch (error) {
 			console.warn(
 				"[DirectoryScanner] Failed to initialize worker pool, falling back to main thread processing:",

src/services/code-index/state-manager.ts

@@ -13,7 +13,11 @@ export class CodeIndexStateManager {
 	// Throttling properties
 	private _throttleTimer?: NodeJS.Timeout
 	private _pendingUpdate?: ReturnType<typeof this.getCurrentStatus>
-	private readonly THROTTLE_INTERVAL_MS = 500
+	private readonly throttleIntervalMs: number
+
+	constructor(throttleIntervalMs: number = 500) {
+		this.throttleIntervalMs = throttleIntervalMs
+	}
 
 	// --- Public API ---
 
@@ -128,7 +132,7 @@ export class CodeIndexStateManager {
 				this._pendingUpdate = undefined
 			}
 			this._throttleTimer = undefined
-		}, this.THROTTLE_INTERVAL_MS)
+		}, this.throttleIntervalMs)
 	}
 
 	public dispose(): void {