docs: clarify dual-memory architecture (fixes #344)#367
docs: clarify dual-memory architecture (fixes #344)#367jlin53882 wants to merge 2 commits intoCortexReach:masterfrom
Conversation
Add three improvements addressing Issue CortexReach#344: 1. README: add Dual-Memory Architecture section explaining Plugin Memory (LanceDB) vs Markdown Memory distinction 2. index.ts: log dual-memory warning on plugin startup 3. cli.ts: add import-markdown command to migrate existing Markdown memories into the plugin store for semantic recall Refs: CortexReach#344
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcdb9b8cf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
index.ts
Outdated
| if (_initialized) { | ||
| api.logger.debug("memory-lancedb-pro: register() called again — skipping re-init (idempotent)"); | ||
| return; | ||
| } | ||
| _initialized = true; |
There was a problem hiding this comment.
Setting _initialized = true before parsing config and creating runtime components makes registration non-recoverable: any thrown error on first startup leaves the module permanently “initialized,” so later register() calls silently no-op. In environments that retry plugin startup (or reload after fixing config), the plugin stays disabled until process restart.
Useful? React with 👍 / 👎.
index.ts
Outdated
| // recallMode: "full" (default when autoRecall=true) | "summary" (L0 only) | "adaptive" (intent-based) | "off" | ||
| const recallMode = config.recallMode || "full"; | ||
| if (config.autoRecall === true && recallMode !== "off") { | ||
| if (config.autoRecall === true) { |
There was a problem hiding this comment.
This condition now enables auto-recall whenever autoRecall === true, but the previously supported recallMode: "off" path is no longer checked. Existing configs that keep autoRecall on but set recallMode to disable injection will now inject memories unexpectedly, changing prompt contents and behavior for deployed users.
Useful? React with 👍 / 👎.
| model: config.embedding.model || "text-embedding-3-small", | ||
| baseURL: config.embedding.baseURL, | ||
| dimensions: config.embedding.dimensions, | ||
| omitDimensions: config.embedding.omitDimensions, | ||
| taskQuery: config.embedding.taskQuery, |
There was a problem hiding this comment.
The embedder is constructed without omitDimensions, so users who configured embedding.omitDimensions=true lose that behavior and will start sending dimensions again. For OpenAI-compatible backends that reject the dimensions field, embedding calls fail, which breaks memory writes and retrieval flows relying on embeddings.
Useful? React with 👍 / 👎.
1. Defer _initialized flag until after all setup succeeds (P1) - Previously set to true before init, making any init error permanent - Now set only at the end of register() after all components initialize - This makes the plugin recoverable if init fails partway through 2. Restore omitDimensions forwarding to embedder config (P1) - Was removed during refactor, breaking embedding.omitDimensions=true configs - Without it, OpenAI-compatible backends that reject dimensions field fail 3. Restore recallMode check in auto-recall gate (P1) - autoRecall=true now checks recallMode !== 'off' before enabling - Prevents unexpected memory injection for users who set recallMode=off
Fixes Applied for Your 3 P1 Review IssuesApplied all 3 fixes directly to this PR: P1 #1: Defer
|
|
To use Codex here, create an environment for this repo. |
1 participant
Summary
Addresses Issue #344: Dual-memory model causes confusing UX — users expect Markdown memories to be recallable, but they aren't.
Adds three improvements:
1. README: Dual-Memory Architecture section
Explains the distinction between Plugin Memory (LanceDB) and Markdown Memory, with a clear table and usage guidance.
2. index.ts: Startup warning log
Logs a human-readable warning on every plugin initialization.
3. cli.ts: import-markdown command
px memory-lancedb-pro memory-pro import-markdown\ — migrate Markdown memories into the plugin store for semantic recall.
Test results
Gateway log after restart:
\
01:23:46 [memory-lancedb-pro] memory_recall queries the plugin store (LanceDB), not MEMORY.md...
01:23:49 memory-lancedb-pro: initialized successfully (embedding: OK, retrieval: OK, mode: hybrid, FTS: enabled)
\\
Closes #344