diff --git a/CHANGELOG.md b/CHANGELOG.md index ff9cca16..589a4718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,39 @@ # Changelog -## Version 2 - MARM Protocol to Universal MCP Server Evolution +
+August 2nd, 2026: Automatic Concept Graph Indexing (v2.36.0) + +### Added: Memories Become Graph Nodes on Their Own + +- The concept graph only grew when someone clicked Build Concept Graph, so it was stale until a human remembered to refresh it. Storing a memory now queues it for indexing, and a background worker turns it into a node about 30 seconds later on both transports. Nothing to click. +- The queue is a durable table in the memory database, written in the same transaction as the memory itself, so a memory cannot exist without its indexing task. A server killed mid-extraction loses nothing: the task is still there on the next start and shutdown never waits for extraction to finish. +- Extraction failures retry with a growing delay and record the reason. A memory that fails three times is parked with its error rather than blocking the queue behind it. A failure never affects the memory itself, which stores and recalls normally throughout. +- Turn it off with `CONCEPT_AUTO_INDEX=false` (or `0`, `no`, `off`). That stops the worker, not the queue: writes keep recording indexing tasks, so re-enabling it indexes everything written while it was off. Pacing is `CONCEPT_INDEX_DEBOUNCE_SECONDS` (30), `CONCEPT_INDEX_BATCH_SIZE` (20, capped at 500), `CONCEPT_INDEX_BATCH_PAUSE_MS` (250), `CONCEPT_INDEX_LEASE_SECONDS` (300), and `CONCEPT_INDEX_MAX_ATTEMPTS` (3). +- Clearing a backlog is not free, and the numbers are published rather than estimated. On a real 768-memory corpus, recall during a drain moves from ~8ms to ~16ms median (p95 ~12ms to ~31ms) while writes are unaffected; `scripts/benchmarking/performance/bench_concept_worker.py` reproduces it. Entity extraction is CPU-bound, so this is contention for cores, not lock waiting, and tuning the encoder would not help. The pause between batches exists for the tail: it cuts worst-case recall during indexing from roughly 270ms to 80ms in exchange for about 18% longer drains. Steady-state indexing of a few new memories is idle almost all the time and none of this applies. +- Running HTTP and STDIO at once is safe. Both take a leased lock in the memory database before touching the graph, so a rebuild in one cannot drop tables while the other is writing to them. A build that finds the graph busy reports `build_in_progress` and can be run again rather than colliding. +- The Console's Knowledge Explorer picks new nodes up while it is open. It polls a small change marker rather than the graph, so an idle Explorer costs a few counts per check, and it stops entirely when the tab is not showing. + +### Fixed: Builds Silently Ignored Everything Past the Newest 500 Memories + +- Every build ended with a hard limit of 500 rows. On a corpus larger than that, the older memories were not slow to reach, they were unreachable: no scope, no setting, and no number of rebuilds would ever index them. +- Builds now page through the whole scope. `CONCEPT_BUILD_ROW_CAP` still exists and still defaults to 500, but it is a page size now, not a ceiling. Anyone who lowered it to bound build cost gets more, smaller pages instead of a truncated graph, and a full build on a large corpus is genuinely long-running as a result. + +### Changed: Compacted Sessions Index Their Sources, Not Their Summary + +- When a session compacts, its original memories are kept as sources and a summary is written alongside them. Builds used to skip the sources and index the summary, which is backwards for a graph: the summary restates concepts the sources already stated, so every entity in a compacted session was attributed to a paraphrase rather than to where it was actually said. +- Sources are now indexed and generated summaries are not. This is also what lets a memory reach the graph the moment it is written instead of waiting for its session to compact. + +### Upgrade Note + +This release requires one graph rebuild. Existing graphs contain entities extracted from compaction summaries that the new rule would never produce, and there is no way to remove only those. MARM detects the old graph and reports `rebuild_required` until you run: + +``` +marm_concept_build(search_all=True) +``` + +The old graph is backed up next to the database first. The build clears the queue it just covered, so the background worker does not immediately re-extract the same corpus. + +
July 31st, 2026: Chunk Durability and Repair (v2.35.0) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index c3fd8005..b7b88586 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -12,7 +12,7 @@ Thank you to everyone helping push local-first, persistent AI memory forward. - **sarvesh1327** ([@sarvesh1327](https://github.com/sarvesh1327)) — Fixed runtime preset handling so an explicit `COMPACTION_TRIGGER_COUNT` environment override is preserved instead of being clobbered by default/swarm/trusted presets. Added regression coverage for operator/Docker tuning paths around compaction trigger configuration ([#43](https://github.com/Lyellr88/marm-memory/pull/43)). - **zza-830** ([@zza-830](https://github.com/zza-830)) — Hardened configuration parsing with bounds checks and clamping warnings across server ports, rate limits, queue sizes, recall limits, compaction settings, and search weights. Also added `MARM_RECALL_DEBUG` observability so recall lane selection, fallback behavior, and candidate breakdowns can be inspected safely through stderr without adding new MCP tools ([#54](https://github.com/Lyellr88/marm-memory/pull/54)). - **Vaishnavi Desai** ([@vaishnavidesai09](https://github.com/vaishnavidesai09)) — Added the exact retrieval lane for code, config, command, and API-contract queries. The work introduced syntax-heavy query detection, the `exact_mode` control surface, deterministic FTS/BM25 recall with LIKE fallback, full HTTP/STDIO/service/core parameter wiring, project/platform scoping in exact recall, and regression coverage for routing, ranking, fallback behavior, and response compatibility ([#71](https://github.com/Lyellr88/marm-memory/pull/71)). -- **Muneeb Ahmad** ([@Mxneeb](https://github.com/Mxneeb)) — Proposed replacing min-max fusion with Reciprocal Rank Fusion on the hybrid recall path, with a complete implementation and a documented mathematical rationale ([#112](https://github.com/Lyellr88/marm-memory/pull/112)). The change was not merged, but the implementation became the reference for a controlled bake-off: a weighted RRF variant preserving MARM's shipped lexical weight, so fusion was the only variable. RRF measured 6.8-7.3pp below min-max across all five LoCoMo categories, which settled a question that had been open on reasoning alone and produced the fusion decision record in `docs/current/`. The experiment also corrected MARM's estimate of its own benchmark noise from ~0.1pp to 0.56pp, a methodology fix that outlives the experiment. Separately reported a real consolidation defect, fixed in 2.33.1 and first released in v2.34.0 ([#113](https://github.com/Lyellr88/marm-memory/issues/113)): `CONSOLIDATION_THRESHOLD` is documented as a cosine threshold but was compared against a blended ranking score, which proved to be live under min-max as well, not introduced by the RRF proposal. Also surfaced the chunk-count bias in `_score_chunk_aware`'s max-over-chunks pooling, since measured and confirmed, and independently identified the recall risk the FTS candidate filter carries for low-token-overlap paraphrases. +- **Muneeb Ahmad** ([@Mxneeb](https://github.com/Mxneeb)) — Proposed replacing min-max fusion with Reciprocal Rank Fusion on the hybrid recall path, with a complete implementation and a documented mathematical rationale ([#112](https://github.com/Lyellr88/marm-memory/pull/112)). The change was not merged, but the implementation became the reference for a controlled bake-off: a weighted RRF variant preserving MARM's shipped lexical weight, so fusion was the only variable. RRF measured 6.8-7.3pp below min-max across all five LoCoMo categories, which settled a question that had been open on reasoning alone and produced the fusion decision record in `docs/current/`. The experiment also corrected MARM's estimate of its own benchmark noise from ~0.1pp to 0.56pp, a methodology fix that outlives the experiment. Separately reported a real consolidation defect, fixed in 2.36.0 and first released in v2.36.0 ([#113](https://github.com/Lyellr88/marm-memory/issues/113)): `CONSOLIDATION_THRESHOLD` is documented as a cosine threshold but was compared against a blended ranking score, which proved to be live under min-max as well, not introduced by the RRF proposal. Also surfaced the chunk-count bias in `_score_chunk_aware`'s max-over-chunks pooling, since measured and confirmed, and independently identified the recall risk the FTS candidate filter carries for low-token-overlap paraphrases. ## Security Acknowledgments diff --git a/README.md b/README.md index afb0b708..6b9f6f03 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ width="900" height="250"> -

Give your AI Agents a permanent memory in 60 seconds.

+

Give your AI Agents a permanent memory in 60 seconds

[![License](https://img.shields.io/badge/license-Apache--2.0-blue)](https://github.com/Lyellr88/marm-memory/blob/MARM-main/LICENSE) [![Python](https://img.shields.io/badge/python-3.10%2B-blue)](https://www.python.org/) @@ -86,7 +86,7 @@ It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. - 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. -- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. +- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -140,8 +140,8 @@ marm-memory uninstall # preview package removal; always pre **Knowledge, projects, and maintenance** ```bash -marm-memory knowledge status # Check available indexers and models -marm-memory knowledge build --all # Build the concept graph from stored memories +marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is +marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) marm-memory projects list # List all tracked workspaces marm-memory projects index # Run deep codebase structural indexing marm-memory projects status # Inspect target repo graph readiness @@ -696,10 +696,10 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | |------|--------------|----------------| -| `marm_concept_build` | Extract entities and typed relationships from stored memories | `session_name`, `project`, or `search_all=True` (one required) | +| `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -865,7 +865,7 @@ Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/ ### Concept Graph: what your memories are about -MARM can extract a knowledge graph from the memories you've already stored. `marm_concept_build` runs entity and relationship extraction over stored memory content, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). Once built, `marm_smart_recall` automatically adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: +MARM extracts a knowledge graph from the memories you store, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). This happens on its own: storing a memory queues it, and a background worker adds it to the graph roughly 30 seconds later. `marm_concept_build` is still there for a full or scoped rebuild. Once there is a graph, `marm_smart_recall` adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: ```text marm_concept_recall(query="write queue") → the entity, its relationships, linked code symbols @@ -874,12 +874,17 @@ marm_concept_recall(query="related to SQLite", depth=3) → multi-hop traversal How to use it: -- **Build first**: call `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True`. There is no data until a build has run at least once. Builds are explicit and on-demand, not a live hook into the write path; re-run after logging significant new memories. -- **Upgrade once**: graphs built before platform attribution require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. -- **Bounded by design**: each build is row-capped (`CONCEPT_BUILD_ROW_CAP`, default 500) so a huge store can't turn one tool call into a runaway job. +- **Automatic by default**: new memories reach the graph without a tool call. Set `CONCEPT_AUTO_INDEX=false` to go back to manual builds only, which stops the worker but keeps recording queue rows, so turning it back on picks up everything written while it was off; `CONCEPT_INDEX_DEBOUNCE_SECONDS` (30) and `CONCEPT_INDEX_BATCH_SIZE` (20) control the pace. +- **Safe on both transports at once**: a leased lock in the memory database keeps a rebuild in one process from dropping graph tables while another process is writing to them. A build that finds the graph busy says so instead of colliding. +- **Failure never reaches your memories**: indexing runs on a durable queue outside the write path. Extraction problems retry, a memory that fails repeatedly is parked with its error, and the memory itself stores and recalls normally throughout. +- **Clearing a backlog costs some recall speed**: entity extraction is CPU-bound, so while the worker is working through a queue, measured recall goes from ~8ms to ~16ms median on a real 768-memory corpus. Writes are unaffected. It only applies while a backlog is draining, which for most people is once, after the upgrade rebuild. Reproduce it with `scripts/benchmarking/performance/bench_concept_worker.py --from-live`. +- **Build for the backlog**: `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True` indexes memories stored before automatic indexing existed, and rebuilds after an upgrade that requires one. +- **Upgrade twice so far**: graphs built before platform attribution, or before compaction sources replaced summaries as the indexed rows, require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. +- **Whole scope, paged**: builds read every memory in scope. `CONCEPT_BUILD_ROW_CAP` (default 500) is the page size, so lowering it makes a build read more, smaller pages rather than skipping the rest. +- **Compacted sessions**: the original memories are indexed and the generated summary is not, so concepts stay attributed to where they were actually stated. - **Recall fails open**: a missing, empty, incompatible, or unavailable concept graph never blocks normal memory recall. The response reports graph status separately. - **Code cross-linking**: when the code graph has indexed the same project, concept entities that match code symbols get linked, connecting "what we decided" to "where it lives in the code." -- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first concept build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. +- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first extraction, which now happens on its own shortly after the first memory is stored rather than when you run a build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. - **Isolated storage**: the concept graph lives in its own SQLite database (`~/.marm/index/marm_index.db`) with its own connection pool, so concept-graph writes can never block or corrupt the production memory database. - **Console atlas**: MARM Console renders the complete atlas up to 750 entities and 6,000 stored relationships. Larger graphs use a deterministic connected sample of up to 600 entities and 4,000 aggregated visual edges, clearly labelled as sampled. @@ -895,7 +900,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **FTS5 full-text index** (`memories_fts`) is maintained as an external-content table over the memories table and powers both the exact lane (BM25) and the filter stage of hybrid recall. - **Chunk storage**: memories past ~180 words are split into overlapping 150-token chunks (50-token overlap) in a `memory_chunks` table, each with its own embedding. Recall scores chunks and collapses to the parent memory. - **Embeddings** come from the fastembed-backed `jinaai/jina-embeddings-v2-small-en` encoder: 33M parameters, 512 dimensions, an 8,192-token context window, and an Apache-2.0 license. It does not require separate query/document text prefixes. The encoder is lazily loaded on first semantic use and serialized behind a lock so concurrent encodes can't corrupt each other. If it is unavailable, writes still succeed; memories are stored without embeddings until it loads. Semantic scoring runs as a single NumPy batch (matrix cosine) rather than a Python loop. -- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. +- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. The one exception is the indexing queue, which lives in the memory database on purpose so a memory and its indexing task commit together; the graph itself stays derived and disposable. ### Write path @@ -904,6 +909,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **Layer 1, exact dedup**: a SHA-256 hash of normalized content is checked within the session; hash hits are verified against the actual content before deduplicating, so a hash collision stores a new row instead of silently merging different content. - **Layer 2, semantic merge**: near-duplicates above `CONSOLIDATION_THRESHOLD` cosine similarity are merged rather than accumulated. This never blocks a write; if the encoder isn't available, the write proceeds unconsolidated. - The tradeoff is measured and published: roughly 9x median write cost (58ms vs 6.5ms) in exchange for a store that stays clean, because reads dominate memory workloads. See section 3 of the benchmarks above. +- **Concept indexing is a durable outbox**: a write records an indexing task in the same transaction as the memory, so a memory cannot exist without one. A background worker drains that queue and writes the concept graph. Nothing on the write path waits for extraction, and a process killed mid-extraction loses no work because the task is a row rather than an in-memory job. Both transports run a worker, so the two coordinate through a leased lock in the memory database rather than an in-process lock, which would not span them. - **Compaction** (opt-in, `COMPACTION_ENABLED=1`) is Layer 3: after enough writes in a session, a background pass detects clusters of related memories using cosine similarity plus union-find connected components, gated by minimum cluster size, minimum age, and an active-session grace period so it never compacts work in flight. MARM then injects a bounded request asking the connected agent to summarize each cluster: `candidates` → `stage` → `review` → `apply` or `discard`. Source memory IDs are preserved on apply, so compacted summaries stay traceable to their originals. Staged summaries expire (`COMPACTION_STAGING_TTL_HOURS`), nudges are capped and cooldown-limited, and the injection has a byte budget. The design is honest about what LLMs are for: MARM detects, the agent summarizes, and a human-reviewable stage/apply/discard loop gates the destructive step. ### Recall path @@ -973,7 +979,13 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | -| `CONCEPT_BUILD_ROW_CAP` | `500` | Max memory rows per concept-graph build | +| `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | +| `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | +| `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | +| `CONCEPT_INDEX_BATCH_SIZE` | `20` | Memories indexed per batch, capped at 500. Lowering it does not reduce contention; it measured slightly worse | +| `CONCEPT_INDEX_BATCH_PAUSE_MS` | `250` | Pause between batches while clearing a backlog. Cuts worst-case recall during indexing from ~270ms to ~80ms for about 18% longer drain. `0` disables it | +| `CONCEPT_INDEX_LEASE_SECONDS` | `300` | How long a claimed indexing task stays owned once nothing is renewing it. Work in progress renews its own lease, so this bounds how long a *killed* process holds tasks, not how long a batch may take. Reclaimed tasks spend no attempt | +| `CONCEPT_INDEX_MAX_ATTEMPTS` | `3` | Failed attempts before a memory is parked with its error instead of retried |
@@ -1050,6 +1062,23 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - First confirm that a scoped concept build actually includes memories with extractable entities. - Run `marm-memory knowledge status`; if it reports a missing runtime or model, repair the install with `python -m pip install -U --force-reinstall marm-mcp-server`. +**New memories are not showing up in the graph** + +- Run `marm-memory knowledge status`. `index_queue.pending` is how many memories are waiting; `index_queue.parked` is how many gave up. `auto_index: false` means indexing is switched off. +- Give it the debounce interval (30 seconds by default) plus extraction time. A burst of writes is indexed as one pass, not one per memory. +- Check that `CONCEPT_AUTO_INDEX` is not set to `false`, `0`, `no`, or `off`. +- A graph awaiting a rebuild is not indexed into. If the Console or `marm-memory knowledge status` reports `rebuild_required`, run `marm_concept_build(search_all=True)` once; queued memories are picked up after it. +- Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. +- A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. + +**A build returns `build_in_progress`** + +- Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. + +**A build returns `lock_lost`** + +- The build was stalled long enough for another process to take over the graph, so it stopped partway rather than writing alongside it. Usually a suspended machine or a debugger pause. Whatever it indexed before stopping is kept, and re-running the build finishes the rest. +
diff --git a/RELEASE-NOTES-v2.35.0.md b/RELEASE-NOTES-v2.35.0.md deleted file mode 100644 index f8551abe..00000000 --- a/RELEASE-NOTES-v2.35.0.md +++ /dev/null @@ -1,35 +0,0 @@ -# MARM v2.35.0 - -This release makes long-memory retrieval more durable and repairable. It also fixes embedding migration failures on databases that contain long content. - -## Long-memory chunks no longer disappear silently at shutdown - -MARM stores memories longer than 500 words as smaller chunks so recall can match the relevant passage rather than judge the entire memory as one block. Those chunks are written after the parent memory, which previously left a gap: if the server stopped during that background work, the parent memory survived but its chunks could be lost without an error. - -Both HTTP and STDIO shutdown now wait up to five seconds for pending chunk writes. The limit is configurable with `CHUNK_DRAIN_TIMEOUT_SECONDS`, and any work left unfinished can be repaired safely with the new rechunk command. - -## New rechunk repair command - -Run either command with all MARM processes stopped: - -```bash -marm-mcp-server --rechunk -# or -marm-memory maintenance chunks rechunk -``` - -It repairs chunks missing after an interrupted write, re-splits chunks created under older sizing rules, and removes chunks from memories now below the long-memory threshold. Already-correct memories are skipped, so the command is safe to run again. - -The command refuses to run if stored vectors use a different embedding dimension from the configured model. In that case, run `marm-mcp-server --migrate-embeddings` first. - -## Embedding migration now handles long content safely - -`--migrate-embeddings` previously processed 100 memories at a time. Because the encoder pads every item in a batch to the longest text, one long memory could force an enormous allocation and crash the migration. - -Migration now sizes batches from the actual content length. Short memories still use efficient large batches, while long-memory databases no longer attempt unsafe allocations. - -## Upgrade note - -Existing installs should run `marm-mcp-server --rechunk` once after upgrading, with MARM stopped. Recall still works without it, but long memories may be less accurate until their chunks are repaired. - -See the [v2.35.0 changelog](CHANGELOG.md) for the full technical details. diff --git a/docs/INSTALL-DOCKER.md b/docs/INSTALL-DOCKER.md index 0fbe67c0..c4140ecf 100644 --- a/docs/INSTALL-DOCKER.md +++ b/docs/INSTALL-DOCKER.md @@ -2,7 +2,7 @@ ## Universal Memory Intelligence Platform for AI Agents -**MARM v2.35.0** - Memory Accurate Response Mode +**MARM v2.36.0** - Memory Accurate Response Mode *Docker deployment guide for Windows, Mac, and Linux* --- diff --git a/docs/INSTALL-LINUX.md b/docs/INSTALL-LINUX.md index 258ded17..a8a2f180 100644 --- a/docs/INSTALL-LINUX.md +++ b/docs/INSTALL-LINUX.md @@ -2,7 +2,7 @@ ## Universal Memory Intelligence Platform for AI Agents -**MARM v2.35.0** - Memory Accurate Response Mode +**MARM v2.36.0** - Memory Accurate Response Mode *Complete Linux installation guide* --- @@ -320,7 +320,7 @@ curl -s http://localhost:8001/health { "status": "healthy", "service": "MARM MCP Server", - "version": "2.35.0", + "version": "2.36.0", "timestamp": "2026-01-01T00:00:00+00:00", "database": "connected", "semantic_search": "available" diff --git a/docs/INSTALL-PLATFORMS.md b/docs/INSTALL-PLATFORMS.md index 5a6a4d84..d082467e 100644 --- a/docs/INSTALL-PLATFORMS.md +++ b/docs/INSTALL-PLATFORMS.md @@ -1,4 +1,4 @@ -# MARM v2.35.0 MCP Server - Platform Integration Guide +# MARM v2.36.0 MCP Server - Platform Integration Guide ## Table of Contents diff --git a/docs/INSTALL-WINDOWS.md b/docs/INSTALL-WINDOWS.md index 3cbba1fd..0787ec7f 100644 --- a/docs/INSTALL-WINDOWS.md +++ b/docs/INSTALL-WINDOWS.md @@ -2,7 +2,7 @@ ## Universal Memory Intelligence Platform for AI Agents -**MARM v2.35.0** - Memory Accurate Response Mode +**MARM v2.36.0** - Memory Accurate Response Mode *Complete Windows installation guide* --- @@ -294,7 +294,7 @@ Invoke-WebRequest -Uri http://localhost:8001/health { "status": "healthy", "service": "MARM MCP Server", - "version": "2.35.0", + "version": "2.36.0", "timestamp": "2026-01-01T00:00:00+00:00", "database": "connected", "semantic_search": "available" diff --git a/marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx b/marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx index 6bec9496..9b7e4233 100644 --- a/marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx +++ b/marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useMemo } from 'react'; -import { useConceptsSummary, useSearchConcepts, useNeighborhood, useConceptGraph, useConcept, useMarmConfig } from '@/hooks/use-marm-queries'; +import { useConceptsSummary, useSearchConcepts, useNeighborhood, useConceptGraph, useConcept, useMarmConfig, useGraphAutoRefresh } from '@/hooks/use-marm-queries'; import { Card, CardContent, CardHeader, Input, Button, Badge, Select, SelectTrigger, SelectValue, SelectContent, SelectItem } from '@/components/ui/core'; import { Search, GitGraph, Network, AlertTriangle, X, ArrowLeft } from 'lucide-react'; import type { Neighborhood, NeighborhoodNode, ConceptDetail } from '@/lib/marm-types'; @@ -94,6 +94,9 @@ function ProvenancePanel({ } export function ExplorerTab() { + // Background indexing adds nodes with nobody watching. This component only + // exists while the Explorer tab is showing, so the polling stops with it. + useGraphAutoRefresh(); const { data: summary } = useConceptsSummary(); const { client } = useMarmConfig(); const [q, setQ] = useState(''); diff --git a/marm-console/artifacts/marm-console/src/hooks/use-marm-queries.ts b/marm-console/artifacts/marm-console/src/hooks/use-marm-queries.ts index ae987a16..26ddc449 100644 --- a/marm-console/artifacts/marm-console/src/hooks/use-marm-queries.ts +++ b/marm-console/artifacts/marm-console/src/hooks/use-marm-queries.ts @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useMarmClient } from '@/lib/use-marm-client'; import { useConnection } from '@/lib/marm-connection'; @@ -20,6 +21,7 @@ export const queryKeys = { compaction: (baseUrl: string) => ['compaction', baseUrl], conceptsSummary: (baseUrl: string) => ['conceptsSummary', baseUrl], conceptsGraph: (baseUrl: string) => ['conceptsGraph', baseUrl], + conceptsGraphVersion: (baseUrl: string) => ['conceptsGraphVersion', baseUrl], conceptsSearch: (baseUrl: string, params?: ConceptSearchParams) => ['conceptsSearch', baseUrl, params], concept: (baseUrl: string, id: number) => ['concept', baseUrl, id], neighborhood: (baseUrl: string, id: number, params?: any) => ['neighborhood', baseUrl, id, params], @@ -292,6 +294,44 @@ export function useConceptGraph(enabled = true) { }); } +/** Polls a cheap change marker so background indexing reaches the screen + * without a reload. Only the marker is fetched on this interval; the atlas + * itself is refetched by useGraphAutoRefresh when the marker moves. + * refetchIntervalInBackground stays off (the default), so a hidden window + * stops polling on its own. */ +export function useConceptGraphVersion(enabled = true, intervalMs = 5000) { + const { baseUrl, client } = useMarmConfig(); + return useQuery({ + queryKey: queryKeys.conceptsGraphVersion(baseUrl), + queryFn: client.getConceptGraphVersion, + enabled, + refetchInterval: enabled ? intervalMs : false, + }); +} + +/** Invalidates the graph views whenever the polled marker changes. Mount it + * in a component that is unmounted when its tab is not showing. */ +export function useGraphAutoRefresh(enabled = true) { + const { baseUrl } = useMarmConfig(); + const qc = useQueryClient(); + const { data } = useConceptGraphVersion(enabled); + const version = data?.version; + const seen = useRef(undefined); + + useEffect(() => { + if (!version) return; + if (seen.current === undefined) { + seen.current = version; + return; + } + if (seen.current === version) return; + seen.current = version; + qc.invalidateQueries({ queryKey: ['conceptsGraph', baseUrl] }); + qc.invalidateQueries({ queryKey: ['neighborhood', baseUrl] }); + qc.invalidateQueries({ queryKey: ['conceptsSummary', baseUrl] }); + }, [version, baseUrl, qc]); +} + export function useConcept(id: number) { const { baseUrl, client } = useMarmConfig(); return useQuery({ queryKey: queryKeys.concept(baseUrl, id), queryFn: () => client.getConcept(id), enabled: !!id }); diff --git a/marm-console/artifacts/marm-console/src/lib/marm-api.ts b/marm-console/artifacts/marm-console/src/lib/marm-api.ts index 3d67a31f..78c5e94f 100644 --- a/marm-console/artifacts/marm-console/src/lib/marm-api.ts +++ b/marm-console/artifacts/marm-console/src/lib/marm-api.ts @@ -15,6 +15,7 @@ import type { ConceptAtlas, ConceptDetail, ConceptEntity, + ConceptGraphVersion, ConceptSearchParams, ConceptsSummary, DuplicateCandidate, @@ -205,6 +206,8 @@ export function createMarmClient(config: MarmClientConfig) { request(config, 'GET', `/concepts/${entityId}`), getConceptGraph: () => request(config, 'GET', '/concepts/graph'), + getConceptGraphVersion: () => + request(config, 'GET', '/concepts/graph/version'), getConceptNeighborhood: ( entityId: number, params?: { depth?: number; direction?: string; predicate?: string }, diff --git a/marm-console/artifacts/marm-console/src/lib/marm-types.ts b/marm-console/artifacts/marm-console/src/lib/marm-types.ts index 2e4cd901..bfdcab48 100644 --- a/marm-console/artifacts/marm-console/src/lib/marm-types.ts +++ b/marm-console/artifacts/marm-console/src/lib/marm-types.ts @@ -291,6 +291,13 @@ export interface ConceptAtlas extends Neighborhood { sample_reason: string | null; } +/** Cheap change marker polled while the Explorer is open. The value is opaque: + * compare it, do not parse it. */ +export interface ConceptGraphVersion { + schema_status: 'current' | 'rebuild_required' | 'unavailable'; + version: string; +} + export interface DuplicateCandidate { entity_a: ConceptEntity; entity_b: ConceptEntity; diff --git a/marm-mcp-server/Dockerfile b/marm-mcp-server/Dockerfile index 10853784..c1573ca7 100644 --- a/marm-mcp-server/Dockerfile +++ b/marm-mcp-server/Dockerfile @@ -73,7 +73,7 @@ HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ LABEL org.opencontainers.image.title="MARM Universal MCP Server" LABEL org.opencontainers.image.description="Production-ready Universal MCP Server with advanced AI memory capabilities, semantic search, and professional-grade architecture" -LABEL org.opencontainers.image.version="2.35.0" +LABEL org.opencontainers.image.version="2.36.0" LABEL org.opencontainers.image.authors="Ryan Lyell - marm-memory" LABEL org.opencontainers.image.url="https://marmsystems.com" LABEL org.opencontainers.image.source="https://github.com/Lyellr88/marm-memory" diff --git a/marm-mcp-server/README.md b/marm-mcp-server/README.md index 29517be7..d8a3c8a8 100644 --- a/marm-mcp-server/README.md +++ b/marm-mcp-server/README.md @@ -7,7 +7,7 @@ mcp-name: io.github.Lyellr88/marm-mcp-server width="900" height="250"> -

Give your AI Agents a permanent memory in 60 seconds.

+

Give your AI Agents a permanent memory in 60 seconds

[![License](https://img.shields.io/badge/license-Apache--2.0-blue)](https://github.com/Lyellr88/marm-memory/blob/MARM-main/LICENSE) [![Python](https://img.shields.io/badge/python-3.10%2B-blue)](https://www.python.org/) @@ -88,7 +88,7 @@ It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. - 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. -- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. +- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -142,8 +142,8 @@ marm-memory uninstall # preview package removal; always pre **Knowledge, projects, and maintenance** ```bash -marm-memory knowledge status # Check available indexers and models -marm-memory knowledge build --all # Build the concept graph from stored memories +marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is +marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) marm-memory projects list # List all tracked workspaces marm-memory projects index # Run deep codebase structural indexing marm-memory projects status # Inspect target repo graph readiness @@ -698,10 +698,10 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | |------|--------------|----------------| -| `marm_concept_build` | Extract entities and typed relationships from stored memories | `session_name`, `project`, or `search_all=True` (one required) | +| `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -867,7 +867,7 @@ Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/ ### Concept Graph: what your memories are about -MARM can extract a knowledge graph from the memories you've already stored. `marm_concept_build` runs entity and relationship extraction over stored memory content, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). Once built, `marm_smart_recall` automatically adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: +MARM extracts a knowledge graph from the memories you store, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). This happens on its own: storing a memory queues it, and a background worker adds it to the graph roughly 30 seconds later. `marm_concept_build` is still there for a full or scoped rebuild. Once there is a graph, `marm_smart_recall` adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: ```text marm_concept_recall(query="write queue") → the entity, its relationships, linked code symbols @@ -876,12 +876,17 @@ marm_concept_recall(query="related to SQLite", depth=3) → multi-hop traversal How to use it: -- **Build first**: call `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True`. There is no data until a build has run at least once. Builds are explicit and on-demand, not a live hook into the write path; re-run after logging significant new memories. -- **Upgrade once**: graphs built before platform attribution require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. -- **Bounded by design**: each build is row-capped (`CONCEPT_BUILD_ROW_CAP`, default 500) so a huge store can't turn one tool call into a runaway job. +- **Automatic by default**: new memories reach the graph without a tool call. Set `CONCEPT_AUTO_INDEX=false` to go back to manual builds only, which stops the worker but keeps recording queue rows, so turning it back on picks up everything written while it was off; `CONCEPT_INDEX_DEBOUNCE_SECONDS` (30) and `CONCEPT_INDEX_BATCH_SIZE` (20) control the pace. +- **Safe on both transports at once**: a leased lock in the memory database keeps a rebuild in one process from dropping graph tables while another process is writing to them. A build that finds the graph busy says so instead of colliding. +- **Failure never reaches your memories**: indexing runs on a durable queue outside the write path. Extraction problems retry, a memory that fails repeatedly is parked with its error, and the memory itself stores and recalls normally throughout. +- **Clearing a backlog costs some recall speed**: entity extraction is CPU-bound, so while the worker is working through a queue, measured recall goes from ~8ms to ~16ms median on a real 768-memory corpus. Writes are unaffected. It only applies while a backlog is draining, which for most people is once, after the upgrade rebuild. Reproduce it with `scripts/benchmarking/performance/bench_concept_worker.py --from-live`. +- **Build for the backlog**: `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True` indexes memories stored before automatic indexing existed, and rebuilds after an upgrade that requires one. +- **Upgrade twice so far**: graphs built before platform attribution, or before compaction sources replaced summaries as the indexed rows, require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. +- **Whole scope, paged**: builds read every memory in scope. `CONCEPT_BUILD_ROW_CAP` (default 500) is the page size, so lowering it makes a build read more, smaller pages rather than skipping the rest. +- **Compacted sessions**: the original memories are indexed and the generated summary is not, so concepts stay attributed to where they were actually stated. - **Recall fails open**: a missing, empty, incompatible, or unavailable concept graph never blocks normal memory recall. The response reports graph status separately. - **Code cross-linking**: when the code graph has indexed the same project, concept entities that match code symbols get linked, connecting "what we decided" to "where it lives in the code." -- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first concept build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. +- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first extraction, which now happens on its own shortly after the first memory is stored rather than when you run a build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. - **Isolated storage**: the concept graph lives in its own SQLite database (`~/.marm/index/marm_index.db`) with its own connection pool, so concept-graph writes can never block or corrupt the production memory database. - **Console atlas**: MARM Console renders the complete atlas up to 750 entities and 6,000 stored relationships. Larger graphs use a deterministic connected sample of up to 600 entities and 4,000 aggregated visual edges, clearly labelled as sampled. @@ -897,7 +902,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **FTS5 full-text index** (`memories_fts`) is maintained as an external-content table over the memories table and powers both the exact lane (BM25) and the filter stage of hybrid recall. - **Chunk storage**: memories past ~180 words are split into overlapping 150-token chunks (50-token overlap) in a `memory_chunks` table, each with its own embedding. Recall scores chunks and collapses to the parent memory. - **Embeddings** come from the fastembed-backed `jinaai/jina-embeddings-v2-small-en` encoder: 33M parameters, 512 dimensions, an 8,192-token context window, and an Apache-2.0 license. It does not require separate query/document text prefixes. The encoder is lazily loaded on first semantic use and serialized behind a lock so concurrent encodes can't corrupt each other. If it is unavailable, writes still succeed; memories are stored without embeddings until it loads. Semantic scoring runs as a single NumPy batch (matrix cosine) rather than a Python loop. -- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. +- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. The one exception is the indexing queue, which lives in the memory database on purpose so a memory and its indexing task commit together; the graph itself stays derived and disposable. ### Write path @@ -906,6 +911,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **Layer 1, exact dedup**: a SHA-256 hash of normalized content is checked within the session; hash hits are verified against the actual content before deduplicating, so a hash collision stores a new row instead of silently merging different content. - **Layer 2, semantic merge**: near-duplicates above `CONSOLIDATION_THRESHOLD` cosine similarity are merged rather than accumulated. This never blocks a write; if the encoder isn't available, the write proceeds unconsolidated. - The tradeoff is measured and published: roughly 9x median write cost (58ms vs 6.5ms) in exchange for a store that stays clean, because reads dominate memory workloads. See section 3 of the benchmarks above. +- **Concept indexing is a durable outbox**: a write records an indexing task in the same transaction as the memory, so a memory cannot exist without one. A background worker drains that queue and writes the concept graph. Nothing on the write path waits for extraction, and a process killed mid-extraction loses no work because the task is a row rather than an in-memory job. Both transports run a worker, so the two coordinate through a leased lock in the memory database rather than an in-process lock, which would not span them. - **Compaction** (opt-in, `COMPACTION_ENABLED=1`) is Layer 3: after enough writes in a session, a background pass detects clusters of related memories using cosine similarity plus union-find connected components, gated by minimum cluster size, minimum age, and an active-session grace period so it never compacts work in flight. MARM then injects a bounded request asking the connected agent to summarize each cluster: `candidates` → `stage` → `review` → `apply` or `discard`. Source memory IDs are preserved on apply, so compacted summaries stay traceable to their originals. Staged summaries expire (`COMPACTION_STAGING_TTL_HOURS`), nudges are capped and cooldown-limited, and the injection has a byte budget. The design is honest about what LLMs are for: MARM detects, the agent summarizes, and a human-reviewable stage/apply/discard loop gates the destructive step. ### Recall path @@ -975,7 +981,13 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | -| `CONCEPT_BUILD_ROW_CAP` | `500` | Max memory rows per concept-graph build | +| `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | +| `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | +| `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | +| `CONCEPT_INDEX_BATCH_SIZE` | `20` | Memories indexed per batch, capped at 500. Lowering it does not reduce contention; it measured slightly worse | +| `CONCEPT_INDEX_BATCH_PAUSE_MS` | `250` | Pause between batches while clearing a backlog. Cuts worst-case recall during indexing from ~270ms to ~80ms for about 18% longer drain. `0` disables it | +| `CONCEPT_INDEX_LEASE_SECONDS` | `300` | How long a claimed indexing task stays owned once nothing is renewing it. Work in progress renews its own lease, so this bounds how long a *killed* process holds tasks, not how long a batch may take. Reclaimed tasks spend no attempt | +| `CONCEPT_INDEX_MAX_ATTEMPTS` | `3` | Failed attempts before a memory is parked with its error instead of retried |
@@ -1052,6 +1064,23 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - First confirm that a scoped concept build actually includes memories with extractable entities. - Run `marm-memory knowledge status`; if it reports a missing runtime or model, repair the install with `python -m pip install -U --force-reinstall marm-mcp-server`. +**New memories are not showing up in the graph** + +- Run `marm-memory knowledge status`. `index_queue.pending` is how many memories are waiting; `index_queue.parked` is how many gave up. `auto_index: false` means indexing is switched off. +- Give it the debounce interval (30 seconds by default) plus extraction time. A burst of writes is indexed as one pass, not one per memory. +- Check that `CONCEPT_AUTO_INDEX` is not set to `false`, `0`, `no`, or `off`. +- A graph awaiting a rebuild is not indexed into. If the Console or `marm-memory knowledge status` reports `rebuild_required`, run `marm_concept_build(search_all=True)` once; queued memories are picked up after it. +- Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. +- A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. + +**A build returns `build_in_progress`** + +- Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. + +**A build returns `lock_lost`** + +- The build was stalled long enough for another process to take over the graph, so it stopped partway rather than writing alongside it. Usually a suspended machine or a debugger pause. Whatever it indexed before stopping is kept, and re-running the build finishes the rest. +
diff --git a/marm-mcp-server/docker-compose.yml b/marm-mcp-server/docker-compose.yml index c0a0271f..702e5223 100644 --- a/marm-mcp-server/docker-compose.yml +++ b/marm-mcp-server/docker-compose.yml @@ -5,7 +5,7 @@ services: dockerfile: Dockerfile # :latest is the all-in-one MCP image (memory + graph, one port). # Pin :memory-only instead if you need the pre-unification image shape. - image: lyellr88/marm-mcp-server:2.35.0 + image: lyellr88/marm-mcp-server:2.36.0 container_name: marm-mcp-server restart: unless-stopped @@ -18,7 +18,7 @@ services: environment: - SERVER_HOST=0.0.0.0 - SERVER_PORT=8001 - - SERVER_VERSION=2.35.0 + - SERVER_VERSION=2.36.0 - ENVIRONMENT=production - LOG_LEVEL=INFO diff --git a/marm-mcp-server/marm_mcp_server/__init__.py b/marm-mcp-server/marm_mcp_server/__init__.py index 685ddf39..63995c64 100644 --- a/marm-mcp-server/marm_mcp_server/__init__.py +++ b/marm-mcp-server/marm_mcp_server/__init__.py @@ -14,10 +14,10 @@ - Production-grade performance Author: Ryan Lyell - marm-memory -Version: 2.35.0 +Version: 2.36.0 """ -__version__ = "2.35.0" +__version__ = "2.36.0" __author__ = "Ryan Lyell" __email__ = "ryanlyell@marmemory.com" diff --git a/marm-mcp-server/marm_mcp_server/config/settings.py b/marm-mcp-server/marm_mcp_server/config/settings.py index 58aa12c4..514b8528 100644 --- a/marm-mcp-server/marm_mcp_server/config/settings.py +++ b/marm-mcp-server/marm_mcp_server/config/settings.py @@ -55,6 +55,33 @@ def _safe_unit_float(env_key: str, default: float) -> float: return clamped +_TRUE_WORDS = ("1", "true", "yes", "on") +_FALSE_WORDS = ("0", "false", "no", "off") + + +def _safe_bool(env_key: str, default: bool) -> bool: + """Read an on/off env var, accepting the spellings people actually type. + + Comparing against a single literal is what makes a flag lie: a check for + != "0" reads CONCEPT_AUTO_INDEX=false as on, which is the opposite of what + the user asked for and of what the docs promise. + """ + raw = os.environ.get(env_key) + if raw is None: + return default + value = raw.strip().lower() + if value in _TRUE_WORDS: + return True + if value in _FALSE_WORDS: + return False + print( + f"WARNING: {env_key}={raw!r} is not a true/false value, " + f"using default {default}", + file=sys.stderr, + ) + return default + + def _safe_choice(env_key: str, default: str, allowed: tuple[str, ...]) -> str: """Read an env var constrained to a fixed set, falling back on anything else.""" raw = os.environ.get(env_key) @@ -179,7 +206,7 @@ def get_analytics_db_path(): f"WARNING: SERVER_PORT={_raw_port} out of [1, 65535], clamped to {SERVER_PORT}", file=sys.stderr, ) -SERVER_VERSION = "2.35.0" +SERVER_VERSION = "2.36.0" GRAPH_ENABLED = os.environ.get("GRAPH_ENABLED", "true").lower() != "false" @@ -273,6 +300,8 @@ def _detect_platform() -> str: file=sys.stderr, ) +# Page size for concept builds, not a truncation limit. Lowering it makes a +# build read more, smaller pages; it no longer makes the build skip rows. _raw_cbc = _safe_int("CONCEPT_BUILD_ROW_CAP", 500) CONCEPT_BUILD_ROW_CAP = max(1, _raw_cbc) if _raw_cbc < 1: @@ -281,6 +310,65 @@ def _detect_platform() -> str: file=sys.stderr, ) +CONCEPT_AUTO_INDEX = _safe_bool("CONCEPT_AUTO_INDEX", True) + +# Quiet period after the most recent write before the worker starts draining. +# An agent storing a burst of memories produces one drain, not one per write. +_raw_cids = _safe_int("CONCEPT_INDEX_DEBOUNCE_SECONDS", 30) +CONCEPT_INDEX_DEBOUNCE_SECONDS = max(1, _raw_cids) +if _raw_cids < 1: + print( + f"WARNING: CONCEPT_INDEX_DEBOUNCE_SECONDS={_raw_cids} below minimum 1, " + f"clamped to {CONCEPT_INDEX_DEBOUNCE_SECONDS}", + file=sys.stderr, + ) + +# Upper bound as well as lower: a claimed batch becomes one IN (...) clause in +# three queries, and a batch past SQLite's variable ceiling would fail the same +# way on every cycle forever. 500 memories of spaCy extraction per batch is +# already far beyond anything useful. +CONCEPT_INDEX_BATCH_SIZE_MAX = 500 +_raw_cibs = _safe_int("CONCEPT_INDEX_BATCH_SIZE", 20) +CONCEPT_INDEX_BATCH_SIZE = max(1, min(CONCEPT_INDEX_BATCH_SIZE_MAX, _raw_cibs)) +if not (1 <= _raw_cibs <= CONCEPT_INDEX_BATCH_SIZE_MAX): + print( + f"WARNING: CONCEPT_INDEX_BATCH_SIZE={_raw_cibs} out of " + f"[1, {CONCEPT_INDEX_BATCH_SIZE_MAX}], clamped to {CONCEPT_INDEX_BATCH_SIZE}", + file=sys.stderr, + ) + +# Pause between batches while draining a backlog. Extraction is CPU-bound and +# competes with recall for cores, so a worker at full throttle measurably slows +# interactive work; this trades drain duration for that latency. 0 disables it. +_raw_cibp = _safe_int("CONCEPT_INDEX_BATCH_PAUSE_MS", 250) +CONCEPT_INDEX_BATCH_PAUSE_MS = max(0, min(10_000, _raw_cibp)) +if not (0 <= _raw_cibp <= 10_000): + print( + f"WARNING: CONCEPT_INDEX_BATCH_PAUSE_MS={_raw_cibp} out of [0, 10000], " + f"clamped to {CONCEPT_INDEX_BATCH_PAUSE_MS}", + file=sys.stderr, + ) + +# How long a claimed task stays owned. A process killed mid-extraction leaves +# its tasks claimable again after this, without burning an attempt. +_raw_cils = _safe_int("CONCEPT_INDEX_LEASE_SECONDS", 300) +CONCEPT_INDEX_LEASE_SECONDS = max(1, _raw_cils) +if _raw_cils < 1: + print( + f"WARNING: CONCEPT_INDEX_LEASE_SECONDS={_raw_cils} below minimum 1, " + f"clamped to {CONCEPT_INDEX_LEASE_SECONDS}", + file=sys.stderr, + ) + +_raw_cima = _safe_int("CONCEPT_INDEX_MAX_ATTEMPTS", 3) +CONCEPT_INDEX_MAX_ATTEMPTS = max(1, _raw_cima) +if _raw_cima < 1: + print( + f"WARNING: CONCEPT_INDEX_MAX_ATTEMPTS={_raw_cima} below minimum 1, " + f"clamped to {CONCEPT_INDEX_MAX_ATTEMPTS}", + file=sys.stderr, + ) + # Starting point to tune from real usage, not a validated-forever constant -- # fastembed's model is tuned for sentence-length input; behavior on single- # word/short-phrase entity names is less validated than on the full-memory- diff --git a/marm-mcp-server/marm_mcp_server/console/concept_store.py b/marm-mcp-server/marm_mcp_server/console/concept_store.py index 45fb8d40..afedc871 100644 --- a/marm-mcp-server/marm_mcp_server/console/concept_store.py +++ b/marm-mcp-server/marm_mcp_server/console/concept_store.py @@ -9,7 +9,12 @@ from math import sqrt from pathlib import Path -_CURRENT_CONCEPT_SCHEMA_VERSION = "2" +from ..core.concept_db import CONCEPT_SCHEMA_VERSION + +# Read from the writer's own constant rather than restated here. As a literal +# it silently disagreed on the next bump, and the Console would have called +# every freshly rebuilt graph stale. +_CURRENT_CONCEPT_SCHEMA_VERSION = str(CONCEPT_SCHEMA_VERSION) def _connect(db_path: Path) -> sqlite3.Connection | None: @@ -53,6 +58,39 @@ def _schema_status(connection: sqlite3.Connection) -> str: return "unavailable" +def graph_version(db_path: Path) -> dict: + """A cheap change marker the Console can poll while the Explorer is open. + + Four counters and the last build's finish time, so a poll costs two counts + and two max lookups instead of serializing the whole atlas. Only the graph + itself is refetched, and only when this moves. + + It does not move when an existing entity is merely mentioned by another + memory: that adds provenance to a row rather than creating one. The node + is already on screen in that case, which is what this exists to deliver. + """ + connection = _connect(db_path) + if connection is None: + return {"schema_status": "unavailable", "version": "unavailable"} + with closing(connection), connection: + schema_status = _schema_status(connection) + if schema_status != "current": + return {"schema_status": schema_status, "version": schema_status} + entities, max_entity = connection.execute( + "SELECT COUNT(*), COALESCE(MAX(id), 0) FROM entities" + ).fetchone() + relationships, max_relationship = connection.execute( + "SELECT COUNT(*), COALESCE(MAX(id), 0) FROM relationships" + ).fetchone() + last_build = connection.execute( + "SELECT COALESCE(MAX(finished_at), '') FROM concept_build_runs" + ).fetchone()[0] + return { + "schema_status": schema_status, + "version": f"{entities}:{max_entity}:{relationships}:{max_relationship}:{last_build}", + } + + def summary(db_path: Path) -> dict: connection = _connect(db_path) if connection is None: diff --git a/marm-mcp-server/marm_mcp_server/console/endpoints/concepts.py b/marm-mcp-server/marm_mcp_server/console/endpoints/concepts.py index 4ae1dad1..a34809fd 100644 --- a/marm-mcp-server/marm_mcp_server/console/endpoints/concepts.py +++ b/marm-mcp-server/marm_mcp_server/console/endpoints/concepts.py @@ -81,6 +81,14 @@ def get_concept_graph() -> dict: return graph_overview(get_concept_db_path()) +@router.get("/api/concepts/graph/version") +def get_concept_graph_version() -> dict: + """Polled while the Explorer is open so background indexing shows up + without a reload. Deliberately cheap: the atlas is only refetched when + this value moves.""" + return concept_store.graph_version(get_concept_db_path()) + + @router.get("/api/concepts/{entity_id}") def get_concept(entity_id: int) -> dict: entity = concept_store.get_entity(get_concept_db_path(), entity_id) diff --git a/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py b/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py new file mode 100644 index 00000000..1276d1d3 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py @@ -0,0 +1,210 @@ +"""Cross-process mutual exclusion for concept graph writes. + +endpoints/concepts.py holds an asyncio.Lock, which serializes builds inside one +interpreter and nothing beyond it. HTTP and STDIO are two processes over one +memory database, and every process now runs an indexing worker, so the manual +build and somebody else's worker can reach the concept database at the same +time. The manual build is the dangerous half: a rebuild backs up and drops the +graph tables, which is exactly what the v2.36.0 upgrade asks every user to run. + +The lock is one row in the memory database, held for the duration of a build +and released after. It expires so a killed process cannot wedge indexing +forever, and both holders take it before the in-process lock so the two can +never be acquired in opposite orders. +""" + +import os +import threading +import uuid +from contextlib import asynccontextmanager +from datetime import datetime, timedelta, timezone +from typing import Any, AsyncIterator, NamedTuple, Optional + +import structlog + +logger = structlog.get_logger(__name__) + +# A full-corpus rebuild is a long operation and must not have the lock pulled +# out from under it mid-run. This only decides how long a *crashed* holder +# blocks the next build, so it is generous on purpose. +MANUAL_BUILD_LOCK_SECONDS = 3600 + + +class ConceptBuildBusy(RuntimeError): + """Another process is writing the concept graph.""" + + +class BuildLease(NamedTuple): + """A held lock, plus the flag that says we stopped holding it. + + `lost` is a threading.Event rather than an asyncio one because the work it + interrupts runs in a worker thread, where an asyncio primitive cannot be + read safely. + """ + + holder: str + lost: threading.Event + + +def _connection() -> Any: + """Resolved on use: this module is reached from endpoints and from the + worker, and core.memory is heavy to bind at import time.""" + from .memory import memory + + return memory.get_connection() + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def try_acquire(holder: str, purpose: str, ttl_seconds: int) -> bool: + """Take the lock if it is free or the current holder's lease has expired.""" + now = _now() + expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() + with _connection() as conn: + conn.execute("BEGIN IMMEDIATE") + try: + row = conn.execute( + "SELECT holder, purpose, expires_at FROM concept_build_lock WHERE id = 1" + ).fetchone() + if row is not None and row[2] > now.isoformat(): + conn.execute("COMMIT") + return False + if row is not None: + logger.info("concept_lock.reclaimed_expired", previous=row[1]) + conn.execute( + """ + INSERT INTO concept_build_lock + (id, holder, purpose, acquired_at, expires_at) + VALUES (1, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET + holder = excluded.holder, + purpose = excluded.purpose, + acquired_at = excluded.acquired_at, + expires_at = excluded.expires_at + """, + (holder, purpose, now.isoformat(), expires_at), + ) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + return True + + +def renew(holder: str, ttl_seconds: int) -> bool: + """Push our own expiry back. False means we no longer hold it. + + Without this the lock is a deadline rather than a lock: a batch or a + rebuild that outlives its TTL gets overtaken by the next process, which is + the collision the lock exists to prevent. + """ + now = _now() + expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() + with _connection() as conn: + cursor = conn.execute( + "UPDATE concept_build_lock SET expires_at = ? " + "WHERE id = 1 AND holder = ? AND expires_at > ?", + (expires_at, holder, now.isoformat()), + ) + return bool(cursor.rowcount > 0) + + +def release(holder: str) -> bool: + """Release only our own hold. A lease that already expired and was taken by + someone else must not be deleted out from under them.""" + with _connection() as conn: + cursor = conn.execute( + "DELETE FROM concept_build_lock WHERE id = 1 AND holder = ?", (holder,) + ) + return bool(cursor.rowcount > 0) + + +def current_holder() -> Optional[tuple[str, str]]: + """(purpose, expires_at) of a live hold, or None.""" + with _connection() as conn: + row = conn.execute( + "SELECT purpose, expires_at FROM concept_build_lock WHERE id = 1" + ).fetchone() + if row is None or row[1] <= _now().isoformat(): + return None + return (row[0], row[1]) + + +def heartbeat_interval(ttl_seconds: float) -> float: + """Renew well inside the TTL so one slow renewal cannot lose the lock. + + Floored so a deliberately tiny lease setting cannot turn the heartbeat + into a busy loop against SQLite. + """ + return max(0.5, ttl_seconds / 3) + + +@asynccontextmanager +async def concept_build_lock( + purpose: str, ttl_seconds: int +) -> AsyncIterator[BuildLease]: + """Hold the graph for one operation, or raise ConceptBuildBusy. + + Renewed by a heartbeat for as long as the body runs, so the TTL bounds how + long a *crashed* holder blocks others rather than how long a legitimate + build is allowed to take. A full rebuild has no bounded runtime and a + batch's runtime depends on the corpus, so a fixed expiry would eventually + be crossed by real work. + + Never waits to acquire. Both callers have something better to do than + block: the worker skips the cycle and keeps its tasks queued, and the + manual build tells the user who has it. + """ + import asyncio + + holder = f"{os.getpid()}:{uuid.uuid4().hex}" + if not await asyncio.to_thread(try_acquire, holder, purpose, ttl_seconds): + raise ConceptBuildBusy("another process is writing the concept graph") + + lease = BuildLease(holder=holder, lost=threading.Event()) + + async def _keep_alive() -> None: + interval = heartbeat_interval(ttl_seconds) + loop = asyncio.get_running_loop() + last_renewed = loop.time() + while True: + await asyncio.sleep(interval) + try: + # A renewal that keeps failing is indistinguishable from one + # that was refused: either way the lease runs out on its own + # clock and someone else can take the graph. Give up at the + # TTL rather than logging warnings while still writing. + if loop.time() - last_renewed >= ttl_seconds: + logger.error("concept_lock.lost", purpose=purpose, reason="stale") + lease.lost.set() + return + if not await asyncio.to_thread(renew, holder, ttl_seconds): + # Only reachable if this process was stalled for longer + # than the whole TTL. Another process owns the graph now, + # so raise the flag: the work cannot be killed from here, + # but it can be asked to stop at its next safe point + # instead of writing alongside the new owner. + logger.error("concept_lock.lost", purpose=purpose) + lease.lost.set() + return + last_renewed = loop.time() + except Exception as exc: + logger.warning("concept_lock.renew_failed", error=str(exc)) + + beat = asyncio.create_task(_keep_alive()) + try: + yield lease + finally: + beat.cancel() + try: + await beat + except (asyncio.CancelledError, Exception): + pass + try: + await asyncio.to_thread(release, holder) + except Exception as exc: + # An unreleased lock expires on its own; failing teardown here + # would be worse than waiting it out. + logger.warning("concept_lock.release_failed", error=str(exc)) diff --git a/marm-mcp-server/marm_mcp_server/core/concept_db.py b/marm-mcp-server/marm_mcp_server/core/concept_db.py index c0dc4dcc..359cb787 100644 --- a/marm-mcp-server/marm_mcp_server/core/concept_db.py +++ b/marm-mcp-server/marm_mcp_server/core/concept_db.py @@ -21,7 +21,10 @@ from .memory_utils import _safe_print MAX_CONCEPT_DB_CONNECTIONS = 3 -CONCEPT_SCHEMA_VERSION = 2 +# 3: builds index compaction sources instead of the generated summary. Graphs +# built under 2 hold summary-derived entities the current rule would never +# produce, and provenance cannot retract them selectively. +CONCEPT_SCHEMA_VERSION = 3 _SCHEMA_VERSION_KEY = "schema_version" @@ -37,8 +40,14 @@ def get_concept_db_path() -> str: return str(index_dir / "marm_index.db") -def init_concept_database(db_path: str) -> None: - """Initialize SQLite database with concept graph tables.""" +def init_concept_database(db_path: str, mark_current: bool = True) -> None: + """Initialize SQLite database with concept graph tables. + + Pass mark_current=False when initializing a graph that still has to be + rebuilt. Writing the version and deleting it again leaves a window where a + crash, or another process reading the schema state, sees an empty graph + reported as current. + """ with sqlite3.connect(db_path) as conn: conn.execute("PRAGMA journal_mode=WAL") conn.execute("PRAGMA foreign_keys=ON") @@ -158,10 +167,16 @@ def init_concept_database(db_path: str) -> None: value TEXT NOT NULL ) """) - conn.execute( - "INSERT OR REPLACE INTO concept_schema_metadata (key, value) VALUES (?, ?)", - (_SCHEMA_VERSION_KEY, str(CONCEPT_SCHEMA_VERSION)), - ) + # Only stamp a database that had no graph before this call. init runs + # on every ConceptDB(...) construction, including the one a console + # memory delete makes, so an unconditional write here would mark a + # stale graph as current and its rebuild would never fire. + if mark_current and "entities" not in existing_tables: + conn.execute( + "INSERT INTO concept_schema_metadata (key, value) VALUES (?, ?) " + "ON CONFLICT(key) DO NOTHING", + (_SCHEMA_VERSION_KEY, str(CONCEPT_SCHEMA_VERSION)), + ) conn.execute(""" CREATE TABLE IF NOT EXISTS concept_build_runs ( id TEXT PRIMARY KEY, @@ -243,6 +258,11 @@ def backup_and_reset_concept_database(db_path: str) -> str: backup.close() source.close() + # The version marker is deliberately NOT written here. Stamping it at + # reset time means a rebuild that dies partway leaves an empty or partial + # graph reporting `current`, so nothing ever prompts for the rebuild again + # and the corpus is silently missing from the graph. mark_schema_current() + # is called by the build once it has actually finished. reset = sqlite3.connect(db_path, timeout=20.0) try: reset.execute("PRAGMA foreign_keys=OFF") @@ -261,10 +281,26 @@ def backup_and_reset_concept_database(db_path: str) -> str: raise finally: reset.close() - init_concept_database(db_path) + init_concept_database(db_path, mark_current=False) return str(backup_path) +def mark_schema_current(db_path: str) -> None: + """Record that this graph was built under the current extraction rules. + + Called only after a full build finishes. Between the reset and this call + the graph reports `rebuild_required`, so an interrupted rebuild is retried + rather than mistaken for a complete one. + """ + with closing(sqlite3.connect(db_path, timeout=20.0)) as conn: + conn.execute( + "INSERT INTO concept_schema_metadata (key, value) VALUES (?, ?) " + "ON CONFLICT(key) DO UPDATE SET value = excluded.value", + (_SCHEMA_VERSION_KEY, str(CONCEPT_SCHEMA_VERSION)), + ) + conn.commit() + + class ConceptDB: """Owns the concept graph's SQLite pool. One instance per process, lazily built.""" diff --git a/marm-mcp-server/marm_mcp_server/core/concept_queue.py b/marm-mcp-server/marm_mcp_server/core/concept_queue.py new file mode 100644 index 00000000..2f3eb8a3 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/concept_queue.py @@ -0,0 +1,345 @@ +"""Durable outbox for concept indexing. + +The queue lives in the memory database so `enqueue` can run inside the same +transaction as the memories INSERT: a memory cannot be stored without its +indexing task, and a rolled-back write leaves no orphan task. Concepts still +live in their own database; only the task list is here. + +Claiming is lease-based rather than lock-based. `_concept_build_lock` in +endpoints/concepts.py is an asyncio.Lock and therefore in-process only, while +an HTTP server and a STDIO session are two processes sharing one memory DB. +The lease in this table is the only thing that stops both from claiming the +same task, and it is also what makes a killed worker recoverable: an expired +lease is reclaimed rather than failed, so a crash costs no attempts. +""" + +import sqlite3 +import uuid +from collections.abc import AsyncIterator, Iterable +from contextlib import asynccontextmanager +from datetime import datetime, timedelta, timezone +from typing import Any, NamedTuple, Optional + +from ..config.settings import ( + CONCEPT_INDEX_DEBOUNCE_SECONDS, + CONCEPT_INDEX_LEASE_SECONDS, + CONCEPT_INDEX_MAX_ATTEMPTS, +) + + +# SQLite's default parameter ceiling is 999. A full-corpus build settles far +# more ids than that in one call. +_DELETE_CHUNK = 500 + + +class ClaimedTask(NamedTuple): + memory_id: str + content_hash: str + lease_token: str + + +def _connection() -> Any: + """Resolved on use, not at import: core.memory imports memory_ops, which + imports this module, so binding the singleton at module level would be a + cycle.""" + from .memory import memory + + return memory.get_connection() + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def enqueue(conn: sqlite3.Connection, memory_id: str, content_hash: str) -> None: + """Queue a memory for indexing on the caller's own connection, inside the + caller's own transaction. + + An upsert, not an insert-or-ignore. A merge reuses an existing memory_id + with new content and a new hash, so an already-queued row must take the + new hash and return to pending. Dedup on memory_id alone would treat + merged content as already indexed and it would never be extracted. + """ + conn.execute( + """ + INSERT INTO concept_index_queue + (memory_id, content_hash, enqueued_at, state, attempts) + VALUES (?, ?, ?, 'pending', 0) + ON CONFLICT(memory_id) DO UPDATE SET + content_hash = excluded.content_hash, + enqueued_at = excluded.enqueued_at, + state = 'pending', + lease_token = NULL, + leased_until = NULL, + attempts = 0, + last_error = NULL + """, + (memory_id, content_hash, _now().isoformat()), + ) + + +def dequeue(conn: sqlite3.Connection, memory_ids: Iterable[str]) -> None: + """Drop tasks for memories that no longer exist, on the caller's + connection. The FK cascade would usually cover this, but it only fires + when foreign keys are enforced on that specific connection, so the delete + is explicit.""" + ids = list(memory_ids) + if not ids: + return + placeholders = ",".join("?" * len(ids)) + conn.execute( + f"DELETE FROM concept_index_queue WHERE memory_id IN ({placeholders})", ids + ) + + +def claim(limit: int) -> list[ClaimedTask]: + """Take ownership of up to `limit` ready tasks. + + Select and update run in one BEGIN IMMEDIATE so two workers cannot read + the same rows before either writes. Parked rows are never returned. + + leased_until means "not claimable before this" in both live states, which + is what lets one predicate cover both cases: a leased row is owned until + it expires, and a row that just failed is backing off until its retry is + due. A fresh enqueue clears it, so new content is never delayed. + """ + if limit < 1: + return [] + now = _now() + token = uuid.uuid4().hex + leased_until = now + timedelta(seconds=CONCEPT_INDEX_LEASE_SECONDS) + + with _connection() as conn: + conn.execute("BEGIN IMMEDIATE") + try: + rows = conn.execute( + """ + SELECT memory_id, content_hash FROM concept_index_queue + WHERE state IN ('pending', 'leased') + AND (leased_until IS NULL OR leased_until <= ?) + ORDER BY enqueued_at + LIMIT ? + """, + (now.isoformat(), limit), + ).fetchall() + if not rows: + conn.execute("COMMIT") + return [] + claimed_ids = [row[0] for row in rows] + placeholders = ",".join("?" * len(claimed_ids)) + conn.execute( + f""" + UPDATE concept_index_queue + SET state = 'leased', lease_token = ?, leased_until = ? + WHERE memory_id IN ({placeholders}) + """, + [token, leased_until.isoformat(), *claimed_ids], + ) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + + return [ClaimedTask(row[0], row[1], token) for row in rows] + + +def renew(memory_ids: Iterable[str], lease_token: str, ttl_seconds: int) -> int: + """Push back the expiry on tasks we still hold. + + The build lock alone is not enough. These leases run on the same clock, so + an extraction that outlives the TTL would let another process reclaim the + very memories being worked on and extract them a second time. + """ + ids = list(memory_ids) + if not ids: + return 0 + now = _now() + leased_until = (now + timedelta(seconds=ttl_seconds)).isoformat() + placeholders = ",".join("?" * len(ids)) + with _connection() as conn: + cursor = conn.execute( + f"UPDATE concept_index_queue SET leased_until = ? " + f"WHERE memory_id IN ({placeholders}) AND lease_token = ?", + [leased_until, *ids, lease_token], + ) + return max(int(cursor.rowcount), 0) + + +@asynccontextmanager +async def keep_claimed( + tasks: "list[ClaimedTask]", ttl_seconds: int +) -> AsyncIterator[None]: + """Hold a batch's leases for as long as the body runs.""" + import asyncio + + from .concept_build_lock import heartbeat_interval + + if not tasks: + yield + return + memory_ids = [task.memory_id for task in tasks] + token = tasks[0].lease_token + + async def _keep_alive() -> None: + interval = heartbeat_interval(ttl_seconds) + while True: + await asyncio.sleep(interval) + try: + await asyncio.to_thread(renew, memory_ids, token, ttl_seconds) + except Exception: + # A missed renewal is recoverable: the task returns to the + # queue and is picked up again. Failing the batch is not. + pass + + beat = asyncio.create_task(_keep_alive()) + try: + yield + finally: + beat.cancel() + try: + await beat + except (asyncio.CancelledError, Exception): + pass + + +def complete(memory_id: str, lease_token: str, content_hash: str) -> bool: + """Retire a finished task. Returns whether it was actually retired. + + Matched on the hash as well as the token: a memory merged while its + extraction was running was re-enqueued with new content, and deleting that + row would drop the merged text from the graph permanently. + """ + with _connection() as conn: + cursor = conn.execute( + "DELETE FROM concept_index_queue " + "WHERE memory_id = ? AND lease_token = ? AND content_hash = ?", + (memory_id, lease_token, content_hash), + ) + return bool(cursor.rowcount > 0) + + +def drop(memory_id: str, lease_token: str) -> bool: + """Retire a task whose memory is gone, regardless of hash. Used for + vanished memories, where there is no current content to match against.""" + with _connection() as conn: + cursor = conn.execute( + "DELETE FROM concept_index_queue WHERE memory_id = ? AND lease_token = ?", + (memory_id, lease_token), + ) + return bool(cursor.rowcount > 0) + + +def fail(memory_id: str, lease_token: str, error: str) -> bool: + """Record a failed attempt, release the lease, and hold the task back. + + The backoff is not optional. The worker drains continuously until the + queue is empty, so a task returned straight to pending would be re-claimed + by the very next iteration and burn every attempt in milliseconds. One + debounce interval per attempt spreads the retries over real time instead. + + A task that reaches CONCEPT_INDEX_MAX_ATTEMPTS is parked rather than + deleted: the queue keeps moving, and the row stays as evidence of what + could not be indexed and why. + """ + now = _now() + with _connection() as conn: + conn.execute("BEGIN IMMEDIATE") + try: + row = conn.execute( + "SELECT attempts FROM concept_index_queue " + "WHERE memory_id = ? AND lease_token = ?", + (memory_id, lease_token), + ).fetchone() + if row is None: + conn.execute("COMMIT") + return False + attempts = row[0] + 1 + parked = attempts >= CONCEPT_INDEX_MAX_ATTEMPTS + retry_at = now + timedelta( + seconds=CONCEPT_INDEX_DEBOUNCE_SECONDS * attempts + ) + conn.execute( + """ + UPDATE concept_index_queue + SET attempts = ?, last_error = ?, lease_token = NULL, + leased_until = ?, state = ? + WHERE memory_id = ? AND lease_token = ? + """, + ( + attempts, + error[:500], + None if parked else retry_at.isoformat(), + "parked" if parked else "pending", + memory_id, + lease_token, + ), + ) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + return True + + +def retire_indexed(memory_ids: Iterable[str], queued_before: str) -> int: + """Drop tasks a completed build has already covered. + + Called after a build with the ids it actually indexed, and the timestamp + the build started. `queued_before` is what makes this safe without + comparing content: a merge, replace, or fresh write during the build + re-stamps enqueued_at to now, so those rows sort after the cutoff and + survive. A memory whose extraction failed is not in memory_ids at all, so + its retry is never dropped. Leased rows belong to another worker. + + Without this, the forced rebuild in v2.36.0 would leave the whole corpus + queued behind a build that just indexed it, and the worker would extract + every memory a second time. + """ + ids = list(memory_ids) + if not ids: + return 0 + total = 0 + with _connection() as conn: + for start in range(0, len(ids), _DELETE_CHUNK): + chunk = ids[start : start + _DELETE_CHUNK] + placeholders = ",".join("?" * len(chunk)) + cursor = conn.execute( + f"DELETE FROM concept_index_queue WHERE memory_id IN ({placeholders}) " + "AND state != 'leased' AND enqueued_at <= ?", + [*chunk, queued_before], + ) + total += max(cursor.rowcount, 0) + return total + + +def current_hashes(memory_ids: Iterable[str]) -> dict[str, Optional[str]]: + """Read each memory's live content_hash, for the worker's post-write + ownership check. A missing key means the memory no longer exists.""" + ids = list(memory_ids) + if not ids: + return {} + placeholders = ",".join("?" * len(ids)) + with _connection() as conn: + rows = conn.execute( + f"SELECT id, content_hash FROM memories WHERE id IN ({placeholders})", ids + ).fetchall() + return {row[0]: row[1] for row in rows} + + +def counts() -> dict[str, int]: + """Queue depth for status reporting. + + Two numbers rather than one: a growing pending count means indexing is + behind or switched off, while a non-zero parked count means specific + memories gave up and will not be retried without help. They call for + different responses, so they are not summed. + """ + with _connection() as conn: + rows = conn.execute( + "SELECT state, COUNT(*) FROM concept_index_queue GROUP BY state" + ).fetchall() + by_state = {row[0]: int(row[1]) for row in rows} + return { + "pending": by_state.get("pending", 0) + by_state.get("leased", 0), + "parked": by_state.get("parked", 0), + } diff --git a/marm-mcp-server/marm_mcp_server/core/concept_worker.py b/marm-mcp-server/marm_mcp_server/core/concept_worker.py new file mode 100644 index 00000000..e5891804 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/concept_worker.py @@ -0,0 +1,317 @@ +"""Background worker that turns stored memories into concept graph nodes. + +Runs in-process on both transports. It owns no state that matters: every task +is a durable row in concept_index_queue, so a killed worker loses nothing and +teardown only has to stop it, never wait for it. That is the opposite of the +chunk drain in memory_utils, which must finish because its work exists only +in RAM. + +Failure here degrades the graph and never the memory. Extraction problems +retry, a poison memory parks after CONCEPT_INDEX_MAX_ATTEMPTS, and no path +from this module can block a write or a recall. +""" + +import asyncio +import threading +from typing import Optional + +import structlog + +from ..config.settings import ( + CONCEPT_AUTO_INDEX, + CONCEPT_INDEX_BATCH_PAUSE_MS, + CONCEPT_INDEX_BATCH_SIZE, + CONCEPT_INDEX_DEBOUNCE_SECONDS, + CONCEPT_INDEX_LEASE_SECONDS, + CONCEPTS_AVAILABLE, +) +from . import concept_queue +from .concept_build_lock import BuildLease, ConceptBuildBusy, concept_build_lock + +logger = structlog.get_logger(__name__) + +# How long stop() waits for an aborted extraction to actually stop before +# releasing the graph anyway. Short on purpose: teardown must stay bounded. +ABORT_GRACE_SECONDS = 2.0 + + +class ConceptIndexWorker: + """Lazy singleton, mirroring graph_supervisor's shape for optional + subsystems. start() and stop() are both idempotent.""" + + def __init__(self) -> None: + self._task: Optional[asyncio.Task] = None + self._stop = asyncio.Event() + self._active_lease: Optional[BuildLease] = None + self._build_finished: Optional[threading.Event] = None + self._cycles = 0 + self._indexed = 0 + + @property + def running(self) -> bool: + return self._task is not None and not self._task.done() + + def start(self) -> None: + """Never raises. A worker that cannot run leaves the queue filling, + which is recoverable; a worker that breaks startup is not.""" + if self.running: + return + if not CONCEPT_AUTO_INDEX: + logger.info("concept_worker.disabled", reason="CONCEPT_AUTO_INDEX=false") + return + if not CONCEPTS_AVAILABLE: + # Dormant, not spinning. Claiming tasks we cannot extract would + # burn the attempt budget and park every memory written while the + # extraction runtime is missing. + logger.info("concept_worker.dormant", reason="concepts_unavailable") + return + try: + self._stop.clear() + self._task = asyncio.get_running_loop().create_task(self._run()) + logger.info( + "concept_worker.started", + debounce_seconds=CONCEPT_INDEX_DEBOUNCE_SECONDS, + batch_size=CONCEPT_INDEX_BATCH_SIZE, + ) + except RuntimeError as exc: + logger.warning("concept_worker.start_failed", error=str(exc)) + + async def stop(self) -> None: + """Signal and return. Deliberately does not wait for an in-flight + extraction: the task is a durable row and the next run picks it up, + so waiting would only reintroduce the shutdown delay v2.35.0 bounded. + + Signals the in-flight build first. Cancelling the task only cancels + the await around asyncio.to_thread; the extraction thread keeps + running and keeps writing, while unwinding releases the cross-process + graph lock. Another transport could then take the lock and reset the + concept database underneath that thread. Raising the flag first makes + the thread stop at its next memory instead, which costs no shutdown + time because it is not awaited.""" + lease = self._active_lease + in_flight = self._build_finished + if lease is not None: + lease.lost.set() + self._stop.set() + + if in_flight is not None and not in_flight.is_set(): + # Bounded, and it has to be. Cancelling below unwinds the lock and + # releases it, but the extraction thread cannot be cancelled and + # keeps writing until it notices the flag above. Releasing the graph + # while that is true lets another process rebuild underneath it. + # + # Waiting the whole extraction out would put spaCy back on the + # teardown path, which v2.35.0 deliberately bounded, so this waits + # only for the flag to land and then proceeds regardless. Past the + # grace period the worst case is stray entities or a logged write + # failure in a graph another process now owns, not corruption. + try: + await asyncio.wait_for( + asyncio.to_thread(in_flight.wait, ABORT_GRACE_SECONDS), + timeout=ABORT_GRACE_SECONDS + 1, + ) + except (asyncio.TimeoutError, TimeoutError): + pass + if not in_flight.is_set(): + logger.warning( + "concept_worker.extraction_still_running", + grace_seconds=ABORT_GRACE_SECONDS, + ) + + task = self._task + self._task = None + if task is None or task.done(): + return + task.cancel() + try: + await task + except asyncio.CancelledError: + pass + except Exception as exc: + logger.warning("concept_worker.stop_error", error=str(exc)) + logger.info("concept_worker.stopped", cycles=self._cycles) + + async def _run(self) -> None: + while not self._stop.is_set(): + await self._wait(CONCEPT_INDEX_DEBOUNCE_SECONDS) + if self._stop.is_set(): + return + self._cycles += 1 + try: + await self._drain() + except Exception as exc: + # One bad cycle must never end the loop. The queue is durable, + # so the next cycle retries whatever was left. + logger.warning("concept_worker.cycle_failed", error=str(exc)) + + async def _wait(self, seconds: float) -> bool: + """Sleep unless stopped first. Returns whether a stop arrived.""" + try: + await asyncio.wait_for(self._stop.wait(), timeout=seconds) + return True + except (asyncio.TimeoutError, TimeoutError): + return False + + async def _drain(self) -> None: + """Claim and process back to back until the queue is empty. + + Waiting between batches instead would cap throughput at one batch per + debounce interval, which on the defaults is 40 memories a minute: a + backlog would never catch up. + + The cross-process lock is taken per batch rather than around the whole + drain, so a manual build can get in between batches instead of waiting + out a long backlog. Taking it before claiming means a busy cycle leaves + nothing claimed and nothing stranded behind an unused lease. + """ + while not self._stop.is_set(): + try: + async with concept_build_lock( + "auto_index", CONCEPT_INDEX_LEASE_SECONDS + ) as lease: + # Published so stop() can signal a build that is already + # running in a thread it cannot cancel. + self._active_lease = lease + tasks = await asyncio.to_thread( + concept_queue.claim, CONCEPT_INDEX_BATCH_SIZE + ) + if not tasks: + return + # The task leases run on the same clock as the build lock, + # so a batch that outlives the TTL would be reclaimed and + # extracted a second time by another process. + build_finished = threading.Event() + self._build_finished = build_finished + try: + async with concept_queue.keep_claimed( + tasks, CONCEPT_INDEX_LEASE_SECONDS + ): + await self._process(tasks, lease.lost, build_finished) + finally: + self._active_lease = None + self._build_finished = None + if lease.lost.is_set(): + return + except ConceptBuildBusy: + logger.info("concept_worker.deferred", reason="graph_busy") + return + + # After the lock is released, so the pause also hands a waiting + # manual build a clean window rather than only yielding CPU. + if CONCEPT_INDEX_BATCH_PAUSE_MS and await self._wait( + CONCEPT_INDEX_BATCH_PAUSE_MS / 1000 + ): + return + + async def _process( + self, + tasks: list[concept_queue.ClaimedTask], + abort: Optional[threading.Event] = None, + finished: Optional[threading.Event] = None, + ) -> None: + from ..endpoints.concepts import build_for_memory_ids + + memory_ids = [task.memory_id for task in tasks] + outcomes = await build_for_memory_ids( + memory_ids, abort=abort, finished=finished + ) + + if abort is not None and abort.is_set(): + # The graph belongs to another process now and this batch stopped + # partway. Settling any of it would either delete a task whose + # extraction never ran or spend an attempt on a memory that never + # failed. Leave every lease to expire and be retried. + logger.error("concept_worker.batch_abandoned", tasks=len(tasks)) + return + + live = await asyncio.to_thread(concept_queue.current_hashes, memory_ids) + + for task in tasks: + if task.memory_id not in live: + await self._retract(task, "deleted") + await asyncio.to_thread( + concept_queue.drop, task.memory_id, task.lease_token + ) + continue + # A NULL stored hash cannot be compared, and treating it as a + # mismatch is worse than not checking: the task is never settled + # and never counted as a failure, so the worker re-extracts that + # memory forever without ever writing it or giving up. Rows + # predating the content_hash column are exactly this case. + if live[task.memory_id] is not None and ( + live[task.memory_id] != task.content_hash + ): + # Content changed under us. The write that changed it already + # re-queued the memory, so leave that row alone and let the + # next cycle index the current content. + # + # Deliberately no retraction here. cleanup_deleted_memory_ + # provenance removes ALL provenance for a memory id, not just + # what this build wrote, so retracting would also erase what a + # worker in the other process may have already written for the + # new content, leaving the graph empty for this memory with no + # queue row left to repair it. The cost of not retracting is + # entities from the previous text lingering, which is the + # staleness this feature already documents. + logger.info("concept_worker.superseded", memory_id=task.memory_id) + continue + + outcome = outcomes.get(task.memory_id, "failed") + if outcome in ("indexed", "no_entities"): + await asyncio.to_thread( + concept_queue.complete, + task.memory_id, + task.lease_token, + task.content_hash, + ) + if outcome == "indexed": + self._indexed += 1 + elif outcome == "vanished": + await asyncio.to_thread( + concept_queue.drop, task.memory_id, task.lease_token + ) + else: + await asyncio.to_thread( + concept_queue.fail, + task.memory_id, + task.lease_token, + "extraction_failed", + ) + + async def _retract(self, task: concept_queue.ClaimedTask, reason: str) -> None: + """Undo provenance for a memory that was deleted while its extraction + was in flight. + + Dequeue-on-delete alone cannot cover this. The build reads the memory + DB and then writes the concept DB, and a delete can commit and run its + own concept cleanup inside that gap, so without this the entities the + user asked to be gone reappear after the cleanup that was supposed to + remove them. + + Only for deletes. The memory is gone, so no other worker can be + indexing it and there is nothing here worth keeping. + """ + from ..endpoints.concepts import _get_concept_db + + try: + await asyncio.to_thread( + lambda: _get_concept_db().cleanup_deleted_memory_provenance( + [task.memory_id] + ) + ) + logger.info( + "concept_worker.retracted", memory_id=task.memory_id, reason=reason + ) + except Exception as exc: + logger.warning("concept_worker.retract_failed", error=str(exc)) + + def status(self) -> dict: + return { + "running": self.running, + "enabled": CONCEPT_AUTO_INDEX, + "cycles": self._cycles, + "memories_indexed": self._indexed, + } + + +concept_worker = ConceptIndexWorker() diff --git a/marm-mcp-server/marm_mcp_server/core/memory_db.py b/marm-mcp-server/marm_mcp_server/core/memory_db.py index 6e301908..54ebe5b4 100644 --- a/marm-mcp-server/marm_mcp_server/core/memory_db.py +++ b/marm-mcp-server/marm_mcp_server/core/memory_db.py @@ -378,6 +378,41 @@ def init_database(db_path: str) -> None: " ON memory_chunks(memory_id, chunk_index)" ) + # Durable outbox for concept indexing. Lives in the memory DB, not the + # concept DB, so the enqueue can join the same transaction as the + # memories INSERT and a memory can never exist without its task. + conn.execute(""" + CREATE TABLE IF NOT EXISTS concept_index_queue ( + memory_id TEXT PRIMARY KEY, + content_hash TEXT NOT NULL, + enqueued_at TEXT NOT NULL, + state TEXT NOT NULL DEFAULT 'pending', + lease_token TEXT, + leased_until TEXT, + attempts INTEGER NOT NULL DEFAULT 0, + last_error TEXT, + FOREIGN KEY (memory_id) REFERENCES memories(id) ON DELETE CASCADE + ) + """) + conn.execute( + "CREATE INDEX IF NOT EXISTS idx_concept_queue_ready" + " ON concept_index_queue(state, leased_until, enqueued_at)" + ) + + # One row, held by whoever is currently writing the concept graph. + # asyncio locks cannot reach across processes, and HTTP and STDIO are + # two processes: without this, a full rebuild can drop the graph tables + # while the other process's worker is writing to them. + conn.execute(""" + CREATE TABLE IF NOT EXISTS concept_build_lock ( + id INTEGER PRIMARY KEY CHECK (id = 1), + holder TEXT NOT NULL, + purpose TEXT NOT NULL, + acquired_at TEXT NOT NULL, + expires_at TEXT NOT NULL + ) + """) + conn.execute("DROP TABLE IF EXISTS session_summary_chunks") conn.execute(""" diff --git a/marm-mcp-server/marm_mcp_server/core/memory_delete.py b/marm-mcp-server/marm_mcp_server/core/memory_delete.py index 59b0b0e3..4cdca18f 100644 --- a/marm-mcp-server/marm_mcp_server/core/memory_delete.py +++ b/marm-mcp-server/marm_mcp_server/core/memory_delete.py @@ -3,6 +3,8 @@ import json from datetime import datetime, timezone +from .concept_queue import dequeue as dequeue_concept_index + async def _delete_memory(mem, memory_id: str) -> bool: result = await _delete_memories(mem, [memory_id]) @@ -168,6 +170,10 @@ async def _delete_memories(mem, memory_ids: list[str]) -> dict: ) for memory_id in existing_ids: conn.execute("DELETE FROM memory_chunks WHERE memory_id = ?", (memory_id,)) + # Same transaction as the delete. A task left behind points at a + # memory that no longer exists, and the worker would keep claiming it + # until it burned the attempt budget and parked it. + dequeue_concept_index(conn, existing_ids) delete_cursor = conn.execute( f"DELETE FROM memories WHERE id IN ({placeholders})", list(existing_ids), diff --git a/marm-mcp-server/marm_mcp_server/core/memory_ops.py b/marm-mcp-server/marm_mcp_server/core/memory_ops.py index 5e0ce0d0..da3c3e6e 100644 --- a/marm-mcp-server/marm_mcp_server/core/memory_ops.py +++ b/marm-mcp-server/marm_mcp_server/core/memory_ops.py @@ -12,6 +12,7 @@ MARM_PLATFORM, MARM_PROJECT, ) +from .concept_queue import enqueue as enqueue_concept_index from .consolidation import ( compute_content_hash, find_exact_duplicate, @@ -122,6 +123,11 @@ async def _update_memory(mem, memory_id: str, new_content: str) -> bool: # disagree with the (already committed) merged content, and # vice versa. conn.execute("DELETE FROM memory_chunks WHERE memory_id = ?", (memory_id,)) + # The merge reuses this memory_id with new content, so the queue + # row is upserted onto the recomputed hash. Keyed on the id alone + # it would read as already indexed and the merged text would never + # reach the graph. + enqueue_concept_index(conn, memory_id, merged_hash) conn.execute("COMMIT") except Exception: conn.execute("ROLLBACK") @@ -261,6 +267,11 @@ async def _store_memory( ), ) + # Same transaction as the INSERT above, on purpose. A memory that + # exists without an indexing task is a memory the graph never learns + # about, and nothing later would notice the gap. + enqueue_concept_index(conn, memory_id, content_hash) + conn.execute( """ INSERT INTO sessions (session_name, last_accessed) @@ -347,6 +358,9 @@ async def _replace_memory( (session, timestamp), ) conn.execute("DELETE FROM memory_chunks WHERE memory_id = ?", (memory_id,)) + # A console edit replaces the content wholesale, so the graph holds + # entities from text that no longer exists until this is re-indexed. + enqueue_concept_index(conn, memory_id, content_hash) chunks = _chunk_text( sanitized_content, threshold=MEMORY_CHUNK_THRESHOLD_WORDS, @@ -467,6 +481,9 @@ async def _store_doc_mirror( (session, timestamp), ) conn.execute("DELETE FROM memory_chunks WHERE memory_id = ?", (memory_id,)) + # A promoted doc's mirror row is an ordinary memory to the graph, + # and a resave changes its content in place. + enqueue_concept_index(conn, memory_id, content_hash) conn.execute("COMMIT") except Exception: conn.execute("ROLLBACK") diff --git a/marm-mcp-server/marm_mcp_server/core/shutdown_manager.py b/marm-mcp-server/marm_mcp_server/core/shutdown_manager.py index b073fd9a..af1a98ed 100644 --- a/marm-mcp-server/marm_mcp_server/core/shutdown_manager.py +++ b/marm-mcp-server/marm_mcp_server/core/shutdown_manager.py @@ -4,6 +4,7 @@ import structlog from ..config.settings import CHUNK_DRAIN_TIMEOUT_SECONDS +from .concept_worker import concept_worker from .graph_supervisor import graph_supervisor from .memory import memory from .memory_utils import drain_chunk_writes @@ -67,6 +68,14 @@ async def graceful_shutdown(self): except Exception: logger.exception("Failed to cancel pending compaction scans") + # Before the write queue stops and the pools begin closing: the worker + # produces concept writes and reads the memory DB, and stopping it only + # signals, it does not wait for an in-flight extraction. + try: + await concept_worker.stop() + except Exception: + logger.exception("Failed to stop concept index worker") + try: await memory.stop_write_queue() logger.info("Serialized write queue drained") diff --git a/marm-mcp-server/marm_mcp_server/endpoints/concepts.py b/marm-mcp-server/marm_mcp_server/endpoints/concepts.py index 2af938b4..86516bd9 100644 --- a/marm-mcp-server/marm_mcp_server/endpoints/concepts.py +++ b/marm-mcp-server/marm_mcp_server/endpoints/concepts.py @@ -9,9 +9,11 @@ """ import asyncio +import itertools import threading import time import uuid +from collections.abc import Iterable, Iterator from datetime import datetime, timezone from typing import Optional @@ -28,6 +30,13 @@ backup_and_reset_concept_database, get_concept_db_path, inspect_concept_schema, + mark_schema_current, +) +from ..core import concept_queue +from ..core.concept_build_lock import ( + MANUAL_BUILD_LOCK_SECONDS, + ConceptBuildBusy, + concept_build_lock, ) from ..core.concept_extraction import extract_entities from ..core.graph_client import find_code_match, is_graph_available @@ -44,6 +53,21 @@ _concept_db_lock = threading.Lock() _concept_build_lock = asyncio.Lock() +MemoryRow = tuple[str, str, Optional[str], Optional[str], Optional[str]] +MemoryPage = list[MemoryRow] + +# Shared by the scope path and the targeted path so the two can never drift +# into indexing different corpora. A compaction source owns its concepts and +# the generated summary restates them, so extracting both double-counts every +# entity in a compacted session. Recall makes the opposite choice on purpose: +# it wants the summary, the graph wants the originals. +_BUILD_ROW_FILTERS = ( + "session_name != 'marm_system'", + "content IS NOT NULL", + "content != ''", + "(compaction_role IS NULL OR compaction_role != 'summary')", +) + def _get_concept_db() -> ConceptDB: """Lazy singleton, mirrors memory.py's own lazy-init style for optional @@ -72,25 +96,21 @@ def _get_concept_db() -> ConceptDB: ) -def _fetch_memory_rows( +def _fetch_memory_pages( session_name: Optional[str], project: Optional[str], search_all: bool -) -> list[tuple[str, str, Optional[str], Optional[str], Optional[str]]]: - """Deterministic row-bounded read from the memory DB layer directly. - Never goes through marm_smart_recall — no ranking/truncation here. +) -> Iterator[MemoryPage]: + """Deterministic read from the memory DB layer directly, paged rather + than truncated. Never goes through marm_smart_recall — no ranking here. Requires at least one of session_name/project/search_all -- without this, an omitted session_name silently fell through to scanning every memory - in the DB (up to the row cap), making search_all's "explicit opt-in for - everything" meaningless.""" + in the DB, making search_all's "explicit opt-in for everything" + meaningless. Validated before the generator is built so the caller still + gets the ValueError at call time, not on first iteration.""" if not (session_name or project or search_all): raise ValueError(_MISSING_BUILD_SCOPE_MESSAGE) - conditions = [ - "session_name != 'marm_system'", - "content IS NOT NULL", - "content != ''", - "(compaction_role IS NULL OR compaction_role != 'source')", - ] + conditions = list(_BUILD_ROW_FILTERS) params: list = [] if not search_all: @@ -101,14 +121,57 @@ def _fetch_memory_rows( conditions.append("project = ?") params.append(project) + return _paged_memory_rows(conditions, params) + + +def _paged_memory_rows(conditions: list[str], params: list) -> Iterator[MemoryPage]: + """Keyset-paginated scan, CONCEPT_BUILD_ROW_CAP rows per page. + + Keyed on (created_at, id), never created_at alone: created_at defaults to + CURRENT_TIMESTAMP, which is second-granular, so a burst of writes shares + one value and an OFFSET or single-column keyset would skip or repeat rows + at every page boundary. Descending order means rows written during a long + build sort ahead of the cursor and are never revisited.""" + cursor: Optional[tuple[str, str]] = None + while True: + page_conditions = list(conditions) + page_params = list(params) + if cursor is not None: + page_conditions.append("(created_at < ? OR (created_at = ? AND id < ?))") + page_params.extend((cursor[0], cursor[0], cursor[1])) + + query = ( + "SELECT id, content, session_name, project, platform, created_at " + f"FROM memories WHERE {' AND '.join(page_conditions)} " + "ORDER BY created_at DESC, id DESC LIMIT ?" + ) + page_params.append(CONCEPT_BUILD_ROW_CAP) + + with memory.get_connection() as conn: + page = conn.execute(query, page_params).fetchall() + + if not page: + return + cursor = (page[-1][5], page[-1][0]) + yield [row[:5] for row in page] + if len(page) < CONCEPT_BUILD_ROW_CAP: + return + + +def _fetch_memory_rows_by_ids(memory_ids: list[str]) -> MemoryPage: + """Targeted read for the incremental path. No pagination: the caller + passes a bounded batch. Applies the same filters as a scope build, so an + id that now points at a summary, an emptied row, or nothing at all simply + comes back missing and the caller settles it as vanished.""" + if not memory_ids: + return [] + placeholders = ",".join("?" * len(memory_ids)) query = ( - f"SELECT id, content, session_name, project, platform FROM memories " - f"WHERE {' AND '.join(conditions)} ORDER BY created_at DESC LIMIT ?" + "SELECT id, content, session_name, project, platform FROM memories " + f"WHERE id IN ({placeholders}) AND {' AND '.join(_BUILD_ROW_FILTERS)}" ) - params.append(CONCEPT_BUILD_ROW_CAP) - with memory.get_connection() as conn: - return conn.execute(query, params).fetchall() + return conn.execute(query, memory_ids).fetchall() def _try_embed(name: str) -> Optional[bytes]: @@ -131,9 +194,30 @@ def _try_embed(name: str) -> Optional[bytes]: def _run_build( - rows: list[tuple[str, str, Optional[str], Optional[str], Optional[str]]], + pages: Iterable[MemoryPage], + outcomes: Optional[dict[str, str]] = None, + abort: Optional[threading.Event] = None, + finished: Optional[threading.Event] = None, ) -> dict: + """Consumes pages lazily so a full-corpus build never holds every row in + memory at once. The concept connection and embed_cache stay outside the + page loop, so both still span the whole build. + + Pass a dict as outcomes to have each memory's result recorded in it. The + aggregate counters cannot carry that: a build in which every extraction + failed returns success with zeros, and a caller settling queue tasks on + that would delete work it promised to retry. It is an out-parameter rather + than a return value so it cannot end up in the route's response, where a + full build would attach one entry per memory to a 1MB-bounded reply. + + Pass an abort event to make the build stoppable between memories. A + process stalled longer than its whole lease loses the cross-process lock, + and a thread already running cannot be killed from outside; this is how it + stops writing alongside the new owner rather than running to completion. + The result carries `aborted` so the caller settles nothing.""" concept_db = _get_concept_db() + memories_processed = 0 + aborted = False entities_extracted = 0 relationships_created = 0 code_links_created = 0 @@ -148,103 +232,136 @@ def _run_build( # is always safe. embed_cache: dict[str, Optional[bytes]] = {} - with concept_db.get_connection() as conn: - for row in rows: - mem_id, content, mem_session, mem_project = row[:4] - mem_platform = row[4] if len(row) > 4 else None - try: - result = extract_entities(content) - except Exception as e: - _safe_print(f"Concept extraction failed for memory {mem_id}: {e}") - continue - - name_to_id: dict[str, int] = {} - for entity in result.entities: - if entity.name not in embed_cache: - embed_cache[entity.name] = _try_embed(entity.name) - emb_bytes = embed_cache[entity.name] + try: + with concept_db.get_connection() as conn: + for row in itertools.chain.from_iterable(pages): + if abort is not None and abort.is_set(): + aborted = True + break + mem_id, content, mem_session, mem_project = row[:4] + mem_platform = row[4] if len(row) > 4 else None + memories_processed += 1 try: - entity_id, was_created = concept_db.get_or_create_entity( - conn, - entity.name, - entity.type, - mem_session, - mem_project, - mem_id, - name_embedding=emb_bytes, - platform=mem_platform, - ) + result = extract_entities(content) except Exception as e: - _safe_print(f"Concept entity write failed for memory {mem_id}: {e}") + _safe_print(f"Concept extraction failed for memory {mem_id}: {e}") + if outcomes is not None: + outcomes[mem_id] = "failed" continue - name_to_id[entity.name] = entity_id - entities_extracted += 1 - if was_created and emb_bytes is not None: + memory_failed = False + name_to_id: dict[str, int] = {} + for entity in result.entities: + if entity.name not in embed_cache: + embed_cache[entity.name] = _try_embed(entity.name) + emb_bytes = embed_cache[entity.name] try: - candidates = concept_db.find_similar_entities( + entity_id, was_created = concept_db.get_or_create_entity( conn, - emb_bytes, + entity.name, + entity.type, mem_session, mem_project, - CONCEPT_DUPLICATE_SIMILARITY_THRESHOLD, - exclude_id=entity_id, + mem_id, + name_embedding=emb_bytes, platform=mem_platform, ) except Exception as e: - _safe_print(f"Concept duplicate-candidate scan failed: {e}") - candidates = [] - if candidates: - possible_duplicates.append( - {"entity": entity.name, "candidates": candidates} + _safe_print( + f"Concept entity write failed for memory {mem_id}: {e}" ) + memory_failed = True + continue + name_to_id[entity.name] = entity_id + entities_extracted += 1 - for name_a, name_b, predicate in result.relationship_pairs: - id_a = name_to_id.get(name_a) - id_b = name_to_id.get(name_b) - if id_a is None or id_b is None: - continue - try: - if concept_db.store_relationship( - conn, - id_a, - id_b, - predicate, - mem_id, - mem_project, - platform=mem_platform, - ): - relationships_created += 1 - except Exception as e: - _safe_print( - f"Concept relationship write failed for memory {mem_id}: {e}" - ) - - if graph_available: - for entity in result.entities: - entity_id = name_to_id.get(entity.name) - if entity_id is None: + if was_created and emb_bytes is not None: + try: + candidates = concept_db.find_similar_entities( + conn, + emb_bytes, + mem_session, + mem_project, + CONCEPT_DUPLICATE_SIMILARITY_THRESHOLD, + exclude_id=entity_id, + platform=mem_platform, + ) + except Exception as e: + _safe_print(f"Concept duplicate-candidate scan failed: {e}") + candidates = [] + if candidates: + possible_duplicates.append( + {"entity": entity.name, "candidates": candidates} + ) + + for name_a, name_b, predicate in result.relationship_pairs: + id_a = name_to_id.get(name_a) + id_b = name_to_id.get(name_b) + if id_a is None or id_b is None: continue try: - match = find_code_match(entity.name, mem_project) + if concept_db.store_relationship( + conn, + id_a, + id_b, + predicate, + mem_id, + mem_project, + platform=mem_platform, + ): + relationships_created += 1 except Exception as e: - _safe_print(f"Concept code-link lookup failed: {e}") - continue - if match: + _safe_print( + f"Concept relationship write failed for memory {mem_id}: {e}" + ) + memory_failed = True + + if graph_available: + for entity in result.entities: + entity_id = name_to_id.get(entity.name) + if entity_id is None: + continue try: - if concept_db.store_code_link( - conn, - entity_id, - match["qualified_name"], - mem_project or "", - label=match.get("label"), - file_path=match.get("file_path"), - ): - code_links_created += 1 + match = find_code_match(entity.name, mem_project) except Exception as e: - _safe_print(f"Concept code-link write failed: {e}") + _safe_print(f"Concept code-link lookup failed: {e}") + continue + if match: + try: + if concept_db.store_code_link( + conn, + entity_id, + match["qualified_name"], + mem_project or "", + label=match.get("label"), + file_path=match.get("file_path"), + ): + code_links_created += 1 + except Exception as e: + _safe_print(f"Concept code-link write failed: {e}") + + if outcomes is not None: + # Code links are deliberately absent from this decision. The + # graph engine is optional and its lookups already fail open, + # so a missing link is degradation, not a reason to re-extract + # the memory. + if memory_failed: + outcomes[mem_id] = "failed" + elif not result.entities: + outcomes[mem_id] = "no_entities" + else: + outcomes[mem_id] = "indexed" + + finally: + # However this exits, the thread has stopped writing the graph. The + # caller cannot observe that any other way: cancelling the await + # around asyncio.to_thread does not stop the thread. + if finished is not None: + finished.set() return { + "aborted": aborted, + "memories_processed": memories_processed, "entities_extracted": entities_extracted, "relationships_created": relationships_created, "code_links_created": code_links_created, @@ -347,15 +464,33 @@ async def marm_concept_build(req: ConceptBuildRequest) -> dict: """🕸️ Extract entities/relationships from memory content into the concept graph. Scope with session_name or project for a targeted build, or pass - search_all=True for everything (row-capped). Links extracted entities to + search_all=True for everything. Links extracted entities to marm-graph code symbols when available. Call this before marm_concept_recall — there's no data until a build has run at least once. """ - async with _concept_build_lock: - return await _marm_concept_build(req) + # Cross-process lock outside the in-process one, in both writers, so the + # pair can never be taken in opposite orders. A rebuild drops the graph + # tables, and the other transport's worker must not be writing into them. + try: + async with concept_build_lock( + "manual_build", MANUAL_BUILD_LOCK_SECONDS + ) as lease: + async with _concept_build_lock: + return await _marm_concept_build(req, lease.lost) + except ConceptBuildBusy: + return { + "status": "error", + "error_code": "build_in_progress", + "message": ( + "Another MARM process is writing the concept graph. " + "Wait for it to finish and run this again." + ), + } -async def _marm_concept_build(req: ConceptBuildRequest) -> dict: +async def _marm_concept_build( + req: ConceptBuildRequest, abort: Optional[threading.Event] = None +) -> dict: if not (req.session_name or req.project or req.search_all): return {"status": "error", "message": _MISSING_BUILD_SCOPE_MESSAGE} @@ -431,12 +566,11 @@ async def _marm_concept_build(req: ConceptBuildRequest) -> dict: status="running", started_at=datetime.now(timezone.utc).isoformat(), ) - rows = await asyncio.to_thread( - _fetch_memory_rows, req.session_name, req.project, req.search_all - ) - result = await asyncio.to_thread(_run_build, rows) + pages = _fetch_memory_pages(req.session_name, req.project, req.search_all) + outcomes: dict[str, str] = {} + result = await asyncio.to_thread(_run_build, pages, outcomes, abort) except ValueError: - # _fetch_memory_rows raises exactly one ValueError, always this + # _fetch_memory_pages raises exactly one ValueError, always this # static, safe-to-surface message -- return the known-good literal # rather than str(e), so the response never carries a live exception # object (CodeQL: exception-info-exposure) even if this branch is @@ -473,11 +607,35 @@ async def _marm_concept_build(req: ConceptBuildRequest) -> dict: } result["duration_ms"] = int((time.monotonic() - start) * 1000) + if result["aborted"]: + # Another process owns the graph now. The partial work stays, since + # extraction is idempotent, but nothing here may be reported as done + # and no queue row may be retired against it. + await asyncio.to_thread( + _finish_build_run, + run_id, + status="error", + error_code="lock_lost", + memories_processed=result["memories_processed"], + duration_ms=result["duration_ms"], + finished_at=datetime.now(timezone.utc).isoformat(), + ) + return { + "status": "error", + "error_code": "lock_lost", + "message": ( + "This build lost the concept graph to another MARM process and " + "stopped partway. Run it again once that one finishes." + ), + "memories_processed": result["memories_processed"], + "build_run_id": run_id, + } + await _retire_queued_tasks(outcomes, created_at) await asyncio.to_thread( _finish_build_run, run_id, status="success", - memories_processed=len(rows), + memories_processed=result["memories_processed"], entities_extracted=result["entities_extracted"], relationships_created=result["relationships_created"], code_links_created=result["code_links_created"], @@ -485,11 +643,100 @@ async def _marm_concept_build(req: ConceptBuildRequest) -> dict: duration_ms=result["duration_ms"], finished_at=datetime.now(timezone.utc).isoformat(), ) + result.pop("aborted", None) + if graph_rebuilt: + # Only now, with the corpus actually extracted. The reset deliberately + # leaves no version marker, so a rebuild that died partway is still + # reported as rebuild_required and gets retried instead of passing for + # a graph that was never populated. + try: + await asyncio.to_thread(mark_schema_current, get_concept_db_path()) + except Exception as e: + logger.warning("concepts.schema_mark_failed", error=str(e)) result["graph_rebuilt"] = graph_rebuilt result["build_run_id"] = run_id return result +async def _retire_queued_tasks(outcomes: dict[str, str], build_started_at: str) -> None: + """Clear queue rows this build has already covered. + + Only ids the build actually settled, and only rows queued before it + started: anything written or merged mid-build re-stamps enqueued_at and + survives, and a memory whose extraction failed was never settled so its + retry is untouched. Without this the forced rebuild in v2.36.0 would leave + the whole corpus queued behind the build that just indexed it. + """ + settled = [ + memory_id + for memory_id, outcome in outcomes.items() + if outcome in ("indexed", "no_entities") + ] + if not settled: + return + try: + retired = await asyncio.to_thread( + concept_queue.retire_indexed, settled, build_started_at + ) + if retired: + logger.info("concepts.queue_retired", tasks=retired) + except Exception as e: + # Leaving rows queued only costs a redundant re-extraction later. + logger.warning("concepts.queue_retire_failed", error=str(e)) + + +def _build_for_memory_ids_sync( + memory_ids: list[str], + abort: Optional[threading.Event] = None, + finished: Optional[threading.Event] = None, +) -> dict[str, str]: + rows = _fetch_memory_rows_by_ids(memory_ids) + outcomes: dict[str, str] = {} + result = _run_build([rows], outcomes=outcomes, abort=abort, finished=finished) + if result["aborted"]: + # Half the batch may be unprocessed and the rest is no longer ours to + # settle. Report nothing: every task stays queued and is retried. + return {} + for memory_id in memory_ids: + outcomes.setdefault(memory_id, "vanished") + return outcomes + + +async def build_for_memory_ids( + memory_ids: list[str], + abort: Optional[threading.Event] = None, + finished: Optional[threading.Event] = None, +) -> dict[str, str]: + """Index a specific set of memories. Returns one outcome per requested id: + indexed, no_entities, failed, or vanished. + + Not a route and not an MCP tool. The background indexing worker is the + only caller, and it settles queue tasks on these outcomes rather than on + whether this raised. Takes _concept_build_lock, so the worker and a manual + marm_concept_build can never write the concept DB concurrently. + + Refuses outright on an unavailable or stale graph. Writing incremental + entities into a graph that is already flagged for rebuild would mix two + extraction rules in one database, and settling those tasks would mean the + rebuild never sees them. + + An empty result means the batch was abandoned partway because the graph + lock was lost. The caller must settle nothing in that case.""" + if not memory_ids: + return {} + if not CONCEPTS_AVAILABLE: + raise RuntimeError("concept extraction unavailable") + state = await asyncio.to_thread(inspect_concept_schema, get_concept_db_path()) + if state == "rebuild_required": + raise RuntimeError("rebuild_required") + if state == "unavailable": + raise RuntimeError("concept database unavailable") + async with _concept_build_lock: + return await asyncio.to_thread( + _build_for_memory_ids_sync, memory_ids, abort, finished + ) + + @router.post("/marm_concept_recall", operation_id="marm_concept_recall") async def marm_concept_recall(req: ConceptRecallRequest) -> dict: """🔎 Search the concept graph: entities, their relationships, and linked code. diff --git a/marm-mcp-server/marm_mcp_server/endpoints/memory.py b/marm-mcp-server/marm_mcp_server/endpoints/memory.py index 161a743f..dc9a9a1e 100644 --- a/marm-mcp-server/marm_mcp_server/endpoints/memory.py +++ b/marm-mcp-server/marm_mcp_server/endpoints/memory.py @@ -137,6 +137,17 @@ async def console_create_memory(payload: ConsoleMemoryPayload) -> dict: @router.put("/internal/memories/{memory_id}") async def console_replace_memory(memory_id: str, payload: ConsoleMemoryPayload) -> dict: + # Retract the old content's concepts before the replacement is written, not + # after. The write queues the memory for reindexing, so cleaning up + # afterwards can delete entities the indexing worker has already written for + # the new content, and the queue row is settled by then with nothing left to + # restore them. Same ordering as the promoted-doc resave path. + # + # Guarded on the memory existing, which is not true of that path: a replace + # against a missing id is an ordinary 404, and cleaning first would strip a + # live memory's provenance on the way to returning one. + if memory.console_memory_row(memory_id) is not None: + await _cleanup_deleted_concepts_async([memory_id]) try: updated = await memory.console_replace_memory( memory_id, @@ -151,7 +162,6 @@ async def console_replace_memory(memory_id: str, payload: ConsoleMemoryPayload) raise _memory_conflict(exc) from exc if not updated: raise HTTPException(status_code=404, detail="Memory not found") - await _cleanup_deleted_concepts_async([memory_id]) return memory.console_memory_row(memory_id) or {"id": memory_id} diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md index 8470ace6..c76933d2 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md @@ -1,6 +1,4 @@ -# Give your AI Agents a permanent memory in 60 seconds. - - +# Give your AI Agents a permanent memory in 60 seconds ## Table of Contents @@ -64,7 +62,7 @@ It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. - 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. -- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. +- 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -118,8 +116,8 @@ marm-memory uninstall # preview package removal; always pre **Knowledge, projects, and maintenance** ```bash -marm-memory knowledge status # Check available indexers and models -marm-memory knowledge build --all # Build the concept graph from stored memories +marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is +marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) marm-memory projects list # List all tracked workspaces marm-memory projects index # Run deep codebase structural indexing marm-memory projects status # Inspect target repo graph readiness @@ -674,10 +672,10 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | |------|--------------|----------------| -| `marm_concept_build` | Extract entities and typed relationships from stored memories | `session_name`, `project`, or `search_all=True` (one required) | +| `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -843,7 +841,7 @@ Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/ ### Concept Graph: what your memories are about -MARM can extract a knowledge graph from the memories you've already stored. `marm_concept_build` runs entity and relationship extraction over stored memory content, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). Once built, `marm_smart_recall` automatically adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: +MARM extracts a knowledge graph from the memories you store, producing typed entities (**concepts, decisions, patterns, errors, tools, people, organizations**) connected by typed relationships (**fixes, implements, depends_on, uses, causes, replaces, extends**). This happens on its own: storing a memory queues it, and a background worker adds it to the graph roughly 30 seconds later. `marm_concept_build` is still there for a full or scoped rebuild. Once there is a graph, `marm_smart_recall` adds bounded related entities, relationships, and linked code as a `graph_context` sidecar without changing primary memory ranking. `marm_concept_recall` remains available for explicit graph exploration: ```text marm_concept_recall(query="write queue") → the entity, its relationships, linked code symbols @@ -852,12 +850,17 @@ marm_concept_recall(query="related to SQLite", depth=3) → multi-hop traversal How to use it: -- **Build first**: call `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True`. There is no data until a build has run at least once. Builds are explicit and on-demand, not a live hook into the write path; re-run after logging significant new memories. -- **Upgrade once**: graphs built before platform attribution require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. -- **Bounded by design**: each build is row-capped (`CONCEPT_BUILD_ROW_CAP`, default 500) so a huge store can't turn one tool call into a runaway job. +- **Automatic by default**: new memories reach the graph without a tool call. Set `CONCEPT_AUTO_INDEX=false` to go back to manual builds only, which stops the worker but keeps recording queue rows, so turning it back on picks up everything written while it was off; `CONCEPT_INDEX_DEBOUNCE_SECONDS` (30) and `CONCEPT_INDEX_BATCH_SIZE` (20) control the pace. +- **Safe on both transports at once**: a leased lock in the memory database keeps a rebuild in one process from dropping graph tables while another process is writing to them. A build that finds the graph busy says so instead of colliding. +- **Failure never reaches your memories**: indexing runs on a durable queue outside the write path. Extraction problems retry, a memory that fails repeatedly is parked with its error, and the memory itself stores and recalls normally throughout. +- **Clearing a backlog costs some recall speed**: entity extraction is CPU-bound, so while the worker is working through a queue, measured recall goes from ~8ms to ~16ms median on a real 768-memory corpus. Writes are unaffected. It only applies while a backlog is draining, which for most people is once, after the upgrade rebuild. Reproduce it with `scripts/benchmarking/performance/bench_concept_worker.py --from-live`. +- **Build for the backlog**: `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True` indexes memories stored before automatic indexing existed, and rebuilds after an upgrade that requires one. +- **Upgrade twice so far**: graphs built before platform attribution, or before compaction sources replaced summaries as the indexed rows, require `marm_concept_build(search_all=True)`. A full build backs up and resets only the derived concept database; targeted builds refuse to guess platform ownership. +- **Whole scope, paged**: builds read every memory in scope. `CONCEPT_BUILD_ROW_CAP` (default 500) is the page size, so lowering it makes a build read more, smaller pages rather than skipping the rest. +- **Compacted sessions**: the original memories are indexed and the generated summary is not, so concepts stay attributed to where they were actually stated. - **Recall fails open**: a missing, empty, incompatible, or unavailable concept graph never blocks normal memory recall. The response reports graph status separately. - **Code cross-linking**: when the code graph has indexed the same project, concept entities that match code symbols get linked, connecting "what we decided" to "where it lives in the code." -- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first concept build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. +- **Bundled extraction runtime**: the spaCy runtime and English extraction model ship with MARM but load only on the first extraction, which now happens on its own shortly after the first memory is stored rather than when you run a build. If a damaged or partial installation makes them unavailable, both concept tools degrade cleanly while core memory remains available; run `marm-memory knowledge status`, then reinstall MARM if needed. - **Isolated storage**: the concept graph lives in its own SQLite database (`~/.marm/index/marm_index.db`) with its own connection pool, so concept-graph writes can never block or corrupt the production memory database. - **Console atlas**: MARM Console renders the complete atlas up to 750 entities and 6,000 stored relationships. Larger graphs use a deterministic connected sample of up to 600 entities and 4,000 aggregated visual edges, clearly labelled as sampled. @@ -873,7 +876,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **FTS5 full-text index** (`memories_fts`) is maintained as an external-content table over the memories table and powers both the exact lane (BM25) and the filter stage of hybrid recall. - **Chunk storage**: memories past ~180 words are split into overlapping 150-token chunks (50-token overlap) in a `memory_chunks` table, each with its own embedding. Recall scores chunks and collapses to the parent memory. - **Embeddings** come from the fastembed-backed `jinaai/jina-embeddings-v2-small-en` encoder: 33M parameters, 512 dimensions, an 8,192-token context window, and an Apache-2.0 license. It does not require separate query/document text prefixes. The encoder is lazily loaded on first semantic use and serialized behind a lock so concurrent encodes can't corrupt each other. If it is unavailable, writes still succeed; memories are stored without embeddings until it loads. Semantic scoring runs as a single NumPy batch (matrix cosine) rather than a Python loop. -- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. +- **The concept graph gets its own database** (`~/.marm/index/marm_index.db`) and its own pool, reusing the same pool implementation but never sharing connections with the memory store. Deliberate isolation: an experimental graph build must not be able to stall the production WAL. The one exception is the indexing queue, which lives in the memory database on purpose so a memory and its indexing task commit together; the graph itself stays derived and disposable. ### Write path @@ -882,6 +885,7 @@ Everything above runs on a small number of deliberate mechanisms. This section i - **Layer 1, exact dedup**: a SHA-256 hash of normalized content is checked within the session; hash hits are verified against the actual content before deduplicating, so a hash collision stores a new row instead of silently merging different content. - **Layer 2, semantic merge**: near-duplicates above `CONSOLIDATION_THRESHOLD` cosine similarity are merged rather than accumulated. This never blocks a write; if the encoder isn't available, the write proceeds unconsolidated. - The tradeoff is measured and published: roughly 9x median write cost (58ms vs 6.5ms) in exchange for a store that stays clean, because reads dominate memory workloads. See section 3 of the benchmarks above. +- **Concept indexing is a durable outbox**: a write records an indexing task in the same transaction as the memory, so a memory cannot exist without one. A background worker drains that queue and writes the concept graph. Nothing on the write path waits for extraction, and a process killed mid-extraction loses no work because the task is a row rather than an in-memory job. Both transports run a worker, so the two coordinate through a leased lock in the memory database rather than an in-process lock, which would not span them. - **Compaction** (opt-in, `COMPACTION_ENABLED=1`) is Layer 3: after enough writes in a session, a background pass detects clusters of related memories using cosine similarity plus union-find connected components, gated by minimum cluster size, minimum age, and an active-session grace period so it never compacts work in flight. MARM then injects a bounded request asking the connected agent to summarize each cluster: `candidates` → `stage` → `review` → `apply` or `discard`. Source memory IDs are preserved on apply, so compacted summaries stay traceable to their originals. Staged summaries expire (`COMPACTION_STAGING_TTL_HOURS`), nudges are capped and cooldown-limited, and the injection has a byte budget. The design is honest about what LLMs are for: MARM detects, the agent summarizes, and a human-reviewable stage/apply/discard loop gates the destructive step. ### Recall path @@ -951,7 +955,13 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | -| `CONCEPT_BUILD_ROW_CAP` | `500` | Max memory rows per concept-graph build | +| `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | +| `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | +| `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | +| `CONCEPT_INDEX_BATCH_SIZE` | `20` | Memories indexed per batch, capped at 500. Lowering it does not reduce contention; it measured slightly worse | +| `CONCEPT_INDEX_BATCH_PAUSE_MS` | `250` | Pause between batches while clearing a backlog. Cuts worst-case recall during indexing from ~270ms to ~80ms for about 18% longer drain. `0` disables it | +| `CONCEPT_INDEX_LEASE_SECONDS` | `300` | How long a claimed indexing task stays owned once nothing is renewing it. Work in progress renews its own lease, so this bounds how long a *killed* process holds tasks, not how long a batch may take. Reclaimed tasks spend no attempt | +| `CONCEPT_INDEX_MAX_ATTEMPTS` | `3` | Failed attempts before a memory is parked with its error instead of retried |
@@ -1028,6 +1038,23 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - First confirm that a scoped concept build actually includes memories with extractable entities. - Run `marm-memory knowledge status`; if it reports a missing runtime or model, repair the install with `python -m pip install -U --force-reinstall marm-mcp-server`. +**New memories are not showing up in the graph** + +- Run `marm-memory knowledge status`. `index_queue.pending` is how many memories are waiting; `index_queue.parked` is how many gave up. `auto_index: false` means indexing is switched off. +- Give it the debounce interval (30 seconds by default) plus extraction time. A burst of writes is indexed as one pass, not one per memory. +- Check that `CONCEPT_AUTO_INDEX` is not set to `false`, `0`, `no`, or `off`. +- A graph awaiting a rebuild is not indexed into. If the Console or `marm-memory knowledge status` reports `rebuild_required`, run `marm_concept_build(search_all=True)` once; queued memories are picked up after it. +- Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. +- A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. + +**A build returns `build_in_progress`** + +- Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. + +**A build returns `lock_lost`** + +- The build was stalled long enough for another process to take over the graph, so it stopped partway rather than writing alongside it. Usually a suspended machine or a debugger pause. Whatever it indexed before stopping is kept, and re-running the build finishes the rest. +
diff --git a/marm-mcp-server/marm_mcp_server/server.py b/marm-mcp-server/marm_mcp_server/server.py index 32138a25..1d945a3b 100644 --- a/marm-mcp-server/marm_mcp_server/server.py +++ b/marm-mcp-server/marm_mcp_server/server.py @@ -5,7 +5,7 @@ FastAPI application, compliant with the MCP protocol via FastApiMCP. Author: Lyell - marm-memory -Version: 2.35.0 +Version: 2.36.0 """ import os @@ -22,6 +22,7 @@ SERVER_VERSION, ) from .core.compaction_scheduler import _maybe_start_compaction_scheduler +from .core.concept_worker import concept_worker from .core.graph_supervisor import graph_supervisor # noqa: F401 from .core.memory import memory from .endpoints.compaction import router as compaction_router @@ -64,6 +65,7 @@ async def lifespan(app: FastAPI): memory.restore_active_session() _compaction_scheduler = _maybe_start_compaction_scheduler() + concept_worker.start() memory_after = get_memory_usage() logger.info("Memory usage after startup", memory_mb=f"{memory_after:.1f}") diff --git a/marm-mcp-server/marm_mcp_server/server_stdio.py b/marm-mcp-server/marm_mcp_server/server_stdio.py index f3212c29..5140df84 100644 --- a/marm-mcp-server/marm_mcp_server/server_stdio.py +++ b/marm-mcp-server/marm_mcp_server/server_stdio.py @@ -35,6 +35,7 @@ SEMANTIC_SEARCH_AVAILABLE, SERVER_VERSION, ) +from marm_mcp_server.core.concept_worker import concept_worker # noqa: E402 from marm_mcp_server.core.graph_supervisor import graph_supervisor # noqa: E402 from marm_mcp_server.core.memory import memory # noqa: E402 from marm_mcp_server.core.memory_utils import drain_chunk_writes # noqa: E402 @@ -64,6 +65,7 @@ async def _stdio_lifespan(_server: FastMCP): the yield skips anything after it. A crashed or cancelled session is exactly when unwritten chunks are most likely to be pending. """ + concept_worker.start() try: yield finally: @@ -79,6 +81,13 @@ async def _stdio_lifespan(_server: FastMCP): # creates the chunk tasks the next call waits on. Same order as # graceful_shutdown(); STDIO starts the queue lazily on first write # and otherwise never stops it. + # Signals and returns. Its tasks are durable rows, so an + # interrupted extraction costs nothing and the next session picks + # it up; awaiting one here would put spaCy on the teardown path. + try: + await concept_worker.stop() + except Exception as exc: + _stdio_log.warning("concept worker stop failed: %s", exc) try: await memory.stop_write_queue() except Exception as exc: diff --git a/marm-mcp-server/marm_mcp_server/services/notebook.py b/marm-mcp-server/marm_mcp_server/services/notebook.py index accc9955..8c4e45ed 100644 --- a/marm-mcp-server/marm_mcp_server/services/notebook.py +++ b/marm-mcp-server/marm_mcp_server/services/notebook.py @@ -238,6 +238,25 @@ async def _save( mirror_status = "synced" memory_id = doc_row.memory_id + + if not was_created and doc_row.memory_id: + # Before the replacement is written, not after. Cleanup strips every + # trace of a memory id, so running it once the new content is already + # queued can delete entities the worker has written for that content, + # with the queue row settled and nothing left to re-index it. + # + # Keyed on the previous id too, not the one store_doc_mirror returns. + # A mirror whose row was deleted out from under it is repaired with a + # fresh id, and it is the old id that carries the stale provenance. + from ..endpoints.memory import _cleanup_deleted_concepts_async + + try: + await _cleanup_deleted_concepts_async([doc_row.memory_id]) + except Exception as e: + _safe_print( + f"Doc mirror concept cleanup failed for {doc_row.memory_id}: {e}" + ) + try: memory_id = await memory.store_doc_mirror( content, diff --git a/marm-mcp-server/marm_mcp_server/services/runtime_status.py b/marm-mcp-server/marm_mcp_server/services/runtime_status.py index 6b82a6a1..56888a41 100644 --- a/marm-mcp-server/marm_mcp_server/services/runtime_status.py +++ b/marm-mcp-server/marm_mcp_server/services/runtime_status.py @@ -15,6 +15,7 @@ from typing import Any from ..config.settings import ( + CONCEPT_AUTO_INDEX, CONCEPT_MODEL_AVAILABLE, DEFAULT_DB_PATH, DEFAULT_SEMANTIC_MODEL, @@ -116,11 +117,28 @@ def knowledge_status() -> dict[str, Any]: "spacy": spacy_available, "model": model_available, "schema": schema, + "auto_index": CONCEPT_AUTO_INDEX, + "index_queue": _index_queue_counts(), "database": {"path": str(concept_path), "exists": concept_path.exists()}, "last_build": _latest_concept_build(concept_path), } +def _index_queue_counts() -> dict[str, Any]: + """How far behind automatic indexing is. Without this the only symptom of + a dormant worker is a graph that quietly stops growing. + + None on any failure: a status command must still report the runtime and + schema even when the memory database cannot be opened. + """ + try: + from ..core import concept_queue + + return concept_queue.counts() + except Exception: + return {"pending": None, "parked": None} + + def maintenance_status() -> dict[str, Any]: runtime = inspect_runtime() remote = runtime.get("runtime") or {} diff --git a/marm-mcp-server/pyproject.toml b/marm-mcp-server/pyproject.toml index dac0f971..dd85e700 100644 --- a/marm-mcp-server/pyproject.toml +++ b/marm-mcp-server/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "marm-mcp-server" -version = "2.35.0" +version = "2.36.0" description = "Local-first 3-in-1 AI memory layer & MCP server for Claude Code, Codex, Grok, Gemini, VS Code and Cursor. Fuses session history, codebase indexing & concept graphs in SQLite. Enables zero-cloud, privacy-first context & instant recall also works with multi-agent swarms." readme = "README.md" license = "Apache-2.0" diff --git a/marm-mcp-server/server.json b/marm-mcp-server/server.json index c16bc423..f30ab0ef 100644 --- a/marm-mcp-server/server.json +++ b/marm-mcp-server/server.json @@ -3,7 +3,7 @@ "_schema_date": "2025-12-11", "name": "io.github.Lyellr88/marm-mcp-server", "description": "Universal MCP Server with advanced AI memory capabilities and semantic search.", - "version": "2.35.0", + "version": "2.36.0", "author": "Ryan Lyell - marm-memory", "license": "Apache-2.0", "homepage": "https://marmsystems.com", @@ -17,12 +17,12 @@ { "registryType": "pypi", "identifier": "marm-mcp-server", - "version": "2.35.0", + "version": "2.36.0", "transport": { "type": "stdio" } }, { "registryType": "oci", - "identifier": "lyellr88/marm-mcp-server:2.35.0", + "identifier": "lyellr88/marm-mcp-server:2.36.0", "transport": { "type": "stdio" } } ], diff --git a/marm-mcp-server/tests/test_concept_build_pagination.py b/marm-mcp-server/tests/test_concept_build_pagination.py new file mode 100644 index 00000000..3ee0d4a3 --- /dev/null +++ b/marm-mcp-server/tests/test_concept_build_pagination.py @@ -0,0 +1,151 @@ +"""Tests that a concept build reads its whole scope instead of truncating. + +Until v2.36.0 every build ended with `ORDER BY created_at DESC LIMIT +CONCEPT_BUILD_ROW_CAP`, so on a corpus larger than 500 rows the older +memories were not slow to reach, they were unreachable. These tests run +against real SQLite (tmp_path-backed, via conftest.load_isolated_server) +and monkeypatch extract_entities at the endpoints module boundary, the same +convention test_concept_endpoints.py uses. +""" + +import importlib +import sys + +import pytest +from conftest import load_isolated_server + + +@pytest.fixture +def concepts_env(monkeypatch, tmp_path): + load_isolated_server(monkeypatch, tmp_path) + monkeypatch.setenv("MARM_CONCEPT_DB_PATH", str(tmp_path / "marm_index.db")) + concepts = importlib.import_module("marm_mcp_server.endpoints.concepts") + memory_module = sys.modules["marm_mcp_server.core.memory"] + return concepts, memory_module + + +def _seed(memory_module, count, session="sess-a"): + with memory_module.memory.get_connection() as conn: + conn.executemany( + "INSERT INTO memories (id, session_name, content, timestamp) " + "VALUES (?, ?, ?, datetime('now'))", + [(f"m{i:05d}", session, f"memory content {i}") for i in range(count)], + ) + + +def _one_entity_per_memory(monkeypatch, concepts): + """Name the entity after the content so each memory produces its own.""" + from marm_mcp_server.core.concept_extraction import Entity, ExtractionResult + + monkeypatch.setattr(concepts, "is_graph_available", lambda: False) + monkeypatch.setattr( + concepts, + "extract_entities", + lambda content: ExtractionResult( + entities=[Entity(content, "concept")], relationship_pairs=[] + ), + ) + + +def test_build_over_1200_memories_reaches_every_row_at_default_cap( + concepts_env, monkeypatch +): + """The regression this feature exists to fix, at the shipped default of + 500: memory 0 is the oldest of 1,200 and must still be extracted.""" + concepts, memory_module = concepts_env + assert concepts.CONCEPT_BUILD_ROW_CAP == 500 + _seed(memory_module, 1200) + _one_entity_per_memory(monkeypatch, concepts) + + pages = concepts._fetch_memory_pages( + session_name=None, project=None, search_all=True + ) + result = concepts._run_build(pages) + + assert result["memories_processed"] == 1200 + assert result["entities_extracted"] == 1200 + + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + total = conn.execute("SELECT COUNT(*) FROM entities").fetchone()[0] + oldest = conn.execute( + "SELECT COUNT(*) FROM entities WHERE name = ?", ("memory content 0",) + ).fetchone()[0] + assert total == 1200 + assert oldest == 1 + + +def test_paged_ids_match_an_unpaginated_baseline_exactly(concepts_env, monkeypatch): + """Page boundaries must lose nothing and repeat nothing. Compared against + the same query run as one statement, not against a hand-written list.""" + concepts, memory_module = concepts_env + monkeypatch.setattr(concepts, "CONCEPT_BUILD_ROW_CAP", 7) + _seed(memory_module, 100) + + paged = [ + row[0] + for page in concepts._fetch_memory_pages( + session_name=None, project=None, search_all=True + ) + for row in page + ] + with memory_module.memory.get_connection() as conn: + baseline = [ + row[0] + for row in conn.execute( + # Must match production's filter, including the v2.36.0 + # inversion to summary. A baseline that still excludes sources + # disagrees with the code exactly where this feature changed + # behavior, which is the one place it needs to agree. + "SELECT id FROM memories WHERE session_name != 'marm_system' " + "AND content IS NOT NULL AND content != '' " + "AND (compaction_role IS NULL OR compaction_role != 'summary') " + "ORDER BY created_at DESC, id DESC" + ).fetchall() + ] + + assert paged == baseline + assert len(paged) == 100 + + +def test_page_size_of_one_still_terminates_and_reads_everything( + concepts_env, monkeypatch +): + """CONCEPT_BUILD_ROW_CAP clamps to a minimum of 1. A cap of 1 makes every + page a single row, which is the degenerate case where a keyset cursor bug + would either loop forever or stop after the first page.""" + concepts, memory_module = concepts_env + monkeypatch.setattr(concepts, "CONCEPT_BUILD_ROW_CAP", 1) + _seed(memory_module, 12) + + pages = list( + concepts._fetch_memory_pages(session_name=None, project=None, search_all=True) + ) + + assert [len(p) for p in pages] == [1] * 12 + assert len({page[0][0] for page in pages}) == 12 + + +def test_memories_written_during_a_build_are_not_reprocessed(concepts_env, monkeypatch): + """Descending keyset means a row written mid-build sorts ahead of the + cursor and is skipped, rather than shifting the window and causing a + repeat. The queue worker re-indexes it separately.""" + concepts, memory_module = concepts_env + monkeypatch.setattr(concepts, "CONCEPT_BUILD_ROW_CAP", 4) + _seed(memory_module, 8) + + seen = [] + pages = concepts._fetch_memory_pages( + session_name=None, project=None, search_all=True + ) + for index, page in enumerate(pages): + seen.extend(row[0] for row in page) + if index == 0: + with memory_module.memory.get_connection() as conn: + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp) " + "VALUES ('m99999', 'sess-a', 'written mid build', datetime('now'))" + ) + + assert "m99999" not in seen + assert len(seen) == len(set(seen)) == 8 diff --git a/marm-mcp-server/tests/test_concept_endpoints.py b/marm-mcp-server/tests/test_concept_endpoints.py index 0bd4cd07..89dd5f24 100644 --- a/marm-mcp-server/tests/test_concept_endpoints.py +++ b/marm-mcp-server/tests/test_concept_endpoints.py @@ -1,6 +1,6 @@ """Tests for endpoints/concepts.py's build/recall orchestration. -_fetch_memory_rows and the concept-DB read/write paths run against real +_fetch_memory_pages and the concept-DB read/write paths run against real SQLite (tmp_path-backed, via conftest.load_isolated_server). extract_entities is monkeypatched at the endpoints module boundary for build tests -- spaCy's actual model isn't installable in this sandbox (see test_concept_extraction.py), @@ -29,6 +29,11 @@ def _fresh_concepts_module(monkeypatch, tmp_path, concept_db_path=None): return server, concepts +def _all_rows(pages): + """Flatten _fetch_memory_pages so a test can assert over the whole scope.""" + return [row for page in pages for row in page] + + def _seed_memory(memory_module, rows): """rows: list of (id, session_name, content, project) tuples.""" with memory_module.memory.get_connection() as conn: @@ -47,7 +52,7 @@ def concepts_env(monkeypatch, tmp_path): return server, concepts, memory_module -def test_fetch_memory_rows_scoped_by_session(concepts_env): +def test_fetch_memory_pages_scoped_by_session(concepts_env): _server, concepts, memory_module = concepts_env _seed_memory( memory_module, @@ -56,18 +61,22 @@ def test_fetch_memory_rows_scoped_by_session(concepts_env): ("m2", "sess-b", "second memory", None), ], ) - rows = concepts._fetch_memory_rows( - session_name="sess-a", project=None, search_all=False + rows = _all_rows( + concepts._fetch_memory_pages( + session_name="sess-a", project=None, search_all=False + ) ) assert [r[0] for r in rows] == ["m1"] -def test_fetch_memory_rows_requires_explicit_scope(concepts_env): +def test_fetch_memory_pages_requires_explicit_scope(concepts_env): """No session_name, no project, search_all=False must raise -- not - silently fall through to scanning every memory in the DB.""" + silently fall through to scanning every memory in the DB. Raised at call + time, not on first iteration, so the route's ValueError handler still + sees it where it always did.""" _server, concepts, _memory_module = concepts_env with pytest.raises(ValueError, match="session_name, project, or search_all"): - concepts._fetch_memory_rows(session_name=None, project=None, search_all=False) + concepts._fetch_memory_pages(session_name=None, project=None, search_all=False) def test_promoted_doc_mirror_reachable_by_matching_scoped_build(monkeypatch, tmp_path): @@ -76,7 +85,7 @@ def test_promoted_doc_mirror_reachable_by_matching_scoped_build(monkeypatch, tmp scoped to the doc's own project/session, and must NOT be picked up by a build scoped to a different project. Drives the real action='save' path (not a hand-inserted memories row) so this proves the actual - mirror-write wiring, not just that _fetch_memory_rows' SQL is capable + mirror-write wiring, not just that _fetch_memory_pages' SQL is capable of finding a row shaped like one.""" from conftest import load_isolated_server @@ -99,13 +108,17 @@ def test_promoted_doc_mirror_reachable_by_matching_scoped_build(monkeypatch, tmp assert result["status"] == "success" assert result["mirror_status"] == "synced" - matching_rows = concepts._fetch_memory_rows( - session_name=None, project="marm-systems", search_all=False + matching_rows = _all_rows( + concepts._fetch_memory_pages( + session_name=None, project="marm-systems", search_all=False + ) ) assert result["memory_id"] in [r[0] for r in matching_rows] - other_project_rows = concepts._fetch_memory_rows( - session_name=None, project="a-different-project", search_all=False + other_project_rows = _all_rows( + concepts._fetch_memory_pages( + session_name=None, project="a-different-project", search_all=False + ) ) assert result["memory_id"] not in [r[0] for r in other_project_rows] @@ -120,10 +133,10 @@ def test_marm_concept_build_route_returns_static_message_on_missing_scope( ConceptBuildRequest's own model_validator already rejects this same input at the pydantic layer (a different message, enforced before the - route body ever runs), so _fetch_memory_rows' runtime ValueError is + route body ever runs), so _fetch_memory_pages' runtime ValueError is unreachable via a normally-constructed request -- model_construct bypasses that validation to exercise the route's own except-ValueError - handling directly, same as it would need to if _fetch_memory_rows' + handling directly, same as it would need to if _fetch_memory_pages' check were ever the only guard left.""" _server, concepts, _memory_module = concepts_env from marm_mcp_server.core.models import ConceptBuildRequest @@ -197,6 +210,7 @@ def test_scoped_legacy_build_persists_rebuild_required_run(concepts_env, monkeyp def test_full_legacy_build_backs_up_and_resets_graph(concepts_env, monkeypatch): _server, concepts, _memory_module = concepts_env + from marm_mcp_server.core.concept_db import CONCEPT_SCHEMA_VERSION from marm_mcp_server.core.models import ConceptBuildRequest concept_db = concepts._get_concept_db() @@ -213,7 +227,9 @@ def test_full_legacy_build_backs_up_and_resets_graph(concepts_env, monkeypatch): monkeypatch.setattr( concepts, "_run_build", - lambda _rows: { + lambda _pages, _outcomes, _abort=None: { + "aborted": False, + "memories_processed": 0, "entities_extracted": 0, "relationships_created": 0, "code_links_created": 0, @@ -241,12 +257,9 @@ def test_full_legacy_build_backs_up_and_resets_graph(concepts_env, monkeypatch): active_concept_db = concepts._get_concept_db() with active_concept_db.get_connection() as conn: assert conn.execute("SELECT COUNT(*) FROM entities").fetchone()[0] == 0 - assert ( - conn.execute( - "SELECT value FROM concept_schema_metadata WHERE key = 'schema_version'" - ).fetchone()[0] - == "2" - ) + assert conn.execute( + "SELECT value FROM concept_schema_metadata WHERE key = 'schema_version'" + ).fetchone()[0] == str(CONCEPT_SCHEMA_VERSION) run = conn.execute( "SELECT status FROM concept_build_runs WHERE id = ?", ("full-rebuild-run",), @@ -254,7 +267,7 @@ def test_full_legacy_build_backs_up_and_resets_graph(concepts_env, monkeypatch): assert run[0] == "success" -def test_fetch_memory_rows_excludes_marm_system_session(concepts_env): +def test_fetch_memory_pages_excludes_marm_system_session(concepts_env): _server, concepts, memory_module = concepts_env _seed_memory( memory_module, @@ -263,15 +276,21 @@ def test_fetch_memory_rows_excludes_marm_system_session(concepts_env): ("m2", "sess-a", "real content", None), ], ) - rows = concepts._fetch_memory_rows(session_name=None, project=None, search_all=True) + rows = _all_rows( + concepts._fetch_memory_pages(session_name=None, project=None, search_all=True) + ) assert [r[0] for r in rows] == ["m2"] -def test_fetch_memory_rows_excludes_compacted_source_rows(concepts_env): - """Mirrors core/memory_recall.py's active-recall filter -- a compacted - session's stale source rows must not be indexed alongside their summary, - or a build reintroduces obsolete concepts/relationships and inflates - mention counts.""" +def test_fetch_memory_pages_indexes_sources_and_skips_compaction_summaries( + concepts_env, +): + """Deliberately the opposite of core/memory_recall.py's active-recall + filter. Recall wants the summary because it is the compact view. The graph + wants the sources, because they are where the concepts were actually + stated and indexing both restates every entity twice. Inverted in v2.36.0; + this is also what makes a memory graph-eligible the moment it is written + rather than after its session compacts.""" _server, concepts, memory_module = concepts_env with memory_module.memory.get_connection() as conn: conn.execute( @@ -283,22 +302,67 @@ def test_fetch_memory_rows_excludes_compacted_source_rows(concepts_env): "INSERT INTO memories (id, session_name, content, timestamp, project) " "VALUES ('m2', 'sess-a', 'compaction summary content', datetime('now'), NULL)" ) + conn.execute("UPDATE memories SET compaction_role = 'summary' WHERE id = 'm2'") + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, project) " + "VALUES ('m3', 'sess-a', 'uncompacted content', datetime('now'), NULL)" + ) - rows = concepts._fetch_memory_rows( - session_name="sess-a", project=None, search_all=False + rows = _all_rows( + concepts._fetch_memory_pages( + session_name="sess-a", project=None, search_all=False + ) ) - assert [r[0] for r in rows] == ["m2"] + assert sorted(r[0] for r in rows) == ["m1", "m3"] -def test_fetch_memory_rows_search_all_respects_row_cap(concepts_env, monkeypatch): +def test_fetch_memory_pages_reaches_every_row_beyond_one_page( + concepts_env, monkeypatch +): + """The row cap is a page size now, not a truncation limit. Before this, + a corpus larger than the cap left its oldest memories permanently + unreachable to every build.""" _server, concepts, memory_module = concepts_env monkeypatch.setattr(concepts, "CONCEPT_BUILD_ROW_CAP", 3) _seed_memory( memory_module, - [(f"m{i}", "sess-a", f"content {i}", None) for i in range(10)], + [(f"m{i:02d}", "sess-a", f"content {i}", None) for i in range(10)], ) - rows = concepts._fetch_memory_rows(session_name=None, project=None, search_all=True) - assert len(rows) == 3 + pages = list( + concepts._fetch_memory_pages(session_name=None, project=None, search_all=True) + ) + + assert [len(p) for p in pages] == [3, 3, 3, 1] + ids = [r[0] for page in pages for r in page] + assert sorted(ids) == [f"m{i:02d}" for i in range(10)] + + +def test_fetch_memory_pages_no_gaps_when_created_at_ties(concepts_env, monkeypatch): + """created_at defaults to CURRENT_TIMESTAMP, which is second-granular, so + a bulk write gives every row the same value. Keyset pagination on + created_at alone would drop or repeat rows at each boundary; the id + tiebreaker is what prevents it.""" + _server, concepts, memory_module = concepts_env + monkeypatch.setattr(concepts, "CONCEPT_BUILD_ROW_CAP", 4) + with memory_module.memory.get_connection() as conn: + for i in range(13): + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, created_at) " + "VALUES (?, 'sess-a', ?, datetime('now'), '2026-01-01 00:00:00')", + (f"m{i:02d}", f"content {i}"), + ) + + ids = [ + r[0] + for r in _all_rows( + concepts._fetch_memory_pages( + session_name=None, project=None, search_all=True + ) + ) + ] + + assert len(ids) == len(set(ids)) == 13 + assert sorted(ids) == [f"m{i:02d}" for i in range(13)] def test_run_build_writes_entities_and_relationship_for_two_entities( @@ -321,7 +385,7 @@ def test_run_build_writes_entities_and_relationship_for_two_entities( monkeypatch.setattr(concepts, "is_graph_available", lambda: False) rows = [("m1", "auth module talks to the rate limiter", "sess-a", "proj-a")] - result = concepts._run_build(rows) + result = concepts._run_build([rows]) assert result["entities_extracted"] == 2 assert result["relationships_created"] == 1 @@ -367,8 +431,8 @@ def test_run_build_is_idempotent_on_repeat_runs(concepts_env, monkeypatch): ) rows = [("m1", "auth module talks to the rate limiter", "sess-a", "proj-a")] - first = concepts._run_build(rows) - second = concepts._run_build(rows) + first = concepts._run_build([rows]) + second = concepts._run_build([rows]) assert first["relationships_created"] == 1 assert first["code_links_created"] == 2 # one per entity @@ -414,8 +478,8 @@ def test_run_recall_does_not_return_duplicate_linked_code_after_repeat_build( ) rows = [("m1", "CbmClient reference", "sess-a", "proj-a")] - concepts._run_build(rows) - concepts._run_build(rows) # repeat build, same corpus + concepts._run_build([rows]) + concepts._run_build([rows]) # repeat build, same corpus result = concepts._run_recall("CbmClient", session_name=None, limit=10) assert len(result["linked_code"]) == 1 @@ -440,7 +504,7 @@ def test_run_build_same_entity_across_two_memories_dedups_in_same_session( ("m1", "auth module content one", "sess-a", None), ("m2", "auth module content two", "sess-a", None), ] - result = concepts._run_build(rows) + result = concepts._run_build([rows]) assert result["entities_extracted"] == 2 # two mentions processed concept_db = concepts._get_concept_db() @@ -469,7 +533,7 @@ def test_run_build_with_graph_unavailable_creates_zero_code_links( lambda *a, **k: pytest.fail("should never be called when graph is unavailable"), ) - result = concepts._run_build([("m1", "CbmClient reference", "sess-a", "proj-a")]) + result = concepts._run_build([[("m1", "CbmClient reference", "sess-a", "proj-a")]]) assert result["code_links_created"] == 0 @@ -495,7 +559,7 @@ def test_run_build_links_code_when_graph_available(concepts_env, monkeypatch): }, ) - result = concepts._run_build([("m1", "CbmClient reference", "sess-a", "proj-a")]) + result = concepts._run_build([[("m1", "CbmClient reference", "sess-a", "proj-a")]]) assert result["code_links_created"] == 1 @@ -517,7 +581,7 @@ def test_run_build_reports_no_duplicates_when_embedding_unavailable( ) monkeypatch.setattr(concepts, "is_graph_available", lambda: False) - result = concepts._run_build([("m1", "auth module content", "sess-a", None)]) + result = concepts._run_build([[("m1", "auth module content", "sess-a", None)]]) assert result["possible_duplicates"] == [] @@ -554,7 +618,7 @@ def test_run_build_reports_possible_duplicate_when_similar_entity_exists( ("m1", "Auth content", "sess-a", None), ("m2", "OAuth content", "sess-a", None), ] - result = concepts._run_build(rows) + result = concepts._run_build([rows]) assert len(result["possible_duplicates"]) == 1 dup = result["possible_duplicates"][0] @@ -594,7 +658,7 @@ def _counting_embed(name): ("m2", "auth module content two", "sess-a", None), ("m3", "auth module content three", "sess-a", None), ] - result = concepts._run_build(rows) + result = concepts._run_build([rows]) assert result["entities_extracted"] == 3 # three mentions processed assert call_count["n"] == 1 # but the name was only ever embedded once @@ -955,8 +1019,10 @@ def test_base_install_keeps_concept_tools_registered(concepts_env): memory_module, [("m1", "sess-a", "The write queue serializes memory writes.", None)], ) - rows = concepts._fetch_memory_rows(session_name=None, project=None, search_all=True) - result = concepts._run_build(rows) + pages = concepts._fetch_memory_pages( + session_name=None, project=None, search_all=True + ) + result = concepts._run_build(pages) if CONCEPTS_AVAILABLE: assert result["entities_extracted"] > 0, ( diff --git a/marm-mcp-server/tests/test_concept_incremental_build.py b/marm-mcp-server/tests/test_concept_incremental_build.py new file mode 100644 index 00000000..5ed67e7c --- /dev/null +++ b/marm-mcp-server/tests/test_concept_incremental_build.py @@ -0,0 +1,434 @@ +"""Tests for the targeted per-memory build path the indexing worker calls. + +build_for_memory_ids is what makes automation possible, and the whole point +of it is the outcome it reports. A build that swallows a failure and returns +success would make the worker delete a task it promised to retry, silently +and permanently, so these tests care more about the outcome map than about +how many entities landed. +""" + +import asyncio +import importlib +import sys + +import pytest +from conftest import load_isolated_server + + +@pytest.fixture +def concepts_env(monkeypatch, tmp_path): + load_isolated_server(monkeypatch, tmp_path) + monkeypatch.setenv("MARM_CONCEPT_DB_PATH", str(tmp_path / "marm_index.db")) + concepts = importlib.import_module("marm_mcp_server.endpoints.concepts") + monkeypatch.setattr(concepts, "CONCEPTS_AVAILABLE", True) + monkeypatch.setattr(concepts, "is_graph_available", lambda: False) + memory_module = sys.modules["marm_mcp_server.core.memory"] + return concepts, memory_module + + +def _seed(memory_module, rows): + """rows: (id, content) or (id, content, compaction_role).""" + with memory_module.memory.get_connection() as conn: + for row in rows: + mem_id, content = row[0], row[1] + role = row[2] if len(row) > 2 else None + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, " + "compaction_role) VALUES (?, 'sess-a', ?, datetime('now'), ?)", + (mem_id, content, role), + ) + + +def _extract(monkeypatch, concepts, by_content): + from marm_mcp_server.core.concept_extraction import Entity, ExtractionResult + + def fake(content): + names = by_content[content] + if names is Exception: + raise RuntimeError("extraction blew up") + return ExtractionResult( + entities=[Entity(name, "concept") for name in names], + relationship_pairs=[], + ) + + monkeypatch.setattr(concepts, "extract_entities", fake) + + +def test_indexes_only_the_requested_ids(concepts_env, monkeypatch): + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha"), ("m2", "beta"), ("m3", "gamma")]) + _extract( + monkeypatch, + concepts, + {"alpha": ["Alpha"], "beta": ["Beta"], "gamma": ["Gamma"]}, + ) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1", "m3"])) + + assert outcomes == {"m1": "indexed", "m3": "indexed"} + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + names = {row[0] for row in conn.execute("SELECT name FROM entities").fetchall()} + assert names == {"Alpha", "Gamma"} + + +def test_memory_with_nothing_extractable_is_done_not_failed(concepts_env, monkeypatch): + """no_entities completes the task. Retrying it would loop until the + attempt cap and then park a memory that is simply uninteresting.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "ok"), ("m2", "empty")]) + _extract(monkeypatch, concepts, {"ok": ["Thing"], "empty": []}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1", "m2"])) + + assert outcomes == {"m1": "indexed", "m2": "no_entities"} + + +def test_extraction_failure_is_reported_as_failed_not_swallowed( + concepts_env, monkeypatch +): + """The aggregate counters return success with zeros here. Only the + per-memory outcome distinguishes this from a genuinely empty memory.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "good"), ("m2", "poison")]) + _extract(monkeypatch, concepts, {"good": ["Good"], "poison": Exception}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1", "m2"])) + + assert outcomes == {"m1": "indexed", "m2": "failed"} + + +def test_entity_write_failure_marks_the_memory_failed(concepts_env, monkeypatch): + """A partially written memory must retry. get_or_create_entity is + idempotent, so re-extracting it is safe.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "two entities")]) + _extract(monkeypatch, concepts, {"two entities": ["First", "Second"]}) + + concept_db = concepts._get_concept_db() + real = concept_db.get_or_create_entity + + def flaky(conn, name, *args, **kwargs): + if name == "Second": + raise RuntimeError("write failed") + return real(conn, name, *args, **kwargs) + + monkeypatch.setattr(concept_db, "get_or_create_entity", flaky) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1"])) + + assert outcomes == {"m1": "failed"} + + +def test_a_build_where_every_memory_fails_settles_nothing_as_complete( + concepts_env, monkeypatch +): + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "a"), ("m2", "b")]) + _extract(monkeypatch, concepts, {"a": Exception, "b": Exception}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1", "m2"])) + + assert set(outcomes.values()) == {"failed"} + + +def test_deleted_memory_reports_vanished(concepts_env, monkeypatch): + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1", "m-gone"])) + + assert outcomes == {"m1": "indexed", "m-gone": "vanished"} + + +def test_compaction_summary_reports_vanished_rather_than_being_indexed( + concepts_env, monkeypatch +): + """A memory queued before its session compacted can be a summary by the + time the worker reaches it. The graph does not index summaries, so the + task is finished rather than retried forever.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "summary text", "summary")]) + _extract(monkeypatch, concepts, {"summary text": ["Should Not Appear"]}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1"])) + + assert outcomes == {"m1": "vanished"} + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + assert conn.execute("SELECT COUNT(*) FROM entities").fetchone()[0] == 0 + + +def test_compaction_source_is_indexed(concepts_env, monkeypatch): + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "source text", "source")]) + _extract(monkeypatch, concepts, {"source text": ["Real Concept"]}) + + outcomes = asyncio.run(concepts.build_for_memory_ids(["m1"])) + + assert outcomes == {"m1": "indexed"} + + +def test_refuses_to_write_into_a_graph_awaiting_rebuild(concepts_env, monkeypatch): + """Mixing incremental writes into a graph already flagged for rebuild + would put two extraction rules in one database, and settling those tasks + would mean the rebuild never sees them.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + + with pytest.raises(RuntimeError, match="rebuild_required"): + asyncio.run(concepts.build_for_memory_ids(["m1"])) + + +def test_refuses_when_concept_extraction_is_unavailable(concepts_env, monkeypatch): + """Returning no_entities here would delete every task written while + spaCy was missing, dropping those memories from the graph for good.""" + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + monkeypatch.setattr(concepts, "CONCEPTS_AVAILABLE", False) + + with pytest.raises(RuntimeError, match="unavailable"): + asyncio.run(concepts.build_for_memory_ids(["m1"])) + + +def test_empty_id_list_is_a_no_op(concepts_env): + concepts, _memory_module = concepts_env + assert asyncio.run(concepts.build_for_memory_ids([])) == {} + + +def test_a_full_build_retires_the_queue_it_just_covered(concepts_env, monkeypatch): + """After the v2.36.0 forced rebuild the queue holds the entire corpus. If + the build that just indexed it does not clear those rows, the worker + extracts every memory a second time.""" + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + queue = importlib.import_module("marm_mcp_server.core.concept_queue") + _seed(memory_module, [("m1", "alpha"), ("m2", "beta")]) + with memory_module.memory.get_connection() as conn: + queue.enqueue(conn, "m1", "h1") + queue.enqueue(conn, "m2", "h2") + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = '2020-01-01T00:00:00+00:00'" + ) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"], "beta": ["Beta"]}) + + asyncio.run(concepts.marm_concept_build(ConceptBuildRequest(search_all=True))) + + assert queue.counts() == {"pending": 0, "parked": 0} + + +def test_a_build_leaves_the_queue_row_of_a_memory_it_failed_to_extract( + concepts_env, monkeypatch +): + """Retirement keyed on the build succeeding overall, rather than on each + memory's outcome, would drop the retry for exactly the memories that need + one.""" + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + queue = importlib.import_module("marm_mcp_server.core.concept_queue") + _seed(memory_module, [("m1", "alpha"), ("m2", "poison")]) + with memory_module.memory.get_connection() as conn: + queue.enqueue(conn, "m1", "h1") + queue.enqueue(conn, "m2", "h2") + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = '2020-01-01T00:00:00+00:00'" + ) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"], "poison": Exception}) + + asyncio.run(concepts.marm_concept_build(ConceptBuildRequest(search_all=True))) + + with memory_module.memory.get_connection() as conn: + remaining = [ + row[0] + for row in conn.execute( + "SELECT memory_id FROM concept_index_queue" + ).fetchall() + ] + assert remaining == ["m2"] + + +def test_a_memory_written_during_a_build_keeps_its_queue_row(concepts_env, monkeypatch): + """Its enqueue is newer than the build's start, so the cutoff protects it + even though the build reported no outcome for it.""" + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + queue = importlib.import_module("marm_mcp_server.core.concept_queue") + _seed(memory_module, [("m1", "alpha")]) + with memory_module.memory.get_connection() as conn: + queue.enqueue(conn, "m1", "h1") + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = '2020-01-01T00:00:00+00:00'" + ) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + real_run_build = concepts._run_build + + def build_then_write(pages, outcomes=None, abort=None): + result = real_run_build(pages, outcomes, abort) + _seed(memory_module, [("m2", "written mid build")]) + with memory_module.memory.get_connection() as conn: + queue.enqueue(conn, "m2", "h2") + return result + + monkeypatch.setattr(concepts, "_run_build", build_then_write) + + asyncio.run(concepts.marm_concept_build(ConceptBuildRequest(search_all=True))) + + with memory_module.memory.get_connection() as conn: + remaining = [ + row[0] + for row in conn.execute( + "SELECT memory_id FROM concept_index_queue" + ).fetchall() + ] + assert remaining == ["m2"] + + +def test_a_rebuild_that_dies_partway_still_asks_to_be_rebuilt( + concepts_env, monkeypatch +): + """The reset drops the old graph before the corpus is extracted. Stamping + the new schema version there would make an interrupted rebuild report + `current`, so nothing prompts for it again and the corpus is silently + missing from the graph, with no queue rows to recover it either.""" + from marm_mcp_server.core.concept_db import inspect_concept_schema + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + concept_db = concepts._get_concept_db() + db_path = concept_db.db_path + with concept_db.get_connection() as conn: + concept_db.get_or_create_entity( + conn, "legacy", "concept", "sess-a", None, "old", platform="cli" + ) + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + assert inspect_concept_schema(db_path) == "rebuild_required" + + def explode(*_args, **_kwargs): + raise RuntimeError("rebuild died partway") + + monkeypatch.setattr(concepts, "_run_build", explode) + asyncio.run(concepts.marm_concept_build(ConceptBuildRequest(search_all=True))) + + assert inspect_concept_schema(db_path) == "rebuild_required" + + +def test_a_completed_rebuild_marks_the_schema_current(concepts_env, monkeypatch): + from marm_mcp_server.core.concept_db import inspect_concept_schema + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + concept_db = concepts._get_concept_db() + db_path = concept_db.db_path + with concept_db.get_connection() as conn: + concept_db.get_or_create_entity( + conn, "legacy", "concept", "sess-a", None, "old", platform="cli" + ) + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + + result = asyncio.run( + concepts.marm_concept_build(ConceptBuildRequest(search_all=True)) + ) + + assert result["graph_rebuilt"] is True + assert inspect_concept_schema(db_path) == "current" + + +def test_a_manual_build_refuses_while_another_process_holds_the_graph( + concepts_env, monkeypatch +): + """The rebuild path backs up and drops the graph tables. It must not run + while the other transport's worker is writing into them, and the + in-process asyncio lock cannot see that worker at all.""" + from marm_mcp_server.core import concept_build_lock + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + assert concept_build_lock.try_acquire("other-process", "auto_index", 300) is True + + result = asyncio.run( + concepts.marm_concept_build(ConceptBuildRequest(search_all=True)) + ) + + assert result["error_code"] == "build_in_progress" + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + assert conn.execute("SELECT COUNT(*) FROM entities").fetchone()[0] == 0 + + +def test_a_manual_build_runs_once_the_graph_is_free(concepts_env, monkeypatch): + from marm_mcp_server.core import concept_build_lock + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + concept_build_lock.try_acquire("other-process", "auto_index", 300) + concept_build_lock.release("other-process") + + result = asyncio.run( + concepts.marm_concept_build(ConceptBuildRequest(search_all=True)) + ) + + assert result.get("error_code") is None + assert result["entities_extracted"] == 1 + + +def test_a_manual_build_releases_the_lock_even_when_it_fails(concepts_env, monkeypatch): + """A build that raises must not leave the graph locked for an hour.""" + from marm_mcp_server.core import concept_build_lock + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + + def explode(*_args, **_kwargs): + raise RuntimeError("build blew up") + + monkeypatch.setattr(concepts, "_run_build", explode) + + asyncio.run(concepts.marm_concept_build(ConceptBuildRequest(search_all=True))) + + assert concept_build_lock.current_holder() is None + + +def test_route_response_never_carries_the_per_memory_outcome_map( + concepts_env, monkeypatch +): + """A full build over a large corpus would otherwise attach one entry per + memory to an MCP response bounded at 1MB.""" + from marm_mcp_server.core.models import ConceptBuildRequest + + concepts, memory_module = concepts_env + _seed(memory_module, [("m1", "alpha")]) + _extract(monkeypatch, concepts, {"alpha": ["Alpha"]}) + + result = asyncio.run( + concepts.marm_concept_build(ConceptBuildRequest(search_all=True)) + ) + + assert "outcomes" not in result + assert result["memories_processed"] == 1 diff --git a/marm-mcp-server/tests/test_concept_queue.py b/marm-mcp-server/tests/test_concept_queue.py new file mode 100644 index 00000000..cbaecbcc --- /dev/null +++ b/marm-mcp-server/tests/test_concept_queue.py @@ -0,0 +1,316 @@ +"""Tests for the durable concept indexing outbox. + +Everything here runs against real SQLite through the real write paths. The +queue's whole reason to exist is that a task survives things an in-memory set +would not, so mocking the database out would test nothing worth testing. +""" + +import asyncio +import importlib +import sys +from datetime import datetime, timedelta, timezone + +import pytest +from conftest import load_isolated_server + + +@pytest.fixture +def queue_env(monkeypatch, tmp_path): + load_isolated_server(monkeypatch, tmp_path) + memory_module = sys.modules["marm_mcp_server.core.memory"] + concept_queue = importlib.import_module("marm_mcp_server.core.concept_queue") + return memory_module.memory, concept_queue + + +def _rows(mem): + with mem.get_connection() as conn: + return conn.execute( + "SELECT memory_id, content_hash, state, attempts, last_error, lease_token " + "FROM concept_index_queue ORDER BY enqueued_at, memory_id" + ).fetchall() + + +def _seed_task(mem, queue, memory_id, content_hash="h1"): + """Insert the memory and its task through enqueue itself.""" + with mem.get_connection() as conn: + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, content_hash) " + "VALUES (?, 'sess-a', 'content', datetime('now'), ?)", + (memory_id, content_hash), + ) + queue.enqueue(conn, memory_id, content_hash) + + +def test_storing_a_memory_queues_it(queue_env): + mem, _queue = queue_env + memory_id = asyncio.run(mem.store_memory("the write queue serializes writes", "s1")) + + rows = _rows(mem) + assert [row[0] for row in rows] == [memory_id] + assert rows[0][2] == "pending" + with mem.get_connection() as conn: + stored_hash = conn.execute( + "SELECT content_hash FROM memories WHERE id = ?", (memory_id,) + ).fetchone()[0] + assert rows[0][1] == stored_hash + + +def test_a_rolled_back_write_leaves_no_task(queue_env): + """The enqueue shares the memory INSERT's transaction, which is the entire + argument for keeping this table in the memory database.""" + mem, queue = queue_env + with mem.get_connection() as conn: + conn.execute("BEGIN IMMEDIATE") + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, content_hash) " + "VALUES ('m1', 'sess-a', 'content', datetime('now'), 'h1')" + ) + queue.enqueue(conn, "m1", "h1") + conn.execute("ROLLBACK") + + assert _rows(mem) == [] + with mem.get_connection() as conn: + assert conn.execute("SELECT COUNT(*) FROM memories").fetchone()[0] == 0 + + +def test_merge_requeues_with_the_new_hash_and_resets_attempts(queue_env): + """A merge reuses the memory_id. Dedup on the id alone would treat the + merged content as already indexed and never extract it.""" + mem, _queue = queue_env + memory_id = asyncio.run(mem.store_memory("original content about auth", "s1")) + with mem.get_connection() as conn: + original_hash = conn.execute( + "SELECT content_hash FROM memories WHERE id = ?", (memory_id,) + ).fetchone()[0] + conn.execute( + "UPDATE concept_index_queue SET attempts = 2, state = 'parked', " + "last_error = 'boom' WHERE memory_id = ?", + (memory_id,), + ) + + assert asyncio.run(mem.update_memory(memory_id, "additional content")) is True + + rows = _rows(mem) + assert len(rows) == 1 + memory_id_row, content_hash, state, attempts, last_error, _token = rows[0] + assert memory_id_row == memory_id + assert content_hash != original_hash + assert (state, attempts, last_error) == ("pending", 0, None) + + +def test_deleting_a_memory_drops_its_task(queue_env): + """A task pointing at a deleted memory would be claimed until it burned + the attempt budget.""" + from marm_mcp_server.core.memory_delete import _delete_memories + + mem, _queue = queue_env + memory_id = asyncio.run(mem.store_memory("content to delete", "s1")) + assert _rows(mem) + + result = asyncio.run(_delete_memories(mem, [memory_id])) + + assert result["deleted_ids"] == [memory_id] + assert _rows(mem) == [] + + +def test_claim_leases_rows_and_a_second_claim_gets_nothing(queue_env): + """Two processes share one memory DB and the in-process build lock cannot + reach across them. The lease is what keeps them off each other's tasks.""" + mem, queue = queue_env + _seed_task(mem, queue, "m1") + _seed_task(mem, queue, "m2") + + first = queue.claim(10) + second = queue.claim(10) + + assert sorted(task.memory_id for task in first) == ["m1", "m2"] + assert second == [] + assert {row[2] for row in _rows(mem)} == {"leased"} + + +def test_claim_respects_the_batch_limit(queue_env): + mem, queue = queue_env + for index in range(5): + _seed_task(mem, queue, f"m{index}") + + assert len(queue.claim(2)) == 2 + assert len(queue.claim(2)) == 2 + assert len(queue.claim(2)) == 1 + + +def test_an_expired_lease_is_reclaimed_without_burning_an_attempt(queue_env): + """A worker killed mid-extraction must not cost the task an attempt: it + never failed, its process died.""" + mem, queue = queue_env + _seed_task(mem, queue, "m1") + first = queue.claim(1) + assert len(first) == 1 + + expired = (datetime.now(timezone.utc) - timedelta(seconds=1)).isoformat() + with mem.get_connection() as conn: + conn.execute("UPDATE concept_index_queue SET leased_until = ?", (expired,)) + + reclaimed = queue.claim(1) + + assert len(reclaimed) == 1 + assert reclaimed[0].memory_id == "m1" + assert reclaimed[0].lease_token != first[0].lease_token + assert _rows(mem)[0][3] == 0 + + +def test_complete_with_a_stale_token_is_rejected(queue_env): + """The task was reclaimed by another worker while this one was running. + Its result belongs to a lease it no longer holds.""" + mem, queue = queue_env + _seed_task(mem, queue, "m1") + stale = queue.claim(1)[0] + expired = (datetime.now(timezone.utc) - timedelta(seconds=1)).isoformat() + with mem.get_connection() as conn: + conn.execute("UPDATE concept_index_queue SET leased_until = ?", (expired,)) + queue.claim(1) + + assert queue.complete("m1", stale.lease_token, stale.content_hash) is False + assert len(_rows(mem)) == 1 + + +def test_complete_with_a_superseded_hash_is_rejected(queue_env): + """The memory was merged while the extraction ran, so this result covers + text that is no longer the whole memory.""" + mem, queue = queue_env + _seed_task(mem, queue, "m1", "old-hash") + task = queue.claim(1)[0] + with mem.get_connection() as conn: + queue.enqueue(conn, "m1", "new-hash") + + assert queue.complete("m1", task.lease_token, "old-hash") is False + rows = _rows(mem) + assert rows[0][1] == "new-hash" + assert rows[0][2] == "pending" + + +def test_complete_retires_the_task(queue_env): + mem, queue = queue_env + _seed_task(mem, queue, "m1", "h1") + task = queue.claim(1)[0] + + assert queue.complete("m1", task.lease_token, "h1") is True + assert _rows(mem) == [] + + +def test_fail_records_the_error_and_returns_the_task_to_pending(queue_env): + mem, queue = queue_env + _seed_task(mem, queue, "m1") + task = queue.claim(1)[0] + + assert queue.fail("m1", task.lease_token, "extraction_failed") is True + + row = _rows(mem)[0] + assert row[2] == "pending" + assert row[3] == 1 + assert row[4] == "extraction_failed" + assert row[5] is None + + +def test_a_failed_task_backs_off_before_it_can_be_claimed_again(queue_env): + """The worker drains until the queue is empty, so a task returned straight + to pending would be re-claimed by the next loop iteration and burn every + attempt in milliseconds.""" + mem, queue = queue_env + _seed_task(mem, queue, "m1") + task = queue.claim(1)[0] + + queue.fail("m1", task.lease_token, "boom") + + assert queue.claim(1) == [] + with mem.get_connection() as conn: + state, leased_until = conn.execute( + "SELECT state, leased_until FROM concept_index_queue" + ).fetchone() + assert state == "pending" + assert leased_until is not None + + +def test_a_task_is_parked_at_the_attempt_cap_and_never_claimed_again( + queue_env, monkeypatch +): + """One poison memory must not sit at the head of the queue forever.""" + mem, queue = queue_env + from marm_mcp_server.config.settings import CONCEPT_INDEX_MAX_ATTEMPTS + + monkeypatch.setattr(queue, "CONCEPT_INDEX_DEBOUNCE_SECONDS", 0) + _seed_task(mem, queue, "poison") + _seed_task(mem, queue, "healthy") + + for _ in range(CONCEPT_INDEX_MAX_ATTEMPTS): + tasks = {task.memory_id: task for task in queue.claim(10)} + queue.fail("poison", tasks["poison"].lease_token, "boom") + if "healthy" in tasks: + queue.complete("healthy", tasks["healthy"].lease_token, "h1") + + states = {row[0]: row[2] for row in _rows(mem)} + assert states == {"poison": "parked"} + assert queue.claim(10) == [] + + +def test_retire_indexed_clears_covered_tasks_only(queue_env): + """After a full build: rows it settled go, rows queued during it stay.""" + mem, queue = queue_env + cutoff = datetime.now(timezone.utc).isoformat() + _seed_task(mem, queue, "before-1") + _seed_task(mem, queue, "before-2") + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = ? WHERE memory_id LIKE 'before%'", + ((datetime.now(timezone.utc) - timedelta(minutes=5)).isoformat(),), + ) + _seed_task(mem, queue, "during") + + retired = queue.retire_indexed(["before-1", "before-2", "during"], cutoff) + + assert retired == 2 + assert [row[0] for row in _rows(mem)] == ["during"] + + +def test_retire_indexed_leaves_a_task_another_worker_holds(queue_env): + mem, queue = queue_env + _seed_task(mem, queue, "m1") + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = ?", + ((datetime.now(timezone.utc) - timedelta(minutes=5)).isoformat(),), + ) + queue.claim(1) + + assert queue.retire_indexed(["m1"], datetime.now(timezone.utc).isoformat()) == 0 + assert len(_rows(mem)) == 1 + + +def test_retire_indexed_handles_more_ids_than_sqlite_takes_parameters(queue_env): + """A full-corpus build settles far more ids than SQLite's 999-parameter + ceiling allows in one statement.""" + mem, queue = queue_env + ids = [f"m{index:05d}" for index in range(1500)] + with mem.get_connection() as conn: + for memory_id in ids: + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, content_hash) " + "VALUES (?, 'sess-a', 'content', datetime('now'), 'h1')", + (memory_id,), + ) + queue.enqueue(conn, memory_id, "h1") + conn.execute( + "UPDATE concept_index_queue SET enqueued_at = ?", + ((datetime.now(timezone.utc) - timedelta(minutes=5)).isoformat(),), + ) + + retired = queue.retire_indexed(ids, datetime.now(timezone.utc).isoformat()) + + assert retired == 1500 + assert _rows(mem) == [] + + +def test_current_hashes_reports_a_missing_memory_by_omission(queue_env): + mem, queue = queue_env + _seed_task(mem, queue, "m1", "h1") + + assert queue.current_hashes(["m1", "gone"]) == {"m1": "h1"} diff --git a/marm-mcp-server/tests/test_concept_two_process.py b/marm-mcp-server/tests/test_concept_two_process.py new file mode 100644 index 00000000..26042b6d --- /dev/null +++ b/marm-mcp-server/tests/test_concept_two_process.py @@ -0,0 +1,323 @@ +"""Two real processes over one memory database. + +Every other queue test runs sequential calls in one interpreter, which +exercises the SQL but not the thing the lease and the build lock exist for: +an HTTP server and a STDIO session are separate processes, and neither +asyncio.Lock nor a Python-level set reaches across that boundary. These tests +spawn a genuine second interpreter against the same database file. +""" + +import asyncio +import json +import sqlite3 +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _run_in_second_process(db_path: Path, body: str, env_extra=None) -> dict: + """Execute body in a fresh interpreter pointed at the same memory DB. + + Prints a single JSON line, so anything the server writes to stderr on + import cannot corrupt the result. + """ + prelude = ( + "import json, os, sys\n" + f"os.environ['MARM_DB_PATH'] = {str(db_path)!r}\n" + f"os.environ['MARM_ANALYTICS_DB_PATH'] = " + f"{str(db_path.parent / 'analytics.db')!r}\n" + "os.environ['WRITE_QUEUE_ENABLED'] = '0'\n" + f"sys.path.insert(0, {str(REPO_ROOT)!r})\n" + ) + script = ( + prelude + + textwrap.dedent(body).strip() + + "\nprint('RESULT ' + json.dumps(result))\n" + ) + env = {**dict(__import__("os").environ), **(env_extra or {})} + completed = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=120, + env=env, + ) + if completed.returncode != 0: + pytest.fail(f"second process failed:\n{completed.stdout}\n{completed.stderr}") + for line in completed.stdout.splitlines(): + if line.startswith("RESULT "): + return json.loads(line[len("RESULT ") :]) + pytest.fail(f"second process printed no result:\n{completed.stdout}") + + +@pytest.fixture +def shared_db(monkeypatch, tmp_path): + from conftest import load_isolated_server + + load_isolated_server(monkeypatch, tmp_path) + import sys as _sys + + memory_module = _sys.modules["marm_mcp_server.core.memory"] + return memory_module.memory, tmp_path / "marm_memory.db" + + +def _seed_task(mem, memory_id, content_hash="h1"): + from marm_mcp_server.core import concept_queue + + with mem.get_connection() as conn: + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp, content_hash) " + "VALUES (?, 'sess-a', 'content', datetime('now'), ?)", + (memory_id, content_hash), + ) + concept_queue.enqueue(conn, memory_id, content_hash) + + +def test_a_second_process_cannot_claim_a_task_this_one_holds(shared_db): + """The reason the queue carries a lease at all. An asyncio.Lock would let + both processes extract the same memory at the same time.""" + from marm_mcp_server.core import concept_queue + + mem, db_path = shared_db + _seed_task(mem, "m1") + _seed_task(mem, "m2") + + mine = concept_queue.claim(10) + assert sorted(task.memory_id for task in mine) == ["m1", "m2"] + + theirs = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import concept_queue + result = [t.memory_id for t in concept_queue.claim(10)] + """, + ) + + assert theirs == [] + + +def test_a_second_process_reclaims_a_task_whose_lease_expired(shared_db): + """A process killed mid-extraction must not strand its tasks.""" + from marm_mcp_server.core import concept_queue + + mem, db_path = shared_db + _seed_task(mem, "m1") + concept_queue.claim(1) + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_index_queue SET leased_until = '2000-01-01T00:00:00+00:00'" + ) + + theirs = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import concept_queue + result = [t.memory_id for t in concept_queue.claim(10)] + """, + ) + + assert theirs == ["m1"] + + +def test_a_second_process_cannot_take_the_build_lock(shared_db): + """A manual rebuild drops the graph tables. It must not run while the + other transport's worker is writing to them.""" + from marm_mcp_server.core import concept_build_lock + + _mem, db_path = shared_db + assert concept_build_lock.try_acquire("worker-a", "auto_index", 300) is True + + result = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import concept_build_lock + result = { + "acquired": concept_build_lock.try_acquire("build-b", "manual_build", 60), + "holder": concept_build_lock.current_holder(), + } + """, + ) + + assert result["acquired"] is False + assert result["holder"][0] == "auto_index" + + +def test_a_second_process_takes_the_lock_once_it_is_released(shared_db): + from marm_mcp_server.core import concept_build_lock + + _mem, db_path = shared_db + assert concept_build_lock.try_acquire("worker-a", "auto_index", 300) is True + assert concept_build_lock.release("worker-a") is True + + result = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import concept_build_lock + result = {"acquired": concept_build_lock.try_acquire("build-b", "manual_build", 60)} + """, + ) + + assert result["acquired"] is True + + +def test_an_expired_build_lock_does_not_wedge_the_graph_forever(shared_db): + """A crashed holder must not stop every future build.""" + from marm_mcp_server.core import concept_build_lock + + mem, db_path = shared_db + concept_build_lock.try_acquire("crashed", "manual_build", 3600) + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_build_lock SET expires_at = '2000-01-01T00:00:00+00:00'" + ) + + result = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import concept_build_lock + result = {"acquired": concept_build_lock.try_acquire("next", "auto_index", 60)} + """, + ) + + assert result["acquired"] is True + + +@pytest.mark.asyncio +async def test_work_outliving_its_ttl_keeps_both_locks(shared_db): + """The lock has to survive work that runs longer than the lease, or it is + a deadline rather than a lock. A full rebuild has no bounded runtime, and + the task leases expire on the same clock, so an unrenewed pair would let + a second process both write the graph and re-extract the same memories. + """ + import asyncio + + from marm_mcp_server.core import concept_build_lock, concept_queue + + mem, db_path = shared_db + _seed_task(mem, "m1") + ttl = 1 + + async with concept_build_lock.concept_build_lock("manual_build", ttl): + tasks = concept_queue.claim(10) + assert len(tasks) == 1 + async with concept_queue.keep_claimed(tasks, ttl): + # Three times the lease. Without renewal both are long expired. + await asyncio.sleep(ttl * 3) + + # Through a thread, because subprocess.run is synchronous: called + # directly it blocks the event loop for the child's whole startup, + # which is exactly when the heartbeat needs to be renewing. That + # would make the test fail for the opposite of the real reason. + other = await asyncio.to_thread( + _run_in_second_process, + db_path, + """ + from marm_mcp_server.core import concept_build_lock, concept_queue + result = { + "took_lock": concept_build_lock.try_acquire("b", "manual_build", 60), + "claimed": [t.memory_id for t in concept_queue.claim(10)], + } + """, + ) + + assert other["took_lock"] is False, "a second process overtook a live build" + assert other["claimed"] == [], "a second process re-claimed a live task" + + +@pytest.mark.asyncio +async def test_both_locks_are_free_again_once_the_work_finishes(shared_db): + """Renewal must not outlive the body it was protecting.""" + from marm_mcp_server.core import concept_build_lock, concept_queue + + mem, db_path = shared_db + _seed_task(mem, "m1") + + async with concept_build_lock.concept_build_lock("manual_build", 1): + tasks = concept_queue.claim(10) + async with concept_queue.keep_claimed(tasks, 1): + pass + concept_queue.complete("m1", tasks[0].lease_token, "h1") + + result = await asyncio.to_thread( + _run_in_second_process, + db_path, + """ + from marm_mcp_server.core import concept_build_lock + result = {"took_lock": concept_build_lock.try_acquire("b", "auto_index", 60)} + """, + ) + + assert result["took_lock"] is True + + +@pytest.mark.asyncio +async def test_a_lease_that_cannot_be_renewed_gives_up_at_the_ttl(shared_db): + """A renewal that keeps raising is indistinguishable from one refused: the + lease runs out either way and another process can take the graph. Logging + warnings while still writing is the one outcome that must not happen.""" + from marm_mcp_server.core import concept_build_lock + + _mem, _db_path = shared_db + + def always_fails(_holder, _ttl): + raise sqlite3.OperationalError("database is locked") + + monkeypatch = pytest.MonkeyPatch() + monkeypatch.setattr(concept_build_lock, "renew", always_fails) + try: + async with concept_build_lock.concept_build_lock("auto_index", 1) as lease: + await asyncio.sleep(2.5) + assert lease.lost.is_set(), ( + "the holder kept going with a lease it could not renew" + ) + finally: + monkeypatch.undo() + + +def test_renew_refuses_once_someone_else_owns_the_lock(shared_db): + """A stalled holder must learn it lost the graph rather than quietly + extending a lease it no longer has.""" + from marm_mcp_server.core import concept_build_lock + + mem, _db_path = shared_db + concept_build_lock.try_acquire("stalled", "auto_index", 3600) + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_build_lock SET expires_at = '2000-01-01T00:00:00+00:00'" + ) + concept_build_lock.try_acquire("new-owner", "manual_build", 300) + + assert concept_build_lock.renew("stalled", 300) is False + assert concept_build_lock.current_holder()[0] == "manual_build" + + +def test_queue_renew_only_extends_tasks_we_still_hold(shared_db): + from marm_mcp_server.core import concept_queue + + mem, _db_path = shared_db + _seed_task(mem, "m1") + task = concept_queue.claim(1)[0] + + assert concept_queue.renew([task.memory_id], task.lease_token, 600) == 1 + assert concept_queue.renew([task.memory_id], "not-our-token", 600) == 0 + + +def test_releasing_after_expiry_does_not_steal_the_new_holders_lock(shared_db): + """The late finisher must not delete a lock somebody else now owns.""" + from marm_mcp_server.core import concept_build_lock + + mem, _db_path = shared_db + concept_build_lock.try_acquire("slow-worker", "auto_index", 3600) + with mem.get_connection() as conn: + conn.execute( + "UPDATE concept_build_lock SET expires_at = '2000-01-01T00:00:00+00:00'" + ) + assert concept_build_lock.try_acquire("new-owner", "manual_build", 300) is True + + assert concept_build_lock.release("slow-worker") is False + assert concept_build_lock.current_holder()[0] == "manual_build" diff --git a/marm-mcp-server/tests/test_concept_worker.py b/marm-mcp-server/tests/test_concept_worker.py new file mode 100644 index 00000000..b0166462 --- /dev/null +++ b/marm-mcp-server/tests/test_concept_worker.py @@ -0,0 +1,621 @@ +"""Tests for the background concept indexing worker. + +The worker's job is to settle durable queue rows correctly. Most of these +tests therefore assert on what is left in the queue and in the graph after a +cycle, not on how many times something was called. Real SQLite throughout; +extract_entities is monkeypatched at the endpoints module boundary, the same +convention the other concept tests use, because spaCy's model is not +installable in this sandbox. +""" + +import asyncio +import importlib +import sys +import time + +import pytest +from conftest import load_isolated_server + + +@pytest.fixture +def worker_env(monkeypatch, tmp_path): + load_isolated_server(monkeypatch, tmp_path) + monkeypatch.setenv("MARM_CONCEPT_DB_PATH", str(tmp_path / "marm_index.db")) + concepts = importlib.import_module("marm_mcp_server.endpoints.concepts") + worker_module = importlib.import_module("marm_mcp_server.core.concept_worker") + queue = importlib.import_module("marm_mcp_server.core.concept_queue") + memory_module = sys.modules["marm_mcp_server.core.memory"] + + monkeypatch.setattr(concepts, "CONCEPTS_AVAILABLE", True) + monkeypatch.setattr(concepts, "is_graph_available", lambda: False) + monkeypatch.setattr(worker_module, "CONCEPTS_AVAILABLE", True) + monkeypatch.setattr(worker_module, "CONCEPT_AUTO_INDEX", True) + monkeypatch.setattr(worker_module, "CONCEPT_INDEX_DEBOUNCE_SECONDS", 0.01) + # The shipped pause is a latency tradeoff, not behavior. Off here so it + # does not add seconds to every drain test; covered on its own below. + monkeypatch.setattr(worker_module, "CONCEPT_INDEX_BATCH_PAUSE_MS", 0) + + worker = worker_module.ConceptIndexWorker() + return worker, worker_module, concepts, queue, memory_module.memory + + +def _extract_named_after_content(monkeypatch, concepts, failing=()): + from marm_mcp_server.core.concept_extraction import Entity, ExtractionResult + + def fake(content): + if content in failing: + raise RuntimeError("extraction failed") + if not content.strip(): + return ExtractionResult(entities=[], relationship_pairs=[]) + return ExtractionResult( + entities=[Entity(content, "concept")], relationship_pairs=[] + ) + + monkeypatch.setattr(concepts, "extract_entities", fake) + + +def _queue_rows(mem): + with mem.get_connection() as conn: + return conn.execute( + "SELECT memory_id, state, attempts FROM concept_index_queue" + ).fetchall() + + +def _entity_names(concepts): + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + return {row[0] for row in conn.execute("SELECT name FROM entities").fetchall()} + + +def test_a_stored_memory_becomes_a_node_without_anyone_asking(worker_env, monkeypatch): + """The whole feature in one test: store, wait, the node exists and the + task is gone.""" + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + await mem.store_memory("the write queue serializes writes", "s1") + worker.start() + for _ in range(200): + await asyncio.sleep(0.01) + if not _queue_rows(mem): + break + await worker.stop() + + asyncio.run(scenario()) + + assert _queue_rows(mem) == [] + assert "the write queue serializes writes" in _entity_names(concepts) + + +def test_a_backlog_drains_continuously_instead_of_one_batch_per_interval( + worker_env, monkeypatch +): + """Claiming one batch then waiting again would cap throughput at + CONCEPT_INDEX_BATCH_SIZE per debounce interval. With the shipped defaults + that is 40 memories a minute and a backlog never catches up.""" + worker, module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + monkeypatch.setattr(module, "CONCEPT_INDEX_BATCH_SIZE", 5) + monkeypatch.setattr(module, "CONCEPT_INDEX_DEBOUNCE_SECONDS", 30) + + async def scenario(): + for index in range(50): + await mem.store_memory(f"memory number {index}", "s1") + # One cycle only. Reaching all 50 proves the drain loops rather than + # returning to the (30 second) wait after its first batch. + await worker._drain() + + asyncio.run(scenario()) + + assert _queue_rows(mem) == [] + assert len(_entity_names(concepts)) == 50 + + +def test_a_failed_extraction_keeps_the_task_and_records_the_error( + worker_env, monkeypatch +): + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts, failing={"poison content"}) + + async def scenario(): + good = await mem.store_memory("healthy content", "s1") + bad = await mem.store_memory("poison content", "s1") + await worker._drain() + return good, bad + + _good, bad = asyncio.run(scenario()) + + rows = {row[0]: row for row in _queue_rows(mem)} + assert list(rows) == [bad] + assert rows[bad][1] == "pending" + assert rows[bad][2] == 1 + assert "healthy content" in _entity_names(concepts) + + +def test_a_memory_deleted_mid_extraction_leaves_nothing_in_the_graph( + worker_env, monkeypatch +): + """Dequeue-on-delete cannot cover this on its own. The build reads the + memory DB and then writes the concept DB, and a delete can commit and run + its own cleanup inside that gap, so the entities the user asked to remove + would reappear behind it.""" + from marm_mcp_server.core.memory_delete import _delete_memories + + worker, _module, concepts, queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + memory_id = await mem.store_memory("doomed content", "s1") + real_build = concepts.build_for_memory_ids + + async def build_then_delete(memory_ids, abort=None, finished=None): + outcomes = await real_build(memory_ids, abort=abort) + await _delete_memories(mem, [memory_id]) + return outcomes + + monkeypatch.setattr(concepts, "build_for_memory_ids", build_then_delete) + tasks = await asyncio.to_thread(queue.claim, 10) + await worker._process(tasks) + + asyncio.run(scenario()) + + assert _queue_rows(mem) == [] + assert _entity_names(concepts) == set() + + +def test_a_memory_merged_mid_extraction_is_reindexed_not_settled( + worker_env, monkeypatch +): + """Settling on the old hash would leave the graph describing text that is + only part of the memory, with nothing queued to correct it.""" + worker, _module, concepts, queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + memory_id = await mem.store_memory("original content", "s1") + real_build = concepts.build_for_memory_ids + + async def build_then_merge(memory_ids, abort=None, finished=None): + outcomes = await real_build(memory_ids, abort=abort) + await mem.update_memory(memory_id, "appended content") + return outcomes + + monkeypatch.setattr(concepts, "build_for_memory_ids", build_then_merge) + # One batch, not a full drain: the assertion is about what the worker + # does with a result that arrived after the memory moved on. + tasks = await asyncio.to_thread(queue.claim, 10) + await worker._process(tasks) + return memory_id + + memory_id = asyncio.run(scenario()) + + rows = _queue_rows(mem) + assert [row[0] for row in rows] == [memory_id] + assert rows[0][1] == "pending" + assert rows[0][2] == 0 + + +def test_a_superseded_result_does_not_wipe_another_workers_fresh_provenance( + worker_env, monkeypatch +): + """Two processes, one memory. Worker A is extracting the old content when + the memory is rewritten; worker B indexes the new content and finishes + first. cleanup_deleted_memory_provenance removes ALL provenance for a + memory id, so if A retracted on finding the hash changed it would erase + B's current graph data, with A's queue row already gone and nothing left + to repair it.""" + worker, _module, concepts, queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + memory_id = await mem.store_memory("original content", "s1") + stale = await asyncio.to_thread(queue.claim, 10) + + # Stand in for the rewrite plus worker B: the memory now holds new + # content and its entities are already in the graph. + await mem.update_memory(memory_id, "appended content") + fresh = await asyncio.to_thread(queue.claim, 10) + await worker._process(fresh) + + # Worker A only now returns, holding a result for text that is gone. + await worker._process(stale) + return memory_id + + asyncio.run(scenario()) + + names = _entity_names(concepts) + assert any("appended content" in name for name in names), ( + f"the current content's entities were erased by a stale result: {names}" + ) + + +def test_losing_the_graph_lock_stops_the_build_at_the_next_memory( + worker_env, monkeypatch +): + """A process stalled past its whole lease loses the lock to someone else. + The running thread cannot be killed from outside, so the build has to stop + cooperatively instead of writing alongside the new owner for however long + it had left.""" + import threading + + _worker, _module, concepts, _queue, mem = worker_env + lost = threading.Event() + seen = [] + + from marm_mcp_server.core.concept_extraction import Entity, ExtractionResult + + def fake(content): + seen.append(content) + lost.set() # the lock goes at the first memory + return ExtractionResult( + entities=[Entity(content, "concept")], relationship_pairs=[] + ) + + monkeypatch.setattr(concepts, "extract_entities", fake) + + async def scenario(): + for index in range(5): + await mem.store_memory(f"memory number {index}", "s1") + pages = concepts._fetch_memory_pages(None, None, True) + return await asyncio.to_thread(concepts._run_build, pages, None, lost) + + result = asyncio.run(scenario()) + + assert result["aborted"] is True + assert len(seen) == 1, f"the build kept going after losing the lock: {seen}" + assert result["memories_processed"] == 1 + + +def test_an_abandoned_batch_settles_nothing(worker_env, monkeypatch): + """Settling part of an abandoned batch would either delete a task whose + extraction never ran, or spend an attempt on a memory that never failed.""" + import threading + + worker, _module, concepts, queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + lost = threading.Event() + + async def scenario(): + for index in range(3): + await mem.store_memory(f"memory number {index}", "s1") + tasks = await asyncio.to_thread(queue.claim, 10) + + real_build = concepts.build_for_memory_ids + + async def build_then_lose(memory_ids, abort=None, finished=None): + lost.set() + return await real_build(memory_ids, abort=abort) + + monkeypatch.setattr(concepts, "build_for_memory_ids", build_then_lose) + await worker._process(tasks, lost) + return tasks + + tasks = asyncio.run(scenario()) + + rows = {row[0]: row for row in _queue_rows(mem)} + assert len(rows) == len(tasks), "an abandoned batch settled some of its tasks" + assert all(row[2] == 0 for row in rows.values()), ( + "an abandoned batch spent attempts on memories that never failed" + ) + + +def test_a_manual_build_that_loses_the_lock_reports_it_rather_than_success( + worker_env, monkeypatch +): + """Reporting success would also retire the queue rows for memories this + build never reached.""" + import threading + + from marm_mcp_server.core.models import ConceptBuildRequest + + _worker, _module, concepts, _queue, mem = worker_env + lost = threading.Event() + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + await mem.store_memory("some content to index", "s1") + lost.set() + return await concepts._marm_concept_build( + ConceptBuildRequest(search_all=True), lost + ) + + result = asyncio.run(scenario()) + + assert result["error_code"] == "lock_lost" + assert len(_queue_rows(mem)) == 1, "an aborted build retired a queue row" + + +def test_a_memory_with_no_entities_is_finished_not_retried(worker_env, monkeypatch): + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + memory_id = await mem.store_memory("some content", "s1") + with mem.get_connection() as conn: + conn.execute( + "UPDATE memories SET content = ' ' WHERE id = ?", (memory_id,) + ) + await worker._drain() + + asyncio.run(scenario()) + + assert _queue_rows(mem) == [] + + +def test_one_bad_cycle_does_not_kill_the_loop(worker_env, monkeypatch): + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + calls = {"n": 0} + real_drain = worker._drain + + async def flaky_drain(): + calls["n"] += 1 + if calls["n"] == 1: + raise RuntimeError("cycle exploded") + await real_drain() + + monkeypatch.setattr(worker, "_drain", flaky_drain) + + async def scenario(): + await mem.store_memory("content that must still be indexed", "s1") + worker.start() + for _ in range(300): + await asyncio.sleep(0.01) + if calls["n"] >= 2 and not _queue_rows(mem): + break + await worker.stop() + + asyncio.run(scenario()) + + assert calls["n"] >= 2 + assert _queue_rows(mem) == [] + + +def test_stop_returns_without_waiting_for_an_in_flight_extraction( + worker_env, monkeypatch +): + """Teardown must not put spaCy on the shutdown path. The task is a durable + row and the next run picks it up. + + stop() does give an aborted extraction a short grace period to stop + writing before the graph lock is released, so this asserts the bound rather + than an instant return: it must not wait out a build that ignores the + abort, which here sleeps far longer than the grace.""" + worker, module, concepts, _queue, mem = worker_env + monkeypatch.setattr(module, "ABORT_GRACE_SECONDS", 0.3) + entered = asyncio.Event() + + async def slow_build(memory_ids, abort=None, finished=None): + entered.set() + await asyncio.sleep(30) + return {} + + monkeypatch.setattr(concepts, "build_for_memory_ids", slow_build) + + async def scenario(): + await mem.store_memory("content", "s1") + worker.start() + await asyncio.wait_for(entered.wait(), timeout=5) + started = asyncio.get_running_loop().time() + await asyncio.wait_for(worker.stop(), timeout=5) + return worker.running, asyncio.get_running_loop().time() - started + + running, elapsed = asyncio.run(scenario()) + + assert running is False + assert elapsed < 3, f"stop() waited {elapsed:.1f}s on a build that ignored abort" + assert len(_queue_rows(mem)) == 1 + + +def test_stop_signals_the_running_build_before_releasing_the_graph( + worker_env, monkeypatch +): + """Cancelling the task only cancels the await around asyncio.to_thread. The + extraction thread keeps writing while unwinding releases the cross-process + lock, so another transport could reset the concept database underneath it. + stop() has to raise the abort flag first.""" + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + seen = {} + + async def slow_build(memory_ids, abort=None, finished=None): + seen["abort"] = abort + await asyncio.sleep(30) + return {} + + monkeypatch.setattr(concepts, "build_for_memory_ids", slow_build) + + async def scenario(): + await mem.store_memory("content being indexed", "s1") + worker.start() + for _ in range(200): + await asyncio.sleep(0.01) + if "abort" in seen: + break + await worker.stop() + + asyncio.run(scenario()) + + assert seen.get("abort") is not None, "the build was given no way to stop" + assert seen["abort"].is_set(), "stop() released the graph without signalling" + + +def test_stop_holds_the_graph_until_the_extraction_thread_stops( + worker_env, monkeypatch +): + """The abort flag alone is not enough. Cancelling unwinds the lock and + releases it, so without waiting for the thread to acknowledge, another + process can take the graph while this one is still writing.""" + import threading + + worker, module, concepts, _queue, mem = worker_env + monkeypatch.setattr(module, "ABORT_GRACE_SECONDS", 3.0) + released_while_running = [] + still_running = threading.Event() + still_running.set() + + async def slow_build(memory_ids, abort=None, finished=None): + # Stands in for a thread that notices the abort and then takes a + # moment to unwind, which is when the lock must still be held. + def work(): + abort.wait(5) + time.sleep(0.4) + if finished is not None: + finished.set() + still_running.clear() + + await asyncio.to_thread(work) + return {} + + monkeypatch.setattr(concepts, "build_for_memory_ids", slow_build) + + from marm_mcp_server.core import concept_build_lock + + real_release = concept_build_lock.release + + def watching_release(holder): + if still_running.is_set(): + released_while_running.append(holder) + return real_release(holder) + + monkeypatch.setattr(concept_build_lock, "release", watching_release) + + async def scenario(): + await mem.store_memory("content being indexed", "s1") + worker.start() + for _ in range(300): + await asyncio.sleep(0.01) + if worker._build_finished is not None: + break + await worker.stop() + + asyncio.run(scenario()) + + assert released_while_running == [], ( + "the graph lock was released while extraction was still writing" + ) + + +def test_disabled_worker_leaves_the_queue_filling(worker_env, monkeypatch): + worker, module, _concepts, _queue, mem = worker_env + monkeypatch.setattr(module, "CONCEPT_AUTO_INDEX", False) + + async def scenario(): + await mem.store_memory("content", "s1") + worker.start() + await asyncio.sleep(0.05) + await worker.stop() + + asyncio.run(scenario()) + + assert worker.running is False + assert len(_queue_rows(mem)) == 1 + + +def test_worker_stays_dormant_when_extraction_is_unavailable(worker_env, monkeypatch): + """Claiming tasks it cannot extract would burn the attempt budget and park + every memory written while the runtime is missing.""" + worker, module, _concepts, _queue, mem = worker_env + monkeypatch.setattr(module, "CONCEPTS_AVAILABLE", False) + + async def scenario(): + await mem.store_memory("content", "s1") + worker.start() + await asyncio.sleep(0.05) + await worker.stop() + + asyncio.run(scenario()) + + assert worker.running is False + rows = _queue_rows(mem) + assert len(rows) == 1 + assert rows[0][2] == 0 + + +def test_a_graph_awaiting_rebuild_does_not_get_incremental_writes( + worker_env, monkeypatch +): + """On upgrade every install starts in rebuild_required, so this is the + normal state until the user runs a full build, not a rare edge.""" + worker, _module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + + async def scenario(): + await mem.store_memory("content", "s1") + concept_db = concepts._get_concept_db() + with concept_db.get_connection() as conn: + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' " + "WHERE key = 'schema_version'" + ) + # The loop swallows the refusal so it can retry after a rebuild. + worker.start() + await asyncio.sleep(0.1) + await worker.stop() + + asyncio.run(scenario()) + + assert len(_queue_rows(mem)) == 1 + assert _entity_names(concepts) == set() + + +def test_the_inter_batch_pause_actually_pauses(worker_env, monkeypatch): + """Measured, not assumed: at the shipped batch size the pause cuts the + worst-case recall during a drain from roughly 270ms to 80ms, and it can + only do that if it is really yielding between batches.""" + worker, module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + monkeypatch.setattr(module, "CONCEPT_INDEX_BATCH_SIZE", 1) + monkeypatch.setattr(module, "CONCEPT_INDEX_BATCH_PAUSE_MS", 120) + + async def scenario(): + for index in range(4): + await mem.store_memory(f"memory number {index}", "s1") + start = asyncio.get_running_loop().time() + await worker._drain() + return asyncio.get_running_loop().time() - start + + elapsed = asyncio.run(scenario()) + + assert _queue_rows(mem) == [] + # Four batches, so at least three pauses land between them. + assert elapsed >= 0.36, f"drain took {elapsed:.3f}s, the pause did not apply" + + +def test_a_stop_during_the_pause_is_not_ignored(worker_env, monkeypatch): + """Shutdown must not wait out a pause it could skip.""" + worker, module, concepts, _queue, mem = worker_env + _extract_named_after_content(monkeypatch, concepts) + monkeypatch.setattr(module, "CONCEPT_INDEX_BATCH_SIZE", 1) + monkeypatch.setattr(module, "CONCEPT_INDEX_BATCH_PAUSE_MS", 30_000) + + async def scenario(): + for index in range(3): + await mem.store_memory(f"memory number {index}", "s1") + drain = asyncio.create_task(worker._drain()) + await asyncio.sleep(0.2) + worker._stop.set() + await asyncio.wait_for(drain, timeout=5) + + asyncio.run(scenario()) + + +def test_start_is_idempotent(worker_env): + worker, _module, _concepts, _queue, _mem = worker_env + + async def scenario(): + worker.start() + first = worker._task + worker.start() + same = worker._task is first + await worker.stop() + return same + + assert asyncio.run(scenario()) is True + + +def test_stop_is_safe_when_never_started(worker_env): + worker, _module, _concepts, _queue, _mem = worker_env + asyncio.run(worker.stop()) + assert worker.running is False diff --git a/marm-mcp-server/tests/test_concept_worker_wiring.py b/marm-mcp-server/tests/test_concept_worker_wiring.py new file mode 100644 index 00000000..fd3a01f4 --- /dev/null +++ b/marm-mcp-server/tests/test_concept_worker_wiring.py @@ -0,0 +1,250 @@ +"""Settings parsing and lifecycle wiring for the concept indexing worker. + +The worker itself is covered in test_concept_worker.py. What is covered here +is everything around it that a unit test of the worker cannot see: whether the +documented env spellings actually take effect, and whether the two servers +really start and stop it. +""" + +import importlib +import sqlite3 +import sys + +import pytest +from conftest import load_isolated_server + + +def _reload_settings(monkeypatch, tmp_path, **env): + """Reload settings with every path it resolves pointed at tmp_path. + + Reload re-executes the module, and module level code resolves the database + path and can create directories and an API key file. The session HOME + sandbox in conftest already keeps that out of the developer's real + ~/.marm, but relying on a distant fixture for that is fragile, so pin the + paths here too. + """ + monkeypatch.setenv("MARM_DB_PATH", str(tmp_path / "settings-probe.db")) + monkeypatch.setenv("MARM_ANALYTICS_DB_PATH", str(tmp_path / "analytics.db")) + monkeypatch.setenv("MARM_CONCEPT_DB_PATH", str(tmp_path / "marm_index.db")) + for key, value in env.items(): + if value is None: + monkeypatch.delenv(key, raising=False) + else: + monkeypatch.setenv(key, value) + settings = importlib.import_module("marm_mcp_server.config.settings") + return importlib.reload(settings) + + +@pytest.fixture(autouse=True) +def restore_settings(): + yield + importlib.reload(importlib.import_module("marm_mcp_server.config.settings")) + + +@pytest.mark.parametrize( + "value", ["false", "False", "FALSE", "0", "no", "off", " off "] +) +def test_the_documented_off_switches_actually_turn_indexing_off( + monkeypatch, tmp_path, value +): + """README and CHANGELOG both tell users CONCEPT_AUTO_INDEX=false. A check + against a single literal read that as on, which is the opposite of what + the user asked for.""" + settings = _reload_settings(monkeypatch, tmp_path, CONCEPT_AUTO_INDEX=value) + assert settings.CONCEPT_AUTO_INDEX is False + + +@pytest.mark.parametrize("value", ["true", "1", "yes", "on", "TRUE"]) +def test_the_on_switches_keep_indexing_on(monkeypatch, tmp_path, value): + settings = _reload_settings(monkeypatch, tmp_path, CONCEPT_AUTO_INDEX=value) + assert settings.CONCEPT_AUTO_INDEX is True + + +def test_indexing_is_on_when_the_variable_is_absent(monkeypatch, tmp_path): + settings = _reload_settings(monkeypatch, tmp_path, CONCEPT_AUTO_INDEX=None) + assert settings.CONCEPT_AUTO_INDEX is True + + +def test_an_unparseable_value_falls_back_to_the_default(monkeypatch, tmp_path, capsys): + settings = _reload_settings(monkeypatch, tmp_path, CONCEPT_AUTO_INDEX="maybe") + assert settings.CONCEPT_AUTO_INDEX is True + assert "not a true/false value" in capsys.readouterr().err + + +def test_batch_size_is_capped_below_sqlites_parameter_ceiling( + monkeypatch, tmp_path, capsys +): + """A claimed batch becomes one IN (...) clause in three queries. An + oversized batch would fail identically on every cycle, forever.""" + settings = _reload_settings( + monkeypatch, tmp_path, CONCEPT_INDEX_BATCH_SIZE="100000" + ) + assert settings.CONCEPT_INDEX_BATCH_SIZE == settings.CONCEPT_INDEX_BATCH_SIZE_MAX + assert settings.CONCEPT_INDEX_BATCH_SIZE < 32766 + assert "clamped" in capsys.readouterr().err + + +def test_batch_size_still_has_a_floor(monkeypatch, tmp_path): + settings = _reload_settings(monkeypatch, tmp_path, CONCEPT_INDEX_BATCH_SIZE="0") + assert settings.CONCEPT_INDEX_BATCH_SIZE == 1 + + +def test_knowledge_status_reports_how_far_behind_indexing_is(monkeypatch, tmp_path): + """A dormant worker's only other symptom is a graph that quietly stops + growing. Pending and parked are reported separately because they call for + different responses.""" + load_isolated_server(monkeypatch, tmp_path) + runtime_status = importlib.import_module("marm_mcp_server.services.runtime_status") + concept_queue = importlib.import_module("marm_mcp_server.core.concept_queue") + mem = sys.modules["marm_mcp_server.core.memory"].memory + + with mem.get_connection() as conn: + for index, state in enumerate(["pending", "leased", "parked"]): + conn.execute( + "INSERT INTO memories (id, session_name, content, timestamp) " + "VALUES (?, 's', 'c', datetime('now'))", + (f"m{index}",), + ) + concept_queue.enqueue(conn, f"m{index}", "h1") + conn.execute( + "UPDATE concept_index_queue SET state = ? WHERE memory_id = ?", + (state, f"m{index}"), + ) + + status = runtime_status.knowledge_status() + + assert status["index_queue"] == {"pending": 2, "parked": 1} + assert status["auto_index"] is True + + +def test_knowledge_status_still_reports_when_the_queue_cannot_be_read( + monkeypatch, tmp_path +): + """An optional number must not take the whole status command down.""" + load_isolated_server(monkeypatch, tmp_path) + runtime_status = importlib.import_module("marm_mcp_server.services.runtime_status") + concept_queue = importlib.import_module("marm_mcp_server.core.concept_queue") + + def explode(): + raise sqlite3.OperationalError("database is locked") + + monkeypatch.setattr(concept_queue, "counts", explode) + + status = runtime_status.knowledge_status() + + assert status["index_queue"] == {"pending": None, "parked": None} + assert status["state"] in ( + "ready", + "ready_no_build", + "missing_spacy", + "missing_model", + ) + + +@pytest.mark.asyncio +async def test_http_shutdown_stops_the_worker_before_the_write_queue( + monkeypatch, tmp_path +): + """Order matters: the worker produces concept writes and reads the memory + DB, so it has to be stopped before the pools start closing.""" + load_isolated_server(monkeypatch, tmp_path) + shutdown_module = importlib.import_module("marm_mcp_server.core.shutdown_manager") + memory_module = sys.modules["marm_mcp_server.core.memory"] + + order = [] + + async def record_worker_stop(): + order.append("worker") + + async def record_queue_stop(): + order.append("write_queue") + + monkeypatch.setattr(shutdown_module.concept_worker, "stop", record_worker_stop) + monkeypatch.setattr(memory_module.memory, "stop_write_queue", record_queue_stop) + + await shutdown_module.ShutdownManager().graceful_shutdown() + + assert order[:2] == ["worker", "write_queue"] + + +@pytest.mark.asyncio +async def test_http_shutdown_survives_a_worker_that_fails_to_stop( + monkeypatch, tmp_path +): + """A broken optional subsystem must not strand the write queue.""" + load_isolated_server(monkeypatch, tmp_path) + shutdown_module = importlib.import_module("marm_mcp_server.core.shutdown_manager") + memory_module = sys.modules["marm_mcp_server.core.memory"] + + stopped = [] + + async def explode(): + raise RuntimeError("worker stop failed") + + async def record_queue_stop(): + stopped.append("write_queue") + + monkeypatch.setattr(shutdown_module.concept_worker, "stop", explode) + monkeypatch.setattr(memory_module.memory, "stop_write_queue", record_queue_stop) + + await shutdown_module.ShutdownManager().graceful_shutdown() + + assert stopped == ["write_queue"] + + +@pytest.mark.asyncio +async def test_stdio_lifespan_starts_and_stops_the_worker(monkeypatch, tmp_path): + """A STDIO session lives as long as its host application, which is long + enough for the worker to matter, and its teardown is shielded and bounded.""" + load_isolated_server(monkeypatch, tmp_path) + stdio = importlib.import_module("marm_mcp_server.server_stdio") + memory_module = sys.modules["marm_mcp_server.core.memory"] + + events = [] + + def record_start(): + events.append("start") + + async def record_stop(): + events.append("stop") + + async def noop_queue_stop(): + return None + + monkeypatch.setattr(stdio.concept_worker, "start", record_start) + monkeypatch.setattr(stdio.concept_worker, "stop", record_stop) + monkeypatch.setattr(memory_module.memory, "stop_write_queue", noop_queue_stop) + + async with stdio._stdio_lifespan(None): + assert events == ["start"] + + assert events == ["start", "stop"] + + +@pytest.mark.asyncio +async def test_stdio_teardown_still_stops_the_worker_after_a_crashed_session( + monkeypatch, tmp_path +): + """The teardown sits in a finally precisely because a session that died is + when unfinished work is most likely.""" + load_isolated_server(monkeypatch, tmp_path) + stdio = importlib.import_module("marm_mcp_server.server_stdio") + memory_module = sys.modules["marm_mcp_server.core.memory"] + + events = [] + + async def record_stop(): + events.append("stop") + + async def noop_queue_stop(): + return None + + monkeypatch.setattr(stdio.concept_worker, "start", lambda: None) + monkeypatch.setattr(stdio.concept_worker, "stop", record_stop) + monkeypatch.setattr(memory_module.memory, "stop_write_queue", noop_queue_stop) + + with pytest.raises(RuntimeError, match="session died"): + async with stdio._stdio_lifespan(None): + raise RuntimeError("session died") + + assert events == ["stop"] diff --git a/marm-mcp-server/tests/test_console_graph_version.py b/marm-mcp-server/tests/test_console_graph_version.py new file mode 100644 index 00000000..a416df8b --- /dev/null +++ b/marm-mcp-server/tests/test_console_graph_version.py @@ -0,0 +1,107 @@ +"""Tests for the graph change marker the Console Explorer polls. + +Without this the Explorer reads the graph once and background indexing is +invisible until a reload, which would make the automation pointless from the +user's side. +""" + +import sqlite3 + +import pytest + +from marm_mcp_server.console import concept_store +from marm_mcp_server.core.concept_db import CONCEPT_SCHEMA_VERSION, ConceptDB + + +@pytest.fixture +def graph(tmp_path): + db_path = tmp_path / "marm_index.db" + concept_db = ConceptDB(str(db_path)) + yield concept_db, db_path + concept_db.close() + + +def _add_entity(concept_db, name, memory_id="m1"): + with concept_db.get_connection() as conn: + return concept_db.get_or_create_entity( + conn, name, "concept", "sess-a", None, memory_id, platform="cli" + ) + + +def test_version_is_stable_while_nothing_changes(graph): + concept_db, db_path = graph + _add_entity(concept_db, "auth module") + + first = concept_store.graph_version(db_path) + second = concept_store.graph_version(db_path) + + assert first["schema_status"] == "current" + assert first == second + + +def test_version_moves_when_an_entity_is_added(graph): + concept_db, db_path = graph + before = concept_store.graph_version(db_path)["version"] + + _add_entity(concept_db, "rate limiter") + + assert concept_store.graph_version(db_path)["version"] != before + + +def test_version_moves_when_a_relationship_is_added(graph): + concept_db, db_path = graph + first, _ = _add_entity(concept_db, "auth module") + second, _ = _add_entity(concept_db, "rate limiter") + before = concept_store.graph_version(db_path)["version"] + + with concept_db.get_connection() as conn: + concept_db.store_relationship( + conn, first, second, "uses", "m1", None, platform="cli" + ) + + assert concept_store.graph_version(db_path)["version"] != before + + +def test_version_moves_when_an_entity_is_removed(graph): + """Counts as well as max ids, so a delete is not invisible.""" + concept_db, db_path = graph + _add_entity(concept_db, "auth module") + _add_entity(concept_db, "rate limiter") + before = concept_store.graph_version(db_path)["version"] + + concept_db.cleanup_deleted_memory_provenance(["m1"]) + + assert concept_store.graph_version(db_path)["version"] != before + + +def test_version_reports_a_graph_that_needs_rebuilding(graph): + concept_db, db_path = graph + with concept_db.get_connection() as conn: + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + + result = concept_store.graph_version(db_path) + + assert result["schema_status"] == "rebuild_required" + assert result["version"] == "rebuild_required" + + +def test_version_on_a_missing_database_is_not_an_error(tmp_path): + result = concept_store.graph_version(tmp_path / "nothing.db") + + assert result["schema_status"] == "unavailable" + + +def test_console_reads_the_schema_version_from_the_writer(graph): + """The Console used to restate the version as a literal. On the next bump + it would have called every freshly rebuilt graph stale.""" + _concept_db, db_path = graph + + assert concept_store._CURRENT_CONCEPT_SCHEMA_VERSION == str(CONCEPT_SCHEMA_VERSION) + with sqlite3.connect(db_path) as conn: + stored = conn.execute( + "SELECT value FROM concept_schema_metadata WHERE key = 'schema_version'" + ).fetchone()[0] + assert stored == concept_store._CURRENT_CONCEPT_SCHEMA_VERSION + assert concept_store.graph_version(db_path)["schema_status"] == "current" diff --git a/marm-mcp-server/tests/test_graph_context.py b/marm-mcp-server/tests/test_graph_context.py index 645480b9..9ad84728 100644 --- a/marm-mcp-server/tests/test_graph_context.py +++ b/marm-mcp-server/tests/test_graph_context.py @@ -4,10 +4,12 @@ import pytest from conftest import load_isolated_server, local_client +from marm_mcp_server.core import concept_db as concept_db_module from marm_mcp_server.core.concept_db import ( ConceptDB, backup_and_reset_concept_database, inspect_concept_schema, + mark_schema_current, ) from marm_mcp_server.core.response_limiter import MCPResponseLimiter from marm_mcp_server.services.graph_context import ( @@ -182,11 +184,107 @@ def test_platformless_graph_requires_explicit_reset(monkeypatch, tmp_path): backup = backup_and_reset_concept_database(str(db_path)) assert backup + # Still rebuild_required: the reset emptied the graph but nothing has been + # extracted into it yet. Marking it current here is what would let a + # rebuild that dies partway pass for a finished one. + assert inspect_concept_schema(str(db_path)) == "rebuild_required" + mark_schema_current(str(db_path)) assert inspect_concept_schema(str(db_path)) == "current" with sqlite3.connect(backup) as conn: assert conn.execute("SELECT name FROM entities").fetchone()[0] == "legacy" +def test_a_reset_never_writes_the_version_even_briefly(tmp_path): + """Writing the marker and deleting it again leaves a window where a crash, + or another process reading the schema state, sees an empty graph reported + as current. The reset must never write it at all.""" + db_path = tmp_path / "legacy.db" + graph = ConceptDB(str(db_path)) + with graph.get_connection() as conn: + graph.get_or_create_entity( + conn, "old", "concept", "sess-a", None, "m1", platform="cli" + ) + graph.close() + + seen = [] + real_init = concept_db_module.init_concept_database + + def watching_init(path, mark_current=True): + real_init(path, mark_current=mark_current) + with sqlite3.connect(path) as conn: + row = conn.execute( + "SELECT value FROM concept_schema_metadata WHERE key = 'schema_version'" + ).fetchone() + seen.append(row) + + with pytest.MonkeyPatch.context() as patch: + patch.setattr(concept_db_module, "init_concept_database", watching_init) + backup_and_reset_concept_database(str(db_path)) + + assert seen == [None], f"the reset stamped a version mid-flight: {seen}" + assert inspect_concept_schema(str(db_path)) == "rebuild_required" + + +def test_constructing_conceptdb_does_not_restamp_an_older_graph(tmp_path): + """init_concept_database runs on every ConceptDB(...) construction. If it + writes the current schema version unconditionally, one construction marks + a graph built under an older rule as current and its rebuild never + fires.""" + db_path = tmp_path / "older.db" + graph = ConceptDB(str(db_path)) + with graph.get_connection() as conn: + graph.get_or_create_entity( + conn, "stale entity", "concept", "sess-a", None, "m1", platform="cli" + ) + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + graph.close() + + assert inspect_concept_schema(str(db_path)) == "rebuild_required" + + ConceptDB(str(db_path)).close() + + with sqlite3.connect(db_path) as conn: + version = conn.execute( + "SELECT value FROM concept_schema_metadata WHERE key = 'schema_version'" + ).fetchone()[0] + assert version == "1" + assert inspect_concept_schema(str(db_path)) == "rebuild_required" + + +def test_console_delete_cleanup_leaves_an_older_graph_needing_rebuild( + monkeypatch, tmp_path +): + """The real path that constructs a ConceptDB outside a build: deleting a + memory in the Console runs provenance cleanup, which must not double as a + schema blessing.""" + db_path = tmp_path / "older.db" + graph = ConceptDB(str(db_path)) + with graph.get_connection() as conn: + graph.get_or_create_entity( + conn, "stale entity", "concept", "sess-a", None, "m1", platform="cli" + ) + conn.execute( + "UPDATE concept_schema_metadata SET value = '1' WHERE key = 'schema_version'" + ) + graph.close() + monkeypatch.setenv("MARM_CONCEPT_DB_PATH", str(db_path)) + + from marm_mcp_server.endpoints import memory as memory_endpoints + + result = memory_endpoints._cleanup_deleted_concepts(["m1"]) + + # Not just "did not fail": a missing concept database returns + # status="skipped", which would satisfy that and prove nothing about + # whether construction restamped the version. + assert result["status"] == "success" + assert result["entities_deleted"] == 1 + with sqlite3.connect(db_path) as conn: + assert conn.execute("SELECT COUNT(*) FROM entities").fetchone()[0] == 0 + assert inspect_concept_schema(str(db_path)) == "rebuild_required" + + def test_targeted_build_cannot_reset_platformless_graph(monkeypatch, tmp_path): from marm_mcp_server.core.models import ConceptBuildRequest from marm_mcp_server.endpoints import concepts @@ -210,7 +308,10 @@ def test_targeted_build_cannot_reset_platformless_graph(monkeypatch, tmp_path): assert inspect_concept_schema(str(db_path)) == "rebuild_required" assert concepts._prepare_build_schema(ConceptBuildRequest(search_all=True)) is True - assert inspect_concept_schema(str(db_path)) == "current" + # Preparing the schema resets the graph; it does not declare it rebuilt. + # The version is stamped by the build that follows, so an interrupted + # rebuild is still asked for on the next start. + assert inspect_concept_schema(str(db_path)) == "rebuild_required" def test_graph_context_is_reduced_before_primary_results(monkeypatch): diff --git a/scripts/benchmarking/README.md b/scripts/benchmarking/README.md index f2805adf..78b3626e 100644 --- a/scripts/benchmarking/README.md +++ b/scripts/benchmarking/README.md @@ -8,8 +8,9 @@ model judging. ``` scripts/benchmarking/ performance/ - bench_hotpath.py # encode/recall/write/hybrid-search latency - dump_tool_schema.py # dumps the real MCP tool schema an agent sees + bench_hotpath.py # encode/recall/write/hybrid-search latency + bench_concept_worker.py # store/recall latency under background indexing + dump_tool_schema.py # dumps the real MCP tool schema an agent sees accuracy/ locomo/ run_eval.py # LoCoMo retrieval accuracy harness @@ -35,6 +36,45 @@ python scripts/benchmarking/performance/bench_hotpath.py The numbers in the root [README's Performance & Scaling Benchmarks section](../../README.md#performance--scaling-benchmarks) come from this script. Don't publish a performance claim this script can't reproduce. +### `bench_concept_worker.py` + +Measures what the test suite structurally cannot: `conftest` disables the real +encoder for isolation, so no test exercises store and recall latency while the +v2.36.0 background indexer is running. + +Times both paths twice, once with the worker stopped and once while it drains a +queue holding the whole corpus, which is the state an upgrade with an existing +corpus passes through. A fresh install has nothing to catch up on. + +``` +python scripts/benchmarking/performance/bench_concept_worker.py +python scripts/benchmarking/performance/bench_concept_worker.py --from-live +``` + +`--from-live` copies `~/.marm/marm_memory.db` (and its WAL sidecars) into a temp +directory and measures against a real corpus. It never writes to the original. + +The code-graph engine is disabled unless `--with-code-graph` is passed: each +extracted entity otherwise costs a ~300ms round trip to that subprocess, which +swamps the in-process contention this script is for. + +Also sweeps the inter-batch pause, reporting recall p95 against total drain +time so the throttle can be set from data: + +``` +python scripts/benchmarking/performance/bench_concept_worker.py --from-live --sweep 0,250,500 +``` + +Interpreting it: the relative deltas look alarming and the absolute numbers +usually do not. Judge both. Check the "N of M indexed" line as well, since a +worker that finished early means part of the timed phase measured an idle +process; raise `--seed` if so. + +Corpus shape changes the answer, so prefer `--from-live` before quoting a +number anywhere. Short synthetic memories produce many small extractions and +show a write regression that a real corpus does not, because entity-name +embeddings are generated far faster than real content generates them. + ## Accuracy (`accuracy/locomo/`) ### `run_eval.py` diff --git a/scripts/benchmarking/performance/bench_concept_worker.py b/scripts/benchmarking/performance/bench_concept_worker.py new file mode 100644 index 00000000..5f143b43 --- /dev/null +++ b/scripts/benchmarking/performance/bench_concept_worker.py @@ -0,0 +1,443 @@ +"""Concept indexing worker contention benchmark. + +The one thing the test suite cannot answer. conftest forces the encoder off for +isolation, so nothing in `pytest` exercises what this feature's own spec calls +its main performance risk: `_try_embed` reaches into `memory._encoder_lock`, +which is serialized process-wide and shared with every recall and every write. +Before v2.36.0 nothing held that lock on a loop. Now a background worker does. + +Measures, against the REAL MARMMemory and the configured fastembed encoder, +store and recall latency twice: + + 1. baseline -- worker stopped, nothing competing for the encoder + 2. contended -- worker actively draining a backlog the whole time + +The gap between them is the number to judge. A worker that doubles recall +latency is not shippable on defaults; a few percent is noise. + +Run from repo root: + python scripts/benchmarking/performance/bench_concept_worker.py + python scripts/benchmarking/performance/bench_concept_worker.py --from-live + +Uses a throwaway temp DB. `--from-live` COPIES ~/.marm/marm_memory.db into it +and never writes to the original, so the numbers come from a real corpus +without touching it. +""" + +import argparse +import asyncio +import os +import statistics +import sys +import tempfile +import time +import uuid +from datetime import datetime, timezone +from pathlib import Path + +_TMP = tempfile.mkdtemp(prefix="marm_bench_worker_") +os.environ["MARM_DB_PATH"] = os.path.join(_TMP, "bench.db") +os.environ["MARM_ANALYTICS_DB_PATH"] = os.path.join(_TMP, "analytics.db") +os.environ["MARM_CONCEPT_DB_PATH"] = os.path.join(_TMP, "bench_index.db") +os.environ["SERVER_HOST"] = "127.0.0.1" +os.environ["WRITE_QUEUE_ENABLED"] = "0" +# Off unless asked for. Every extracted entity otherwise costs a ~300ms +# round trip to the code-graph subprocess, which swamps the encoder-lock +# contention this benchmark exists to isolate and says more about that +# child process than about the worker. --with-code-graph re-enables it. +if "--with-code-graph" not in sys.argv: + os.environ["GRAPH_ENABLED"] = "false" + +_REPO_ROOT = os.path.dirname( + os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +) +sys.path.insert(0, os.path.join(_REPO_ROOT, "marm-mcp-server")) + + +def _parse_args(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--from-live", + action="store_true", + help="copy ~/.marm/marm_memory.db and measure against the real corpus", + ) + parser.add_argument("--seed", type=int, default=600, help="synthetic corpus size") + parser.add_argument("--iters", type=int, default=40, help="timed samples per phase") + parser.add_argument( + "--with-code-graph", + action="store_true", + help="leave the code-graph engine on (adds subprocess cost per entity)", + ) + parser.add_argument( + "--sweep", + type=str, + default="", + help=( + "compare inter-batch pauses instead of the two-phase run, " + "e.g. --sweep 0,250,500 (milliseconds)" + ), + ) + return parser.parse_args() + + +ARGS = _parse_args() + +if ARGS.from_live: + _live = Path.home() / ".marm" / "marm_memory.db" + if not _live.exists(): + print(f"No live database at {_live}", file=sys.stderr) + raise SystemExit(1) + # sqlite3's own backup API, not a file copy. Copying the database and its + # -wal and -shm files one at a time can capture three different instants if + # MARM is running, which either loses committed frames or produces a corpus + # that will not open. backup() takes one consistent snapshot and folds the + # WAL in, so no sidecars are needed. Read-only on the source either way. + import sqlite3 as _sqlite3 + + _source = _sqlite3.connect(f"file:{_live}?mode=ro", uri=True) + _dest = _sqlite3.connect(os.environ["MARM_DB_PATH"]) + try: + _source.backup(_dest) + finally: + _dest.close() + _source.close() + print(f"Snapshotted live corpus from {_live}\n") + +from marm_mcp_server.config.settings import ( # noqa: E402 + CONCEPT_INDEX_BATCH_SIZE, + CONCEPTS_AVAILABLE, +) +from marm_mcp_server.core import concept_queue, consolidation # noqa: E402 +from marm_mcp_server.core.concept_worker import ConceptIndexWorker # noqa: E402 +from marm_mcp_server.core.memory import MARMMemory # noqa: E402 + +RECALL_LIMIT = 5 + +QUERIES = [ + "how does the write queue work", + "what did we decide about compaction", + "sqlite connection pool", + "embedding model dimensions", + "concept graph rebuild", +] + +VOCAB = ( + "deploy rollback latency embedding session compaction queue sqlite vector " + "rate limit semantic merge consolidation worker token bloat refactor schema " + "websocket transport docker registry pipeline migration encoder cosine recall" +).split() + + +def _pct(values, p): + s = sorted(values) + k = max(0, min(len(s) - 1, round((p / 100) * (len(s) - 1)))) + return s[k] + + +def _stat(label, samples_ms): + return ( + f"{label:<26} " + f"med={statistics.median(samples_ms):7.1f} " + f"p95={_pct(samples_ms, 95):7.1f} " + f"max={max(samples_ms):7.1f} (ms)" + ) + + +def _make_text(i): + import random + + rnd = random.Random(i) + return f"benchmark memory {i}: " + " ".join( + rnd.choice(VOCAB) for _ in range(rnd.randint(10, 30)) + ) + + +def seed_synthetic(mem, n): + """Insert n rows with real embeddings, bypassing the write path for speed.""" + mem._load_encoder_lazily() + texts = [_make_text(i) for i in range(n)] + embeddings = mem.encoder.encode(texts) + timestamp = datetime.now(timezone.utc).isoformat() + with mem.get_connection() as conn: + for text, vector in zip(texts, embeddings): + conn.execute( + "INSERT INTO memories (id, session_name, content, embedding, " + "content_hash, timestamp, context_type, metadata) " + "VALUES (?, 'bench', ?, ?, ?, ?, 'general', '{}')", + ( + str(uuid.uuid4()), + text, + vector.astype("float32").tobytes(), + consolidation.compute_content_hash(text), + timestamp, + ), + ) + + +def queue_ids(mem) -> list[str]: + """Recreate the post-upgrade state: every memory waiting to be indexed. + + This is the worst realistic case and the one every user hits once, so it + is what the worker should be measured under rather than a trickle. + """ + with mem.get_connection() as conn: + rows = conn.execute( + "SELECT id, content_hash, content FROM memories " + "WHERE content IS NOT NULL AND content != ''" + ).fetchall() + for memory_id, content_hash, content in rows: + if content_hash is None: + # Rows predating the content_hash column, common in a corpus + # that has been through upgrades. Backfilling the real hash + # matters: enqueueing a fabricated one makes the worker treat + # every result as superseded, so it burns the extraction cost + # and writes nothing, and the benchmark silently measures a + # worker that never indexes anything. + content_hash = consolidation.compute_content_hash(content) + conn.execute( + "UPDATE memories SET content_hash = ? WHERE id = ?", + (content_hash, memory_id), + ) + concept_queue.enqueue(conn, memory_id, content_hash) + return [row[0] for row in rows] + + +async def measure_writes(mem, iters): + samples = [] + for i in range(iters): + text = f"latency probe {uuid.uuid4()}: " + _make_text(10_000 + i) + start = time.perf_counter() + await mem.store_memory(text, "bench-probe") + samples.append((time.perf_counter() - start) * 1000) + return samples + + +async def measure_recalls(mem, iters): + samples = [] + for i in range(iters): + query = QUERIES[i % len(QUERIES)] + start = time.perf_counter() + await mem.recall_similar(query, limit=RECALL_LIMIT) + samples.append((time.perf_counter() - start) * 1000) + return samples + + +def _delta(baseline, contended): + base = statistics.median(baseline) + cont = statistics.median(contended) + if base <= 0: + return "n/a" + return f"{((cont - base) / base) * 100:+.1f}%" + + +def remaining_of(mem, queued_ids) -> int: + """How many of the originally queued memories are still waiting. + + Not the raw queue depth: the latency probes store memories of their own, + which enqueue themselves and would otherwise make the backlog look like + it grew while the worker was clearing it. + """ + with mem.get_connection() as conn: + pending = { + row[0] + for row in conn.execute( + "SELECT memory_id FROM concept_index_queue WHERE state != 'parked'" + ).fetchall() + } + return len(pending & queued_ids) + + +def reset_graph(concepts_module): + """Drop the concept database between sweep points. + + Without this, entity dedup makes every run after the first cheaper than + the one before it, and the pause would take credit for work the previous + run already did. + """ + if concepts_module._concept_db is not None: + concepts_module._concept_db.close() + concepts_module._concept_db = None + base = Path(os.environ["MARM_CONCEPT_DB_PATH"]) + for path in (base, Path(str(base) + "-wal"), Path(str(base) + "-shm")): + if path.exists(): + path.unlink() + + +async def drain_with_pause(mem, pause_ms, worker_module, concepts_module): + """Time a full backlog drain while sampling recall throughout.""" + reset_graph(concepts_module) + with mem.get_connection() as conn: + conn.execute("DELETE FROM concept_index_queue") + queued_ids = set(queue_ids(mem)) + queued = len(queued_ids) + + worker_module.CONCEPT_INDEX_BATCH_PAUSE_MS = pause_ms + worker_module.CONCEPT_INDEX_DEBOUNCE_SECONDS = 0.01 + worker = worker_module.ConceptIndexWorker() + + samples = [] + started = time.perf_counter() + worker.start() + while True: + query = QUERIES[len(samples) % len(QUERIES)] + probe = time.perf_counter() + await mem.recall_similar(query, limit=RECALL_LIMIT) + samples.append((time.perf_counter() - probe) * 1000) + if remaining_of(mem, queued_ids) == 0: + break + if time.perf_counter() - started > 900: + print(" timed out after 15 minutes", file=sys.stderr) + break + elapsed = time.perf_counter() - started + await worker.stop() + return queued, elapsed, samples + + +async def run_sweep(mem, pauses): + import marm_mcp_server.core.concept_worker as worker_module + from marm_mcp_server.endpoints import concepts as concepts_module + + # spaCy loads its model on the first extraction, which costs seconds. Left + # to the sweep, that lands entirely on whichever pause runs first and can + # make a throttled run look faster than an unthrottled one. + print("warming up the extractor...") + concepts_module.extract_entities("warmup sentence about the write queue") + + print("Idle recall baseline (no worker running)") + idle = await measure_recalls(mem, ARGS.iters) + print(" " + _stat("recall_similar", idle) + "\n") + + rows = [] + for pause_ms in pauses: + print(f"draining with a {pause_ms}ms pause between batches...") + queued, elapsed, samples = await drain_with_pause( + mem, pause_ms, worker_module, concepts_module + ) + rows.append((pause_ms, queued, elapsed, samples)) + print( + f" {queued} memories in {elapsed:6.1f}s " + + _stat("recall during drain", samples) + ) + + print("\n--- pause sweep --------------------------------------------------") + print( + f"{'pause':>7} {'drain':>8} {'rate':>10} " + f"{'recall med':>11} {'recall p95':>11} {'p95 vs idle':>12}" + ) + idle_p95 = _pct(idle, 95) + for pause_ms, queued, elapsed, samples in rows: + print( + f"{pause_ms:>5}ms {elapsed:>7.1f}s " + f"{queued / elapsed:>7.1f}/s " + f"{statistics.median(samples):>9.1f}ms " + f"{_pct(samples, 95):>9.1f}ms " + f"{((_pct(samples, 95) - idle_p95) / idle_p95) * 100:>+11.1f}%" + ) + print( + f"\nidle recall p95 was {idle_p95:.1f}ms. The pause buys interactive\n" + "latency and costs drain duration; pick the point where p95 stops\n" + "improving faster than the drain slows down." + ) + + +async def main(): + if not CONCEPTS_AVAILABLE: + print( + "Concept extraction is unavailable in this environment, so there is\n" + "no worker to contend with and this benchmark would measure nothing.\n" + "Run: python -m pip install -U --force-reinstall marm-mcp-server", + file=sys.stderr, + ) + raise SystemExit(1) + + mem = MARMMemory() + if not mem._load_encoder_lazily(): + print( + "The semantic encoder could not load. The encoder lock is exactly\n" + "what this benchmark measures contention on, so there is nothing\n" + "to report without it.", + file=sys.stderr, + ) + raise SystemExit(1) + + if not ARGS.from_live: + seed_synthetic(mem, ARGS.seed) + + with mem.get_connection() as conn: + corpus = conn.execute("SELECT COUNT(*) FROM memories").fetchone()[0] + + print(f"corpus: {corpus} memories temp dir: {_TMP}") + print(f"batch size: {CONCEPT_INDEX_BATCH_SIZE} iters per phase: {ARGS.iters}\n") + + if ARGS.sweep: + pauses = [int(value) for value in ARGS.sweep.split(",") if value.strip()] + await run_sweep(mem, pauses) + return + + print("--- baseline: worker stopped -------------------------------------") + base_writes = await measure_writes(mem, ARGS.iters) + base_recalls = await measure_recalls(mem, ARGS.iters) + print(_stat("store_memory", base_writes)) + print(_stat("recall_similar", base_recalls)) + + queued_ids = set(queue_ids(mem)) + queued = len(queued_ids) + worker = ConceptIndexWorker() + import marm_mcp_server.core.concept_worker as worker_module + + # Start draining immediately: the debounce is a real-usage nicety and only + # delays the contention this run exists to observe. + worker_module.CONCEPT_INDEX_DEBOUNCE_SECONDS = 0.01 + worker.start() + + # Do not start timing until the worker is genuinely busy, or the first + # samples measure an idle process and flatter the result. + for _ in range(300): + await asyncio.sleep(0.1) + if remaining_of(mem, queued_ids) < queued: + break + + print(f"\n--- contended: worker draining {queued} queued memories ----------") + cont_writes = await measure_writes(mem, ARGS.iters) + cont_recalls = await measure_recalls(mem, ARGS.iters) + left = remaining_of(mem, queued_ids) + + await worker.stop() + + print(_stat("store_memory", cont_writes)) + print(_stat("recall_similar", cont_recalls)) + + indexed = queued - left + if indexed == 0: + # Distinct from "still draining". Zero progress means the worker was + # spinning without indexing anything, so the deltas below are not + # contention measurements at all. A queue-hash mismatch produced + # exactly this once, and the run was reported as valid. + verdict = "NOTHING WAS INDEXED, DELTAS ARE NOT VALID" + elif left > 0: + verdict = "worker was busy throughout" + else: + verdict = "WORKER FINISHED EARLY" + print(f"\n{indexed} of {queued} indexed during the timed phase ({verdict})") + if indexed == 0: + print( + " The worker claimed tasks but wrote nothing. Check for superseded\n" + " or failed tasks in the queue before trusting any number here." + ) + elif left == 0: + print( + " Re-run with a larger --seed: the worker drained the backlog before\n" + " the timed phase ended, so part of it measured an idle process." + ) + + print("\n--- median deltas under load -------------------------------------") + print(f"{'store_memory':<26} {_delta(base_writes, cont_writes)}") + print(f"{'recall_similar':<26} {_delta(base_recalls, cont_recalls)}") + print( + "\nRestored settings are not written anywhere; this process used a\n" + f"throwaway database at {_TMP} and never touched ~/.marm." + ) + + +if __name__ == "__main__": + asyncio.run(main())