feat(recall): activate keyword candidates for natural-language recall - #117
Conversation
Semantic recall now finds keyword candidates for ordinary questions. The FTS query builder space-joined every token, which SQLite FTS5 reads as an implicit AND, so a question only matched a memory containing every word. Measured on LoCoMo: 0 of 400 questions produced candidates, leaving the entire lexical layer dormant on natural-language recall. The semantic lane now drops filler words and matches any remaining term. The exact/lexical lane keeps strict AND and is untouched, because its BM25 hits are returned without semantic reranking to clean up over-broad matches; test_exact_retrieval_lane.py passes unmodified as proof. Benchmark (1,977 questions, 5,882 memories, top-5, keyword weight 0.0): any-hit 53.0% -> 57.8% all-hit 43.4% -> 47.8% evidence recall 47.6% -> 52.1% candidate coverage 0% -> 99.9% of queries Largest gain on adversarial (39.7% -> 49.8%); multi-hop regressed (39.3% -> 33.7%) because a keyword-filtered pool can drop a memory that shares no wording with the question. FTS_CANDIDATE_LIMIT is the lever and is now load-bearing: 99.4% of queries fill it to the default 50. HYBRID_SEARCH_TEXT_WEIGHT defaults to 0.0 instead of 0.35. Widening makes the keyword term live on natural-language recall for the first time, and 0.35 was chosen while it never applied there. At 0.0 keyword matching narrows which memories are considered but does not reorder them, so no unvalidated ranking signal ships. An explicit env var still applies. Also fixes: the protocol was never delivered on pip installs. marm_start, the protocol-injection middleware, and the STDIO lifecycle read PROTOCOL.md from a path outside the installed package, which is not in the wheel, so every pip install returned "PROTOCOL.md file not found". Docker and source checkouts were unaffected, hiding it. Root cause was two documentation copies with nothing enforcing which shipped; marm-mcp-server/marm-docs/ is removed and resources/marm-docs/ is the single in-package location, with the indexer and protocol readers sharing one resolver. Recall debug logging no longer records query text at any of its four sites, only shape (term counts, mode, candidate counts). New settings: FTS_QUERY_MODE (or_nostop|or|and), FTS_EXTRA_STOPWORDS. Both surface in `marm-memory doctor` under "Recall tuning". Tests: 864 passed, 10 skipped. Verified in a clean pip install of the built wheel that both protocol files now read back non-empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis release bumps MARM to v2.31.0, consolidates documentation under packaged resources, adds configurable wide FTS recall behavior, exposes retrieval settings through doctor diagnostics, and updates related tests, documentation, links, and release metadata. ChangesRelease metadata and public documentation
Packaged documentation consolidation
Configurable wide FTS recall
Recall diagnostics and doctor output
Estimated code review effort: 4 (Complex) | ~45 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 |
![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: cc1d4d3819
ℹ️ 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".
| return None | ||
|
|
||
| if FTS_QUERY_MODE == "or_nostop": | ||
| kept = [t for t in tokens if t.lower() not in _FTS_STOPWORDS] |
There was a problem hiding this comment.
Avoid lowercasing every token before stopword removal, because several listed stopwords are also common proper nouns or acronyms (May, Will, US, IT). For example, What happened in May? becomes an FTS query for only "happened"; if any unrelated memory contains that generic word, the nonempty candidate pool prevents the semantic fallback and excludes a relevant memory such as The launch is scheduled for May. Preserve case-significant tokens or remove ambiguous terms from the built-in stopword set so the new filter does not regress these queries.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/test_bundled_docs.py`:
- Around line 23-33: Restore byte-for-byte source-to-package assertions in
test_docs_live_inside_the_package for FAQ.md, PROTOCOL.md, and PROTOCOL-LITE.md,
while retaining the existing package-location and expected-file checks. Resync
the FAQ content at docs/FAQ.md:153 into
marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md; no other direct
change is required at the test site beyond restoring those comparisons.
🪄 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 Plus
Run ID: 811b2706-f9fe-4258-91e2-07f39a2e38d7
📒 Files selected for processing (40)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/config.yml.gitignoreAGENTS.mdCHANGELOG.mdCONTRIBUTING.mdREADME.mddocs/FAQ.mddocs/INSTALL-DOCKER.mddocs/INSTALL-LINUX.mddocs/INSTALL-PLATFORMS.mddocs/INSTALL-WINDOWS.mddocs/TECHNICAL-OVERVIEW.mdmarm-mcp-server/Dockerfilemarm-mcp-server/README.mdmarm-mcp-server/docker-compose.ymlmarm-mcp-server/marm-docs/FAQ.mdmarm-mcp-server/marm-docs/PROTOCOL-LITE.mdmarm-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/memory.pymarm-mcp-server/marm_mcp_server/core/memory_recall.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/README.mdmarm-mcp-server/marm_mcp_server/resources/skills/marm-init/SKILL.mdmarm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/services/cli_output.pymarm-mcp-server/marm_mcp_server/services/documentation.pymarm-mcp-server/marm_mcp_server/services/runtime_status.pymarm-mcp-server/marm_mcp_server/utils/helpers.pymarm-mcp-server/pyproject.tomlmarm-mcp-server/server.jsonmarm-mcp-server/tests/test_bundled_docs.pymarm-mcp-server/tests/test_hybrid_search.pyscripts/find-tools.pyscripts/find-versions.pyskills/marm-init/SKILL.md
💤 Files with no reviewable changes (4)
- marm-mcp-server/marm-docs/PROTOCOL.md
- marm-mcp-server/marm-docs/FAQ.md
- marm-mcp-server/marm-docs/PROTOCOL-LITE.md
- marm-mcp-server/marm-docs/README.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Keep changes surgical: modify only what the task requires, match existing style, and preserve behavior during refactors.
Files:
marm-mcp-server/docker-compose.ymlmarm-mcp-server/marm_mcp_server/server.pydocs/TECHNICAL-OVERVIEW.mddocs/INSTALL-PLATFORMS.mddocs/INSTALL-WINDOWS.mdmarm-mcp-server/pyproject.tomlCONTRIBUTING.mddocs/FAQ.mddocs/INSTALL-DOCKER.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.mdskills/marm-init/SKILL.mdmarm-mcp-server/marm_mcp_server/core/memory.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/services/cli_output.pymarm-mcp-server/marm_mcp_server/resources/skills/marm-init/SKILL.mdmarm-mcp-server/Dockerfiledocs/INSTALL-LINUX.mdmarm-mcp-server/marm_mcp_server/utils/helpers.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/server.jsonCHANGELOG.mdmarm-mcp-server/marm_mcp_server/core/memory_recall.pymarm-mcp-server/marm_mcp_server/services/runtime_status.pymarm-mcp-server/README.mdscripts/find-tools.pymarm-mcp-server/marm_mcp_server/resources/marm-docs/README.mdmarm-mcp-server/marm_mcp_server/services/documentation.pymarm-mcp-server/tests/test_hybrid_search.pyAGENTS.mdREADME.mdmarm-mcp-server/tests/test_bundled_docs.pyscripts/find-versions.pymarm-mcp-server/marm_mcp_server/config/settings.py
marm-mcp-server/{pyproject.toml,server.json,Dockerfile,docker-compose.yml,marm_mcp_server/__init__.py,marm_mcp_server/config/settings.py,marm_mcp_server/server.py,README.md,marm_mcp_server/resources/marm-docs/README.md}
📄 CodeRabbit inference engine (AGENTS.md)
When bumping the version, update every listed version source, identifier, label, docstring, README heading, and installation-document header; audit with
python scripts/find-versions.py. Use SemVer: MAJOR for breaking changes, MINOR for new tools, parameters, or features, and PATCH for fixes or documentation updates.
Files:
marm-mcp-server/docker-compose.ymlmarm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/pyproject.tomlmarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/Dockerfilemarm-mcp-server/server.jsonmarm-mcp-server/README.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/README.mdmarm-mcp-server/marm_mcp_server/config/settings.py
marm-mcp-server/marm_mcp_server/server.py
📄 CodeRabbit inference engine (AGENTS.md)
HTTP MCP tools must be registered in
MCP_TOOL_OPERATIONS; tools not in this whitelist do not exist over HTTP.
Files:
marm-mcp-server/marm_mcp_server/server.py
marm-mcp-server/marm_mcp_server/{server.py,server_stdio.py,services/stdio_graph_tools.py}
📄 CodeRabbit inference engine (AGENTS.md)
HTTP and STDIO transports must remain in exact behavioral parity; never fork tool behavior between transports.
Files:
marm-mcp-server/marm_mcp_server/server.py
marm-mcp-server/marm_mcp_server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
marm-mcp-server/marm_mcp_server/**/*.py: All memory writes must use the serialized asynchronous write queue; do not add bypass write paths.
marm_log_entrymust dual-write alog_entriesrow and semantic memory, while semantic-store failure must never fail the log write.
Graph and concept failures must never break the seven core memory tools.
marm_smart_recallmust keep primary memory ranking authoritative; graph enrichment is bounded, read-only, fail-open, and trimmed before primary results when enforcing response limits.
Use one lazily loaded, lock-serialized fastembed encoder with 512 dimensions; writes must succeed when the encoder is unavailable.
Never share database connections between the memory SQLite database and the isolated concept-graph database.
Prefer the smallest solution; avoid speculative abstractions and unnecessary configuration flags, and extract modules only at real boundaries.
Use minimal comments only for non-obvious rationale; never add comments that narrate the next line.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/marm_mcp_server/core/memory.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/services/cli_output.pymarm-mcp-server/marm_mcp_server/utils/helpers.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/memory_recall.pymarm-mcp-server/marm_mcp_server/services/runtime_status.pymarm-mcp-server/marm_mcp_server/services/documentation.pymarm-mcp-server/marm_mcp_server/config/settings.py
marm-mcp-server/{marm_mcp_server/endpoints/**/*.py,marm_mcp_server/server.py,marm_mcp_server/server_stdio.py,server.json,scripts/find-tools.py}
📄 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, andCANONICAL_TOOLS. Also update complete tool-list documentation and tests for both transports, then runpython scripts/find-tools.pyand require every surface to report OK.
Files:
marm-mcp-server/marm_mcp_server/server.pymarm-mcp-server/server.json
**/*.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/core/memory.pymarm-mcp-server/marm_mcp_server/__init__.pymarm-mcp-server/marm_mcp_server/services/cli_output.pymarm-mcp-server/marm_mcp_server/utils/helpers.pymarm-mcp-server/marm_mcp_server/core/memory_utils.pymarm-mcp-server/marm_mcp_server/core/memory_recall.pymarm-mcp-server/marm_mcp_server/services/runtime_status.pyscripts/find-tools.pymarm-mcp-server/marm_mcp_server/services/documentation.pymarm-mcp-server/tests/test_hybrid_search.pymarm-mcp-server/tests/test_bundled_docs.pyscripts/find-versions.pymarm-mcp-server/marm_mcp_server/config/settings.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/TECHNICAL-OVERVIEW.mddocs/INSTALL-PLATFORMS.mddocs/INSTALL-WINDOWS.mdCONTRIBUTING.mddocs/FAQ.mddocs/INSTALL-DOCKER.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.mdskills/marm-init/SKILL.mdmarm-mcp-server/marm_mcp_server/resources/skills/marm-init/SKILL.mddocs/INSTALL-LINUX.mdCHANGELOG.mdmarm-mcp-server/README.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/README.mdAGENTS.mdREADME.md
marm-mcp-server/marm_mcp_server/resources/marm-docs/{FAQ,PROTOCOL,PROTOCOL-LITE}.md
📄 CodeRabbit inference engine (AGENTS.md)
Packaged
FAQ.md,PROTOCOL.md, andPROTOCOL-LITE.mdmust remain exact copies of their corresponding files underdocs/; resync with the prescribedcpcommand after edits.
Files:
marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md
marm-mcp-server/marm_mcp_server/resources/marm-docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Only documentation under
resources/marm-docs/is packaged in the wheel and served or indexed; do not rely on copies outside that package directory.
Files:
marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.mdmarm-mcp-server/marm_mcp_server/resources/marm-docs/README.md
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 meaningfully speeds tests while retaining at least 95% behavioral 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, and usepytest.mark.skiponly for genuinely unavailable dependencies.
Files:
marm-mcp-server/tests/test_hybrid_search.pymarm-mcp-server/tests/test_bundled_docs.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_hybrid_search.pymarm-mcp-server/tests/test_bundled_docs.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Lyellr88/marm-memory
Timestamp: 2026-07-27T09:19:09.498Z
Learning: Never commit changes without an explicit user request.
🪛 SkillSpector (2.3.11)
skills/marm-init/SKILL.md
[info] 58: [RP1] null: pip install without ==version installs the latest release, which could include malicious changes.
Remediation: Pin the version: pip install package==1.2.3
(MCP Rug Pull (RP1))
[warning] 60: [RP1] null: Docker image references without a specific tag (:latest is implicit) or digest (@sha256:...) can be silently replaced by a malicious image.
Remediation: Pin the image: image:tag or image@sha256:abc123
(MCP Rug Pull (RP1))
[warning] 189: [RP1] null: Docker image references without a specific tag (:latest is implicit) or digest (@sha256:...) can be silently replaced by a malicious image.
Remediation: Pin the image: image:tag or image@sha256:abc123
(MCP Rug Pull (RP1))
[warning] 7: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
marm-mcp-server/marm_mcp_server/resources/skills/marm-init/SKILL.md
[info] 58: [RP1] null: pip install without ==version installs the latest release, which could include malicious changes.
Remediation: Pin the version: pip install package==1.2.3
(MCP Rug Pull (RP1))
[warning] 60: [RP1] null: Docker image references without a specific tag (:latest is implicit) or digest (@sha256:...) can be silently replaced by a malicious image.
Remediation: Pin the image: image:tag or image@sha256:abc123
(MCP Rug Pull (RP1))
[warning] 189: [RP1] null: Docker image references without a specific tag (:latest is implicit) or digest (@sha256:...) can be silently replaced by a malicious image.
Remediation: Pin the image: image:tag or image@sha256:abc123
(MCP Rug Pull (RP1))
[warning] 7: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🔇 Additional comments (36)
marm-mcp-server/marm_mcp_server/config/settings.py (1)
41-62: LGTM!Also applies to: 147-147, 253-261, 291-302
marm-mcp-server/marm_mcp_server/core/memory_utils.py (1)
14-15: LGTM!Also applies to: 95-152
marm-mcp-server/marm_mcp_server/core/memory.py (1)
51-51: LGTM!marm-mcp-server/marm_mcp_server/core/memory_recall.py (1)
12-20: LGTM!Also applies to: 56-56, 176-176, 210-210, 223-231, 352-352
marm-mcp-server/tests/test_hybrid_search.py (1)
1-1: LGTM!Also applies to: 54-159, 747-752, 815-892
marm-mcp-server/marm_mcp_server/services/runtime_status.py (1)
21-24: LGTM!Also applies to: 226-234
marm-mcp-server/marm_mcp_server/services/cli_output.py (2)
102-104: 🎯 Functional Correctness | ⚡ Quick win"Ignored words added" is misleading outside
or_nostopmode.
FTS_EXTRA_STOPWORDSonly affects filtering whenfts_query_modeisor_nostop(see_wide_fts_queryinmemory_utils.py); inand/ormodes these words are never dropped. The doctor output prints the "Ignored words added" line whenever the set is non-empty, irrespective of mode, misrepresenting actual behavior during debugging — the exact scenario this diagnostics section exists to prevent.🩹 Proposed fix
extra = retrieval.get("fts_extra_stopwords") or [] - if extra: + if extra and retrieval.get("fts_query_mode") == "or_nostop": print(f" Ignored words added: {', '.join(extra)}") + elif extra: + print(f" Extra stopwords configured (no effect in {retrieval.get('fts_query_mode')!r} mode): {', '.join(extra)}")
91-101: LGTM!marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md (1)
153-153: LGTM!docs/INSTALL-WINDOWS.md (1)
5-5: LGTM!Also applies to: 297-297
.github/ISSUE_TEMPLATE/config.yml (1)
10-10: LGTM!CONTRIBUTING.md (1)
323-323: LGTM!.gitignore (1)
135-135: LGTM!AGENTS.md (1)
28-28: LGTM!Also applies to: 37-42, 52-52
marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md (1)
1-1: LGTM!Also applies to: 950-952, 1091-1091
marm-mcp-server/marm_mcp_server/utils/helpers.py (1)
5-17: LGTM!Also applies to: 19-22, 35-35
marm-mcp-server/marm_mcp_server/services/documentation.py (1)
12-12: LGTM!Also applies to: 48-56, 78-81
marm-mcp-server/marm_mcp_server/resources/skills/marm-init/SKILL.md (1)
77-77: LGTM!CHANGELOG.md (1)
5-29: LGTM!README.md (1)
8-8: LGTM!Also applies to: 984-986, 1125-1125, 1171-1171
docs/INSTALL-DOCKER.md (1)
5-5: LGTM!docs/INSTALL-LINUX.md (1)
5-5: LGTM!Also applies to: 323-323
docs/INSTALL-PLATFORMS.md (1)
1-1: LGTM!skills/marm-init/SKILL.md (1)
77-77: 🩺 Stability & AvailabilityVerify this fallback works outside the repository root.
The new value is a repository-relative path, not a package-resource lookup. If the skill runs from an installed/distributed location or a different working directory, offline protocol loading will still fail. Confirm the skill is always executed from the repository root or switch this fallback to package-aware resolution.
Source: Coding guidelines
scripts/find-tools.py (1)
46-55: LGTM!scripts/find-versions.py (1)
51-51: LGTM!Also applies to: 113-117
marm-mcp-server/tests/test_bundled_docs.py (1)
23-55: 🩺 Stability & AvailabilityVerify this regression runs against a built wheel.
These tests validate whichever
marm_mcp_serverthe test process imports; a source checkout can pass even if package-data configuration omits the markdown files. Ensure CI installs a clean wheel before running this test, as required for the pip-install regression.docs/TECHNICAL-OVERVIEW.md (1)
3-3: LGTM!marm-mcp-server/Dockerfile (1)
76-76: LGTM!marm-mcp-server/README.md (1)
10-10: LGTM!Also applies to: 984-986, 1125-1125, 1171-1171
marm-mcp-server/docker-compose.yml (1)
8-8: LGTM!Also applies to: 21-21
marm-mcp-server/marm_mcp_server/__init__.py (1)
17-20: LGTM!marm-mcp-server/pyproject.toml (1)
7-7: LGTM!marm-mcp-server/server.json (1)
6-6: LGTM!Also applies to: 20-25
marm-mcp-server/marm_mcp_server/server.py (1)
8-8: LGTM!.github/ISSUE_TEMPLATE/bug_report.yml (1)
9-9: LGTM!
| def test_docs_live_inside_the_package(): | ||
| """The resolved docs dir must be under the installed package, not beside it. | ||
|
|
||
| Anything outside `marm_mcp_server/` is not packaged and breaks on pip. | ||
| """ | ||
| package_root = Path(helpers.__file__).resolve().parent.parent | ||
| resolved = helpers.docs_dir() | ||
|
|
||
| SOURCE_DOCS = Path(__file__).resolve().parents[1] / "marm-docs" | ||
| PACKAGED_DOCS = ( | ||
| Path(documentation.__file__).resolve().parent.parent / "resources" / "marm-docs" | ||
| ) | ||
| assert resolved is not None, "packaged marm-docs directory is missing" | ||
| assert package_root in resolved.parents | ||
| assert {p.name for p in resolved.glob("*.md")} == EXPECTED_DOCS |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the source/package documentation contract.
The test no longer detects stale packaged documentation, while this FAQ source edit must be mirrored into the packaged copy used at runtime.
marm-mcp-server/tests/test_bundled_docs.py#L23-L33: restore byte-for-byte checks forFAQ.md,PROTOCOL.md, andPROTOCOL-LITE.md.docs/FAQ.md#L153-L153: resyncmarm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md.
📍 Affects 2 files
marm-mcp-server/tests/test_bundled_docs.py#L23-L33(this comment)docs/FAQ.md#L153-L153
🤖 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/tests/test_bundled_docs.py` around lines 23 - 33, Restore
byte-for-byte source-to-package assertions in test_docs_live_inside_the_package
for FAQ.md, PROTOCOL.md, and PROTOCOL-LITE.md, while retaining the existing
package-location and expected-file checks. Resync the FAQ content at
docs/FAQ.md:153 into marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md;
no other direct change is required at the test site beyond restoring those
comparisons.
Source: Coding guidelines
The stopword list dropped words that also carry a content sense, and the
FTS5 tokenizer is case-insensitive ("porter ascii"), so a listed word loses
that meaning for every query -- there is no case-preserving check that would
help. "What happened in May?" searched only for "happened", and because any
non-empty candidate pool suppresses the full semantic scan, a memory reading
"The launch is scheduled for May" became unreachable rather than lower-ranked.
Removes modals (may/will/can/could/would/should/might/must), content verbs
(get/got/give/want/need/know/think/tell/say/said), the acronym collision
"us", and contraction fragments (won/don/didn/doesn/isn/aren/wasn/weren).
"won" was the worst of these: included as the "won't" remnant, it discarded
the victory sense outright.
Omitting them costs little because BM25 already discounts frequent terms by
inverse document frequency, so a mid-frequency word ranks low on its own
instead of swamping the candidate pool. "it"/"its" stay listed despite the
"IT" collision: the pronoun is common enough that keeping it as a search
term would flood the 50-slot pool, which costs more than the acronym sense.
LoCoMo re-run on the same corpus, changing only this list:
any-hit 57.8% -> 57.9%
all-hit 47.8% -> 48.1%
evidence recall 52.1% -> 52.4%
candidate coverage unchanged at 99.9%
Deltas are within single-run noise; the corpus has few proper-noun queries,
so it confirms no cost rather than demonstrating the correctness gain.
Reported by Codex on PR #117.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Semantic recall now finds keyword candidates for ordinary questions. The FTS query builder space-joined every token, which SQLite FTS5 reads as an implicit AND, so a question only matched a memory containing every word. Measured on LoCoMo: 0 of 400 questions produced candidates, leaving the entire lexical layer dormant on natural-language recall.
The semantic lane now drops filler words and matches any remaining term. The exact/lexical lane keeps strict AND and is untouched, because its BM25 hits are returned without semantic reranking to clean up over-broad matches; test_exact_retrieval_lane.py passes unmodified as proof.
Benchmark (1,977 questions, 5,882 memories, top-5, keyword weight 0.0):
any-hit 53.0% -> 57.8%
all-hit 43.4% -> 47.8%
evidence recall 47.6% -> 52.1%
candidate coverage 0% -> 99.9% of queries
Largest gain on adversarial (39.7% -> 49.8%); multi-hop regressed
(39.3% -> 33.7%) because a keyword-filtered pool can drop a memory that
shares no wording with the question. FTS_CANDIDATE_LIMIT is the lever and
is now load-bearing: 99.4% of queries fill it to the default 50.
HYBRID_SEARCH_TEXT_WEIGHT defaults to 0.0 instead of 0.35. Widening makes the keyword term live on natural-language recall for the first time, and 0.35 was chosen while it never applied there. At 0.0 keyword matching narrows which memories are considered but does not reorder them, so no unvalidated ranking signal ships. An explicit env var still applies.
Also fixes: the protocol was never delivered on pip installs. marm_start, the protocol-injection middleware, and the STDIO lifecycle read PROTOCOL.md from a path outside the installed package, which is not in the wheel, so every pip install returned "PROTOCOL.md file not found". Docker and source checkouts were unaffected, hiding it. Root cause was two documentation copies with nothing enforcing which shipped; marm-mcp-server/marm-docs/ is removed and resources/marm-docs/ is the single in-package location, with the indexer and protocol readers sharing one resolver.
Recall debug logging no longer records query text at any of its four sites, only shape (term counts, mode, candidate counts).
New settings: FTS_QUERY_MODE (or_nostop|or|and), FTS_EXTRA_STOPWORDS. Both surface in
marm-memory doctorunder "Recall tuning".Tests: 864 passed, 10 skipped. Verified in a clean pip install of the built wheel that both protocol files now read back non-empty.
Summary by CodeRabbit