-
Notifications
You must be signed in to change notification settings - Fork 26
Development #280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development #280
Conversation
Remove file watcher
- Introduces persistent codebase analysis using SQLite for caching analysis results. - Implements a background analysis worker to perform intensive codebase analysis operations without blocking the main VS Code thread. - Adds commands to show cache status, clear the cache, and refresh the analysis. - Improves performance and responsiveness by caching analysis results and reusing them when appropriate.
…alysis - Replaces the previous JSON-based database implementation with SQLite using the sql.js library. - Adds sql.js as a project dependency. - Implements database initialization, table creation, data caching, and cleanup using SQLite. - Introduces an ErrorHandler class to centralize error handling across services. - Implements stub methods for chat history functionality when the database is disabled. - Updates codebase analysis worker to analyze file contents, endpoints, and models.
This commit introduces a new approach to codebase analysis by implementing file-specific analyzers, improving maintainability and testability. It also addresses error handling during file analysis and enhances data model extraction. - Implemented file-specific analyzers using the Strategy Pattern for different file types (TypeScript, JavaScript, Python, Config). - Added AnalyzerFactory to manage and provide the appropriate analyzer for a given file. - Improved error handling during file reading and analysis to provide more informative error messages to the user. - Enhanced data model extraction to include classes, React components, and potential API endpoints from analyzed files. - Switched to in-memory chat history repository
…revention, and performance optimizations - Implemented persistent chat history using SQLite database for cross-session data retention - Added input sanitization to prevent prompt injection in architectural recommendation command - Optimized performance by caching regex patterns at class level in file analyzers - Added new SQL execution methods to SqliteDatabaseService for extensibility - Enhanced error handling for file operations in CodebaseAnalysisWorker
This commit introduces a worker-based architecture for managing chat history to prevent blocking the main VS Code thread. It enhances the AgentService with asynchronous chat history operations using SQLite for persistent storage. - Implemented ChatHistoryWorker for non-blocking database operations - Integrated ChatHistoryWorker into AgentService for managing chat history - Added support for saving, retrieving, clearing, and adding chat messages asynchronously - Implemented optimized retrieval of recent chat history with limit - Added background cleanup operations for old chat history across all agents - Included comprehensive tests for worker operations, concurrency, and error handling - Added documentation for the new architecture in
…Documentation Refactor ChatHistoryWorker to use an enum (ChatHistoryWorkerOperation) for operation types, improving type safety. Add comprehensive JSDoc-style comments to ChatHistoryWorker, detailing its purpose, architecture, and usage. Create new documentation files (SETTIMEOUT_EXPLANATION.md and WEBWORKER_BLOCKERS_VSCODE.md) to explain the rationale behind using setTimeout and the limitations of web workers in VS Code extensions.
- Remove redundant file storage writes in AgentService, using SQLite as the primary storage and file storage as a fallback. - Fix 10-second delay in history restoration by loading chat history immediately. - Synchronize provider arrays with the database on startup to ensure all providers see the same persistent history. - Upgrade better-sqlite3 dependency to version 12.2.0 and add sql.js dependency
- Remove redundant file storage writes in AgentService, using SQLite as the primary storage and file storage as a fallback. - Fix 10-second delay in history restoration by loading chat history immediately. - Synchronize provider arrays with the database on startup to ensure all providers see the same persistent history. - Upgrade better-sqlite3 dependency to version 12.2.0 and add sql.js dependency
…reads - Introduces SQLite database for persistent chat history storage, replacing in-memory storage. - Implements a web worker for asynchronous chat history operations to prevent blocking the main thread. - Adds class and to manage database interactions. - Introduces to support SQLite operations. - Upgrades package version to 3.4.6.
Remove file watcher
- Integrates WASM SQLite for chat history storage via chat history worker. - Adds sql-wasm.wasm file. - Implements fallback mechanism to file storage for backward compatibility. - Optimizes recent chat history retrieval using the worker.
Feature chathistory
Feature chathistory2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comprehensive development infrastructure upgrade for CodeBuddy, focusing on persistent codebase understanding, optimized chat history management, and enhanced architectural patterns. The changes eliminate redundant storage operations, remove artificial delays, and introduce robust SQLite-based persistence for better user experience and data consistency.
Key Changes:
- Removes redundant file storage writes in favor of SQLite-first approach with intelligent fallback
- Eliminates 10-second artificial delay in chat history restoration for immediate loading
- Synchronizes provider-specific chat history arrays with persistent database on startup
Reviewed Changes
Copilot reviewed 38 out of 48 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
src/webview-providers/manager.ts |
Removes 10-second delay, adds immediate chat history restoration with error handling |
src/webview-providers/groq.ts |
Adds provider-specific chat history synchronization method |
src/webview-providers/gemini.ts |
Implements chat history array synchronization with database |
src/webview-providers/deepseek.ts |
Adds updateProviderChatHistory override for Deepseek-specific format |
src/webview-providers/base.ts |
Implements synchronizeChatHistoryFromDatabase infrastructure |
src/webview-providers/anthropic.ts |
Adds Message import and provider-specific history synchronization |
src/utils/error-handling.ts |
New centralized error handling utility with timeout and safety patterns |
src/test/suite/persistent-chat-history.test.ts |
Comprehensive test suite for SQLite-based chat history operations |
src/test/suite/codebase-understanding-cache.test.ts |
Updates fs module mocking for native file operations |
src/test/suite/codebase-analysis-cache.test.ts |
Adds fs import for test compatibility |
src/services/sqlite-database.service.ts |
Complete SQLite service with WASM-based persistence and schema management |
src/services/persistent-codebase-understanding.service.ts |
New service for intelligent codebase analysis caching with git state tracking |
src/services/file-watcher.ts |
Comments out file watching service (temporarily disabled) |
src/services/context-retriever.ts |
Comments out CodeRepository dependency |
src/services/codebase-analysis-worker.ts |
Comprehensive worker for non-blocking codebase analysis operations |
src/services/code-indexing.ts |
Comments out deprecated code indexing service |
src/services/chat-history-worker.ts |
Worker-based chat history operations with SQLite persistence |
src/services/analyzers/* |
Modular file analyzers with static regex optimization |
src/services/agent-state.ts |
Enhanced with ChatHistoryWorker integration and intelligent fallback |
src/infrastructure/repository/db-chat-history.ts |
SQLite-based chat history repository with comprehensive CRUD operations |
src/infrastructure/repository/code.ts |
Removes better-sqlite3 dependency |
src/extension.ts |
Integrates persistent services and adds cache management commands |
src/emitter/interface.ts |
Removes deprecated file watcher events |
src/commands/architectural-recommendation.ts |
Enhanced with persistent codebase service and security improvements |
package.json |
Updates dependencies, removes better-sqlite3, adds sql.js |
docs/* |
Comprehensive documentation for architecture and testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.value([mockWorkspaceFolder]); | ||
|
||
// Mock file system operations | ||
// Mock file system operations for the native fs module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment should be more specific about why native fs module is being mocked instead of vscode.workspace.fs. Consider explaining that this is for CodebaseAnalysisWorker which uses native fs operations.
// Mock file system operations for the native fs module | |
// Mock native fs module operations because CodebaseAnalysisWorker uses native fs methods (not vscode.workspace.fs) |
Copilot uses AI. Check for mistakes.
executeSql(query: string, params: any[] = []): any[] { | ||
if (!this.db) { | ||
throw new Error("Database not initialized"); | ||
} | ||
|
||
try { | ||
const stmt = this.db.prepare(query); | ||
const results = stmt.getAsObject(params); | ||
stmt.free(); | ||
return Array.isArray(results) ? results : [results]; | ||
} catch (error) { | ||
this.logger.error(`SQL execution failed: ${query}`, error); | ||
throw error; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name suggests it returns results but the implementation uses stmt.getAsObject(params) which doesn't execute the statement properly. It should use stmt.bind(params) followed by stmt.step() and stmt.getAsObject() for SELECT queries, or use stmt.all(params) for getting all results.
Copilot uses AI. Check for mistakes.
executeSqlAll(query: string, params: any[] = []): any[] { | ||
if (!this.db) { | ||
throw new Error("Database not initialized"); | ||
} | ||
|
||
try { | ||
const stmt = this.db.prepare(query); | ||
const results = stmt.all(params); | ||
stmt.free(); | ||
return results; | ||
} catch (error) { | ||
this.logger.error(`SQL query execution failed: ${query}`, error); | ||
throw error; | ||
} | ||
} | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The executeSqlAll method correctly uses stmt.all(params) while executeSql uses stmt.getAsObject(params). Consider renaming executeSql to executeSqlFirst or removing it in favor of executeSqlAll since the current implementation is confusing.
executeSqlAll(query: string, params: any[] = []): any[] { | |
if (!this.db) { | |
throw new Error("Database not initialized"); | |
} | |
try { | |
const stmt = this.db.prepare(query); | |
const results = stmt.all(params); | |
stmt.free(); | |
return results; | |
} catch (error) { | |
this.logger.error(`SQL query execution failed: ${query}`, error); | |
throw error; | |
} | |
} | |
/** | |
/** | |
* Execute SQL query and get the first result (or undefined if none) | |
*/ | |
executeSqlFirst(query: string, params: any[] = []): any | undefined { | |
if (!this.db) { | |
throw new Error("Database not initialized"); | |
} | |
try { | |
const stmt = this.db.prepare(query); | |
const result = stmt.getAsObject(params); | |
stmt.free(); | |
return result; | |
} catch (error) { | |
this.logger.error(`SQL query execution failed: ${query}`, error); | |
throw error; | |
} | |
} | |
/** |
Copilot uses AI. Check for mistakes.
private isRunning = false; | ||
|
||
/** Flag indicating if current analysis should be cancelled */ | ||
private readonly shouldCancel = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shouldCancel property is declared as readonly false but never used for cancellation logic. The cancellation checking relies on currentCancellationToken?.isCancellationRequested. Either remove this unused property or implement proper cancellation logic with it.
private readonly shouldCancel = false; |
Copilot uses AI. Check for mistakes.
private isProcessing: boolean = false; | ||
private currentRequestId: string | null = null; | ||
|
||
constructor() { | ||
this.chatHistoryRepo = ChatHistoryRepository.getInstance(); | ||
} | ||
|
||
/** | ||
* Process chat history operation request | ||
*/ | ||
async processRequest( | ||
operation: ChatHistoryWorkerOperation, | ||
data: ChatHistoryWorkerData, | ||
requestId: string, | ||
): Promise<any> { | ||
if (this.isProcessing) { | ||
throw new Error("Worker is busy processing another request"); | ||
} | ||
|
||
this.isProcessing = true; | ||
this.currentRequestId = requestId; | ||
|
||
try { | ||
let result: any; | ||
|
||
switch (operation) { | ||
case ChatHistoryWorkerOperation.GET_CHAT_HISTORY: | ||
result = await this.getChatHistory(data.agentId); | ||
break; | ||
|
||
case ChatHistoryWorkerOperation.SAVE_CHAT_HISTORY: | ||
if (!data.history) { | ||
throw new Error("History data is required for save operation"); | ||
} | ||
await this.saveChatHistory(data.agentId, data.history); | ||
result = { success: true }; | ||
break; | ||
|
||
case ChatHistoryWorkerOperation.CLEAR_CHAT_HISTORY: | ||
await this.clearChatHistory(data.agentId); | ||
result = { success: true }; | ||
break; | ||
|
||
case ChatHistoryWorkerOperation.ADD_CHAT_MESSAGE: | ||
if (!data.message) { | ||
throw new Error("Message data is required for add operation"); | ||
} | ||
await this.addChatMessage(data.agentId, data.message); | ||
result = { success: true }; | ||
break; | ||
|
||
case ChatHistoryWorkerOperation.GET_RECENT_HISTORY: { | ||
const limit = data.config?.limit || 50; | ||
result = await this.getRecentHistory(data.agentId, limit); | ||
break; | ||
} | ||
|
||
case ChatHistoryWorkerOperation.CLEANUP_OLD_HISTORY: { | ||
const daysToKeep = data.config?.daysToKeep || 30; | ||
await this.cleanupOldHistory(daysToKeep); | ||
result = { success: true }; | ||
break; | ||
} | ||
|
||
default: | ||
throw new Error(`Unknown operation: ${operation}`); | ||
} | ||
|
||
return result; | ||
} finally { | ||
this.isProcessing = false; | ||
this.currentRequestId = null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The worker rejects concurrent requests instead of queuing them. For a production system, consider implementing a request queue to handle multiple concurrent operations gracefully rather than failing with 'busy' errors.
private isProcessing: boolean = false; | |
private currentRequestId: string | null = null; | |
constructor() { | |
this.chatHistoryRepo = ChatHistoryRepository.getInstance(); | |
} | |
/** | |
* Process chat history operation request | |
*/ | |
async processRequest( | |
operation: ChatHistoryWorkerOperation, | |
data: ChatHistoryWorkerData, | |
requestId: string, | |
): Promise<any> { | |
if (this.isProcessing) { | |
throw new Error("Worker is busy processing another request"); | |
} | |
this.isProcessing = true; | |
this.currentRequestId = requestId; | |
try { | |
let result: any; | |
switch (operation) { | |
case ChatHistoryWorkerOperation.GET_CHAT_HISTORY: | |
result = await this.getChatHistory(data.agentId); | |
break; | |
case ChatHistoryWorkerOperation.SAVE_CHAT_HISTORY: | |
if (!data.history) { | |
throw new Error("History data is required for save operation"); | |
} | |
await this.saveChatHistory(data.agentId, data.history); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.CLEAR_CHAT_HISTORY: | |
await this.clearChatHistory(data.agentId); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.ADD_CHAT_MESSAGE: | |
if (!data.message) { | |
throw new Error("Message data is required for add operation"); | |
} | |
await this.addChatMessage(data.agentId, data.message); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.GET_RECENT_HISTORY: { | |
const limit = data.config?.limit || 50; | |
result = await this.getRecentHistory(data.agentId, limit); | |
break; | |
} | |
case ChatHistoryWorkerOperation.CLEANUP_OLD_HISTORY: { | |
const daysToKeep = data.config?.daysToKeep || 30; | |
await this.cleanupOldHistory(daysToKeep); | |
result = { success: true }; | |
break; | |
} | |
default: | |
throw new Error(`Unknown operation: ${operation}`); | |
} | |
return result; | |
} finally { | |
this.isProcessing = false; | |
this.currentRequestId = null; | |
} | |
private requestQueue: Array<{ | |
operation: ChatHistoryWorkerOperation, | |
data: ChatHistoryWorkerData, | |
requestId: string, | |
resolve: (value: any) => void, | |
reject: (reason?: any) => void | |
}> = []; | |
private processing: boolean = false; | |
constructor() { | |
this.chatHistoryRepo = ChatHistoryRepository.getInstance(); | |
} | |
/** | |
* Process chat history operation request (queues requests) | |
*/ | |
processRequest( | |
operation: ChatHistoryWorkerOperation, | |
data: ChatHistoryWorkerData, | |
requestId: string, | |
): Promise<any> { | |
return new Promise((resolve, reject) => { | |
this.requestQueue.push({ operation, data, requestId, resolve, reject }); | |
this.processQueue(); | |
}); | |
} | |
/** | |
* Internal method to process the request queue sequentially | |
*/ | |
private async processQueue() { | |
if (this.processing) return; | |
this.processing = true; | |
while (this.requestQueue.length > 0) { | |
const { operation, data, requestId, resolve, reject } = this.requestQueue.shift()!; | |
try { | |
let result: any; | |
switch (operation) { | |
case ChatHistoryWorkerOperation.GET_CHAT_HISTORY: | |
result = await this.getChatHistory(data.agentId); | |
break; | |
case ChatHistoryWorkerOperation.SAVE_CHAT_HISTORY: | |
if (!data.history) { | |
throw new Error("History data is required for save operation"); | |
} | |
await this.saveChatHistory(data.agentId, data.history); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.CLEAR_CHAT_HISTORY: | |
await this.clearChatHistory(data.agentId); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.ADD_CHAT_MESSAGE: | |
if (!data.message) { | |
throw new Error("Message data is required for add operation"); | |
} | |
await this.addChatMessage(data.agentId, data.message); | |
result = { success: true }; | |
break; | |
case ChatHistoryWorkerOperation.GET_RECENT_HISTORY: { | |
const limit = data.config?.limit || 50; | |
result = await this.getRecentHistory(data.agentId, limit); | |
break; | |
} | |
case ChatHistoryWorkerOperation.CLEANUP_OLD_HISTORY: { | |
const daysToKeep = data.config?.daysToKeep || 30; | |
await this.cleanupOldHistory(daysToKeep); | |
result = { success: true }; | |
break; | |
} | |
default: | |
throw new Error(`Unknown operation: ${operation}`); | |
} | |
resolve(result); | |
} catch (err) { | |
reject(err); | |
} | |
} | |
this.processing = false; |
Copilot uses AI. Check for mistakes.
const persistentCodebaseService = | ||
PersistentCodebaseUnderstandingService.getInstance(); | ||
persistentCodebaseService.shutdown(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shutdown() method is called without await, but it's an async method. This should be 'await persistentCodebaseService.shutdown()' and the deactivate function should be marked as async.
Copilot uses AI. Check for mistakes.
protected async synchronizeChatHistoryFromDatabase(): Promise<void> { | ||
try { | ||
// Get chat history from database via AgentService | ||
const agentId = "agentId"; // Using the same hardcoded ID as WebViewProviderManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded agent ID should be extracted to a constant or passed as a parameter. This magic string creates coupling and makes the code harder to maintain and test.
Copilot uses AI. Check for mistakes.
This commit introduces detailed documentation for several key features, including: - Adds a roadmap outlining the future vision and development plans for CodeBuddy. - Provides an in-depth analysis and implementation plan for context-aware code completion. - Creates a demo script for showcasing CodeBuddy's capabilities at the Singapore AI Showcase. - Introduces the concept of an embedded knowledge graph for enhanced code understanding. - Describes the implementation of a knowledge graph to transform codebase understanding. - Includes a pitch deck tailored for investors and business stakeholders. - Contains visual slides suitable for a lightning talk presentation.
This commit introduces the initial documentation for the MCP and A2A integration within CodeBuddy. - Add documents describing architecture, the A2A protocol, specialist agents, implementation strategy, and use cases. - These files detail the strategic implementation of the Model Context Protocol (MCP) and Agent-to-Agent (A2A) integration into CodeBuddy. - Outline the goal of transforming CodeBuddy into the definitive AI development platform.
…tures Adds complete Model Context Protocol (MCP) client and server implementations with all official features, including sampling, roots, and elicitation. Implements a hybrid architecture for multi-server coordination and improved resilience. - Implements official MCP client features: sampling, roots, and elicitation - Implements official MCP server features: tools, resources, and prompts - Introduces hybrid MCP architecture with central and per-agent fallback strategies - Adds VS Code commands for MCP server status, root management, and elicitation settings - Implements dynamic agent registry for auto-discovery and health monitoring - Includes detailed documentation and implementation notes
No description provided.