Develop #34
Develop #34
Conversation
…kage build verification
…iption management
- Changed CopilotParser to extend BaseParser instead of AIAssistantParser. - Updated message extraction logic to utilize ChatMessage and ChatMessageData types. - Introduced a new method parseResponseItem to handle various response types from the assistant. - Removed deprecated ChatHubService and ChatImportService implementations. - Cleaned up CLI commands by removing chat import functionalities and related utilities. - Updated ChatSessionEntity to remove unused fields and methods related to tags and linked devlogs.
…and delete obsolete Docker test script
- Removed Logger interface and ConsoleLogger implementation from AI parsers. - Enhanced ChatMessageMetadata with detailed message types and additional fields. - Introduced ChatTurn model to encapsulate request-response cycles in chat sessions. - Updated ChatSession to include new metadata and improved structure. - Renamed NoteCategory to DevlogNoteCategory for clarity and consistency. - Updated all references to NoteCategory across the codebase to DevlogNoteCategory. - Refactored MCPAdapter methods to use new naming conventions for devlog operations. - Adjusted schemas and API endpoints to align with new note handling structure. - Improved note category handling in the frontend components and utilities.
…ebounce functionality
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements major architectural refactoring to improve realtime functionality, centralize project management, and enhance type safety across the devlog system. The changes introduce proper debouncing for project fetching, implement project CRUD operations with settings UI, and refactor the realtime system for better server-client coordination.
Key changes:
- Project Management Enhancement: Adds project settings page with update/delete operations, debounced project fetching, and proper navigation integration
- Realtime System Refactoring: Replaces direct store usage with custom hooks, implements server-side provider selection, and adds proper configuration API
- Type System Updates: Renames
NoteCategorytoDevlogNoteCategorythroughout codebase for consistency
Reviewed Changes
Copilot reviewed 91 out of 93 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/app/stores/project-store.ts | Adds debounced project fetching and CRUD operations (updateProject, deleteProject) |
| packages/web/app/projects/[id]/settings/ | New project settings page with form handling and delete confirmation |
| packages/web/app/lib/realtime/ | Major refactoring of realtime system with server config API and provider selection |
| packages/web/app/hooks/use-realtime.ts | Replaces direct store usage with proper event subscription hooks |
| packages/core/src/types/core.ts | Renames NoteCategory to DevlogNoteCategory for consistency |
| packages/mcp/src/ | Updates tool schemas and handlers to match core type changes |
Comments suppressed due to low confidence (1)
packages/web/app/lib/realtime/realtime-service.ts:109
- [nitpick] Console.log statement inside a try block could create noise in production logs. Consider using a proper logging system or removing debug logs for production.
return;
| }, | ||
|
|
||
| fetchCurrentProject: async () => { | ||
| debouncedFetch(); |
There was a problem hiding this comment.
The debounced fetch function is called without await, which could lead to race conditions or unhandled promise rejections. Consider awaiting the call or handling errors explicitly.
| debouncedFetch(); | |
| await debouncedFetch(); |
|
|
||
| useEffect(() => { | ||
| setCurrentProjectId(projectId); | ||
| }, [projectId]); |
There was a problem hiding this comment.
The setCurrentProjectId call in useEffect doesn't include setCurrentProjectId in the dependency array, which could cause issues if the function reference changes. Add it to the dependency array.
| }, [projectId]); | |
| }, [projectId, setCurrentProjectId]); |
| error, | ||
| ); |
There was a problem hiding this comment.
The fallback to client detection doesn't preserve the original server error information, making debugging difficult. Consider logging the server error before falling back.
| error, | |
| ); | |
| error, | |
| error && error.stack ? error.stack : '', | |
| ); | |
| // Note: The error is logged but not propagated; fallback config is returned. |
|
|
||
| fetchCurrentDevlog(); | ||
| fetchCurrentDevlogNotes(); | ||
| try { |
There was a problem hiding this comment.
The try-catch block around fetchCurrentDevlog() and fetchCurrentDevlogNotes() only logs warnings but doesn't handle the error properly. Consider showing user feedback or retrying the operation.
| await fetchProjects(); | ||
| toast.success('Project created successfully'); |
There was a problem hiding this comment.
The async callback in subscribe could lead to unhandled promise rejections if fetchProjects() fails. Consider wrapping in try-catch or using a non-async callback.
| await fetchProjects(); | |
| toast.success('Project created successfully'); | |
| try { | |
| await fetchProjects(); | |
| toast.success('Project created successfully'); | |
| } catch (error) { | |
| console.error('Error fetching projects after creation:', error); | |
| toast.error('Failed to refresh project list'); | |
| } |
| } | ||
|
|
||
| // Cache for server configuration to avoid repeated API calls | ||
| let serverConfigCache: RealtimeConfig | null = null; |
There was a problem hiding this comment.
Module-level caching without proper invalidation could lead to stale configuration being used across the application lifecycle. Consider implementing cache invalidation or using a more robust caching strategy.
No description provided.