perf(api): add Redis-cached QUERY and MCP reads - #1172
Conversation
Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
![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: 17ecb670bc
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67e3d5bb46
ℹ️ 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".
| redis_url: Optional[str] = Field( | ||
| default=None, | ||
| description="Optional Redis URL for standalone MCP read caching. Bare hostnames are accepted.", | ||
| ) |
There was a problem hiding this comment.
When BASIC_MEMORY_REDIS_URL contains standard Redis userinfo or query credentials, registering it as a scalar config field makes bm config get/list and basic_memory_diagnostics expose the complete URL verbatim because the shared redaction module's URL_FIELDS set was not updated. Diagnostics may be returned to MCP clients or logs, so add redis_url to the URL-redaction fields and cover credential-bearing values.
Useful? React with 👍 / 👎.
| await asyncio.sleep(args.quiesce_seconds) | ||
| await warm_targets(session, project, targets) |
There was a problem hiding this comment.
When the Redis-enabled sweep takes more than the 60-second read-cache TTL, this single warmup leaves later size/concurrency rows measuring a mixture of expired authoritative misses and cache hits; cache lookups do not refresh the response-key TTL. This can invert or dilute the comparison on slower hosts or with larger --reads, despite every row being labeled as warmed, so warm each scenario immediately before starting its timer.
Useful? React with 👍 / 👎.
| record: dict[str, object] = { | ||
| "benchmark": (f"read-load size={size_label(size_bytes)} c={concurrency}"), | ||
| "timestamp_utc": datetime.now(UTC).isoformat(), | ||
| "label": args.label, | ||
| "metadata": { |
There was a problem hiding this comment.
When these JSONL files are retained or used for a performance comparison, the metadata records only a caller-supplied label and runtime settings, not the invoked Basic Memory source/resolved SHA or version, and the harness emits no manifest.json. Consequently two artifacts cannot establish which builds produced them and are not reproducible; emit the required provenance manifest alongside the results.
AGENTS.md reference: benchmarks/AGENTS.md:L78-L83
Useful? React with 👍 / 👎.
1 participant
Why
Search is a read, but Basic Memory's v2 API exposed it only as
POST. That made the transportsemantics misleading and kept the typed client from using HTTP QUERY. Search and whole-note reads
are also among the slowest hosted MCP operations, so exact repeated reads should avoid rebuilding
the same typed response from the authoritative database and file path.
This PR adds the non-mutating search transport, caches the complete semantic operation, and makes
the same optional Redis cache available to standalone
bm mcpwithout breaking existing callersor adding Redis to the default local-first runtime.
What Changed
QUERYand legacyPOSTon the same/v2/projects/{project_id}/search/FastAPIfunction and send QUERY from
SearchClient.project-generation invalidation.
BASIC_MEMORY_REDIS_URLis set; bare hosts such asredisnormalize toredis://redis.invalidation, then close its Redis client during shutdown.
Nonewhen unconfigured instead of introducing no-op cachebehavior.
justrecipes for the complete cachesuite and paired read-load benchmark.
distinction between semantic cache data and Cloud rate-limit/control-plane data.
Implementation Details
Search and read-cache semantics
QUERY and POST share one authoritative search function and one cache identity because transport
method is not semantic request identity. The key includes the trusted namespace, canonical project
UUID, project generation, operation, complete validated request, and pagination. Canonical JSON
stabilizes object-key ordering while preserving list order. Successful empty pages are cacheable;
exceptions and cancellation never populate Redis.
Mutations invalidate by replacing the project generation rather than scanning Redis. The request,
indexing, relation-resolution, and storage-event paths therefore share one correctness boundary.
Standalone MCP lifecycle
src/basic_memory/read_cache/lifecycle.pyowns the optional import, URL normalization, clientconstruction,
RedisReadCachefacade, and cleanup.src/basic_memory/mcp/server.pycreates thatresource once per MCP lifespan and installs the same cache in both
McpContainerand theoff-lifespan
ApiContainer. This matters because local MCP requests use FastAPI through anin-process ASGI transport and do not enter FastAPI's lifespan.
BASIC_MEMORY_REDIS_URLis the sole standalone switch. When it is absent or blank, the dependencyis
Noneand callers execute only the authoritative path. The optionalredispackage extra isstill required when the variable is enabled.
Cloud tenant boundary
Cloud runtime deliberately ignores the standalone Redis setting. Cloud constructs and injects its
own cache from trusted tenant context, using an opaque tenant namespace plus the canonical project
UUID. A physical Redis deployment and connection pool may be shared across tenants and with rate
limiting, but semantic-cache keys remain under
bm:read:v1, tenant namespaces never come fromrequest input, and rate-limit/control-plane keyspaces retain separate ownership and failure policy.
The coordinated hosted integration remains in
basic-memory-cloud#1644.
Benchmark tooling
just bench-read-cache <redis-url> <run-id>runs adjacent authoritative and warmed-Redis MCP stdiosweeps over 1, 16, and 64 KiB notes at concurrency 1, 8, 32, and 64. Corpus creation, indexing,
connection setup, and cache warmup stay outside the measured region. Each run ID preserves separate
scratch directories and JSONL artifacts; the harness records latency percentiles, throughput,
bandwidth, errors, validation failures, and workload metadata without recording Redis credentials.
Testing
Automated
just fast-check: passed (ruff, formatting, andty).just test-read-cache: 63 passed against real Redis.just doctor: passed the end-to-end file, database, indexing, search, and status loop.just --dry-run bench-read-cache redis://127.0.0.1:6379/0 check: generated the expectedrun-scoped authoritative, Redis, and comparison commands.
git diff --check: passed before the signed commit.Benchmark observation
The first adjacent local SQLite pair completed 3,072 measured reads per mode with zero read or
response-validation errors. Across twelve size/concurrency scenarios it showed a directional
median p50 reduction of about 54% and median throughput gain of about 126% for warmed Redis.
A second pair run through the checked-in
justrecipe also completed without correctness errors,but its Redis-side corpus preparation took roughly 11.2 seconds versus 5.1 seconds for the baseline
and several latency rows inverted under workstation contention. It is retained as proof that the
tooling completes, not included in the performance claim. The benchmark guide requires at least
six alternating paired repetitions and rejects materially contended runs before drawing a stable
conclusion.
Risks / Follow-ups
compatibility route and QUERY is intentionally hidden from the generated schema.
BASIC_MEMORY_REDIS_URLwithout installing theredisextrafails at MCP startup instead of silently disabling the requested cache.
repeated and hosted measurements are still needed for a production performance claim.
directory, and resource access. Short TTLs and project invalidation bound stale-data risk.
hosted activation. No per-tenant Redis instance is required.