feat(agent): align the document tools with the harness-trained shape — ls, read(path), grep(pattern, path?)
This commit is contained in:
@@ -22,12 +22,12 @@ from typing import Any
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import func, select, text
|
||||
from sqlalchemy import delete, func, select, text
|
||||
|
||||
from app.api import chat as chat_api
|
||||
from app.config import Settings, get_settings
|
||||
from app.main import app as fastapi_app
|
||||
from app.models import Chunk, QueryLog
|
||||
from app.models import Chunk, GitSource, QueryLog
|
||||
from app.rag import agent
|
||||
from app.rag.agent import AGENT_TOOLS
|
||||
from app.rag.importer import import_sources
|
||||
@@ -550,13 +550,13 @@ def test_grounded_turn_streams_tool_frames_and_cites_read_doc(
|
||||
tool_script=[
|
||||
[
|
||||
StreamPiece("thinking", "Let me list what is indexed…"),
|
||||
ToolCallPiece(id="call_1", name="list_documents", arguments={}),
|
||||
ToolCallPiece(id="call_1", name="ls", arguments={}),
|
||||
],
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_2",
|
||||
name="read_document",
|
||||
arguments={"source": "docs", "path": "homelab/backups.md"},
|
||||
name="read",
|
||||
arguments={"path": "docs/homelab/backups.md"},
|
||||
)
|
||||
],
|
||||
# the answer request still carries the tools (2 rounds < the
|
||||
@@ -580,10 +580,12 @@ def test_grounded_turn_streams_tool_frames_and_cites_read_doc(
|
||||
|
||||
list_frame, read_frame = frames[1], frames[2]
|
||||
assert set(list_frame) == {"type", "name", "argument"}
|
||||
assert list_frame["name"] == "list_documents"
|
||||
assert list_frame["argument"] is None # the tool takes no parameters
|
||||
assert list_frame["name"] == "ls"
|
||||
assert list_frame["argument"] is None # no ``path`` argument was passed
|
||||
assert set(read_frame) == {"type", "name", "argument"}
|
||||
assert read_frame["name"] == "read_document"
|
||||
assert read_frame["name"] == "read"
|
||||
# Phase 70: the frame's argument is the single string the model
|
||||
# passed — the combined ``source/path``.
|
||||
assert read_frame["argument"] == "docs/homelab/backups.md"
|
||||
|
||||
deltas = [f for f in frames if f["type"] == "delta"]
|
||||
@@ -621,28 +623,24 @@ def test_grounded_turn_streams_tool_frames_and_cites_read_doc(
|
||||
assert "'docs/homelab/backups.md'" in lines[-1]
|
||||
|
||||
|
||||
def test_grounded_turn_streams_search_tool_frames(
|
||||
def test_grounded_turn_streams_grep_tool_frames(
|
||||
client, db, seeded_kb: FakeRagLLM
|
||||
) -> None:
|
||||
"""Phase 68: a scripted ``search_documents`` call streams as
|
||||
``{type: "tool", name: "search_documents", argument: <pattern>}`` —
|
||||
"""Phase 68 (renamed ``grep`` in phase 70): a scripted ``grep`` call
|
||||
streams as ``{type: "tool", name: "grep", argument: <pattern>}`` —
|
||||
the raw pattern is the frame's ``argument`` (the UI renders the
|
||||
"searching for" line from it). A non-string pattern — a model error
|
||||
the backend refuses — yields ``argument: null``. A search adds no
|
||||
the backend refuses — yields ``argument: null``. A grep adds no
|
||||
source: ``done.sources`` stays the retrieval docs (locked A5)."""
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_1",
|
||||
name="search_documents",
|
||||
arguments={"pattern": "Cilium"},
|
||||
),
|
||||
ToolCallPiece(id="call_1", name="grep", arguments={"pattern": "Cilium"}),
|
||||
],
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_2",
|
||||
name="search_documents",
|
||||
name="grep",
|
||||
arguments={"pattern": 42}, # model error: non-string
|
||||
),
|
||||
],
|
||||
@@ -659,25 +657,90 @@ def test_grounded_turn_streams_search_tool_frames(
|
||||
|
||||
types = [f["type"] for f in frames]
|
||||
assert "error" not in types
|
||||
assert len(scripted.seen_tools) == 3 # both searches executed (rounds)
|
||||
assert len(scripted.seen_tools) == 3 # both greps executed (rounds)
|
||||
|
||||
tool_frames = [f for f in frames if f["type"] == "tool"]
|
||||
assert len(tool_frames) == 2
|
||||
first, second = tool_frames
|
||||
assert set(first) == {"type", "name", "argument"}
|
||||
assert first["name"] == "search_documents"
|
||||
assert first["name"] == "grep"
|
||||
assert first["argument"] == "Cilium" # the raw pattern
|
||||
assert set(second) == {"type", "name", "argument"}
|
||||
assert second["name"] == "search_documents"
|
||||
assert second["name"] == "grep"
|
||||
assert second["argument"] is None # the non-string pattern → null
|
||||
|
||||
# The searches still answered: deltas, then a grounded done.
|
||||
# The greps still answered: deltas, then a grounded done.
|
||||
assert [f for f in frames if f["type"] == "delta"]
|
||||
done = frames[-1]
|
||||
assert done["type"] == "done" and done["deflected"] is False
|
||||
paths = [s["path"] for s in done["sources"]]
|
||||
assert "homelab/kubernetes.md" in paths # retrieval docs, unchanged
|
||||
assert "homelab/backups.md" not in paths # a search adds no source
|
||||
assert "homelab/backups.md" not in paths # a grep adds no source
|
||||
|
||||
|
||||
def test_tool_frames_carry_the_model_arguments_regardless_of_execution(
|
||||
client, db, seeded_kb: FakeRagLLM, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""Phase 70 pins: the frame's ``argument`` is the single string
|
||||
argument the model passed — an ``ls`` frame carries the scope when
|
||||
the model gave one (null only when it is omitted, pinned above) —
|
||||
and frame emission is execution-independent: a rejected call (an
|
||||
unknown ``read`` path) still streams its frame with the model's
|
||||
argument as-is. The rejected read adds no source (``done.sources``
|
||||
stays the retrieval docs), and rejected calls count nothing
|
||||
(``tool_calls=1`` — only the executed scoped ``ls``)."""
|
||||
# The scoped ``ls`` source-name check reads the registry — insert a
|
||||
# row resolving to ``docs`` (the fixture's source name) and delete
|
||||
# it again afterwards.
|
||||
src = GitSource(url="https://github.com/reese/docs.git", kind="git")
|
||||
db.add(src)
|
||||
db.commit()
|
||||
try:
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[
|
||||
[ToolCallPiece(id="call_1", name="ls", arguments={"path": "docs"})],
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_2", name="read", arguments={"path": "docs/homelab/nope.md"}
|
||||
)
|
||||
],
|
||||
]
|
||||
)
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: scripted
|
||||
try:
|
||||
caplog.set_level(logging.INFO, logger="app.chat")
|
||||
_, _, frames = _stream_chat(client, QUESTION)
|
||||
finally:
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
finally:
|
||||
db.execute(delete(GitSource).where(GitSource.id == src.id))
|
||||
db.commit()
|
||||
|
||||
types = [f["type"] for f in frames]
|
||||
assert "error" not in types
|
||||
# Both calls stream a frame — the rejected read included.
|
||||
tool_frames = [f for f in frames if f["type"] == "tool"]
|
||||
assert len(tool_frames) == 2
|
||||
ls_frame, read_frame = tool_frames
|
||||
assert set(ls_frame) == {"type", "name", "argument"}
|
||||
assert ls_frame["name"] == "ls"
|
||||
assert ls_frame["argument"] == "docs" # the model's scope, as passed
|
||||
assert set(read_frame) == {"type", "name", "argument"}
|
||||
assert read_frame["name"] == "read"
|
||||
# The rejected call's frame still carries the model's argument as
|
||||
# passed — frame emission is execution-independent.
|
||||
assert read_frame["argument"] == "docs/homelab/nope.md"
|
||||
|
||||
# The rejected read adds no source — done.sources stays retrieval.
|
||||
done = frames[-1]
|
||||
assert done["type"] == "done" and done["deflected"] is False
|
||||
paths = [s["path"] for s in done["sources"]]
|
||||
assert "homelab/kubernetes.md" in paths # retrieval docs, unchanged
|
||||
assert "homelab/nope.md" not in paths # the refused read cites nothing
|
||||
|
||||
# The rejected call counts nothing — only the executed scoped ls.
|
||||
lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()]
|
||||
assert lines and "tool_calls=1" in lines[-1]
|
||||
|
||||
|
||||
def test_deflected_turn_stays_byte_identical_without_tools(
|
||||
@@ -690,12 +753,12 @@ def test_deflected_turn_stays_byte_identical_without_tools(
|
||||
``tools`` key."""
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[
|
||||
[ToolCallPiece(id="call_1", name="list_documents", arguments={})],
|
||||
[ToolCallPiece(id="call_1", name="ls", arguments={})],
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_2",
|
||||
name="read_document",
|
||||
arguments={"source": "docs", "path": "homelab/backups.md"},
|
||||
name="read",
|
||||
arguments={"path": "docs/homelab/backups.md"},
|
||||
)
|
||||
],
|
||||
[StreamPiece("content", "never used — the agent never runs")],
|
||||
@@ -743,12 +806,12 @@ def test_zero_max_rounds_reproduce_pre_phase_single_request(
|
||||
the kill switch survives the phase-45 budget removal."""
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[
|
||||
[ToolCallPiece(id="call_1", name="list_documents", arguments={})],
|
||||
[ToolCallPiece(id="call_1", name="ls", arguments={})],
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_2",
|
||||
name="read_document",
|
||||
arguments={"source": "docs", "path": "homelab/backups.md"},
|
||||
name="read",
|
||||
arguments={"path": "docs/homelab/backups.md"},
|
||||
)
|
||||
],
|
||||
]
|
||||
@@ -799,7 +862,7 @@ def test_tool_execution_db_failure_yields_error_event(
|
||||
``error`` event as the pre-stream retrieval path — never a severed
|
||||
stream (the "never stale" contract, PLAN §7.4)."""
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[[ToolCallPiece(id="call_1", name="list_documents", arguments={})]]
|
||||
tool_script=[[ToolCallPiece(id="call_1", name="ls", arguments={})]]
|
||||
)
|
||||
|
||||
def boom(*_a: Any, **_k: Any) -> Any:
|
||||
@@ -815,7 +878,7 @@ def test_tool_execution_db_failure_yields_error_event(
|
||||
# The ``tool`` frame went out first (the model requested the call);
|
||||
# the failed execution ends the turn with the structured error event.
|
||||
assert [f["type"] for f in frames] == ["tool", "error"]
|
||||
assert frames[0]["name"] == "list_documents"
|
||||
assert frames[0]["name"] == "ls"
|
||||
assert "offline mid-question" in frames[1]["detail"]
|
||||
assert db.scalars(select(QueryLog)).all() == [] # no row for a failed turn
|
||||
|
||||
|
||||
Reference in New Issue
Block a user