feat(memory): improve memory handling and regex performance across co…#198
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes regex patterns for linear-time matching, refactors the MemoryView component to use derived state via useMemo instead of an effect, and improves type safety in the memory store by explicitly handling undefined values. It also updates the sorting logic for orphan files and refines the copy-to-clipboard UI. Feedback was provided regarding the mirroring of IPC channel constants in the main process, suggesting they be moved to a shared directory to maintain a single source of truth and reduce maintenance overhead.
| const MEMORY_HAS_MEMORY = 'memory:hasMemory'; | ||
| const MEMORY_GET_INDEX = 'memory:getIndex'; | ||
| const MEMORY_READ_FILE = 'memory:readFile'; | ||
| const MEMORY_LIST_OPENERS = 'memory:listAvailableOpeners'; | ||
| const MEMORY_OPEN_IN = 'memory:openIn'; | ||
| const MEMORY_COPY_PATH = 'memory:copyPath'; |
There was a problem hiding this comment.
Mirroring IPC channel constants locally in the main process creates a maintenance burden and risks desynchronization with the preload script. While respecting module boundaries is important, the standard pattern in Electron is to place shared constants in a src/shared directory that can be imported by both main, renderer, and preload processes. This ensures a single source of truth for IPC channels.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe PR refactors the memory viewer's display logic to use a computed fallback selection (displayedFile) derived from user choice or the first row. State types are widened to allow undefined entries for distinguishing "not yet loaded" from loaded values. Regex patterns are hardened against backtracking, IPC constants are moved to local scope, and supporting documentation is clarified. ChangesMemory Feature State and Display
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR improves the Memory feature’s parsing/performance characteristics and clarifies “not loaded yet” state handling across renderer components, while also tightening some implementation details (regexes, sorting, and IPC channel constant ownership).
Changes:
- Replaced potentially backtracking regex patterns with bounded/negated character classes for linear-time matching (memory index entries + wikilinks).
- Updated the memory Zustand slice caches to use
undefinedfor “not loaded yet” to distinguish from loaded falsy values. - Refactored
MemoryViewto compute the displayed file during render (instead of effect-driven state syncing) and adjusted copy UI state handling.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/utils/memoryIndex.ts | Hardens entry parsing regex for linear-time behavior and adjusts orphan sorting comparator. |
| src/renderer/store/slices/memorySlice.ts | Changes cache record value types to include undefined to represent “not loaded yet”; updates refresh invalidation typing. |
| src/renderer/components/sidebar/memory/OpenInMenu.tsx | Expands rationale on a hooks lint-disable comment. |
| src/renderer/components/memory/MemoryView.tsx | Uses a precompiled wikilink regex and refactors selection/display logic + copy button state rendering. |
| src/main/utils/openInLauncher.ts | Improves inline documentation for the Antigravity opener detection behavior. |
| src/main/ipc/memory.ts | Replaces preload import of IPC channel constants with locally defined channel strings and documents the module boundary rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hasMemoryByProjectId: Record<string, boolean | undefined>; | ||
| indexByProjectId: Record<string, MemoryIndex | null | undefined>; | ||
| expandedEntriesByProjectId: Record<string, string[] | undefined>; | ||
| fileContents: Record<string, string | undefined>; | ||
| memoryLoadingByProjectId: Record<string, boolean | undefined>; |
| // Channel constants (mirrored from preload/constants/ipcChannels.ts to respect | ||
| // module boundaries — main process cannot import from preload). | ||
| const MEMORY_HAS_MEMORY = 'memory:hasMemory'; | ||
| const MEMORY_GET_INDEX = 'memory:getIndex'; | ||
| const MEMORY_READ_FILE = 'memory:readFile'; | ||
| const MEMORY_LIST_OPENERS = 'memory:listAvailableOpeners'; | ||
| const MEMORY_OPEN_IN = 'memory:openIn'; | ||
| const MEMORY_COPY_PATH = 'memory:copyPath'; | ||
|
|
||
| import { validateProjectId } from './guards'; | ||
|
|
| // Channel constants (mirrored from preload/constants/ipcChannels.ts to respect | ||
| // module boundaries — main process cannot import from preload). | ||
| const MEMORY_HAS_MEMORY = 'memory:hasMemory'; | ||
| const MEMORY_GET_INDEX = 'memory:getIndex'; | ||
| const MEMORY_READ_FILE = 'memory:readFile'; | ||
| const MEMORY_LIST_OPENERS = 'memory:listAvailableOpeners'; | ||
| const MEMORY_OPEN_IN = 'memory:openIn'; | ||
| const MEMORY_COPY_PATH = 'memory:copyPath'; |
…mponents
Summary by CodeRabbit
Bug Fixes
Documentation