Skip to content

set up eslint and prettier#10

Open
Ogenbertrand wants to merge 1 commit intomainfrom
feat-setup-formating
Open

set up eslint and prettier#10
Ogenbertrand wants to merge 1 commit intomainfrom
feat-setup-formating

Conversation

@Ogenbertrand
Copy link
Copy Markdown
Collaborator

@Ogenbertrand Ogenbertrand commented Sep 21, 2025

This PR aims at setting up eslinting and prettier for code formating,. No closes for this yet.

I am used in pushing the package-lock file 😆 since our project requires you push it, hope it no big deal.

Summary by CodeRabbit

  • New Features

    • File watcher now reports a ready state with watched file count.
    • Skips indexing oversized files based on configuration, with clear warnings.
    • Improved initial indexing progress logs.
  • Documentation

    • Revamped README with setup, CI/CD, deployment, and config guidance.
    • Updated TODO to mark PocketFlow integration tasks as complete.
  • Chores

    • Added ESLint/Prettier configurations and ignore rules.
    • Introduced scripts for linting, formatting, type checks, and a pre-commit workflow.
    • Applied consistent formatting across the codebase.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Sep 21, 2025

Walkthrough

Adds linting/formatting tooling (ESLint/Prettier) and scripts, updates docs, and reformats source/tests. Introduces VectorStore lifecycle methods (clear, close). Updates runtime flow to handle file-watcher ready event and to skip indexing files exceeding a configured size. Minor logger rotation safety tweak; extensive integration test restructuring.

Changes

Cohort / File(s) Summary
Tooling & Scripts
package.json, .eslintrc.json, .prettierrc, .prettierignore, tsconfig.json
Adds ESLint/Prettier configs and dev scripts (lint, format, type-check, pre-commit). Sets Node/ES2022 env, TS parser, Prettier rules, and ignore patterns. No runtime impact.
Documentation
README.md, docs/todo-phase1.md
Reworked README structure/content; added Dev & CI/CD section. Marked PocketFlow Integration tasks complete.
Runtime Flow Updates
src/index.ts
Adds file-watcher 'ready' handling (log with count). Skips indexing on add/change when file exceeds max size; logs warnings. Adjusted logging formats.
Public Types Update
src/types.ts
Adds VectorStore methods: clear(): Promise<void>, close(): Promise<void>. No other signature changes.
Logging Internals
src/logger.ts
Minor formatting; ensures stream is ended before log rotation; template string usage; behavior otherwise preserved.
Source Formatting
src/chroma-vector-store.ts, src/cli-main.ts, src/cli.ts, src/code-parser.ts, src/code-storage.ts, src/config.ts, src/embedding-service.ts, src/file-watcher.ts, src/semantic-search.ts, src/tree-sitter-types.d.ts, src/vector-store.ts
Formatting-only adjustments (trailing commas, whitespace, line wraps). No functional/public API changes.
Tests – Integration & Watcher
test/test-integration.ts, test/debug-watcher.ts
Integration test fully restructured to explicit init, event-driven processing, multi-step searches, and cleanup. Added watcher 'ready' log in debug watcher.
Tests – Formatting
test/test-file-watcher.ts, test/test-semantic-search.ts, test/test-storage.ts, test/test-vector-store.ts, test/test-embedding.ts, test/test-parser.ts
Formatting-only updates (trailing commas, spacing, wrapping). No behavioral changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as User/CLI
  participant App as App (index.ts)
  participant FW as FileWatcher
  participant Parser as CodeParser
  participant Store as CodeStorage
  participant VS as VectorStore
  participant SS as SemanticSearch

  User->>App: start()
  App->>FW: create/start watcher
  FW-->>App: ready
  Note over App,FW: New: 'ready' event logs readiness and file count

  FW-->>App: add/change(file)
  App->>App: check file size vs max
  alt exceeds limit
    App-->>App: skip indexing
    App->>User: warn (skipped due to size)
  else within limit
    App->>Parser: parse(file)
    Parser-->>App: chunks
    App->>Store: persist(chunks)
    Store-->>App: ok
    App->>SS: embed+index(chunks)
    SS->>VS: upsert embeddings
    VS-->>SS: ok
    SS-->>App: ok
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

I tap my paws on tidy trails,
Lints and pretties in woven veils.
A watcher blinks—“I’m ready!”—bright,
Big files hop past, kept out of sight.
Vectors learn to clear and close—
Logs roll softly, as code flows. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "set up eslint and prettier" is concise and accurately summarizes the primary change — adding ESLint and Prettier configuration and related scripts — and aligns with the PR objectives and file diffs.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-setup-formating

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)
src/code-storage.ts (1)

141-149: Avoid blocking I/O inside an async method. Use fs.promises.writeFile instead of writeFileSync.

persistChunkToDisk is async but uses synchronous disk write, which blocks the event loop and can hurt throughput.

Apply this diff:

   private async persistChunkToDisk(chunk: CodeChunk): Promise<void> {
     try {
-      const fs = require('fs');
+      const fs = require('fs').promises;
       const path = require('path');
 
       const chunkFile = path.join(this.options.storagePath, `${chunk.id}.json`);
-      fs.writeFileSync(chunkFile, JSON.stringify(chunk));
+      await fs.writeFile(chunkFile, JSON.stringify(chunk));
     } catch (error) {
       console.error('Failed to persist chunk to disk:', error);
     }
   }
tsconfig.json (1)

3-5: Preserve native dynamic import for ESM-only packages (fix tsconfig).

  • Found: TypeScript ^5.3.3; tsconfig.json sets "module": "CommonJS"; src/embedding-service.ts:67 uses await import('@xenova/transformers').
  • Risk: tsc will downlevel import() to a require(...) wrapper when emitting CommonJS, causing ERR_REQUIRE_ESM for ESM-only packages. (github.com)
  • Node supports dynamic import() from CommonJS at runtime, so the correct fix is to emit native import() instead of a require wrapper. (nodejs.org)
  • Action: change tsconfig to emit ESM-aware code (e.g. set "module": "NodeNext" or "module": "ESNext" and pair with an appropriate "moduleResolution" or use a bundler that preserves dynamic import), rebuild and test the import in src/embedding-service.ts:67. (typescriptlang.org)
src/code-parser.ts (1)

849-859: Bug: parseFileIncremental returns non-nullable tree but may pass undefined.

oldTree! can still be undefined. Make tree optional or guarantee a valid tree object.

Apply:

-  ): Promise<{ chunks: CodeChunk[]; tree: ParserType.Tree }> {
+  ): Promise<{ chunks: CodeChunk[]; tree?: ParserType.Tree }> {
@@
-    if (!language) {
-      return { chunks: [], tree: oldTree! };
-    }
+    if (!language) {
+      return { chunks: [], tree: oldTree };
+    }
@@
-    if (!parser) {
-      return { chunks: [], tree: oldTree! };
-    }
+    if (!parser) {
+      return { chunks: [], tree: oldTree };
+    }
@@
-    } catch (error) {
+    } catch (error) {
       console.error(`Failed to parse file incrementally ${filePath}:`, error);
-      return { chunks: [], tree: oldTree! };
+      return { chunks: [], tree: oldTree };
     }
README.md (1)

192-199: Fix incorrect directory name after clone.

Repository is 42context, but docs say cd mcp-local-context-engine.

Apply:

- git clone https://github.com/AssahBismarkabah/42context.git
- cd mcp-local-context-engine
+ git clone https://github.com/AssahBismarkabah/42context.git
+ cd 42context
src/vector-store.ts (1)

33-40: Type name collision with project-wide VectorSearchResult.

This file defines and exports a VectorSearchResult incompatible with src/types.ts’ VectorSearchResult. This will cause confusion and type bugs.

Apply this diff to rename and stop exporting the conflicting type:

-interface VectorSearchResult {
+interface InternalVectorSearchResult {
   chunkId: string;
   similarity: number;
   metadata: {
     chunk: CodeChunk;
     embedding: EmbeddingResult;
   };
 }
@@
-export type { VectorStoreOptions, VectorSearchResult };
+export type { VectorStoreOptions };

And update usages:

-  ): Promise<VectorSearchResult[]> {
+  ): Promise<InternalVectorSearchResult[]> {

Also applies to: 418-419

src/index.ts (2)

159-162: Size check compares characters, not bytes

content.length counts UTF‑16 code units, not file bytes. This can under‑ or over‑count vs a byte limit.

Use byte length:

-      if (content.length > config.security.maxFileSize) {
+      const bytes = Buffer.byteLength(content, 'utf8');
+      if (bytes > config.security.maxFileSize) {
         this.logger.warn(`File too large, skipping: ${filePath}`);
         return;
       }

189-191: Same byte/char size issue on change path

Mirror the fix for changed files.

-      if (content.length > config.security.maxFileSize) {
+      const bytes = Buffer.byteLength(content, 'utf8');
+      if (bytes > config.security.maxFileSize) {
         this.logger.warn(`File too large, skipping re-index: ${filePath}`);
         return;
       }
src/logger.ts (1)

128-141: Log rotation may lose data; end() is async

You call end() then renameSync immediately; writes might not be flushed.

Ensure rotation happens after the stream finishes:

-    try {
-      this.fileStream.end();
-
-      const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
-      const rotatedFile = `${this.logFile}.${timestamp}`;
-
-      require('fs').renameSync(this.logFile, rotatedFile);
-
-      // Clean up old rotated files
-      this.cleanupOldLogs();
-
-      // Create new log file
-      this.fileStream = createWriteStream(this.logFile, { flags: 'a' });
-    } catch (error) {
+    try {
+      const current = this.fileStream;
+      current.end(() => {
+        const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
+        const rotatedFile = `${this.logFile!}.${timestamp}`;
+        try {
+          require('fs').renameSync(this.logFile!, rotatedFile);
+          this.cleanupOldLogs();
+        } catch (e) {
+          console.error('Failed during log rotation:', e);
+        }
+        this.fileStream = createWriteStream(this.logFile!, { flags: 'a' });
+      });
+    } catch (error) {
       console.error('Failed to rotate log file:', error);
     }
🧹 Nitpick comments (37)
src/code-storage.ts (2)

156-172: Minor: tighten candidate filtering with Set.has to cut O(n·m) includes scans.

Small perf win without behavior change.

Apply this diff:

-    if (query.filePath) {
-      const fileChunks = this.fileIndex.get(query.filePath) || [];
-      candidateIds = new Set([...candidateIds].filter(id => fileChunks.includes(id)));
-    }
+    if (query.filePath) {
+      const fileChunks = new Set(this.fileIndex.get(query.filePath) || []);
+      candidateIds = new Set([...candidateIds].filter(id => fileChunks.has(id)));
+    }
 
-    if (query.language) {
-      const languageChunks = this.languageIndex.get(query.language) || [];
-      candidateIds = new Set([...candidateIds].filter(id => languageChunks.includes(id)));
-    }
+    if (query.language) {
+      const languageChunks = new Set(this.languageIndex.get(query.language) || []);
+      candidateIds = new Set([...candidateIds].filter(id => languageChunks.has(id)));
+    }

174-178: Apply the same Set.has optimization for type filter.

-    if (query.chunkType) {
-      const typeChunks = this.typeIndex.get(query.chunkType) || [];
-      candidateIds = new Set([...candidateIds].filter(id => typeChunks.includes(id)));
-    }
+    if (query.chunkType) {
+      const typeChunks = new Set(this.typeIndex.get(query.chunkType) || []);
+      candidateIds = new Set([...candidateIds].filter(id => typeChunks.has(id)));
+    }
test/test-embedding.ts (1)

146-146: Avoid CommonJS export in a TS module; prefer ES export for consistency.

Use a named export to align with the rest of the file’s ES imports.

-module.exports = { testEmbeddingService };
+export { testEmbeddingService };
src/embedding-service.ts (5)

53-55: Device option note.

'gpu' requires a WebGPU-capable runtime for @xenova/transformers; in Node this may fall back to CPU. Consider surfacing actual device used in getModelInfo().


111-112: Model name vs. dimension coupling.

getModelInfo/getEmbeddingDimension hardcode 384; if modelName changes, this drifts. Suggest deriving from output shape once and caching.

-  getModelInfo(): {
+  getModelInfo(): {
     name: string;
     dimension: number | null;
     maxSequenceLength: number;
     device: string;
   } {
-    return {
-      name: this.options.modelName,
-      dimension: this.isModelLoaded ? 384 : null, // all-MiniLM-L6-v2 outputs 384 dimensions
+    return {
+      name: this.options.modelName,
+      dimension: this.isModelLoaded ? this.getEmbeddingDimension() : null,
       maxSequenceLength: this.options.maxSequenceLength,
       device: this.options.device,
     };
   }
@@
-  getEmbeddingDimension(): number {
-    return 384; // all-MiniLM-L6-v2 outputs 384 dimensions
-  }
+  getEmbeddingDimension(): number {
+    // Prefer cached inference from a tiny probe string if not known
+    if (this.embeddingCache.size > 0) {
+      return this.embeddingCache.values().next().value.dimension;
+    }
+    return 384;
+  }

135-137: Batch logging: keep logs at debug to reduce noise.

Consider gating verbose logs behind an env flag.


160-166: Error logging is good; consider attaching chunk id/context.

You already include the id; optionally include model name to aid triage.


306-306: Expose actual device used.

If transformers chooses a backend automatically, consider capturing it after initialize() and returning it here.

test/test-parser.ts (3)

7-14: Silent catch blocks.

The empty catch swallows unexpected errors; at least log a brief message in tests to aid debugging.

-} catch {}
+} catch (e) {
+  console.warn('Setup cleanup warning:', e instanceof Error ? e.message : String(e));
+}

96-99: Prefer consistent module style.

Mixing require() and ESM import in TS can confuse linters. Consider converting the require to an import if possible.


153-156: Teardown silent catch.

Same as setup: log teardown errors to avoid hiding filesystem issues.

.eslintrc.json (1)

31-31: Reconsider ignoring all “*.js”.

If you add any JS sources (CLI shims, config scripts), this will silently exclude them from linting. Consider removing "*.js" here and rely on dist/ ignore instead.

src/code-parser.ts (2)

96-103: Replace console logging with an injectable logger or debug flag.

Library code spamming console.* is hard to control in consumers.

Also applies to: 206-211


918-921: Remove misleading CommonJS type exports.

module.exports.CodeParserOptions/CodeChunk/ASTNodeInfo = {} export empty objects not tied to real runtime values and can confuse users.

Apply:

-module.exports.CodeParserOptions = {};
-module.exports.CodeChunk = {};
-module.exports.ASTNodeInfo = {};
src/config.ts (2)

330-335: Guard env parsing against NaN.

Current parseInt can yield NaN, silently corrupting config.

Apply:

-    if (env.DEV_CONTEXT_MAX_CONCURRENT_OPERATIONS) {
-      config.performance.maxConcurrentOperations = parseInt(
-        env.DEV_CONTEXT_MAX_CONCURRENT_OPERATIONS
-      );
-    }
+    if (env.DEV_CONTEXT_MAX_CONCURRENT_OPERATIONS) {
+      const v = Number(env.DEV_CONTEXT_MAX_CONCURRENT_OPERATIONS);
+      if (Number.isFinite(v) && v > 0) {
+        config.performance.maxConcurrentOperations = v;
+      }
+    }

134-146: Align parser.supportedLanguages with actual SupportedLanguage.

Defaults include languages not supported by CodeParser (e.g., ruby, php). Either add support or constrain defaults to the union type to avoid confusion.

package.json (1)

16-16: Make clean cross‑platform.

Use rimraf to work on Windows as well.

Apply:

-    "clean": "rm -rf dist",
+    "clean": "rimraf dist",

And add to devDependencies:

+    "rimraf": "^6.0.0",
src/cli.ts (5)

445-474: Wrap switch case declarations in blocks (noSwitchDeclarations).

Prevents scope bleed for const/let inside cases.

Apply:

   private async configCommand(action: string, key?: string, value?: string): Promise<void> {
     switch (action) {
       case 'get':
-        if (!key) {
-          console.error('Please specify a configuration key');
-          return;
-        }
-        const configValue = this.configManager.getConfig();
-        console.log(`Configuration: ${key} = ${JSON.stringify(configValue, null, 2)}`);
-        break;
+        {
+          if (!key) {
+            console.error('Please specify a configuration key');
+            return;
+          }
+          const configValue = this.configManager.getConfig();
+          console.log(`Configuration: ${key} = ${JSON.stringify(configValue, null, 2)}`);
+          break;
+        }
       case 'set':
-        if (!key || !value) {
-          console.error('Please specify both key and value');
-          return;
-        }
-        console.log(`Setting ${key} = ${value}`);
-        console.log('Config set implementation pending...');
-        break;
+        {
+          if (!key || !value) {
+            console.error('Please specify both key and value');
+            return;
+          }
+          console.log(`Setting ${key} = ${value}`);
+          console.log('Config set implementation pending...');
+          break;
+        }
       case 'list':
-        console.log('Current configuration:');
-        console.log(JSON.stringify(this.configManager.getConfig(), null, 2));
-        break;
+        {
+          console.log('Current configuration:');
+          console.log(JSON.stringify(this.configManager.getConfig(), null, 2));
+          break;
+        }
       case 'reset':
-        console.log('Resetting configuration to defaults...');
-        console.log('Config reset implementation pending...');
-        break;
+        {
+          console.log('Resetting configuration to defaults...');
+          console.log('Config reset implementation pending...');
+          break;
+        }
       default:
         console.error(`Unknown action: ${action}`);
         console.log('Available actions: get, set, list, reset');
     }
   }

606-676: Wrap switch case declarations in blocks (noSwitchDeclarations).

Same issue in debugCommand cases.

Apply:

   private async debugCommand(action: string, file?: string): Promise<void> {
     switch (action) {
       case 'parse':
-        if (!file) {
-          console.error('Please specify a file to parse');
-          return;
-        }
-        if (!existsSync(file)) {
-          console.error(`File not found: ${file}`);
-          return;
-        }
-        console.log(`Parsing file: ${file}`);
-        const parser = new CodeParser();
-        const content = readFileSync(file, 'utf-8');
-        const language = parser.detectLanguage(file);
-        if (!language) {
-          console.error(`Unsupported file type: ${file}`);
-          return;
-        }
-        console.log(`Language: ${language}`);
-        const chunks = await parser.parseFile(file, content);
-        console.log(`Found ${chunks.length} code chunks:`);
-        chunks.forEach((chunk, index) => {
-          console.log(
-            `  ${index + 1}. ${chunk.type}: ${chunk.name} (${chunk.startLine}-${chunk.endLine})`
-          );
-        });
-        break;
+        {
+          if (!file) {
+            console.error('Please specify a file to parse');
+            return;
+          }
+          if (!existsSync(file)) {
+            console.error(`File not found: ${file}`);
+            return;
+          }
+          console.log(`Parsing file: ${file}`);
+          const parser = new CodeParser();
+          const content = readFileSync(file, 'utf-8');
+          const language = parser.detectLanguage(file);
+          if (!language) {
+            console.error(`Unsupported file type: ${file}`);
+            return;
+          }
+          console.log(`Language: ${language}`);
+          const chunks = await parser.parseFile(file, content);
+          console.log(`Found ${chunks.length} code chunks:`);
+          chunks.forEach((chunk, index) => {
+            console.log(
+              `  ${index + 1}. ${chunk.type}: ${chunk.name} (${chunk.startLine}-${chunk.endLine})`
+            );
+          });
+          break;
+        }
       case 'embed':
-        if (!file) {
-          console.error('Please specify a file to embed');
-          return;
-        }
-        console.log(`Generating embeddings for: ${file}`);
-        const embeddingService = new EmbeddingService();
-        await embeddingService.initialize();
-        console.log('Embedding debug implementation pending...');
-        break;
+        {
+          if (!file) {
+            console.error('Please specify a file to embed');
+            return;
+          }
+          console.log(`Generating embeddings for: ${file}`);
+          const embeddingService = new EmbeddingService();
+          await embeddingService.initialize();
+          console.log('Embedding debug implementation pending...');
+          break;
+        }
       case 'test-connection':
-        console.log('Testing connections...');
-        try {
-          const config = this.configManager.getConfig();
-          const vectorStore = new ChromaVectorStore(
-            config.vectorStore.collectionName,
-            config.vectorStore.host || 'localhost',
-            config.vectorStore.port || 8000,
-            config.vectorStore.authToken || 'test-token'
-          );
-          await vectorStore.initialize();
-          const stats = await vectorStore.getCollectionStats();
-          console.log(`✓ ChromaDB connection successful (${stats.count} vectors)`);
-          await vectorStore.close();
-        } catch (error) {
-          console.error('✗ ChromaDB connection failed:', error);
-        }
-        break;
+        {
+          console.log('Testing connections...');
+          try {
+            const config = this.configManager.getConfig();
+            const vectorStore = new ChromaVectorStore(
+              config.vectorStore.collectionName,
+              config.vectorStore.host || 'localhost',
+              config.vectorStore.port || 8000,
+              config.vectorStore.authToken || 'test-token'
+            );
+            await vectorStore.initialize();
+            const stats = await vectorStore.getCollectionStats();
+            console.log(`✓ ChromaDB connection successful (${stats.count} vectors)`);
+            await vectorStore.close();
+          } catch (error) {
+            console.error('✗ ChromaDB connection failed:', error);
+          }
+          break;
+        }
       default:
         console.error(`Unknown debug action: ${action}`);
         console.log('Available actions: parse, embed, test-connection');
     }
   }

210-216: Harden number parsing with defaults.

Avoid NaN propagation from CLI args.

Apply:

-    const searchOptions = {
-      topK: parseInt(options.topK),
+    const topK = Number.parseInt(options.topK, 10);
+    const minSim = Number.parseFloat(options.minSimilarity);
+    const searchOptions = {
+      topK: Number.isFinite(topK) ? topK : 5,
       language: options.language,
       chunkType: options.type,
       filePath: options.file,
-      minSimilarity: parseFloat(options.minSimilarity),
+      minSimilarity: Number.isFinite(minSim) ? minSim : 0.2,
     };

592-594: “Embedding cache cleared” is misleading; nothing is cleared.

Either wire through an actual cache instance to clear or adjust the message/flag until implemented.


451-453: Config “get” ignores key path and dumps full config.

Implement dot‑path resolution (e.g., vectorStore.host) so the command matches README examples.

test/test-vector-store.ts (2)

195-195: Avoid mixing CommonJS in a TypeScript module.

Prefer ESM export to align with your TS imports and ESLint/Prettier setup.

Apply this diff:

-module.exports = { testVectorStore };
+export { testVectorStore };

75-81: Env var naming: standardize the auth token name.

This file uses CHROMA_AUTH_TOKEN while SemanticSearch uses CHROMA_SERVER_AUTH_CREDENTIALS. Pick one to avoid surprise misconfigurations.

Also applies to: 178-181

src/vector-store.ts (2)

315-341: Tighten types: avoid any for embeddingService.

Use the project’s EmbeddingService interface for compile-time safety.

Apply this diff:

-import { EmbeddingResult } from './embedding-service';
+import { EmbeddingResult, EmbeddingService as IEmbeddingService } from './embedding-service';
@@
-  async searchByText(
+  async searchByText(
     queryText: string,
-    embeddingService: any, // EmbeddingService
+    embeddingService: IEmbeddingService,

90-97: Chroma metadata: 'dimension' is likely ignored.

Chroma sets dimension from first insert; storing it in metadata doesn’t enforce schema. Keep it only if you explicitly read it elsewhere.

Also applies to: 374-384

test/test-storage.ts (2)

148-148: Avoid CommonJS export in TS.

Use ESM export for consistency.

Apply this diff:

-module.exports = { testCodeStorage };
+export { testCodeStorage };

62-141: This is a script, not an assertion-based test.

Consider moving under a scripts/ or examples/ folder or convert to node:test with assertions so it doesn’t accidentally run in CI.

src/semantic-search.ts (2)

36-39: Env var inconsistency with tests.

This file uses CHROMA_SERVER_AUTH_CREDENTIALS, tests use CHROMA_AUTH_TOKEN. Standardize to one name and document it.


122-126: Preserve explicit zero values in config defaults.

Using || overrides valid 0. Use ?? instead.

Apply this diff:

-      const searchOptions = {
-        topK: this.config.maxResults || 10,
-        minSimilarity: this.config.minSimilarity || 0.7,
-        ...options,
-      };
+      const searchOptions = {
+        topK: this.config.maxResults ?? 10,
+        minSimilarity: this.config.minSimilarity ?? 0.7,
+        ...options,
+      };
src/chroma-vector-store.ts (1)

83-101: Include distances/documents explicitly in query.

Some client versions require include to return distances/documents reliably.

Apply this diff:

-import { ChromaClient, Collection } from 'chromadb';
+import { ChromaClient, Collection, IncludeEnum } from 'chromadb';
@@
-      const queryParams: any = {
+      const queryParams: any = {
         queryEmbeddings: [queryVector],
-        nResults: topK,
+        nResults: topK,
+        include: [IncludeEnum.Metadatas, IncludeEnum.Documents, IncludeEnum.Distances],
       };

Also applies to: 104-106

test/debug-watcher.ts (1)

51-53: Await watcher readiness instead of sleeping.

Waiting on the 'ready' event is more robust than a fixed timeout.

Apply this diff:

+import { once } from 'node:events';
@@
-    // Wait for ready
-    await new Promise(resolve => setTimeout(resolve, 1000));
+    // Wait for ready
+    await once(watcher as any, 'ready');

Also applies to: 43-45

src/index.ts (2)

209-214: Unnecessary language field in delete event

handleFileChange('delete') doesn’t require language; passing 'unknown' may fight stricter type‑guards downstream.

       await this.semanticSearch.handleFileChange({
         type: 'delete',
-        filePath,
-        language: 'unknown',
+        filePath,
       });

80-80: Redundant option (nit)

persistent defaults to true in FileWatcher; you can omit it for brevity.

test/test-integration.ts (3)

66-69: Handle both add and change events

New files may emit add; listening to both removes the need to manually process files created after start.

-    fileWatcher.on('fileChanged', handleFileChange);
+    fileWatcher.on('fileAdded', handleFileChange);
+    fileWatcher.on('fileChanged', handleFileChange);

109-111: Avoid arbitrary sleeps in tests

handleFileChange is awaited; extra timeouts slow CI without benefit.

-    // Wait for processing
-    await new Promise(resolve => setTimeout(resolve, 2000));
+    // No sleep needed; processing already awaited

Repeat the same removal after the update.

Also applies to: 181-182


208-212: Duplicate “Test 4” log lines

There are two identical headings back‑to‑back.

-    // Test 4: Find similar patterns
-    console.log('\n7. Test 4: Finding similar patterns...');
-
-    // Test 4: Find similar patterns (already implemented above)
-    console.log('\n7. Test 4: Finding similar patterns...');
+    // Test 4: Find similar patterns
+    console.log('\n7. Test 4: Finding similar patterns...');
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfdfbba and c2e26f8.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (29)
  • .eslintrc.json (1 hunks)
  • .prettierignore (1 hunks)
  • .prettierrc (1 hunks)
  • README.md (10 hunks)
  • docs/todo-phase1.md (1 hunks)
  • package.json (2 hunks)
  • src/chroma-vector-store.ts (9 hunks)
  • src/cli-main.ts (1 hunks)
  • src/cli.ts (31 hunks)
  • src/code-parser.ts (26 hunks)
  • src/code-storage.ts (4 hunks)
  • src/config.ts (6 hunks)
  • src/embedding-service.ts (10 hunks)
  • src/file-watcher.ts (9 hunks)
  • src/index.ts (10 hunks)
  • src/logger.ts (7 hunks)
  • src/semantic-search.ts (10 hunks)
  • src/tree-sitter-types.d.ts (1 hunks)
  • src/types.ts (2 hunks)
  • src/vector-store.ts (12 hunks)
  • test/debug-watcher.ts (3 hunks)
  • test/test-embedding.ts (7 hunks)
  • test/test-file-watcher.ts (12 hunks)
  • test/test-integration.ts (3 hunks)
  • test/test-parser.ts (4 hunks)
  • test/test-semantic-search.ts (9 hunks)
  • test/test-storage.ts (5 hunks)
  • test/test-vector-store.ts (9 hunks)
  • tsconfig.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/vector-store.ts (1)
src/types.ts (1)
  • VectorSearchResult (31-41)
src/semantic-search.ts (3)
src/code-parser.ts (1)
  • CodeParser (65-890)
src/embedding-service.ts (1)
  • EmbeddingService (41-315)
src/types.ts (1)
  • EmbeddingService (58-63)
test/test-file-watcher.ts (1)
src/file-watcher.ts (1)
  • FileWatcher (95-281)
src/chroma-vector-store.ts (1)
src/types.ts (1)
  • VectorSearchResult (31-41)
src/cli.ts (5)
src/index.ts (1)
  • ContextEngine (34-342)
src/semantic-search.ts (1)
  • SemanticSearch (12-228)
src/code-parser.ts (1)
  • CodeParser (65-890)
src/embedding-service.ts (1)
  • EmbeddingService (41-315)
src/types.ts (1)
  • EmbeddingService (58-63)
test/test-integration.ts (3)
src/file-watcher.ts (2)
  • FileWatcher (95-281)
  • FileWatcherOptions (8-27)
src/code-parser.ts (1)
  • CodeParser (65-890)
src/semantic-search.ts (2)
  • createSemanticSearchService (231-233)
  • handleFileChange (175-195)
src/code-parser.ts (1)
src/types.ts (1)
  • CodeChunk (1-17)
src/index.ts (1)
src/logger.ts (1)
  • error (170-172)
🪛 Biome (2.1.2)
src/cli.ts

[error] 619-620: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 620-621: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 621-623: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 629-630: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 646-647: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🪛 markdownlint-cli2 (0.18.1)
README.md

218-218: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (39)
src/code-storage.ts (2)

60-66: Trailing comma in options object is fine (Prettier‑friendly).

No functional change; keeps diffs cleaner. LGTM.


258-264: Trailing comma in getStats return object is fine.

Consistent with Prettier; no behavior change.

tsconfig.json (1)

27-28: Formatting-only change LGTM.

Include/exclude arrays compressed to single lines; no behavioral impact.

test/test-embedding.ts (1)

25-26: Formatting-only changes look fine.

Trailing commas and log wrapping are consistent with the new Prettier rules.

Also applies to: 42-43, 60-61, 71-72, 91-92, 101-103

src/tree-sitter-types.d.ts (1)

35-35: EOF newline fix LGTM.

Good for tooling compatibility.

src/embedding-service.ts (4)

65-70: ESM import runtime check.

Dynamic import of '@xenova/transformers' must remain native import() after transpile. With CommonJS module output, confirm TS preserves it for your TS version.

See the tsconfig comment for a verification script.


96-101: Normalize=true and cosine similarity are aligned.

No issues spotted.


276-276: Similarity calculation usage LGTM.

Correct guard for dimension mismatch.


320-320: Factory export unchanged; fine.

No action needed.

.prettierrc (1)

1-22: Prettier config looks sane and consistent.

Matches common conventions; JSON override is helpful.

test/test-parser.ts (2)

116-119: Console output formatting change LGTM.

Purely aesthetic; no behavior change.


136-149: Incremental parsing test: good coverage.

Nice touch adding a new function to validate change detection.

.prettierignore (1)

1-7: Ignore list generally good.

Covers typical artifacts and large test corpora.

Also applies to: 11-66

docs/todo-phase1.md (1)

36-40: Do not mark "PocketFlow Integration" done — no implementation/tests found.
rg search of src/** returned no occurrences of "PocketFlow", "orchestrat", or "workflow"; only matches were src/config.ts:8 and src/config.ts:94 (retryDelay). docs/todo-phase1.md (lines 36–40) lists the item as completed; add the PocketFlow implementation and tests and ensure CI is green, or revert the checklist entry.

.eslintrc.json (1)

6-12: Strengthen TS/Prettier integration; prevent rule conflicts.

Extend with TS/Prettier presets and use the TS-aware unused-vars rule. Also drop style rules that Prettier will manage to avoid churn.

[ suggest_recommended_refactor ]

Apply:

-  "extends": ["eslint:recommended"],
+  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "plugin:prettier/recommended"],

   "plugins": ["@typescript-eslint", "prettier"],
   "rules": {
     "prettier/prettier": "warn",
     "no-console": "off",
     "no-debugger": "error",
     "prefer-const": "error",
     "no-var": "error",
-    "object-shorthand": "warn",
-    "prefer-arrow-callback": "warn",
-    "prefer-template": "warn",
-    "template-curly-spacing": "error",
-    "arrow-spacing": "error",
-    "comma-dangle": ["warn", "always-multiline"],
-    "quotes": ["error", "single", { "avoidEscape": true }],
-    "semi": ["error", "always"],
-    "no-unused-vars": "warn",
+    "object-shorthand": "warn",
+    "prefer-arrow-callback": "warn",
+    "prefer-template": "warn",
+    "@typescript-eslint/no-unused-vars": ["warn", { "argsIgnorePattern": "^_", "varsIgnorePattern": "^_" }],
     "no-empty": "warn",
     "no-case-declarations": "warn"
   },

Also applies to: 13-31

src/cli-main.ts (1)

12-15: LGTM.

EOF newline fix and CLI bootstrap look good.

src/types.ts (1)

54-56: No action required — implementers already define clear() and close(). Only implementer found: src/chroma-vector-store.ts — it contains both clear() and close().

package.json (1)

53-59: Confirm ESLint/TypeScript/Prettier peer-dependency compatibility.

node_modules not present in the sandbox (MODULE_NOT_FOUND); unable to verify peerDependencies. Verify package.json (lines 53–59) against each package's peerDependencies/engines locally (e.g., run npm view @ peerDependencies or npm ls).

test/test-vector-store.ts (2)

71-72: LGTM: sensible embedding options for tests.

Using an explicit cacheSize keeps memory bounded during the run.


121-127: LGTM: searchSimilar usage and logging.

Signature matches ChromaVectorStore; logging guards are safe.

test/test-storage.ts (2)

24-25: LGTM: deterministic timestamps in fixtures.

Keeps test objects uniform and sortable.

Also applies to: 41-42, 58-59


67-70: LGTM: explicit non-persistent storage for tests.

Avoids polluting local FS.

src/file-watcher.ts (2)

192-197: LGTM: structured DirectoryAdded event payload.

Matches the typed contract and improves readability.


156-157: Confirmed: atomic & awaitWriteFinish forwarded correctly.
Passed to chokidar as this.options.awaitWriteFinish and this.options.atomic; defaults are set before the ...options spread so caller-provided values override.

src/semantic-search.ts (1)

138-151: LGTM: result filtering and ordering.

Simple and clear; respects minSimilarity then sorts desc.

src/chroma-vector-store.ts (2)

65-67: LGTM: persist timestamp in metadata.

Useful for staleness checks and reindexing strategies.

Also applies to: 173-175


24-30: Auth shape verified — supported by Chroma JS client.

ChromaClient accepts auth: { provider: 'token', credentials: '...' } (tokenHeaderType is optional); no change required.

test/debug-watcher.ts (1)

18-22: LGTM: awaitWriteFinish tuned for debug runs.

Reasonable values to reduce event flapping.

test/test-file-watcher.ts (5)

29-35: LGTM: instance creation with ignoreInitial.

Matches typical test baseline.


65-73: Tests expect atomic/awaitWriteFinish to be configurable.

These rely on FileWatcher honoring awaitWriteFinish: false and atomic: false. Ensure constructor fix lands (see src/file-watcher.ts).


75-86: LGTM: event listener registration before start.

Prevents missed events during watcher boot.

Also applies to: 131-141, 232-241


249-258: LGTM: tracked files assertions are resilient.

Substring match handles platform-specific separators.


291-316: LGTM: error-handling test is pragmatic.

Accepts either emitted error or running watcher on nonexistent path.

test/test-semantic-search.ts (3)

65-66: Formatting reflow looks good

Wrapping the long string literal improves readability without changing behavior.


168-174: Multi‑line call formatting is fine

No functional changes; call site remains equivalent.


215-215: Do not convert this file to ESM — repo targets CommonJS

tsconfig.json sets "module": "CommonJS" and package.json has no "type" field; multiple files use module.exports, so replacing this single export with export { ... } would create mixed module systems and likely break build/lint.
Occurrences: test/test-embedding.ts:146, test/test-vector-store.ts:195, test/test-semantic-search.ts:215, test/test-storage.ts:148, src/code-parser.ts:902-920.

Likely an incorrect or invalid review comment.

src/index.ts (1)

135-139: Nice: watcher ready visibility

Ready event + watched count improve observability at startup.

src/logger.ts (2)

88-99: Console method selection is fine

Behavior preserved; reads clean and satisfies typical ESLint style.


245-245: Default export retained

No issues with the default export naming.

Comment on lines +8 to +9
*.js
*.js.map
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not ignore all .js files.

This will prevent Prettier from formatting any JS sources (including config scripts). Narrow the pattern to build outputs only.

-*.js
-*.js.map
+# Built JS maps (if any) in dist/build only
+dist/**/*.js
+dist/**/*.js.map
+build/**/*.js
+build/**/*.js.map
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
*.js
*.js.map
# Built JS maps (if any) in dist/build only
dist/**/*.js
dist/**/*.js.map
build/**/*.js
build/**/*.js.map
🤖 Prompt for AI Agents
In .prettierignore around lines 8-9, the entries "*.js" and "*.js.map" are too
broad and prevent Prettier from formatting all JavaScript sources; replace them
with patterns that only ignore build/artifact outputs (for example
/dist/**/*.js, /build/**/*.js, *.min.js or specific generated folders) so source
JS and config scripts remain formatted; remove the global "*.js" and "*.js.map"
lines and add one or more targeted paths that match only compiled output.

Comment on lines 51 to 52
"@types/chokidar": "^2.1.3",
"@types/node": "^20.11.0",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove @types/chokidar (v3 already ships types) to avoid conflicts.

chokidar@^3 includes its own types; @types/chokidar@^2 can cause type mismatches.

Apply:

-    "@types/chokidar": "^2.1.3",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"@types/chokidar": "^2.1.3",
"@types/node": "^20.11.0",
"@types/node": "^20.11.0",
🤖 Prompt for AI Agents
In package.json around lines 51-52, remove the "@types/chokidar": "^2.1.3"
dependency because chokidar v3 includes its own types and the separate @types
package (v2) causes type conflicts; delete that line from
dependencies/devDependencies, run your package manager (npm/yarn/pnpm) to update
node_modules and the lockfile, and then rebuild/tsc to verify no type errors
remain.

Comment on lines +420 to +422
- [CI/CD Setup Guide](CI_CD_SETUP.md) - Quick start and configuration
- [CI/CD Documentation](CI_CD_DOCUMENTATION.md) - Technical details

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Verify referenced CI/CD docs exist (and link paths).

Ensure CI_CD_SETUP.md and CI_CD_DOCUMENTATION.md are present or adjust links.


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
fd -a 'CI*_*.md' -E 'node_modules' -E 'dist' || true
test -f CI_CD_SETUP.md && echo "OK: CI_CD_SETUP.md" || echo "Missing: CI_CD_SETUP.md"
test -f CI_CD_DOCUMENTATION.md && echo "OK: CI_CD_DOCUMENTATION.md" || echo "Missing: CI_CD_DOCUMENTATION.md"

Length of output: 55


Fix README links: missing CI/CD docs

README.md (lines 420–422) links to CI_CD_SETUP.md and CI_CD_DOCUMENTATION.md but both files are missing from the repo (verification: Missing: CI_CD_SETUP.md; Missing: CI_CD_DOCUMENTATION.md). Add the two files at the referenced paths or update the links to the correct locations.

🤖 Prompt for AI Agents
In README.md around lines 420 to 422 the links point to CI_CD_SETUP.md and
CI_CD_DOCUMENTATION.md which are missing from the repo; either add new files at
those exact paths containing the CI/CD quick start and technical docs, or update
the README links to point to the correct existing files or locations (e.g.,
docs/ci/CI_CD_SETUP.md and docs/ci/CI_CD_DOCUMENTATION.md) and verify relative
paths and filenames; after adding or updating, run a repo link check to ensure
no broken references remain.

Comment on lines 122 to 129
awaitWriteFinish: options.awaitWriteFinish || {
stabilityThreshold: 2000,
pollInterval: 100
pollInterval: 100,
},
ignorePermissionErrors: options.ignorePermissionErrors !== false,
atomic: options.atomic || true,
...options
...options,
} as Required<FileWatcherOptions>;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Boolean options incorrectly override user-specified false.

awaitWriteFinish: options.awaitWriteFinish || {…} and atomic: options.atomic || true force defaults when false is passed. Use nullish coalescing.

Apply this diff:

-      awaitWriteFinish: options.awaitWriteFinish || {
+      awaitWriteFinish: options.awaitWriteFinish ?? {
         stabilityThreshold: 2000,
         pollInterval: 100,
       },
-      ignorePermissionErrors: options.ignorePermissionErrors !== false,
-      atomic: options.atomic || true,
+      ignorePermissionErrors: options.ignorePermissionErrors !== false,
+      atomic: options.atomic ?? true,
       ...options,

This also aligns with tests that pass awaitWriteFinish: false and atomic: false.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
awaitWriteFinish: options.awaitWriteFinish || {
stabilityThreshold: 2000,
pollInterval: 100
pollInterval: 100,
},
ignorePermissionErrors: options.ignorePermissionErrors !== false,
atomic: options.atomic || true,
...options
...options,
} as Required<FileWatcherOptions>;
awaitWriteFinish: options.awaitWriteFinish ?? {
stabilityThreshold: 2000,
pollInterval: 100,
},
ignorePermissionErrors: options.ignorePermissionErrors !== false,
atomic: options.atomic ?? true,
...options,
} as Required<FileWatcherOptions>;
🤖 Prompt for AI Agents
In src/file-watcher.ts around lines 122 to 129, the boolean defaults use logical
OR which treats explicit false as falsy and thus overrides user-specified false;
replace `options.awaitWriteFinish || { ... }` with `options.awaitWriteFinish ??
{ ... }` and `options.atomic || true` with `options.atomic ?? true` so only null
or undefined fall back to defaults, keeping the rest of the options
spread/casting as-is.

Comment on lines +262 to +275
this.logger.debug(
`Indexing batch ${Math.floor(i / batchSize) + 1}/${Math.ceil(supportedFiles.length / batchSize)}`
);

await Promise.all(
batch.map(async filePath => {
try {
const content = readFileSync(filePath, 'utf-8');
await this.semanticSearch!.indexFile(filePath, content);
} catch (error) {
this.logger.error(`Failed to index file ${filePath} during initial indexing:`, error);
}
})
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Initial indexing bypasses size limit and may read huge files

The initial batch path reads content before any size check and skips the configured limit entirely.

Add a stat‑based check before reading:

         await Promise.all(
           batch.map(async filePath => {
             try {
-              const content = readFileSync(filePath, 'utf-8');
-              await this.semanticSearch!.indexFile(filePath, content);
+              const { size } = require('fs').statSync(filePath);
+              const config = this.configManager.getConfig();
+              if (size > config.security.maxFileSize) {
+                this.logger.warn(`File too large, skipping during initial indexing: ${filePath}`);
+                return;
+              }
+              const content = readFileSync(filePath, 'utf-8');
+              await this.semanticSearch!.indexFile(filePath, content);
             } catch (error) {
               this.logger.error(`Failed to index file ${filePath} during initial indexing:`, error);
             }
           })
         );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.logger.debug(
`Indexing batch ${Math.floor(i / batchSize) + 1}/${Math.ceil(supportedFiles.length / batchSize)}`
);
await Promise.all(
batch.map(async filePath => {
try {
const content = readFileSync(filePath, 'utf-8');
await this.semanticSearch!.indexFile(filePath, content);
} catch (error) {
this.logger.error(`Failed to index file ${filePath} during initial indexing:`, error);
}
})
);
this.logger.debug(
`Indexing batch ${Math.floor(i / batchSize) + 1}/${Math.ceil(supportedFiles.length / batchSize)}`
);
await Promise.all(
batch.map(async filePath => {
try {
const { size } = require('fs').statSync(filePath);
const config = this.configManager.getConfig();
if (size > config.security.maxFileSize) {
this.logger.warn(`File too large, skipping during initial indexing: ${filePath}`);
return;
}
const content = readFileSync(filePath, 'utf-8');
await this.semanticSearch!.indexFile(filePath, content);
} catch (error) {
this.logger.error(`Failed to index file ${filePath} during initial indexing:`, error);
}
})
);

Comment on lines +339 to 340
uptime: this.isRunning ? Date.now() - this.startTime : 0,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

uptime never increases; startTime is never set

getStats uses startTime, but start() doesn’t assign it.

Add after the watcher starts (before logging “started successfully”):

// in start(), after: await this.fileWatcher.start();
this.startTime = Date.now();

🏁 Script executed:

#!/bin/bash
rg -nP -C2 '\bstartTime\b' --type=ts

Length of output: 2095


Set startTime in start() so uptime increments

startTime is declared (src/index.ts:41) and used in getStats (src/index.ts:339) but never assigned — uptime will remain 0. Add after the watcher starts (after await this.fileWatcher.start(), before logging "started successfully"):

this.startTime = Date.now();
🤖 Prompt for AI Agents
In src/index.ts around lines 339-340, startTime is declared but never assigned
so getStats uptime stays 0; after the watcher starts (immediately after the
await this.fileWatcher.start() call and before logging "started successfully")
assign the start timestamp to this.startTime using the current time so uptime
increments correctly; ensure this assignment only happens on successful start
and consider clearing or resetting startTime when stopping the watcher.

Comment on lines +71 to +77
const formattedArgs =
args.length > 0
? ` ${args
.map(arg => (typeof arg === 'object' ? JSON.stringify(arg, null, 2) : String(arg)))
.join(' ')}`
: '';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Safer arg formatting; include Error stacks

JSON.stringify drops Error details and can throw on cycles. Prefer util.inspect and handle Error specially.

-    const formattedArgs =
-      args.length > 0
-        ? ` ${args
-            .map(arg => (typeof arg === 'object' ? JSON.stringify(arg, null, 2) : String(arg)))
-            .join(' ')}`
-        : '';
+    const util = require('util');
+    const formattedArgs =
+      args.length > 0
+        ? ' ' +
+          args
+            .map(arg =>
+              arg instanceof Error ? arg.stack || arg.message : util.inspect(arg, { depth: 6, colors: false })
+            )
+            .join(' ')
+        : '';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const formattedArgs =
args.length > 0
? ` ${args
.map(arg => (typeof arg === 'object' ? JSON.stringify(arg, null, 2) : String(arg)))
.join(' ')}`
: '';
const util = require('util');
const formattedArgs =
args.length > 0
? ' ' +
args
.map(arg =>
arg instanceof Error ? arg.stack || arg.message : util.inspect(arg, { depth: 6, colors: false })
)
.join(' ')
: '';
🤖 Prompt for AI Agents
In src/logger.ts around lines 71 to 77, the current args formatting uses
JSON.stringify which drops Error stacks and can throw on cyclic structures;
replace that logic to use Node's util.inspect and special-case Error instances
to include their stack (err.stack || err.message) so errors are fully logged,
and call util.inspect(obj, { depth: null, maxArrayLength: null }) for other
objects to avoid cyclic-structure exceptions and preserve readable output;
ensure util is imported and trim/format the resulting string similarly to the
existing code.

Comment on lines +61 to 62
path: this.options.persistDirectory,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

ChromaClient path likely incorrect (points to a local folder).

path should be the server URL (e.g., http://localhost:8000). Passing a filesystem directory here will not connect to a server.

Example:

-    this.client = new ChromaClient({
-      path: this.options.persistDirectory,
-    });
+    this.client = new ChromaClient({
+      path: process.env.CHROMA_URL || 'http://localhost:8000',
+    });

If you intended true local persistence, confirm JS client supports it; otherwise drop persistDirectory from here and document server config.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
path: this.options.persistDirectory,
});
this.client = new ChromaClient({
path: process.env.CHROMA_URL || 'http://localhost:8000',
});
🤖 Prompt for AI Agents
In src/vector-store.ts around lines 61 to 62, the ChromaClient is being
constructed with path: this.options.persistDirectory which points at a local
filesystem folder; change this to the Chroma server URL (e.g.,
http://localhost:8000) when connecting to a running Chroma server, or if you
truly intend local on-disk persistence, verify the JS client supports a
filesystem-backed mode and switch to the correct client/config option for local
persistence; if server mode is used, remove persistDirectory from the client
config and instead document/accept a serverUrl option and pass that to
ChromaClient.

Comment on lines +24 to +27
const { CodeParser } = require('../src/code-parser');
const parser = new CodeParser();
const storage = new CodeStorage();
const semanticSearch = createSemanticSearchService();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prefer ESM import over require to satisfy ESLint

Avoid require in TS; dynamic import works inside async function and keeps tests ESM‑friendly.

-    const { CodeParser } = require('../src/code-parser');
-    const parser = new CodeParser();
+    const { CodeParser } = await import('../src/code-parser');
+    const parser = new CodeParser();

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In test/test-integration.ts around lines 24-27, replace the CommonJS require
usage with ESM imports to satisfy ESLint: remove the require calls and use
top-level ES module imports for CodeParser, CodeStorage and
createSemanticSearchService (or, if the test runner executes tests as CommonJS,
perform dynamic import() inside an async beforeAll/describe setup), then
instantiate the classes as before; ensure the file is treated as ESM by the test
runner config or by using dynamic import in async setup.

Comment on lines +256 to +267
// Clean up
console.log('\n🧹 Cleaning up...');
if (fileWatcher) {
await fileWatcher.stop();
}

if (fs.existsSync(testDir)) {
fs.rmSync(testDir, { recursive: true, force: true });
}

console.log(' Cleanup completed');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Close semanticSearch in cleanup (resource hygiene)

fileWatcher is stopped; also close the semantic search service to release resources.

Add (outside the shown range) a variable hoist and close call:

// near line 12, hoist:
let semanticSearch: ReturnType<typeof createSemanticSearchService> | null = null;

// near line 27, assign:
semanticSearch = createSemanticSearchService();

// in finally, before removing dir:
if (semanticSearch) {
  await semanticSearch.close();
}

To find other tests needing this:


🏁 Script executed:

#!/bin/bash
rg -nP --type=ts 'createSemanticSearchService\(' -C2

Length of output: 1029


Close semanticSearch in cleanup (resource hygiene)

  • Hoist semanticSearch to the test scope and close it in the finally before removing testDir. In test/test-integration.ts semanticSearch is created as const at line 27 and cleanup is at lines 256–267 — change to:

    • let semanticSearch: ReturnType<typeof createSemanticSearchService> | null = null;
    • semanticSearch = createSemanticSearchService();
    • in finally (before fs.rmSync): if (semanticSearch) await semanticSearch.close();
  • Also update test/test-semantic-search.ts (semanticSearch created at line 104 inside try) to the same hoist-and-close pattern.

🤖 Prompt for AI Agents
In test/test-integration.ts around lines 256–267 and
test/test-semantic-search.ts around line 104, semanticSearch is created inside
narrow scopes and never explicitly closed before removing the testDir; hoist the
variable to test scope as: let semanticSearch: ReturnType<typeof
createSemanticSearchService> | null = null, assign it where currently created
(semanticSearch = createSemanticSearchService()), and in the finally block
before fs.rmSync call check if (semanticSearch) await semanticSearch.close(); to
ensure the service is closed prior to deleting test artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant