Skip to content

perf(mcp): optimize semantic note reads - #1255

Merged
phernandez merged 1 commit into
mainfrom
cloud-cache-note-reads
Aug 13, 2026
Merged

perf(mcp): optimize semantic note reads#1255
phernandez merged 1 commit into
mainfrom
cloud-cache-note-reads

Conversation

@phernandez

@phernandez phernandez commented Aug 13, 2026

Copy link
Copy Markdown
Member

Why

Core JSON note reads currently perform avoidable resolve and resource requests, and ordinary semantic cache entries expire after only 60 seconds. The read paths also need enough bounded telemetry to distinguish lookup failures from store failures without weakening Redis fail-open behavior.

This change gives Cloud a reviewable Core commit to pin while keeping deployment work out of scope.

What Changed

  • Added a reusable typed exact-external-ID helper that returns the existing JSON note payload.
  • Made accepted knowledge-entity content authoritative, with one resource request only when entity content is genuinely absent.
  • Made exact UUID JSON reads skip resolve and resource calls, permalink reads resolve once then fetch the entity once, and exact-title reads use the returned external ID directly.
  • Preserved the existing text-mode read behavior.
  • Raised ordinary semantic read-cache TTL from 60 to 300 seconds while keeping search at 30 seconds.
  • Split cache lookup and store outcomes and added configured TTL, remaining hit TTL, payload size, operation, and SHA-256 generation digest telemetry.
  • Added terminal corrupt outcomes for invalid model JSON, invalid Redis lookup/store contracts, and cached payloads without expiry while preserving fail-fast corruption and fail-open availability behavior.

Implementation Details

The new note-read helper depends on narrow typed entity/resource reader protocols. It reads the accepted entity exactly once and only falls back to the resource route when content is None; an accepted empty string remains authoritative. Title, permalink, and file path come from the accepted entity response, while frontmatter is parsed from the same canonical Markdown so indexed or normalized entity metadata cannot fabricate or alter the existing JSON payload.

Generic JSON routing now selects the minimum deterministic request sequence before calling the helper. Syntactically valid UUID identifiers are exact external IDs in JSON mode, so misses do not enter an ambiguous resolver fallback. Text mode retains its prior resolve/resource flow.

Read-cache telemetry uses bounded outcome names and hashes generation values before reporting them. Redis availability exceptions remain bypasses, while malformed Lua results, persistent payload keys, and invalid cached model JSON are reported as corrupt and re-raised.

Testing

Automated

  • uv run pytest -p pytest_mock --no-cov -q tests/mcp/test_read_note_request_counts.py tests/mcp/test_tool_read_note.py: 51 passed.
  • focused real-Redis and telemetry corruption regressions: 9 passed.
  • just test-read-cache: 65 passed.
  • OPENAI_API_KEY= just fast-test: 11 passed, 1 skipped, 38 deselected.
  • just fast-check: passed.
  • just doctor: passed.
  • git diff --check: passed.

Risks / Follow-ups

  • Cache corruption remains deliberately fail-fast; only backend availability failures fail open.
  • JSON identifiers that parse as UUIDs are exact external IDs by contract and do not fall back to title/permalink resolution on a miss.
  • The impacted-test run skipped the live OpenAI-provider test because no API key was supplied; the changed Core paths have focused coverage.
  • No production deployment is included. Cloud can pin the reviewed Core commit separately.

@phernandez
phernandez force-pushed the cloud-cache-note-reads branch from 31861cc to 76f1fe6 Compare August 13, 2026 18:36
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76f1fe6ef3

ℹ️ 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".

Comment thread src/basic_memory/mcp/note_reads.py Outdated
@phernandez
phernandez force-pushed the cloud-cache-note-reads branch from 76f1fe6 to 7f0ae4e Compare August 13, 2026 18:46
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f0ae4ea18

ℹ️ 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".

Comment thread src/basic_memory/mcp/tools/read_note.py
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f0ae4ea18

ℹ️ 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".

Comment thread src/basic_memory/read_cache/redis.py Outdated
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez
phernandez force-pushed the cloud-cache-note-reads branch from 7f0ae4e to 9fa61b7 Compare August 13, 2026 18:58
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 9fa61b7247

ℹ️ 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".

@phernandez
phernandez merged commit 426c174 into main Aug 13, 2026
28 checks passed
@phernandez
phernandez deleted the cloud-cache-note-reads branch August 13, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant