From 8cf3a827eea776bef3c97cb30de42f32363ce47d Mon Sep 17 00:00:00 2001 From: ducoterra Date: Wed, 2 Sep 2026 12:04:34 -0400 Subject: [PATCH] =?UTF-8?q?feat(agent):=20search=5Fdocuments=20tool=20?= =?UTF-8?q?=E2=80=94=20the=20model=20can=20grep=20the=20indexed=20document?= =?UTF-8?q?s=20for=20an=20exact=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../67_llm_retry/00_phase.md | 0 .../68_search_tool/01_search_tool_backend.md | 0 .../68_search_tool/02_search_tool_api_ui.md | 0 .../68_search_tool/03_e2e_and_commit.md | 0 .../67_llm_retry/67_llm_retry__00_phase.a1.md | 11 + .../67_llm_retry__00_phase.a1.validate | 72 +++ ...search_tool__01_search_tool_backend.a1.err | 0 ..._search_tool__01_search_tool_backend.a1.md | 15 + ...h_tool__01_search_tool_backend.a1.validate | 73 +++ ..._search_tool__02_search_tool_api_ui.a1.err | 0 ...8_search_tool__02_search_tool_api_ui.a1.md | 16 + ...ch_tool__02_search_tool_api_ui.a1.validate | 73 +++ .../68_search_tool__03_e2e_and_commit.a1.err | 0 app/api/chat.py | 69 +-- app/rag/agent.py | 184 ++++++- app/schemas.py | 22 +- frontend/assets/app.js | 45 +- tests/e2e/mock_llm.py | 118 +++++ tests/e2e/test_search_tool.py | 477 ++++++++++++++++++ tests/fixtures/search_docs/reese-notes.md | 9 + tests/integration/test_agent_tools.py | 163 +++++- tests/integration/test_api.py | 11 +- tests/integration/test_chat_api.py | 59 +++ tests/unit/test_agent.py | 319 +++++++++++- tests/unit/test_frontend_tool_states.py | 34 +- tests/unit/test_mock_tool_flow.py | 95 ++++ 26 files changed, 1780 insertions(+), 85 deletions(-) rename .agent/phases/{todo => complete}/67_llm_retry/00_phase.md (100%) rename .agent/phases/{todo => complete}/68_search_tool/01_search_tool_backend.md (100%) rename .agent/phases/{todo => complete}/68_search_tool/02_search_tool_api_ui.md (100%) rename .agent/phases/{todo => complete}/68_search_tool/03_e2e_and_commit.md (100%) create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.validate create mode 100644 .agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.err create mode 100644 .agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.md create mode 100644 .agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.validate create mode 100644 .agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.err create mode 100644 .agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.md create mode 100644 .agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.validate create mode 100644 .agent/reports/68_search_tool/68_search_tool__03_e2e_and_commit.a1.err create mode 100644 tests/e2e/test_search_tool.py create mode 100644 tests/fixtures/search_docs/reese-notes.md diff --git a/.agent/phases/todo/67_llm_retry/00_phase.md b/.agent/phases/complete/67_llm_retry/00_phase.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/00_phase.md rename to .agent/phases/complete/67_llm_retry/00_phase.md diff --git a/.agent/phases/todo/68_search_tool/01_search_tool_backend.md b/.agent/phases/complete/68_search_tool/01_search_tool_backend.md similarity index 100% rename from .agent/phases/todo/68_search_tool/01_search_tool_backend.md rename to .agent/phases/complete/68_search_tool/01_search_tool_backend.md diff --git a/.agent/phases/todo/68_search_tool/02_search_tool_api_ui.md b/.agent/phases/complete/68_search_tool/02_search_tool_api_ui.md similarity index 100% rename from .agent/phases/todo/68_search_tool/02_search_tool_api_ui.md rename to .agent/phases/complete/68_search_tool/02_search_tool_api_ui.md diff --git a/.agent/phases/todo/68_search_tool/03_e2e_and_commit.md b/.agent/phases/complete/68_search_tool/03_e2e_and_commit.md similarity index 100% rename from .agent/phases/todo/68_search_tool/03_e2e_and_commit.md rename to .agent/phases/complete/68_search_tool/03_e2e_and_commit.md diff --git a/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.md new file mode 100644 index 0000000..1d3ba97 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.md @@ -0,0 +1,11 @@ +**Phase 67 final verification — all green (no defects found; one commit-hygiene fix).** + +- Verified all 5 task deliverables in place: `BOR_LLM_RETRIES`/`BOR_LLM_RETRY_DELAY` settings + validators, `chat_stream_retried`/`RetryPiece` in `app/rag/llm.py`, embedding loop + deflected-stream retry + `ChatRetryEvent` in `app/api/chat.py`, per-round retry in `run_agent`, `retry` branch in `app.js`, mock failure injection, `.env.example` docs. +- Fixed: the task-05 file move + harness reports were uncommitted — amended the phase commit to include them (house convention: one atomic commit per phase, per phase 66). +- `uv run pytest --cov=app --cov-report=term-missing` → **1281 passed**, TOTAL **99%** (>90% ✓) +- `uv run ruff check . && uv run pyright` → **All checks passed / 0 errors** +- `uv run pytest tests/e2e/test_llm_retry.py -v --no-cov` → **4 passed** (dead-then-recovered deflected + grounded, embedding retry, exhaustion → error banner; DB up) +- Regressions in isolation: `test_chat_rag` 3 ✓, `test_agent_document_tools` 4 ✓, `test_stop_generation` 3 ✓, `test_retry_answer` 4 ✓ + +Completion criteria: (1) env knobs honored e2e + documented ✓ (2) recovered→completed turn with "retrying (n of N)…" status, dead→terminal banner after N ✓ (3) post-token failure → `error`, no retry/duplication ✓ (4) suite/coverage/lint ✓ (5) retry E2E isolated ✓ (6) regression E2E ✓ (7) one `--no-gpg-sign` commit `88293ed`, tasks 01–05 in `complete/` (overview move left to harness) ✓. +No deviations. Next pending phase: **`68_search_tool`**. diff --git a/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.validate new file mode 100644 index 0000000..069fe22 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 33%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 50%] +........................................................................ [ 56%] +........................................................................ [ 61%] +........................................................................ [ 67%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 84%] +........................................................................ [ 89%] +........................................................................ [ 95%] +......................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2587 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.err b/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.md b/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.md new file mode 100644 index 0000000..af424ea --- /dev/null +++ b/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.md @@ -0,0 +1,15 @@ +All green. Final report: + +**Phase 68, Task 01 — `search_documents` backend: complete** + +- `app/rag/agent.py`: third `AGENT_TOOLS` entry `search_documents` (required `pattern`, optional `source`/`path` with phase-63 labeled-field wording); constants `SEARCH_MAX_MATCHES=20`, `SEARCH_LINE_LIMIT=200`, `MISSING_SEARCH_ARGS`, `NO_MATCHES(_SCOPED)`; module-level `grep_document` (case-insensitive fixed substring, 1-based, rstripped) and `all_documents` (bulk `select(Document)` ordered by source,path — chosen over N+1 `find_document` calls); `_execute_tool` branch (after `read_document`, before `UNKNOWN_TOOL`): validated pattern, half-specified target fails loud, scoped via `find_document`, global 20-cap in catalog order with early stop, 200-char line truncation, pattern truncated to 100 chars in no-match lines; search never touches `read_docs`; docstring updated (3 tools; search = locator). +- No-match search **counts** as an executed call (`tool_calls += 1`) — a result, not a refusal, matching the `list_documents` "0 documents:" precedent; only missing/invalid args and unknown scoped doc are uncounted refusals. +- Tests: 11 new unit tests (+ updated `AGENT_TOOLS` pin) in `tests/unit/test_agent.py`; 6 new integration tests in `tests/integration/test_agent_tools.py` (incl. scripted `ToolCallPiece` through `run_agent` against real Postgres). + +Results: +- `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v` → 56 passed +- `uv run pytest --cov=app` → 1305 passed; TOTAL coverage 99% (>90%; `agent.py` 100%) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors +- Regression E2E in isolation: `test_agent_document_tools.py` 4 passed; `test_agent_unlimited_tools.py` 4 passed + +No deviations from locked A5/A6. No commit — the phase commit belongs to task 03. Next pending task: `.agent/phases/todo/68_search_tool/02_search_tool_api_ui.md`. diff --git a/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.validate b/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.validate new file mode 100644 index 0000000..178a35c --- /dev/null +++ b/.agent/reports/68_search_tool/68_search_tool__01_search_tool_backend.a1.validate @@ -0,0 +1,73 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 27%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 44%] +........................................................................ [ 49%] +........................................................................ [ 55%] +........................................................................ [ 60%] +........................................................................ [ 66%] +........................................................................ [ 71%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 99%] +......... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 120 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2629 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.err b/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.md b/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.md new file mode 100644 index 0000000..6a57618 --- /dev/null +++ b/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.md @@ -0,0 +1,16 @@ +All green. Task complete — here's the report: + +**Implemented (task 02 — SSE mapping + frontend search status/tool line):** +- `app/api/chat.py`: `ToolCallPiece` branch now maps `search_documents` → `argument = pattern` (non-string pattern → null); `read_document`/`list_documents` shapes byte-identical; module docstring updated +- `app/schemas.py`: `ChatToolEvent` docstring/comments — third tool name, `argument` = path/pattern/null +- `frontend/assets/app.js`: status ternary gains locked `…is searching for ${argument}` branch (gated on name+argument, read→search→listing order); `appendToolLine` gains `🔎 Searching for ` branch (pattern-as-data via textContent, listing stays the else); header comments updated; persistence/restore already generic (`{name, argument}`) +- Tests: JS pins extended in `tests/unit/test_frontend_tool_states.py` (status gate+order, search branch `` pin, generic `toolAcc.push({ name, argument })`); new integration test in `test_chat_api.py` (pattern frame + non-string→null + no source added); emoji guard in `test_api.py` extended for the third owner-locked tool-line literal +- Notable: status/tool-line strings follow the task's exact code (no literal quotes); no deviations from locked decisions + +**Results:** +- `uv run pytest` → **1306 passed**, exit 0; coverage TOTAL **99%** (>90% gate) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- `uv run pytest tests/unit/test_frontend_tool_states.py tests/integration/test_chat_api.py -v` → 32 passed +- Regression sanity: `uv run pytest tests/e2e/test_agent_document_tools.py -v --no-cov` → 4 passed + +**Next pending task:** `.agent/phases/todo/68_search_tool/03_e2e_and_commit.md` (E2E suite + commit + phase move) diff --git a/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.validate b/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.validate new file mode 100644 index 0000000..59412ed --- /dev/null +++ b/.agent/reports/68_search_tool/68_search_tool__02_search_tool_api_ui.a1.validate @@ -0,0 +1,73 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 27%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 44%] +........................................................................ [ 49%] +........................................................................ [ 55%] +........................................................................ [ 60%] +........................................................................ [ 66%] +........................................................................ [ 71%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 99%] +.......... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 153 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 120 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2635 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/68_search_tool/68_search_tool__03_e2e_and_commit.a1.err b/.agent/reports/68_search_tool/68_search_tool__03_e2e_and_commit.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/app/api/chat.py b/app/api/chat.py index 6921b99..69147a8 100644 --- a/app/api/chat.py +++ b/app/api/chat.py @@ -49,25 +49,30 @@ outline (0 when absent) and the per-turn log line records Agent document tools (phase 37, PLAN §4 extension, owner permission 2026-08-26; phase 45 removed the per-tool budgets — owner permission -2026-08-27): a **grounded** turn (``not plan.deflected``) no longer -streams a bare ``chat_stream`` — it runs the agent loop -(``app.rag.agent.run_agent``), which offers the model the two -server-side tools ``list_documents`` / ``read_document`` for the whole -turn (as many calls as the model wants, re-lists included) until it -answers or the round cap (``BOR_AGENT_MAX_ROUNDS``, default 10) forces -one final no-tools answer. Each model-requested call streams as an SSE -``tool`` event — ``{"type": "tool", "name": …, "argument": -"source/path" | null}`` — ahead of the answer's ``delta`` frames. -``done.sources``, ``query_log.sources`` and the per-turn log line all -report the same combined source list (retrieval docs + the agent's -read docs, deduped by ``(source, path)``, order preserved), and the log -line records ``tool_calls=N`` after ``thinking_chars=N`` (PLAN §9 line -extension — ``N`` counts executed tool calls; rejected calls do not -count). **Deflected turns keep the direct ``chat_stream`` — -byte-identical to the pre-phase path (A8):** the LOW prompt never -carries tools, and with ``agent_max_rounds`` at **0** ``run_agent`` -makes exactly one ``tools=None`` request, reproducing the pre-phase -behavior (the kill switch). +2026-08-27; phase 68 added the ``search_documents`` grep): a +**grounded** turn (``not plan.deflected``) no longer streams a bare +``chat_stream`` — it runs the agent loop (``app.rag.agent.run_agent``), +which offers the model the three server-side tools +``list_documents`` / ``read_document`` / ``search_documents`` for the +whole turn (as many calls as the model wants, re-lists and re-searches +included) until it answers or the round cap (``BOR_AGENT_MAX_ROUNDS``, +default 10) forces one final no-tools answer. Each model-requested call +streams as an SSE ``tool`` event — ``{"type": "tool", "name": …, +"argument": "source/path" | pattern | null}`` — ahead of the answer's +``delta`` frames: ``argument`` is the read document's path for +``read_document``, the raw search pattern for ``search_documents`` +(a non-string pattern — a model error the backend refuses — yields +null), and null for ``list_documents``. ``done.sources``, +``query_log.sources`` and the per-turn log line all report the same +combined source list (retrieval docs + the agent's read docs, deduped +by ``(source, path)``, order preserved — a search adds no source; it is +a locator, locked A5), and the log line records ``tool_calls=N`` after +``thinking_chars=N`` (PLAN §9 line extension — ``N`` counts executed +tool calls; rejected calls do not count). **Deflected turns keep the +direct ``chat_stream`` — byte-identical to the pre-phase path (A8):** +the LOW prompt never carries tools, and with ``agent_max_rounds`` at +**0** ``run_agent`` makes exactly one ``tools=None`` request, +reproducing the pre-phase behavior (the kill switch). LLM retries (phase 67, ``BOR_LLM_RETRIES`` / ``BOR_LLM_RETRY_DELAY``, owner-locked 2026-09-01): when the aipi endpoint dies before a request @@ -396,18 +401,22 @@ async def chat( async for piece in answer_stream: # StreamPiece | ToolCallPiece | RetryPiece if isinstance(piece, ToolCallPiece): # Phase 37 (PLAN §4 extension): one SSE ``tool`` - # frame per model-requested call; ``argument`` is - # the read_document "source/path" (null - # otherwise). + # frame per model-requested call. ``argument`` is + # the read_document "source/path"; phase 68 + # extends it with the search_documents pattern + # (a non-string pattern — a model error the + # backend refuses — is null); null otherwise. + if piece.name == "read_document": + argument = ( + f"{piece.arguments.get('source')}/{piece.arguments.get('path')}" + ) + elif piece.name == "search_documents": + pattern = piece.arguments.get("pattern") + argument = pattern if isinstance(pattern, str) else None + else: + argument = None yield sse_event( - ChatToolEvent( - name=piece.name, - argument=( - f"{piece.arguments.get('source')}/{piece.arguments.get('path')}" - if piece.name == "read_document" - else None - ), - ).model_dump() + ChatToolEvent(name=piece.name, argument=argument).model_dump() ) continue if isinstance(piece, RetryPiece): diff --git a/app/rag/agent.py b/app/rag/agent.py index f1518f5..be2e397 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -15,27 +15,38 @@ probe came back "supported". Loop contract (one grounded chat turn; the API layer wires this in, task 04): -1. The model is offered the two OpenAI functions in :data:`AGENT_TOOLS` +1. The model is offered the three OpenAI functions in :data:`AGENT_TOOLS` for the whole turn — phase 45 removed the phase-37 per-tool budgets (owner permission 2026-08-27, ``TODO.md`` L8: "allow the LLM to make - as many tool calls as it wants"): ``list_documents`` and - ``read_document`` can each be called as many times as the model needs, - re-lists included. With ``settings.agent_max_rounds`` - (``BOR_AGENT_MAX_ROUNDS``, default 10) at 0 the loop makes exactly one - request with ``tools=None`` — byte-identical to the pre-phase-37 chat - path (the kill switch). + as many tool calls as it wants"): ``list_documents``, + ``read_document`` and ``search_documents`` can each be called as many + times as the model needs, re-lists and re-searches included. With + ``settings.agent_max_rounds`` (``BOR_AGENT_MAX_ROUNDS``, default 10) + at 0 the loop makes exactly one request with ``tools=None`` — + byte-identical to the pre-phase-37 chat 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: 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). + the model does) — ``read_document`` returns the document's **full** + content (A7-revised contract: never truncated) — and + ``search_documents`` greps the indexed documents (or one named + document) for a case-insensitive fixed substring and returns up to 20 + ``source/path:line: text`` match lines (owner-locked A5, phase 68), + each line truncated to 200 chars. A search is a **locator**, not a + context-adder: it never appends to the answer context (only + ``read_document`` does — ``holder.read_docs`` is untouched by a + search). 3. Rejected calls get a one-line refusal and count in nothing (``holder.tool_calls`` tracks executed calls only): unknown tool name - → ``"Unknown tool."``; missing ``source``/``path`` arguments; a - document already in context (seed or previously read) → ``"Already in - your context."``; an unknown ``source/path`` → ``"No document at …"``. + → ``"Unknown tool."``; missing ``source``/``path`` arguments; a search + without a usable ``pattern`` (missing, blank or non-string) or with a + half-specified ``source``/``path`` target; a document already in + context (seed or previously read) → ``"Already in your + context."``; an unknown ``source/path`` (read or scoped search) → + ``"No document at …"``. A search that ran but found nothing is NOT a + rejection — its ``"No matches for …"`` line is a (counted) result. A rejected call still consumes a *round* in the loop, so a pathological stream that keeps emitting rejected calls is bounded by the cap (point 4). @@ -65,7 +76,8 @@ task 04): exactly one round. With ``settings.llm_retries=0`` every request is a single plain attempt (the pre-phase-67 path). -The DB accessors (:func:`list_catalog`, :func:`find_document`) are +The DB accessors (:func:`list_catalog`, :func:`find_document`, +:func:`all_documents`) and the :func:`grep_document` line matcher are module-level functions so unit tests can monkeypatch them without a database. """ @@ -92,10 +104,11 @@ from app.rag.llm import ( logger = logging.getLogger("app.agent") -#: The two agent tools (phase 37): OpenAI function definitions passed as -#: ``tools=AGENT_TOOLS`` to ``chat_stream`` for the whole grounded turn — -#: phase 45 removed the per-tool budgets; the round cap -#: (``BOR_AGENT_MAX_ROUNDS``) is the only bound. +#: The three agent tools (phase 37; ``search_documents`` added in phase +#: 68): OpenAI function definitions passed as ``tools=AGENT_TOOLS`` to +#: ``chat_stream`` for the whole grounded turn — phase 45 removed the +#: per-tool budgets; the round cap (``BOR_AGENT_MAX_ROUNDS``) is the only +#: bound. AGENT_TOOLS: list[dict[str, Any]] = [ { "type": "function", @@ -140,6 +153,49 @@ AGENT_TOOLS: list[dict[str, Any]] = [ }, }, }, + { + "type": "function", + "function": { + "name": "search_documents", + "description": ( + "Search every indexed document for an exact string " + "(case-insensitive) and return up to 20 matching lines as " + "'source/path:line: text' — use this to locate content, " + "then read_document the winner. Optionally pass 'source' " + "and 'path' (as shown in list_documents) to search one " + "document only." + ), + "parameters": { + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": ( + "The exact text to search for (a plain " + "substring, not a regex)" + ), + }, + "source": { + "type": "string", + "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')." + ), + }, + "path": { + "type": "string", + "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')." + ), + }, + }, + "required": ["pattern"], + }, + }, + }, ] #: Tool refusal texts (phase 37): rejected calls count in nothing @@ -148,6 +204,19 @@ AGENT_TOOLS: list[dict[str, Any]] = [ ALREADY_IN_CONTEXT = "Already in your context." UNKNOWN_TOOL = "Unknown tool." MISSING_READ_ARGS = "read_document requires string arguments 'source' and 'path'." +MISSING_SEARCH_ARGS = "search_documents requires a string argument 'pattern'." + +#: Search caps (owner-locked A5, phase 68): a global per-call match cap +#: (across documents, in catalog order) and a per-match-line char limit. +SEARCH_MAX_MATCHES = 20 +SEARCH_LINE_LIMIT = 200 + +#: No-match result lines (templates — the pattern is truncated to 100 +#: chars before formatting, to keep a long pattern from bloating the +#: tool result). A no-match line is a *result* of an executed search, +#: not a refusal (see the module docstring, point 3). +NO_MATCHES = "No matches for '{pattern}' in the knowledge base." +NO_MATCHES_SCOPED = "No matches for '{pattern}' in {source}/{path}." def list_catalog(db: Session) -> list[tuple[str, str, str]]: @@ -174,6 +243,37 @@ def find_document(db: Session, source: str, path: str) -> Document | None: ) +def all_documents(db: Session) -> list[Document]: + """Every indexed document (full rows), ordered by ``(source, path)`` + — catalog order. + + The whole-KB ``search_documents`` path loads all contents in this one + bulk query (catalog order is the locked match order, owner-locked A5). + Module-level (not a method) so unit tests can monkeypatch it. + """ + return list( + db.execute( + select(Document).order_by(Document.source, Document.path) + ).scalars() + ) + + +def grep_document(content: str, pattern: str) -> list[tuple[int, str]]: + """Every line of *content* that contains *pattern*, in file order. + + Case-insensitive **fixed substring** (owner-locked A5: no regex — no + ReDoS surface, a simple contract for the model). Returns + ``(1-based line number, line.rstrip())`` pairs; an empty *content* + never matches a non-empty pattern. + """ + needle = pattern.lower() + return [ + (number, line.rstrip()) + for number, line in enumerate(content.split("\n"), start=1) + if needle in line.lower() + ] + + @dataclass class AgentHolder: """Per-turn agent state the API layer reads after the stream (task 04). @@ -200,8 +300,11 @@ def _execute_tool( Returns the tool result text. A successful call bumps ``holder.tool_calls`` (a successful read also appends the - :class:`Document` to ``holder.read_docs``); rejected calls return - their refusal line and count in nothing. + :class:`Document` to ``holder.read_docs``; a search never does — it + is a locator, locked A5); rejected calls return their refusal line + and count in nothing. A search that ran but found nothing is still a + successful (counted) call — its no-match line is a result, not a + refusal. """ if call.name == "list_documents": rows = list_catalog(db) @@ -229,6 +332,49 @@ def _execute_tool( holder.read_docs.append(doc) holder.tool_calls += 1 return f"Document {source}/{path}:\n{doc.content}" + if call.name == "search_documents": + raw_pattern = call.arguments.get("pattern") + pattern = raw_pattern.strip() if isinstance(raw_pattern, str) else "" + if not pattern: + return MISSING_SEARCH_ARGS + raw_source = call.arguments.get("source") + raw_path = call.arguments.get("path") + source = raw_source.strip() if isinstance(raw_source, str) else "" + path = raw_path.strip() if isinstance(raw_path, str) else "" + if (source == "") != (path == ""): + # A half-specified target is a model error — fail loud with + # the missing-args refusal instead of silently widening to a + # whole-KB search (house style). + return MISSING_SEARCH_ARGS + if source: + target = find_document(db, source, path) + if target is None: + return ( + f"No document at {source}/{path} — check the list_documents output." + ) + docs: list[Document] = [target] + else: + docs = all_documents(db) + matches: list[str] = [] + for doc in docs: + for lineno, line in grep_document(doc.content, pattern): + matches.append( + f"{doc.source}/{doc.path}:{lineno}: {line[:SEARCH_LINE_LIMIT]}" + ) + if len(matches) >= SEARCH_MAX_MATCHES: + break + if len(matches) >= SEARCH_MAX_MATCHES: + break # the global cap is hit — stop scanning + holder.tool_calls += 1 # the search executed (no-match counts too) + # Locked A5: a search never adds context — read_docs untouched. + if not matches: + shown = pattern[:100] # keep a long pattern short in the line + if source: + return NO_MATCHES_SCOPED.format( + pattern=shown, source=source, path=path + ) + return NO_MATCHES.format(pattern=shown) + return "\n".join(matches) return UNKNOWN_TOOL diff --git a/app/schemas.py b/app/schemas.py index 7ac1ab4..e5194f0 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -71,20 +71,24 @@ class ChatThinkingEvent(BaseModel): class ChatToolEvent(BaseModel): """SSE frame for one agent tool call (phase 37, PLAN §4 extension). - A15 extension (owner permission 2026-08-26): a grounded turn may call - the server-side document tools (``list_documents`` / ``read_document``, - see :mod:`app.rag.agent`); each model-requested call streams as - ``{type: "tool", name: str, argument: str | null}`` ahead of the - answer's ``delta`` frames. ``argument`` is the read document's - ``"source/path"`` for ``read_document`` and null otherwise. The client + A15 extension (owner permission 2026-08-26; ``search_documents`` + added in phase 68): a grounded turn may call the server-side + document tools (``list_documents`` / ``read_document`` / + ``search_documents``, see :mod:`app.rag.agent`); each model-requested + call streams as ``{type: "tool", name: str, argument: str | null}`` + ahead of the answer's ``delta`` frames. ``argument`` is the read + document's ``"source/path"`` for ``read_document``, the search + pattern for ``search_documents``, and null otherwise (a non-string + pattern — a model error the backend refuses — is null). The client renders each frame as a "calling tool" line/state (phase 37 task 05); the ``delta`` / ``done`` shapes are unchanged — the read document is - reflected in ``done.sources`` instead. + reflected in ``done.sources`` instead (a search adds no source: it is + a locator, locked A5). """ type: Literal["tool"] = "tool" - name: str # "list_documents" | "read_document" - argument: str | None = None # "source/path" for read_document + name: str # "list_documents" | "read_document" | "search_documents" + argument: str | None = None # "source/path" for read_document, pattern for search_documents class ChatDoneEvent(BaseModel): diff --git a/frontend/assets/app.js b/frontend/assets/app.js index 796a839..b16bd6d 100644 --- a/frontend/assets/app.js +++ b/frontend/assets/app.js @@ -55,20 +55,22 @@ * "New chat" (#new-chat-btn — bound by the shared header module, * phase 34 task 02) clears the key + the list back to the empty state. * - * Agent tool calls (phase 37, PLAN §4 extension): a grounded turn may - * call the two server-side document tools (list_documents / - * read_document, budgeted server-side). Each call streams a `tool` SSE - * frame, and the UI shows the "calling tool" state IN ADDITION to - * "thinking": the UI state itself stays "thinking" (the button stays - * the enabled "Stop" control — phase 48 — never stale, PLAN §7.4) while - * the STATUS LABELS change — the #send-status + typing-indicator labels - * say what Brain is doing ("…is listing documents" / - * "…is reading source/path" — the name prefix resolves from - * window.BOR_BRAND at call time, phase 39) — the button no longer - * relabels to "Calling tool…" (phase 48, owner-locked: it stays "Stop" - * for the whole turn) — and a visible `.tool-call` line (own icon + - * accent color, distinct from the brand-ink Thinking block) is appended - * above the answer, one per call, in order. + * Agent tool calls (phase 37, PLAN §4 extension; phase 68 added + * search_documents): a grounded turn may call the three server-side + * document tools (list_documents / read_document / search_documents, + * bounded only by the round cap — phases 45/68). Each call streams a + * `tool` SSE frame, and the UI shows the "calling tool" state IN + * ADDITION to "thinking": the UI state itself stays "thinking" (the + * button stays the enabled "Stop" control — phase 48 — never stale, + * PLAN §7.4) while the STATUS LABELS change — the #send-status + + * typing-indicator labels say what Brain is doing ("…is listing + * documents" / "…is reading source/path" / "…is searching for + * pattern" — the name prefix resolves from window.BOR_BRAND at call + * time, phase 39) — the button no longer relabels to "Calling tool…" + * (phase 48, owner-locked: it stays "Stop" for the whole turn) — and a + * visible `.tool-call` line (own icon + accent color, distinct from + * the brand-ink Thinking block) is appended above the answer, one per + * call, in order. * Append-only like thinking: frames are tolerated in any interleaving * (a frame after the first delta just appends — the agent loop never * emits one, but it must not crash). The turn record persists an @@ -808,9 +810,9 @@ function closeThinkingBlock(wrap) { * interleaving with thinking frames, even after the first delta (the * agent loop never emits one, but a late frame must not crash) — just * append another line, in order. The SAME helper re-renders the - * persisted lines on restore (phase 14 convention): the path argument - * goes through textContent, so nothing HTML-shaped can come from - * storage. Lines are not interactive (no focus targets). */ + * persisted lines on restore (phase 14 convention): the path/pattern + * argument goes through textContent, so nothing HTML-shaped can come + * from storage. Lines are not interactive (no focus targets). */ function appendToolLine(wrap, name, argument) { const body = wrap?.querySelector?.(".msg-body"); if (!body) return; @@ -832,6 +834,11 @@ function appendToolLine(wrap, name, argument) { const code = document.createElement("code"); code.textContent = argument; // the path is data, never markup line.appendChild(code); + } else if (name === "search_documents" && argument) { + line.textContent = "🔎 Searching for "; + const code = document.createElement("code"); + code.textContent = argument; // the pattern is data, never markup + line.appendChild(code); } else { line.textContent = "🔎 Listing documents"; } @@ -1936,7 +1943,9 @@ async function runTurn(text, { reask = false } = {}) { const toolStatus = name === "read_document" && argument ? `${brand()} is reading ${argument}` - : `${brand()} is listing documents`; + : name === "search_documents" && argument + ? `${brand()} is searching for ${argument}` + : `${brand()} is listing documents`; if (uiState === UI_STATE.thinking) { sendStatus.textContent = toolStatus; document diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 79fa3e9..4136cc2 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -100,6 +100,24 @@ Implements just enough of the aipi surface: offered — e.g. ``agent_max_rounds=0``) behave exactly as today. ``E2E_REAL_LLM=1`` ignores the mock entirely (the real model does what it does). + - user message containing ``search your documents`` + (``SEARCH_TRIGGER``, phase 68, search tool) **and** the system + prompt carries the ```` section -> the deterministic SEARCH + tool flow, discriminated statelessly from the messages (streaming + only): + * request 1 (``tools`` offered, no search result yet): stream + ONLY ``tool_calls`` deltas — ``search_documents`` with + ``{"pattern": SEARCH_PATTERN}`` (id ``call_0``); + * request 2 (a ``tool``-role search result in the messages — + recognizable by its ``source/path:line: text`` match lines or + the sentinel in its content): the content answer, deterministic: + ``Found `` — so a + suite can assert the search result reached the model and landed + in the answer. + Checked BEFORE the plain ``use your tools`` flow (it is the more + specific phrase — same convention as ``think in paragraphs``); no + existing E2E question or fixture file contains the trigger, so + every other suite is unaffected. - user message containing ``show me a table`` (phase 44, markdown tables, TODO.md L6) -> the fixed table answer (``TABLE_ANSWER``): a 3-column service table, an ```` XSS probe line, and @@ -264,6 +282,24 @@ TOOLS_TRIGGER = "use your tools" #: so the 3-step flow is untouched. MULTI_READ_TRIGGER = "read two documents" +#: Phase 68 (search tool, TODO.md L4): a user message containing this +#: substring (case-insensitive) — combined with the ```` section +#: in the system prompt — drives the deterministic SEARCH tool flow +#: (search_documents for ``SEARCH_PATTERN`` → the "Found …" answer), +#: documented in the module docstring. Checked BEFORE ``TOOLS_TRIGGER`` +#: (the more specific phrase wins — the same convention as +#: ``THINK_PARAS_TRIGGER``); verified 2026-09-01: no existing E2E +#: question or fixture file contains the phrase, so every other suite +#: is unaffected. +SEARCH_TRIGGER = "search your documents" + +#: The sentinel the search flow greps for: the e2e fixture document +#: (``tests/fixtures/search_docs/reese-notes.md``) carries exactly one +#: line containing it, so the search result — and the "Found …" answer +#: that quotes its first matched line — is byte-stable (the sentinel +#: convention of ``END_OF_NOTES_TRIGGER``). +SEARCH_PATTERN = "reese-sentinel-42" + #: Phase 44 (markdown-tables story, TODO.md L6): a user message #: containing this substring (case-insensitive) gets the fixed table #: answer (``TABLE_ANSWER`` below) — a 3-column table, an XSS probe @@ -424,6 +460,69 @@ def _catalog_docs(body: dict[str, Any]) -> list[tuple[str, str]]: return docs +#: One line of the agent's ``search_documents`` output (app.rag.agent +#: ``_execute_tool``, phase 68): ``source/path:LINE: text``. The +#: non-greedy prefix keeps nested paths (``/`` in the path) intact. +_SEARCH_LINE_RE = re.compile(r"^(?P.+?):(?P\d+): (?P.*)$") + + +def _search_result_line(body: dict[str, Any]) -> str | None: + """The first matched line's text of a search result in the messages. + + A search result is a ``tool``-role message — never a read result + (those start with the agent's ``"Document "`` prefix) — that either + carries ``source/path:LINE: text`` match lines (the agent's + ``search_documents`` output, phase 68) or the sentinel pattern + itself (its no-match line quotes the pattern). Returns the first + match line's ``text`` part (already 200-char-capped server-side), + or the message's first line in the sentinel-only shape, or ``None`` + when no search result is in the messages yet. + """ + sentinel = SEARCH_PATTERN.lower() + for m in _messages(body): + if m.get("role") != "tool": + continue + content = str(m.get("content") or "") + if content.startswith(_READ_RESULT_PREFIX): + continue + for line in content.splitlines(): + match = _SEARCH_LINE_RE.match(line) + if match: + return match.group("text") + if sentinel in content.lower(): + lines = content.splitlines() + return lines[0] if lines else "" + return None + + +def _search_flow(body: dict[str, Any]) -> tuple[str, ...] | None: + """Classify a SEARCH_TRIGGER request into a step of the search flow. + + * ``("search",)`` — ``tools`` are offered and no search result is + in the messages yet: the model greps the whole KB for + ``SEARCH_PATTERN`` (id ``call_0``). + * ``("found", first_line)`` — a ``tool``-role search result is in + the messages: the model answers, quoting the first matched line + (``Found ``). Reached + regardless of the ``tools`` parameter (phase 45 keeps the tools + offered until the round cap). + * ``None`` — not the search flow: the trigger is absent, the + ```` section is missing (deflected turns never carry it), + or ``tools`` are not offered and no search result is in the + messages yet (e.g. ``agent_max_rounds=0``). + """ + if SEARCH_TRIGGER not in _user(body).lower(): + return None + if "" not in _system(body): + return None + first_line = _search_result_line(body) + if first_line is not None: + return ("found", first_line) + if not body.get("tools"): + return None + return ("search",) + + def _tool_flow(body: dict[str, Any]) -> tuple[str, ...] | None: """Classify a marker request into one step of the tool flow. @@ -948,6 +1047,25 @@ def chat_completions(body: dict[str, Any]) -> Any: if _chat_dead(RETRY_TRIGGER, RETRY_DEAD_ATTEMPTS): return _llm_500(RETRY_TRIGGER) _fail_posts[RETRY_TRIGGER] = 0 # the answer streamed — restart + # Phase 68 (search tool): the deterministic search marker flow — + # checked BEFORE the phase-37 tool flow (the more specific + # trigger phrase wins, same convention as THINK_PARAS_TRIGGER). + search_flow = _search_flow(body) + if search_flow is not None: + if search_flow[0] == "search": + stream = _tool_call_stream( + "search_documents", {"pattern": SEARCH_PATTERN}, "call_0" + ) + else: # "found" — quote the first matched line (80 chars) + answer = _apply_max_tokens( + f"Found {search_flow[1][:80]}", body.get("max_tokens") + ) + stream = _sse_stream(answer, 0.0) + return StreamingResponse( + stream, + media_type="text/event-stream", + headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, + ) flow = _tool_flow(body) if flow is not None: if flow[0] == "list": diff --git a/tests/e2e/test_search_tool.py b/tests/e2e/test_search_tool.py new file mode 100644 index 0000000..e87d2d3 --- /dev/null +++ b/tests/e2e/test_search_tool.py @@ -0,0 +1,477 @@ +"""Phase 68 E2E (Playwright, mock-only): the ``search_documents`` tool. + +Story: n/a (TODO-derived — the owner roadmap confirmation 2026-09-01, +TODO.md L4: "Add a search tool that allows the LLM to grep through the +uploaded documents for a given string"). +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_search_tool.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the gate is the +deterministic SEARCH marker flow in ``tests/e2e/mock_llm.py`` (user +message contains ``search your documents`` (``SEARCH_TRIGGER``) +**and** the system prompt carries the ```` section of the HIGH +prompt): + +1. request 1 (``tools`` offered, no search result yet) → streams ONLY + ``tool_calls`` deltas calling ``search_documents`` with + ``{"pattern": SEARCH_PATTERN}`` (id ``call_0``); +2. request 2 (a ``tool``-role search result — the + ``source/path:line: text`` match line) → the content answer + ``Found `` — so this + suite can assert the search result reached the model and landed in + the answer. + +KB fixture — ONE document, imported through the real importer (the same +admin-import pipeline the admin Sources page drives) against +``tests/fixtures/search_docs/`` with the deterministic mock embeddings: + +* ``search_docs/reese-notes.md`` — homelab kubernetes backup notes; the + marker question cosines ≈0.51 against it (mock bag-of-words, well + past the E2E 0.30 threshold, and it FTS-matches too) → grounded, so + the HIGH prompt carries the ```` section. Its line 6 carries + the sentinel ``reese-sentinel-42`` exactly once — the deterministic + match of the mock's grep. + +Regression-safe marker: ``search your documents`` appears in NO other +suite's question or fixture text (verified 2026-09-01 by repo grep; +``tests/unit/test_mock_tool_flow.py`` pins that the trigger does not +shadow the phase-37/45 flows and vice versa). + +Test → phase mapping: +1. ``test_search_flow_searches_and_answers_from_match`` — the live + search flow: the SSE carries the ``tool`` frame + (``search_documents`` with ``argument = ``, ahead of any + delta), #send-status recorded the transient "… is searching for + " state, the bubble shows ONE ``🔎 Searching for`` + tool line with the sentinel in a ```` element, the answer + quotes the matched line (``Found …`` — the match reached the + model), and the turn settles to idle with no error banner. +2. ``test_search_adds_no_source_by_itself`` — context accounting + (locked A5): the search-only flow (no read) leaves + ``done.sources`` / the source chips / ``query_log.sources`` at the + retrieval baseline — the search adds no source by itself. +3. ``test_search_tool_line_re_renders_after_reload`` — the persisted + record (phase 14 convention: the generic ``{name, argument}`` + toolAcc) re-renders the search line through the same helper. +""" +from __future__ import annotations + +import asyncio +import json +import re +import time +from pathlib import Path +from threading import Thread +from typing import Any + +from playwright.sync_api import Page, expect +from sqlalchemy import select, text + +from app.config import Settings +from app.db import SessionLocal +from app.models import QueryLog +from app.rag.importer import ImportSummary, import_sources +from app.rag.llm import LLMClient +from tests.e2e.mock_llm import SEARCH_PATTERN, SEARCH_TRIGGER + +REPO = Path(__file__).resolve().parents[2] +FIXTURES = REPO / "tests" / "fixtures" / "search_docs" + +#: The one imported document (the importer derives ``source`` from the +#: fixture root's name and ``path`` from the file's relative position). +SEED_SOURCE = "search_docs" +SEED_PATH = "reese-notes.md" +SEED_SP = f"{SEED_SOURCE}/{SEED_PATH}" + +#: The fixture's sentinel line (line 6) — the mock's grep matches it +#: exactly once, and its ``text`` part is what the "Found …" answer +#: quotes. Pinned against the fixture file itself below. +SENTINEL_LINE = f"The offsite vault passphrase marker is {SEARCH_PATTERN}." +FOUND_ANSWER = f"Found {SENTINEL_LINE[:80]}" + +#: Carries ``SEARCH_TRIGGER`` and is on-topic (cosine ≈0.51 against the +#: fixture + FTS hits → HIGH gate, the ```` section rides along). +SEARCH_QUESTION = ( + "Search your documents for the vault passphrase marker in my homelab " + "kubernetes backup notes?" +) +assert SEARCH_TRIGGER in SEARCH_QUESTION.lower() +# The trigger must not collide with any other mock marker flow. +for other in ( + "use your tools", + "read two documents", + "write a long answer", + "think in paragraphs", + "think out loud", + "show the end of your notes", + "show me a table", + "fail then answer", + "always fail", + "embed fail once", + "pretend to think slowly", +): + assert other not in SEARCH_QUESTION.lower(), other + + +def _pin_fixture() -> None: + """The fixture carries the sentinel on line 6, exactly once.""" + content = (FIXTURES / SEED_PATH).read_text(encoding="utf-8") + lines = content.split("\n") + assert lines[5] == SENTINEL_LINE, lines[5] + assert sum(SEARCH_PATTERN in line for line in lines) == 1 + + +# -------------------------------------------------------------------------- +# DB seeding (TRUNCATE-then-import via the real importer, cf. +# test_chat_rag.py — the same pipeline the admin Sources import drives) +# -------------------------------------------------------------------------- + + +async def _import_fixtures(mock_port: int) -> ImportSummary: + kwargs: dict[str, Any] = {"_env_file": None, "llm_base_url": f"http://127.0.0.1:{mock_port}/v1"} + settings = Settings(**kwargs) # pyright: ignore[reportCallIssue] + return await import_sources([FIXTURES], LLMClient(settings)) + + +def _run_in_thread(coro: Any) -> Any: + """Run a coroutine on a worker thread. + + Playwright's sync API keeps an asyncio loop running on the test + thread, so ``asyncio.run`` cannot be called directly from a test + body (the established house helper). + """ + box: dict[str, Any] = {} + + def runner() -> None: + try: + box["value"] = asyncio.run(coro) + except BaseException as e: # noqa: BLE001 — re-raised on the test thread + box["error"] = e + + t = Thread(target=runner) + t.start() + t.join() + if "error" in box: + raise box["error"] + return box["value"] + + +def _reset_db(mock_port: int) -> ImportSummary: + """Truncate the KB (plus the prompt-shaping tables), then re-import. + + ``steering_notes`` / ``kb_overview`` are truncated too, so the HIGH + prompt is exactly ```` + ```` + ```` + regardless of leftovers from other suites — byte-stable prompts, + byte-stable answers.""" + with SessionLocal() as db: + db.execute( + text("TRUNCATE chunks, documents, query_log, steering_notes, kb_overview") + ) + db.commit() + summary = _run_in_thread(_import_fixtures(mock_port)) + assert summary is not None and summary.added == 1, summary + return summary + + +def _last_query_log() -> QueryLog: + with SessionLocal() as db: + rows = db.scalars(select(QueryLog)).all() + assert len(rows) == 1, f"expected exactly one query_log row, got {len(rows)}" + return rows[0] + + +# -------------------------------------------------------------------------- +# Page helpers (the test_agent_document_tools.py pattern) +# -------------------------------------------------------------------------- + +#: Records every value #send-label takes during the turn (a +#: MutationObserver on the element), so the in-flight label state is +#: captured deterministically — no polling race. Phase 48: the label is +#: the Send↔Stop morph ("Stop" holds for the whole in-flight turn). +LABEL_RECORDER = """ +() => { + if (window.__labelsInstalled) return; + window.__labelsInstalled = true; + window.__labels = []; + const el = document.querySelector('#send-label'); + if (!el) return; + const rec = (v) => { + const l = window.__labels; + if (!l.length || l[l.length - 1] !== v) l.push(v); + }; + rec(el.textContent); + new MutationObserver(() => rec(el.textContent)).observe(el, { + childList: true, + subtree: true, + }); +} +""" + +#: Records every value #send-status takes during the turn — the +#: transient "… is searching for " calling-tool state is held +#: only from the first `tool` frame until the first answer delta, so the +#: pre-submit observer is the deterministic source of truth for it +#: (no polling race — the phase-44 task-03 flake fix). +STATUS_RECORDER = """ +() => { + if (window.__statusesInstalled) return; + window.__statusesInstalled = true; + window.__statuses = []; + const el = document.querySelector('#send-status'); + if (!el) return; + const rec = (v) => { + const l = window.__statuses; + if (!l.length || l[l.length - 1] !== v) l.push(v); + }; + rec(el.textContent); + new MutationObserver(() => rec(el.textContent)).observe(el, { + childList: true, + subtree: true, + }); +} +""" + +#: Captures the raw SSE ``data:`` payloads of the /api/chat stream +#: (a response clone read in the background) — wire-level assertions +#: for the ``tool`` frames, independent of the UI rendering. +SSE_HOOK = """ +() => { + if (window.__sseInstalled) return; + window.__sseInstalled = true; + window.__sseFrames = []; + const origFetch = window.fetch; + window.fetch = async function (...args) { + const res = await origFetch.apply(this, args); + try { + const url = typeof args[0] === 'string' ? args[0] : args[0].url; + if (url.includes('/api/chat')) { + res.clone().text().then((bodyText) => { + for (const block of bodyText.split('\\n\\n')) { + const line = block.trim(); + if (line.startsWith('data: ')) { + window.__sseFrames.push(line.slice(6)); + } + } + }); + } + } catch (e) { /* non-clonable responses: ignored */ } + return res; + }; +} +""" + + +def _install_page_hooks(page: Page) -> None: + """Install all hooks on the loaded page (post-goto, pre-submit).""" + page.evaluate(SSE_HOOK) + page.evaluate(LABEL_RECORDER) + page.evaluate(STATUS_RECORDER) + + +def _frames(page: Page) -> list[dict]: + """The captured SSE frames, once the hook's background read settles. + + The hook reads ``res.clone().text()`` in a background promise that + resolves right after the stream closes — poll briefly until the + final ``done`` frame lands (fail loud if the hook captured nothing). + """ + deadline = time.monotonic() + 10.0 + while True: + raw = page.evaluate("() => window.__sseFrames || []") + parsed = [json.loads(line) for line in raw if line] + if any(f.get("type") == "done" for f in parsed): + return parsed + if time.monotonic() > deadline: + raise AssertionError( + f"SSE hook captured no `done` frame (frames so far: " + f"{len(parsed)}) — hook install failed?" + ) + time.sleep(0.05) + + +def _tool_frames(frames: list[dict]) -> list[dict]: + return [f for f in frames if f.get("type") == "tool"] + + +def _submit(page: Page, question: str) -> None: + page.fill("#message-input", question) + page.click("#send-btn") + # The user bubble lands synchronously with the submit handler. + expect(page.locator(".msg.user .bubble").last).to_contain_text(question) + + +def _wait_settled(page: Page) -> None: + """The turn is complete: answer text in the bubble, button recovered. + + Phase 48: the label assertion carries the settle wait with an + explicit timeout — the in-flight button is the enabled Stop control + (never disabled), so ``to_be_enabled`` no longer blocks until the + turn settles, and Playwright expect's default (5s) does not inherit + the page default.""" + expect(page.locator(".msg.brain .bubble").last).not_to_have_text("", timeout=30_000) + expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) + expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) + + +def _assert_no_error_banner(page: Page) -> None: + """The turn must settle WITHOUT the terminal error banner (the red + role=alert error banner — the KB-offline banner is a separate, + non-error state).""" + banner = page.locator("#kb-banner") + expect(banner).to_be_hidden() + expect(banner).not_to_have_attribute("role", "alert") + expect(banner).not_to_have_class(re.compile(r"is-error")) + + +# -------------------------------------------------------------------------- +# 1. The live search flow: search → matched line → "Found …" answer +# -------------------------------------------------------------------------- + + +def test_search_flow_searches_and_answers_from_match( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _pin_fixture() + page.set_default_timeout(30_000) + _reset_db(mock_llm) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, SEARCH_QUESTION) + # The "calling tool" STATUS window is transient: the first `tool` + # frame sets #send-status and it holds until the FIRST answer delta + # (the agent loop completes before the answer stream) — a polling + # expect can stride straight over that window (the phase-44 flake), + # so the pre-submit MutationObserver records below are the + # deterministic source of truth for the status transitions. + _wait_settled(page) + + # Phase 48 (owner-locked): the in-flight button is the Stop control + # for the whole turn; #send-status walked "… is thinking" → + # "… is searching for ", in order. + labels = page.evaluate("() => window.__labels") + assert "Stop" in labels, labels + statuses = page.evaluate("() => window.__statuses") + i_search = next( + ( + i + for i, s in enumerate(statuses) + if f"is searching for {SEARCH_PATTERN}" in s + ), + None, + ) + assert i_search is not None, statuses + i_think = next( + (i for i, s in enumerate(statuses) if "is thinking" in s), None + ) + assert i_think is not None and i_think < i_search, statuses + + # Wire level: exactly ONE `tool` frame — search_documents carrying + # the PATTERN as its argument (phase 68 task 02) — ahead of the + # first `delta` frame. + frames = _frames(page) + assert _tool_frames(frames) == [ + {"type": "tool", "name": "search_documents", "argument": SEARCH_PATTERN} + ] + first_delta = next(i for i, f in enumerate(frames) if f.get("type") == "delta") + assert all( + i < first_delta for i, f in enumerate(frames) if f.get("type") == "tool" + ) + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is False + + # ONE tool line above the answer: "🔎 Searching for " + the sentinel + # in a element (the pattern is data, never markup). + lines = page.locator(".msg.brain .tool-call") + expect(lines).to_have_count(1) + expect(lines.nth(0)).to_contain_text("Searching for") + expect(lines.nth(0).locator("code")).to_have_text(SEARCH_PATTERN) + + # The answer quotes the MATCHED LINE — the search result reached the + # model and landed in the answer (the mock's deterministic echo). + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(FOUND_ANSWER) + + # The turn settled to idle WITHOUT the error banner, and the durable + # record is grounded with the retrieval doc only (see test 2 for the + # sources contract). + _assert_no_error_banner(page) + row = _last_query_log() + assert row.question == SEARCH_QUESTION + assert row.deflected is False + + +# -------------------------------------------------------------------------- +# 2. Context accounting (locked A5): a search adds no source by itself +# -------------------------------------------------------------------------- + + +def test_search_adds_no_source_by_itself( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _pin_fixture() + page.set_default_timeout(30_000) + _reset_db(mock_llm) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, SEARCH_QUESTION) + _wait_settled(page) + + # The search really ran (its wire frame is present) — yet the + # search-only flow (no read) leaves done.sources at the RETRIEVAL + # baseline: the one fixture doc, nothing added by the search. + frames = _frames(page) + assert _tool_frames(frames) == [ + {"type": "tool", "name": "search_documents", "argument": SEARCH_PATTERN} + ] + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is False + assert [(s["source"], s["path"]) for s in done["sources"]] == [ + (SEED_SOURCE, SEED_PATH) + ] + + # UI: exactly one source chip — the retrieval doc (the search + # renders no chip of its own). + chips = page.locator(".msg.brain .source-chip") + expect(chips).to_have_count(1) + expect(chips.nth(0)).to_contain_text(SEED_SP) + + # Durable record: the sources row is unchanged by the search alone. + row = _last_query_log() + assert row.deflected is False + assert row.sources == SEED_SP + + +# -------------------------------------------------------------------------- +# 3. Persistence: the search tool line re-renders after a reload +# -------------------------------------------------------------------------- + + +def test_search_tool_line_re_renders_after_reload( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _pin_fixture() + page.set_default_timeout(30_000) + _reset_db(mock_llm) + page.goto(app_url) + + _submit(page, SEARCH_QUESTION) + _wait_settled(page) + expect(page.locator(".msg.brain .tool-call")).to_have_count(1) + + page.reload() + expect(page.locator("#empty-state")).to_be_hidden() + + # The persisted record (the generic {name, argument} toolAcc — + # phase 14 convention) re-renders the search line through the same + # append helper as the live frames: sentinel in a element. + restored = page.locator(".msg.brain .tool-call") + expect(restored).to_have_count(1) + expect(restored.nth(0)).to_contain_text("Searching for") + expect(restored.nth(0).locator("code")).to_have_text(SEARCH_PATTERN) + + # The answer (the matched-line echo) is intact after the restore. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(FOUND_ANSWER) diff --git a/tests/fixtures/search_docs/reese-notes.md b/tests/fixtures/search_docs/reese-notes.md new file mode 100644 index 0000000..e143ef2 --- /dev/null +++ b/tests/fixtures/search_docs/reese-notes.md @@ -0,0 +1,9 @@ +# Homelab Backup Notes + +The homelab kubernetes cluster's etcd volume is backed up nightly to the +offsite vault; the backup cron runs on homelab-gw at 03:00. + +The offsite vault passphrase marker is reese-sentinel-42. + +The restore procedure is in the homelab runbook; the vault key rotates +every ninety days. diff --git a/tests/integration/test_agent_tools.py b/tests/integration/test_agent_tools.py index e1717bd..c1ee48e 100644 --- a/tests/integration/test_agent_tools.py +++ b/tests/integration/test_agent_tools.py @@ -3,21 +3,31 @@ ``list_catalog`` must order rows by ``(source, path)`` — the same order as ``GET /api/docs`` — and ``find_document`` must resolve a hit to the full document row (content included, for the never-truncated read) and return -``None`` for unknown ``source``/``path`` pairs. +``None`` for unknown ``source``/``path`` pairs. Phase 68: the +``search_documents`` tool is pinned here too — its locked parameter +shape in ``AGENT_TOOLS``, and a scripted ``ToolCallPiece`` executed +through ``run_agent`` against the real DB (``all_documents`` for a +whole-KB search, ``find_document`` for a scoped one). Requires: podman compose up -d db """ from __future__ import annotations +import asyncio import uuid -from collections.abc import Iterator +from collections.abc import AsyncIterator, Iterator +from copy import deepcopy +from typing import Any, cast import pytest from sqlalchemy import text from sqlalchemy.orm import Session +from app.config import Settings from app.models import Document from app.rag import agent +from app.rag.agent import AGENT_TOOLS, AgentHolder, run_agent +from app.rag.llm import LLMClient, RetryPiece, StreamPiece, ToolCallPiece def _doc(db: Session, source: str, path: str, title: str, content: str) -> Document: @@ -81,3 +91,152 @@ def test_find_document_none_for_unknown_pairs(kb, db) -> None: assert agent.find_document(db, "Alpha", "nope.md") is None # wrong path assert agent.find_document(db, "Beta", "x.md") is None # wrong source assert agent.find_document(db, "nope", "nope.md") is None # nothing at all + + +# ---------- search_documents (phase 68) ---------- + + +def test_all_documents_orders_by_source_then_path(kb, db) -> None: + _doc(db, "Zeta", "b/second.md", "Zeta B", "ZB") + _doc(db, "Zeta", "a/first.md", "Zeta A", "ZA") + _doc(db, "Alpha", "c/third.md", "Alpha C", "AC") + db.commit() + + docs = agent.all_documents(db) + assert [(d.source, d.path) for d in docs] == [ + ("Alpha", "c/third.md"), + ("Zeta", "a/first.md"), + ("Zeta", "b/second.md"), + ] + assert [d.content for d in docs] == ["AC", "ZA", "ZB"] # full rows + + +def test_agent_tools_offers_search_documents_with_locked_shape() -> None: + by_name = {t["function"]["name"]: t for t in AGENT_TOOLS} + assert list(by_name) == [ # the third tool, in order + "list_documents", + "read_document", + "search_documents", + ] + search = by_name["search_documents"]["function"]["parameters"] + assert search["type"] == "object" + assert search["required"] == ["pattern"] + assert set(search["properties"]) == {"pattern", "source", "path"} + assert all(p["type"] == "string" for p in search["properties"].values()) + + +class ScriptedToolLLM: + """One scripted tool-call stream, then one canned answer stream. + Records every ``chat_stream`` request's messages and tools.""" + + def __init__(self, call: ToolCallPiece) -> None: + self.call = call + self.requests: list[ + tuple[list[dict[str, Any]], list[dict[str, Any]] | None] + ] = [] + + async def chat_stream( + self, + messages: list[dict[str, str]], + tools: list[dict[str, Any]] | None = None, + ) -> AsyncIterator[StreamPiece | ToolCallPiece]: + self.requests.append((deepcopy(messages), deepcopy(tools))) + if len(self.requests) == 1: + yield self.call + else: + yield StreamPiece("content", "ans") + + +def _settings(**kwargs: Any) -> Settings: + kwargs.setdefault("_env_file", None) + return Settings(**kwargs) # pyright: ignore[reportCallIssue] + + +def _run_search( + db: Session, arguments: dict[str, Any] +) -> tuple[AgentHolder, ScriptedToolLLM]: + """Drive one scripted ``search_documents`` call through ``run_agent``.""" + holder = AgentHolder() + llm = ScriptedToolLLM( + ToolCallPiece(id="call_1", name="search_documents", arguments=arguments) + ) + asyncio.run(_consume(llm, db, holder)) + return holder, llm + + +async def _consume( + llm: ScriptedToolLLM, db: Session, holder: AgentHolder +) -> list[StreamPiece | ToolCallPiece | RetryPiece]: + out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] + async for piece in run_agent( + cast("LLMClient", llm), + db, + system_prompt="SYSTEM_PROMPT", + user_message="QUESTION", + seed_docs=[], + settings=_settings(), + holder=holder, + ): + out.append(piece) + return out + + +def test_search_whole_kb_through_run_agent(kb, db) -> None: + _doc(db, "Beta", "b/two.md", "Two", "no hit\nNEEDLE in two\nlast") + _doc(db, "Alpha", "a/one.md", "One", "first\nneedle in one\nthird") + db.commit() + + holder, llm = _run_search(db, {"pattern": "needle"}) + + # Offered: the first request carries AGENT_TOOLS (the 3-tool list). + assert llm.requests[0][1] == AGENT_TOOLS + # Executed against the real DB: catalog order, grep-style lines. + assert llm.requests[1][0][3]["content"] == ( + "Alpha/a/one.md:2: needle in one\n" + "Beta/b/two.md:2: NEEDLE in two" + ) + assert holder.tool_calls == 1 + assert holder.read_docs == [] # locked A5: search adds no context + + +def test_search_scoped_through_run_agent(kb, db) -> None: + _doc(db, "Alpha", "a/one.md", "One", "first\nNeedle here\nthird") + _doc(db, "Beta", "b/two.md", "Two", "NEEDLE too") + db.commit() + + holder, llm = _run_search( + db, {"pattern": "needle", "source": "Alpha", "path": "a/one.md"} + ) + + # Only the named document is searched — the other one's hit is absent. + assert llm.requests[1][0][3]["content"] == "Alpha/a/one.md:2: Needle here" + assert holder.tool_calls == 1 + assert holder.read_docs == [] + + +def test_search_scoped_missing_doc_refused_through_run_agent(kb, db) -> None: + _doc(db, "Alpha", "a/one.md", "One", "nothing") + db.commit() + + holder, llm = _run_search( + db, {"pattern": "needle", "source": "Alpha", "path": "ghost.md"} + ) + + assert ( + llm.requests[1][0][3]["content"] + == "No document at Alpha/ghost.md — check the list_documents output." + ) + assert holder.tool_calls == 0 and holder.read_docs == [] + + +def test_search_no_matches_through_run_agent(kb, db) -> None: + _doc(db, "Alpha", "a/one.md", "One", "nothing matching") + db.commit() + + holder, llm = _run_search(db, {"pattern": "zebra"}) + + assert llm.requests[1][0][3]["content"] == ( + "No matches for 'zebra' in the knowledge base." + ) + assert holder.tool_calls == 1 # an executed search with zero hits + assert holder.read_docs == [] diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index 19ab7eb..61bc366 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -298,17 +298,20 @@ def test_ui_chrome_has_no_emoji(client, path: str) -> None: the JS that renders it, and the stylesheet — is emoji-free. Phase 37 revision (owner permission 2026-08-26, PLAN §4): the agent's - ``.tool-call`` line carries two CONTENT marks — 🔎 (list) and 📄 + ``.tool-call`` line carries the CONTENT marks — 🔎 (list) and 📄 (read) — the only emoji in the whole frontend, and only as the exact - tool-line template strings in app.js. The guard strips precisely - those two literals; any other emoji, or those marks anywhere else, - still fails.""" + tool-line template strings in app.js. Phase 68 revision: the + ``search_documents`` tool line adds the third template literal + ("🔎 Searching for "). The guard strips precisely those three + literals; any other emoji, or those marks anywhere else, still + fails.""" r = client.get(path) assert r.status_code == 200 text = r.text if path in ("/assets/app.js", "/assets/shared.js"): text = text.replace('"🔎 Listing documents"', "") text = text.replace('"📄 Reading "', "") + text = text.replace('"🔎 Searching for "', "") assert _find_emoji(text) == [], f"emoji found in {path}: {_find_emoji(text)!r}" diff --git a/tests/integration/test_chat_api.py b/tests/integration/test_chat_api.py index e542bde..8bdee06 100644 --- a/tests/integration/test_chat_api.py +++ b/tests/integration/test_chat_api.py @@ -621,6 +621,65 @@ 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( + client, db, seeded_kb: FakeRagLLM +) -> None: + """Phase 68: a scripted ``search_documents`` call streams as + ``{type: "tool", name: "search_documents", argument: }`` — + 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 + 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_2", + name="search_documents", + arguments={"pattern": 42}, # model error: non-string + ), + ], + # the answer request still carries the tools (2 rounds < the + # default cap of 10); the fake's tool_script is exhausted, so + # it falls back to the thinking + answer stream + ] + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: scripted + try: + _, _, frames = _stream_chat(client, QUESTION) + finally: + fastapi_app.dependency_overrides.clear() + + types = [f["type"] for f in frames] + assert "error" not in types + assert len(scripted.seen_tools) == 3 # both searches 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["argument"] == "Cilium" # the raw pattern + assert set(second) == {"type", "name", "argument"} + assert second["name"] == "search_documents" + assert second["argument"] is None # the non-string pattern → null + + # The searches 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 + + def test_deflected_turn_stays_byte_identical_without_tools( client, db, seeded_kb: FakeRagLLM ) -> None: diff --git a/tests/unit/test_agent.py b/tests/unit/test_agent.py index 031796d..8979352 100644 --- a/tests/unit/test_agent.py +++ b/tests/unit/test_agent.py @@ -103,7 +103,8 @@ async def _run( def test_agent_tools_names_and_parameters() -> None: by_name = {t["function"]["name"]: t for t in AGENT_TOOLS} - assert set(by_name) == {"list_documents", "read_document"} + assert len(AGENT_TOOLS) == 3 # list / read / search (phase 68) + assert set(by_name) == {"list_documents", "read_document", "search_documents"} assert all(t["type"] == "function" for t in AGENT_TOOLS) list_params = by_name["list_documents"]["function"]["parameters"] assert list_params["type"] == "object" @@ -128,6 +129,34 @@ def test_agent_tools_names_and_parameters() -> None: "list_documents output (e.g. 'homelab/aws-route53.md' from " "'source: Homelab | path: homelab/aws-route53.md')." ) + # Phase 68: search_documents — the third tool, a locator (locked A5). + search = by_name["search_documents"]["function"] + assert search["description"] == ( + "Search every indexed document for an exact string " + "(case-insensitive) and return up to 20 matching lines as " + "'source/path:line: text' — use this to locate content, " + "then read_document the winner. Optionally pass 'source' " + "and 'path' (as shown in list_documents) to search one " + "document only." + ) + search_params = search["parameters"] + assert search_params["type"] == "object" + assert search_params["required"] == ["pattern"] + assert set(search_params["properties"]) == {"pattern", "source", "path"} + assert search_params["properties"]["pattern"]["description"] == ( + "The exact text to search for (a plain substring, not a regex)" + ) + # Phase 63 labeled-field wording, same as read_document's parameters. + assert search_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 search_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 ---------- @@ -547,6 +576,294 @@ def test_read_document_missing_arguments_refused( assert llm.requests[1][1] == AGENT_TOOLS +# ---------- search_documents (phase 68, locked A5/A6) ---------- + + +def test_grep_document_case_insensitive_line_numbers() -> None: + """Case-insensitive fixed substring, 1-based line numbers, file order, + repeated matches within a line collapse to one match (grep semantics).""" + content = "The NEEDLE is here\nno hit\nneedle again\nNEEDLE NEEDLE\n" + assert agent.grep_document(content, "NEEDLE") == [ + (1, "The NEEDLE is here"), + (3, "needle again"), + (4, "NEEDLE NEEDLE"), + ] + + +def test_grep_document_rstrips_lines_and_empty_content() -> None: + assert agent.grep_document("hello \t\nworld ", "WORLD") == [(2, "world")] + assert agent.grep_document("", "x") == [] + assert agent.grep_document("no newlines", "NO") == [(1, "no newlines")] + assert agent.grep_document("a\nb\n", "MISSING") == [] + + +def test_search_whole_kb_grep_style_output( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Whole-KB search: catalog order, `source/path:line: text` lines, + case-insensitive; the call counts in ``tool_calls`` and never touches + ``read_docs``; the tools stay offered on the answer request.""" + d1 = _doc("Alpha", "a/one.md", "One", "first\nNEEDLE in one\nlast") + d2 = _doc("Beta", "b/two.md", "Two", "no hit\nneedle in two\n") + monkeypatch.setattr(agent, "all_documents", lambda db: [d1, d2]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", name="search_documents", arguments={"pattern": "needle"} + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == ( + "Alpha/a/one.md:2: NEEDLE in one\n" + "Beta/b/two.md:2: needle in two" + ) + assert holder.tool_calls == 1 + assert holder.read_docs == [] # locked A5: a search adds no context + assert llm.requests[1][1] == AGENT_TOOLS # tools stay offered + + +def test_search_capped_at_20_matches_in_catalog_order( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The 20-match cap is GLOBAL across documents in catalog order, and + the scan stops once it is hit (a 35-match corpus yields exactly 20).""" + d1 = _doc("S", "a.md", "A", "\n".join(f"hit-{i}" for i in range(15))) + d2 = _doc("S", "b.md", "B", "\n".join(f"hit-{i}" for i in range(20))) + monkeypatch.setattr(agent, "all_documents", lambda db: [d1, d2]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", name="search_documents", arguments={"pattern": "hit-"} + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + lines = llm.requests[1][0][3]["content"].split("\n") + assert len(lines) == agent.SEARCH_MAX_MATCHES + assert lines[0] == "S/a.md:1: hit-0" + assert lines[14] == "S/a.md:15: hit-14" # all of a.md + assert lines[15] == "S/b.md:1: hit-0" # then b.md, in order + assert lines[19] == "S/b.md:5: hit-4" # cut at the global cap + assert holder.tool_calls == 1 + + +def test_search_truncates_match_lines_at_200_chars( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A 300-char match line yields exactly 200 chars of it (no crash).""" + d1 = _doc("S", "a.md", "A", "top\n" + "x" * 300 + " NEEDLE tail") + monkeypatch.setattr(agent, "all_documents", lambda db: [d1]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", name="search_documents", arguments={"pattern": "needle"} + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert ( + llm.requests[1][0][3]["content"] == f"S/a.md:2: {'x' * agent.SEARCH_LINE_LIMIT}" + ) + assert holder.tool_calls == 1 + + +def test_search_scoped_to_one_document(monkeypatch: pytest.MonkeyPatch) -> None: + """Scoped search: only the named document is loaded (find_document), + ``all_documents`` never runs, and the match line carries its path.""" + d1 = _doc("S", "a.md", "A", "needle here") + + def _find(db: Any, source: str, path: str) -> Document | None: + if (source, path) == ("S", "a.md"): + return d1 + raise AssertionError( + f"find_document({source}, {path}) — the scoped " + "search must not load any other document" + ) + + def _boom(*_a: Any, **_k: Any) -> None: + raise AssertionError("all_documents must not run for a scoped search") + + monkeypatch.setattr(agent, "find_document", _find) + monkeypatch.setattr(agent, "all_documents", _boom) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="search_documents", + arguments={"pattern": "needle", "source": "S", "path": "a.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == "S/a.md:1: needle here" + assert holder.tool_calls == 1 + assert holder.read_docs == [] # searched doc did not enter the context + + +def test_search_scoped_missing_document_refused( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="search_documents", + arguments={"pattern": "x", "source": "S", "path": "ghost.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert ( + llm.requests[1][0][3]["content"] + == "No document at S/ghost.md — check the list_documents output." + ) + assert holder.tool_calls == 0 and holder.read_docs == [] # a refusal + + +@pytest.mark.parametrize( + ("arguments", "label"), + [ + ({}, "no arguments"), + ({"pattern": ""}, "empty pattern"), + ({"pattern": " "}, "whitespace pattern"), + ({"pattern": 42}, "non-string pattern"), + ({"pattern": None}, "null pattern"), + ({"pattern": "x", "source": "S"}, "source without path"), + ({"pattern": "x", "path": "a.md"}, "path without source"), + ], +) +def test_search_missing_arguments_refused( + monkeypatch: pytest.MonkeyPatch, arguments: dict[str, Any], label: str +) -> None: + """Unusable pattern OR a half-specified source/path pair → the + missing-args refusal, with no DB access at all.""" + + def _boom(*_a: Any, **_k: Any) -> None: + raise AssertionError(f"no DB access for a refused search ({label})") + + monkeypatch.setattr(agent, "all_documents", _boom) + monkeypatch.setattr(agent, "find_document", _boom) + holder = AgentHolder() + llm = ScriptedLLM( + [ToolCallPiece(id="call_1", name="search_documents", arguments=arguments)], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == agent.MISSING_SEARCH_ARGS + assert holder.tool_calls == 0 and holder.read_docs == [] + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + +def test_search_no_matches_whole_kb(monkeypatch: pytest.MonkeyPatch) -> None: + """Zero hits across the KB → the no-match line (pattern quoted); the + search still executed, so it counts — and never adds context.""" + monkeypatch.setattr( + agent, "all_documents", lambda db: [_doc("S", "a.md", "A", "nothing here")] + ) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", name="search_documents", arguments={"pattern": "zebra"} + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == ( + "No matches for 'zebra' in the knowledge base." + ) + assert holder.tool_calls == 1 + assert holder.read_docs == [] + + +def test_search_no_matches_scoped(monkeypatch: pytest.MonkeyPatch) -> None: + doc = _doc("S", "a.md", "A", "nothing here") + monkeypatch.setattr(agent, "find_document", lambda db, source, path: doc) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="search_documents", + arguments={"pattern": "zebra", "source": "S", "path": "a.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == "No matches for 'zebra' in S/a.md." + assert holder.tool_calls == 1 + assert holder.read_docs == [] + + +def test_search_no_match_truncates_long_pattern( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A pattern longer than 100 chars is truncated in the no-match line + (kept short); the search itself still runs on the full pattern.""" + monkeypatch.setattr(agent, "all_documents", lambda db: []) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="search_documents", + arguments={"pattern": "p" * 150}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == ( + f"No matches for '{'p' * 100}' in the knowledge base." + ) + assert holder.tool_calls == 1 + + +def test_search_counts_but_never_adds_context( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The locate-then-read workflow: a search finds the document but does + NOT add it — the subsequent read_document does (and is not rejected as + already-in-context, because the search touched nothing).""" + doc = _doc("S", "a.md", "A", "needle here") + monkeypatch.setattr(agent, "all_documents", lambda db: [doc]) + monkeypatch.setattr(agent, "find_document", lambda db, source, path: doc) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", name="search_documents", arguments={"pattern": "needle"} + ) + ], + [ + ToolCallPiece( + id="call_2", + name="read_document", + arguments={"source": "S", "path": "a.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.tool_calls == 2 # search + read, both executed + assert holder.read_docs == [doc] # only the read added context (A5) + assert llm.requests[2][0][5]["content"] == "Document S/a.md:\nneedle here" + + # ---------- retries inside the agent loop (phase 67, locked A2) ---------- diff --git a/tests/unit/test_frontend_tool_states.py b/tests/unit/test_frontend_tool_states.py index 21130f2..0c923d1 100644 --- a/tests/unit/test_frontend_tool_states.py +++ b/tests/unit/test_frontend_tool_states.py @@ -5,7 +5,8 @@ No new Python app logic exists for this task — the behavior lives in suite (task 06). Like the other frontend-adjacent unit files, this module pins the JS/CSS markers the story depends on, so a silent regression in the tool branch, the persistence shape, or the tool-line styling is -caught without a browser. +catched without a browser. Phase 68 extends the pins with the +``search_documents`` status/line contract. """ from __future__ import annotations @@ -39,7 +40,11 @@ def test_tool_branch_is_a_first_class_turn_branch() -> None: "the turn handler must branch on tool frames" ) branch = js[tool_idx:delta_idx] - assert "toolAcc.push" in branch, "every tool frame is recorded for persistence" + assert "toolAcc.push({ name, argument })" in branch, ( + "every tool frame is recorded for persistence — the record stays" + " {name, argument}-generic, no per-tool shape (phase 68: the" + " search tool rides the same accumulator)" + ) assert "clearTurnTimeout()" in branch, "a tool frame proves the stream is alive" assert 'addMessage("brain", "")' in branch, "first frame creates the brain wrap" assert "uiState === UI_STATE.thinking" in branch, ( @@ -69,6 +74,16 @@ def test_calling_tool_label_strings() -> None: assert "sendLabel" not in branch, "phase 48: the button keeps its Stop label" assert "`${brand()} is listing documents`" in branch assert "`${brand()} is reading ${argument}`" in branch + # Phase 68: the search status — locked name+argument gate, sitting + # BETWEEN the read branch and the listing fallback in the ternary. + assert "name === \"search_documents\" && argument" in branch, ( + "the search status requires the name AND a string argument" + ) + assert "`${brand()} is searching for ${argument}`" in branch + read = branch.find("is reading") + search = branch.find("is searching for") + listing = branch.find("is listing documents") + assert -1 < read < search < listing, "ternary order: read → search → listing" assert "sendStatus.textContent = toolStatus" in branch, ( "the #send-status live region announces what Brain is doing" ) @@ -106,6 +121,21 @@ def test_tool_lines_render_into_the_bubble_wrap() -> None: "the path is data — textContent, never innerHTML" ) assert "name === \"read_document\" && argument" in body + # Phase 68: the search branch mirrors the read branch — the same + # name+argument gate, a element, and the pattern through + # textContent (never markup); the listing stays the final else. + assert "name === \"search_documents\" && argument" in body + assert 'line.textContent = "🔎 Searching for "' in body + search_part = body.split('name === "search_documents"', 1)[1] + assert 'document.createElement("code")' in search_part, ( + "the pattern gets the same treatment as the read path" + ) + assert "code.textContent = argument" in search_part, ( + "the pattern is data — textContent, never innerHTML" + ) + assert 'line.textContent = "🔎 Listing documents"' in search_part, ( + "the listing fallback remains the final else" + ) def test_tool_branch_is_append_only_and_interleaving_safe() -> None: diff --git a/tests/unit/test_mock_tool_flow.py b/tests/unit/test_mock_tool_flow.py index 3c16644..10608c6 100644 --- a/tests/unit/test_mock_tool_flow.py +++ b/tests/unit/test_mock_tool_flow.py @@ -17,7 +17,10 @@ from typing import Any from tests.e2e.mock_llm import ( MULTI_READ_TRIGGER, + SEARCH_PATTERN, + SEARCH_TRIGGER, TOOLS_TRIGGER, + _search_flow, _tool_flow, ) @@ -256,3 +259,95 @@ def test_multi_trigger_without_tools_trigger_is_none() -> None: def test_multi_flow_requires_tools_section() -> None: assert _tool_flow(_body(MULTI_USER, system=SYSTEM_LOW)) is None + + +# -------------------------------------------------------------------------- +# Phase-68 search flow (task 03) +# -------------------------------------------------------------------------- + +#: Carries ONLY the search trigger (never ``use your tools`` — the +#: phase-68 suite's live question shape, regression-safe by assertion). +SEARCH_USER = ( + "Search your documents for the vault passphrase marker in my homelab " + "kubernetes backup notes?" +) +assert SEARCH_TRIGGER in SEARCH_USER.lower() +assert TOOLS_TRIGGER not in SEARCH_USER.lower() + +#: The agent's ``search_documents`` result for the e2e fixture +#: (``app/rag/agent.py`` ``_execute_tool``): one ``source/path:LINE: text`` +#: match line (the sentinel line, 200-char-capped server-side). +SEARCH_RESULT = ( + f"search_docs/reese-notes.md:6: The offsite vault passphrase marker " + f"is {SEARCH_PATTERN}." +) +#: The agent's no-match line quotes the pattern — the sentinel-only +#: shape ``_search_result_line`` also recognizes (degenerate path). +SEARCH_NO_MATCH = f"No matches for '{SEARCH_PATTERN}' in the knowledge base." + + +def test_search_flow_search_step() -> None: + # tools offered, no search result yet: the model greps. + assert _search_flow(_body(SEARCH_USER)) == ("search",) + + +def test_search_flow_search_step_requires_tools_offered() -> None: + # agent_max_rounds=0 path: trigger + prompt, but no tools + # and no search result — regular answer, not a flow. + assert _search_flow(_body(SEARCH_USER, tools=None)) is None + + +def test_search_flow_found_step_quotes_first_match_line() -> None: + flow = _search_flow(_body(SEARCH_USER, (SEARCH_RESULT,))) + assert flow == ("found", f"The offsite vault passphrase marker is {SEARCH_PATTERN}.") + + +def test_search_flow_found_step_with_nested_path() -> None: + # A nested path (``/`` in it) stays intact in the match-line parse. + result = f"search_docs/deep/nested-note.md:12: line with {SEARCH_PATTERN} inside" + flow = _search_flow(_body(SEARCH_USER, (result,))) + assert flow == ("found", f"line with {SEARCH_PATTERN} inside") + + +def test_search_flow_found_step_without_tools_offered() -> None: + # The answer is content, not a tool call — it must not be gated on + # the ``tools`` parameter (phase 45 keeps the tools offered until + # the round cap, but the no-tools final request must still answer). + flow = _search_flow(_body(SEARCH_USER, (SEARCH_RESULT,), tools=None)) + assert flow == ("found", f"The offsite vault passphrase marker is {SEARCH_PATTERN}.") + + +def test_search_flow_ignores_catalog_and_read_results() -> None: + # A catalog (labeled lines) and a read result ("Document …" prefix) + # are NOT search results — the flow stays at the search step. + flow = _search_flow(_body(SEARCH_USER, (CATALOG_2, _read_result(DOC1_SP, DOC1_CONTENT)))) + assert flow == ("search",) + + +def test_search_flow_sentinel_only_result_is_a_search_result() -> None: + # The no-match line quotes the pattern — sentinel-only recognition + # (degenerate path; the e2e fixture always matches). + flow = _search_flow(_body(SEARCH_USER, (SEARCH_NO_MATCH,))) + assert flow == ("found", SEARCH_NO_MATCH) + + +def test_search_flow_requires_tools_section() -> None: + # Deflected turns never carry the section. + assert _search_flow(_body(SEARCH_USER, system=SYSTEM_LOW)) is None + + +def test_search_flow_plain_question_is_none() -> None: + assert _search_flow(_body(PLAIN_USER)) is None + + +def test_search_trigger_does_not_shadow_the_tool_flow() -> None: + # The search question carries no ``use your tools`` — the phase-37 + # classifier must stay inert on it (regression-safe marker). + assert _tool_flow(_body(SEARCH_USER)) is None + + +def test_tool_trigger_does_not_shadow_the_search_flow() -> None: + # The phase-37/45 questions carry no ``search your documents`` — + # the search classifier must stay inert on them. + assert _search_flow(_body(SINGLE_USER)) is None + assert _search_flow(_body(MULTI_USER)) is None