docs(modularization): D10 spec amendment — back-align §G D10.e cursor error codes to §C.3 canonical#1710
Merged
Conversation
… error codes to §C.3 canonical Per [D10 spec amendment] thread (Bryce msg=441c5e56 + PM msg=40e98684 + architect 双签): §G D10.e Deliverable summary (line 1115) had 6 SCREAMING_SNAKE codes that did not match §C.3 body's 6 snake_case codes. The §G summary was a drafting slip from the rushed §G decomposition amendment commit (36b5835); §C.3 body remains the canonical source because: 1. Casing — wire format is snake_case to match the rest of the ApeRAG API surface (existing error codes use snake_case in JSON wire format). 2. Granularity — §C.3 body splits invariant violation into 3 distinct codes (cursor_filter_mismatch / cursor_tenant_mismatch / cursor_index_changed) because line 567-571 maps DIFFERENT client recovery paths to each: - cursor_filter_mismatch → client bug, surface to user - cursor_tenant_mismatch → security violation, distinct telemetry - cursor_index_changed → backend ops issue, retry from null Collapsing them into a single cursor_invariant_mismatch would lose this distinction and force clients to over-react. 3. CURSOR_FOREIGN and CURSOR_PAGE_OUT_OF_RANGE in §G summary did not appear in §C.3 body and had no client-recovery path defined — they were drafting noise, not real codes. §G D10.e summary now cites §C.3 verbatim and points readers at the §C.3 body for the client-recovery mapping (single source of truth). Doc-only change. No implementation impact. Unblocks task #97 (D10.e cursor errors.py).
4 tasks
earayu
pushed a commit
that referenced
this pull request
Apr 25, 2026
…_cursor Per Weston msg=cc4a3ab0 二线 CR blocker: decode_cursor() previously surfaced malformed / wrong-schema / expired wire payloads as bare ValueError / KeyError, leaving every D10.c / D10.d caller to re-derive the canonical mapping. That violates the §C.3 explicit-not-silent invariant by construction — any forgotten mapping silently degrades into ValueError → tool error → first-page restart, which is exactly the anti-pattern SILENT_RESET_FORBIDDEN guards against. Fix: - decode_cursor() now raises CursorError directly with the right canonical code: cursor_invalid (malformed wire / base64 / json / missing field), cursor_schema_unsupported (unknown schema_version), cursor_expired (past issued_at + ttl_seconds clock). - _decode_cursor_payload() preserved as a private structural-only decode for tests that need to craft expired / wrong-schema payloads to exercise the canonical error paths. - 3 new canonical-code tests + 1 internal-decode escape hatch test added; old raw-error test deleted (pre-#1710 wire shape no longer reachable through public surface). _payload() fixture's issued_at now defaults to current time so round-trip tests stay green when run far from the fixture's drafting date; tests that need expired / fixed payloads override explicitly. 21/21 tests pass; ruff check + format clean.
earayu
added a commit
that referenced
this pull request
Apr 25, 2026
…1712) * feat(phase9 #97 D10.e prep): cursor pagination contract + 18 unit tests Per design pack §C (canonical post-#1710 SSoT): - aperag/mcp/cursor/codec.py: CursorPayload (sort_key + last_position + invariant_hash + issued_at + ttl_seconds 1h default + server_id + schema_version 1) + base64url JSON encode/decode + is_expired TTL check - aperag/mcp/cursor/invariants.py: compute_invariant_hash sha256 over (sort_key + filters + collection_id + tenant_id + index_id) deterministic across dict ordering - aperag/mcp/cursor/schemas.py: PaginationParams (cursor + limit conint 1..200) + PaginationResult[T] generic (items + next_cursor + total_count) - aperag/mcp/cursor/errors.py: 6 canonical snake_case codes (cursor_invalid / cursor_expired / cursor_filter_mismatch / cursor_tenant_mismatch / cursor_index_changed / cursor_schema_unsupported per §C.3 + #1710 amendment) + CursorError exception + CursorErrorEnvelope wire shape + SILENT_RESET_FORBIDDEN guard - aperag/mcp/cursor/__init__.py: public surface for D10.c read primitive imports tests/unit_test/mcp/test_cursor_contract.py: - 5 codec round-trip / wire format / TTL boundary tests - 2 invariant_hash determinism + binding sensitivity tests - 7 error envelope round-trip tests (parametrized over each canonical code) + SILENT_RESET_FORBIDDEN pin - 4 PaginationParams/PaginationResult shape tests including end-to-end cursor flow Pending D10.c stub head landing for `aperag/service/pagination.py` integration helper + `tests/e2e_http/hurl/<NN>_d10_pagination.hurl` cross-tool e2e — those are Window 1 work. * fix(phase9 #97 D10.e): enforce §C.3 explicit error contract in decode_cursor Per Weston msg=cc4a3ab0 二线 CR blocker: decode_cursor() previously surfaced malformed / wrong-schema / expired wire payloads as bare ValueError / KeyError, leaving every D10.c / D10.d caller to re-derive the canonical mapping. That violates the §C.3 explicit-not-silent invariant by construction — any forgotten mapping silently degrades into ValueError → tool error → first-page restart, which is exactly the anti-pattern SILENT_RESET_FORBIDDEN guards against. Fix: - decode_cursor() now raises CursorError directly with the right canonical code: cursor_invalid (malformed wire / base64 / json / missing field), cursor_schema_unsupported (unknown schema_version), cursor_expired (past issued_at + ttl_seconds clock). - _decode_cursor_payload() preserved as a private structural-only decode for tests that need to craft expired / wrong-schema payloads to exercise the canonical error paths. - 3 new canonical-code tests + 1 internal-decode escape hatch test added; old raw-error test deleted (pre-#1710 wire shape no longer reachable through public surface). _payload() fixture's issued_at now defaults to current time so round-trip tests stay green when run far from the fixture's drafting date; tests that need expired / fixed payloads override explicitly. 21/21 tests pass; ruff check + format clean. --------- Co-authored-by: Bryce <bryce@aperag.local>
Merged
3 tasks
earayu
added a commit
that referenced
this pull request
Apr 25, 2026
…0.c list primitives (#1715) Closes the D10.e write-set per design pack §G: - NEW aperag/service/pagination.py — `encode_offset_cursor` / `decode_offset_cursor` helper that wraps the canonical `aperag.mcp.cursor` codec around the offset bookkeeping the D10.c list primitives already perform; binds invariants over (sort_key, filters, collection_id, tenant_id) so any scope drift between cursor issuance and re-use surfaces as canonical CursorError. - MOD aperag/mcp/tools/list_collections.py + list_documents.py — drop the `_decode_cursor` / `_encode_cursor` placeholders #1714 left as a D10.e seam; route every cursor through the helper. Malformed / expired / scope-mismatched cursors now raise canonical CursorError ("cursor_invalid" / "cursor_expired" / "cursor_filter_mismatch" / "cursor_schema_unsupported") instead of bare ValueError. Tests: - NEW tests/unit_test/service/test_pagination_helper.py — 16 tests pin every canonical-error path: None/empty start-fresh, round-trip, garbage / non-json wire (cursor_invalid), sort_key drift / filters drift / collection_id drift (cursor_filter_mismatch), TTL expiry (cursor_expired), unknown schema_version (cursor_schema_unsupported), malformed last_position offset (string / negative / bool). - MOD tests/unit_test/test_d10c_read_primitives_surface.py — drop the 8 `_decode_cursor` tests that now belong to the helper module; left a comment redirecting readers to the new test file. The type_filter pre-pagination regression tests (Weston msg=246c84d3) stay where they are. Stale-narrative cleanup (architect msg=343f2e32 follow-up): - aperag/mcp/cursor/__init__.py + codec.py — drop the "pending spec amendment double-sign per architect msg=669db73c" docstring fragments now that #1710 has merged the canonical lock to main. `tests/e2e_http/hurl/<NN>_d10_pagination.hurl` listed in §G is intentionally deferred: there is no MCP-over-HTTP coverage in the hurl suite today (D10.c #1714 followed the same scope), and the helper's behaviour is fully exercised by the 16 unit tests above. A dedicated MCP-over-HTTP e2e infrastructure pass is the right time to add it, not this lane. 65/65 tests pass (21 cursor + 16 helper + 28 D10.c surface); ruff check + format clean. Co-authored-by: Bryce <bryce@aperag.local>
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
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
[D10 spec amendment]doc-only fix. Back-aligns §G D10.e Deliverable summary cursor error codes to the §C.3 canonical 6-code list.Triggered by Bryce msg=441c5e56 in the parent channel — D10.e prep inventory found internal inconsistency between §C.3 body and §G summary. PM msg=40e98684 escalated to formal blocker; architect 双签 canonical decision is §C.3 body wins.
Canonical decision (3 hard answers per PM)
snake_case(matches rest of ApeRAG API; client-recovery mapping in §C.3 already uses snake_case names).cursor_invalid/cursor_expired/cursor_filter_mismatch/cursor_tenant_mismatch/cursor_index_changed/cursor_schema_unsupported.Why §C.3 wins (not §G)
§C.3 body precedes §G summary in drafting order, has full client-recovery path mapping at line 567-571, and is the detailed contract surface. §G summary was drafting noise from the rushed §G decomposition amendment commit
36b58350.In particular, §G's
CURSOR_INVARIANT_MISMATCHcollapsed three §C.3 codes (cursor_filter_mismatch/cursor_tenant_mismatch/cursor_index_changed) into one — but §C.3 line 567-571 explicitly maps each to different client recovery paths:cursor_filter_mismatch→ client bug, surface to usercursor_tenant_mismatch→ security violation, distinct telemetrycursor_index_changed→ backend ops issue, retry from nullCollapsing them would force clients to over-react and lose recovery precision. §G's
CURSOR_FOREIGN/CURSOR_PAGE_OUT_OF_RANGEhad no §C.3 backing and no client-recovery path — drafting noise, removed.Unblocks
aperag/mcp/cursor/errors.pywith the canonical 6 snake_case codes.Diff
1 file changed, 1 insertion(+), 1 deletion(-) — single line at
docs/modularization/d10-design-pack.md§G D10.e Deliverable bullet (line 1115).Test plan
🤖 Generated with Claude Code — 符炫炜 (chief architect agent, dual-sign with @bryce per §G amendment protocol)