feat(phase9 #96 D10.d): cursor placeholder + kw-only sentinel on split search tools (#1717)

* feat(phase9 #96 D10.d): cursor placeholder + kw-only sentinel on split search tools

D10.d task #96 same-lane follow-up per `[D10 spec amendment]` thread
(msg=b9b7072a) PM canonical decisions:

- Drift #5: add `*,` kw-only barrier after `query` on the 3
  collection-scoped split tools (`vector_search` / `graph_search` /
  `fulltext_search`), aligning with the D10.c read-primitive precedent
  established in #1714. `web_search` keeps its existing wire signature
  per §B.4 / amendment (parameter canonicalization deferred to D10.h
  cutover).

- Drift #4 (c): publish `cursor: str | None = None` placeholder on the
  same 3 tools so external MCP clients see the canonical surface, but
  the body explicitly fails with
  `CursorError("cursor_invalid", "search pagination cursor is not yet
  implemented", details={"reason": "search_not_paginated", "tool":
  ...})` on any non-null value. `cursor=None` continues to mean "first
  page" (current behavior). Real search pagination requires a backend
  capability that will land in a dedicated D11+ upgrade.

`fulltext_search` also gains `rerank: bool = True` to match the §B.3
spec; previously the rerank flag was reachable only via the omnibus
`search_collection`.

Drifts #1 (`SearchResultItem` with chunk_id) and #2 (`web_search`
canonicalization) intentionally NOT in this PR — both deferred to the
D10.h cutover lane per the amendment thread. Drift #3 (capability
annotations) belongs to D10.f (task #98).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(phase9 #96 D10.d): cursor=="" → first page, NotImplementedError on truthy cursor

Per Weston blocker review (msg=177a1dd8) on PR #1717 + architect
sign-off (msg=ebfcdabe):

1. **Empty-string cursor must equal None** — both should preserve the
   existing single-page `top_k` behavior. The previous `is not None`
   guard incorrectly raised on `cursor=""`. Switched to `if cursor:`
   (truthiness check) so the loud-fail only triggers on truly
   non-empty cursor values.

2. **Stop pretending feature-not-implemented is a malformed cursor** —
   the canonical `CursorError("cursor_invalid", ...)` describes
   wire-level malformed cursors. Using it for "search pagination is
   not implemented" camouflages a missing capability as a client-side
   cursor bug. Switched to plain `NotImplementedError("search
   pagination is not yet implemented (tool=..., reason=
   search_not_paginated)")` so the loud-fail accurately surfaces the
   deferred-capability semantic.

Test updates:
- `cursor=""` removed from the bad-cursor parametrize (it is no
  longer a bad cursor)
- New `test_collection_scoped_split_tools_treat_none_and_empty_cursor_as_first_page`
  monkeypatches httpx + get_api_key and asserts that both `None` and
  `""` cursor pass through the guard and reach the backend search call
- Loud-fail test renamed to `_reject_non_empty_cursor` and asserts
  `NotImplementedError` (not `CursorError`) with a clear "not
  implemented" message including the originating tool and reason

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: earayu

aperag/mcp/tools/search_fulltext.py

@@ -21,8 +21,14 @@
 
 Wire shape: returns the existing ``SearchResult`` dict shape with
 ``recall_type = "fulltext_search"`` for produced items. §B canonical
-shape follow-up gated on the chunk_id propagation amendment thread —
-see ``search_vector.py`` module docstring.
+shape follow-up settled by the ``[D10 spec amendment]`` thread
+(msg=b9b7072a) — defer canonical SearchResultItem to D10.h cutover.
+
+The ``cursor`` parameter is a placeholder per the same thread (Drift
+#4 (c)): signature lands now, body raises ``NotImplementedError``
+on any non-empty value until real search pagination ships.
+``None`` and ``""`` both preserve single-page ``top_k`` behavior per
+the Weston blocker review (msg=177a1dd8).
 """
 
 from __future__ import annotations
@@ -42,8 +48,11 @@
 async def fulltext_search(
     collection_id: str,
     query: str,
+    *,
     top_k: int = 5,
     keywords: list[str] | None = None,
+    rerank: bool = True,
+    cursor: str | None = None,
 ) -> Dict[str, Any]:
     """Full-text keyword search within a collection (§B.3).
 
@@ -76,12 +85,23 @@ async def fulltext_search(
         top_k: Maximum number of results to return (default: 5).
         keywords: Optional explicit keyword list overriding the
             auto-extracted keywords from the query.
+        rerank: Whether to apply reranker on returned candidates
+            (default: True).
+        cursor: Pagination cursor placeholder (§B.3 / amendment
+            msg=b9b7072a Drift #4 (c)). ``None`` and ``""`` return
+            first page; any non-empty value raises
+            ``NotImplementedError`` with a clear "not implemented"
+            message until real search pagination ships.
 
     Returns:
         Search results with ``items`` carrying ``recall_type =
         "fulltext_search"``. Highlight snippets / matched terms are
         surfaced via ``items[*].metadata``.
     """
+    if cursor:
+        raise NotImplementedError(
+            "search pagination is not yet implemented (tool=fulltext_search, reason=search_not_paginated)"
+        )
     try:
         api_key = get_api_key()
 
@@ -91,6 +111,7 @@ async def fulltext_search(
 
         search_data: Dict[str, Any] = {
             "query": query,
+            "rerank": rerank,
             "fulltext_search": fulltext_payload,
         }
 

aperag/mcp/tools/search_graph.py

@@ -20,9 +20,15 @@
 until the D10.h cutover.
 
 Wire shape: returns the existing ``SearchResult`` dict shape with
-``recall_type = "graph_search"`` for produced items. §B canonical shape
-follow-up gated on the chunk_id propagation amendment thread — see
-``search_vector.py`` module docstring.
+``recall_type = "graph_search"`` for produced items. §B canonical
+shape follow-up settled by the ``[D10 spec amendment]`` thread
+(msg=b9b7072a) — defer canonical SearchResultItem to D10.h cutover.
+
+The ``cursor`` parameter is a placeholder per the same thread (Drift
+#4 (c)): signature lands now, body raises ``NotImplementedError``
+on any non-empty value until real search pagination ships.
+``None`` and ``""`` both preserve single-page ``top_k`` behavior per
+the Weston blocker review (msg=177a1dd8).
 """
 
 from __future__ import annotations
@@ -42,7 +48,9 @@
 async def graph_search(
     collection_id: str,
     query: str,
+    *,
     top_k: int = 5,
+    cursor: str | None = None,
 ) -> Dict[str, Any]:
     """Knowledge-graph search within a collection (§B.2).
 
@@ -74,12 +82,21 @@ async def graph_search(
         collection_id: The ID of the collection to search.
         query: The natural-language search query.
         top_k: Maximum number of results to return (default: 5).
+        cursor: Pagination cursor placeholder (§B.2 / amendment
+            msg=b9b7072a Drift #4 (c)). ``None`` and ``""`` return
+            first page; any non-empty value raises
+            ``NotImplementedError`` with a clear "not implemented"
+            message until real search pagination ships.
 
     Returns:
         Search results with ``items`` carrying ``recall_type =
         "graph_search"``. Graph-specific fields (entity / relation / path)
         are surfaced via ``items[*].metadata``.
     """
+    if cursor:
+        raise NotImplementedError(
+            "search pagination is not yet implemented (tool=graph_search, reason=search_not_paginated)"
+        )
     try:
         api_key = get_api_key()
 

aperag/mcp/tools/search_vector.py

@@ -23,10 +23,24 @@
 ``aperag/api/v2/collections/{id}/searches`` backend (``recall_type =
 "vector_search"`` for produced items). The §B canonical
 ``SearchResult`` / ``SearchResultItem`` with ``chunk_id`` /
-``section_path`` / ``heading_anchor`` is deferred to a D10.d follow-up
-PR after the chunk_id propagation question is resolved via a
-``[D10 spec amendment]`` thread (current backend does not surface
-``chunk_id`` in the public response shape — see PR description).
+``section_path`` / ``heading_anchor`` exposure was settled by the
+``[D10 spec amendment]`` thread (msg=b9b7072a) — Drift #1 resolution
+(c): defer the canonical shape to the D10.h cutover lane (one-shot
+``aperag/domains/retrieval/`` upgrade) so this lane stays focused on
+the split surface + omnibus deprecation.
+
+The ``cursor`` parameter is a placeholder per the same amendment
+thread (Drift #4 resolution (c)): the signature is published now so
+external MCP clients see the canonical shape, but the body explicitly
+raises ``NotImplementedError("search pagination is not yet
+implemented")`` on any **non-empty** value. The architect sign-off
+(msg=ebfcdabe) plus Weston's blocker review (msg=177a1dd8) explicitly
+forbid reusing the canonical ``CursorError("cursor_invalid", ...)``
+malformed-wire semantic for this "feature not implemented" case —
+that would camouflage missing capability as a client-side cursor bug.
+``cursor=None`` and ``cursor=""`` both keep the existing single-page
+``top_k`` behavior (Weston msg=177a1dd8 lock); only truthy /
+non-empty cursor values trigger the loud-fail.
 """
 
 from __future__ import annotations
@@ -46,9 +60,11 @@
 async def vector_search(
     collection_id: str,
     query: str,
+    *,
     top_k: int = 5,
     similarity_threshold: float | None = None,
     rerank: bool = True,
+    cursor: str | None = None,
 ) -> Dict[str, Any]:
     """Vector similarity search within a collection (§B.1).
 
@@ -83,11 +99,26 @@ async def vector_search(
         similarity_threshold: Minimum similarity score [0, 1]; ``None``
             uses the collection's default threshold.
         rerank: Whether to apply reranker on returned candidates (default: True).
+        cursor: Pagination cursor placeholder (§B.1). ``None`` and
+            ``""`` both return the first page (current behavior); any
+            non-empty value raises ``NotImplementedError`` with a
+            ``"search pagination is not yet implemented"`` message per
+            the ``[D10 spec amendment]`` (msg=b9b7072a + sign-off
+            msg=ebfcdabe + Weston msg=177a1dd8). Real search
+            pagination requires a backend capability that is not yet
+            available and will land in a dedicated D11+ upgrade.
 
     Returns:
         Search results with ``items`` ranked by vector similarity. Each
         item carries ``recall_type = "vector_search"``.
     """
+    if cursor:
+        # ``cursor`` is truthy iff non-None and non-empty (Weston
+        # msg=177a1dd8 lock: ``None`` and ``""`` both preserve
+        # single-page ``top_k`` behavior).
+        raise NotImplementedError(
+            "search pagination is not yet implemented (tool=vector_search, reason=search_not_paginated)"
+        )
     try:
         api_key = get_api_key()
 

tests/unit_test/mcp/test_search_split.py

@@ -39,6 +39,8 @@
 import inspect
 from pathlib import Path
 
+import pytest
+
 # Importing ``aperag.mcp.server`` triggers the bottom-of-module
 # registration block which loads ``aperag.mcp.tools.search_*`` and
 # triggers ``@mcp_server.tool`` decorators. The tool functions are
@@ -88,41 +90,68 @@ def _params(func) -> dict[str, inspect.Parameter]:
     return dict(inspect.signature(func).parameters)
 
 
+def _kw_only_params(func) -> set[str]:
+    return {
+        name
+        for name, param in inspect.signature(func).parameters.items()
+        if param.kind is inspect.Parameter.KEYWORD_ONLY
+    }
+
+
 def test_vector_search_signature_matches_b1_lock():
-    """``vector_search`` signature follows §B.1: collection_id + query
-    positional, then keyword-tunable top_k / similarity_threshold /
-    rerank.
+    """``vector_search`` signature follows §B.1: ``collection_id`` +
+    ``query`` positional, then a ``*,`` kw-only barrier with
+    ``top_k`` / ``similarity_threshold`` / ``rerank`` / ``cursor``.
+    The kw-only enforcement aligns with the D10.c precedent (per
+    `[D10 spec amendment]` msg=b9b7072a Drift #5 resolution).
     """
     params = _params(vector_search)
     assert list(params)[:2] == ["collection_id", "query"]
-    for name in ("top_k", "similarity_threshold", "rerank"):
-        assert name in params, f"§B.1 requires `{name}` parameter"
+    assert _kw_only_params(vector_search) == {
+        "top_k",
+        "similarity_threshold",
+        "rerank",
+        "cursor",
+    }, "§B.1 requires kw-only `top_k / similarity_threshold / rerank / cursor`."
     assert params["top_k"].default == 5
     # similarity_threshold defaults to None to mean "use collection default".
     assert params["similarity_threshold"].default is None
     assert params["rerank"].default is True
+    # cursor placeholder per amendment msg=b9b7072a Drift #4 (c).
+    assert params["cursor"].default is None
 
 
 def test_graph_search_signature_matches_b2_lock():
-    """``graph_search`` signature follows §B.2: collection_id + query +
-    top_k. The §B.2 spec mentions ``depth`` / ``entity_types`` as
+    """``graph_search`` signature follows §B.2 + amendment msg=b9b7072a:
+    collection_id + query positional then kw-only top_k + cursor.
+    The §B.2 spec mentions ``depth`` / ``entity_types`` as
     forward-looking knobs; first-cut keeps them out until the backend
     surface for graph traversal is wired (deferred to D10.d follow-up).
     """
     params = _params(graph_search)
     assert list(params)[:2] == ["collection_id", "query"]
-    assert "top_k" in params
+    assert _kw_only_params(graph_search) == {"top_k", "cursor"}, "§B.2 requires kw-only `top_k / cursor`."
     assert params["top_k"].default == 5
+    assert params["cursor"].default is None
 
 
 def test_fulltext_search_signature_matches_b3_lock():
-    """``fulltext_search`` signature follows §B.3: collection_id +
-    query + top_k + optional keyword override.
+    """``fulltext_search`` signature follows §B.3 + amendment
+    msg=b9b7072a: collection_id + query positional then kw-only
+    top_k + keywords + rerank + cursor.
     """
     params = _params(fulltext_search)
     assert list(params)[:2] == ["collection_id", "query"]
-    assert "top_k" in params and params["top_k"].default == 5
-    assert "keywords" in params and params["keywords"].default is None
+    assert _kw_only_params(fulltext_search) == {
+        "top_k",
+        "keywords",
+        "rerank",
+        "cursor",
+    }, "§B.3 requires kw-only `top_k / keywords / rerank / cursor`."
+    assert params["top_k"].default == 5
+    assert params["keywords"].default is None
+    assert params["rerank"].default is True
+    assert params["cursor"].default is None
 
 
 def test_web_search_signature_preserves_existing_wire_for_b4():
@@ -139,6 +168,128 @@ def test_web_search_signature_preserves_existing_wire_for_b4():
     assert "source" in params and params["source"].default == ""
 
 
+# --- 2b. Cursor placeholder explicit-not-silent (§B / amendment Drift #4) ---
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "tool",
+    [vector_search, graph_search, fulltext_search],
+    ids=["vector_search", "graph_search", "fulltext_search"],
+)
+@pytest.mark.parametrize(
+    "bad_cursor",
+    ["garbage", "AAAA", "{}", "0"],
+    ids=["garbage", "base64_no_payload", "empty_json", "single_zero"],
+)
+async def test_collection_scoped_split_tools_reject_non_empty_cursor(tool, bad_cursor):
+    """Per `[D10 spec amendment]` msg=b9b7072a Drift #4 (c) +
+    architect sign-off msg=ebfcdabe + Weston blocker msg=177a1dd8:
+    any non-empty cursor must loud-fail with ``NotImplementedError``
+    bearing a "search pagination is not yet implemented" message.
+
+    The sign-off explicitly forbids reusing
+    ``CursorError("cursor_invalid", ...)`` for this case — the canonical
+    cursor codes describe wire-level malformed / expired / drifted
+    cursors, and the "feature not implemented yet" semantic must not
+    be camouflaged as a malformed-cursor error.
+
+    ``cursor=None`` and ``cursor=""`` are covered by the
+    happy-path-passthrough test below; this case only exercises the
+    truthy-cursor loud-fail path.
+    """
+
+    with pytest.raises(NotImplementedError) as excinfo:
+        await tool("collection-id", "query", cursor=bad_cursor)
+
+    message = str(excinfo.value)
+    assert "search pagination is not yet implemented" in message, (
+        f"{tool.__name__} loud-fail message must clearly state that "
+        f"search pagination is not implemented (got {message!r})."
+    )
+    assert tool.__name__ in message, (
+        "Loud-fail message must identify the originating tool so logs can attribute the rejection."
+    )
+    assert "search_not_paginated" in message, (
+        "Loud-fail message must surface the deferred-pagination reason so clients understand they should not retry."
+    )
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "tool",
+    [vector_search, graph_search, fulltext_search],
+    ids=["vector_search", "graph_search", "fulltext_search"],
+)
+@pytest.mark.parametrize(
+    "noop_cursor",
+    [None, ""],
+    ids=["none", "empty_string"],
+)
+async def test_collection_scoped_split_tools_treat_none_and_empty_cursor_as_first_page(tool, noop_cursor, monkeypatch):
+    """Weston blocker msg=177a1dd8 lock: ``None`` *and* ``""`` cursor
+    must both preserve the existing single-page ``top_k`` behavior
+    (``""`` is **not** a malformed cursor; it is the canonical
+    "no pagination requested" wire value).
+
+    We stub ``get_api_key()`` and the outbound httpx call so the test
+    only validates that the cursor guard does **not** raise — it does
+    not exercise the backend search path itself.
+    """
+
+    import aperag.mcp.tools.search_fulltext as sft
+    import aperag.mcp.tools.search_graph as sg
+    import aperag.mcp.tools.search_vector as sv
+
+    monkeypatch.setattr(sv, "get_api_key", lambda: "test-token")
+    monkeypatch.setattr(sg, "get_api_key", lambda: "test-token")
+    monkeypatch.setattr(sft, "get_api_key", lambda: "test-token")
+
+    class _FakeResponse:
+        status_code = 200
+
+        def json(self):
+            return {"items": []}
+
+    class _FakeAsyncClient:
+        def __init__(self, *args, **kwargs):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, exc_type, exc, tb):
+            return False
+
+        async def post(self, url, headers=None, json=None):
+            return _FakeResponse()
+
+    import httpx
+
+    monkeypatch.setattr(httpx, "AsyncClient", _FakeAsyncClient)
+
+    # No exception expected — the cursor guard must not raise on
+    # ``None`` or ``""``.
+    result = await tool("collection-id", "query", cursor=noop_cursor)
+    assert isinstance(result, dict), (
+        f"{tool.__name__} must return the SearchResult dict for None/empty cursor (got {type(result).__name__})."
+    )
+
+
+@pytest.mark.asyncio
+async def test_web_search_does_not_carry_cursor_param():
+    """``web_search`` is intentionally cursor-less — its provider
+    chain (Jina / DDG) is provider-bounded, not paginated, per §B.4.
+    The amendment thread (msg=b9b7072a Drift #4) only added cursor
+    placeholders to the 3 collection-scoped split tools.
+    """
+
+    params = _params(web_search)
+    assert "cursor" not in params, (
+        "web_search must not carry a cursor parameter — its scope is provider-bounded per §B.4 (no pagination)."
+    )
+
+
 # --- 3. Deprecation banner on omnibus aliases ------------------------