fix(agent): unambiguous document listing format for LLM parsing
This commit is contained in:
@@ -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).
|
||||
|
||||
|
||||
+11
-7
@@ -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
|
||||
|
||||
+25
-18
@@ -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 <source/path>. <first 80
|
||||
chars of the read document's content>`` — so a suite can assert
|
||||
@@ -269,6 +270,14 @@ TABLE_ANSWER = (
|
||||
#: ``_execute_tool``): ``"Document <source/path>:\n<content>"``.
|
||||
_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<source>.+?) \| path: (?P<path>.+?) \| 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 <source/path>:\n<content>"``) is in the
|
||||
messages: the model answers, quoting the read document. Reached
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -33,21 +33,25 @@ SYSTEM_LOW = "<relevance>LOW</relevance>\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.
|
||||
|
||||
Reference in New Issue
Block a user