chore(release): promote develop to main - #233
Merged
Merged
Conversation
…0ing
`_vector_search` always builds its filter with
`excluded_types=RECALL_EXCLUDED_TYPES`, and that setting defaults to
`MetaPattern` — a non-empty value — so every vector search sends Qdrant a
condition on the `type` payload field. `ensure_qdrant_collection` only ever
indexed `tags` and `tag_prefixes`, and Qdrant rejects an entire search with 400
when a filter references an unindexed field:
Bad request: Index required but not found for "type" of one of the
following types: [keyword].
`_vector_search` catches that broadly and returns `[]`, so recall silently
degrades to keyword-only rather than surfacing an error. On a live instance the
symptom is `vector_search.matched: false` on every query, `vector_count: null`
in `/health`, and semantically-phrased queries returning unrelated keyword
collisions — with nothing in the response indicating that vector search is
failing.
Add `type` to the ensured indexes, collapsing the duplicated enum/string
branches into one loop over a named tuple so the set of filtered fields stays
visible in one place.
Verified against a production instance (~25k points): creating the `type`
payload index restored `vector_search.matched: true` and `match_type: "vector"`
results immediately, with no other change.
Records where the streamable-HTTP/SSE bridge and the stdio package had drifted apart, and pins the short list of differences that stay intentional. Follow-up to #224, which fixed one symptom of the split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Connects one MCP client to each transport — the bridge in-process over streamable HTTP, the published stdio package as a child process — and diffs tools/list and the initialize result. Both point at the same live AutoMem, so any difference is the transports, not the data. Red as of this commit: remote omits _meta, title, outputSchema, two annotation hints, and server instructions, and its descriptions are one-liners. Pins @modelcontextprotocol/sdk to 1.20.0 so the new devDependency's newer SDK gets nested rather than hoisted over the deployed bridge's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nineteen scenarios covering every mode on both transports — batch and supersede store, ranked/detailed/items/json/empty recall, id fetch, tag enumeration, relation-prop and batch association, update, single and bulk delete, health, and three error paths. Each transport writes under its own uuid4 tag namespace so neither sees the other's memories; UUIDs, timestamps, scores and query_time_ms are redacted before comparison. Cleanup deletes both namespaces by tag. Red as of this commit, first failure on `store single`: remote Memory stored: <UUID> stdio Memory stored successfully!\n\nMemory ID: <UUID> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
make test now chains the MCP bridge's node suite, which was previously only reachable in CI and invisible locally. make test-parity brings the stack up and diffs the two transports in one command. The workflow runs on PRs touching the bridge or the HTTP API, plus weekly as a drift alarm against newly published mcp-automem versions. It is continue-on-error until the bridge moves onto the shared surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Summary `develop` is the GitHub default branch again. Spell that in AGENTS.md so agents stop opening feature PRs against `main`. Release-please and GHCR `:stable` stay on `main`. ## Test plan - [x] Confirm `gh repo view --json defaultBranchRef` is `develop` - [x] Confirm this PR itself targets `develop`
## Summary
`GET /stream` was silent for store and recall. Those `emit_event` calls
lived only in unused `runtime_*_routes` handlers after the app.py split;
the live blueprints never published them. This wires the live path and
fills in the rest of the write surface.
## Events
| Type | Source |
|---|---|
| `memory.store` | `POST /memory`; one summary event for `POST
/memory/batch` |
| `memory.recall` | `GET /recall` |
| `memory.update` | `PATCH /memory/{id}` |
| `memory.delete` | `DELETE /memory/{id}` and bulk `DELETE
/memory/by-tag` |
| `memory.associate` | `POST /associate` (single) and one summary event
for batch |
Failed `4xx` requests do not emit. Batch operations send one summary
(`count`) instead of one event per item. `GET /memory/{id}` and by-tag
listing stay off the stream.
`scripts/automem_watch.py` tracks the new types, skips garbage detection
on batch summaries, and preserves explicit `count: 0`. `docs/API.md`
documents the catalog.
## Test plan
- [x] `pytest tests/test_stream_events.py` (13 passed)
- [x] `pytest tests/test_app.py tests/contracts/test_routes_contract.py`
- [x] flake8 on touched Python files
- [ ] CI unit/lint jobs on this PR
## Risk
Additive SSE payloads only. Existing enrichment and consolidation events
are unchanged. Watcher still runs as `python scripts/automem_watch.py`
without a package import.
`export_falkordb_artifact` pages the graph with `SKIP <offset> LIMIT
<batch_size>`. That loses data two ways on a graph of any size.
## 1. Deep SKIP exceeds the server timeout
FalkorDB re-scans and discards every skipped row, so each batch costs
more than the last. On my instance (20,682 nodes / 238,238
relationships) the tail batches reached 1120 ms of server execution and
tripped the instance's `TIMEOUT` config (1000), killing the backup
partway through:
```
INFO | Exported FalkorDB relationship batch: 10000 relationships (total: 210000)
INFO | Exported FalkorDB relationship batch: 10000 relationships (total: 220000)
ERROR | ❌ FalkorDB backup failed: Query timed out
ERROR | ❌ Backup failed: Query timed out
```
Measured against that graph, same rows returned:
| Pagination | Server execution |
|---|---|
| `SKIP 220000 LIMIT 10000` | 1120 ms (`Filter` over `All Node Scan`) |
| `WHERE id(a) >= lo AND id(a) < hi` | 37–42 ms (`NodeByIdSeek`) |
This has been failing roughly every other scheduled run for me since the
graph crossed ~220k relationships.
## 2. `RESULTSET_SIZE` truncates silently
FalkorDB caps a result set at `RESULTSET_SIZE` and returns the short set
**with no error**. A query covering 65,900 relationships came back with
exactly 10,000 rows and no warning.
The loop breaks when a batch returns fewer rows than `batch_size`, so
any `RESULTSET_SIZE` below `batch_size` ends the export after the first
batch. Running the current exporter unmodified against a 5,000-node /
15,000-relationship graph with `RESULTSET_SIZE=1000`:
```
UPSTREAM exported: {'node_count': 1000, 'relationship_count': 1000}
lost: 4000 nodes, 14000 relationships (no error raised)
PATCHED exported: {'node_count': 5000, 'relationship_count': 15000}
```
80% of nodes and 93% of relationships silently missing, and the backup
reports success. At the 10000 default the batch size and the cap
coincide, so nothing is lost today — but the failure mode is invisible
when it does fire, and it fires on the *first* batch.
## The change
Pages on internal node id ranges, which FalkorDB plans as
`NodeByIdSeek`. Every relationship has exactly one source node, so
ranges over `id(a)` partition the edge set with no overlap or gaps.
- Batches stay under the result-set cap, read from `GRAPH.CONFIG GET
RESULTSET_SIZE` and bounded by `batch_size`.
- A batch that *reaches* the cap is ambiguous between complete and
truncated, so its range is subdivided and retried rather than trusted.
- A single node whose out-degree alone reaches the cap falls back to
`SKIP` bounded by that one node's edges.
- Queries carry an explicit timeout — a bulk export shouldn't inherit an
interactive query's `TIMEOUT` budget.
- Exported totals are verified against `count()` before the artifact is
written, with a 1% tolerance for concurrent writes. A larger shortfall
raises `BackupError` instead of writing a backup that looks complete.
Relationship windows self-tune (256 node ids, doubling while batches
stay clear of the cap), so batch count adapts to graph density rather
than being fixed.
**The backup file format is unchanged**, so existing restore tooling
reads these backups as-is.
## Verification
Against a live FalkorDB instance:
- The current exporter reproduces the timeout at 220,000 relationships.
- This one exports 20,715 nodes and 238,668 relationships, matching live
`count()` exactly, zero duplicate ids, in 24 s.
Test suite: 642 passed, 1 skipped (`pytest -m "not integration and not
live"`), including the 24 existing `test_backup_endpoint.py` tests.
`black`, `isort`, and `flake8` clean.
`tests/support/fake_graph.py` gains id-range and `count()`/`max()`
handling plus an optional `resultset_cap` that truncates silently the
way the server does. **Six of the eight new tests fail against the
current exporter**; the two that pass are the small-graph and
empty-graph cases it already handled.
## One thing I did not change
The export is not atomic — nodes are read, then relationships. A
relationship created mid-export can point at a node newer than the node
phase, so it lands in the backup with a target that isn't there. I saw
16 such edges in a run where the graph grew by 24 nodes during the
export. `_restore_relationship_batches` already skips these with a
warning, and the race predates this change (the old exporter had a wider
window, being slower), so I left it alone. Happy to follow up if you'd
like it addressed.
Three defects found by local review before this ever reached GitHub: - The weekly drift alarm could never alarm. npm ci reinstalls exactly what package-lock.json pins, so the scheduled run re-tested the same version forever. It now resolves the latest published package on the schedule path only; PR runs stay reproducible. - The health scenario compared global memory/vector/enrichment counters. The two transports run their batches sequentially against one AutoMem, so the second batch always sees the first's fixtures — a guaranteed false failure no ordering could fix. Those counters are now redacted. - structuredContent was read for chaining and then discarded, so a mismatch in memory_ids, recall count, or health statistics would pass whenever the text matched. It is now normalized, redacted and compared. Also report every differing scenario in one run rather than throwing on the first, so a multi-scenario gap is not a fix-one-rerun loop at ~90s a cycle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second local review pass, three more defects in the harness itself: - redact() stripped only the UUID inside the remote's " (request_id: ...)" error suffix, but the contract allowlists the whole suffix. The three error scenarios would have stayed red forever once everything else aligned. - The initialize check asserted both server names and neither version, so the remote's hardcoded 0.1.0 disagreeing with its own package.json — drift the audit explicitly records — could pass undetected. Each side now has to report its own package version; cross-transport equality is not required because the two packages version independently. - make test-parity fell through its readiness loop and ran the diff against a dead stack, reporting transport mismatches that were really a missing API. It now exits non-zero with service logs, matching the workflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
connectBothTransports opens the HTTP listener before connecting the stdio child. If that child failed to start, the rejection escaped before close() was returned, leaving the listener and remote client alive — and node --test waits on open handles, so the scheduled job would hang until GitHub's timeout instead of reporting the startup failure. The weekly @latest install added in this branch is exactly what makes that reachable: an incompatible entry point resolves to a child that cannot start. Setup is now wrapped, and the accumulated closers run before rethrowing. Verified by removing the package and re-running: the error surfaces in 82ms and the process exits on its own instead of being killed by a watchdog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 2 of Codex review, three findings plus one caught locally. - Register transport/client closers *before* awaiting the handshake. The previous fix only covered a child that failed to spawn; if the process started but the MCP handshake rejected, the closer was never registered and the live child kept node --test open. - Give every scenario its own <tag>-sN namespace and seed its own fixtures. Sharing one namespace meant a write-capability gap corrupted every later comparison: the remote rejects `store batch` while stdio inserts three records, so subsequent recalls compared two different datasets and reported mismatches even where recall rendering was already in parity. That hides which fixes actually worked, precisely when landing them one at a time. Seeding uses single-store only, never a mode one transport lacks. - Remove the weekly cron and the @latest install. While the harness is intentionally red, continue-on-error leaves the workflow successful either way, so the scheduled run had the same external result before and after a regression — an alarm that could not alarm. It returns with the change that turns the harness green. Also, found while verifying: structuredContent was re-parsed after redaction, but redaction substitutes <MS>/<COUNT> placeholders for numbers, so the redacted form is deliberately not valid JSON and JSON.parse threw. It is now compared as a normalized, redacted string. test-parity now uses port 8011. A developer running a local AutoMem install holds 8001, and this harness writes fixtures and bulk-deletes by tag — it must never point at a real memory store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…0ing (#223) Every vector search sends Qdrant a filter condition on the `type` payload field, but that field is never indexed — so Qdrant rejects the search and recall silently falls back to keyword-only. ## The bug `_vector_search` builds its filter with `excluded_types=RECALL_EXCLUDED_TYPES` on every call: ```python query_filter = build_qdrant_tag_filter( tag_filters, tag_mode, tag_match, excluded_types=RECALL_EXCLUDED_TYPES, ) ``` `RECALL_EXCLUDED_TYPES` defaults to a **non-empty** value: ```python RECALL_EXCLUDED_TYPES = frozenset( t.strip() for t in os.getenv("RECALL_EXCLUDED_TYPES", "MetaPattern").split(",") if t.strip() ) ``` So the `type` condition is present by default. But `ensure_qdrant_collection` only indexes `tags` and `tag_prefixes`, and Qdrant refuses to run a filter over an unindexed payload field: ``` Bad request: Index required but not found for "type" of one of the following types: [keyword]. Help: Create an index for this key or use a different filter. ``` ## Why it is easy to miss `_vector_search` catches broadly and returns `[]`: ```python except Exception: logger.exception("Qdrant search failed") return [] ``` Recall keeps returning results, so nothing looks broken — the vector half is just gone. On a live instance the symptoms are: - `vector_search.matched: false` on every recall, every hit `match_type: "keyword"` - `vector_count: null` and `vector_dimensions.collection: null` in `/health` - semantically-phrased queries returning unrelated keyword collisions None of which names the actual cause. The 400 only appears in server logs. ## The fix Add `type` to the ensured payload indexes, and collapse the duplicated enum/string branches into one loop over a named tuple so the set of filtered fields is stated once: ```python QDRANT_FILTERED_PAYLOAD_FIELDS = ("tags", "tag_prefixes", "type") ``` ## Verification Reproduced and fixed on a live instance (~25k points, 768d collection, upgraded from a pre-0.16.0 build). Creating the `type` payload index by hand restored `vector_search.matched: true` and `match_type: "vector"` results immediately, with no other change — and `/health` went from `vector_count: null` back to `vector_count: 24937`. Two regression tests cover both the enum and string-schema branches. Both fail when `type` is removed from the tuple and pass with it (tamper-checked, not just observed green). Note: `tests/test_vector_size_safety.py` has 12 pre-existing failures in a minimal env (`AttributeError: module 'automem' has no attribute 'config'`); they fail identically on an unmodified checkout. This PR takes that file from 9 passing to 11. --------- Co-authored-by: Jack Arturo <johngarturo@gmail.com>
…#230) ## Problem [#224](#224) fixed one symptom: the remote bridge's compact recall block dropped the memory's stored date, so an agent replaying that text read a two-week-old itinerary as today's plan. AutoHub's parser already looked for a `Created:` line — the remote transport simply never emitted one, while the stdio package always had. That single missing line is one instance of a much wider split. `mcp-automem`'s `server.json` publishes stdio, streamable-HTTP, and SSE as **one server with one shared 6-tool array**. A client picking "AutoMem" out of the registry should get the same tools and the same output regardless of transport. It currently does not. A manual schema sync was already attempted once — `d99b86d fix(mcp-sse): sync tool schemas for SSE/MCP parity (#104)` — and has since drifted again. ## What this PR adds **It does not close the gap. It documents the gap and makes it machine-checkable**, so the fix (collapsing both transports onto one shared implementation) lands against a red-to-green target instead of a hand-maintained checklist. - **`docs/MCP_TRANSPORT_PARITY.md`** — the full divergence audit, plus an *accepted transport-level differences* table that is the live contract the harness allowlists against. - **`mcp-sse-server/parity/`** — connects one MCP client to each transport (bridge in-process over streamable HTTP, published stdio package as a child process), both pointed at the same live AutoMem, so any difference is the transports and not the data. - **`make test-parity`** — brings the stack up and diffs them in one command. `make test` now also chains the bridge's node suite, which was previously only reachable in CI and invisible locally. - **`.github/workflows/mcp-parity.yml`** — runs on PRs touching the bridge or the HTTP API, plus weekly as a drift alarm against newly published `mcp-automem` versions. ## What the harness asserts 1. `tools/list` deep-equal after key-order normalization. 2. Server `capabilities` and `instructions` match; `serverInfo.name` is allowlisted to differ, but each side must report its **own** package version. 3. 19 `tools/call` scenarios render identically — covering batch and supersede store, ranked/detailed/items/json/empty recall, id fetch, tag enumeration, relation-prop and batch association, update, single and bulk delete, health, and three error paths. Each transport writes under its own `uuid4` tag namespace so neither sees the other's memories. UUIDs, timestamps, scores, `query_time_ms`, global service counters and the allowlisted `request_id` suffix are redacted before comparison. Cleanup deletes both namespaces by tag. ## Current result: intentionally red All 19 scenarios differ, which is the finding, not a failure of the harness: ``` 19/19 scenarios differ: store single, store batch, store supersede, recall ranked text, recall detailed, recall items, recall json, recall empty, recall id fetch, recall exhaustive, associate single, associate batch partial failure, update, delete single, delete by tag, health, error: exhaustive without tags, error: store content over hard limit, error: unknown tool ``` `tools/list` differs on `_meta`, `title`, `outputSchema`, two annotation hints and all six descriptions; `getInstructions()` is `undefined` on the remote side. The remote is also missing `recall_memory`'s id-fetch and `exhaustive` modes, `store_memory`'s batch and supersede modes, `delete_memory`'s bulk-by-tag, and all nine of `associate_memories`' relation-specific properties. **The workflow is `continue-on-error: true` for exactly this reason** and flips to blocking in the change that closes the gap. ## Test plan ```bash make test # 656 python unit + 22 node; 3 parity tests skip cleanly make test-parity # brings the stack up, reproduces the red baseline ``` - Without `AUTOMEM_RUN_PARITY_TESTS=1` the parity tests skip, which is what CI's existing `node-test` job sees — verified. - The harness leaves no residue: `memory_count` returned to exactly 6000 after repeated runs. ## Notes - **Size:** 2132 changed lines, but 1502 of those are `package-lock.json` — 89 transitive dependency entries from adding the stdio package as a devDependency. **630 lines are reviewable code.** The lockfile has to ship with the harness because CI runs `npm ci`, so splitting would not reduce the reviewable surface. - `@modelcontextprotocol/sdk` is pinned to `1.20.0` exact. Adding the devDependency otherwise hoists the newer SDK over the **deployed** bridge's — a production runtime bump riding in on a test-only change. Pinning makes the devDependency nest its own copy instead. - No behavior change to the bridge itself: this PR adds tests, docs and CI only.
## Summary Closes #222. When the enrichment LLM returns a definitive quota exhaustion error, repeated stores previously retried summarization and classification independently. This adds a shared, fail-soft cooldown circuit across both enrichment paths. ## Changes - Open the circuit on `insufficient_quota`/quota errors for a configurable `ENRICHMENT_CIRCUIT_COOLDOWN_SECONDS` period (default 300 seconds). - Skip additional upstream LLM requests while the circuit is open, while preserving deterministic fallback behavior. - Allow one probe after cooldown and close the circuit after a successful response. - Expose circuit-open skips and recoveries through the enrichment status response. - Add focused regression tests for open, skip, non-quota, and recovery behavior. The default remains backward compatible for successful requests and non-quota transient failures. The change only suppresses further enrichment requests after a quota-related failure; stores remain fail-soft. ## Verification - `python -m compileall -q automem app.py` — passed. - Direct circuit regression assertions — passed. - `python -m black --check app.py automem tests/test_enrichment_circuit.py` — passed. - `python -m flake8 app.py automem --count --select=E9,F63,F7,F82 --show-source --statistics` — passed. - `python -m pytest -q tests/test_enrichment_circuit.py` — not runnable in this environment: pytest collection resolves an unrelated installed `tests` package (`D:\project\tscodex\connectonion-pr\tests`) and cannot import this checkout's `tests.support`; no test assertion was executed. - Full `make test` and Docker integration tests were not run because the focused pytest collection issue must be resolved before they can provide meaningful results.
## Summary - configure newly created Qdrant collections with disk-backed vectors, HNSW, and payloads - add an opt-in HNSW-on-disk restore option for migrating existing collections - document the destructive Qdrant-only migration, validation, rollback, and manual Railway resize boundary Closes #225 ## Testing - `/Users/jgarturo/Projects/OpenAI/automem/.venv/bin/black --check .` - `/Users/jgarturo/Projects/OpenAI/automem/.venv/bin/isort --check-only .` - `/Users/jgarturo/Projects/OpenAI/automem/.venv/bin/flake8 .` - `PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /Users/jgarturo/Projects/OpenAI/automem/.venv/bin/pytest -q -m unit` ## Operational notes Existing production collections are not modified at startup. The runbook requires an explicit backup/restore migration and leaves Railway volume resizing as a manual post-validation operation. ## Breaking changes None.
## Summary - Recommend Voyage embeddings, document OpenAI fallback, local Ollama BGE-M3, and FastEmbed local-only guidance. - Add a canonical scripts catalog, remove machine-specific/internal artifacts from the repository, and refresh the beginner-facing README and benchmark badges. - Correct migration and recovery behavior for authenticated Qdrant and custom FalkorDB graph deployments. ## Validation - `make test` - `make lint` - `docker compose config --quiet` - Markdown link/anchor validation - Local Codex review (two bounded passes) ## Notes - This is an intentionally large documentation and repository-hygiene PR authorized by the maintainer.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c40d1d2653
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release promotion
Promotes the validated
developbranch tomainfor the next stable release.Release Please will update #220 from the resulting
mainhistory.