feat(notebook): per-session scratch pad, permanent docs store, chunking rework - #105
Conversation
…ng rework - notebook_entries gains session_name as part of its identity; legacy rows migrate to session_name='main' so nothing already saved becomes unreachable. add/use/show and marm_delete(type="notebook") now scope by session end to end. - New marm_notebook(action="save") promotes a scratch entry (or new inline content) into a separate marm_docs.db, mirrored into memories via a new queue-backed, non-consolidating store_doc_mirror path so marm_smart_recall and marm_concept_build can reach it. Mirror sync is best-effort (mirror_status="pending" on failure, never rolls back the durable save); the mirror always excludes the reserved marm_system session. - Replaced the fixed-window memory chunker with an even-split algorithm sized for the current embedding model's larger context window: separate memory (500/250/50) and doc (1000/800/100) profiles. Fixes the old algorithm's tiny-trailing-fragment bug. --migrate-embeddings is unchanged (re-embeds existing text only, never re-derives chunk boundaries). - Scratch entries stop writing embeddings; embedding inspection/migration no longer touch the retired notebook_entries.embedding column. - Console (server + frontend) threads session_name through notebook list/create/update/delete so same-named entries from different sessions are no longer conflated. - Independently reviewed; fixed the two findings (docs_db.save_doc now runs under BEGIN IMMEDIATE; added an end-to-end concept-build test over a promoted doc's mirror). - Version bump to 2.25.0 (MINOR). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s8tBNnQTmb1q5nRAKwhvv
- server.json: notebook tool description still listed the old 5 actions, and version strings across the file were never bumped to 2.25.0 - README.md (all 3 copies): version banner still said v2.24.0; embedding migration text still claimed notebook embeddings get re-embedded, which is now false since that column is retired - services/log_entry.py: unscoped marm_delete(type="notebook") could wipe every project/platform-scoped entry sharing a name within a session -- the new 4-part identity made that collision newly possible. Now refuses and asks the caller to disambiguate when more than one match exists; a single match still deletes cleanly as before - memory_chunks gains a (memory_id, chunk_index) unique index, and _write_chunks now uses INSERT OR REPLACE: two resaves of identical doc content share the same content_hash, so the existing staleness guard couldn't tell them apart, and rapid identical resaves could each insert a full duplicate set of chunk rows - Fixed a Windows-only test bug (test_docs_db.py used the POSIX-only HOME env var instead of patching Path.home() directly) and a Console test fixture that predated the session_name schema change (hand-rolled notebook_entries table missing the column, stale call assertions) All fixes independently validated against the actual code before being applied. Full backend suite (664 passed, 28 skipped) and full Console suite (27 passed) green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s8tBNnQTmb1q5nRAKwhvv
…up index The idx_memory_chunks_dedup unique index added in the prior fix commit would fail with UNIQUE constraint failed on any database that already had duplicate (memory_id, chunk_index) rows from the pre-fix identical- content chunk-write race -- an upgraded server could never start. init_database() now deletes duplicate rows (keeping the highest id, i.e. the most recent write) before creating the index, guarded behind the index not already existing so it only runs once, not on every startup. Verified via mutation test: reverted the guard, confirmed the test fails with the exact IntegrityError this fix prevents, then restored it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s8tBNnQTmb1q5nRAKwhvv
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)marm-mcp-server/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
marm-mcp-server/{marm_mcp_server/endpoints/**/*.py,server.py,server_stdio.py,server.json,scripts/find-tools.py,tests/**/*.py,README.md,docs/**/*.md,marm_mcp_server/marm-docs/**/*.md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
marm-mcp-server/tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/tests/**⚙️ CodeRabbit configuration file
Files:
**/*.py⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughMARM v2.25.0 adds session-scoped, embedding-free notebook scratch entries, permanent document promotion with queued memory mirrors, revised chunking and chunk deduplication, updated embedding migration scope, and synchronized Console, protocol, tests, and release metadata. ChangesNotebook and document lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
marm-mcp-server/marm_mcp_server/core/memory_db.py (1)
363-379: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMissing dedicated test for the pre-existing-duplicate collapse migration.
The one-time
DELETE ... GROUP BY memory_id, chunk_indexcollapse (guarded byidx_memory_chunks_dedupabsence) is exactly the "collapse pre-existing duplicate chunks before creating the deduplication index" migration called out in the commit summary, but none of the provided tests seed amemory_chunkstable with real duplicate(memory_id, chunk_index)rows and then callinit_databaseto verify the collapse (keep-max-id) runs correctly and the subsequentCREATE UNIQUE INDEXsucceeds.test_chunking.py's idempotency test only exercises_write_chunks'INSERT OR REPLACEagainst a DB where the index already exists.Given this runs against production data on every upgrade, a dedicated test seeding duplicates pre-migration would meaningfully de-risk it.
As per path instructions for
marm-mcp-server/tests/**/*.py, "Focus on tests that are ... missing coverage for a changed high-risk path."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@marm-mcp-server/marm_mcp_server/core/memory_db.py` around lines 363 - 379, Add a dedicated migration test covering the pre-existing-duplicate path in init_database: seed memory_chunks with multiple rows sharing the same (memory_id, chunk_index) before initialization, call init_database, and assert only the row with the maximum id remains while idx_memory_chunks_dedup is created successfully. Keep the existing idempotency test unchanged and place the new coverage with the database initialization tests.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@marm-mcp-server/marm_mcp_server/core/memory_ops.py`:
- Around line 368-467: Update _store_doc_mirror so that when an
existing_memory_id is successfully replaced in place, it marks any active
compaction_staging rows referencing that memory as stale, matching
_replace_memory’s behavior. Apply this within the same transaction before
committing, while leaving fresh mirror insertion unchanged.
In `@marm-mcp-server/README.md`:
- Line 10: Revert the hand-edited version in the package README and regenerate
both README mirrors from the root README using the repository’s generator
script, ensuring the generated content is synchronized rather than manually
modified.
---
Nitpick comments:
In `@marm-mcp-server/marm_mcp_server/core/memory_db.py`:
- Around line 363-379: Add a dedicated migration test covering the
pre-existing-duplicate path in init_database: seed memory_chunks with multiple
rows sharing the same (memory_id, chunk_index) before initialization, call
init_database, and assert only the row with the maximum id remains while
idx_memory_chunks_dedup is created successfully. Keep the existing idempotency
test unchanged and place the new coverage with the database initialization
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f49eee4-bb10-4186-a5ca-df7a2266569b
📒 Files selected for processing (43)
CHANGELOG.mdREADME.mddocs/PROTOCOL.mdmarm-console/artifacts/marm-console/src/components/memory/NotebookAndCompactionTabs.tsxmarm-console/artifacts/marm-console/src/hooks/use-marm-queries.tsmarm-console/artifacts/marm-console/src/lib/marm-api.tsmarm-console/artifacts/marm-console/src/lib/marm-types.tsmarm-console/server/endpoints/notebook.pymarm-console/server/memory_store.pymarm-console/server/models.pymarm-console/tests/test_memory_dashboard_gap_routes.pymarm-mcp-server/Dockerfilemarm-mcp-server/README.mdmarm-mcp-server/docker-compose.ymlmarm-mcp-server/marm-docs/PROTOCOL.mdmarm-mcp-server/marm-docs/README.mdmarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/config/settings.pymarm-mcp-server/marm_mcp_server/core/docs_db.pymarm-mcp-server/marm_mcp_server/core/memory.pymarm-mcp-server/marm_mcp_server/core/memory_db.pymarm-mcp-server/marm_mcp_server/core/memory_ops.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/models.pymarm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/server_stdio.pymarm-mcp-server/marm_mcp_server/services/log_entry.pymarm-mcp-server/marm_mcp_server/services/notebook.pymarm-mcp-server/marm_mcp_server/utils/embedding_migration.pymarm-mcp-server/marm_mcp_server/utils/embedding_state.pymarm-mcp-server/pyproject.tomlmarm-mcp-server/server.jsonmarm-mcp-server/tests/test_chunking.pymarm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_http_tools.pymarm-mcp-server/tests/test_notebook_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-mcp-server/tests/test_stdio_transport.py
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: Ruff CI / 0_ruff.txt: feat(notebook): per-session scratch pad, permanent docs store, chunking rework
Conclusion: failure
##[group]Run ruff format --check
�[36;1mruff format --check�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib
##[endgroup]
Would reformat: tests/test_chunking.py
Would reformat: tests/test_http_tools.py
Would reformat: tests/test_notebook_service.py
3 files would be reformatted, 130 files already formatted
##[error]Process completed with exit code 1.
GitHub Actions: Ruff CI / ruff: feat(notebook): per-session scratch pad, permanent docs store, chunking rework
Conclusion: failure
##[group]Run ruff format --check
�[36;1mruff format --check�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib
##[endgroup]
Would reformat: tests/test_chunking.py
Would reformat: tests/test_http_tools.py
Would reformat: tests/test_notebook_service.py
3 files would be reformatted, 130 files already formatted
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (11)
marm-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Keep changes surgical: touch only what the task requires, match existing style, and preserve behavior during refactors.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/utils/embedding_state.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/config/settings.pymarm-mcp-server/marm_mcp_server/utils/embedding_migration.pymarm-mcp-server/marm_mcp_server/core/models.pymarm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_notebook_migration.pymarm-mcp-server/tests/test_stdio_transport.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-mcp-server/marm_mcp_server/services/log_entry.pymarm-mcp-server/marm_mcp_server/core/docs_db.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/memory.pymarm-mcp-server/marm_mcp_server/server_stdio.pymarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_chunking.pymarm-mcp-server/marm_mcp_server/core/memory_ops.pymarm-mcp-server/marm_mcp_server/services/notebook.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/marm_mcp_server/core/memory_db.pymarm-mcp-server/tests/test_http_tools.py
marm-mcp-server/marm_mcp_server/{server.py,server_stdio.py,endpoints/**/*.py,services/stdio_graph_tools.py}
📄 CodeRabbit inference engine (AGENTS.md)
HTTP and STDIO transports must maintain exact parity; keep the HTTP whitelist and STDIO registrations synchronized and never fork behavior between transports.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/server_stdio.py
marm-mcp-server/marm_mcp_server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
marm-mcp-server/marm_mcp_server/**/*.py: All memory writes must go through the serialized asynchronous write queue; do not add bypass paths.
A semantic-store failure inmarm_log_entrymust never fail the log write.
Keep the memory database and concept-graph database isolated; never share connections between their pools.
Graph and concept failures must never break the seven core memory tools.
Use one lazy-loaded, lock-serialized fastembed encoder with 512 dimensions; writes must succeed when the encoder is unavailable.
Prefer the smallest change that solves the problem; avoid speculative abstractions and unrequested configuration flags.
Use minimal comments that explain only non-obvious reasons; never narrate the next line.
Keep orchestration in its current owner file and extract modules only at real architectural boundaries.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/utils/embedding_state.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/config/settings.pymarm-mcp-server/marm_mcp_server/utils/embedding_migration.pymarm-mcp-server/marm_mcp_server/core/models.pymarm-mcp-server/marm_mcp_server/services/log_entry.pymarm-mcp-server/marm_mcp_server/core/docs_db.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/memory.pymarm-mcp-server/marm_mcp_server/server_stdio.pymarm-mcp-server/marm_mcp_server/core/memory_ops.pymarm-mcp-server/marm_mcp_server/services/notebook.pymarm-mcp-server/marm_mcp_server/core/memory_db.py
{marm-mcp-server/pyproject.toml,marm-mcp-server/server.json,marm-mcp-server/marm_mcp_server/__init__.py,marm-mcp-server/marm_mcp_server/config/settings.py,marm-mcp-server/marm_mcp_server/server.py,marm-mcp-server/Dockerfile,docker-compose.yml,README.md,docs/INSTALL-*.md}
📄 CodeRabbit inference engine (AGENTS.md)
When bumping the version, update every enumerated version source and run
python scripts/find-versions.py; follow SemVer definitions for breaking changes, features, and fixes.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/pyproject.tomlmarm-mcp-server/Dockerfilemarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/config/settings.pymarm-mcp-server/server.jsonREADME.md
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Prioritize runtime correctness, async/concurrency safety, SQLite transaction safety, auth/rate-limit behavior, release-breaking packaging issues, and MCP protocol compatibility.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/utils/embedding_state.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/config/settings.pymarm-mcp-server/marm_mcp_server/utils/embedding_migration.pymarm-console/server/memory_store.pymarm-mcp-server/marm_mcp_server/core/models.pymarm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_notebook_migration.pymarm-console/server/models.pymarm-mcp-server/tests/test_stdio_transport.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-mcp-server/marm_mcp_server/services/log_entry.pymarm-mcp-server/marm_mcp_server/core/docs_db.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/memory.pymarm-console/server/endpoints/notebook.pymarm-mcp-server/marm_mcp_server/server_stdio.pymarm-console/tests/test_memory_dashboard_gap_routes.pymarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_chunking.pymarm-mcp-server/marm_mcp_server/core/memory_ops.pymarm-mcp-server/marm_mcp_server/services/notebook.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/marm_mcp_server/core/memory_db.pymarm-mcp-server/tests/test_http_tools.py
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Only flag documentation issues that are materially wrong, misleading for installation/release behavior, or inconsistent with live MCP behavior. Skip style, phrasing, formatting, and wording preferences.
Files:
docs/PROTOCOL.mdmarm-mcp-server/marm-docs/PROTOCOL.mdmarm-mcp-server/README.mdREADME.mdCHANGELOG.mdmarm-mcp-server/marm-docs/README.md
marm-mcp-server/marm_mcp_server/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Keep endpoint logic in
endpoints/, split by surface, and keep shared helpers incore/.
Files:
marm-mcp-server/marm_mcp_server/endpoints/notebook.py
marm-mcp-server/{marm_mcp_server/endpoints/**/*.py,server.py,server_stdio.py,server.json,scripts/find-tools.py,tests/**/*.py,README.md,docs/**/*.md,marm_mcp_server/marm-docs/**/*.md}
📄 CodeRabbit inference engine (AGENTS.md)
When adding or removing an MCP tool, update the endpoint implementation, HTTP route and whitelist, STDIO registration or wrapper,
server.json, canonical tool list, all full tool-list documentation and counts, and tests for both transports.
Files:
marm-mcp-server/marm_mcp_server/endpoints/notebook.pymarm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_notebook_migration.pymarm-mcp-server/README.mdmarm-mcp-server/tests/test_stdio_transport.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-mcp-server/server.jsonmarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_chunking.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/tests/test_http_tools.py
marm-mcp-server/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
marm-mcp-server/tests/**/*.py: Run tests withpytestfrommarm-mcp-server/; use real FastAPI endpoints and SQLite, mocking only when it preserves at least 95% fidelity.
Every new MARM Console API route requires at least one happy-path FastAPI response-contract test with the MCP adapter stubbed.
Do not write existence-check or coded-to-pass tests; prefer deep tests exercising real paths over broad shallow coverage.
Usepytest.mark.skiponly for genuinely unavailable dependencies, never merely to avoid implementation effort.
Files:
marm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_notebook_migration.pymarm-mcp-server/tests/test_stdio_transport.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_chunking.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/tests/test_http_tools.py
**/tests/**
⚙️ CodeRabbit configuration file
**/tests/**: Focus on tests that are flaky, non-isolated, incorrectly asserting behavior, or missing coverage for a changed high-risk path. Skip minor naming, comments, and layout preferences.
Files:
marm-mcp-server/tests/test_concept_endpoints.pymarm-mcp-server/tests/test_notebook_migration.pymarm-mcp-server/tests/test_stdio_transport.pymarm-mcp-server/tests/test_sqlite_write_atomicity.pymarm-console/tests/test_memory_dashboard_gap_routes.pymarm-mcp-server/tests/test_docs_db.pymarm-mcp-server/tests/test_embedding_state.pymarm-mcp-server/tests/test_chunking.pymarm-mcp-server/tests/test_embedding_migration.pymarm-mcp-server/tests/test_notebook_service.pymarm-mcp-server/tests/test_http_tools.py
{README.md,marm-mcp-server/README.md,marm-mcp-server/marm-docs/README.md}
📄 CodeRabbit inference engine (AGENTS.md)
Root
README.mdis the source of truth; the two package README mirrors are generated and must never be hand-edited.
Files:
marm-mcp-server/README.mdREADME.mdmarm-mcp-server/marm-docs/README.md
🪛 ast-grep (0.44.1)
marm-mcp-server/marm_mcp_server/core/memory_ops.py
[info] 421-421: use jsonify instead of json.dumps for JSON output
Context: json.dumps(metadata)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 448-448: use jsonify instead of json.dumps for JSON output
Context: json.dumps(metadata)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 GitHub Actions: Ruff CI / 0_ruff.txt
marm-mcp-server/tests/test_chunking.py
[error] 1-1: Ruff format --check would reformat the file. Update formatting (e.g., run 'ruff format tests/test_chunking.py').
marm-mcp-server/tests/test_notebook_service.py
[error] 1-1: Ruff format --check would reformat the file. Update formatting (e.g., run 'ruff format tests/test_notebook_service.py').
marm-mcp-server/tests/test_http_tools.py
[error] 1-1: Ruff format --check would reformat the file. Update formatting (e.g., run 'ruff format tests/test_http_tools.py').
🪛 GitHub Actions: Ruff CI / ruff
marm-mcp-server/tests/test_chunking.py
[error] 1-1: ruff format --check would reformat this file. Run 'ruff format' (or 'ruff format --write') to apply formatting.
marm-mcp-server/tests/test_notebook_service.py
[error] 1-1: ruff format --check would reformat this file. Run 'ruff format' (or 'ruff format --write') to apply formatting.
marm-mcp-server/tests/test_http_tools.py
[error] 1-1: ruff format --check would reformat this file. Run 'ruff format' (or 'ruff format --write') to apply formatting.
🪛 OpenGrep (1.25.0)
marm-mcp-server/marm_mcp_server/core/memory_db.py
[ERROR] 237-243: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (45)
CHANGELOG.md (2)
5-36: LGTM!
38-45: LGTM!README.md (3)
8-8: LGTM!
110-110: LGTM!
582-582: LGTM!marm-mcp-server/Dockerfile (1)
68-68: LGTM!marm-mcp-server/docker-compose.yml (1)
21-21: LGTM!marm-mcp-server/marm_mcp_server/__init__.py (1)
17-20: LGTM!marm-mcp-server/marm_mcp_server/config/settings.py (1)
125-125: LGTM!marm-mcp-server/marm_mcp_server/server.py (1)
8-8: LGTM!marm-mcp-server/pyproject.toml (1)
7-7: LGTM!marm-mcp-server/server.json (1)
6-6: LGTM!Also applies to: 20-25, 44-44
marm-mcp-server/marm_mcp_server/server_stdio.py (1)
30-48: LGTM!Also applies to: 182-189, 297-300
marm-mcp-server/marm_mcp_server/core/models.py (1)
21-33: LGTM!Also applies to: 100-104
marm-mcp-server/marm_mcp_server/services/notebook.py (1)
3-30: LGTM!Also applies to: 40-86, 89-152, 176-280, 283-309
marm-console/server/endpoints/notebook.py (1)
23-59: LGTM!Also applies to: 63-78
marm-console/server/memory_store.py (1)
378-395: LGTM!marm-mcp-server/tests/test_notebook_service.py (1)
1-22: LGTM!Also applies to: 133-153, 157-172, 176-187, 204-432
marm-mcp-server/tests/test_stdio_transport.py (1)
422-447: LGTM!marm-mcp-server/marm_mcp_server/endpoints/notebook.py (1)
12-35: LGTM!docs/PROTOCOL.md (1)
34-34: LGTM!marm-mcp-server/tests/test_concept_endpoints.py (1)
71-110: LGTM!marm-mcp-server/marm-docs/README.md (1)
1-1: 📐 Maintainability & Code QualityGenerated mirror stays aligned with the root README; no change needed.
marm-mcp-server/marm_mcp_server/core/memory_db.py (1)
96-393: LGTM on the remaining schema changes (session_name migration/backfill, unique index rebuild, FTS setup). The flagged f-string SQL-injection static-analysis hint at 237-243 is a false positive —project_expr/platform_expr/session_exprare drawn only from a fixed set of hardcoded literals, never from column values or user input.marm-console/server/models.py (1)
80-92: LGTM!marm-mcp-server/tests/test_http_tools.py (1)
161-267: LGTM! Fixtures and new tests correctly reflect the session-scoped notebook identity and no-embedding-on-add contracts; the rest are non-behavioral assertion-formatting refactors.Also applies to: 307-334, 374-410, 433-506, 576-703, 835-940, 1101-1374, 1401-1444
marm-mcp-server/tests/test_notebook_migration.py (1)
1-114: LGTM!marm-mcp-server/tests/test_sqlite_write_atomicity.py (1)
397-481: LGTM!marm-mcp-server/marm_mcp_server/core/docs_db.py (1)
1-166: LGTM!marm-mcp-server/marm_mcp_server/core/memory_ops.py (1)
12-33: LGTM on the rest — chunk-profile parameterization is correctly applied per call site, and_store_doc_mirror's chunk write/embedding-fallback path correctly reuses the tested_write_chunksidempotency guard. (The ast-grep "use jsonify" hints at 421/448 are false positives — thesejson.dumps(metadata)calls are for a SQLite TEXT column, not an HTTP response.)Also applies to: 36-140, 143-288, 469-481
marm-mcp-server/tests/test_docs_db.py (1)
1-223: LGTM!marm-mcp-server/marm_mcp_server/utils/embedding_state.py (1)
20-27: LGTM!marm-mcp-server/tests/test_embedding_migration.py (1)
38-266: LGTM!marm-mcp-server/tests/test_embedding_state.py (1)
40-247: LGTM!marm-mcp-server/marm_mcp_server/services/log_entry.py (1)
404-459: LGTM!marm-mcp-server/marm_mcp_server/core/memory.py (1)
7-57: LGTM!Also applies to: 178-190, 339-365, 448-452
marm-console/artifacts/marm-console/src/lib/marm-types.ts (1)
148-164: LGTM!marm-console/artifacts/marm-console/src/hooks/use-marm-queries.ts (1)
234-236: LGTM!Also applies to: 248-255
marm-console/artifacts/marm-console/src/lib/marm-api.ts (1)
177-189: LGTM!marm-console/artifacts/marm-console/src/components/memory/NotebookAndCompactionTabs.tsx (1)
16-27: LGTM!Also applies to: 59-65, 86-92, 109-110, 127-134
marm-console/tests/test_memory_dashboard_gap_routes.py (1)
54-63: LGTM!Also applies to: 250-272, 359-434
marm-mcp-server/marm-docs/PROTOCOL.md (1)
34-34: LGTM!marm-mcp-server/marm_mcp_server/core/memory_utils.py (1)
93-151: LGTM!Also applies to: 181-182
marm-mcp-server/tests/test_chunking.py (1)
12-131: LGTM!Also applies to: 314-314, 439-549
marm-mcp-server/marm_mcp_server/utils/embedding_migration.py (1)
18-23: LGTM!
| height="250"> | ||
| </picture> | ||
| <h1 align="center">MARM: Local-First Persistent Multi-Agent Memory Layer for MCP Clients v2.24.0</h1> | ||
| <h1 align="center">MARM: Local-First Persistent Multi-Agent Memory Layer for MCP Clients v2.25.0</h1> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Revert hand-edited generated file.
As per coding guidelines, the package README mirrors (marm-mcp-server/README.md and marm-mcp-server/marm-docs/README.md) are generated from the root README.md and must never be hand-edited. Please revert the manual changes to this file and use the generator script to sync it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@marm-mcp-server/README.md` at line 10, Revert the hand-edited version in the
package README and regenerate both README mirrors from the root README using the
repository’s generator script, ensuring the generated content is synchronized
rather than manually modified.
Source: Coding guidelines
… README provenance Addresses PR #105 CodeRabbit review: - _store_doc_mirror's in-place replace branch now marks active compaction_staging rows referencing the overwritten memory id as stale, matching _replace_memory's existing handling of the same case. Without this, a doc resave could leave a staged summary pointing at content that no longer exists. - Reverted the hand-edited package README mirrors and regenerated them via scripts/make-readme-mirrors.py instead (output was byte-identical -- this fixes provenance, not content). - Ruff-formatted 3 test files that were failing the Ruff CI format check (black vs ruff multi-line assert wrapping disagreement, no behavior change). The reported "missing dedicated test" nitpick for memory_db.py's duplicate-chunk collapse migration was already covered by test_init_database_collapses_preexisting_duplicate_chunks_before_indexing (added in the prior commit) -- stale finding, no action needed.
2 participants
Summary by CodeRabbit
New Features
session_name).marm_notebook(action="save")to promote scratch content into permanent, graph-linked documents, mirrored into memory with best-effort sync (reporting pending when delayed).Bug Fixes
Documentation
saveaction, and embedding migration behavior.