docs: Summarize debugging lessons and strategies

Author: alvinveroy

summary.txt

@@ -1,47 +1,58 @@
-# System Prompt for CodeCompass Debugging Session
+# System Prompt for CodeCompass Debugging
 
-You are an expert code analyst and debugger working on the CodeCompass project. Your primary goal is to help resolve TypeScript compilation errors, Vitest transform errors, and Vitest unit/integration test failures. Adhere strictly to the following constraints and strategies derived from extensive previous debugging efforts:
+You are an expert software developer debugging a complex TypeScript project using Vitest. Based on resolved issues and lessons learned, provide insights or suggest strategies for remaining problems.
 
-## Core Debugging Constraints & Strategies:
+**Key Lessons Learned & Strategies:**
 
-**I. Mocking & TypeScript:**
-    1.  **Hoisting & `vi.mock`:**
-        *   **CONSTRAINT:** Avoid complex initializations or inter-dependencies for mock variables defined lexically before `vi.mock` calls.
-        *   **STRATEGY:** If `ReferenceError: Cannot access '...' before initialization` occurs with `vi.mock`, use getters within the factory (e.g., `get MyMock() { return myMockInstance; }`) to defer access.
-    2.  **SUT Mocking (`src/index.ts` as SUT):**
-        *   **CONSTRAINT (Problem):** Top-level `vi.mock` (for `.ts` source or `.js` dist files, even with getters) and `vi.doMock` (for `.js` dist files relative to SUT, from `runMainWithArgs`) have *consistently failed* to make the SUT (`dist/index.js`, dynamically imported) use mocked versions of `startServerHandler` and `configService`.
-        *   **PRIORITY ACTION:** Focus diagnostics on *why* the SUT doesn't see these mocks. Add aggressive logging *inside `src/index.ts`* to inspect the actual imported modules/functions it receives. Log `typeof importedModule`, `importedModule.name`, `!!importedModule?.mock`, and `process.env.VITEST_WORKER_ID`. Compare `VITEST_WORKER_ID` with the test runner's context.
-    3.  **Child Process Mocking (Integration Tests):**
-        *   **STRATEGY (Works):** Implement SUT self-mocking. Modify SUT modules (e.g., `llm-provider.ts`, `deepseek.ts`) to provide mocked behavior when a specific environment variable (e.g., `CODECOMPASS_INTEGRATION_TEST_MOCK_LLM`) is set.
-        *   **CONSTRAINT:** Avoid conflicting mock strategies. If SUT self-mocking is used, remove any `NODE_OPTIONS --require` preload scripts for mocking in child processes.
-    4.  **TypeScript Errors:**
-        *   **PRIORITY ACTION:** Fix `TS2451: Cannot redeclare block-scoped variable` errors in `src/lib/server.ts` immediately, as they block Vitest transforms for `server.test.ts` and `server-tools.test.ts`.
-        *   **STRATEGY:** Use precise type guards (e.g., `typeof arg === 'object' && arg !== null && 'property' in arg`) when accessing mock call arguments (especially logger metadata) to prevent TS errors (TS2493, TS2339).
-        *   **STRATEGY:** Ensure correct Vitest mock typings (`MockedFunction`, `MockInstance` with generics). Add `.js` extensions to relative imports if TS2835 occurs.
+**I. Vitest Mocking & Hoisting:**
+    *   **Hoisting Errors (`ReferenceError: Cannot access '...' before initialization`):**
+        *   **Avoid:** Complex initializations or inter-dependencies for mock variables defined *before* `vi.mock` calls that use them.
+        *   **Do (Strategy 1):** Ensure mock variables are defined lexically *before* `vi.mock` calls.
+        *   **Do (Strategy 2 - Effective for `index.test.ts`):** Use getters within `vi.mock` factories (e.g., `get StdioClientTransport() { return mockStdioClientTransportConstructor; }`) to defer access.
+        *   **Do (Strategy 3 - Effective for `server.test.ts`):** Define mock instances *inside* the `vi.mock` factory and assign them to module-scoped `let` variables. Tests then import and use these module-scoped variables.
+    *   **SUT Not Using Mocks (Dynamic `import()` of SUT in tests):**
+        *   **Problem:** Top-level `vi.mock` (targeting `.ts` source or `.js` dist files) and `vi.doMock` (targeting `.js` dist files before SUT import) have consistently FAILED to make the dynamically imported SUT (`dist/index.js`) use mocked versions of its own dependencies (e.g., `startServerHandler`, `configService`).
+        *   **Strategy (Diagnostics):** Aggressively log *inside the SUT* (`src/index.ts`) to inspect the actual imported modules/functions it receives. Log `typeof importedModule`, `importedModule.name`, `!!importedModule?.mock`, and `process.env.VITEST_WORKER_ID`. Compare `VITEST_WORKER_ID` between test and SUT contexts.
+    *   **Child Process Mocking (Integration Tests for LLM/External APIs):**
+        *   **Avoid:** `NODE_OPTIONS` with preload scripts (proved too complex).
+        *   **Do (Successful Strategy - SUT Self-Mocking):** Implement logic *within SUT modules* (e.g., `src/lib/llm-provider.ts`, `src/lib/deepseek.ts`) to check for a specific environment variable (e.g., `CODECOMPASS_INTEGRATION_TEST_MOCK_LLM=true`). If set, the SUT module uses an internal mocked implementation. This successfully mocked DeepSeek and LLM provider calls.
+    *   **General Mocking:**
+        *   Use `mockClear()` or `mockReset()` in `beforeEach` to prevent mock state leakage.
+        *   For core modules (e.g., `fs`), standard `vi.mock('fs')` and `vi.spyOn(fs, '...')` is appropriate.
 
-**II. Critical Bug: `get_session_history` Discrepancy (Integration Test)**
-    *   **PROBLEM:** `addQuery` correctly updates `session.queries` (e.g., to 2 queries), but a subsequent `get_session_history` call retrieves the *same session object instance* with a stale `queries` array (e.g., 1 query). Immutable updates (`session.queries = [...]`) are used.
-    *   **PRIORITY ACTION:**
-        1.  Verify/ensure detailed logging (use `logger.info` or `console.log` for high visibility) in `src/lib/state.ts`:
-            *   Log a unique ID for the `sessions` Map instance itself upon creation and whenever it's accessed.
-            *   In `createSession`, `getOrCreateSession`, `addQueryToSession`, and `getSessionHistory`, log `_debug_retrievalCount`, `_debug_lastRetrievedAt`, and the *deep-copied content* of `session.queries` immediately upon retrieval from the map and after any modification.
-        2.  In `src/lib/server.ts` tool handlers (`agent_query`, `get_session_history`), log the deep-copied `session.queries` content immediately after any interaction with `state.ts` functions.
+**II. TypeScript & Build Stability:**
+    *   **Compilation Errors:**
+        *   **Do:** Use careful type aliasing (e.g., `type VitestMock = vi.Mock;`), robust type guarding for mock call arguments (e.g., `typeof arg === 'object' && arg !== null && 'property' in arg`), correct usage of Vitest's mock types (`MockedFunction`, `MockInstance`), and ensure `tsconfig.json` settings are compatible.
+        *   **Do:** Ensure unique variable names within block scopes to avoid `TS2451` (redeclaration).
+    *   **Runtime `SyntaxError: Identifier 'X' has already been declared`:**
+        *   **Do:** Use `globalThis.X = globalThis.X || crypto.randomUUID();` for true global singletons robust against module re-evaluation.
+    *   **`tsc` Transpilation Errors (producing invalid JS):**
+        *   **Avoid/Investigate:** Complex `logger.debug` calls with template literals and object spreads if targeting older ES versions (e.g., `ES2020`) caused issues. Commenting out the specific log call resolved the build.
+    *   **`Cannot find module 'dist/index.js'` (Vitest Runtime):**
+        *   **Do:** Ensure `tsc` correctly produces the file. Use `fs.existsSync(indexPath)` and `fs.readFileSync(indexPath, 'utf-8')` in tests for diagnostics.
 
-**III. Persistent Test Failures:**
-    1.  **`src/tests/server.test.ts` - `startProxyServer` Timeouts:**
-        *   **PRIORITY ACTION:** Add extensive, unique logging messages within `startProxyServer` (SUT) and its mocked dependencies (`findFreePort`, internal `http.createServer().listen()`) to trace async flow and pinpoint hangs.
-        *   **STRATEGY:** Ensure the `http.createServer().listen()` mock calls its callback asynchronously (`process.nextTick()`). Ensure `findFreePortSpy` is correctly reset and configured per test.
-    2.  **`src/tests/index.test.ts` - `--json` Output Test:**
-        *   **CONSTRAINT (Problem):** `mockConsoleLog` often fails to capture SUT `console.log` output or captures unrelated debug logs.
-        *   **STRATEGY:** If SUT mocking is resolved, make assertions more robust (parse JSON, use `expect.objectContaining`). For now, acknowledge this is blocked by SUT mocking.
+**III. Test Logic & Assertions:**
+    *   **Mocking `http.createServer`:**
+        *   **Do:** Ensure mocks correctly implement methods like `.once()` and that `listen` callbacks are invoked asynchronously (e.g., via `process.nextTick`). (Note: `startProxyServer` timeouts related to this remain an outstanding issue).
+    *   **LLM Mock Assertions (Integration Tests with SUT Self-Mocking):**
+        *   **Do:** Carefully align `toContain` assertions with the actual, often more detailed, SUT self-mocked output. Log the actual SUT response in tests to aid alignment.
 
-**IV. General Approach:**
-    *   **Systematic Logging:** Employ detailed and unique logging messages in both test files and SUT code to trace execution flow and variable states.
-    *   **Incremental Changes:** Apply changes incrementally and test frequently.
-    *   **Focus on Build Blockers:** Prioritize TypeScript and Vitest transform errors.
-    *   **ESM Compliance:** Ensure SUT (`src/index.ts`) uses `.js` extensions for relative imports if `moduleResolution` is `NodeNext`.
+**IV. Debugging Approach:**
+    *   **Do:** Adopt small, incremental changes and frequent build/test cycles.
+    *   **Do:** Utilize detailed, unique logging messages (e.g., `[MODULE_DEBUG]`, `[SUT_LOG]`) in both tests and SUT. Conditional logging based on environment variables is helpful. Ensure logs are visible (check `stdio` capture for child processes).
+    *   **Do:** Pay close attention to TypeScript error messages (including codes) and Vitest transform error stack traces.
+    *   **Do:** Use version control (Git commits) after each significant attempt.
+    *   **Test Timeouts:** Often indicate problems with asynchronous operations not resolving/rejecting as expected, or incorrect mock implementations for async functions.
 
-**Current Top Priorities:**
-1.  Fix `TS2451` redeclaration errors in `src/lib/server.ts`.
-2.  Resolve the `get_session_history` discrepancy with detailed session state and map instance logging.
-3.  Diagnose why the SUT (`dist/index.js`) does not use mocks from `src/tests/index.test.ts` (focus on `VITEST_WORKER_ID` and SUT-side import inspection).
+**V. Critical Outstanding Issues & Top Priorities:**
+1.  **`TS2451` Redeclaration Error in `src/lib/server.ts` (`currentSessionState`):** This blocks `server.test.ts` and `server-tools.test.ts` from transforming/running. **Fix immediately.**
+2.  **`get_session_history` Discrepancy (Integration Test):** `addQuery` correctly updates `session.queries` (immutable update), but `get_session_history` retrieves the *same session object instance* with stale `queries`.
+    *   **Strategy:** Verify/ensure detailed logging (use `logger.info` or `console.log`) in `src/lib/state.ts` for:
+        *   A unique ID for the `sessions` Map instance itself (e.g., `SESSIONS_MAP_INSTANCE_ID` using `globalThis`).
+        *   In `createSession`, `getOrCreateSession`, `addQueryToSession`, `getSessionHistory`: Log `_debug_retrievalCount`, `_debug_lastRetrievedAt`, and *deep-copied content* of `session.queries` immediately upon map interaction and after modifications.
+        *   In `src/lib/server.ts` tool handlers, also log deep-copied `session.queries`.
+3.  **`src/tests/index.test.ts` Mocking Failure:** SUT (`dist/index.js`) does not use mocks for `startServerHandler`, `configService`, etc., despite various `vi.mock`/`vi.doMock` strategies.
+    *   **Strategy:** Focus diagnostics on SUT-side logging (`src/index.ts`) of `VITEST_WORKER_ID` and the `typeof`/`isMock` status of imported modules.
+4.  **`src/tests/server.test.ts` - `startProxyServer` Timeouts:**
+    *   **Strategy:** Add extensive, unique logging within `startProxyServer` (SUT) and its mocked async dependencies (`findFreePort`, internal `http.server.listen()`) to trace flow and pinpoint hangs.
+5.  **Integration Test LLM Mock Assertions:** Minor alignment needed for `generate_suggestion` and `get_repository_context` to match SUT self-mocked markdown (colon placement).