diff --git a/apps/api/tests/contract/test_evidence_renderer_contract.py b/apps/api/tests/contract/test_evidence_renderer_contract.py index 8188408b..a714ff54 100644 --- a/apps/api/tests/contract/test_evidence_renderer_contract.py +++ b/apps/api/tests/contract/test_evidence_renderer_contract.py @@ -1,7 +1,7 @@ from shared.services.retrieval.execution.routes import _render_rows_evidence -def test_render_rows_evidence_should_group_by_traceable_path() -> None: +def test_render_rows_evidence_should_group_siblings_under_parent_path() -> None: rows = [ { "chunk_id": "c2", @@ -31,12 +31,13 @@ def test_render_rows_evidence_should_group_by_traceable_path() -> None: evidence_text = _render_rows_evidence(rows) - assert "[E1]" in evidence_text - assert "[E2]" in evidence_text - assert "[E3]" in evidence_text - assert "[§ alpha.pdf / Alpha / One]" in evidence_text - assert "[§ alpha.pdf / Alpha / Two]" in evidence_text - assert "[§ beta.pdf / Beta / Table]" in evidence_text + assert evidence_text.count("[E1]") == 1 + assert evidence_text.count("[E2]") == 1 + assert "[E3]" not in evidence_text + assert "[§ alpha.pdf / Alpha]" in evidence_text + assert "[§ beta.pdf / Beta]" in evidence_text + assert "[§ alpha.pdf / Alpha / One]" not in evidence_text + assert "[§ alpha.pdf / Alpha / Two]" not in evidence_text assert "first section content" in evidence_text assert "second section content" in evidence_text assert "
metric
" in evidence_text diff --git a/deploy/ecs/README.md b/deploy/ecs/README.md index 37a8c572..4a2427b5 100644 --- a/deploy/ecs/README.md +++ b/deploy/ecs/README.md @@ -6,7 +6,7 @@ The templates intentionally omit `S3_ACCESS_KEY_ID` and `S3_SECRET_ACCESS_KEY`. The environment-specific Secrets Manager secret supplied to the renderer must be a JSON secret with these keys: -- API: `DATABASE_URL`, `REDIS_HOST`, `REDIS_PORT`, `REDIS_PASSWORD`, `CELERY_REDIS_URL`, `SECRET_KEY`, `DS_KEY`, `ALI_API_KEYS`, `ARK_API_KEY`, `GPT_API_KEY`, `MINERU_API_KEYS`, `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, `WEBHOOK_MASTER_KEY`, `LOGFIRE_TOKEN`, `QSTASH_TOKEN`, `QSTASH_CURRENT_SIGNING_KEY`, `QSTASH_NEXT_SIGNING_KEY` +- API: `DATABASE_URL`, `REDIS_HOST`, `REDIS_PORT`, `REDIS_PASSWORD`, `CELERY_REDIS_URL`, `SECRET_KEY`, `DS_KEY`, `ALI_API_KEYS`, `ARK_API_KEY`, `GPT_API_KEY`, `CURSOR_API_KEY`, `MINERU_API_KEYS`, `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, `WEBHOOK_MASTER_KEY`, `LOGFIRE_TOKEN`, `QSTASH_TOKEN`, `QSTASH_CURRENT_SIGNING_KEY`, `QSTASH_NEXT_SIGNING_KEY` - Worker: the API keys above plus `CELERY_REDIS_PASSWORD` and `ILOVEAPI_KEYS` Render only after the secret and log groups exist, substituting the exact immutable ECR image digests and IAM role/secret ARNs: diff --git a/packages/shared-python/shared/services/retrieval/agent_explore/harness/cursor_harness.py b/packages/shared-python/shared/services/retrieval/agent_explore/harness/cursor_harness.py index 6678be43..ebade0ea 100644 --- a/packages/shared-python/shared/services/retrieval/agent_explore/harness/cursor_harness.py +++ b/packages/shared-python/shared/services/retrieval/agent_explore/harness/cursor_harness.py @@ -66,6 +66,7 @@ import contextlib import json import os +import threading import time from typing import Any @@ -147,6 +148,15 @@ async def run_episode( tool_budget = ToolBudget() loop = asyncio.get_running_loop() + # The Cursor SDK delivers same-turn parallel tool calls as concurrent + # HTTP requests on separate threads (ThreadingHTTPServer in + # cursor_sdk._tool_callback) — CORPUS_SCHEMA.md's loop contract + # explicitly invites the model to call several independent tools in + # one turn. Without this lock, the check-then-increment on + # budget.steps_used below is a race: several concurrently-dispatched + # calls can each read "not exhausted yet" before any of them + # increments, letting more calls through than max_steps allows. + budget_lock = threading.Lock() steps: list[AgentStep] = [] trajectory_refs: list[dict[str, Any]] = [] @@ -158,21 +168,22 @@ async def run_episode( stop_reason = "finished" def _dispatch_sync(tool_name: str, args: dict[str, Any]) -> str: - if budget.steps_used >= budget.max_steps: - steps.append( - AgentStep( - step_index=len(steps), - tool_name=tool_name, - tool_args=args, - observation_text=_BUDGET_EXHAUSTED_MESSAGE, - error="budget_max_steps", - elapsed_ms=0, - tokens_used_delta=0, - tokens_used_total=budget.tokens_used, + with budget_lock: + if budget.steps_used >= budget.max_steps: + steps.append( + AgentStep( + step_index=len(steps), + tool_name=tool_name, + tool_args=args, + observation_text=_BUDGET_EXHAUSTED_MESSAGE, + error="budget_max_steps", + elapsed_ms=0, + tokens_used_delta=0, + tokens_used_total=budget.tokens_used, + ) ) - ) - return _BUDGET_EXHAUSTED_MESSAGE - budget.record_step() + return _BUDGET_EXHAUSTED_MESSAGE + budget.record_step() tool_started = time.perf_counter() future = asyncio.run_coroutine_threadsafe( dispatch_tool_call( diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/CORPUS_SCHEMA.md b/packages/shared-python/shared/services/retrieval/agent_tools/CORPUS_SCHEMA.md index ec65d189..ca11365e 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/CORPUS_SCHEMA.md +++ b/packages/shared-python/shared/services/retrieval/agent_tools/CORPUS_SCHEMA.md @@ -26,7 +26,10 @@ Namespace - **Namespace**: the retrieval scope. One namespace can hold documents parsed by different tracks (see §2) — do not assume a namespace is uniform. - **Document**: `document_id` is the stable identifier for every other tool - call. `parse_track` is `page_memory` or `chunk` (see §2). + call. `parse_track` is `page_memory` or `chunk` (see §2). Tool hit lines + print `document_id` in parentheses after the filename (or alone, for + `node_filter`); copy that value into later calls. Never pass + `source_file_name` as `document_id`. - **Section**: the navigable tree. `section_path` segments are joined with `" / "` (space-slash-space), one segment per heading/synthetic level. - **Chunk**: the retrieval unit. `chunk_type` is `text`, `page`, `image`, or @@ -134,8 +137,8 @@ for anything finer-grained than a document pair. | `list_documents` | Starting cold: which documents exist, what are they about | namespace | Returns per-document keywords/summary/type mix/`parse_track`. | | `outline` | The task only needs titles/summaries — overview, "what does chapter N cover," picking where to look before reading | one document, or a `section_path` prefix within it | Titles + summaries + `chunk_count`, no body text, no folding. Depth-limited by argument, not by a token budget. Use this to build your own map instead of relying on a pre-folded one. | | `node_filter` | The task is a traversal/exclusion predicate — FOR ALL / EXISTS / ANY / NOT — over section titles or summaries ("which docs mention X in a heading," "sections NOT about Y") | one or more documents | Deterministic substring/regex match against `section_path` and `summary` only, not body text. Returns the full matching set and count, never a truncated top-K. If the predicate must run against body text, use `grep` instead. | -| `grep` | Exact string / regex / identifier / number lookup that must run against body text | scoped by document/section/chunk_type | Returns match count plus snippets, so ANY/ALL logic can also close over body text, not just titles. | -| `recall` | A fuzzy question where you don't know where the answer lives | namespace or scoped | Ranked candidates from `path_content` (BM25) + `term` (substring) channels fused by RRF; `vector` is reserved — see §5. Returns path and snippet, not full content. | +| `grep` | Exact string / regex / identifier / number lookup that must run against body text | scoped by document/section/chunk_type | Returns match count plus snippets, so ANY/ALL logic can also close over body text, not just titles. Each hit includes `document_id`. | +| `recall` | A fuzzy question where you don't know where the answer lives | namespace or scoped | Ranked candidates from `path_content` (BM25) + `term` (substring) channels fused by RRF; `vector` is reserved — see §5. Returns `document_id`, path and snippet, not full content. | | `read` | You already know which section(s)/chunk(s) to read | one or more sections/chunks | Returns full body content, resolves `SAME-AS` markers into the owner's text, expands `connect_to` assets, and converts `page_assets` into URLs. If your `section_path` omits ancestor segments (e.g. missing a top-level volume like `附件目录 /`), `read` tries a unique suffix match within the document; if several sections match, it returns an ambiguity error listing the full paths — copy the full path from `outline`/`grep`/`refs` when that happens. | | `assets` | You need images/tables directly, or need to find which section(s) host a given asset | one or more documents | Forward (by type/query) and reverse (asset → hosting section) lookup — see §3. | | `neighbors` | You need related documents in the same namespace | one document | Document-level `related` edges only — see §4. | diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/tools/grep.py b/packages/shared-python/shared/services/retrieval/agent_tools/tools/grep.py index af7224c7..34f86504 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/tools/grep.py +++ b/packages/shared-python/shared/services/retrieval/agent_tools/tools/grep.py @@ -157,7 +157,10 @@ async def grep(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: if requested_max_results > max_results: lines.append(f"note: capped to budget.max_items={ctx.budget.max_items}") for r in results: - lines.append(f"- {r['source_file_name']} / {r['section_path']}: {r['snippet']!r}") + lines.append( + f"- {r['source_file_name']} ({r['document_id']}) / {r['section_path']}: " + f"{r['snippet']!r}" + ) return ToolResult( text="\n".join(lines), diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/tools/node_filter.py b/packages/shared-python/shared/services/retrieval/agent_tools/tools/node_filter.py index 6c0269fa..84c918fc 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/tools/node_filter.py +++ b/packages/shared-python/shared/services/retrieval/agent_tools/tools/node_filter.py @@ -180,7 +180,7 @@ async def node_filter(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: header = f"hits={len(matched_sections)}" lines = [header] for entry in matched_sections: - block = [entry["section_path"]] + block = [f"{entry['document_id']} / {entry['section_path']}"] if entry["summary"]: block.append(f" summary: {entry['summary']}") lines.append("\n".join(block)) diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/tools/outline.py b/packages/shared-python/shared/services/retrieval/agent_tools/tools/outline.py index db155d77..2596d468 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/tools/outline.py +++ b/packages/shared-python/shared/services/retrieval/agent_tools/tools/outline.py @@ -142,8 +142,9 @@ async def outline(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: line += f"\n{indent} summary: {node['summary']}" lines.append(line) - text = f"document={document.source_file_name} sections={len(nodes)}\n" + "\n".join( - lines + text = ( + f"document={document.source_file_name} ({document_id}) " + f"sections={len(nodes)}\n" + "\n".join(lines) ) return ToolResult( text=text, diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/tools/read.py b/packages/shared-python/shared/services/retrieval/agent_tools/tools/read.py index f5f99117..9d7072f7 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/tools/read.py +++ b/packages/shared-python/shared/services/retrieval/agent_tools/tools/read.py @@ -132,6 +132,42 @@ async def _resolve_same_as_markers( row["content"] = content +def _normalize_read_refs(args: dict[str, Any]) -> list[Any]: + """Accept the canonical ``refs`` list, or a flat single-document shorthand. + + Deterministic, not model-guessing: observed live tool calls sometimes + hoist ``document_id`` to the top level alongside ``section_path(s)`` / + ``chunk_id(s)`` instead of nesting each pair inside ``refs`` — the exact + shape the ``json_schema`` above documents. Rather than relying on the + model to always match the schema, normalize the known equivalent flat + shape here so a well-formed ``document_id`` isn't discarded over an + outer-structure mismatch. Does not change behavior when ``refs`` is + already a non-empty list. + """ + refs = args.get("refs") + if isinstance(refs, list) and refs: + return refs + + document_id = str(args.get("document_id") or "").strip() + if not document_id: + return [] + + normalized: list[dict[str, Any]] = [] + section_path = args.get("section_path") + if isinstance(section_path, str) and section_path.strip(): + normalized.append({"document_id": document_id, "section_path": section_path.strip()}) + for path in args.get("section_paths") or []: + if isinstance(path, str) and path.strip(): + normalized.append({"document_id": document_id, "section_path": path.strip()}) + chunk_id = args.get("chunk_id") + if isinstance(chunk_id, str) and chunk_id.strip(): + normalized.append({"document_id": document_id, "chunk_id": chunk_id.strip()}) + for cid in args.get("chunk_ids") or []: + if isinstance(cid, str) and cid.strip(): + normalized.append({"document_id": document_id, "chunk_id": cid.strip()}) + return normalized + + @register_tool( name="corpus.read", description=( @@ -174,7 +210,7 @@ async def _resolve_same_as_markers( }, ) async def read(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: - refs = args.get("refs") or [] + refs = _normalize_read_refs(args) if not refs: return ToolResult(text="", error="read requires refs") mode = str(args.get("mode") or "self").strip().lower() @@ -384,8 +420,8 @@ async def read(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: lines.append(f"errors: {'; '.join(errors)}") for row in assembled: lines.append( - f"### {row.get('source_file_name')} / {row.get('section_path')} " - f"[{row.get('chunk_type')}]" + f"### {row.get('source_file_name')} ({row.get('document_id')}) / " + f"{row.get('section_path')} [{row.get('chunk_type')}]" ) lines.append(str(row.get("content") or "")) diff --git a/packages/shared-python/shared/services/retrieval/agent_tools/tools/recall.py b/packages/shared-python/shared/services/retrieval/agent_tools/tools/recall.py index 6e906cd6..ec46ece5 100644 --- a/packages/shared-python/shared/services/retrieval/agent_tools/tools/recall.py +++ b/packages/shared-python/shared/services/retrieval/agent_tools/tools/recall.py @@ -155,7 +155,8 @@ async def _term_channel_rows( "Fuzzy ranked candidate search for a question when you don't know " "where the answer lives. Fuses a path+content BM25 channel with a " "term substring channel via RRF. Returns candidates with path and " - "snippet, not full content — call corpus.read on the winners." + "snippet and document_id, not full content — call corpus.read on " + "the winners using that document_id, not the filename." ), json_schema={ "type": "object", @@ -256,8 +257,8 @@ async def recall(ctx: ToolContext, args: dict[str, Any]) -> ToolResult: for row in fused: snippet = build_snippet(str(row.get("content") or row.get("snippet") or "")) lines.append( - f"- {row.get('source_file_name')} / {row.get('section_path')} " - f"score={row.get('score')}: {snippet!r}" + f"- {row.get('source_file_name')} ({row.get('document_id')}) / " + f"{row.get('section_path')} score={row.get('score')}: {snippet!r}" ) return ToolResult( diff --git a/packages/shared-python/shared/services/retrieval/execution/routes.py b/packages/shared-python/shared/services/retrieval/execution/routes.py index cd56514c..b3a89282 100644 --- a/packages/shared-python/shared/services/retrieval/execution/routes.py +++ b/packages/shared-python/shared/services/retrieval/execution/routes.py @@ -6,18 +6,19 @@ from loguru import logger from sqlalchemy.ext.asyncio import AsyncSession -from shared.services.retrieval.search.map_unit_discovery import map_unit_discovery from shared.services.retrieval.execution.reference_resolver import ( resolve_workflow_references, ) -from shared.services.retrieval.hydration.result_assembly import ( - assemble_retrieval_results, -) -from shared.services.retrieval.hydration.evidence_text import render_evidence_blocks from shared.services.retrieval.execution.route_types import ( RetrievalRouteContext, RetrievalRouteOutcome, ) +from shared.services.retrieval.hydration.evidence_text import render_evidence_blocks +from shared.services.retrieval.hydration.result_assembly import ( + assemble_retrieval_results, +) +from shared.services.retrieval.search.lexical_text import split_section_path +from shared.services.retrieval.search.map_unit_discovery import map_unit_discovery from shared.services.retrieval.search.ranking import rank_retrieval_candidates from shared.services.retrieval.search.scoped_corpus import ( count_scoped_chunks, @@ -38,6 +39,9 @@ def _evidence_path_header(row: dict) -> str: source = row file_name = str(source.get("source_file_name") or "").strip() section_path = str(source.get("section_path") or "").strip() + parts = split_section_path(section_path) + if len(parts) > 1: + section_path = " / ".join(parts[:-1]) if file_name and section_path: return f"{file_name} / {section_path}" return file_name or section_path diff --git a/packages/shared-python/shared/tests/test_agent_tools_read_normalize.py b/packages/shared-python/shared/tests/test_agent_tools_read_normalize.py new file mode 100644 index 00000000..50f759e9 --- /dev/null +++ b/packages/shared-python/shared/tests/test_agent_tools_read_normalize.py @@ -0,0 +1,99 @@ +"""Pure tests for ``corpus.read``'s ``_normalize_read_refs`` argument shim. + +Covers the live-observed malformed shape (flat ``document_id`` + +``section_paths``/``chunk_ids`` instead of nested ``refs``) alongside the +canonical shape, without touching the DB-backed ``read`` tool function +itself. +""" + +from __future__ import annotations + +from shared.services.retrieval.agent_tools.tools.read import _normalize_read_refs + + +def test_normalize_read_refs_passes_through_canonical_refs() -> None: + args = {"refs": [{"document_id": "doc_1", "section_path": "A / B"}]} + assert _normalize_read_refs(args) == args["refs"] + + +def test_normalize_read_refs_ignores_flat_shape_when_refs_present() -> None: + args = { + "refs": [{"document_id": "doc_1", "chunk_id": "chunk_1"}], + "document_id": "doc_2", + "section_paths": ["should not be used"], + } + assert _normalize_read_refs(args) == args["refs"] + + +def test_normalize_read_refs_flat_document_id_with_section_paths_list() -> None: + # The exact malformed shape observed live (debug_agent_explore_episode.py, + # cursor_sdk harness, 2026-09-10): document_id hoisted to the top level + # alongside a plural section_paths array instead of nested refs. + args = { + "document_id": "doc_1cec16fef768", + "section_paths": [ + "基层心血管病综合管理实践指南2020 / 3 危险因素干预", + "基层心血管病综合管理实践指南2020 / 4 疾病干预", + ], + } + assert _normalize_read_refs(args) == [ + { + "document_id": "doc_1cec16fef768", + "section_path": "基层心血管病综合管理实践指南2020 / 3 危险因素干预", + }, + { + "document_id": "doc_1cec16fef768", + "section_path": "基层心血管病综合管理实践指南2020 / 4 疾病干预", + }, + ] + + +def test_normalize_read_refs_flat_document_id_with_singular_section_path() -> None: + args = {"document_id": "doc_1", "section_path": "A / B"} + assert _normalize_read_refs(args) == [{"document_id": "doc_1", "section_path": "A / B"}] + + +def test_normalize_read_refs_flat_document_id_with_chunk_ids_list() -> None: + args = {"document_id": "doc_1", "chunk_ids": ["c1", "c2"]} + assert _normalize_read_refs(args) == [ + {"document_id": "doc_1", "chunk_id": "c1"}, + {"document_id": "doc_1", "chunk_id": "c2"}, + ] + + +def test_normalize_read_refs_flat_document_id_with_singular_chunk_id() -> None: + args = {"document_id": "doc_1", "chunk_id": "c1"} + assert _normalize_read_refs(args) == [{"document_id": "doc_1", "chunk_id": "c1"}] + + +def test_normalize_read_refs_combines_all_flat_variants() -> None: + args = { + "document_id": "doc_1", + "section_path": "A", + "section_paths": ["B"], + "chunk_id": "c1", + "chunk_ids": ["c2"], + } + assert _normalize_read_refs(args) == [ + {"document_id": "doc_1", "section_path": "A"}, + {"document_id": "doc_1", "section_path": "B"}, + {"document_id": "doc_1", "chunk_id": "c1"}, + {"document_id": "doc_1", "chunk_id": "c2"}, + ] + + +def test_normalize_read_refs_no_document_id_returns_empty() -> None: + assert _normalize_read_refs({"section_paths": ["A"]}) == [] + + +def test_normalize_read_refs_empty_args_returns_empty() -> None: + assert _normalize_read_refs({}) == [] + + +def test_normalize_read_refs_blank_strings_are_skipped() -> None: + args = { + "document_id": "doc_1", + "section_path": " ", + "section_paths": ["", " ", "A"], + } + assert _normalize_read_refs(args) == [{"document_id": "doc_1", "section_path": "A"}]