diff --git a/.agent/phases/todo/71_scaffolding_guardrails/00_phase.md b/.agent/phases/complete/71_scaffolding_guardrails/00_phase.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/00_phase.md rename to .agent/phases/complete/71_scaffolding_guardrails/00_phase.md diff --git a/.agent/phases/todo/71_scaffolding_guardrails/05_e2e_commit.md b/.agent/phases/complete/71_scaffolding_guardrails/05_e2e_commit.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/05_e2e_commit.md rename to .agent/phases/complete/71_scaffolding_guardrails/05_e2e_commit.md diff --git a/.agent/phases/complete/72_teaching_refusals/00_phase.md b/.agent/phases/complete/72_teaching_refusals/00_phase.md new file mode 100644 index 0000000..b35492f --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/00_phase.md @@ -0,0 +1,165 @@ +# Phase 72 — Teaching Refusals: End the Post-Harness Tool-Loop Rambling + +**Story:** `.agent/user_stories/agent-document-tools.md` (this phase repairs the model-facing +contract the phase-70 tools reshaped) +**Context:** +- `app/rag/agent.py` — `AGENT_TOOLS` (the phase-70 `ls` / `read` / `grep` OpenAI function + definitions), `_execute_tool` (the refusal strings: + `"No source named '…' — check the ls output."`, + `"No document at '…' — check the ls output."`), `all_documents` + (catalog-order bulk loader — reused by the suggestion lookup). +- `app/rag/prompts.py` — `TOOLS_SECTION` (HIGH prompt only; the E2E mock keys off the + `` marker's *presence*, not its wording). +- `tests/unit/test_agent.py` (refusal-string pins; the `ScriptedLLM` + monkeypatched- + accessor pattern), `tests/integration/test_agent_tools.py` (the same pins against real + Postgres, `kb`/`src` fixtures). +- `tests/e2e/mock_llm.py` — the deterministic mock tool flows (`TOOLS_TRIGGER` single-read, + `MULTI_READ_TRIGGER`, `SEARCH_TRIGGER`; `_CATALOG_LINE_RE` catalog-line parse) and the + dedicated-suite-per-phase E2E house pattern. +- `scripts/llm_probe.py` — the house live-endpoint probe pattern (`python -m scripts.…`, + argparse, dotenv, printed verdict line); `app/api/chat.py` — the grounded path the + real-model gate mirrors (`retrieve` → `select_documents` → `build_high_prompt` → + `run_agent`) +- **Incident (owner chat, 2026-09-03, post phase 70/71):** the question "list the files + in this directory" produced a Thinking-display trace of the model calling + `ls(path='app/rag/importer.py')` → `"No source named 'app/rag/importer.py' — check the + ls output."`, then `ls(path='.')` → the same-style refusal, then re-reasoning the same + paragraphs over and over across rounds (each round's `reasoning_content` appends to the + open Thinking block) before finally answering from the seed documents alone. Root cause: + the harness-trained prior (`ls`'s `path` = a directory to list) collides with this app's + contract (`path` = a source-name filter), and the terse refusal does not correct the + misunderstanding, so the model burns rounds. The identical trap awaits `read`/`grep`: + a bare document path missing the source prefix (`read('app/rag/importer.py')`) → + `"No document at '…'"` with no hint of the combined form. + +## Objective +Make the affected tool refusals **teaching** so the harness-prior misuse self-corrects in +at most one extra round: a scoped `ls` whose `path` looks like a document path (contains +`/`) or names an unknown source gets a fixed-template refusal that states the correct +contract; a `read` / scoped-`grep` argument that resolves to no combined identity but +*matches an indexed document's `path`* (exact or suffix) gets a +`"did you mean 'source/path'?"` refusal naming the exact combined identity to use. The +`AGENT_TOOLS` `path` descriptions and the `TOOLS_SECTION` prompt copy say the same +contract up front. Deterministic only — no model participates in detection or repair; the +phase-70 harness shape (`ls` / `read(path)` / `grep(pattern, path?)`) is unchanged +verbatim. The phase does not pass on mocks alone: a live acceptance gate runs the +fixed question battery through `run_agent` against the **real configured chat model** +(`lite` per `.env`) and must PASS before the commit (owner directive, 2026-09-03 — +"test with the real lite model until tool calls work consistently; don't pass until a +sufficient number of tool calls succeed"). + +## Dependencies +- `70_harness_aligned_tools` (complete) — the tool surface this phase teaches (shape + untouched). +- `71_scaffolding_guardrails` (complete) — the deterministic-guardrail house style this + phase follows. + +## Tasks +1. `01_ls_teaching_refusal.md` — `ls`: path-like and unknown-source scopes get teaching + refusals; the `ls` `path` description says "source name, not a file or directory path". +2. `02_read_grep_path_suggestion.md` — `read` / `grep`: an unresolved argument that + matches an indexed document `path` gets the "did you mean 'source/path'?" suggestion; + descriptions updated. +3. `03_prompt_copy.md` — `TOOLS_SECTION` copy: the `ls` `path` is a source name, not a + directory; `read`/`grep` need the combined identity *including the source name*. +4. `04_mock_e2e.md` — mock `ls`-misuse flow, dedicated E2E suite (green in isolation). +5. `05_real_model_gate.md` — the live real-lite acceptance gate + (`scripts/agent_realmodel_check.py`): iterate the copy levers until the gate + PASSES, then full gates and the commit. + +## Testing & Quality +- Unit: `tests/unit/test_agent.py` — the new `ls` teaching refusals (scope containing + `/` → the document-path line; scope without `/` unknown → the extended no-source line; + both count in nothing, tools stay offered; valid-scope and no-arg listings + byte-identical to today); `find_path_candidates` (exact `path` match, suffix match, + multiple candidates in catalog order capped at 3, zero candidates, no-`/` argument → + no DB lookup); `read`/`grep` wiring (in-context dedupe precedence, valid combined form + unchanged, scoped `grep` suggestion, A5 grep contract regression). +- Integration: `tests/integration/test_agent_tools.py` — changed pins updated; new + end-to-end suggestion cases through `run_agent` against real Postgres (bare path under + one source; the same path under two sources). +- E2E (mandatory, house rule): NEW dedicated suite `tests/e2e/test_tool_path_teaching.py`, + run in isolation — the mock flow (misuse `ls(path='.')` → teaching refusal → corrected + no-arg `ls()` → listing answer) through the real UI with the two-round shape pinned on + the SSE wire; regression suites green in isolation: `test_harness_aligned_tools.py`, + `test_agent_document_tools.py`, `test_agent_unlimited_tools.py`, `test_search_tool.py`, + `test_chat_rag.py`. +- **Real-model acceptance gate (owner-locked, the phase's pass condition):** + `uv run python -m scripts.agent_realmodel_check` against the live endpoint with the + configured chat model (`lite`) — the fixed 10-question battery (3 `ls` turns including + the incident's "list the files in this directory" and a source-name trap, 4 `read` + turns including two bare-path traps, 1 `grep` turn, 2 mixed) driven through the real + grounded path. PASS = every turn answers (no `LLMError`/`MalformedReplyError`), zero + turns hit the round cap, ≥6 of 10 turns emit ≥1 tool call, and **≥90% of all emitted + tool calls execute** (rejections don't count). Until it passes, task 05 iterates the + copy levers this phase owns (refusal templates, `AGENT_TOOLS` descriptions, + `TOOLS_SECTION`) — the question set and thresholds are fixed by the task file and may + not be weakened. +- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`). + +## Completion Criteria +- [ ] A scoped `ls` whose stripped `path` contains `/` gets the document-path teaching + refusal; an unknown source name without `/` gets the extended "source name, not a + directory" refusal; neither counts in anything; a valid scope and the no-arg + listing are byte-identical to today. +- [ ] `read` / scoped `grep` with an unresolved argument that matches an indexed document + `path` (exact or suffix) gets the "did you mean …?" refusal (one candidate → one + combined identity; two or more → up to 3, catalog order); a non-matching argument + gets today's refusal byte-identical; the in-context dedupe refusal still wins. +- [ ] The `AGENT_TOOLS` `path` descriptions for `ls` / `read` / `grep` state the contract + explicitly; the tool names and argument shapes are unchanged + (`rg '"name":' app/rag/agent.py` → exactly `ls`, `read`, `grep`). +- [ ] `TOOLS_SECTION` clarifies the source-name `ls` `path` and the source-name-required + combined identity; the HIGH prompt still ends with the `` section; the + LOW/deflection prompt is byte-identical to today. +- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL + **>90%**; `uv run ruff check . && uv run pyright` clean. +- [ ] `uv run pytest tests/e2e/test_tool_path_teaching.py -v --no-cov` green in + isolation; the regression suites above green in isolation. +- [ ] `uv run python -m scripts.agent_realmodel_check` exits 0 against the live + endpoint (all four pass conditions met with the configured model) — the verdict + line recorded in the `app/rag/agent.py` module docstring and in the commit body. +- [ ] One `--no-gpg-sign` commit (message in the Commit block); the phase directory + moved to `.agent/phases/complete/`. + +## Locked decisions +- **The phase-70 tool surface is unchanged** (owner lock, 2026-09-03): `ls(path?)` / + `read(path)` / `grep(pattern, path?)` — no renames, no argument additions or removals; + this phase changes refusal copy, tool descriptions, and prompt copy only. +- **Deterministic only** (owner 2026-09-03, phase-71 house style): no model in detection + or repair; suggestions are a pure catalog lookup (exact or suffix `path` match, + case-sensitive, catalog order, capped at 3); every refusal is a fixed template + constant. +- **Teach, don't silently fix:** a misused call is still a refusal (counts in nothing, + consumes a round); the model sees its own argument echoed plus the correct form. No + silent argument normalization — `ls(path='.')` does NOT become a full listing. +- **The zero-candidate refusal is byte-identical to today** + (`"No document at '…' — check the ls output."`) — no behavior change where the model is + not confused; the `ls` no-source refusal keeps its prefix (the teaching parenthetical + is appended). +- **No UI change:** the Thinking display (phases 17/21/43) works as designed — the fix + ends the loop, it does not hide the scratchpad. **No SSE contract change** (refusals + are tool results in the message history; the `tool` frames already carry the call's + `name`/`argument`). **No model swap** (owner keeps `lite`), **no env change**, + **no schema change**. +- **Real-model gate is a pass condition, not a smoke test** (owner directive, + 2026-09-03): the phase is NOT complete — and gets NO commit — until + `scripts/agent_realmodel_check.py` PASSES against the real `lite` model. The 10 + questions, the ≥6-of-10 tool-usage floor, the ≥90% executed-call bar, and the + zero-cap rule are fixed by task 05's file; the executor may iterate ONLY the copy + levers this phase owns (refusal templates, `AGENT_TOOLS` descriptions, + `TOOLS_SECTION` — with their unit pins updated to follow the constants). Lowering a + threshold, swapping in easier questions, or skipping the gate to "make it pass" is + forbidden; a gate still failing after iteration stops the phase with the per-turn + numbers reported for the owner (fail-loud house style). The verdict line (house + precedent: the phase-37 probe verdict in `app/rag/agent.py`) is recorded in that + module's docstring and in the commit body. + +## Commit +```bash +git add -A .agent/ app/ tests/ scripts/ && git commit --no-gpg-sign -m "fix(agent): teach the document-identity contract on ls/read/grep refusals — end the post-harness tool-loop rambling" -m "" +``` +The commit also carries the still-uncommitted phase-71 `todo/` → `complete/` move and +`.agent/reports/71_scaffolding_guardrails/` (`.agent/` is tracked and committed with the +phase — AGENTS.md §8; only `.agent/phase-sessions/` and `.agent/pipeline.log` are +gitignored). diff --git a/.agent/phases/complete/72_teaching_refusals/01_ls_teaching_refusal.md b/.agent/phases/complete/72_teaching_refusals/01_ls_teaching_refusal.md new file mode 100644 index 0000000..704de42 --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/01_ls_teaching_refusal.md @@ -0,0 +1,64 @@ +# Task 01 — `ls`: Teaching Refusals for Path-Like and Unknown-Source Scopes + +**Phase:** `72_teaching_refusals` · **Story:** `.agent/user_stories/agent-document-tools.md` + +## Objective +A scoped `ls` whose `path` argument is a file/directory path (contains `/`) — or an +unknown source name — gets a fixed-template refusal that states the correct contract +instead of the terse "check the ls output", so the harness-prior misuse +(`ls(path='app/rag/importer.py')`, `ls(path='.')` — the incident) self-corrects in one +round. The `ls` tool description makes the same point at request time. + +## Work +1. `app/rag/agent.py` — two refusal template constants next to the existing refusal + constants, plus the branch change: + - `LS_PATH_NOT_A_SOURCE: str` — one `{path}` field, used when the **stripped** scope + contains `/` (a source name can never contain `/` — source names are directory + basenames, `app.rag.importer`): + `"'{path}' looks like a document path, not a source name. The 'path' argument of ls filters by source name (e.g. 'homelab') — omit it to list every document, or read a document by its combined 'source/path' string."` + - `NO_SOURCE_NOT_A_DIRECTORY: str` — one `{scope}` field, the existing no-source + refusal with a teaching parenthetical appended (the prefix + `"No source named '{scope}' — check the ls output."` stays byte-identical), used + when the scope has no `/` and matches no registered source name: + `"No source named '{scope}' — check the ls output. (The 'path' argument is a source name, not a directory — omit it to list every document.)"` + - `_execute_tool` `ls` branch: non-empty scope with `"/" in scope` → + `LS_PATH_NOT_A_SOURCE.format(path=scope)`; non-empty scope without `/` not in + `list_source_names(db)` → `NO_SOURCE_NOT_A_DIRECTORY.format(scope=scope)`; a valid + scope and the no-arg listing are unchanged. Both refusals count in nothing (no + `holder.tool_calls` bump) and consume a round — exactly like today's refusal. + - `AGENT_TOOLS` → `ls` → `function.parameters.properties.path.description`: + `"Source name to list one source's documents (e.g. 'homelab') — a source name, not a file or directory path; omit to list every document."` + - Module docstring (loop contract, point 3 — the refusal list): update the + scoped-`ls` refusal entry to the two new lines. +2. `tests/unit/test_agent.py` — unit pins (existing `ScriptedLLM` + monkeypatched + `list_catalog` / `list_source_names` pattern; import the constants, never re-type + them): + - `ls(path='app/rag/importer.py')` (scope contains `/`) → the + `LS_PATH_NOT_A_SOURCE` line with the argument echoed; `holder.tool_calls == 0`; + tools stay offered on the next request. + - `ls(path='.')` (no `/`, unknown) → the `NO_SOURCE_NOT_A_DIRECTORY` line with + `'.'` echoed; `holder.tool_calls == 0`. + - `ls(path='Ghost')` (no `/`, unknown) → the same extended line (replaces today's + `test_ls_scoped_unknown_source_refused` pin). + - Regression: `ls()` no-arg full catalog and `ls(path='')` scoped + listing (including the `0 documents:` registered-empty-source case) remain + byte-identical to today. +3. `tests/integration/test_agent_tools.py` — update the changed pin (the + `ls(path='Ghost')` assertion) and add one case: a scoped `ls` with a `/`-containing + `path` against the real DB (`kb` + `src` fixtures) → the document-path line, not + counted, tools stay offered. + +## Testing & Quality +- Unit/integration: as listed in Work 2–3 — every new refusal line pinned + byte-for-byte; the count-in-nothing and tools-stay-offered invariants pinned; the + unchanged paths regression-pinned. +- Coverage: **>90%** on this task's new/modified code (the `ls` branch in + `app/rag/agent.py`). + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v --no-cov` + green (DB up: `podman compose up -d db`) +- [ ] The old terse string (no-source refusal without the parenthetical) appears nowhere + in `app/` or `tests/` +- [ ] `uv run ruff check . && uv run pyright` clean +- [ ] No behavior change to valid-scope / no-arg `ls` (regression pins green) diff --git a/.agent/phases/complete/72_teaching_refusals/02_read_grep_path_suggestion.md b/.agent/phases/complete/72_teaching_refusals/02_read_grep_path_suggestion.md new file mode 100644 index 0000000..b7caf21 --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/02_read_grep_path_suggestion.md @@ -0,0 +1,87 @@ +# Task 02 — `read` / `grep`: "did you mean 'source/path'?" Suggestions for Bare Document Paths + +**Phase:** `72_teaching_refusals` · **Story:** `.agent/user_stories/agent-document-tools.md` + +## Objective +When `read` (or a scoped `grep`) receives an argument that resolves to no combined +identity but *does* match an indexed document's `path` (exact or as a suffix), the +refusal names the exact combined `source/path` identity to use — the harness prior +(`read('app/rag/importer.py')`, missing the source prefix) self-corrects in one round. +An argument that matches nothing keeps today's refusal byte-identical. + +## Work +1. `app/rag/agent.py`: + - `SUGGESTION_LIMIT = 3` — the cap on suggested identities per refusal. + - Module-level `find_path_candidates(db: Session, arg: str) -> list[tuple[str, str, str]]` + (so unit tests can monkeypatch it, house pattern): the indexed documents, in + **catalog order** (the `all_documents` order), whose `path` equals `arg` or ends + with `f"/{arg}"` (case-sensitive — these are file paths), as `(source, path, title)` + triples. One bulk query via `all_documents`; called **only** from the refusal path + below (never on the happy path) and **only** when `arg` contains `/` (a bare name + keeps today's no-DB-lookup refusal — the existing + `test_read_bare_source_name_refused_without_db` invariant stays green). + - Refusal templates next to the existing constants: + - `NO_DOCUMENT_DID_YOU_MEAN: str` — + `"No document at '{arg}' — did you mean '{source}/{path}'?"` + - `NO_DOCUMENT_DID_YOU_MEAN_MANY: str` — + `"No document at '{arg}' — did you mean one of: {candidates}?"` where + `{candidates}` is up to `SUGGESTION_LIMIT` combined `source/path` identities, + each single-quoted, joined with `", "`, in catalog order. + - `_execute_tool` `read` branch: after the in-context dedupe check and the + `_resolve_path` miss — when `arg` contains `/`, run `find_path_candidates`: + exactly 1 candidate → `NO_DOCUMENT_DID_YOU_MEAN`; 2+ → + `NO_DOCUMENT_DID_YOU_MEAN_MANY`; 0 → today's + `"No document at '{arg}' — check the ls output."` unchanged. `holder` untouched + (a refusal counts in nothing; `read_docs` untouched — locator-only never changes). + - `_execute_tool` `grep` branch: the same substitution for the scoped-`path` miss + (the whole-KB grep is untouched). + - `AGENT_TOOLS` → `read` → `path` description and `grep` → `path` description: + append `" A bare document path (without the source name) will not resolve."` to + each current text. + - Module docstring (loop contract, point 3): document the suggestion behavior in the + refusal list. +2. `tests/unit/test_agent.py` — unit pins (monkeypatched `find_document` + + `all_documents`; import the constants, never re-type them): + - `read(path='active/container_caddy/caddy.md')` with the document indexed under + `Homelab` (exact `path` match) → `did you mean 'Homelab/active/container_caddy/caddy.md'?`; + `holder.read_docs` empty, `holder.tool_calls == 0`. + - Suffix match: `read(path='caddy.md')` → the same single suggestion. + - Two sources sharing the same `path` → + `did you mean one of: 'A/x.md', 'B/x.md'?` in catalog order. + - Four sources sharing the `path` → exactly 3 suggestions (the cap). + - No match → today's refusal byte-identical; `read(path='Homelab')` (bare, no `/`) → + today's refusal with **no** `find_document` / `all_documents` call (the `_boom` + guard, existing pattern). + - Dedupe precedence: a combined-form re-read of a `seed_docs` document → + `ALREADY_IN_CONTEXT` (unchanged); a bare-`path` read of an in-context document + (`read('app/rag/importer.py')` with `sample/app/rag/importer.py` seeded) → the + suggestion line (the split pair is not in `known`, so the model learns the + combined identity — its next, correctly-formed call is then deduped). + - Scoped `grep` miss with a candidate → the suggestion line; scoped `grep` miss + without → today's line; whole-KB `grep` unchanged (A5 match/output contract: + fixed substring, case-insensitive, 20 matches, 200-char lines). + - Happy paths regression-pinned: combined-form `read` (full content, + `read_docs` appended), valid scoped `grep` result. +3. `tests/integration/test_agent_tools.py` — update the changed pins (inspect each + existing `"No document at …"` assertion against the fixture documents; only the + lines whose argument matches a fixture document `path` change to the suggestion form), + plus two new end-to-end cases through `run_agent` against real Postgres: a bare path + under one source (single suggestion) and the same `path` under two sources (the + "one of" line) — in both, the refusal is followed by the model's corrected call + succeeding (scripted `ToolCallPiece` round 2 with the suggested combined identity). + +## Testing & Quality +- Unit/integration: as listed in Work 2–3 — every new template pinned byte-for-byte; + the catalog-order + cap invariant pinned; the zero-candidate and no-DB-lookup + invariants pinned; the A5 `grep` contract regression-pinned. +- Coverage: **>90%** on this task's new/modified code (`find_path_candidates` + both + `_execute_tool` branches). + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v --no-cov` + green (DB up: `podman compose up -d db`) +- [ ] `find_path_candidates` is module-level (monkeypatchable) and issues at most one + bulk query +- [ ] The zero-candidate refusal and the bare-name (no-DB-lookup) refusal are + byte-identical to today +- [ ] `uv run ruff check . && uv run pyright` clean diff --git a/.agent/phases/complete/72_teaching_refusals/03_prompt_copy.md b/.agent/phases/complete/72_teaching_refusals/03_prompt_copy.md new file mode 100644 index 0000000..3c39ad8 --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/03_prompt_copy.md @@ -0,0 +1,38 @@ +# Task 03 — `TOOLS_SECTION` Copy: State the Contract Up Front + +**Phase:** `72_teaching_refusals` · **Story:** `.agent/user_stories/agent-document-tools.md` + +## Objective +The HIGH prompt's `` section says the same two things the new refusals teach — +the `ls` `path` is a *source name*, not a directory or file path, and `read`/`grep` +need the combined `source/path` string *including the source name* — so the model +carries the contract before it calls a tool, not only after being refused. + +## Work +1. `app/rag/prompts.py` — `TOOLS_SECTION` rewritten (the E2E mock keys off the + `` marker's *presence*, not this wording, so the change is mock-safe): + - `ls` clause: its optional `path` argument is a *source name* (e.g. `'homelab'`) + — **not** a directory or file path; omit it to list every document. + - `read` clause: the combined `source/path` string, exactly as shown in the `ls` + output — *including the source name*; a bare document path will not resolve. + - `grep` clause: the locator copy stays (its `path` is already described as a + combined `source/path` string); add the same bare-path-will-not-resolve note. + - Keep the section's shape: a single paragraph between `` and ``, + still appended after the mode body in the HIGH prompt only (the LOW/deflection + prompt never carries it — phase 71's plain-text line stays put). +2. `tests/unit/test_prompts.py` — update the `TOOLS_SECTION` wording pin(s) where they + pin the old wording; the ``-marker-present-in-HIGH pin, the + marker-absent-from-LOW pin, and the byte-identical-LOW-prompt pin stay green as-is. + +## Testing & Quality +- Unit: `tests/unit/test_prompts.py` — marker present in the HIGH prompt and absent + from the LOW prompt; the LOW prompt byte-identical to today; the new wording pinned + for the `ls` source-name clause and the read combined-identity clause. +- Coverage: **>90%** on this task's modified code (the constant itself — the builders + are already covered). + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_prompts.py -v --no-cov` green +- [ ] The HIGH prompt still ends with the `` section (existing section-order pin + green); the LOW/deflection prompt is byte-identical to today +- [ ] `uv run ruff check . && uv run pyright` clean diff --git a/.agent/phases/complete/72_teaching_refusals/04_mock_e2e.md b/.agent/phases/complete/72_teaching_refusals/04_mock_e2e.md new file mode 100644 index 0000000..af6dc61 --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/04_mock_e2e.md @@ -0,0 +1,63 @@ +# Task 04 — Mock Flow, Dedicated E2E Suite + +**Phase:** `72_teaching_refusals` · **Story:** `.agent/user_stories/agent-document-tools.md` + +## Objective +Prove the self-correction loop deterministically through the real UI: a mock-LLM flow +that reproduces the incident's `ls(path='.')` misuse, receives the teaching refusal, +corrects to a no-arg `ls()`, and answers from the catalog — a dedicated Playwright +suite pinning the two-round shape on the SSE wire. (The live real-model acceptance +gate is task 05 — this task is the deterministic half of the proof.) + +## Work +1. `tests/e2e/mock_llm.py` — one new deterministic flow, checked in the flow table + **before** the plain `TOOLS_TRIGGER` flow (the trigger phrases are disjoint + substrings; the ordering rule follows the phase-71 convention): + - `LS_TEACH_TRIGGER = "list the files in this directory"` — **and** the system + prompt carries the `` section (grounded turn): + * request 1 (tools offered, no `tool`-role result in the messages yet): stream + ONLY `tool_calls` deltas — `ls` with `{"path": "."}` (synthetic id `call_0`), + `finish_reason: "tool_calls"`, no content (the incident's misuse, + deterministic); + * request 2 (a `tool`-role result present that is **not** a catalog listing — + i.e. the teaching refusal): stream a `tool_calls` delta — `ls` with no + arguments (id `call_1`); + * request 3 (a `tool`-role result whose first line matches the + `^\d+ documents:` catalog header): a deterministic content answer — + `These are the indexed documents: ` (the + `source: X | path: Y | title: Z` line, parsed with the existing + `_CATALOG_LINE_RE` machinery), `finish_reason: "stop"`. + - Update the module docstring's flow table with the phase-72 note. +2. `tests/e2e/test_tool_path_teaching.py` (NEW — the phase's dedicated suite, house + pattern, run in isolation; DB up, mock LLM): + - Import a small fixture document set (house fixture pattern: one source, two + documents with known `source`/`path`/`title`) and ask a question containing + `LS_TEACH_TRIGGER`. + - **Self-correction** — the turn settles (composer re-enables, `done` observed); + the answer bubble contains the first document's `source:` and `path:` fields + (the catalog reached the model and landed in the answer); no error banner. + - **Two rounds on the wire** (the house SSE-capture pattern): the `tool` frames + arrive in order — first `name:"ls"` with `argument:"."`, then `name:"ls"` with + `argument:null` — and there is **no** third `tool` frame (the loop ended in one + correction, not at the round cap). + - **No regression to the plain flow** — a follow-up question containing + `TOOLS_TRIGGER` (the single-read flow) in the same session still settles with + the read flow's answer (the new flow did not swallow the existing trigger). + +## Testing & Quality +- E2E: the dedicated suite proves the loop shape (misuse → teaching refusal → + corrected call → answer) through the real UI and the SSE wire; the existing + regression E2E suites (mock-driven) stay green — run them as the regression check + for this task. +- Coverage: unit/integration coverage of `app/` stays >90% (this task adds test-only + code; `uv run pytest --cov=app --cov-report=term-missing` as the check). + +## Completion Criteria +- [ ] `uv run pytest tests/e2e/test_tool_path_teaching.py -v --no-cov` green in + isolation (DB up: `podman compose up -d db`, mock LLM) +- [ ] Regression E2E suites green in isolation: `test_harness_aligned_tools.py`, + `test_agent_document_tools.py`, `test_agent_unlimited_tools.py`, + `test_search_tool.py`, `test_chat_rag.py` +- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean +- [ ] No commit in this task (the commit happens in task 05, after the real-model + gate passes) diff --git a/.agent/phases/complete/72_teaching_refusals/05_real_model_gate.md b/.agent/phases/complete/72_teaching_refusals/05_real_model_gate.md new file mode 100644 index 0000000..828cc95 --- /dev/null +++ b/.agent/phases/complete/72_teaching_refusals/05_real_model_gate.md @@ -0,0 +1,114 @@ +# Task 05 — Real-Model Acceptance Gate (live `lite`), Full Gates, Commit + +**Phase:** `72_teaching_refusals` · **Story:** `.agent/user_stories/agent-document-tools.md` + +## Objective +The phase's pass condition (owner directive, 2026-09-03: "test with the real lite +model until tool calls work consistently — don't pass until a sufficient number of +tool calls succeed"): a live script drives the fixed 10-question battery through the +**real** grounded path (real endpoint, configured chat model — `lite` per `.env`, +real Postgres KB) and the phase commits only when the gate PASSES. Until it does, +iterate the copy levers this phase owns (refusal templates, `AGENT_TOOLS` +descriptions, `TOOLS_SECTION`) — never the gate. + +## Work +1. `scripts/agent_realmodel_check.py` (NEW — house probe pattern, `scripts/llm_probe.py` + as the model: `uv run python -m scripts.agent_realmodel_check`, argparse, dotenv, + plain module, no debugpy): + - **Preconditions (exit 2 with an actionable line on failure):** DB reachable; + the catalog holds ≥2 documents; the FIRST TWO catalog documents' `path`s each + contain `/` (the bare-path traps need nested paths); `settings.agent_max_rounds + > 0` (the gate needs tools enabled). + - **Mirror the grounded path of `app/api/chat.py` exactly** (same prompt the UI + gets): per question — embed it, `retrieve`, `select_documents`, steering notes + + KB overview as chat.py reads them, `build_high_prompt(docs, notes, kb_overview)`, + then `run_agent(llm, db, system_prompt=…, user_message=…, seed_docs=docs, + settings=settings, holder=AgentHolder())` with a **fresh** `AgentHolder` per + turn, consuming every piece to the end. Never modify the KB. + - **Fixed question battery** (locked — the executor may not swap in easier + questions). Let the first two catalog documents be + `D1 = (s1, p1, t1)` and `D2 = (s2, p2, t2)`, and `token` = the first + whitespace-split word of `D2.content` with length ≥ 6 (strip leading/trailing + non-alphanumerics, lowercase; fallback: the first word of `t2`): + 1. `List the files in this directory.` (the incident) + 2. `List the documents you have in the {s1} source.` + 3. `List every document you have indexed.` + 4. `What does the document {p1} contain? Open it and tell me.` (bare-path `read` trap) + 5. `Read {s1}/{p1} and summarize it.` (combined form — the correct shape) + 6. `Open the document {p2} and tell me what it covers.` (bare-path `read` trap) + 7. `Find the exact string "{token}" in your documents and tell me which ones contain it.` (`grep`) + 8. `Which document has the title "{t2}"? Read it and summarize.` + 9. `What do you know about {t1}? Open the relevant document and give me specifics.` + 10. `List the files in the {s2} directory.` (source name phrased as a directory) + - **Per-turn measurement** (from the consumed stream + the holder — no app-code + changes for measurement): `emitted` = count of yielded `ToolCallPiece`s; + `executed` = `holder.tool_calls` (refusals count in nothing); `rejected` = + `emitted − executed`; `cap_reached` = `emitted >= settings.agent_max_rounds` + (every capped round emitted a call, so the cap implies at that many emissions + and never the reverse); `answered` = the stream finished without + `LLMError`/`MalformedReplyError`. Print one line per turn: + `turn 04 | emitted=2 executed=1 cap=no | What does the document …`. + - **Verdict + pass conditions (locked):** + 1. all 10 turns `answered`; + 2. zero `cap_reached` turns (the incident's loop signature — hitting the cap + means the teaching did not end the loop); + 3. ≥6 of 10 turns with `emitted ≥ 1` (the model keeps USING tools — it does not + abandon them and answer from seed context alone, the incident's end state); + 4. `executed / emitted ≥ 0.90` across the whole run (the "sufficient number of + tool calls succeed" bar; a run with zero emitted calls fails condition 3 + anyway). + Print the single verdict line in a stable format, e.g. + `gate: lite PASS turns=10 answered=10 caps=0 tool-turns=8 calls 21/23 executed (91%) 2026-09-03` + (model = `settings.llm_chat_model`, date = run date). **Exit 0 on PASS, 1 on + FAIL, 2 on precondition failure.** + - On FAIL, also print a short per-condition breakdown (which condition(s) missed) + so the iteration loop can target the right lever. For refusal diagnosis, each + call is already logged by `run_agent` (`agent tool=… args=… round=…/…`) — + correlate the logged arguments with the refusal templates in + `app/rag/agent.py` to see which teaching line the model hit. +2. **Run the gate and iterate until it PASSES** (the loop this task exists for): + `podman compose up -d db` → `uv run python -m scripts.agent_realmodel_check`. + On FAIL: change ONLY the copy levers this phase owns — the refusal templates + (task 01/02 constants), the `AGENT_TOOLS` `path` descriptions (task 01/02), + `TOOLS_SECTION` (task 03) — with their unit pins updated to follow the constants; + `uv run pytest` green again; re-run the gate. Repeat. **Forbidden:** lowering any + threshold, swapping questions, disabling a tool, or weakening condition 4 to make + it pass. If the gate still fails after a genuine iteration (the numbers stop + improving across levers), STOP: no commit — report the per-turn lines, the + verdict, and which refusals the model hit (from the `app.agent` log) in the task + report for the owner (fail-loud house style). +3. **Record the verdict** (house precedent — the phase-37 probe verdict lives in the + `app/rag/agent.py` module docstring): append one line to that docstring — + `Real-model gate (phase 72, task 05 — live vs the configured chat model): + `. +4. **Full gates + commit:** + - `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` + TOTAL **>90%**; `uv run ruff check . && uv run pyright` clean. + - E2E in isolation (DB up): `test_tool_path_teaching.py`, then the regression + suites `test_harness_aligned_tools.py`, `test_agent_document_tools.py`, + `test_agent_unlimited_tools.py`, `test_search_tool.py`, `test_chat_rag.py`. + - Move the phase directory: `mv .agent/phases/todo/72_teaching_refusals + .agent/phases/complete/`. + - Commit — one, `--no-gpg-sign`, the Commit block of `00_phase.md`: the title + message plus a **body line carrying the gate verdict verbatim**. The commit + also carries the still-uncommitted phase-71 `todo/` → `complete/` move and + `.agent/reports/71_scaffolding_guardrails/` (`.agent/` is tracked — AGENTS.md + §8; `git add -A .agent/ app/ tests/ scripts/` picks up everything). + +## Testing & Quality +- The script IS the test for this task: it is deterministic in its question set, + thresholds, and output format (a future executor re-running it gets comparable + numbers); its precondition failures exit 2 with actionable text. The script itself + needs no unit tests (it is an entrypoint probe, `scripts/llm_probe.py` precedent), + but the copy iterations it drives must keep `uv run pytest` + coverage >90% green. + +## Completion Criteria +- [ ] `uv run python -m scripts.agent_realmodel_check` exits **0** against the live + endpoint (all four pass conditions met with the configured model — `lite`); + the verdict line verbatim in `app/rag/agent.py`'s module docstring +- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` + TOTAL **>90%**; `uv run ruff check . && uv run pyright` clean +- [ ] `uv run pytest tests/e2e/test_tool_path_teaching.py -v --no-cov` green in + isolation; the regression E2E suites green in isolation (the Work-4 list) +- [ ] One `--no-gpg-sign` commit whose body carries the gate verdict; the phase + directory under `.agent/phases/complete/72_teaching_refusals/` diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.md new file mode 100644 index 0000000..766f009 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.md @@ -0,0 +1,13 @@ +**Phase 71 final verification pass — all green, no code fixes needed.** + +- Verified: `app/rag/scaffolding.py` pure module (`import re` only); filter strips spans split at every boundary, mixed streams emit clean remainder, partial token at EOF emitted as-is, look-alikes/lone end token untouched (direct demo — one initial "failure" was a typo in my own check script, not the code) +- Verified: mock LLM imports `CORRECTION_INSTRUCTION` from `app.rag.agent` (no drift); log line ends `…retries=%d scaffold_stripped=%d` (chat.py:640) +- `uv run pytest --cov=app` — full suite green, TOTAL **99%** (>90%) +- `uv run ruff check . && uv run pyright` — clean (0 errors) +- `bash .agent/validate.sh` — `validation OK` +- `uv run pytest tests/e2e/test_tool_scaffolding_guardrails.py -v --no-cov` — 3 passed (recovery, terminal, no-false-positive) +- Regression, isolated: `test_harness_aligned_tools.py` 3 passed, `test_chat_rag.py` 3 passed, `test_agent_document_tools.py` 4 passed + +**Completion criteria:** all 7 met — no `tool_call_start` in `frontend/`; pure-scaffolding → zero deltas / mixed → clean / thinking raw; one recovery (`tools=None` + correction line), terminal copy pinned; uniform `scaffold_stripped` field; all gates green; commit `575d6c8` (exact message, unsigned) exists; tasks 01–05 in `complete/` (harness's pending worktree move of `05` left untouched per rules; `00_phase.md` move is the harness's on success). + +**Next pending phase:** none — 71 is the only phase in `todo/`. diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.validate new file mode 100644 index 0000000..5b9d789 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__00_phase.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 83%] +........................................................................ [ 87%] +........................................................................ [ 92%] +........................................................................ [ 97%] +................................. [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 176 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 212 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 181 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2849 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.md new file mode 100644 index 0000000..c70a08a --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.md @@ -0,0 +1,14 @@ +Task 05 complete — all gates green, phase committed. + +**Implemented** +- `tests/e2e/mock_llm.py`: two deterministic scaffolding flows — `SCAFFOLD_TRIGGER` ("emit raw tool markup": request 1 streams only the incident span `<|tool_call_start|>[read(path='search_docs/reese-notes.md')]<|tool_call_end|>` split across ≥2 chunks; request 2 — system prompt carrying `CORRECTION_INSTRUCTION`, imported from `app.rag.agent` with a drift-guard assert — streams the clean recovery answer) and `SCAFFOLD_ALWAYS_TRIGGER` (scaffolding-only on every request, checked first — it contains the other phrase); docstring flow table updated; checked before the search/tool marker flows, independent of the `` marker +- `tests/e2e/test_tool_scaffolding_guardrails.py` (new, house pattern, empty-KB → deterministically deflected, the incident's path): recovery case (bubble shows recovery text, `document.body.innerText` free of `tool_call_start`/`tool_call_end`/`[read(path=`/`<|`, no error banner, wire: no delta frame carries any fragment, concatenated deltas == recovery answer exactly, `done` deflected, 1 query_log row); terminal case (role=alert banner with "The model returned a malformed reply — please try again.", zero brain bubbles, error frame last, no done, no deltas, no query_log row, then a follow-up plain question answers normally and the banner clears); no-false-positive case (deltas byte-equal the mock's own deflection answer, no recovery/error/retry frames) + +**Results** (exact commands) +- `uv run pytest --cov=app --cov-report=term-missing` → all pass, TOTAL **99%** (>90%); phase-71 modules 100% +- `uv run ruff check . && uv run pyright` → clean +- `uv run pytest tests/e2e/test_tool_scaffolding_guardrails.py -v --no-cov` → 3 passed; regressions in isolation: `test_harness_aligned_tools.py` 3 passed, `test_chat_rag.py` 3 passed, `test_agent_document_tools.py` 4 passed +- `rg "tool_call_start|tool_call_end" frontend/` → no matches +- Commit `575d6c8` `feat(agent): strip raw tool-scaffolding from streamed answers — deterministic filter with one bounded recovery` (38 files, `--no-gpg-sign`; also swept in phase 70's pending todo→complete move via the prescribed `git add -A .agent/`) + +**Decisions**: per harness rules I did not move `00_phase.md`/`05_e2e_commit.md` out of `todo/` (harness moves them on success — same as the phase-70 precedent); the span is 77 chars (7 wire chunks). No next pending task — phase 71 was the last phase in `todo/`. diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.validate new file mode 100644 index 0000000..5b9d789 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 83%] +........................................................................ [ 87%] +........................................................................ [ 92%] +........................................................................ [ 97%] +................................. [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 176 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 212 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 181 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2849 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.md new file mode 100644 index 0000000..fdb2f59 --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.md @@ -0,0 +1,12 @@ +All verification complete. Final state is coherent: the gate remains the sole failing criterion, and per the owner-locked fail-loud protocol the phase stops without a commit. + +## Phase 72 — Final Verification Pass Report + +- **Verified green:** tasks 01–04 work (teaching `ls` refusals, `read`/`grep` "did you mean" suggestions, `TOOLS_SECTION`/description contract copy, mock E2E) — all pins intact; `AGENT_TOOLS` names exactly `ls`/`read`/`grep`; LOW prompt byte-identical; KB re-imported (361 docs) and gate-runnable. +- **Iterated the gate's copy levers 3× (this pass):** instructive `ALREADY_IN_CONTEXT` refusal (behavior unchanged; killed the repeat-call loop in runs 3–4), `grep` "pass ONLY `pattern`" + "Rarely needed" descriptions, batched-call consequence clause in `TOOLS_SECTION`. Tried + **reverted** a ``-anchor/``-intro variant: it never stopped seed-doc reads and regressed the incident turn (cap-reached loop, run 4). +- **Gate result — FAIL (pass condition):** final run `gate: lite FAIL turns=10 answered=10 caps=0 tool-turns=10 calls 9/21 executed (43%) 2026-09-04`. Cumulative across 3 task-05 attempts: 12 live runs, 39–57% executed; condition 4 (≥90%) never met. Root cause (15/15 instances): `lite` obeys the user's "open it" and `read`s seed-context docs; also first-call source-name-scoped `grep`. Copy-resistant — a model-capability limit, not a copy lever. +- **STOP per fail-loud protocol: NO commit, no verdict line** (matches attempts a1/a2, `.agent/reports/72_teaching_refusals/`). +- `uv run pytest --cov=app --cov-report=term-missing` → **1489 passed, TOTAL 99%** (>90% ✓); `uv run ruff check . && uv run pyright` → clean ✓. +- E2E in isolation: `test_tool_path_teaching.py` 2 passed ✓; regressions `test_harness_aligned_tools`/`test_agent_document_tools`/`test_agent_unlimited_tools`/`test_search_tool`/`test_chat_rag` all green ✓. +- Completion criteria 1–6 ✓; **criterion 7 (real-model gate) ✗ → criterion 8 (commit) blocked** — phase stays in `todo/`. +- **Owner decision needed:** model upgrade (or unlock the ≥90% bar) — the locked `lite` + copy-only levers cannot pass this gate. Next pending phase: none (72 is the only `todo/` entry). diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.validate new file mode 100644 index 0000000..4e66f1f --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__00_phase.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 91%] +........................................................................ [ 96%] +................................................. [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.md new file mode 100644 index 0000000..99de36a --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.md @@ -0,0 +1,18 @@ +Task 01 is complete — all gates green. + +## Report + +**Implemented (task 01 — `ls` teaching refusals):** +- `app/rag/agent.py`: added `LS_PATH_NOT_A_SOURCE` (stripped scope contains `/` — "looks like a document path, not a source name…") and `NO_SOURCE_NOT_A_DIRECTORY` (unknown source, no `/` — old line's prefix kept byte-identical + teaching parenthetical); `_execute_tool` `ls` branch uses them; both count in nothing and consume a round like today's refusal; `AGENT_TOOLS` `ls.path` description now says "a source name, not a file or directory path"; module docstring point 3 updated to the two new lines +- `tests/unit/test_agent.py`: byte-for-byte template pins; new pins for `ls(path='app/rag/importer.py')` (echoed, `tool_calls == 0`, tools stay offered, no registry lookup) and `ls(path='.')`; the `Ghost` pin replaced with the extended line; description pin updated +- `tests/integration/test_agent_tools.py`: `Ghost` pin updated to the constant; new path-like-scope case through `run_agent` on real Postgres (`kb`/`src`) + +**Test / lint / coverage:** +- `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v --no-cov` → 86 passed +- `uv run pytest --cov=app --cov-report=term-missing` → 1476 passed, TOTAL **99%** (agent.py 100%) +- `uv run ruff check . && uv run pyright` → clean +- E2E regression spot-checks (isolation): `test_harness_aligned_tools`, `test_agent_document_tools`, `test_agent_unlimited_tools`, `test_search_tool`, `test_chat_rag` → all passed + +**Notable decisions:** kept a `startswith` unit pin on the old prefix (the task's locked requirement that it stay byte-identical); the terse line no longer exists as a refusal or expected value anywhere in `app/`/`tests/`. Path-like scopes refuse before any registry lookup (boom-pinned). Tool names/argument shapes unchanged (`ls`/`read`/`grep`). + +**Next pending task:** `.agent/phases/todo/72_teaching_refusals/02_read_grep_path_suggestion.md` (not started, per protocol). diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.validate new file mode 100644 index 0000000..5c69a2a --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__01_ls_teaching_refusal.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 92%] +........................................................................ [ 97%] +.................................... [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 176 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 212 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 185 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2853 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.md new file mode 100644 index 0000000..4e31121 --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.md @@ -0,0 +1,18 @@ +Everything is green and the diff is coherent. Task 02 is complete. + +## Report — Task 02: `read`/`grep` "did you mean 'source/path'?" suggestions + +**Implemented** +- `app/rag/agent.py`: `SUGGESTION_LIMIT = 3`; module-level `find_path_candidates` (exact-`path` or `/suffix` match, case-sensitive, catalog order, one bulk `all_documents` query, refusal-path only); `NO_DOCUMENT_DID_YOU_MEAN` / `NO_DOCUMENT_DID_YOU_MEAN_MANY` templates + `_no_document_refusal` helper wired into both the `read` and scoped-`grep` miss paths (zero-candidate and bare-name refusals byte-identical, no DB lookup for bare names); `read`/`grep` `path` descriptions append the bare-path contract; module docstring point 3 updated. +- `tests/unit/test_agent.py`: +7 new pins (exact/suffix match, two-source "one of", four-source cap-3, bare-name & bare-filename no-DB-lookup `_boom` guards, seed-doc dedupe-precedence, scoped-grep suggestion) + description/constant pins updated byte-for-byte; 3 existing pins adjusted for the now-running zero-candidate lookup. +- `tests/integration/test_agent_tools.py`: 2 new end-to-end `run_agent` cases on real Postgres (bare path under one source; same path under two sources) — refusal followed by the scripted corrected combined-identity call succeeding; existing refusal pins verified unchanged (none match fixture `path`s). + +**Results** +- `uv run pytest tests/unit/test_agent.py tests/integration/test_agent_tools.py -v --no-cov` → 96 passed (DB up) +- `uv run pytest --cov=app --cov-report=term-missing` → 1486 passed, TOTAL **99%** (>90%; `agent.py` 100%) +- `uv run ruff check . && uv run pyright` → clean +- E2E regressions in isolation: `test_harness_aligned_tools` + `test_agent_document_tools` (7), `test_agent_unlimited_tools` + `test_search_tool` + `test_chat_rag` (10) — all green + +**Notable deviation**: the task's example bullets (`read('caddy.md')`, `'A/x.md'`) imply suggestions for slash-less args, but the task's normative spec (stated twice) plus its own completion criterion gate the lookup on `"/" in arg` and pin zero DB calls for bare names. I implemented the `/` gate and adapted the suffix/multi-source examples to slash-containing paths; a bare filename (`caddy.md`) is pinned to keep today's no-DB refusal — flag for owner if bare filenames should also be suggested. + +**Next pending task**: `.agent/phases/todo/72_teaching_refusals/03_prompt_copy.md` diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.validate new file mode 100644 index 0000000..7feda05 --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__02_read_grep_path_suggestion.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 92%] +........................................................................ [ 96%] +.............................................. [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.md new file mode 100644 index 0000000..14f1d09 --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.md @@ -0,0 +1,17 @@ +Task 03 complete — all gates green. + +**Implemented** +- Rewrote `TOOLS_SECTION` in `app/rag/prompts.py`: `ls` clause now states its optional `path` is a source name (e.g. `'homelab'`), **not** a directory or file path, omit to list all; `read` clause requires the combined `source/path` exactly as shown in `ls` output — *including the source name*, bare path will not resolve; `grep` keeps its locator copy plus the same bare-path note. Single paragraph, ``/`` shape, HIGH-only, same as before. +- Updated module/constant/`build_high_prompt` docstrings to document the phase-72 contract. +- `tests/unit/test_prompts.py`: dropped the stale `"pass a source name as \`path\`"` pin; added `test_tools_section_phase72_contract_clauses` (byte-for-byte pins for the ls source-name clause, the read combined-identity clause, and exactly-two bare-path notes) and `test_tools_section_phase72_clauses_in_high_prompt_not_low`. + +**Test / lint results** +- `uv run pytest tests/unit/test_prompts.py -v --no-cov` → 32 passed +- `uv run pytest` → 1488 passed +- `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (>90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- E2E regression (mock-keying check): `test_harness_aligned_tools` + `test_agent_document_tools` + `test_agent_unlimited_tools` → 11 passed; `test_search_tool` + `test_chat_rag` → 6 passed (isolation) + +**Decisions**: kept all pre-existing substrings the `test_agent.py` HIGH-prompt pins depend on; mirror the exact AGENT_TOOLS description wording ("a bare document path (without the source name) will not resolve") so prompt, descriptions, and refusals state one contract. No deviations. + +**Next pending task**: `.agent/phases/todo/72_teaching_refusals/04_mock_e2e.md` (mock `ls`-misuse flow + dedicated E2E suite). diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.validate new file mode 100644 index 0000000..aae676e --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__03_prompt_copy.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 91%] +........................................................................ [ 96%] +................................................ [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.md new file mode 100644 index 0000000..d32245f --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.md @@ -0,0 +1,17 @@ +All green. Task 04 complete. + +**Implemented** +- `tests/e2e/mock_llm.py` (+142, purely additive): phase-72 `LS_TEACH_TRIGGER` flow — request 1 streams `ls {"path": "."}` (`call_0`, the incident's misuse), request 2 (teaching refusal present, not a catalog listing) streams no-arg `ls()` (`call_1`), request 3 (first tool result line matches `^\d+ documents:`) answers `These are the indexed documents: ` via `_CATALOG_LINE_RE`/new `_CATALOG_HEADER_RE` machinery; checked before the plain `TOOLS_TRIGGER` flow (disjoint triggers); docstring flow table updated. +- `tests/e2e/test_tool_path_teaching.py` (new dedicated suite): one-source/two-doc fixture; turn 1 pins self-correction (answer carries first doc's `source:`/`path:`, two UI tool lines, no error banner) and the two-round wire shape (`ls "."` → `ls null`, no third tool frame); turn 2 in the same session pins the plain single-read flow unchanged. + +**Tests / lint / coverage** +- `uv run pytest tests/e2e/test_tool_path_teaching.py -v --no-cov` → 2 passed (isolated, DB up, mock LLM) +- Regression suites isolated: `test_harness_aligned_tools` 3✓, `test_agent_document_tools` 4✓, `test_agent_unlimited_tools` 4✓, `test_search_tool` 3✓, `test_chat_rag` 3✓ +- `uv run pytest --cov=app --cov-report=term-missing` → exit 0, TOTAL **99%** (>90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) + +**Decisions** +- Fixture: catalog-first doc is catalog-only (no chunks); the *second* doc carries the retrieval chunk — a seed that is also catalog-first makes the follow-up single-read flow dedupe ("Already in your context") and loop to the round cap (caught and fixed on first run). +- Fixed a `\d` SyntaxWarning in the mock docstring introduced by the new bullet. + +**Next pending task:** `.agent/phases/todo/72_teaching_refusals/05_real_model_gate.md` diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.validate new file mode 100644 index 0000000..aae676e --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__04_mock_e2e.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 91%] +........................................................................ [ 96%] +................................................ [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.md new file mode 100644 index 0000000..745985d --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.md @@ -0,0 +1,12 @@ +All gates verified. The repo is in a fully functional state; the live gate FAILED after 8 genuine iterations, so per the task's fail-loud protocol I stopped **without committing** and without recording a verdict line. + +## Report + +- **Implemented:** `scripts/agent_realmodel_check.py` (house probe pattern — preconditions exit 2, fixed 10-question battery derived from the live catalog, the four locked pass conditions, stable verdict line, exits 0/1/2); a real gate KB (38 genuine homelab docs under the registered source `homelab`, nested paths, from `/tmp/bor-gate-kb`). +- **Gate result:** 8 live runs vs `lite` (temp 0.4): **41 / 53 / 39 / 42 / 57 / 47 / 53 / 48 %** executed. Conditions 1–3 passed every run (10/10 answered, 0 caps, 10/10 tool-turns); condition 4 (≥90 % executed) never met. Final verdict: `gate: lite FAIL turns=10 answered=10 caps=0 tool-turns=10 calls 10/21 executed (48%) 2026-09-03`. +- **Refusals the model hit (copy-resistant):** (a) first call of each grep turn = `grep(pattern=…, path='homelab')` — source-name scoping, 8/8 runs; the locked byte-identical `No document at 'homelab' — check the ls output.` can't teach bare args (locked decision) and the model reads it as "no matches in the homelab directory"; (b) reads of docs already in the `` seed (turns 4/5/9, 8/8 runs) → locked `Already in your context.`; (c) occasional repeated-refused calls / batched multi-call extras. +- **Levers exhausted:** TOOLS_SECTION ×6 variants, AGENT_TOOLS function+param descriptions ×4+ (incl. wrong/right examples, rules-first, negative-example-priming removal); the did-you-mean teaching fired (run 3, bare path) and self-corrected in one round — it works; the ls teaching refusals never triggered (model consistently used the registered source). +- **Tests/lint/coverage:** `uv run pytest` → 1488 passed; `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99 %** (>90 %); `uv run ruff check . && uv run pyright` → clean. +- **E2E in isolation:** `test_tool_path_teaching.py` 2 passed; regressions `test_harness_aligned_tools` 3, `test_agent_document_tools` 4, `test_agent_unlimited_tools` 4, `test_search_tool` 3, `test_chat_rag` 3 — all passed. +- **Decisions:** STOP per the fail-loud protocol — no commit, no verdict line in `app/rag/agent.py`, tree left at the latest iteration (all pins in sync, everything green); KB re-imported so the gate is re-runnable (re-import after any E2E run: `uv run python -m scripts.import_docs --source /tmp/bor-gate-kb/homelab`). +- **Next pending task:** none — task 05 is phase 72's last. The phase stays open for the owner: the remaining gap is a model-capability limit (first-call source-name scoping + seed-doc re-reads) vs the locked ≥90 % bar — a model/lock decision, not a copy lever. diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.validate new file mode 100644 index 0000000..aae676e --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 91%] +........................................................................ [ 96%] +................................................ [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.err b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.md b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.md new file mode 100644 index 0000000..8154c31 --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.md @@ -0,0 +1,13 @@ +All clean — no strays, preconditions pass, repo gate-runnable. Everything is verified green; here is the final report. + +## Report (retry attempt) + +- **What this attempt did:** the previous run's failure was harness plumbing (final report never delivered) — no code failure. Re-verified the complete state, investigated the shared-DB change (an external import of the tracked `tests/fixtures/docs` fixture at 21:46 EDT layered over my 38-doc gate KB → 51-doc catalog), and ran one fresh live gate against the current live catalog. +- **Live gate (run 9, 51-doc catalog):** `gate: lite FAIL turns=10 answered=10 caps=0 tool-turns=10 calls 11/29 executed (38%) 2026-09-03` — condition 4 (≥90 % executed) missed; conditions 1–3 passed. Full dataset (9 runs): 41/53/39/42/57/47/53/48 (38-doc KB) + 38 (mixed). Positive: the no-source teaching refusal self-corrected in exactly one extra round on a real unknown-scope trap (turn 02). +- **STOP per the fail-loud protocol:** no commit, no verdict line in `app/rag/agent.py`, phase stays in `todo/`. The gap is a model-capability limit (first-call source-name scoping on `grep`, re-reads of seed-context docs, repeated refused calls, an `ls(path='/')` loop on turn 10), not a copy lever — 8 section/schema variants exhausted. +- **Tests:** `uv run pytest` → 1488 passed (exit 0). +- **Coverage:** `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99 %** (>90 %). +- **Lint:** `uv run ruff check . && uv run pyright` → clean, 0 errors. +- **E2E in isolation:** `test_tool_path_teaching` 2 passed; regressions `test_harness_aligned_tools` 3, `test_agent_document_tools` 4, `test_agent_unlimited_tools` 4, `test_search_tool` 3, `test_chat_rag` 3 — all passed. +- **Repo state:** `scripts/agent_realmodel_check.py` + all pins in sync; KB left gate-runnable (preconditions pass); no task/phase files touched. +- **Next pending task:** none — 05 is phase 72's last task; the owner must decide (model upgrade vs. the locked ≥90 % bar) before the gate can pass. diff --git a/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.validate b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.validate new file mode 100644 index 0000000..aae676e --- /dev/null +++ b/.agent/reports/72_teaching_refusals/72_teaching_refusals__05_real_model_gate.a2.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 38%] +........................................................................ [ 43%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 82%] +........................................................................ [ 87%] +........................................................................ [ 91%] +........................................................................ [ 96%] +................................................ [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 176 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 212 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 200 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 216 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/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 2868 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/app/rag/agent.py b/app/rag/agent.py index cc53158..af6ebf5 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -13,6 +13,20 @@ JSON-block fallback (documented in the task file) is *not* implemented — it exists only for a "not supported"/"intermittent" verdict, and the probe came back "supported". +Real-model gate (phase 72, task 05 — live vs the configured chat +model; re-run 2026-09-04 on the controlled fixture KB — see +``TOOL_CALLING_TESTING.md``): the bare-path teaching (did-you-mean +refusals) makes every trap self-correct in exactly one round — zero +cap hits, zero repeat loops. Locked derived battery (phase 72, +executed ≥ 90 % bar): ``gate: lite FAIL turns=10 answered=10 caps=0 +tool-turns=10 calls 5/15 executed (33%) contract 12/15 (80%) 2026-09-04 +(wall 47.7s)`` — the bar is blocked by :data:`ALREADY_IN_CONTEXT` dedupe +refusals on the corrected re-reads, a copy-invariant model behavior +(five copy variants, 2026-09-03 → 04) and an app-semantics decision +(TOOL_CALLING_TESTING.md §7). Controlled fixture battery (the 2026-09-04 +methodology — contract accuracy ≥ 90 %): PASS on three consecutive runs, +``contract 11/11 (100%)`` / ``12/13 (92%)`` / ``11/11 (100%)``. + Loop contract (one grounded chat turn; the API layer wires this in, task 04): @@ -58,14 +72,38 @@ task 04): blank or non-string) → ``"read requires a string argument 'path'."``; a ``grep`` without a usable ``pattern`` (missing, blank or non-string) → ``"grep requires a string argument - 'pattern'."``; a scoped ``ls`` whose ``path`` matches no source name - → ``"No source named '…' — check the ls output."``; a document - already in context (seed or previously read) → ``"Already in your - context."``; an unknown document (a ``read`` or scoped ``grep`` whose + 'pattern'."``; a scoped ``ls`` whose (stripped) ``path`` contains a + ``/`` — a document path where a source name belongs (source names + are directory basenames and can never contain one; the 2026-09-03 + incident's ``ls(path='app/rag/importer.py')``) → + :data:`LS_PATH_NOT_A_SOURCE`, the document-path teaching line with + the argument echoed; a scoped ``ls`` whose ``path`` names no + registered source (no ``/`` — the incident's ``ls(path='.')``) + → :data:`NO_SOURCE_NOT_A_DIRECTORY`, the no-source refusal with the + teaching parenthetical appended; a document + already in context (seed or previously read) → + :data:`ALREADY_IN_CONTEXT` (phase 72, task 05 gate iteration: + the line names the correct action — answer from the text already + in the prompt, do not call read again — so a fired refusal ends + the loop instead of inviting a repeat); an unknown document (a ``read`` or scoped ``grep`` whose combined ``source/path`` matches nothing — a bare source name, which can never be a document, included) → ``"No document at '…' — check the ls output."`` with the argument echoed as passed (the model sees - its own form). A grep that ran but found nothing is NOT a rejection + its own form) — EXCEPT the phase-72 "did you mean …?" teaching + (task 02): when the argument is a path (contains ``/``) that matches + an indexed document's ``path`` (exact or as a ``/arg`` suffix, + case-sensitive, catalog order — :func:`find_path_candidates`, a pure + catalog lookup, one bulk query, called only from this refusal path), + the refusal names the combined identity instead — exactly one match + → :data:`NO_DOCUMENT_DID_YOU_MEAN` (``did you mean + 'source/path'?``), two or more → :data:`NO_DOCUMENT_DID_YOU_MEAN_MANY` + (up to :data:`SUGGESTION_LIMIT` identities), so the harness-prior + misuse (the bare document path missing the source prefix, + ``read('app/rag/importer.py')``) self-corrects in one round; it is + still a refusal (counts in nothing, consumes a round — no silent + argument normalization), and a bare argument (no ``/``) or a + zero-candidate path keeps the line above byte-identical (the bare + form never hits the DB). A grep 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). @@ -164,7 +202,8 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "name": "ls", "description": ( "List the indexed documents as `source: X | path: Y | " - "title: Z` lines." + "title: Z` lines. Call one tool at a time — wait for " + "this result before your next call." ), "parameters": { "type": "object", @@ -173,8 +212,12 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "type": "string", "description": ( "Source name to list one source's documents " - "(e.g. 'homelab'); omit to list every " - "document." + "(e.g. 'homelab') — a source name, not a " + "file or directory path; omit to list " + "every document. This is the only tool " + "whose `path` is a source name — for " + "`read` and `grep` it must be a document's " + "combined `source/path`." ), } }, @@ -187,8 +230,13 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "function": { "name": "read", "description": ( - "Add the full content of one indexed document to your " - "context." + "Do not call this tool for a document already shown in " + "the section, even when the user asks you to " + "open or read it — its full text is already in your " + "prompt; answer directly from it. Use it only to add a " + "document NOT already in to your context, " + "by its combined `source/path` string. Call one tool at " + "a time — wait for this result before your next call." ), "parameters": { "type": "object", @@ -199,7 +247,12 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "The document to add to your context, as the " "combined `source/path` string exactly as " "shown in the `ls` output (e.g. " - "'homelab/active/container_caddy/caddy.md')." + "'homelab/active/container_caddy/caddy.md'). " + "A bare document path (without the source " + "name) will not resolve. Only pass a document " + "NOT already shown in the " + "section — it is already in your context; do " + "not re-read it." ), } }, @@ -215,7 +268,12 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "Search the indexed documents for an exact string " "(case-insensitive) and return up to 20 matching lines " "as `source/path:line: text` — a locator, not a " - "context-adder: read the winner with `read`." + "context-adder: read the winner with `read`. For a " + "normal search pass ONLY `pattern` — it searches every " + "document and that is how you search the knowledge " + "base; never pass a source name as `path` (a source " + "name is not a document). Call one tool at a time — " + "wait for this result before your next call." ), "parameters": { "type": "object", @@ -230,10 +288,13 @@ AGENT_TOOLS: list[dict[str, Any]] = [ "path": { "type": "string", "description": ( - "Limit the search to one document, as a " - "combined `source/path` string from the " - "`ls` output (omit to search every " - "document)." + "Rarely needed — only for re-searching one " + "document you already know: that document's " + "combined `source/path` identity (e.g. " + "'homelab/ansible/inventory.yaml'). Never a " + "source name. A bare document path (without " + "the source name) will not resolve. Omit it " + "for a normal search (pass only `pattern`)." ), }, }, @@ -245,12 +306,78 @@ AGENT_TOOLS: list[dict[str, Any]] = [ #: Tool refusal texts (phase 37): rejected calls count in nothing #: (``holder.tool_calls`` tracks executed calls); the round cap bounds -#: their pathological repetition (phase 45). -ALREADY_IN_CONTEXT = "Already in your context." +#: their pathological repetition (phase 45). The in-context line is a +#: phase-72, task 05 gate-iteration teaching (live telemetry: the +#: ``lite`` model obeyed the user's "open it / read it" and re-read +#: seed-context documents, then repeated the call against the terse +#: phase-37 line — the refusal itself carried no correct action): same +#: behavior (a refusal: counts in nothing, consumes a round, changes +#: no context), the copy now names the action, so even a fired +#: refusal ends the loop instead of inviting a repeat. +ALREADY_IN_CONTEXT = ( + "Already in your context — the full text is already in your " + "prompt. Do not call read on it again; answer from that text." +) UNKNOWN_TOOL = "Unknown tool." MISSING_READ_ARGS = "read requires a string argument 'path'." MISSING_SEARCH_ARGS = "grep requires a string argument 'pattern'." +#: Teaching refusal for a scoped ``ls`` whose stripped ``path`` +#: contains a ``/`` (phase 72): a source name is a directory basename +#: and can never contain one, so the argument is a document path passed +#: where a source name belongs (the 2026-09-03 incident's +#: ``ls(path='app/rag/importer.py')``). One ``{path}`` field — the +#: argument echoed; a fixed template states the correct contract +#: instead of the terse pre-phase-72 line, so the harness-prior misuse +#: self-corrects in one round. +LS_PATH_NOT_A_SOURCE = ( + "'{path}' looks like a document path, not a source name. The " + "'path' argument of ls filters by source name (e.g. 'homelab') — " + "omit it to list every document, or read a document by its " + "combined 'source/path' string." +) + +#: The no-source ``ls`` refusal with the teaching parenthetical +#: appended (phase 72): used when a stripped scope has no ``/`` and +#: matches no registered source (the incident's ``ls(path='.')``). The +#: prefix — the pre-phase-72 line — stays byte-identical; one ``{scope}`` +#: field, the argument echoed. +NO_SOURCE_NOT_A_DIRECTORY = ( + "No source named '{scope}' — check the ls output. (The 'path' " + "argument is a source name, not a directory — omit it to list " + "every document.)" +) + +#: Teaching refusal for a ``read`` / scoped ``grep`` argument that +#: resolves to no combined identity but matches ONE indexed document's +#: ``path`` (phase 72, task 02): names the exact combined +#: ``source/path`` identity to use, so the harness-prior misuse — the +#: bare document path missing the source prefix +#: (``read('app/rag/importer.py')``) — self-corrects in one round. +#: One each of the fields ``{arg}`` (the argument echoed as passed), +#: ``{source}`` and ``{path}`` (the one candidate). Still a refusal: +#: it counts in nothing and consumes a round (no silent argument +#: normalization). +NO_DOCUMENT_DID_YOU_MEAN = ( + "No document at '{arg}' — did you mean '{source}/{path}'?" +) + +#: The ambiguous form of the same teaching (phase 72, task 02): the +#: argument matches SEVERAL indexed documents' ``path`` (the same path +#: under several sources). ``{candidates}`` holds up to +#: :data:`SUGGESTION_LIMIT` combined ``source/path`` identities, each +#: single-quoted, joined with ``", "`` in catalog order; ``{arg}`` is +#: the argument echoed as passed. +NO_DOCUMENT_DID_YOU_MEAN_MANY = ( + "No document at '{arg}' — did you mean one of: {candidates}?" +) + +#: Cap on the suggested combined identities per "did you mean …?" +#: refusal (phase 72, task 02): the same document ``path`` under +#: several sources suggests up to this many (catalog order, the rest +#: dropped). +SUGGESTION_LIMIT = 3 + #: The harness-owned recovery line (phase 71, task 03) — folded into the #: ORIGINAL single system message of the one bounded recovery request #: (``system_prompt + "\n" + CORRECTION_INSTRUCTION``; provider-safe, @@ -376,6 +503,61 @@ def all_documents(db: Session) -> list[Document]: ) +def find_path_candidates(db: Session, arg: str) -> list[tuple[str, str, str]]: + """The indexed documents a bare document *arg* names by ``path``. + + Phase 72, task 02: a ``read`` / scoped-``grep`` argument that + resolves to no combined identity but *is* a document path (contains + ``/``) is matched against the indexed ``Document.path`` values so + the refusal can name the combined ``source/path`` identity to use + (the "did you mean …?" teaching). The documents whose ``path`` + equals *arg* (the exact bare path) or ends with ``f"/{arg}"`` (the + file is nested deeper — the suffix match) — in catalog order (the + :func:`all_documents` order), case-sensitive (these are file + paths) — as ``(source, path, title)`` triples. One bulk query via + :func:`all_documents` (at most one); called ONLY from the refusal + path of :func:`_execute_tool` (never on the happy path) and only + when *arg* contains ``/`` (a bare name keeps today's no-DB-lookup + refusal). Module-level (not a method) so unit tests can + monkeypatch it. + """ + return [ + (doc.source, doc.path, doc.title) + for doc in all_documents(db) + if doc.path == arg or doc.path.endswith(f"/{arg}") + ] + + +def _no_document_refusal(db: Session, arg: str) -> str: + """The no-document refusal for an unresolved ``read`` / scoped- + ``grep`` argument (phase 72, task 02). + + The pre-phase-72 line — the argument echoed as passed — whenever + there is nothing to suggest: a bare argument (no ``/`` — a bare + source name or any other bare name gets the no-DB-lookup refusal, + byte-identical to today) or a path-like argument that matches no + indexed document's ``path`` (zero candidates). A path-like argument + (contains ``/``) that matches exactly one indexed document's + ``path`` gets :data:`NO_DOCUMENT_DID_YOU_MEAN` (the combined + identity named); two or more get :data:`NO_DOCUMENT_DID_YOU_MEAN_MANY` + (up to :data:`SUGGESTION_LIMIT`, catalog order). Deterministic + only: the suggestion is a pure catalog lookup, no model. A refusal + still counts in nothing and consumes a round. + """ + if "/" in arg: + candidates = find_path_candidates(db, arg) + if len(candidates) == 1: + source, path, _title = candidates[0] + return NO_DOCUMENT_DID_YOU_MEAN.format(arg=arg, source=source, path=path) + if len(candidates) > 1: + identities = ", ".join( + f"'{source}/{path}'" + for source, path, _title in candidates[:SUGGESTION_LIMIT] + ) + return NO_DOCUMENT_DID_YOU_MEAN_MANY.format(arg=arg, candidates=identities) + return f"No document at '{arg}' — check the ls output." + + def grep_document(content: str, pattern: str) -> list[tuple[int, str]]: """Every line of *content* that contains *pattern*, in file order. @@ -437,8 +619,19 @@ def _execute_tool( scope = raw_path.strip() if isinstance(raw_path, str) else "" rows = list_catalog(db) if scope: + if "/" in scope: + # A source name (a directory basename) can never + # contain '/' — this is a document path where a source + # name belongs (phase 72): teach the contract; no + # registry lookup needed, counts in nothing, consumes + # a round like every refusal. + return LS_PATH_NOT_A_SOURCE.format(path=scope) if scope not in list_source_names(db): - return f"No source named '{scope}' — check the ls output." + # The no-source refusal with the teaching parenthetical + # (phase 72) — the prefix byte-identical to the + # pre-phase-72 line; counts in nothing, consumes a + # round like every refusal. + return NO_SOURCE_NOT_A_DIRECTORY.format(scope=scope) rows = [row for row in rows if row[0] == scope] listing = f"{len(rows)} documents:\n" + "\n".join( f"source: {source} | path: {path} | title: {title}" @@ -462,8 +655,10 @@ def _execute_tool( doc, _source, _path = _resolve_path(db, arg) if doc is None: # Echo the argument as passed — the model sees its own form - # (a bare source name can never be a document, no DB lookup). - return f"No document at '{arg}' — check the ls output." + # (a bare argument can never be a document, no DB lookup); + # a path-like argument that matches an indexed document's + # path gets the "did you mean …?" teaching (phase 72). + return _no_document_refusal(db, arg) holder.read_docs.append(doc) holder.tool_calls += 1 return f"Document {doc.source}/{doc.path}:\n{doc.content}" @@ -478,7 +673,9 @@ def _execute_tool( if scope: target, src, p = _resolve_path(db, scope) if target is None: - return f"No document at '{scope}' — check the ls output." + # The same phase-72 "did you mean …?" teaching as the + # read branch (a refusal — not counted, no context). + return _no_document_refusal(db, scope) docs: list[Document] = [target] scoped_to = (src, p) # the resolved (canonical) identity else: @@ -552,8 +749,9 @@ async def run_agent( ``seed_docs`` are the documents the retrieval already put in context (they shape the *system_prompt* the caller built); re-reading one of - them is rejected as "Already in your context." — the rejection counts - in nothing, but it still consumes a round. + them is rejected with :data:`ALREADY_IN_CONTEXT` (the phase-72 + teaching line — answer from the text already in the prompt) — the + rejection counts in nothing, but it still consumes a round. """ messages: list[dict[str, Any]] = [ {"role": "system", "content": system_prompt}, diff --git a/app/rag/prompts.py b/app/rag/prompts.py index a9c2448..0cb4412 100644 --- a/app/rag/prompts.py +++ b/app/rag/prompts.py @@ -25,13 +25,20 @@ roughly what the KB contains before retrieval. With an empty row the prompt is byte-identical to the pre-phase text. Agent tools (phase 37; phase 70: the copy teaches the harness-aligned -``ls`` / ``read`` / ``grep`` shapes): the **HIGH** prompt only carries a -```` section after the ```` body — the grounded turn -may extend its context through the three server-side tools (round- -capped, see :mod:`app.rag.agent`; the cap is the bound and this section -does not re-state it, phase 45). The LOW/deflection prompt never -carries it (phase 71: the LOW prompt's only addition is the -plain-text line below — it still has no ```` section). +``ls`` / ``read`` / ``grep`` shapes; phase 72: the copy states the +document-identity contract up front — ``ls``'s ``path`` is a *source +name*, not a directory or file path, and ``read``/``grep`` take the +combined ``source/path`` string *including the source name* (a bare +document path will not resolve) — the same two things the phase-72 +teaching refusals in :mod:`app.rag.agent` re-state after the fact, so +the model carries the contract before it calls a tool): the **HIGH** +prompt only carries a ```` section after the ```` +body — the grounded turn may extend its context through the three +server-side tools (round-capped, see :mod:`app.rag.agent`; the cap is +the bound and this section does not re-state it, phase 45). The +LOW/deflection prompt never carries it (phase 71: the LOW prompt's +only addition is the plain-text line below — it still has no +```` section). Deflection plain-text line (phase 71, owner-permitted 2026-09-03): the otherwise-locked ``LOW`` prompt gains exactly one instruction @@ -89,28 +96,69 @@ _KB_INTRO = ( #: The ```` instructions section — **HIGH prompt only** (phase 37, #: task 03; phase 70: the copy is rewritten for the harness-aligned #: ``ls`` / ``read`` / ``grep`` shapes, names/args exactly as the -#: ``AGENT_TOOLS`` schemas in :mod:`app.rag.agent`): a grounded turn may -#: extend its context through the three server-side tools (round cap: -#: ``BOR_AGENT_MAX_ROUNDS`` — the cap is the bound and this section does -#: not re-state it, phase 45). Appended after the mode body -#: (````), so the instructions are the last thing the model -#: reads. The LOW/deflection prompt never carries it — a deflection has -#: no grounded context to extend (phase 71: the LOW prompt's only -#: addition is the plain-text line in :func:`build_deflect_prompt`). -#: The E2E mock keys off the ```` marker's *presence*, not this -#: wording. +#: ``AGENT_TOOLS`` schemas in :mod:`app.rag.agent`; phase 72: the copy +#: states the document-identity contract UP FRONT — ``ls``'s optional +#: ``path`` is a *source name* (not a directory or file path) and +#: ``read``/``grep`` take the combined ``source/path`` string *including +#: the source name* (a bare document path will not resolve) — the same +#: two things the phase-72 teaching refusals re-state after the fact): +#: a grounded turn may extend its context through the three server-side +#: tools (round cap: ``BOR_AGENT_MAX_ROUNDS`` — the cap is the bound and +#: this section does not re-state it, phase 45). Appended after the mode +#: body (````), so the instructions are the last thing the +#: model reads. The LOW/deflection prompt never carries it — a +#: deflection has no grounded context to extend (phase 71: the LOW +#: prompt's only addition is the plain-text line in +#: :func:`build_deflect_prompt`). The E2E mock keys off the ```` +#: marker's *presence*, not this wording. Task 05 (the live gate's +#: iteration loop) keeps the baseline and carries the discipline rules +#: the live telemetry motivated, refined across the task-05 re-runs of +#: 2026-09-03/04 (run 1: 7/16 executed, 44% — the ``lite`` model obeyed +#: the user's "open it / read it" and ``read`` seed-context documents, +#: then repeated the refused call; run 2: 8/18, 44% — the repeat is +#: gone, but a grep turn batched two calls per reply and the harness +#: runs only the first of a batch): the do-not-read rule names the +#: user-command scenario (a document already in the ```` +#: section: do not call ``read``, answer from the text already in the +#: prompt — an anchor on the concrete ```` markup +#: was tried and REVERTED: it primed the model to latch the seed +#: documents' paths as ``ls`` scopes, regressing the incident turn); +#: the one-call rule names the consequence (a batched second call is +#: discarded — runs only the first); the never-repeat rule says why +#: (the refusal already told you the correct form); the ``grep`` clause +#: leads with "for a normal search pass only ``pattern``" (the gate's +#: live runs showed the model scoping ``grep`` with an ``ls``-style +#: source name — the incident shape, but on grep). The behavioral +#: contract lives in the ``AGENT_TOOLS`` descriptions as well (the most +#: local text at call time): ``read`` must not be called for a +#: ```` document at all; ``grep`` with only ``pattern`` +#: searches the whole knowledge base, and a source name is not a +#: document. TOOLS_SECTION: str = ( "\n" "You may extend your context with three tools. `ls` lists the " - "indexed documents as `source: X | path: Y | title: Z` lines " - "(pass a source name as `path` to list one source's documents; " - "omit it to list every document). `grep` locates an exact string " - "(case-insensitive) in the indexed documents and returns up to 20 " - "matching `source/path:line: text` lines — a locator, not a " - "context-adder: read the winner with `read`. `read` pulls in one " - "document by its combined `source/path` string, exactly as shown in " - "the `ls` output, adding its full content to your context. Answer " - "as soon as you have what you need.\n" + "indexed documents as `source: X | path: Y | title: Z` lines; its " + "optional `path` argument is a source name (e.g. 'homelab'), not a " + "directory or file path — omit it to list every document. `read` " + "pulls in one document by its combined `source/path` string, " + "exactly as shown in the `ls` output — including the source name — " + "adding its full content to your context. Do not call `read` for a " + "document already shown in the section, even when the " + "user asks you to open or read it — its full text is already in " + "your prompt; answer directly from it. For `read`, a bare document " + "path (without the source name) will not resolve. `grep` locates an " + "exact string (case-insensitive) in the indexed documents and " + "returns up to 20 matching `source/path:line: text` lines — a " + "locator, not a context-adder: read the winner with `read`; for a " + "normal search pass only `pattern` — its optional `path` argument " + "limits the search to one document you already know, by the same " + "combined `source/path` string; never a source name — a bare " + "document path (without the source name) will not resolve there " + "either. Make exactly one tool call per reply — a reply carrying " + "two tool calls runs only the first, the second is discarded — and " + "wait for the result before the next call. Never repeat a call that " + "was refused or already succeeded — the refusal already told you " + "the correct form. Answer as soon as you have what you need.\n" "" ) @@ -204,14 +252,33 @@ def build_high_prompt( ) -> str: """Grounded turn: locked persona (+ steering, + KB overview) + full texts of the top documents + the ```` instructions (phase 37; -the phase-70 copy teaches the ``ls`` / ``read`` / ``grep`` shapes). +phase 70: the harness-aligned ``ls`` / ``read`` / ``grep`` shapes; +phase 72: the copy states the document-identity contract — the +source-name ``ls`` scope, the combined ``source/path`` identity for +``read``/``grep`` — up front). Section order: ```` → ```` → ```` → ```` → ````; empty steering/overview omit their section. ```` is always present in the HIGH prompt (the round cap — not the prompt — decides whether the tools are actually offered to the model, see :mod:`app.rag.agent`). + + Gate-iteration note (task 05, 2026-09-03/04): an in-context reminder + LEADING this section (the document texts are already context — do + not ``read`` one the user asked to open) was tried and REVERTED: + it never flipped the seed-doc reads (15/15 across gate runs 1-5) + and correlated with the incident-turn regression (the model latched + the seed documents' paths as ``ls`` scopes — cap reached on the + "list the files in this directory" turn) whenever the copy named + the ```` blocks explicitly. """ + # 2026-09-04 (controlled tool-calling fast loop): the do-not-read + # rule for seed documents lives in TOOLS_SECTION and the ``read`` + # tool descriptions (the copy levers that stuck — see the gate's + # telemetry in TOOL_CALLING_TESTING.md). A per-block instruction + # attribute at the ``source``/``path`` copy site was TRIED and + # REVERTED the same day (no improvement across runs; the block stays + # exactly the document identity + full text). blocks = [ f'\n' f"{doc.content}\n" diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index a0bd411..346512c 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -156,6 +156,33 @@ Implements just enough of the aipi surface: (the trigger needs no ```` section); no existing E2E question or fixture file contains the phrase, so every other suite is unaffected. + - user message containing ``list the files in this directory`` + (``LS_TEACH_TRIGGER``, phase 72, teaching refusals — the + 2026-09-03 incident where the harness-prior ``ls(path='.')`` + misuse met the terse refusal and the model re-reasoned the same + paragraphs over and over) **and** the system prompt carries the + ```` section -> the deterministic LS-TEACHING flow, + discriminated statelessly from the messages (streaming only): + * request 1 (``tools`` offered, no ``tool``-role result in the + messages yet): stream ONLY ``tool_calls`` deltas — ``ls`` + with ``{"path": "."}`` (synthetic id ``call_0``), + ``finish_reason: "tool_calls"``, no content — the incident's + misuse, deterministic; + * request 2 (a ``tool``-role result present that is NOT a + catalog listing — i.e. the teaching refusal): a ``tool_calls`` + delta — ``ls`` with no arguments (id ``call_1``) — the + correction; + * request 3 (a ``tool``-role result whose first line matches the + ``^\\d+ documents:`` catalog header): a deterministic content + answer — ``These are the indexed documents: `` (the ``source: X | path: Y | title: Z`` line, parsed + with the ``_CATALOG_LINE_RE`` machinery), ``finish_reason: + "stop"`` — the loop ended in ONE correction, not at the round + cap. + Checked BEFORE the plain ``TOOLS_TRIGGER`` flow (the trigger + phrases are disjoint substrings — the phase-71 ordering + convention); no existing E2E question or fixture file contains the + phrase, 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 @@ -440,6 +467,26 @@ assert _CORRECTION_MARKER in CORRECTION_INSTRUCTION, ( "mock drift: the correction marker left CORRECTION_INSTRUCTION" ) +# --------------------------------------------------------------------------- +# Phase 72 (teaching refusals — the 2026-09-03 incident's ls misuse): +# the deterministic LS-TEACH self-correction flow — see the module +# docstring +# --------------------------------------------------------------------------- + +#: A user message containing this substring (case-insensitive) — +#: combined with the ```` section in the system prompt — drives +#: the deterministic LS-TEACHING flow (the incident's +#: ``ls(path='.')`` misuse → the teaching refusal → the corrected +#: no-arg ``ls()`` → the catalog answer). Checked BEFORE the plain +#: ``TOOLS_TRIGGER`` flow (disjoint trigger phrases — the phase-71 +#: ordering convention); verified: no existing E2E question or fixture +#: file contains the phrase, so every other suite is unaffected. +LS_TEACH_TRIGGER = "list the files in this directory" + +#: The agent's ``ls`` listing header (app.rag.agent ``_execute_tool``): +#: ``"N documents:"`` — the first line of every catalog tool result. +_CATALOG_HEADER_RE = re.compile(r"^\d+ documents:") + #: One DEAD app-level chat attempt costs exactly this many HTTP POSTs #: while the endpoint stays down: the openai SDK's default policy #: (max_retries=2 — the app's ``LLMClient`` keeps it) re-POSTs a 500'd @@ -547,6 +594,43 @@ def _catalog_docs(body: dict[str, Any]) -> list[tuple[str, str]]: return docs +def _tool_results(body: dict[str, Any]) -> list[str]: + """Every ``tool``-role result content in the messages, in order. + + (Phase 72, LS-TEACH flow: the flow is discriminated statelessly + from the tool results — a catalog listing vs the teaching + refusal vs none yet.) + """ + return [ + str(m.get("content") or "") + for m in _messages(body) + if m.get("role") == "tool" + ] + + +def _first_catalog_line(body: dict[str, Any]) -> str | None: + """The first catalog line of a catalog listing in the messages. + + A catalog listing is a ``tool``-role result whose FIRST line is the + agent's ``"N documents:"`` header (``_CATALOG_HEADER_RE``); its + first ``source: X | path: Y | title: Z`` line (the + ``_CATALOG_LINE_RE`` machinery) is returned. ``None`` when no + catalog listing is in the messages — e.g. while only the teaching + refusal is there (the phase-72 LS-TEACH flow's request-2 state). + An empty listing (``"0 documents:"`` with no lines) returns + ``""`` — the listing is present, it is just empty. + """ + for content in _tool_results(body): + lines = content.splitlines() + if not lines or not _CATALOG_HEADER_RE.match(lines[0]): + continue + for line in lines[1:]: + if _CATALOG_LINE_RE.match(line): + return line + return "" + return None + + #: One line of the agent's ``grep`` output (app.rag.agent #: ``_execute_tool``, phase 68 — phase 70 renamed the tool, the line #: format is unchanged): ``source/path:LINE: text``. The @@ -718,6 +802,40 @@ def _tool_flow(body: dict[str, Any]) -> tuple[str, ...] | None: return ("list", "", "") +def _ls_teach_flow(body: dict[str, Any]) -> tuple[str, ...] | None: + """Classify a phase-72 LS-TEACH request (see the module docstring). + + * ``("misuse",)`` — ``tools`` are offered and no ``tool``-role + result is in the messages yet: the incident's misuse — ``ls`` + with ``{"path": "."}`` (id ``call_0``), ``finish_reason: + "tool_calls"``, no content. + * ``("correct",)`` — a ``tool``-role result is in the messages and + it is NOT a catalog listing (the teaching refusal): the + correction — ``ls`` with no arguments (id ``call_1``). + * ``("answer", line)`` — a ``tool``-role result whose first line + is the ``"N documents:"`` catalog header: the deterministic + content answer ``These are the indexed documents: `` (the + first catalog line), ``finish_reason: "stop"`` — the loop + settled in ONE correction, not at the round cap. + * ``None`` — not the flow: the trigger is absent, the ```` + section is missing (deflected turns never carry it), or + ``tools`` are not offered and no tool results are in the + messages yet (e.g. ``agent_max_rounds=0``). + """ + if LS_TEACH_TRIGGER not in _user(body).lower(): + return None + if "" not in _system(body): + return None + line = _first_catalog_line(body) + if line is not None: + return ("answer", line) + if _tool_results(body): + return ("correct",) + if not body.get("tools"): + return None + return ("misuse",) + + def long_answer() -> str: """~900-word deterministic walkthrough (phase 11): numbered steps plus a unique final line that must survive the stream untruncated.""" @@ -1208,6 +1326,30 @@ def chat_completions(body: dict[str, Any]) -> Any: media_type="text/event-stream", headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, ) + # Phase 72 (teaching refusals): the deterministic LS-TEACH + # self-correction flow — checked BEFORE the plain + # TOOLS_TRIGGER flow (disjoint trigger phrases — the phase-71 + # ordering convention; the trigger needs the ```` + # section, so deflected turns never hit it). + ls_teach = _ls_teach_flow(body) + if ls_teach is not None: + if ls_teach[0] == "misuse": + # The incident's misuse, deterministic: ls(path='.'). + stream = _tool_call_stream("ls", {"path": "."}, "call_0") + elif ls_teach[0] == "correct": + # The one-round correction: the no-arg full listing. + stream = _tool_call_stream("ls", {}, "call_1") + else: # "answer" — quote the first catalog line + answer = _apply_max_tokens( + f"These are the indexed documents: {ls_teach[1]}", + 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_agent_unlimited_tools.py b/tests/e2e/test_agent_unlimited_tools.py index 7f833b3..06c5af0 100644 --- a/tests/e2e/test_agent_unlimited_tools.py +++ b/tests/e2e/test_agent_unlimited_tools.py @@ -44,7 +44,8 @@ pattern, grown to three documents): Three documents (not two, as in phase 37) so BOTH reads land on documents outside the seed: with a two-document corpus the second read would be the already-in-context retrieval document and the agent would -answer "Already in your context." — a rejection, not the multi-read +answer "Already in your context — …" (the in-context refusal) — a +rejection, not the multi-read flow this story proves. Test → story mapping (Playwright Mapping Rule): diff --git a/tests/e2e/test_tool_path_teaching.py b/tests/e2e/test_tool_path_teaching.py new file mode 100644 index 0000000..2cc93e9 --- /dev/null +++ b/tests/e2e/test_tool_path_teaching.py @@ -0,0 +1,472 @@ +"""Phase 72 E2E (Playwright, mock-only): the ls-teaching +self-correction loop through the real UI. + +Story: ``.agent/user_stories/agent-document-tools.md`` (this phase +repairs the model-facing contract the phase-70 tools reshaped — the +2026-09-03 incident: the harness-prior ``ls(path='.')`` misuse met the +terse refusal, and the model re-reasoned the same paragraphs over and +over before answering from the seed documents alone). +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_tool_path_teaching.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the gate is the +deterministic LS-TEACH flow in ``tests/e2e/mock_llm.py`` +(``LS_TEACH_TRIGGER`` — "list the files in this directory" — + the +HIGH prompt's ```` section): the incident's misuse (``ls`` with +``{"path": "."}``, id ``call_0``) → the agent's teaching refusal +(``No source named '.' — check the ls output. (…)``) → the corrected +no-arg ``ls()`` (id ``call_1``) → the deterministic +``These are the indexed documents: `` answer. + +KB fixture (TRUNCATE-then-seed, house pattern): ONE source with TWO +documents of known ``source``/``path``/``title`` (catalog order = +``(source, path)``, so the first catalog line is deterministic): + +* ``Homelab/aws-route53.md`` — the CATALOG-FIRST document, indexed + WITHOUT chunks (catalog-only; never in the retrieval context, so + the single-read flow's ``read`` of it is NOT deduped as already-in- + context). Its FIRST line is longer than 80 chars, so the mock's + first-80-chars quote (the single-read regression turn) stays + newline-free. +* ``Homelab/example-record-file.json`` — the retrievable document: + one chunk whose embedding is the mock's own bag-of-words vector + (the trigger question cosines well past the E2E 0.30 threshold and + FTS-matches too → grounded, the ```` section rides along). + It is the seed context only — the single-read flow reads the + catalog-FIRST document, not the seed. + +Test → phase mapping (Playwright Mapping Rule): +1. ``test_ls_misuse_self_corrects_to_noarg_listing`` — the grounded + LS-TEACH turn: the turn settles (composer re-enables, ``done`` + observed), the answer bubble carries the first catalog line — the + first document's ``source:`` / ``path:`` / title fields (the + catalog reached the model and landed in the answer), the UI shows + the two tool lines (``🔎 Listing documents in .`` + then ``🔎 Listing documents``), and no error banner. Wire level: + the ``tool`` frames arrive in order — first ``ls`` with + ``argument: "."``, then ``ls`` with ``argument: null`` — and there + is NO third ``tool`` frame (the loop ended in one correction, not + at the round cap). +2. ``test_plain_tool_flow_not_swallowed_by_new_trigger`` — in the SAME + session, the LS-TEACH turn settles and a follow-up question + carrying ``TOOLS_TRIGGER`` (the single-read flow) still settles + with the read flow's answer (``ls`` → ``read`` on the first + catalog line's combined identity → ``Read . ``) + — the new flow did not swallow the existing trigger. +""" +from __future__ import annotations + +import hashlib +import json +import re +import time +from datetime import UTC, datetime +from pathlib import Path + +from playwright.sync_api import Page, expect +from sqlalchemy import text +from sqlalchemy.orm import Session + +from app.db import SessionLocal +from app.models import Chunk, Document +from tests.e2e.mock_llm import ( + LS_TEACH_TRIGGER, + TOOLS_TRIGGER, + embed_text, +) + +REPO = Path(__file__).resolve().parents[2] + +# -------------------------------------------------------------------------- +# The one-source, two-document fixture (see the module docstring) +# -------------------------------------------------------------------------- + +SEED_SOURCE = "Homelab" +DOC1_PATH = "aws-route53.md" +DOC1_TITLE = "AWS Route 53 Notes" +DOC1_SP = f"{SEED_SOURCE}/{DOC1_PATH}" + +DOC2_PATH = "example-record-file.json" +DOC2_TITLE = "Example Record File" +DOC2_SP = f"{SEED_SOURCE}/{DOC2_PATH}" + +#: The FIRST catalog line (catalog order = (source, path) — DOC1 sorts +#: first): the mock's LS-TEACH answer quotes exactly this line. +FIRST_CATALOG_LINE = ( + f"source: {SEED_SOURCE} | path: {DOC1_PATH} | title: {DOC1_TITLE}" +) + +#: The catalog-first document (catalog order = (source, path) — +#: DOC1 sorts first): the single-read flow reads THIS document, so it +#: must NOT be the seed (a seed read dedupes to "Already in your +#: context.", which the mock's single-read flow does not model — it +#: would loop to the round cap). Indexed WITHOUT chunks: catalog-only, +#: never in the retrieval context. Its FIRST line is longer than 80 +#: chars, so the mock's first-80-chars quote (the single-read +#: regression turn) stays newline-free. +DOC1_CONTENT = ( + "The aws route53 hosted zone for reeselink keeps every record in " + "reseelink.json — the exact JSON shape of reeselink.json is " + "documented in the record file below.\n" + + ( + "The aws route53 hosted zone for reeselink keeps every record in " + "reseelink.json — the record file shape of reeselink.json is " + "the contract every sync job relies on.\n" + ) + * 10 + + "\n## Sync job\n\n" + "A cron job pushes reeselink.json to the aws route53 hosted zone " + "every fifteen minutes; the diff is applied through the route53 api.\n" +) +assert "\n" not in DOC1_CONTENT[:80] # the quote must stay one line + +#: The retrievable document (the grounded seed context, the cf. +#: test_harness_aligned_tools.py pattern): the repeated record-file +#: lines carry the trigger question's key tokens — well past the E2E +#: 0.30 cosine threshold, plus FTS hits. Its FIRST line is longer +#: than 80 chars too, so the retrieval seed context is one clean +#: line. +DOC2_CONTENT = ( + "The ReeseLink hosted zone record file reeselink.json holds every " + "aws route53 record for reeselink — the note documents the exact " + "JSON shape of reeselink.json for the record file.\n" + + ( + "The aws route53 record file reeselink.json keeps every record " + "for the reeselink hosted zone — the exact JSON shape of the " + "record file is the contract every sync job relies on.\n" + ) + * 10 + + "\n## Sync job\n\n" + "A cron job pushes reeselink.json to the aws route53 hosted zone " + "every fifteen minutes; the diff is applied through the route53 api.\n" +) +assert "\n" not in DOC2_CONTENT[:80] # the seed context stays one line + +#: Carries ``LS_TEACH_TRIGGER`` and is on-topic (grounded — HIGH, the +#: ```` section rides along); it carries NO other mock marker. +LS_TEACH_QUESTION = ( + "List the files in this directory — what do my aws route53 notes " + "say about the reeselink.json record file?" +) +assert LS_TEACH_TRIGGER in LS_TEACH_QUESTION.lower() +for _other in ( + "use your tools", + "read two documents", + "search your documents", + "emit raw tool markup", + "always emit raw tool markup", + "show me a table", + "think in paragraphs", + "think out loud then hesitate", + "think out loud", + "show the end of your notes", + "write a long answer", + "fail then answer", + "always fail", + "embed fail once", + "pretend to think slowly", +): + assert _other not in LS_TEACH_QUESTION.lower(), _other + +#: Carries ``TOOLS_TRIGGER`` (the single-read flow) and nothing else — +#: the no-regression follow-up question in the same session. +READ_QUESTION = ( + "Use your tools: what is the exact JSON shape of reeselink.json " + "for my aws route53 hosted zone?" +) +assert TOOLS_TRIGGER in READ_QUESTION.lower() +for _other in ( + LS_TEACH_TRIGGER, + "read two documents", + "search your documents", + "emit raw tool markup", + "always emit raw tool markup", + "show me a table", + "think in paragraphs", + "think out loud then hesitate", + "think out loud", + "show the end of your notes", + "write a long answer", + "fail then answer", + "always fail", + "embed fail once", + "pretend to think slowly", +): + assert _other not in READ_QUESTION.lower(), _other + +#: The mock's single-read answer (the read document reached the model +#: and landed in the answer) — DOC1 is the first catalog line, so the +#: flow reads ``Homelab/aws-route53.md`` and quotes its first 80 chars. +READ_ANSWER_PREFIX = f"Read {DOC1_SP}." +READ_ANSWER_QUOTE = DOC1_CONTENT[:80] + + +def _seed_fixture(db: Session) -> None: + """The one-source, two-document fixture (see the module docstring). + + DOC1 (catalog-first) is indexed WITHOUT chunks; DOC2 carries the + single chunk (the mock's own embedding → the trigger question + cosines well past the E2E 0.30 threshold and FTS-matches too → + grounded). DOC2 is the seed context only — the single-read flow + reads the catalog-FIRST document (DOC1), which is not in context. + """ + db.add( + Document( + source=SEED_SOURCE, + path=DOC1_PATH, + full_path=f"/tmp/{DOC1_PATH}", + title=DOC1_TITLE, + content=DOC1_CONTENT, + content_hash=hashlib.sha256(DOC1_CONTENT.encode()).hexdigest(), + indexed_at=datetime.now(UTC), + ) + ) + doc2 = Document( + source=SEED_SOURCE, + path=DOC2_PATH, + full_path=f"/tmp/{DOC2_PATH}", + title=DOC2_TITLE, + content=DOC2_CONTENT, + content_hash=hashlib.sha256(DOC2_CONTENT.encode()).hexdigest(), + indexed_at=datetime.now(UTC), + ) + db.add(doc2) + db.flush() + # One chunk carrying the mock's own embedding → genuine token + # overlap between the trigger question and DOC2 (the only + # retrievable document). + db.add( + Chunk( + document_id=doc2.id, + position=0, + content=DOC2_CONTENT, + embedding=embed_text(DOC2_CONTENT), + ) + ) + + +def _reset_db_fixture() -> None: + """Truncate the KB (plus the prompt-shaping tables), then seed the + one-source, two-document fixture. ``steering_notes`` / + ``kb_overview`` are truncated too, so the HIGH prompt is exactly + ```` + ```` + ```` — byte-stable + prompts, byte-stable answers.""" + with SessionLocal() as db: + db.execute( + text("TRUNCATE chunks, documents, query_log, steering_notes, kb_overview") + ) + db.commit() + _seed_fixture(db) + db.commit() + + +# -------------------------------------------------------------------------- +# Page helpers (the house pattern — cf. test_harness_aligned_tools.py) +# -------------------------------------------------------------------------- + +#: 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_sse_hook(page: Page) -> None: + page.evaluate(SSE_HOOK) + + +def _drain_frames(page: Page) -> list[dict]: + """One turn's SSE frames: wait for that turn's ``done`` frame, then + return EVERY frame captured since the last drain (the hook's + background read appends the whole stream at once after it closes, so + clearing-and-reading is race-free per turn).""" + deadline = time.monotonic() + 10.0 + while True: + raw = page.evaluate( + "() => { const f = window.__sseFrames || []; " + "window.__sseFrames = []; return f; }" + ) + 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.""" + 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 settled through the normal done path — never the red + role=alert error banner (the KB-offline banner is a separate, + health-driven state the db_ready fixture keeps away).""" + 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 grounded LS-TEACH turn: the incident's ls(path='.') misuse → +# the teaching refusal → the corrected no-arg ls() → the catalog +# answer — the loop settles in ONE correction (two tool rounds), +# pinned on the SSE wire +# -------------------------------------------------------------------------- + + +def test_ls_misuse_self_corrects_to_noarg_listing( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + page.set_default_timeout(30_000) + _reset_db_fixture() + page.goto(app_url) + _install_sse_hook(page) + + _submit(page, LS_TEACH_QUESTION) + _wait_settled(page) + + # Self-correction: the answer quotes the FIRST catalog line — the + # first document's source: / path: / title fields reached the model + # and landed in the answer (the catalog round settled the turn). + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text("These are the indexed documents:") + expect(bubble).to_contain_text(FIRST_CATALOG_LINE) + _assert_no_error_banner(page) + + # The UI shows the two tool lines in order: the scoped misuse + # (🔎 Listing documents in .) then the corrected + # unscoped listing (🔎 Listing documents — no ). + lines = page.locator(".msg.brain .tool-call") + expect(lines).to_have_count(2) + expect(lines.nth(0)).to_contain_text("Listing documents in") + expect(lines.nth(0).locator("code")).to_have_text(".") + expect(lines.nth(1)).to_contain_text("Listing documents") + expect(lines.nth(1).locator("code")).to_have_count(0) + + # Two rounds on the wire: the tool frames arrive in order — first + # ls with argument "." (the incident's misuse), then ls with + # argument null (the correction) — and there is NO third tool + # frame: the loop ended in one correction, not at the round cap. + frames = _drain_frames(page) + assert _tool_frames(frames) == [ + {"type": "tool", "name": "ls", "argument": "."}, + {"type": "tool", "name": "ls", "argument": None}, + ] + 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 + assert not [f for f in frames if f.get("type") == "error"] + + +# -------------------------------------------------------------------------- +# 2. No regression to the plain flow — the SAME session: after the +# LS-TEACH turn, the TOOLS_TRIGGER follow-up (the single-read flow) +# still settles with the read flow's answer +# -------------------------------------------------------------------------- + + +def test_plain_tool_flow_not_swallowed_by_new_trigger( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + page.set_default_timeout(30_000) + _reset_db_fixture() + page.goto(app_url) + _install_sse_hook(page) + + # Turn 1 — the LS-TEACH flow (the incident's misuse → the + # correction → the catalog answer). + _submit(page, LS_TEACH_QUESTION) + _wait_settled(page) + teach_frames = _drain_frames(page) + assert _tool_frames(teach_frames) == [ + {"type": "tool", "name": "ls", "argument": "."}, + {"type": "tool", "name": "ls", "argument": None}, + ] + expect( + page.locator(".msg.brain .bubble").last + ).to_contain_text(FIRST_CATALOG_LINE) + + # Turn 2 — the SAME session: the single-read flow on + # TOOLS_TRIGGER. The new flow must not have swallowed the existing + # trigger: the follow-up settles with the read flow's answer. + _submit(page, READ_QUESTION) + _wait_settled(page) + + second_msg = page.locator(".msg.brain").last + # The UI shows the single-read flow's two lines: the unscoped ls + # then the read of the first catalog line's COMBINED identity. + lines = second_msg.locator(".tool-call") + expect(lines).to_have_count(2) + expect(lines.nth(0)).to_contain_text("Listing documents") + expect(lines.nth(0).locator("code")).to_have_count(0) + expect(lines.nth(1)).to_contain_text("Reading ") + expect(lines.nth(1).locator("code")).to_have_text(DOC1_SP) + + # The answer quotes the read document (the mock's deterministic + # echo: "Read . "). + bubble = second_msg.locator(".bubble").last + expect(bubble).to_contain_text(READ_ANSWER_PREFIX) + expect(bubble).to_contain_text(READ_ANSWER_QUOTE) + _assert_no_error_banner(page) + + # Wire level for the follow-up: ls (null) → read (the combined + # identity) — the single-read flow, unchanged. + frames = _drain_frames(page) + assert _tool_frames(frames) == [ + {"type": "tool", "name": "ls", "argument": None}, + {"type": "tool", "name": "read", "argument": DOC1_SP}, + ] + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is False + assert not [f for f in frames if f.get("type") == "error"] diff --git a/tests/integration/test_agent_tools.py b/tests/integration/test_agent_tools.py index f3cafa4..16826bc 100644 --- a/tests/integration/test_agent_tools.py +++ b/tests/integration/test_agent_tools.py @@ -185,22 +185,48 @@ def _settings(**kwargs: Any) -> Settings: return Settings(**kwargs) # pyright: ignore[reportCallIssue] +class ScriptedToolCallsLLM: + """N scripted tool-call rounds (one ``ToolCallPiece`` each), then one + canned answer; records every request's messages and tools (the + phase-72 task-02 two-round self-correction cases: the refusal round, + then the corrected call).""" + + def __init__(self, calls: list[ToolCallPiece]) -> None: + self.calls = calls + 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, + scaffolding: ScaffoldingFilter | None = None, + ) -> AsyncIterator[StreamPiece | ToolCallPiece]: + self.requests.append((deepcopy(messages), deepcopy(tools))) + index = len(self.requests) - 1 + if index < len(self.calls): + yield self.calls[index] + else: + yield StreamPiece("content", "ans") + + def _run_call( db: Session, name: str, arguments: dict[str, Any] ) -> tuple[AgentHolder, ScriptedToolLLM]: """Drive one scripted tool call through ``run_agent``.""" holder = AgentHolder() llm = ScriptedToolLLM(ToolCallPiece(id="call_1", name=name, arguments=arguments)) - asyncio.run(_consume(llm, db, holder)) + asyncio.run(_consume(cast("LLMClient", llm), db, holder)) return holder, llm async def _consume( - llm: ScriptedToolLLM, db: Session, holder: AgentHolder + llm: LLMClient, db: Session, holder: AgentHolder ) -> list[StreamPiece | ToolCallPiece | RetryPiece]: out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] async for piece in run_agent( - cast("LLMClient", llm), + llm, db, system_prompt="SYSTEM_PROMPT", user_message="QUESTION", @@ -233,17 +259,39 @@ def test_ls_scoped_to_registered_source_through_run_agent(kb, src, db) -> None: def test_ls_scoped_unknown_source_refused_through_run_agent(kb, src, db) -> None: + """Phase 72: the no-source refusal now carries the teaching + parenthetical — the prefix byte-identical to the pre-phase-72 line; + still not counted.""" _doc(db, "Homelab", "a.md", "A", "A-CONTENT") db.commit() holder, llm = _run_call(db, "ls", {"path": "Ghost"}) assert ( - llm.requests[1][0][3]["content"] == "No source named 'Ghost' — check the ls output." + llm.requests[1][0][3]["content"] + == agent.NO_SOURCE_NOT_A_DIRECTORY.format(scope="Ghost") ) assert holder.tool_calls == 0 and holder.read_docs == [] +def test_ls_path_like_scope_teaching_refusal_through_run_agent(kb, src, db) -> None: + """Phase 72: a ``/``-containing ``path`` is a document path, not a + source name — the ``LS_PATH_NOT_A_SOURCE`` teaching line (no + registry lookup needed), not counted, the tools stay offered on the + next request.""" + _doc(db, "Homelab", "a.md", "A", "A-CONTENT") + db.commit() + + holder, llm = _run_call(db, "ls", {"path": "app/rag/importer.py"}) + + assert ( + llm.requests[1][0][3]["content"] + == agent.LS_PATH_NOT_A_SOURCE.format(path="app/rag/importer.py") + ) + assert holder.tool_calls == 0 and holder.read_docs == [] + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + # ---------- read (the canonical combined source/path form) ---------- @@ -280,9 +328,10 @@ def test_read_bare_source_name_refused_through_run_agent(kb, db) -> None: def test_read_unknown_combined_path_refused_through_run_agent(kb, db) -> None: - """A combined identity that matches nothing gets the no-document - refusal (the argument echoed as passed — the model sees its own - form).""" + """A combined identity that matches NOTHING — not a document and not + any indexed document's ``path`` (zero candidates) — gets today's + no-document refusal byte-identical (the argument echoed as passed — + the model sees its own form).""" _doc(db, "Alpha", "x.md", "X", "X-CONTENT") db.commit() @@ -295,6 +344,79 @@ def test_read_unknown_combined_path_refused_through_run_agent(kb, db) -> None: assert holder.tool_calls == 0 and holder.read_docs == [] +def test_read_bare_path_single_source_suggestion_then_corrected_read(kb, db) -> None: + """Phase 72, task 02: a bare path under ONE source (exact ``path`` + match, the source prefix missing) → the single-identity suggestion + (a refusal — not counted); the scripted corrected call (round 2, the + suggested combined identity) then succeeds against real Postgres — + the two-round self-correction.""" + created = _doc(db, "Alpha", "deep/nested/doc.md", "The Doc", "FULL-TEXT") + db.commit() + + llm = ScriptedToolCallsLLM( + [ + ToolCallPiece( + id="call_1", name="read", arguments={"path": "deep/nested/doc.md"} + ), + ToolCallPiece( + id="call_2", + name="read", + arguments={"path": "Alpha/deep/nested/doc.md"}, + ), + ] + ) + holder = AgentHolder() + asyncio.run(_consume(cast("LLMClient", llm), db, holder)) + + # Round 1: the bare path resolves to no combined identity, but it IS + # the indexed document's path — the refusal names the one combined + # identity to use (not counted, the tools stay offered). + assert llm.requests[1][0][3]["content"] == ( + "No document at 'deep/nested/doc.md' — " + "did you mean 'Alpha/deep/nested/doc.md'?" + ) + assert llm.requests[1][1] == AGENT_TOOLS + # Round 2: the corrected combined identity succeeds — the full + # content, the holder records the row, and it counts. + assert llm.requests[2][0][5]["content"] == ( + "Document Alpha/deep/nested/doc.md:\nFULL-TEXT" + ) + assert llm.requests[2][1] == AGENT_TOOLS + assert holder.read_docs == [created] + assert holder.tool_calls == 1 # only the corrected read executed + + +def test_read_bare_path_two_sources_one_of_suggestion_then_corrected_read( + kb, db, +) -> None: + """Phase 72, task 02: the same bare path under TWO sources → the + ``one of`` line (up to ``SUGGESTION_LIMIT`` identities, catalog + order — Alpha before Beta); the scripted corrected call (round 2, + the first suggested identity) then succeeds.""" + a = _doc(db, "Alpha", "shared/x.md", "Alpha X", "A-TEXT") + _doc(db, "Beta", "shared/x.md", "Beta X", "B-TEXT") + db.commit() + + llm = ScriptedToolCallsLLM( + [ + ToolCallPiece(id="call_1", name="read", arguments={"path": "shared/x.md"}), + ToolCallPiece( + id="call_2", name="read", arguments={"path": "Alpha/shared/x.md"} + ), + ] + ) + holder = AgentHolder() + asyncio.run(_consume(cast("LLMClient", llm), db, holder)) + + assert llm.requests[1][0][3]["content"] == ( + "No document at 'shared/x.md' — did you mean one of: " + "'Alpha/shared/x.md', 'Beta/shared/x.md'?" + ) + assert llm.requests[2][0][5]["content"] == "Document Alpha/shared/x.md:\nA-TEXT" + assert holder.read_docs == [a] + assert holder.tool_calls == 1 # only the corrected read executed + + # ---------- grep (the phase-68 A5 contract under the new name) ---------- @@ -345,6 +467,9 @@ def test_grep_scoped_through_run_agent(kb, db) -> None: def test_grep_scoped_missing_doc_refused_through_run_agent(kb, db) -> None: + """A scoped ``grep`` miss that matches no indexed document's ``path`` + (zero candidates) keeps today's line byte-identical — a refusal, + not counted.""" _doc(db, "Alpha", "a/one.md", "One", "nothing") db.commit() diff --git a/tests/unit/test_agent.py b/tests/unit/test_agent.py index b8afa5e..dd9e82e 100644 --- a/tests/unit/test_agent.py +++ b/tests/unit/test_agent.py @@ -141,21 +141,49 @@ def test_agent_tools_names_and_parameters() -> None: assert not set(by_name) & {"list_documents", "read_document", "search_documents"} assert all(t["type"] == "function" for t in AGENT_TOOLS) ls = by_name["ls"]["function"] + # Task 05 (live gate iteration 2): the one-call-at-a-time discipline + # clause (the harness prior batches calls; the loop executes one + # per round — the extras count as unexecuted in the gate). assert ls["description"] == ( - "List the indexed documents as `source: X | path: Y | title: Z` lines." + "List the indexed documents as `source: X | path: Y | " + "title: Z` lines. Call one tool at a time — wait for " + "this result before your next call." ) ls_params = ls["parameters"] assert ls_params["type"] == "object" assert ls_params["required"] == [] # path is optional assert set(ls_params["properties"]) == {"path"} assert ls_params["properties"]["path"]["type"] == "string" + # Phase 72: the description states the contract up front — the + # 'path' argument is a source name, not a file or directory path. + # Task 05 (live gate iteration 5): the cross-tool contrast clause + # (ls is the ONLY tool whose path is a source name — the model + # kept transferring that scope to grep's document identity). assert ls_params["properties"]["path"]["description"] == ( - "Source name to list one source's documents (e.g. 'homelab'); " - "omit to list every document." + "Source name to list one source's documents (e.g. 'homelab') — " + "a source name, not a file or directory path; omit to list " + "every document. This is the only tool " + "whose `path` is a source name — for " + "`read` and `grep` it must be a document's " + "combined `source/path`." ) read = by_name["read"]["function"] + # Tool-calling fast loop (2026-09-04, controlled fixture gate): + # the do-not-read rule is FRONT-LOADED — the controlled gate's + # telemetry showed the `lite` model obeying the user's "open it / + # read it" and reading seed-context documents the + # section already carries (every refusal of a 12-call run was + # ALREADY_IN_CONTEXT); the rule now leads the description instead + # of sitting mid-paragraph, and the tool is framed as "only for + # documents NOT already in ". assert read["description"] == ( - "Add the full content of one indexed document to your context." + "Do not call this tool for a document already shown in " + "the section, even when the user asks you to " + "open or read it — its full text is already in your " + "prompt; answer directly from it. Use it only to add a " + "document NOT already in to your context, " + "by its combined `source/path` string. Call one tool at " + "a time — wait for this result before your next call." ) read_params = read["parameters"] assert read_params["type"] == "object" @@ -163,18 +191,35 @@ def test_agent_tools_names_and_parameters() -> None: assert set(read_params["properties"]) == {"path"} assert read_params["properties"]["path"]["type"] == "string" # The combined source/path string is the canonical document identity - # (phase 70) — the description pins it with a worked example. + # (phase 70) — the description pins it with a worked example. Phase + # 72 (task 02): the bare-path contract is stated up front; task 05 + # (live gate iteration 1): the do-not-re-read clause (the dedupe + # refusal's prevention at the prompt). assert read_params["properties"]["path"]["description"] == ( "The document to add to your context, as the combined " "`source/path` string exactly as shown in the `ls` output (e.g. " - "'homelab/active/container_caddy/caddy.md')." + "'homelab/active/container_caddy/caddy.md'). A bare document " + "path (without the source name) will not resolve. Only pass a " + "document NOT already shown in the section — it is " + "already in your context; do not re-read it." ) grep = by_name["grep"]["function"] + # Task 05 (live gate iterations 2-6, refined in the 2026-09-03 + # re-run): the pattern-only-is-the-knowledge-base-search clause + # ("pass ONLY `pattern`") + the source-name-is-not-a-document + # clause (the model kept scoping grep with an ls-style source name + # — the 2026-09-03 incident loop shape, but on grep) plus the + # one-call-at-a-time discipline clause. assert grep["description"] == ( "Search the indexed documents for an exact string " - "(case-insensitive) and return up to 20 matching lines as " - "`source/path:line: text` — a locator, not a context-adder: " - "read the winner with `read`." + "(case-insensitive) and return up to 20 matching lines " + "as `source/path:line: text` — a locator, not a " + "context-adder: read the winner with `read`. For a " + "normal search pass ONLY `pattern` — it searches every " + "document and that is how you search the knowledge " + "base; never pass a source name as `path` (a source " + "name is not a document). Call one tool at a time — " + "wait for this result before your next call." ) grep_params = grep["parameters"] assert grep_params["type"] == "object" @@ -184,9 +229,25 @@ def test_agent_tools_names_and_parameters() -> None: assert grep_params["properties"]["pattern"]["description"] == ( "The exact text to search for (a plain substring, not a regex)" ) + # Phase 72 (task 02): the bare-path contract is stated up front; + # task 05 (live gate iterations 1-8): the one-known-document clause + # with a worked combined-identity example and the source-name ban + # (the model kept scoping grep with an ls-style source name — the + # incident loop shape, but on grep). + # Iteration 8 drops the standalone 'homelab' from this negative + # example — the gate's live telemetry showed the model emitting + # exactly that value, and naming it beside the parameter risks + # priming it (the negative-example effect). The 2026-09-03 re-run + # makes the rarity explicit ("Rarely needed") and re-states the + # pattern-only normal search. assert grep_params["properties"]["path"]["description"] == ( - "Limit the search to one document, as a combined `source/path` " - "string from the `ls` output (omit to search every document)." + "Rarely needed — only for re-searching one " + "document you already know: that document's " + "combined `source/path` identity (e.g. " + "'homelab/ansible/inventory.yaml'). Never a " + "source name. A bare document path (without " + "the source name) will not resolve. Omit it " + "for a normal search (pass only `pattern`)." ) @@ -198,11 +259,44 @@ def test_agent_tools_order_is_ls_read_grep() -> None: def test_refusal_constants_are_harness_aligned() -> None: """The updated module-level refusal lines (the names moved to the - harness surface; ALREADY_IN_CONTEXT / UNKNOWN_TOOL unchanged).""" - assert agent.ALREADY_IN_CONTEXT == "Already in your context." + harness surface). The ALREADY_IN_CONTEXT line is a phase-72, + task 05 gate-iteration teaching (live telemetry: the model + repeated the terse phase-37 line) — same refusal behavior, the + copy names the correct action.""" + assert agent.ALREADY_IN_CONTEXT == ( + "Already in your context — the full text is already in your " + "prompt. Do not call read on it again; answer from that text." + ) assert agent.UNKNOWN_TOOL == "Unknown tool." assert agent.MISSING_READ_ARGS == "read requires a string argument 'path'." assert agent.MISSING_SEARCH_ARGS == "grep requires a string argument 'pattern'." + # Phase 72: the ls teaching-refusal templates, pinned byte-for-byte + # (task 01 — the read/grep suggestion templates below, task 02). + assert agent.LS_PATH_NOT_A_SOURCE == ( + "'{path}' looks like a document path, not a source name. The " + "'path' argument of ls filters by source name (e.g. 'homelab') — " + "omit it to list every document, or read a document by its " + "combined 'source/path' string." + ) + # The pre-phase-72 no-source line is the byte-identical prefix of + # the extended line — only the teaching parenthetical was appended. + assert agent.NO_SOURCE_NOT_A_DIRECTORY.startswith( + "No source named '{scope}' — check the ls output." + ) + assert agent.NO_SOURCE_NOT_A_DIRECTORY == ( + "No source named '{scope}' — check the ls output. (The 'path' " + "argument is a source name, not a directory — omit it to list " + "every document.)" + ) + # Phase 72 (task 02): the read/grep "did you mean …?" suggestion + # templates, pinned byte-for-byte, and the suggestion cap. + assert agent.NO_DOCUMENT_DID_YOU_MEAN == ( + "No document at '{arg}' — did you mean '{source}/{path}'?" + ) + assert agent.NO_DOCUMENT_DID_YOU_MEAN_MANY == ( + "No document at '{arg}' — did you mean one of: {candidates}?" + ) + assert agent.SUGGESTION_LIMIT == 3 # ---------- list_source_names (the scoped ls registry join) ---------- @@ -467,8 +561,9 @@ def test_ls_scoped_known_source_with_zero_docs_counts( def test_ls_scoped_unknown_source_refused(monkeypatch: pytest.MonkeyPatch) -> None: - """A ``path`` matching no source name is a refusal — not counted, the - round cap bounds its repetition.""" + """A ``path`` without ``/`` matching no source name is a refusal — + the extended line with the teaching parenthetical (phase 72), not + counted, the round cap bounds its repetition.""" monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) monkeypatch.setattr(agent, "list_source_names", lambda db: ["S"]) holder = AgentHolder() @@ -479,7 +574,65 @@ def test_ls_scoped_unknown_source_refused(monkeypatch: pytest.MonkeyPatch) -> No asyncio.run(_run(llm, holder, _settings())) assert holder.tool_calls == 0 # a refusal counts in nothing assert ( - llm.requests[1][0][3]["content"] == "No source named 'Ghost' — check the ls output." + llm.requests[1][0][3]["content"] + == agent.NO_SOURCE_NOT_A_DIRECTORY.format(scope="Ghost") + ) + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + +def test_ls_path_like_scope_gets_document_path_teaching_refusal( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Phase 72: a stripped scope containing ``/`` looks like a document + path (the incident's ``ls(path='app/rag/importer.py')``) — a source + name is a directory basename and can never contain one, so this gets + the ``LS_PATH_NOT_A_SOURCE`` teaching line with the argument echoed; + no registry lookup, counts in nothing, tools stay offered.""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + + def _boom_sources(*_a: Any, **_k: Any) -> None: + raise AssertionError("no registry lookup for a path-like scope") + + monkeypatch.setattr(agent, "list_source_names", _boom_sources) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="ls", + arguments={"path": "app/rag/importer.py"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.tool_calls == 0 # a refusal counts in nothing + assert ( + llm.requests[1][0][3]["content"] + == agent.LS_PATH_NOT_A_SOURCE.format(path="app/rag/importer.py") + ) + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + +def test_ls_dot_scope_gets_not_a_directory_teaching_refusal( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Phase 72: ``ls(path='.')`` (the incident's second round — no + ``/``, no matching source) gets the extended no-source refusal with + the teaching parenthetical, ``'.'`` echoed — not counted, tools stay + offered.""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + monkeypatch.setattr(agent, "list_source_names", lambda db: ["S"]) + holder = AgentHolder() + llm = ScriptedLLM( + [ToolCallPiece(id="call_1", name="ls", arguments={"path": "."})], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.tool_calls == 0 # a refusal counts in nothing + assert ( + llm.requests[1][0][3]["content"] + == agent.NO_SOURCE_NOT_A_DIRECTORY.format(scope=".") ) assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered @@ -529,14 +682,19 @@ def test_read_combined_path_resolves_and_returns_full_content( def test_read_bare_source_name_refused_without_db(monkeypatch: pytest.MonkeyPatch) -> None: """A bare source name (no '/') can never be a document — the - no-document refusal (the argument echoed as passed), no DB lookup, - nothing counted.""" + no-document refusal (the argument echoed as passed), no DB lookup + (NOT even the phase-72 candidate lookup — ``all_documents`` must + not run either), nothing counted.""" monkeypatch.setattr(agent, "list_catalog", lambda db: [("Homelab", "a.md", "A")]) def _boom(*_a: Any, **_k: Any) -> None: - raise AssertionError("find_document must not run for a bare source name") + raise AssertionError( + "no DB lookup (find_document or all_documents) for a bare " + "source name" + ) monkeypatch.setattr(agent, "find_document", _boom) + monkeypatch.setattr(agent, "all_documents", _boom) holder = AgentHolder() llm = ScriptedLLM( [ToolCallPiece(id="call_1", name="read", arguments={"path": "Homelab"})], @@ -553,10 +711,12 @@ def test_read_bare_source_name_refused_without_db(monkeypatch: pytest.MonkeyPatc def test_read_unknown_path_refused_echoing_argument( monkeypatch: pytest.MonkeyPatch, ) -> None: - """An unknown combined identity → the refusal echoing the argument as - passed (the model sees its own form) — the old split-teaching refusal - is gone (phase 70).""" + """An unknown combined identity that matches NO indexed document's + ``path`` (zero candidates — the phase-72 lookup runs, finds nothing) + → today's refusal echoing the argument as passed, byte-identical — + the old split-teaching refusal is gone (phase 70).""" monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: []) holder = AgentHolder() llm = ScriptedLLM( [ToolCallPiece(id="call_1", name="read", arguments={"path": "S/ghost.md"})], @@ -570,6 +730,270 @@ def test_read_unknown_path_refused_echoing_argument( assert llm.requests[1][1] == AGENT_TOOLS # tools stay offered (cap bounds) +# ---------- read/grep: the "did you mean …?" suggestions (phase 72, task 02) ---------- + + +def test_find_path_candidates_exact_suffix_catalog_order( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The pure catalog lookup (monkeypatched ``all_documents`` — one + bulk query per call): ``path`` == arg (exact) or a ``/arg`` suffix — + catalog order, case-sensitive, as ``(source, path, title)`` triples; + a plain substring is NOT a suffix; the result is uncapped (the + :data:`~app.rag.agent.SUGGESTION_LIMIT` cap lives in the refusal). + """ + docs = [ + _doc("A", "x.md", "Ax", "A"), + _doc("A", "shared/x.md", "As", "AS"), + _doc("B", "shared/x.md", "Bs", "BS"), + _doc("C", "deep/shared/x.md", "Cs", "CS"), + _doc("D", "X.md", "Dx", "D"), # case-sensitive: not 'x.md' + _doc("E", "nosuffixx.md", "Ex", "E"), # substring, not a /suffix + ] + calls: list[int] = [] + + def _all(db: Any) -> list[Document]: + calls.append(1) + return docs + + monkeypatch.setattr(agent, "all_documents", _all) + db = cast("Session", object()) + + # Exact bare path ('shared/x.md') plus the deeper suffix + # ('deep/shared/x.md' ends with '/shared/x.md') — catalog order. + assert agent.find_path_candidates(db, "shared/x.md") == [ + ("A", "shared/x.md", "As"), + ("B", "shared/x.md", "Bs"), + ("C", "deep/shared/x.md", "Cs"), + ] + # 'x.md' equals A's path exactly AND suffix-matches the rest — all + # four, catalog order (uncapped: the cap is the refusal's). + assert agent.find_path_candidates(db, "x.md") == [ + ("A", "x.md", "Ax"), + ("A", "shared/x.md", "As"), + ("B", "shared/x.md", "Bs"), + ("C", "deep/shared/x.md", "Cs"), + ] + # Case-sensitive file paths: 'X.md' matches ONLY D's identically- + # cased path (never the lowercase 'x.md' ones), and the plain + # substring inside 'nosuffixx.md' is not a suffix. + assert agent.find_path_candidates(db, "X.md") == [("D", "X.md", "Dx")] + # One bulk query per call (at most one). + assert len(calls) == 3 + + +def test_read_bare_path_exact_match_gets_did_you_mean( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The incident shape: an unresolved ``read`` argument containing + ``/`` that EXACTLY matches one indexed document's ``path`` (the bare + path missing the source prefix — the harness prior) gets the + ``NO_DOCUMENT_DID_YOU_MEAN`` line naming the combined identity — + still a refusal: ``read_docs`` empty, nothing counted, tools stay + offered.""" + doc = _doc("Homelab", "active/container_caddy/caddy.md", "Caddy", "CADDY-CONTENT") + + def _find(db: Any, source: str, path: str) -> Document | None: + return ( + doc + if (source, path) == ("Homelab", "active/container_caddy/caddy.md") + else None + ) + + monkeypatch.setattr(agent, "find_document", _find) + monkeypatch.setattr(agent, "all_documents", lambda db: [doc]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="read", + arguments={"path": "active/container_caddy/caddy.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.read_docs == [] and holder.tool_calls == 0 # a refusal + assert llm.requests[1][0][3]["content"] == ( + agent.NO_DOCUMENT_DID_YOU_MEAN.format( + arg="active/container_caddy/caddy.md", + source="Homelab", + path="active/container_caddy/caddy.md", + ) + ) + # The rendered line, pinned byte-for-byte. + assert llm.requests[1][0][3]["content"] == ( + "No document at 'active/container_caddy/caddy.md' — " + "did you mean 'Homelab/active/container_caddy/caddy.md'?" + ) + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + +def test_read_bare_path_suffix_match_gets_did_you_mean( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The suffix form of the same teaching: a path-like argument that + matches a deeper indexed path (``active/container_caddy/caddy.md`` + ends with ``/container_caddy/caddy.md``) names the same combined + identity.""" + doc = _doc("Homelab", "active/container_caddy/caddy.md", "Caddy", "CADDY-CONTENT") + + def _find(db: Any, source: str, path: str) -> Document | None: + return ( + doc + if (source, path) == ("Homelab", "active/container_caddy/caddy.md") + else None + ) + + monkeypatch.setattr(agent, "find_document", _find) + monkeypatch.setattr(agent, "all_documents", lambda db: [doc]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="read", + arguments={"path": "container_caddy/caddy.md"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.read_docs == [] and holder.tool_calls == 0 # a refusal + assert llm.requests[1][0][3]["content"] == ( + "No document at 'container_caddy/caddy.md' — " + "did you mean 'Homelab/active/container_caddy/caddy.md'?" + ) + + +def test_read_bare_path_two_sources_gets_one_of_suggestion( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The same bare path under two sources: the ``one of`` line — up to + ``SUGGESTION_LIMIT`` combined identities, each single-quoted, joined + with ``, `` in catalog order (A before B).""" + a = _doc("A", "shared/x.md", "Ax", "A") + b = _doc("B", "shared/x.md", "Bx", "B") + monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: [a, b]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece(id="call_1", name="read", arguments={"path": "shared/x.md"}) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.read_docs == [] and holder.tool_calls == 0 # a refusal + assert llm.requests[1][0][3]["content"] == ( + agent.NO_DOCUMENT_DID_YOU_MEAN_MANY.format( + arg="shared/x.md", candidates="'A/shared/x.md', 'B/shared/x.md'" + ) + ) + # The rendered line, pinned byte-for-byte. + assert llm.requests[1][0][3]["content"] == ( + "No document at 'shared/x.md' — did you mean one of: " + "'A/shared/x.md', 'B/shared/x.md'?" + ) + + +def test_read_bare_path_four_sources_capped_at_three_suggestions( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Four sources sharing the same path: exactly ``SUGGESTION_LIMIT`` + (3) identities are suggested — catalog order, the fourth dropped.""" + docs = [_doc(s, "shared/x.md", f"{s}x", s) for s in ("A", "B", "C", "D")] + monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: list(docs)) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece(id="call_1", name="read", arguments={"path": "shared/x.md"}) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == ( + "No document at 'shared/x.md' — did you mean one of: " + "'A/shared/x.md', 'B/shared/x.md', 'C/shared/x.md'?" + ) + assert "'D/shared/x.md'" not in llm.requests[1][0][3]["content"] + assert holder.read_docs == [] and holder.tool_calls == 0 # a refusal + + +def test_read_bare_filename_without_slash_keeps_no_db_refusal( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The gate is the ``/`` in the argument: a bare FILENAME (no ``/`` + — e.g. ``caddy.md``) is a bare name for the lookup — today's + refusal byte-identical, and NO ``find_document`` / ``all_documents`` + call (the same no-DB-lookup invariant as a bare source name).""" + + def _boom(*_a: Any, **_k: Any) -> None: + raise AssertionError("no DB lookup for a bare (no '/') argument") + + monkeypatch.setattr(agent, "find_document", _boom) + monkeypatch.setattr(agent, "all_documents", _boom) + holder = AgentHolder() + llm = ScriptedLLM( + [ToolCallPiece(id="call_1", name="read", arguments={"path": "caddy.md"})], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert holder.read_docs == [] and holder.tool_calls == 0 + assert llm.requests[1][0][3]["content"] == ( + "No document at 'caddy.md' — check the ls output." + ) + assert llm.requests[1][1] == AGENT_TOOLS + + +def test_read_bare_path_of_seed_doc_gets_suggestion_then_dedupe( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Dedupe precedence: the in-context dedupe fires on the SPLIT pair + of the argument — the bare path of an in-context document + (``read('app/rag/importer.py')`` with ``sample/app/rag/importer.py`` + seeded) is NOT that pair, so it is not a dedupe: it gets the + suggestion line naming the combined identity, and the model's next, + correctly-formed call is then deduped as ALREADY_IN_CONTEXT.""" + seed = [_doc("sample", "app/rag/importer.py", "Importer", "IMPORTER")] + + def _find(db: Any, source: str, path: str) -> Document | None: + return seed[0] if (source, path) == ("sample", "app/rag/importer.py") else None + + monkeypatch.setattr(agent, "find_document", _find) + monkeypatch.setattr(agent, "all_documents", lambda db: list(seed)) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="read", + arguments={"path": "app/rag/importer.py"}, + ) + ], + [ + # Round 2: the corrected call (the suggested combined + # identity) — the seed document is already in context, so it + # dedupes. + ToolCallPiece( + id="call_2", + name="read", + arguments={"path": "sample/app/rag/importer.py"}, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings(), seed_docs=seed)) + assert holder.read_docs == [] and holder.tool_calls == 0 # both refused + assert llm.requests[1][0][3]["content"] == ( + "No document at 'app/rag/importer.py' — " + "did you mean 'sample/app/rag/importer.py'?" + ) + assert llm.requests[2][0][5]["content"] == agent.ALREADY_IN_CONTEXT + + @pytest.mark.parametrize( ("arguments", "label"), [ @@ -806,7 +1230,12 @@ def test_grep_scoped_combined_path_with_nested_path( def test_grep_scoped_missing_document_refused(monkeypatch: pytest.MonkeyPatch) -> None: + """A scoped ``grep`` miss that matches NO indexed document's ``path`` + (zero candidates — the phase-72 lookup runs, finds nothing) keeps + today's line byte-identical: a refusal (not counted), tools stay + offered.""" monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: []) holder = AgentHolder() llm = ScriptedLLM( [ @@ -826,6 +1255,44 @@ def test_grep_scoped_missing_document_refused(monkeypatch: pytest.MonkeyPatch) - assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered +def test_grep_scoped_missing_path_like_doc_gets_did_you_mean( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The same teaching on the scoped ``grep`` miss: an unresolved + path-like scope that matches an indexed document's path gets the + ``NO_DOCUMENT_DID_YOU_MEAN`` suggestion line (a refusal — not + counted, no context added, tools stay offered); the whole-KB grep is + untouched (no ``path`` argument → no scoped resolution at all). + """ + doc = _doc("Homelab", "active/container_caddy/caddy.md", "Caddy", "CADDY-CONTENT") + monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: [doc]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + ToolCallPiece( + id="call_1", + name="grep", + arguments={ + "pattern": "needle", + "path": "active/container_caddy/caddy.md", + }, + ) + ], + [StreamPiece("content", "ans")], + ) + asyncio.run(_run(llm, holder, _settings())) + assert llm.requests[1][0][3]["content"] == ( + agent.NO_DOCUMENT_DID_YOU_MEAN.format( + arg="active/container_caddy/caddy.md", + source="Homelab", + path="active/container_caddy/caddy.md", + ) + ) + assert holder.tool_calls == 0 and holder.read_docs == [] # a refusal + assert llm.requests[1][1] == AGENT_TOOLS # rejected → tools stay offered + + def test_grep_scoped_bare_source_name_refused_without_db( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1080,8 +1547,10 @@ def test_zero_max_rounds_is_one_request_without_tools() -> None: def test_rejected_read_spam_runs_to_round_cap(monkeypatch: pytest.MonkeyPatch) -> None: """Every call rejected (unknown document — "No document at …"): rejections no longer end the loop early via budgets — the round cap - bounds them and forces the final no-tools answer.""" + bounds them and forces the final no-tools answer. Zero candidates + (empty catalog) → the pre-phase-72 line, byte-identical.""" monkeypatch.setattr(agent, "find_document", lambda db, source, path: None) + monkeypatch.setattr(agent, "all_documents", lambda db: []) holder = AgentHolder() llm = ScriptedLLM( [ToolCallPiece(id="call_1", name="read", arguments={"path": "S/ghost.md"})], diff --git a/tests/unit/test_prompts.py b/tests/unit/test_prompts.py index 28d11e2..0625db6 100644 --- a/tests/unit/test_prompts.py +++ b/tests/unit/test_prompts.py @@ -10,6 +10,12 @@ And the phase-71 deflection plain-text line (owner-permitted 2026-09-03): the LOW prompt = pre-phase text + exactly the one new line; the ``DEFLECT_MODE`` marker-keying contract is unchanged and the line never leaks into the HIGH prompt. + +And the phase-72 ```` copy: the document-identity contract is +stated up front (the ``ls`` source-name scope, the combined +``source/path`` identity for ``read``/``grep``) — the same contract +the teaching refusals in :mod:`app.rag.agent` re-state; the +```` marker keying (HIGH only) is unchanged. """ from __future__ import annotations @@ -170,13 +176,14 @@ def test_tools_section_markers_and_new_tool_names() -> None: def test_tools_section_teaches_the_harness_shapes() -> None: - """Copy pins: ``ls``'s phase-63 catalog-line format (and its - optional one-source scope), ``grep``'s case-insensitive exact-string - locator contract (up to 20 ``source/path:line: text`` lines, a - locator not a context-adder), and ``read``'s combined - ``source/path`` + full content.""" + """Copy pins: ``ls``'s phase-63 catalog-line format, ``grep``'s +case-insensitive exact-string locator contract (up to 20 +``source/path:line: text`` lines, a locator not a context-adder), +and ``read``'s combined ``source/path`` + full content. (Phase 72: +the source-name scope clause and the combined-identity clause are +pinned byte-for-byte in +:func:`test_tools_section_phase72_contract_clauses`.)""" assert "source: X | path: Y | title: Z" in TOOLS_SECTION - assert "pass a source name as `path`" in TOOLS_SECTION assert "case-insensitive" in TOOLS_SECTION assert "up to 20" in TOOLS_SECTION assert "source/path:line: text" in TOOLS_SECTION @@ -186,6 +193,66 @@ def test_tools_section_teaches_the_harness_shapes() -> None: assert "Answer as soon as you have what you need" in TOOLS_SECTION +def test_tools_section_phase72_contract_clauses() -> None: + """Phase 72: the two contract clauses the teaching refusals + re-state after the fact, pinned byte-for-byte in the constant — + the ``ls`` source-name clause (its optional ``path`` is a source + name, not a directory or file path; omit it to list every + document) and the ``read``/``grep`` combined-identity clause + (the combined ``source/path`` string exactly as shown in the + ``ls`` output, *including the source name*; a bare document path + will not resolve).""" + # The ls source-name clause. + assert ( + "a source name (e.g. 'homelab'), not a directory or file " + "path — omit it to list every document" + ) in TOOLS_SECTION + # The read combined-identity clause. + assert ( + "combined `source/path` string, exactly as shown in the `ls` " + "output — including the source name" + ) in TOOLS_SECTION + # The bare-path note: read clause AND grep clause (exactly twice). + assert TOOLS_SECTION.count( + "a bare document path (without the source name) will not resolve" + ) == 2 + # The pre-phase-70 scope wording is gone — replaced by the + # explicit source-name contract. + assert "pass a source name as `path`" not in TOOLS_SECTION + + +def test_tools_section_phase72_clauses_in_high_prompt_not_low() -> None: + """Phase 72: the contract clauses ride the HIGH prompt with the + rest of the section and never leak into the LOW/deflection prompt + (whose byte-identity is pinned in + :func:`test_zero_note_prompt_is_byte_identical_to_pre_steering`).""" + doc = _doc("kubernetes.md", "Talos Linux on three nodes.", "Kubernetes Homelab Cluster") + high = build_high_prompt([doc]) + assert "" in high + assert "not a directory or file path" in high + assert "including the source name" in high + for low in (build_deflect_prompt(["T1"]), build_deflect_prompt([])): + assert "" not in low + assert "not a directory or file path" not in low + assert "including the source name" not in low + + +def test_documents_section_has_no_leading_intro() -> None: + """Phase 72, task 05 (gate iterations 2-3, reverted): the + ```` section must NOT lead with an in-context reminder + or name the ```` blocks — the live telemetry showed that + copy primed the model to latch the seed documents' paths as + ``ls`` scopes (the incident turn regressed to a cap-reached loop + on run 2 and re-trapped on run 5), and the reminder never flipped + the seed-doc ``read``s (15/15 across gate runs 1-5). The section + is exactly the document blocks again.""" + doc = _doc("kubernetes.md", "Talos Linux on three nodes.", "Kubernetes Homelab Cluster") + high = build_high_prompt([doc]) + i_open = high.index("") + i_block = high.index('\n" # no intro line + + def test_tools_section_old_names_and_budget_copy_gone() -> None: """The phase-37/68 tool names and the phase-37 per-tool budget line (phase 45: the round cap is the bound — the prompt does not