diff --git a/README.md b/README.md index 6d48791..2d469e3 100644 --- a/README.md +++ b/README.md @@ -165,7 +165,8 @@ example-record-file.json"), the model can extend its own context with two server-side tools — on **grounded** (high-relevance) turns only: * **`list_documents`** — lists every indexed document, one - `source/path — title` line each (the same order as the Sources page); + `source: X | path: Y | title: Z` line each (the same order as the + Sources page); * **`read_document(source, path)`** — appends the **full** text of one more indexed document to the context (never truncated). diff --git a/app/rag/agent.py b/app/rag/agent.py index f4d28cd..6d45f84 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -26,7 +26,8 @@ task 04): path (the kill switch). 2. Each tool call the model emits is executed server-side against Postgres only (no LLM, no network): ``list_documents`` returns the - indexed catalog — one ``source/path — title`` line per document, + indexed catalog — one ``source: X | path: Y | title: Z`` line per + document (phase 63: labeled fields — unambiguous for LLM parsing), ``GET /api/docs`` order (uncapped in v1; the UI never shows it, only the model does) — and ``read_document`` returns the document's **full** content (A7-revised contract: never truncated). @@ -85,7 +86,7 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "name": "list_documents", "description": ( "List every document indexed in the knowledge base, one " - "`source/path — title` line each" + "`source: X | path: Y | title: Z` line each" ), "parameters": {"type": "object", "properties": {}, "required": []}, }, @@ -104,15 +105,17 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "source": { "type": "string", "description": ( - "The document's source (a directory basename, " - "e.g. 'Homelab')." + "The document's source, as shown after 'source: ' in the " + "list_documents output (e.g. 'Homelab' from " + "'source: Homelab | path: homelab/aws-route53.md')." ), }, "path": { "type": "string", "description": ( - "The document's path relative to its source " - "directory." + "The document's path, as shown after 'path: ' in the " + "list_documents output (e.g. 'homelab/aws-route53.md' from " + "'source: Homelab | path: homelab/aws-route53.md')." ), }, }, @@ -186,7 +189,8 @@ def _execute_tool( if call.name == "list_documents": rows = list_catalog(db) listing = f"{len(rows)} documents:\n" + "\n".join( - f"{source}/{path} — {title}" for source, path, title in rows + f"source: {source} | path: {path} | title: {title}" + for source, path, title in rows ) holder.tool_calls += 1 return listing diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 1417c39..2fe99c7 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -65,9 +65,10 @@ Implements just enough of the aipi surface: ``call_0``, no arguments), ``finish_reason: "tool_calls"``, no content; * request 2 (a ``tool``-role catalog result in the messages): - parse the FIRST catalog line (``source/path — title`` → split on - ``" — "`` → ``rsplit("/", 1)``) and stream a ``tool_calls`` delta - calling ``read_document`` on it (id ``call_1``); + parse the FIRST catalog line (``source: X | path: Y | title: Z`` + — the labeled ``source:`` / ``path:`` fields, phase 63) and + stream a ``tool_calls`` delta calling ``read_document`` on it + (id ``call_1``); * request 3 (a ``tool``-role read result in the messages): a content answer, deterministic: ``Read . `` — so a suite can assert @@ -269,6 +270,14 @@ TABLE_ANSWER = ( #: ``_execute_tool``): ``"Document :\n"``. _READ_RESULT_PREFIX = "Document " +#: One line of the agent's ``list_documents`` output (app.rag.agent +#: ``_execute_tool``, phase 63): labeled, pipe-delimited fields — +#: ``source: X | path: Y | title: Z`` — unambiguous for LLM parsing even +#: when the path contains ``/`` characters. +_CATALOG_LINE_RE = re.compile( + r"^source: (?P.+?) \| path: (?P.+?) \| title: .+$" +) + def _read_results(body: dict[str, Any]) -> list[tuple[str, str]]: """The read results in the messages, in order: ``(source/path, content)``. @@ -291,15 +300,15 @@ def _read_results(body: dict[str, Any]) -> list[tuple[str, str]]: def _catalog_docs(body: dict[str, Any]) -> list[tuple[str, str]]: - """Every ``source/path`` in the catalog tool result, in listing order. + """Every ``(source, path)`` in the catalog tool result, in listing order. - Catalog lines are ``source/path — title`` (the agent's - ``list_documents`` output): split on ``" — "``, keep the head, and - recover ``(source, path)`` with ``rsplit("/", 1)`` (``rpartition``) - — the same convention the single-read flow's read step uses. The - ``"N documents:"`` header line carries no ``/`` and is skipped; read- - result messages are full documents, not listings, and are skipped - too. + Catalog lines are ``source: X | path: Y | title: Z`` (the agent's + ``list_documents`` output — phase 63: labeled, pipe-delimited + fields, unambiguous even for paths full of ``/``): the line-level + regex recovers the ``source`` and ``path`` fields directly. The + ``"N documents:"`` header line matches no line and is skipped; + read-result messages are full documents, not listings, and are + skipped too. """ docs: list[tuple[str, str]] = [] for m in _messages(body): @@ -309,11 +318,9 @@ def _catalog_docs(body: dict[str, Any]) -> list[tuple[str, str]]: if content.startswith(_READ_RESULT_PREFIX): continue for line in content.splitlines(): - head = line.split(" — ", 1)[0].strip() - if "/" in head: - source, _, path = head.rpartition("/") - if source and path: - docs.append((source, path)) + match = _CATALOG_LINE_RE.match(line) + if match: + docs.append((match.group("source"), match.group("path"))) return docs @@ -327,8 +334,8 @@ def _tool_flow(body: dict[str, Any]) -> tuple[str, ...] | None: are in the messages yet: the model lists the catalog. * ``("read", source, path, "call_1")`` — a ``tool``-role catalog result is in the messages: the model reads its FIRST - ``source/path — title`` line (split on ``" — "``, then - ``rsplit("/", 1)``). + ``source: X | path: Y | title: Z`` line (the labeled + ``source:`` / ``path:`` fields, phase 63). * ``("answer", "source/path", content)`` — a ``tool``-role read result (``"Document :\n"``) is in the messages: the model answers, quoting the read document. Reached diff --git a/tests/unit/test_agent.py b/tests/unit/test_agent.py index 8098551..de712b1 100644 --- a/tests/unit/test_agent.py +++ b/tests/unit/test_agent.py @@ -110,6 +110,18 @@ def test_agent_tools_names_and_parameters() -> None: assert by_name["read_document"]["function"]["description"] == ( "Add the full content of one more indexed document to your context" ) + # Phase 63 (A2): the parameter descriptions point the LLM at the + # labeled `source:` / `path:` fields of the list_documents output. + assert read_params["properties"]["source"]["description"] == ( + "The document's source, as shown after 'source: ' in the " + "list_documents output (e.g. 'Homelab' from " + "'source: Homelab | path: homelab/aws-route53.md')." + ) + assert read_params["properties"]["path"]["description"] == ( + "The document's path, as shown after 'path: ' in the " + "list_documents output (e.g. 'homelab/aws-route53.md' from " + "'source: Homelab | path: homelab/aws-route53.md')." + ) # ---------- happy path: list → read → answer ---------- @@ -184,8 +196,8 @@ def test_list_then_read_then_answer( "tool_call_id": "call_1", "content": ( "2 documents:\n" - "Deployments/backups.md — Backup Strategy\n" - "Homelab/aws-route53.md — AWS Route53 Records" + "source: Deployments | path: backups.md | title: Backup Strategy\n" + "source: Homelab | path: aws-route53.md | title: AWS Route53 Records" ), } # The second follow-up request carries the read call + the FULL text. @@ -243,7 +255,7 @@ def test_always_list_bounded_by_round_cap(monkeypatch: pytest.MonkeyPatch) -> No ``agent_max_rounds`` tool rounds, then one forced ``tools=None`` request streams the answer — the cap is the only forced exit.""" monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) - listing = "1 documents:\nS/a.md — A" + listing = "1 documents:\nsource: S | path: a.md | title: A" holder = AgentHolder() llm = ScriptedLLM( [ToolCallPiece(id="call_1", name="list_documents", arguments={})], @@ -359,8 +371,8 @@ def test_relist_executes_and_counts(monkeypatch: pytest.MonkeyPatch) -> None: assert holder.tool_calls == 2 # both re-lists executed and counted listing = ( "2 documents:\n" - "Deployments/backups.md — Backup Strategy\n" - "Homelab/aws-route53.md — AWS Route53 Records" + "source: Deployments | path: backups.md | title: Backup Strategy\n" + "source: Homelab | path: aws-route53.md | title: AWS Route53 Records" ) # The answer request carries the catalog a second time as a tool result. assert llm.requests[2][0][3]["content"] == listing # first listing diff --git a/tests/unit/test_mock_tool_flow.py b/tests/unit/test_mock_tool_flow.py index 49e4684..3c16644 100644 --- a/tests/unit/test_mock_tool_flow.py +++ b/tests/unit/test_mock_tool_flow.py @@ -33,21 +33,25 @@ SYSTEM_LOW = "LOW\n" TOOLS = [{"type": "function", "function": {"name": "list_documents"}}] #: The agent's ``list_documents`` output for a two-document KB -# (``app/rag/agent.py`` ``_execute_tool``): one ``source/path — title`` -#: line per document, ``(source, path)`` order. +#: (``app/rag/agent.py`` ``_execute_tool``): one +#: ``source: X | path: Y | title: Z`` line per document (phase 63: labeled, +#: unambiguous fields), ``(source, path)`` order. CATALOG_2 = ( "2 documents:\n" - "Deployments/example-record-file.json — Example Record File\n" - "Homelab/aws-route53.md — AWS Route 53 Notes" + "source: Deployments | path: example-record-file.json | title: Example Record File\n" + "source: Homelab | path: aws-route53.md | title: AWS Route 53 Notes" ) -CATALOG_1 = "1 documents:\nDeployments/example-record-file.json — Example Record File" +CATALOG_1 = ( + "1 documents:\n" + "source: Deployments | path: example-record-file.json | title: Example Record File" +) CATALOG_3 = ( "3 documents:\n" - "Deployments/aaa.md — AAA\n" - "Deployments/bbb.md — BBB\n" - "Homelab/ccc.md — CCC" + "source: Deployments | path: aaa.md | title: AAA\n" + "source: Deployments | path: bbb.md | title: BBB\n" + "source: Homelab | path: ccc.md | title: CCC" ) DOC1_SP = "Deployments/example-record-file.json" @@ -116,10 +120,23 @@ def test_single_flow_list_step() -> None: def test_single_flow_read_step_first_catalog_line() -> None: flow = _tool_flow(_body(SINGLE_USER, (CATALOG_3,))) - # The FIRST listing line (Deployments/aaa.md), rsplit convention. + # The FIRST listing line (Deployments/aaa.md), labeled fields. assert flow == ("read", "Deployments", "aaa.md", "call_1") +def test_read_step_nested_path_stays_intact() -> None: + # Phase 63 bug report: the path itself contains ``/`` — the old + # ``source/path — title`` + ``rpartition("/")`` parse misread the + # split (``source=brain-of-reese-main/homelab``). The labeled fields + # recover the nested path intact, however deep. + catalog = ( + "1 documents:\n" + "source: brain-of-reese-main | path: homelab/aws-route53.md | title: aws-route53" + ) + flow = _tool_flow(_body(SINGLE_USER, (catalog,))) + assert flow == ("read", "brain-of-reese-main", "homelab/aws-route53.md", "call_1") + + def test_single_flow_answer_step_with_tools_offered() -> None: # Phase 45: the round cap keeps the tools offered until it is hit — # the answer step fires regardless of the ``tools`` parameter.