feat(phase9 #98 D10.f): capability negotiation Option A + annotation metadata#1719
Merged
Conversation
…metadata Per docs/modularization/d10-design-pack.md §D every D10 tool now carries a frozen ToolAnnotation envelope (requires / capabilities / deprecated / deprecated_until / fallback_to) onto the MCP wire so external Agents can do client-side filtering without server-side session state. §G D10.f write-set: - aperag/mcp/capabilities.py — ToolAnnotation Pydantic model + closed KNOWN_CAPABILITIES / KNOWN_REQUIRES sets + to_mcp_dict() for FastMCP annotations= kwarg. - aperag/mcp/tools/_annotations.py — name → ToolAnnotation registry with re-registration guard (idempotent on identical re-register; ValueError on conflict). - aperag/mcp/server.py + aperag/mcp/tools/search_*.py — additive decorator wrap on all 8 D10.c read primitives + 4 D10.d search primitives. No body / signature change. - aperag/sdk/capability_filter.py — Option A client-side filter (FilterDecision explicit-not-silent reasons per §D.3, deprecated pass-through, missing-capability sorted output). - tests/unit_test/mcp/test_capability_negotiation.py — 37 tests covering schema validity, registry, register() semantics, §D.2 filter pseudocode, §D.3 explicit-not-silent decisions. - tests/e2e_http/hurl/full/21_d10_capabilities.hurl — wire-shape contract: tools/list returns every D10 tool name plus annotation field markers (requires / capabilities / graph_index / fulltext_index / web_access / long_context / collection_access / deprecated_until). Server-side filter (Option B per §D.4) intentionally not implemented; deferred to a future task per architect lock if narrow legal/compliance scenarios ever demand it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ilities hurl CI on 9d8e196 flagged 21_d10_capabilities.hurl asserting the literal "deprecated_until" / "fallback_to" field markers. The MCP wire format strips null-valued fields, so those keys do not appear on the wire while every D10 tool keeps them at None — matches the architect's §D.1 spec sample (the keys reappear automatically once a tool sets them). Adjustment is hurl-only and additive to test coverage: - Drop the always-failing markers; add a one-line note pointing at test_capability_negotiation.py for the schema-default behavior. - Replace the bare "deprecated_until:null" probe with "deprecated":false so we still confirm the discriminator surfaces under its current default. No production code change. Unit suite still asserts the full §D.1 envelope (default None values, frozen, extra=forbid) so the contract remains locked at the model level. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
earayu
pushed a commit
that referenced
this pull request
Apr 26, 2026
CI e2e-http-provider failed at ``22_d10_read_primitives.hurl:52`` with ``the HTTP method <body> is not valid``. The four new hurl files I added in block E left the body assertions hanging directly under ``HTTP 200`` without an ``[Asserts]`` block, so the hurl parser tried to read each ``body contains "..."`` line as the start of a new request (looking for an HTTP method like GET/POST/etc). The existing ``21_d10_capabilities.hurl`` (which my files were modeled on) does have ``[Asserts]`` after its final ``HTTP 200`` — I missed that line when copying the pattern. Add it to all four: - 22_d10_read_primitives.hurl - 23_d10_pagination.hurl - 24_d10_search_split.hurl - 25_d10_cutover.hurl Same hurl-only fix pattern as huangheng's #1719 follow-up (`379d2535`): no production code touched, only the hurl assertion framing.
earayu
pushed a commit
that referenced
this pull request
Apr 26, 2026
…in 23_d10_pagination.hurl
CI run on head ``7d64991`` failed at
``23_d10_pagination.hurl:67`` and ``:68`` — same false-positive
pattern Weston already flagged for 24_d10_search_split.hurl
(msg=8a691444). The MCP tool functions in ``aperag/mcp/server.py``
are typed ``-> Dict[str, Any]``, so FastMCP exposes only
``"outputSchema": {"additionalProperties": true}`` on the
``tools/list`` wire — the ``items / next_cursor / total_count``
PaginationResult envelope field names never reach the body, and
``body contains "\\"next_cursor\\""`` / ``"\\"total_count\\""``
genuinely fail.
The PaginationParams input names (``cursor / limit``) DO surface
on the wire because they are inputSchema parameters; those four
substrings stay. Replaced the misleading "FastMCP emits the
Pydantic model JSON schema" comment with a header note pointing at
the cursor unit suite, which is where the envelope-shape pin
already lives (``tests/unit_test/mcp/test_cursor_contract.py`` +
``tests/unit_test/service/test_pagination_helper.py``).
Same hurl-only fix pattern as the previous ``[Asserts]`` push and
huangheng's #1719 ``379d2535`` follow-up — no production code
touched.
earayu
added a commit
that referenced
this pull request
Apr 26, 2026
…lization + retrieval handle exposure + D10 hurl coverage (#1721) * feat(phase9 #100 D10.h): cutover blocks A/B/D/F-partial — search legacy delete + web_search canonicalize + cursor narrative Per design pack §G D10.h + amendment-#2 + PM scope lock (msg=dc63c7e6): Block A — search_collection / search_chat_files hard-cut + caller sweep: - aperag/mcp/server.py: delete `search_collection` (line 295-465 ~170 LOC) + `search_chat_files` (468-583 ~116 LOC) bodies; rewrite the `aperag_usage_guide` resource docstring to document the canonical D10 split tools (vector_search / graph_search / fulltext_search / web_search) instead of the deprecated omnibus surface; replace the "deferred to D10.h cutover" NOTE comment with a closure-state note pointing at the split tools. - aperag/domains/agent_runtime/services.py: update `_KNOWLEDGE_SEARCH_TOOLS` from `{"list_collections", "search_collection"}` to the split-tool set `{list_collections, list_documents, vector_search, graph_search, fulltext_search}`; expand `_READING_TOOLS` to cover the 6 D10.c read primitives so user-activity routing tracks the post-cutover surface end-to-end. Block B — web_search §B.4 canonicalization: - aperag/mcp/tools/search_web.py: `query` is now positional + required (raises ValueError on empty); every other parameter is keyword-only; the result-count limit is named `top_k` (not `max_results`); `source` is `str | None` so a missing domain filter is null. The wrapper still passes `max_results` to the internal `/api/v2/web/search` payload — backend rename is intentionally out of this lane (PM constraint #2 msg=309b3ed3 "目标是 cutover contract 收口，不是重写 hurl 框架"). - tests/unit_test/mcp/test_search_split.py: replace the deferred-shape signature pin with `test_web_search_signature_matches_b4_canonical` — asserts query is required, the 4 other params are kw-only with the canonical names + defaults, and the legacy `max_results` parameter is gone. Block D — stale narrative cleanup: - aperag/mcp/cursor/invariants.py: drop the "(pending architect canonical lock per msg=441c5e56)" parenthetical now that #1710 has merged the canonical lock to main. Block C (retrieval `chunk_id` / `section_path` / `heading_anchor` propagation per Drift #1) is left for the architect to author on this same branch per msg=b9e7f91e cross-lane offer; block E (D10 hurl coverage suite) follows after C lands. #80 territory deliberately untouched (snapshot_assembler / RedisChatMessageHistory / StoredChatMessagePart / chat/history / utils/history) per PM msg=309b3ed3 #100/#80 disjoint lock. * feat(phase9 #100 D10.h): block E — D10 hurl coverage suite 4 new hurl files exercise the post-cutover MCP wire surface end-to- end; mirror 21_d10_capabilities's tools/list + substring-contains pattern (the wire is JSON-RPC framed inside SSE, so jsonpath would skip the envelope): - 22_d10_read_primitives.hurl — pin the §A.1-§A.8 input schema names for every paginated and metadata read primitive plus the ``read_document`` byte-range parameters and the ``read_document_chunk`` stable handle. - 23_d10_pagination.hurl — pin §C.5 PaginationParams (cursor + limit) inputs and PaginationResult (items + next_cursor + total_count) outputs on every paginated read primitive. - 24_d10_search_split.hurl — pin the §B.1-§B.4 split-search surface and the §B.4 canonical parameter set (top_k present, max_results intentionally absent). Includes assertions on chunk_id/section_path/heading_anchor that exercise the retrieval- propagation block authored by the architect on this same branch — these will fail until that commit lands, which is the intended cross-block gate. - 25_d10_cutover.hurl — cutover gate: every canonical D10 tool name is still on tools/list, the legacy search_collection / search_chat_files omnibus pair is gone. A future regression that re-registers either legacy tool fails CI loudly before merge. Per PM constraint #2 (msg=309b3ed3) the hurl coverage matches this hard-cut one-for-one — no infra refactor, no new transport, just substring-contains over the existing ``/mcp/`` mount. * feat(phase9 #100 D10.h Block C): expose chunk_id / section_path / heading_anchor on SearchResultMetadata Per amendment-#2 Drift #1 (msg=ebfcdabe) + PM final scope lock (msg=dc63c7e6 / msg=5760999e) — D10.h cutover Block C: surface the 3 LOCKED §A.9 stable handle fields on the retrieval domain public allowlist so external Agents (Claude Code / Codex / Cursor) can navigate from search hits back to the canonical D10.c read primitives (``read_document_chunk(chunk_id)`` / ``read_document_section(section_path)``). ## Changes - ``aperag/domains/retrieval/schemas.py``: - ``SearchResultMetadata`` allowlist adds 3 fields (``chunk_id``, ``section_path``, ``heading_anchor``). The model keeps ``extra="forbid"`` — adding the fields does not relax the allowlist. - ``SearchResultMetadata.from_raw()`` extends extraction so the 3 fields propagate from upstream backends that already include them (e.g. ``aperag/domains/indexing/fulltext_index.py:541-553`` already surfaces ``chunk_id``). - ``tests/unit_test/domains/retrieval/test_search_result_metadata.py`` (NEW) — 7 tests pin the contract: 1. Each of the 3 fields is constructible. 2. Unknown keys still rejected (``extra="forbid"`` regression). 3. ``from_raw()`` extracts all 3 when present. 4. Missing fields surface as ``None`` (upstream propagation gap is not a schema break). 5. Non-string / empty values filtered so the public surface never carries a numeric chunk_id or empty string. ## Out of scope (per PM msg=5760999e #4 constraint) Indexing-layer attachment of ``section_path`` / ``heading_anchor`` to chunk metadata at index time is NOT included here. Per the constraint, multi-indexer expansion (vector / fulltext / graph / summary / vision each writing section context) would balloon the write-set; we keep D10.h Block C as a 1-2-touch retrieval-domain surface change. The 3 fields surface as ``None`` until the indexing-layer enhancement lands in a follow-up. ``chunk_id`` populates immediately via the existing fulltext index ``_source.chunk_id`` propagation (``fulltext_index.py:541-553``). ## §G hard gate compliance - #1 (3-root grep): no caller assertion drift on ``SearchResultMetadata`` shape outside the allowlist additions (allowlist additions are additive). - #5 (cross-stack): only ``aperag/domains/retrieval/schemas.py`` + ``tests/unit_test/domains/retrieval/`` touched on the architect side; Bryce's Block A/B/D commits cover the rest of D10.h scope. Block C complement to architect+Bryce co-own #100 (msg=a17a4017 execution split: architect commits Block C on shared branch). * test(phase9 #100 D10.h): caller migration assertion semantics for the cutover Per §G hard gate #4 the cutover lane is the right place to update the test surface that previously pinned the legacy ``search_collection`` behaviour, so the assertions match the post-cutover reality: - tests/unit_test/mcp/test_search_split.py: drop the two ``[DEPRECATED]`` banner tests and the two body-still-targets-v2 tests; replace them with two cutover-removal tests (``test_search_collection_legacy_omnibus_removed_from_module`` / ``test_search_chat_files_legacy_omnibus_removed_from_module``) that pin both the runtime attribute absence and the absence of the ``async def`` in source. Inline the ``_async_def_source`` ast walk into the one remaining caller (``test_web_search_module_targets_v2_web_path``). - tests/unit_test/test_mcp_server.py: rename ``test_search_collection_docstring_explains_step_and_failure_meaning`` to ``test_search_collection_legacy_omnibus_no_longer_registered`` — the user-visible step language for the new split tools is covered in ``test_search_split.py`` so this file just pins the absence. - tests/unit_test/test_mcp_contract.py: rewrite the ``search_collection`` URL-and-import invariants as ``test_legacy_search_collection_omnibus_stays_removed`` + ``test_search_result_legacy_import_stays_gone``. The original tests guarded the Phase 2 retrieval hard-cut (URL must be v2, SearchResult must come from the retrieval domain); the post-cutover invariant is that neither symbol re-enters the server module at all. - tests/unit_test/agent_runtime/test_agent_runtime_v3.py: ``test_event_service_to_event_envelope_adds_user_activity_contract`` switches its sample tool name from the removed ``search_collection`` to ``vector_search`` so the user-activity inference contract is exercised against the canonical ``_KNOWLEDGE_SEARCH_TOOLS`` set updated in ``aperag/domains/agent_runtime/services.py``. 1001/1001 unit tests pass; ruff check + format clean. * fix(phase9 #100 D10.h): Weston review — fixture migration + false-positive hurl gate + stale split-tool docstrings Per Weston msg=8a691444: 1. tests/fixtures/mcp_agent.py The ``searcher`` agent's instructions still pointed at the omnibus ``search_collection()`` call — a real instruction surface, not just prose. After the cutover that tool is gone from MCP ``tools/list`` so the fixture would teach the agent to call a non-existent tool. Migrate the instruction to the canonical D10 split-search flow: ``list_collections`` first, then compose ``vector_search`` / ``fulltext_search`` / ``graph_search`` per the question, and chain into ``read_document_chunk`` / ``read_document_section`` via the ``chunk_id`` / ``section_path`` handles on each ``SearchResultItem``. 2. tests/e2e_http/hurl/full/24_d10_search_split.hurl The three ``contains "\"chunk_id\""`` / ``"\"section_path\""`` / ``"\"heading_anchor\""`` assertions claimed to gate the ``SearchResultItem.metadata`` outputSchema, but the split-search tools return ``Dict[str, Any]`` whose FastMCP outputSchema is ``additionalProperties: true`` — those substrings would actually be matched by unrelated read-primitive input schemas (e.g., ``read_document_chunk(chunk_id=...)``), making the hurl a false-positive gate. Drop the three assertions and add a header comment pointing at the proper Pydantic-level pin in ``tests/unit_test/domains/retrieval/test_search_result_metadata.py`` (which the architect's block C already authored). 3. aperag/mcp/tools/search_{vector,graph,fulltext}.py module docstrings Each one still narrated the omnibus deprecation timeline ("the alias remains until D10.h cutover") even though D10.h is the lane that just deleted it. Rewrite the three module docstrings to describe the post-cutover state: each split tool is the sole public entry point for its recall mode, and the retrieval ``SearchResultMetadata`` allowlist now exposes the three §A.9 stable handle fields. Non-blocking per Weston, but cleaner to land while we are touching this lane. 1001 unit tests pass; ruff check + format clean. * fix(phase9 #100 D10.h): add missing [Asserts] block to D10 hurl files CI e2e-http-provider failed at ``22_d10_read_primitives.hurl:52`` with ``the HTTP method <body> is not valid``. The four new hurl files I added in block E left the body assertions hanging directly under ``HTTP 200`` without an ``[Asserts]`` block, so the hurl parser tried to read each ``body contains "..."`` line as the start of a new request (looking for an HTTP method like GET/POST/etc). The existing ``21_d10_capabilities.hurl`` (which my files were modeled on) does have ``[Asserts]`` after its final ``HTTP 200`` — I missed that line when copying the pattern. Add it to all four: - 22_d10_read_primitives.hurl - 23_d10_pagination.hurl - 24_d10_search_split.hurl - 25_d10_cutover.hurl Same hurl-only fix pattern as huangheng's #1719 follow-up (`379d2535`): no production code touched, only the hurl assertion framing. * fix(phase9 #100 D10.h): drop false-positive output-schema assertions in 23_d10_pagination.hurl CI run on head ``7d64991`` failed at ``23_d10_pagination.hurl:67`` and ``:68`` — same false-positive pattern Weston already flagged for 24_d10_search_split.hurl (msg=8a691444). The MCP tool functions in ``aperag/mcp/server.py`` are typed ``-> Dict[str, Any]``, so FastMCP exposes only ``"outputSchema": {"additionalProperties": true}`` on the ``tools/list`` wire — the ``items / next_cursor / total_count`` PaginationResult envelope field names never reach the body, and ``body contains "\\"next_cursor\\""`` / ``"\\"total_count\\""`` genuinely fail. The PaginationParams input names (``cursor / limit``) DO surface on the wire because they are inputSchema parameters; those four substrings stay. Replaced the misleading "FastMCP emits the Pydantic model JSON schema" comment with a header note pointing at the cursor unit suite, which is where the envelope-shape pin already lives (``tests/unit_test/mcp/test_cursor_contract.py`` + ``tests/unit_test/service/test_pagination_helper.py``). Same hurl-only fix pattern as the previous ``[Asserts]`` push and huangheng's #1719 ``379d2535`` follow-up — no production code touched. --------- Co-authored-by: Bryce <bryce@aperag.local> Co-authored-by: 符炫炜 <fuxuanwei@apecloud.io>
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
Per docs/modularization/d10-design-pack.md §D every D10 tool now carries a frozen
ToolAnnotationenvelope (requires/capabilities/deprecated/deprecated_until/fallback_to) onto the MCP wire so external Agents (Claude Code / Codex / Cursor / ApeRAG own-Agent) can do client-side filtering without server-side session state.§G D10.f write-set delivered:
aperag/mcp/capabilities.py—ToolAnnotationPydantic model (frozen=True, extra="forbid") + closedKNOWN_CAPABILITIES/KNOWN_REQUIRESsets.mode="before"validators reject non-bool capability values, unknown keys, malformeddeprecated_until.to_mcp_dict()renders the FastMCPannotations=kwarg payload.aperag/mcp/tools/_annotations.py— name → annotation registry.register()returns the FastMCP dict so call sites stay one-liners, with idempotent re-register on identical annotation andValueErroron conflict.get_all()returns aMappingProxyTypeimmutable view for downstream consumers.aperag/mcp/server.py+aperag/mcp/tools/search_*.py— additive decorator wrap on all 8 D10.c read primitives + 4 D10.d search primitives. No body / signature change in any of those files.read_documentis the only primitive markedlong_context: True(full markdown body); chunk / section / outline are explicitlyFalse.graph_searchcarriesgraph_index: True,fulltext_searchcarriesfulltext_index: True,web_searchcarriesweb_access: Trueandrequires=("web_access",).aperag/sdk/capability_filter.py— Option A client-side filter helper.FilterDecisionis the explicit-not-silent reason payload (§D.3) — every excluded tool reportsreason="capability_required"plus the sortedmissingcapability tuple, deprecated tools surfacereason="deprecated".usable_tool_names()andrequired_capabilities()are convenience wrappers over the same decision pipeline.tests/unit_test/mcp/test_capability_negotiation.py— 37 tests covering §D.1 schema validity, §D.5 registry contract, register() semantics (idempotent / conflict / type-checks), §D.2 client-side filter pseudocode, §D.3 explicit-not-silent decisions.tests/e2e_http/hurl/full/21_d10_capabilities.hurl— wire-shape contract: posts JSON-RPCtools/listagainst the/mcp/mount and asserts every D10 tool name + every §D.1 annotation field marker on the wire.Verification
uv run --extra test python -m pytest tests/unit_test/mcp/ tests/unit_test/test_d10c_read_primitives_surface.py tests/unit_test/cache/→ 146 passed (post-rebase on68c24e19D10.d follow-up).uvx ruff check+uvx ruff format --check→ all green on the D10.f write-set.§G hard gates self-audit
Out of scope (per §G D10.f boundary lock)
Test plan
Reviewer per RR2 single-Opus-CR
🤖 Generated with Claude Code