fix(mcp): accept page/page_size in read_note for parity with sibling tools (#933)

Closes #883

Refs #882

Signed-off-by: phernandez <paul@basicmemory.com>

Author: phernandez

src/basic_memory/mcp/tools/read_note.py

@@ -1,13 +1,14 @@
 """Read note tool for Basic Memory MCP server."""
 
 from textwrap import dedent
-from typing import Optional, Literal, cast
+from typing import Annotated, Optional, Literal, cast
 
 import logfire
 import yaml
 
 from loguru import logger
 from fastmcp import Context
+from pydantic import AliasChoices, Field
 
 from basic_memory.config import ConfigManager
 from basic_memory.mcp.project_context import (
@@ -20,6 +21,17 @@
 from basic_memory.schemas.memory import memory_url_path
 from basic_memory.utils import validate_project_path
 
+# The title-match fallback exists to find THE note by exact title, so it scans
+# fixed-size pages of title results instead of the caller's page/page_size
+# (which apply only to the text-search suggestion listing).
+_TITLE_LOOKUP_PAGE_SIZE = 10
+
+# Hard safety cap on title-lookup pages. The loop normally stops as soon as an
+# exact match is found or results run out (has_more=False); the cap only bounds
+# pathological knowledge bases where hundreds of fuzzy titles contain the
+# queried phrase. Exhausting the cap falls through to the suggestion behavior.
+_TITLE_LOOKUP_MAX_PAGES = 10
+
 
 def _is_exact_title_match(identifier: str, title: str) -> bool:
     """Return True when identifier exactly matches a title (case-insensitive)."""
@@ -71,6 +83,21 @@ async def read_note(
     identifier: str,
     project: Optional[str] = None,
     project_id: Optional[str] = None,
+    # Accept common pagination aliases models reach for from training data
+    # (page_number/limit/per_page), matching the sibling navigation tools
+    # (search_notes, build_context, recent_activity). The schema advertises
+    # only the canonical names; aliases are silently mapped at validation time.
+    # `offset` is intentionally NOT aliased: offset is item-indexed (skip N
+    # items) while page is a 1-indexed page-number, so direct aliasing would
+    # return the wrong slice.
+    page: Annotated[
+        int,
+        Field(default=1, validation_alias=AliasChoices("page", "page_number")),
+    ] = 1,
+    page_size: Annotated[
+        int,
+        Field(default=10, validation_alias=AliasChoices("page_size", "limit", "per_page")),
+    ] = 10,
     output_format: Literal["text", "json"] = "text",
     include_frontmatter: bool = False,
     context: Context | None = None,
@@ -99,6 +126,14 @@ async def read_note(
                 workspaces. Takes precedence over `project`. Get from list_memory_projects().
         identifier: The title or permalink of the note to read
                    Can be a full memory:// URL, a permalink, a title, or search text
+        page: Page of fallback-search results to use when the identifier does not
+            resolve to a note directly (default: 1). A direct or exact-title match
+            always returns the full note content — page/page_size never chunk the
+            note itself, and the title-match lookup pages through fixed-size pages
+            of title results until an exact match is found or results are
+            exhausted, regardless of page or page_size.
+        page_size: Number of fallback-search results per page (default: 10). When no
+            match is found, this caps how many related-note suggestions are listed.
         output_format: "text" returns markdown content or guidance text.
             "json" returns a structured object with title/permalink/file_path/content/frontmatter.
         include_frontmatter: When output_format="json", whether content should include the
@@ -122,6 +157,9 @@ async def read_note(
         # Read recent meeting notes
         read_note("team-docs", "Weekly Standup")
 
+        # Page through fallback-search suggestions when nothing matches directly
+        read_note("unknown topic", page=2, page_size=5)
+
     Raises:
         HTTPError: If project doesn't exist or is inaccessible
         SecurityError: If identifier attempts path traversal
@@ -130,6 +168,15 @@ async def read_note(
         If the exact note isn't found, this tool provides helpful suggestions
         including related notes, search commands, and note creation templates.
     """
+    # Trigger: page < 1 or page_size < 1 (e.g. page_size=0 or negative).
+    # Why: both flow into the fallback search's server-side slicing, where
+    #      non-positive values produce empty result pages with unreachable
+    #      pagination. Fail fast, matching search_notes/build_context.
+    if page < 1:
+        raise ValueError(f"page must be >= 1, got {page}")
+    if page_size < 1:
+        raise ValueError(f"page_size must be >= 1, got {page_size}")
+
     # Detect project from a memory URL or permalink prefix before routing.
     # project_id routes by external UUID, so it bypasses URL discovery entirely.
     if project is None and project_id is None:
@@ -147,6 +194,8 @@ async def read_note(
         tool_name="read_note",
         requested_project=project,
         requested_project_id=project_id,
+        page=page,
+        page_size=page_size,
         output_format=output_format,
         include_frontmatter=include_frontmatter,
     ):
@@ -245,7 +294,7 @@ def _search_results(payload: object) -> list[dict[str, object]]:
                 ]
 
             async def _search_candidates(
-                identifier_text: str, *, title_only: bool
+                identifier_text: str, *, title_only: bool, lookup_page: int = 1
             ) -> dict[str, object]:
                 # Trigger: direct entity resolution failed for the caller's identifier.
                 # Why: search_notes applies the same memory:// normalization and tool-level
@@ -256,11 +305,24 @@ async def _search_candidates(
                 # Without this, project names that collide across workspaces could re-resolve
                 # to a different tenant via the default-workspace fallback (CLI/context=None).
                 search_type = "title" if title_only else "text"
+                # Trigger: title_only — the title search exists to find THE note by
+                #          exact title, not to page through suggestions.
+                # Why: paginating it by the caller's page would skip an exact match
+                #      sitting on page 1 (read_note("Exact Title", page=2)), and a
+                #      small caller page_size could let a higher-ranked fuzzy title
+                #      displace the exact match out of the lookup window
+                #      (read_note("Foo Bar", page_size=1) when "Foo Bar Foo Bar"
+                #      ranks first) — both returning suggestions instead of the note.
+                # Outcome: title lookup uses its own lookup_page with a fixed lookup
+                #          size, walked by the caller below; caller page/page_size
+                #          apply only to the text-search suggestion listing.
                 response = await search_notes(
                     project=active_project.name,
                     project_id=active_project.external_id,
                     query=identifier_text,
                     search_type=search_type,
+                    page=lookup_page if title_only else page,
+                    page_size=_TITLE_LOOKUP_PAGE_SIZE if title_only else page_size,
                     output_format="json",
                     context=context,
                 )
@@ -297,12 +359,24 @@ def _result_file_path(item: dict[str, object]) -> Optional[str]:
                 logger.info(f"Direct lookup failed for '{entity_path}': {e}")
                 # Continue to fallback methods
 
-            # Fallback 1: Try title search via API
+            # Fallback 1: Try title search via API, walking fixed-size pages of
+            # title results until an exact match is found or results run out.
+            # A single page is not enough: when more than _TITLE_LOOKUP_PAGE_SIZE
+            # higher-ranked fuzzy titles contain the queried phrase, the exact
+            # title lands on a later page and a one-page lookup would miss it.
             logger.info(f"Search title for: {identifier}")
-            title_results = await _search_candidates(identifier, title_only=True)
-
-            title_candidates = _search_results(title_results)
-            if title_candidates:
+            result: dict[str, object] | None = None
+            for lookup_page in range(1, _TITLE_LOOKUP_MAX_PAGES + 1):
+                title_results = await _search_candidates(
+                    identifier, title_only=True, lookup_page=lookup_page
+                )
+                title_candidates = _search_results(title_results)
+                if not title_candidates:
+                    logger.info(
+                        f"No results in title search for: {identifier} "
+                        f"in project {active_project.name}"
+                    )
+                    break
                 # Trigger: direct resolution failed and title search returned candidates.
                 # Why: avoid returning unrelated notes when search yields only fuzzy matches.
                 # Outcome: fetch content only when a true exact title match exists.
@@ -314,33 +388,37 @@ def _result_file_path(item: dict[str, object]) -> Optional[str]:
                     ),
                     None,
                 )
-                if not result:
+                if result is not None:
+                    break
+                # Trigger: this page held only fuzzy titles and the server reports
+                #          no further pages (has_more is False or absent).
+                # Why: continuing past the last page would issue empty lookups.
+                # Outcome: give up on the title fallback and try text search below.
+                if title_results.get("has_more") is not True:
                     logger.info(f"No exact title match found for: {identifier}")
-                elif _result_permalink(result):
-                    try:
-                        # Resolve the permalink to entity ID
-                        entity_id = await knowledge_client.resolve_entity(
-                            _result_permalink(result) or "", strict=True
-                        )
+                    break
+
+            if result is not None and _result_permalink(result):
+                try:
+                    # Resolve the permalink to entity ID
+                    entity_id = await knowledge_client.resolve_entity(
+                        _result_permalink(result) or "", strict=True
+                    )
+
+                    # Fetch content using the entity ID
+                    response = await resource_client.read(entity_id)
 
-                        # Fetch content using the entity ID
-                        response = await resource_client.read(entity_id)
-
-                        if response.status_code == 200:
-                            logger.info(
-                                f"Found note by exact title search: {_result_permalink(result)}"
-                            )
-                            if output_format == "json":
-                                return await _read_json_payload(entity_id)
-                            return response.text
-                    except Exception as e:  # pragma: no cover
+                    if response.status_code == 200:
                         logger.info(
-                            f"Failed to fetch content for found title match {_result_permalink(result)}: {e}"
+                            f"Found note by exact title search: {_result_permalink(result)}"
                         )
-            else:
-                logger.info(
-                    f"No results in title search for: {identifier} in project {active_project.name}"
-                )
+                        if output_format == "json":
+                            return await _read_json_payload(entity_id)
+                        return response.text
+                except Exception as e:  # pragma: no cover
+                    logger.info(
+                        f"Failed to fetch content for found title match {_result_permalink(result)}: {e}"
+                    )
 
             # Fallback 2: Text search as a last resort
             logger.info(f"Title search failed, trying text search for: {identifier}")
@@ -352,6 +430,9 @@ def _result_file_path(item: dict[str, object]) -> Optional[str]:
                 if output_format == "json":
                     return _empty_json_payload()
                 return format_not_found_message(active_project.name, identifier)
+            # The fallback search is paginated server-side to page_size, so list
+            # the whole returned page instead of a hardcoded cap — otherwise the
+            # caller's page_size would be silently ignored past the cap.
             if output_format == "json":
                 payload = _empty_json_payload()
                 payload["related_results"] = [
@@ -360,10 +441,10 @@ def _result_file_path(item: dict[str, object]) -> Optional[str]:
                         "permalink": _result_permalink(result),
                         "file_path": _result_file_path(result),
                     }
-                    for result in text_candidates[:5]
+                    for result in text_candidates
                 ]
                 return payload
-            return format_related_results(active_project.name, identifier, text_candidates[:5])
+            return format_related_results(active_project.name, identifier, text_candidates)
 
 
 def format_not_found_message(project: str | None, identifier: str) -> str:

src/basic_memory/mcp/tools/write_note.py

@@ -87,8 +87,12 @@ async def write_note(
                    Use forward slashes (/) as separators. Use "/" or "" to write to project root.
                    Examples: "notes", "projects/2025", "research/ml", "/" (root)
         project: Project name to write to. Optional - server will resolve using the
-                hierarchy above. If unknown, use list_memory_projects() to discover
-                available projects.
+                hierarchy above. Use "workspace/project" to route to a project in a
+                specific cloud workspace. A bare name that exists in multiple
+                workspaces resolves to the default workspace, so use the qualified
+                form (or project_id) to disambiguate. If unknown, use
+                list_memory_projects() to discover available projects and their
+                qualified names.
         project_id: Project external_id (UUID). Prefer this over `project` when known —
                 it routes to the exact project regardless of name collisions across cloud
                 workspaces. Takes precedence over `project`. Get from list_memory_projects().

test-int/mcp/test_param_aliases_integration.py

@@ -13,9 +13,49 @@
 from fastmcp import Client
 
 
-# --- read_note: pagination params removed in #693 (were no-ops) ---
-# The `page` / `page_size` parameters were removed because the API endpoint
-# silently dropped them. Search-fallback pagination is unrelated to read_note.
+# --- read_note: pagination params restored in #883 ---
+# `page` / `page_size` were removed in #693 because they were no-ops on the
+# resource read. #883 restored them for parity with the sibling navigation
+# tools: they paginate the server-side search fallback (a direct match still
+# returns the full note content).
+
+
+@pytest.mark.asyncio
+async def test_read_note_accepts_pagination_params_and_aliases(mcp_server, app, test_project):
+    """Agents passing page/page_size (or their aliases) must not get a validation error."""
+    async with Client(mcp_server) as client:
+        await client.call_tool(
+            "write_note",
+            {
+                "project": test_project.name,
+                "title": "Paged Read Note",
+                "directory": "test",
+                "content": "# Paged Read Note\n\npaged read body",
+            },
+        )
+
+        result = await client.call_tool(
+            "read_note",
+            {
+                "project": test_project.name,
+                "identifier": "Paged Read Note",
+                "page": 1,
+                "page_size": 10,
+            },
+        )
+        assert "paged read body" in result.content[0].text
+
+        # Aliases map silently: page_number -> page, limit -> page_size
+        result = await client.call_tool(
+            "read_note",
+            {
+                "project": test_project.name,
+                "identifier": "Paged Read Note",
+                "page_number": 1,
+                "limit": 10,
+            },
+        )
+        assert "paged read body" in result.content[0].text
 
 
 # --- edit_note: find_text / content / section aliases ---
@@ -485,12 +525,12 @@ async def test_aliases_not_advertised_in_schema(mcp_server, app):
 
         # tool_name -> (must_have_canonical, must_not_have_aliases)
         checks = {
-            # read_note has no pagination params (#693 — they were no-ops; removed).
-            # The must_not_have list still includes the rejected aliases so future
-            # contributors don't reintroduce them.
+            # read_note pagination restored in #883 (paginates the search fallback).
+            # Accepted aliases (limit/page_number/per_page) plus the rejected
+            # `offset` must stay out of the advertised schema.
             "read_note": (
-                [],
-                ["page", "page_size", "offset", "limit", "page_number", "per_page"],
+                ["page", "page_size"],
+                ["offset", "limit", "page_number", "per_page"],
             ),
             "edit_note": (
                 ["find_text", "section", "content"],

tests/mcp/test_tool_contracts.py

@@ -62,6 +62,8 @@
         "identifier",
         "project",
         "project_id",
+        "page",
+        "page_size",
         "output_format",
         "include_frontmatter",
     ],