feat(agent): search_documents tool — the model can grep the indexed documents for an exact string
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
# Phase 67 — LLM Retry with Live "Trying Again" Feedback
|
||||
|
||||
**Source:** `TODO.md` L3 — "Add a .env configurable retry in case the LLM server fails to respond. Allow 3 retries by default, with 5 seconds between each retry. Update the user interface to show 'communication interrupted, trying again' or something like that if the LLM server stops communicating."
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
**Context:**
|
||||
- `app/rag/llm.py` — `LLMClient.chat_stream` is the single streaming surface (deflected turns + every agent round); `LLMError` is the typed failure the API layer already turns into an SSE `error` frame (`app/api/chat.py` — `except LLMError` around the piece loop; `EmbeddingError` around the pre-stream `llm.embed_one(request.message)`).
|
||||
- `app/rag/agent.py` — `run_agent` issues one `chat_stream` per round (plus a final `tools=None` call at the round cap); the phase-48 teardown binds each stream and closes it in a `finally`.
|
||||
- `app/config.py` — the `BOR_*` settings block (LLM section: `llm_base_url`, `llm_chat_model`, …); `agent_max_rounds` shows the house pattern for a tunable with a startup validator.
|
||||
- `app/schemas.py` — the SSE event family (`ChatThinkingEvent`, `ChatToolEvent`, `ChatDoneEvent`, `ChatErrorEvent`).
|
||||
- `frontend/assets/app.js` — `runTurn`'s `readSSE` callback is the event state machine; the `tool` branch is the house pattern for a server-driven STATUS change (`#send-status` + typing-indicator `aria-label`, no new bubble, `clearTurnTimeout()` because a frame arrived). `tests/e2e/test_agent_document_tools.py` (L236+) records every `#send-status` value during a turn for assertions.
|
||||
- `tests/e2e/mock_llm.py` — deterministic marker-driven OpenAI-compatible stand-in (chat + embeddings), run as a uvicorn subprocess by `tests/e2e/conftest.py`; the marker flow is discriminated statelessly from the request.
|
||||
- **Not in scope (owner-locked A1):** the one-shot `LLMClient.chat()` path (document summaries, KB overview) and the sync probe (`check_models`) — those are admin/import paths with their own fail-fast behavior (phase 41) and no live user to notify.
|
||||
|
||||
## Objective
|
||||
When the aipi endpoint dies mid-turn, the app retries the LLM request automatically — `.env`-tunable, **3 retries / 5 s delay by default** — and the UI tells the user what is happening ("Communication interrupted — retrying (n of N)…") instead of the turn dead-ending in an error banner. A retry only ever restarts a request that has **not yet streamed a single output frame** to the client (locked A2), so no answer token is ever duplicated.
|
||||
|
||||
## Dependencies
|
||||
- `66_history_auto_save_copy` (todo, preceding — no functional dependency; ordering by number)
|
||||
|
||||
## Tasks
|
||||
1. `01_config_and_retry_primitive.md` — `BOR_LLM_RETRIES` / `BOR_LLM_RETRY_DELAY` settings + the `RetryPiece` + `chat_stream_retried()` primitive in `app/rag/llm.py`.
|
||||
2. `02_chat_endpoint_retry.md` — the `retry` SSE event, the embedding retry loop, and the deflected-stream retry in `app/api/chat.py`.
|
||||
3. `03_agent_round_retry.md` — per-round retries inside `run_agent` (loop rounds + final no-tools call).
|
||||
4. `04_frontend_retry_status.md` — the `retry` branch in `runTurn`'s SSE handler: the live "retrying (n of N)…" status.
|
||||
5. `05_e2e_and_commit.md` — `mock_llm.py` failure injection, `tests/e2e/test_llm_retry.py`, regressions, commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_config.py` (new vars, defaults, validators), `tests/unit/test_llm_client.py` (`chat_stream_retried` semantics — retry only before the first piece, `RetryPiece` ordering, exhaustion, `retries=0`), `tests/unit/test_agent.py` (per-round retry), `tests/unit/test_frontend_tool_states.py` pattern (JS pins for the `retry` branch).
|
||||
- Integration: `tests/integration/test_chat_api.py` — SSE frame ordering (embed-fail → `retry` frame → completed turn; embed-exhausted → `retry` frames + terminal `error` frame; mid-stream failure AFTER a delta → no retry, `error` frame).
|
||||
- E2E (mandatory, house rule): `tests/e2e/test_llm_retry.py`, run in isolation (mock LLM with deterministic failure injection; `BOR_LLM_RETRY_DELAY=0` on the test server so the suite stays fast).
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `BOR_LLM_RETRIES` (default 3) and `BOR_LLM_RETRY_DELAY` (default 5 s) are honored end to end and documented in `.env.example`.
|
||||
- [ ] A dead-then-recovered endpoint: the turn completes with a normal answer and the UI showed the "retrying" status while waiting; a dead endpoint: after N attempts the existing terminal error banner appears.
|
||||
- [ ] A stream failure after the first output frame still terminates with the `error` event — no retry, no duplicated tokens.
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `uv run pytest tests/e2e/test_llm_retry.py -v --no-cov` green in isolation (DB up).
|
||||
- [ ] Regression E2E suites green in isolation: `test_chat_rag.py`, `test_agent_document_tools.py`, `test_stop_generation.py`, `test_retry_answer.py`.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **Owner-locked (2026-09-01, roadmap confirmation, A1):** scope is the **chat turn only** — question embedding + the answer stream (deflected path and every agent round). `LLMClient.chat()` (summaries, KB overview) and `check_models` (sync probe) are untouched.
|
||||
- **Owner-locked (2026-09-01, roadmap confirmation, A2):** a retry restarts the LLM request **only if no output frame has been streamed to the client yet** for that request (no thinking/tool/delta emitted). Once tokens are flowing, the failure stays terminal (the existing `error` frame) — a partial answer is never redone.
|
||||
- **Owner-locked (2026-09-01, roadmap confirmation, A3):** env names `BOR_LLM_RETRIES` (int, default **3**) and `BOR_LLM_RETRY_DELAY` (seconds, default **5**) — a flat delay between attempts, no exponential backoff (the TODO specifies a fixed 5 s).
|
||||
- **Owner-locked (2026-09-01, roadmap confirmation, A4):** UI copy — `#send-status` reads `Communication interrupted — retrying (n of N)…` (n = current attempt, N = the configured retry count) on the existing status line; no new banner, no bubble.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agent/ app/ tests/ frontend/ && git commit --no-gpg-sign -m "feat(rag): retry a failed LLM request before the first token lands — BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY with a live 'retrying' status"
|
||||
```
|
||||
@@ -0,0 +1,45 @@
|
||||
# Task 01 — `search_documents`: Tool Definition, Grep Helper, Execution Branch
|
||||
|
||||
**Phase:** `68_search_tool` · **Source:** `TODO.md:4` — "Add a search tool that allows the LLM to grep through the uploaded documents for a given string"
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
|
||||
## Objective
|
||||
The tool exists end to end server-side: it is in `AGENT_TOOLS` with a model-legible contract, and `_execute_tool` executes it — case-insensitive fixed-substring grep over `documents.content`, grep-style output, hard caps, and the house refusal strings.
|
||||
|
||||
## Work
|
||||
1. `app/rag/agent.py` — module constants (next to the caps/refusals):
|
||||
- `SEARCH_MAX_MATCHES = 20` — global per-call cap, catalog order (owner-locked A5).
|
||||
- `SEARCH_LINE_LIMIT = 200` — per-line output truncation (owner-locked A5).
|
||||
- `MISSING_SEARCH_ARGS = "search_documents requires a string argument 'pattern'."`
|
||||
- `NO_MATCHES = "No matches for '{pattern}' in the knowledge base."` / scoped variant `"No matches for '{pattern}' in {source}/{path}."`
|
||||
2. `app/rag/agent.py` — `grep_document(content: str, pattern: str) -> list[tuple[int, str]]` (module-level so unit tests can use/monkeypatch it, house pattern of `list_catalog`/`find_document`):
|
||||
- split `content` on `\n`; a line matches when `pattern.lower() in line.lower()` (case-insensitive fixed substring — owner-locked A5); return `(1-based line number, line.rstrip())` pairs.
|
||||
3. `app/rag/agent.py` — `AGENT_TOOLS`: append the third function definition:
|
||||
- `name`: `search_documents` (owner-locked A6).
|
||||
- `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`: `pattern` (string, **required** — "The exact text to search for (a plain substring, not a regex)"); `source` + `path` (strings, optional — the same "as shown after 'source: '/'path: ' in the list_documents output" wording `read_document` uses, phase 63 labeled fields).
|
||||
4. `app/rag/agent.py` — `_execute_tool` branch (`call.name == "search_documents"`, placed after the `read_document` branch, before the `UNKNOWN_TOOL` fallback):
|
||||
- `pattern = call.arguments.get("pattern")`; must be a non-empty string after `.strip()` → else `MISSING_SEARCH_ARGS`.
|
||||
- if BOTH `source` and `path` are non-empty after strip: `find_document(db, source, path)` → `None` → `"No document at {source}/{path} — check the list_documents output."` (the existing read_document refusal style); search only that document (scoped no-match message).
|
||||
- if only ONE of `source`/`path` is given → treat it as a missing pair: `MISSING_SEARCH_ARGS` (a half-specified target is a model error, not a whole-KB search — fail loud, house style).
|
||||
- else: iterate `list_catalog(db)` in `(source, path)` order, load each `Document.content` via `find_document` (or one bulk `select(Document)` ordered by source,path — executor's call, note which in the commit body), accumulating `f"{doc.source}/{doc.path}:{lineno}: {line[:SEARCH_LINE_LIMIT]}"` until `SEARCH_MAX_MATCHES` total; stop scanning once the cap is hit.
|
||||
- no matches → the no-match line (pattern quoted; a pattern longer than 100 chars is truncated in the message to keep it short).
|
||||
- success: `holder.tool_calls += 1` (an executed call, re-searches included — same counting as `list_documents`); `holder.read_docs` is **not** touched (locked A5 — the search never adds context).
|
||||
- log line: the existing `logger.info("agent tool=%s args=%s round=%d/%d", ...)` already covers it (the `arguments` dump includes `pattern`).
|
||||
5. `app/rag/agent.py` — update the module docstring: three tools now (list/read/search); a search is a locator, not a context-adder.
|
||||
6. Unit tests — `tests/unit/test_agent.py`:
|
||||
- case-insensitive match across multiple lines, 1-based line numbers, multi-line and repeated matches.
|
||||
- the 20-match global cap across two documents (catalog order); line truncation at 200 chars (a 300-char line yields 200 + no crash).
|
||||
- scoped search: found doc, missing doc (refusal), single-arg (only `source`) → `MISSING_SEARCH_ARGS`.
|
||||
- no-match (whole KB and scoped) messages; empty/whitespace `pattern` → `MISSING_SEARCH_ARGS`; non-string `pattern` → `MISSING_SEARCH_ARGS`.
|
||||
- `holder.tool_calls` counts a search; `holder.read_docs` unchanged after a search.
|
||||
- `AGENT_TOOLS` shape: three tools, `search_documents` has `required: ["pattern"]` (and optional `source`/`path`).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: Work 6 + `tests/integration/test_agent_tools.py` (the new tool is offered and executed through `run_agent` with a scripted `ToolCallPiece`).
|
||||
- Coverage: **>90%** on this task's new/modified code.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A scripted `search_documents` call through `run_agent` returns the grep-style result text and bumps `tool_calls` without touching `read_docs`.
|
||||
- [ ] `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] no behavior change for `list_documents` / `read_document` (existing pins green).
|
||||
@@ -0,0 +1,46 @@
|
||||
# Task 02 — SSE Argument Mapping + the Frontend Search Status/Tool Line
|
||||
|
||||
**Phase:** `68_search_tool` · **Source:** `TODO.md:4` — "Add a search tool that allows the LLM to grep through the uploaded documents for a given string"
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
|
||||
## Objective
|
||||
The search call is visible in the UI like the other two tools: the SSE `tool` frame carries the pattern as its `argument`, the status line reads `Brain is searching for 'pattern'`, and a `🔎 Searching for '<pattern>'` line lands above the answer — persisted and restored with the conversation like the existing tool lines.
|
||||
|
||||
## Work
|
||||
1. `app/api/chat.py` — the `ToolCallPiece` branch currently computes `argument` as `f"{source}/{path}"` for `read_document`, else `None`:
|
||||
- extend: `elif piece.name == "search_documents":` → `argument = piece.arguments.get("pattern")` (the raw string; a non-string pattern — a model error the backend refuses — yields `None`).
|
||||
2. `app/schemas.py` — `ChatToolEvent`: update the docstring + field comments — `name` is `"list_documents" | "read_document" | "search_documents"`; `argument` is `"source/path"` for `read_document`, the **search pattern** for `search_documents`, null otherwise. (No field-shape change.)
|
||||
3. `frontend/assets/app.js` — `runTurn`'s `tool` branch, the `toolStatus` computation:
|
||||
```js
|
||||
const toolStatus =
|
||||
name === "read_document" && argument
|
||||
? `${brand()} is reading ${argument}`
|
||||
: name === "search_documents" && argument
|
||||
? `${brand()} is searching for ${argument}`
|
||||
: `${brand()} is listing documents`;
|
||||
```
|
||||
4. `frontend/assets/app.js` — `appendToolLine` (L796): add the search branch BEFORE the `else` fallback:
|
||||
```js
|
||||
} 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);
|
||||
}
|
||||
```
|
||||
(The `else` keeps `"🔎 Listing documents"` for `list_documents` and any unknown name.) The pattern goes in a `<code>` element exactly like the read path — data, never markup (the existing XSS-safe convention).
|
||||
5. Persistence/restore: the `toolAcc` record is already `{name, argument}`-generic and the restore path calls the same `appendToolLine(t.name, arg)` (L1205) — no extra work; verify the restore branch renders the search line (covered by the E2E in task 03 only if a reload happens in that suite — otherwise by the unit pin below).
|
||||
6. JS unit pins — `tests/unit/test_frontend_tool_states.py` (the phase-37 frontend contract file):
|
||||
- the status ternary contains the locked `is searching for` branch with the correct name/argument gate;
|
||||
- `appendToolLine` contains the `search_documents` branch with the `<code>` element (pattern-as-data pin);
|
||||
- the persisted tool record still serializes `{name, argument}` generically (no per-tool shape).
|
||||
7. Integration pin — `tests/integration/test_chat_api.py` (or the SSE pin module): a scripted `search_documents` `ToolCallPiece` streams as `{type: "tool", name: "search_documents", argument: "<pattern>"}`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: Work 6 (JS pins) + Work 7 (SSE shape).
|
||||
- Coverage: **>90%** on this task's new/modified code.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A `search_documents` tool frame streams with `argument = pattern` and renders the locked status + tool line in the browser (E2E in task 03).
|
||||
- [ ] `uv run pytest tests/unit/test_frontend_tool_states.py tests/integration/test_chat_api.py -v` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] no behavior change for the existing two tool frames (their SSE shapes + UI lines are untouched).
|
||||
@@ -0,0 +1,39 @@
|
||||
# Task 03 — Mock Search Flow, E2E Suite, Regressions, Commit
|
||||
|
||||
**Phase:** `68_search_tool` · **Source:** `TODO.md:4` — the full item (end-to-end proof: the model greps, sees the match, answers from it).
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
|
||||
## Objective
|
||||
Prove the tool in a browser with the deterministic mock: a grounded question makes the mock model call `search_documents`, the match line reaches the model, and the answer quotes the found content — with the search visible in the status line and tool lines. One dedicated Playwright suite, green in isolation, plus the regression pass and the phase commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` — a new marker flow, following the `use your tools` flow's structure (stateless discrimination from the messages, streaming only):
|
||||
- `SEARCH_TRIGGER = "search your documents"` (checked BEFORE the plain `use your tools` check — it is more specific, same convention as `think in paragraphs`):
|
||||
- request 1 (`tools` offered, no tool results yet): stream ONLY a `tool_calls` delta — `search_documents` with `{"pattern": "<SEARCH_PATTERN>"}` (id `call_0`); `<SEARCH_PATTERN>` is a sentinel string the e2e places in a fixture document (e.g. `reese-sentinel-42` — the sentinel convention from the `show the end of your notes` marker).
|
||||
- request 2 (a `tool`-role search result in the messages — recognizable as a search result by its `source/path:line: text` shape or the sentinel in its content): the content answer, deterministic: `Found <first matched line's content up to 80 chars>` — so the suite can assert the search result reached the model and landed in the answer.
|
||||
- document the flow in the module docstring's marker list.
|
||||
2. `tests/e2e/test_search_tool.py` — the dedicated suite (DB up; import one fixture document containing the sentinel line, via the existing admin import fixtures in `conftest.py`/`auth_helpers.py`):
|
||||
- **live search flow:** ask a KB question containing `search your documents` →
|
||||
- a `.msg.brain .tool-call` line appears containing `🔎 Searching for` and the sentinel in a `<code>` (assert via `to_contain_text("Searching for")` + the code element text);
|
||||
- the recorded `#send-status` values contain `is searching for <sentinel>` (the init-script status-recording pattern from `test_agent_document_tools.py`);
|
||||
- the answer bubble contains the deterministic `Found …` echo (the match reached the model);
|
||||
- NO error banner; the turn settles to idle with the send button re-enabled.
|
||||
- **context accounting:** the search does not add a source by itself — if the mock flow searches and then answers WITHOUT a read, `done.sources` reflects only the retrieval docs (assert the sources row is unchanged by the search alone). (If the executor finds the mock flow must also read to produce a stable answer, keep the flow search-only and assert the sources row equals the retrieval baseline.)
|
||||
- **regression-safe markers:** the existing `use your tools` questions in `test_agent_document_tools.py` / `test_agent_unlimited_tools.py` do NOT contain the new trigger (verify — the trigger phrase must not appear in any other suite's fixture text).
|
||||
3. Regression pass (each in isolation, DB up): `uv run pytest tests/e2e/test_agent_document_tools.py -v --no-cov`, `uv run pytest tests/e2e/test_agent_unlimited_tools.py -v --no-cov`, `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov` — the list/read flow, the unlimited-calls behavior, and the plain RAG turn must be untouched (the third tool changes the `tools` payload — confirm no existing suite pins an exact two-tool payload; if one does, update it to expect three and note it in the commit body).
|
||||
4. Full gate: `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), `uv run ruff check . && uv run pyright`.
|
||||
5. Commit (AGENTS.md rule 8 — one atomic phase commit):
|
||||
```bash
|
||||
git add -A .agent/ app/ tests/ frontend/ && git commit --no-gpg-sign -m "feat(agent): search_documents tool — the model can grep the indexed documents for an exact string"
|
||||
```
|
||||
Then move the phase dir to `.agent/phases/complete/68_search_tool/` and include the move in the SAME commit (house convention — mirror phase 67's commit/move pattern).
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: as listed (Work 2–3).
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_search_tool.py -v --no-cov` green in isolation (DB up).
|
||||
- [ ] The three regression suites green in isolation.
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` Conventional Commits commit; phase dir in `.agent/phases/complete/`.
|
||||
Reference in New Issue
Block a user