From 575d6c88d0e615ac5119a5ba48600f636eba7632 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Thu, 3 Sep 2026 13:39:15 -0400 Subject: [PATCH] =?UTF-8?q?feat(agent):=20strip=20raw=20tool-scaffolding?= =?UTF-8?q?=20from=20streamed=20answers=20=E2=80=94=20deterministic=20filt?= =?UTF-8?q?er=20with=20one=20bounded=20recovery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../70_harness_aligned_tools/00_phase.md | 0 .../05_mock_e2e_commit.md | 0 .../71_scaffolding_guardrails/01_filter.md | 0 .../02_llm_integration.md | 0 .../03_recovery_policy.md | 0 .../04_deflect_prompt.md | 0 .../70_harness_aligned_tools__00_phase.a1.err | 0 .../70_harness_aligned_tools__00_phase.a1.md | 20 + ...arness_aligned_tools__00_phase.a1.validate | 75 +++ ...ss_aligned_tools__05_mock_e2e_commit.a1.md | 20 + ...gned_tools__05_mock_e2e_commit.a1.validate | 75 +++ ...1_scaffolding_guardrails__01_filter.a1.err | 0 ...71_scaffolding_guardrails__01_filter.a1.md | 15 + ...ffolding_guardrails__01_filter.a1.validate | 77 +++ ...ding_guardrails__02_llm_integration.a1.err | 0 ...lding_guardrails__02_llm_integration.a1.md | 17 + ...guardrails__02_llm_integration.a1.validate | 77 +++ ...ding_guardrails__03_recovery_policy.a1.err | 0 ...lding_guardrails__03_recovery_policy.a1.md | 18 + ...guardrails__03_recovery_policy.a1.validate | 77 +++ ...lding_guardrails__04_deflect_prompt.a1.err | 0 ...olding_guardrails__04_deflect_prompt.a1.md | 17 + ..._guardrails__04_deflect_prompt.a1.validate | 77 +++ ...affolding_guardrails__05_e2e_commit.a1.err | 0 app/api/chat.py | 144 +++++- app/rag/agent.py | 194 +++++++- app/rag/llm.py | 64 ++- app/rag/prompts.py | 30 +- app/rag/scaffolding.py | 170 +++++++ tests/e2e/mock_llm.py | 128 +++++ tests/e2e/test_tool_scaffolding_guardrails.py | 438 ++++++++++++++++++ tests/integration/test_agent_tools.py | 6 +- tests/integration/test_chat_api.py | 210 ++++++++- tests/unit/test_agent.py | 308 +++++++++++- tests/unit/test_chat_gate.py | 13 +- tests/unit/test_llm_client.py | 216 ++++++++- tests/unit/test_prompts.py | 77 +++ tests/unit/test_scaffolding_filter.py | 280 +++++++++++ 38 files changed, 2793 insertions(+), 50 deletions(-) rename .agent/phases/{todo => complete}/70_harness_aligned_tools/00_phase.md (100%) rename .agent/phases/{todo => complete}/70_harness_aligned_tools/05_mock_e2e_commit.md (100%) rename .agent/phases/{todo => complete}/71_scaffolding_guardrails/01_filter.md (100%) rename .agent/phases/{todo => complete}/71_scaffolding_guardrails/02_llm_integration.md (100%) rename .agent/phases/{todo => complete}/71_scaffolding_guardrails/03_recovery_policy.md (100%) rename .agent/phases/{todo => complete}/71_scaffolding_guardrails/04_deflect_prompt.md (100%) create mode 100644 .agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.err create mode 100644 .agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.md create mode 100644 .agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.validate create mode 100644 .agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.md create mode 100644 .agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.validate create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.err create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.md create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.validate create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.err create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.md create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.validate create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.err create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.md create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.validate create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.err create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.md create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.validate create mode 100644 .agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.err create mode 100644 app/rag/scaffolding.py create mode 100644 tests/e2e/test_tool_scaffolding_guardrails.py create mode 100644 tests/unit/test_scaffolding_filter.py diff --git a/.agent/phases/todo/70_harness_aligned_tools/00_phase.md b/.agent/phases/complete/70_harness_aligned_tools/00_phase.md similarity index 100% rename from .agent/phases/todo/70_harness_aligned_tools/00_phase.md rename to .agent/phases/complete/70_harness_aligned_tools/00_phase.md diff --git a/.agent/phases/todo/70_harness_aligned_tools/05_mock_e2e_commit.md b/.agent/phases/complete/70_harness_aligned_tools/05_mock_e2e_commit.md similarity index 100% rename from .agent/phases/todo/70_harness_aligned_tools/05_mock_e2e_commit.md rename to .agent/phases/complete/70_harness_aligned_tools/05_mock_e2e_commit.md diff --git a/.agent/phases/todo/71_scaffolding_guardrails/01_filter.md b/.agent/phases/complete/71_scaffolding_guardrails/01_filter.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/01_filter.md rename to .agent/phases/complete/71_scaffolding_guardrails/01_filter.md diff --git a/.agent/phases/todo/71_scaffolding_guardrails/02_llm_integration.md b/.agent/phases/complete/71_scaffolding_guardrails/02_llm_integration.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/02_llm_integration.md rename to .agent/phases/complete/71_scaffolding_guardrails/02_llm_integration.md diff --git a/.agent/phases/todo/71_scaffolding_guardrails/03_recovery_policy.md b/.agent/phases/complete/71_scaffolding_guardrails/03_recovery_policy.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/03_recovery_policy.md rename to .agent/phases/complete/71_scaffolding_guardrails/03_recovery_policy.md diff --git a/.agent/phases/todo/71_scaffolding_guardrails/04_deflect_prompt.md b/.agent/phases/complete/71_scaffolding_guardrails/04_deflect_prompt.md similarity index 100% rename from .agent/phases/todo/71_scaffolding_guardrails/04_deflect_prompt.md rename to .agent/phases/complete/71_scaffolding_guardrails/04_deflect_prompt.md diff --git a/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.err b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.md b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.md new file mode 100644 index 0000000..c91b1e4 --- /dev/null +++ b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.md @@ -0,0 +1,20 @@ +All criteria verified green; no defects found, no fixes needed. + +## Phase 70 — Harness-Aligned Tools: verification report + +**Verified (final pass — all 5 tasks already complete in `complete/`):** +- `AGENT_TOOLS` = exactly `ls`(path?) / `read`(path) / `grep`(pattern, path?); `rg "list_documents|read_document|search_documents" app/` → no matches +- `read` splits combined `source/path` at first slash, full content (A7); `grep` locked A5 (fixed substring, case-insens., 20×200, locator-only); `ls` prints `source: X | path: Y | title: Z` +- SSE `tool` frames: new names; `argument` = the single string passed (`grep`→pattern, else path) or null +- Kill switch (`MAX_ROUNDS=0` → one `tools=None` request) and LOW/deflected byte-identity pinned by unit tests and green +- `TOOLS_SECTION` rewritten for the new surface; frontend `read`/`grep`/`ls` render (old names still render for persisted chats); README §"Agent document tools (ls + read + grep)" updated + +**Gates (exact commands):** +- `uv run pytest --cov=app` → exit 0, all pass; TOTAL coverage **99%** (>90%) +- `uv run ruff check . && uv run pyright` → "All checks passed!" / "0 errors" +- `uv run pytest tests/e2e/test_harness_aligned_tools.py -v --no-cov` → **3 passed** (ls→read, grep→read, wire-argument rule) +- Isolated regressions: `test_agent_document_tools` **4 passed**, `test_agent_unlimited_tools` **4 passed**, `test_search_tool` **3 passed**, `test_chat_rag` **3 passed** +- Commit `801639e` exists with the exact Commit-block message; phase dir in `.agent/phases/complete/` + +**Deviations:** none — no code changes required this pass. +**Next pending phase:** `71_scaffolding_guardrails` (in `todo/`). diff --git a/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.validate b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.validate new file mode 100644 index 0000000..d4dc250 --- /dev/null +++ b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__00_phase.a1.validate @@ -0,0 +1,75 @@ +........................................................................ [ 5%] +........................................................................ [ 10%] +........................................................................ [ 15%] +........................................................................ [ 20%] +........................................................................ [ 25%] +........................................................................ [ 31%] +........................................................................ [ 36%] +........................................................................ [ 41%] +........................................................................ [ 46%] +........................................................................ [ 51%] +........................................................................ [ 57%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 83%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 98%] +................... [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 149 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 139 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/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 2722 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.md b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.md new file mode 100644 index 0000000..13334a4 --- /dev/null +++ b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.md @@ -0,0 +1,20 @@ +Done — the empty `.err` is a harness runtime artifact (tracked-report convention, same as prior phases); working tree is clean. + +**Phase 70, task 05 complete** + +- `tests/e2e/mock_llm.py`: deterministic flows now emit `ls` (no args, `call_0`), `read` with the **combined** `source/path` (mock joins the two catalog fields — only parse change), `grep` with `pattern`; docstrings + phase-70 notes updated +- Existing E2E suites updated to new frames: `test_agent_document_tools.py` (`ls`→`read`), `test_agent_unlimited_tools.py` (multi-read `ls`+2×`read`), `test_search_tool.py` (`grep` + pattern argument) +- NEW `tests/e2e/test_harness_aligned_tools.py` (3 tests): READ flow (ls line → 📄 Reading line → quoted answer → done.sources incl. read doc → no raw `<|…|>`/`tool_call` markup in DOM), SEARCH flow (🔎 Searching for sentinel → answer), wire argument rule across both flows (ls→null, read→combined path, grep→pattern; names ⊆ {ls, read, grep}) +- README agent-tools section rewritten for `ls`/`read(path)`/`grep(pattern, path?)` (combined identity + A5 locator semantics); `.env.example` header comment refreshed +- Old-name cleanup so the rg criterion holds: `app/api/docs.py` route fn → `list_indexed_documents`, `app/rag/llm.py` docstrings, `test_llm_client.py`, `test_llm_stream_teardown.py`, `test_sse_events.py`, `test_mock_tool_flow.py`, `test_api.py` docstring. Remaining matches: only the explicitly-marked legacy frontend branches/pins + the marked legacy-chat persistence pin + the mandated "old names absent" regression pins from tasks 01–02 + +**Gates (all green):** +- `uv run pytest` → exit 0; `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (>90%) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors +- E2E in isolation: `test_harness_aligned_tools` 3 passed, `test_agent_document_tools` 4, `test_agent_unlimited_tools` 4, `test_search_tool` 3, `test_chat_rag` 3 + +**Decisions:** mock `_tool_flow` tuple kept `(source, path)`; the join happens at the wire-emission point (keeps `test_mock_tool_flow.py` pins valid). `.env.example` included in the commit for atomicity (not in the block's path list, but task 05 touched it). Phase-dir move left to the harness per executor rules. + +**Commit:** `801639e feat(agent): align the document tools with the harness-trained shape — ls, read(path), grep(pattern, path?)` (55 files, `--no-gpg-sign`) + +**Next pending:** `71_scaffolding_guardrails/01_filter.md` diff --git a/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.validate b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.validate new file mode 100644 index 0000000..d4dc250 --- /dev/null +++ b/.agent/reports/70_harness_aligned_tools/70_harness_aligned_tools__05_mock_e2e_commit.a1.validate @@ -0,0 +1,75 @@ +........................................................................ [ 5%] +........................................................................ [ 10%] +........................................................................ [ 15%] +........................................................................ [ 20%] +........................................................................ [ 25%] +........................................................................ [ 31%] +........................................................................ [ 36%] +........................................................................ [ 41%] +........................................................................ [ 46%] +........................................................................ [ 51%] +........................................................................ [ 57%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 72%] +........................................................................ [ 77%] +........................................................................ [ 83%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 98%] +................... [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 149 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 139 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/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 2722 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__01_filter.a1.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.md new file mode 100644 index 0000000..9bbaf64 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.md @@ -0,0 +1,15 @@ +Task complete — all criteria green. + +**Implemented** +- `app/rag/scaffolding.py`: pure module (`import re` only) with `SCAFFOLD_PATTERNS` (observed span + 2 standalone siblings, non-greedy span) and `ScaffoldingFilter` — streaming state machine: `feed()` strips leftmost-complete matches repeatedly, holds the live tail (open span or longest proper-prefix suffix of the 3 opening literals), `flush()` emits the tail as-is (pinned EOF choice), `stripped_chars` read-only property. +- `tests/unit/test_scaffolding_filter.py`: 61-test matrix — span at every start-token split offset (0..19) + mid-span/end-token splits, multi-span, standalone tokens (alone/embedded/char-by-char), look-alikes verbatim, partial markers at EOF, exact surrounding-text preservation. + +**Results** +- `uv run pytest tests/unit/test_scaffolding_filter.py -v --no-cov` → 61 passed +- `rg "import (os|sys|logging|app)" app/rag/scaffolding.py` → no matches +- `uv run pytest --cov=app` → 1448 passed; `app/rag/scaffolding.py` 100%, TOTAL 99% (>90%) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors + +**Decisions** — open-span defined as "start with no end token *after* it" (prevents scaffolding leak when a lone end token precedes a start); repeated leftmost-strip loop pins spec behavior where removing one match can expose another. No deviations. + +**Next pending task:** `02_llm_integration.md` (wire filter into `chat_stream`/`chat_stream_retried`). diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.validate new file mode 100644 index 0000000..e3980f8 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__01_filter.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 49%] +........................................................................ [ 54%] +........................................................................ [ 59%] +........................................................................ [ 64%] +........................................................................ [ 69%] +........................................................................ [ 74%] +........................................................................ [ 79%] +........................................................................ [ 84%] +........................................................................ [ 89%] +........................................................................ [ 94%] +........................................................................ [ 99%] +........ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 149 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 139 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/scaffolding.py 50 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 2772 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__02_llm_integration.a1.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.md new file mode 100644 index 0000000..9ecf9e4 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.md @@ -0,0 +1,17 @@ +Task 02 complete. All gates green. + +**Implemented** +- `app/rag/llm.py`: `chat_stream(..., scaffolding: ScaffoldingFilter | None = None)` — content deltas fed through the caller-owned filter (empty clean result yields nothing), thinking never filtered, `None` keeps the byte-identical raw path; `flush()` tail is yielded as a content piece **before** tool-call materialization; phase-48 teardown untouched. `chat_stream_retried(..., scaffolding=None)` forwards the same filter to every attempt (safe by construction — a restart only happens pre-piece, i.e. filter never fed). +- Tool materialization unified to a single post-loop emission point (the old in-loop `finish_reason="tool_calls"` early emission was removed) so the flushed tail always precedes `ToolCallPiece`s; identical wire behavior for all real/fixture stream shapes. +- Tests: 7 new `chat_stream` filter tests + 2 retry pass-through tests in `tests/unit/test_llm_client.py` (span mid-stream, split across chunks, pure-scaffolding → zero pieces, thinking raw, `None` raw pin, flush-before-tools order, abandon-with-filter teardown, filter forwarded to every attempt, unfed-filter reuse). +- Added the `scaffolding` kwarg to the five other `chat_stream` fakes (test_agent, test_chat_gate, test_agent_tools, test_chat_api) — required interface maintenance for the retry primitive to forward it. + +**Results** +- `uv run pytest tests/unit/test_llm_client.py tests/unit/test_llm_stream_teardown.py -v --no-cov` → 60 passed +- `uv run pytest --cov=app --cov-report=term-missing` → 1457 passed; `app/rag/llm.py` 100%, TOTAL 99% (>90%) +- `uv run ruff check . && uv run pyright` → clean +- E2E regressions in isolation: `test_harness_aligned_tools` (3), `test_agent_document_tools` (4), `test_chat_rag` (3) → all passed + +**Decisions:** single post-loop tool-materialization point (simpler; pins the content-before-tools order at both former emission points); no commit — the phase commit belongs to task 05. + +**Next pending task:** `.agent/phases/todo/71_scaffolding_guardrails/03_recovery_policy.md` diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.validate new file mode 100644 index 0000000..e1437fe --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__02_llm_integration.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 49%] +........................................................................ [ 54%] +........................................................................ [ 59%] +........................................................................ [ 64%] +........................................................................ [ 69%] +........................................................................ [ 74%] +........................................................................ [ 79%] +........................................................................ [ 84%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 98%] +................. [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 149 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 139 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 50 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 2775 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__03_recovery_policy.a1.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.md new file mode 100644 index 0000000..9e27f71 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.md @@ -0,0 +1,18 @@ +All green. Task 03 is complete. + +**Implemented** +- `app/rag/agent.py`: `CORRECTION_INSTRUCTION` (verbatim constant), `MalformedReplyError(LLMError)`, `AgentHolder.scaffold_stripped`; `run_agent` now runs every request (rounds, forced final, recovery) through a fresh `ScaffoldingFilter`, logs one capture warning per stripped span (200-char truncation), and on a scaffolding-only no-calls round runs exactly one `tools=None` recovery with the correction folded into the single system message; a second empty reply raises `MalformedReplyError` +- `app/api/chat.py`: deflected path uses a turn filter + extracted `_pump` piece loop, one bounded recovery when the filter wiped the reply, `MalformedReplyError` caught before the generic `LLMError` (dedicated error copy, no `done`/`query_log`), log line ends `…retries=N scaffold_stripped=N` +- `app/rag/scaffolding.py`: `stripped_spans` (allowed by task); tests: 10 new unit (grounded matrix) + 3 new integration (deflected matrix), fakes now feed the filter like the real client + +**Results** +- `uv run pytest tests/unit/test_agent.py tests/integration/test_chat_api.py -v --no-cov` → 94 passed +- `uv run pytest --cov=app --cov-report=term-missing` → green, TOTAL **99%** (>90%; all touched files 100%) +- `uv run ruff check . && uv run pyright` → clean +- E2E regressions in isolation: `test_harness_aligned_tools.py` (3 passed), `test_chat_rag.py` + `test_agent_document_tools.py` (7 passed) + +**Decisions** +- Task's message formula conflicted with the locked "single system message" decision — implemented the locked one (replace original system msg with folded correction, history intact), pinned by unit tests +- Forced-final request is filtered too; scaffolding-only final settles `MalformedReplyError` (no recovery after it — at most one per turn) + +**Next pending task:** `.agent/phases/todo/71_scaffolding_guardrails/04_deflect_prompt.md` diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.validate new file mode 100644 index 0000000..d22449c --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__03_recovery_policy.a1.validate @@ -0,0 +1,77 @@ +........................................................................ [ 4%] +........................................................................ [ 9%] +........................................................................ [ 14%] +........................................................................ [ 19%] +........................................................................ [ 24%] +........................................................................ [ 29%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 48%] +........................................................................ [ 53%] +........................................................................ [ 58%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 83%] +........................................................................ [ 88%] +........................................................................ [ 93%] +........................................................................ [ 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__04_deflect_prompt.a1.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.md b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.md new file mode 100644 index 0000000..4ba426f --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.md @@ -0,0 +1,17 @@ +All done. Final report: + +**Task 04 complete — deflection plain-text line (prevention)** + +- `app/rag/prompts.py`: appended "Reply in plain text only — you have no tools in this mode." as its own line to the `DEFLECT_MODE` body (LOW prompt's only change); module + function docstrings carry the phase-71 note (owner-permitted 2026-09-03; marker-keying contract unchanged); stale "LOW byte-identical" claims in the `` paragraph/comment corrected. +- `tests/unit/test_prompts.py`: diff/reconstruction pin (LOW = pre-phase text + exactly the one line, `count == 1`, title list untouched), line-present + marker-present pin, no-leak-into-HIGH pin (all 4 HIGH build variants); both empty-steering/overview byte-identity pins updated to the new LOW baseline. +- `tests/unit/test_agent.py`: LOW byte pin updated to include the line. `tests/unit/test_chat_gate.py`: line pinned present in the deflected `plan_turn` prompt and absent from the HIGH prompt. + +**Results** +- `uv run pytest tests/unit/test_prompts.py tests/unit/test_chat_gate.py -v --no-cov` → 61 passed +- `uv run pytest --cov=app --cov-report=term-missing` → 1473 passed, TOTAL 99% (`app/rag/prompts.py` 100%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- `uv run pytest tests/e2e/test_honest_deflection.py -v --no-cov` → 3 passed (mock marker-keying intact) + +**Decisions**: the line goes on its own line (the completion criterion's "pre-phase text + exactly one new line" diff pin), not appended to the existing sentence. No commit — commits belong to task 05 (phase 70 + 71/01–03 work is still uncommitted in-tree). + +**Next pending task**: `.agent/phases/todo/71_scaffolding_guardrails/05_e2e_commit.md` diff --git a/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.validate b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.a1.validate new file mode 100644 index 0000000..5b9d789 --- /dev/null +++ b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__04_deflect_prompt.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.err b/.agent/reports/71_scaffolding_guardrails/71_scaffolding_guardrails__05_e2e_commit.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/app/api/chat.py b/app/api/chat.py index 6f8b444..d2fbbc2 100644 --- a/app/api/chat.py +++ b/app/api/chat.py @@ -91,6 +91,30 @@ the same budget, and the deflected answer stream goes through The per-turn log line records ``retries=N`` after ``total_ms=N`` (0 when nothing was retried — the field is uniform across all turn shapes). + +Tool-scaffolding guardrail (phase 71, deterministic only — owner +permission 2026-09-03: "deterministic guardrails only right now, forget +using a model for that"): the raw chat-template tokens +``<|tool_call_start|>…<|tool_call_end|>`` the ``lite`` model sometimes +emits as plain answer text can never reach the user. The DEFLECTED +path's request runs ``delta.content`` through a caller-owned +``ScaffoldingFilter`` (content only — thinking stays raw); when the +filter wipes the whole reply (visible content 0, ``stripped_chars > 0``) +the turn gets exactly ONE bounded recovery: the same messages with +``agent.CORRECTION_INSTRUCTION`` folded into the single system prompt, +``tools=None``, a fresh filter, the same phase-67 retry budget, streamed +through the same piece loop (extracted as the inner ``_pump`` helper). +A recovery that also comes back empty — and any grounded round where the +recovery policy in ``run_agent`` fails — settles with the dedicated +structured ``error`` frame (``MalformedReplyError`` caught before the +generic ``LLMError`` handler): no ``done``, no ``query_log`` row, +byte-for-byte the existing terminal-error shape. The per-turn log line +records ``scaffold_stripped=N`` after ``retries=N`` — the sum across the +turn's requests (grounded: the agent's rounds + forced final + any +recovery, via the holder; deflected: this turn's filters), 0 on clean +turns (the field is uniform, the phase-67 ``retries=N`` pattern); the +recovery does not bump ``retries=N`` (it is not a phase-67 +endpoint-retry). """ from __future__ import annotations @@ -110,7 +134,12 @@ from app.api.steering import load_steering_notes from app.config import Settings, get_settings from app.db import db_available, get_db from app.models import Document, QueryLog -from app.rag.agent import AgentHolder, run_agent +from app.rag.agent import ( + CORRECTION_INSTRUCTION, # phase 71: the harness-owned recovery line + AgentHolder, + MalformedReplyError, # phase 71: the recovery policy's terminal signal + run_agent, +) from app.rag.llm import ( EmbeddingError, LLMClient, @@ -123,6 +152,7 @@ from app.rag.llm import ( from app.rag.overview import load_kb_overview from app.rag.prompts import build_deflect_prompt, build_high_prompt from app.rag.retriever import RetrievedChunk, retrieve, select_documents, weak_hit_titles +from app.rag.scaffolding import ScaffoldingFilter # phase 71: the streaming filter from app.rag.suggestions import derive_suggestions from app.schemas import ( ChatDoneEvent, @@ -373,19 +403,27 @@ async def chat( # ``tools=None`` request anyway (the kill switch). holder = AgentHolder() answer_stream: AsyncIterator[StreamPiece | ToolCallPiece | RetryPiece] + deflected_filter: ScaffoldingFilter | None = None if plan.deflected: # Phase 67: the deflected stream goes through the retry # primitive — a dead endpoint is restarted (SSE ``retry`` # frames) only before its first piece (locked A2); the # grounded path stays a plain ``run_agent`` call (task 03 # makes IT retry internally) — its ``RetryPiece``s flow - # through the shared piece loop below. + # through the shared piece loop below. Phase 71: the + # request's content also runs through a caller-owned + # filter (one per request) — a scaffolding-only reply + # streams zero ``delta`` frames instead of raw tokens, and + # the filter's ``stripped_chars`` drives the recovery + # decision after the piece loop. + deflected_filter = ScaffoldingFilter() answer_stream = chat_stream_retried( llm, messages, tools=None, retries=settings.llm_retries, delay=settings.llm_retry_delay, + scaffolding=deflected_filter, ) else: answer_stream = run_agent( @@ -398,8 +436,19 @@ async def chat( holder=holder, ) thinking_chars = 0 - try: - async for piece in answer_stream: # StreamPiece | ToolCallPiece | RetryPiece + content_chars = 0 # phase 71: the turn's visible (clean) content + scaffold_stripped = 0 # phase 71: sum across the turn's requests + + async def _pump( + pieces: AsyncIterator[StreamPiece | ToolCallPiece | RetryPiece], + ) -> AsyncIterator[str]: + """One request's piece loop (phase 71 extraction): the + thinking/tool/retry/delta handling shared by the turn's + first pass and — deflected path only — the one bounded + recovery. Behavior-preserving for the first pass (pinned + by the existing integration suite).""" + nonlocal thinking_chars, content_chars, retries_used + async for piece in pieces: # StreamPiece | ToolCallPiece | RetryPiece if isinstance(piece, ToolCallPiece): # Phase 37 (PLAN §4 extension; phase 70): one SSE # ``tool`` frame per model-requested call. @@ -435,7 +484,91 @@ async def chat( if settings.stream_thinking: yield sse_event(ChatThinkingEvent(text=piece.text).model_dump()) else: + content_chars += len(piece.text) yield sse_event({"type": "delta", "text": piece.text}) + + try: + async for frame in _pump(answer_stream): + yield frame + if plan.deflected and deflected_filter is not None: + scaffold_stripped = deflected_filter.stripped_chars + # Phase 71: the deflected reply's visible content was + # wiped by the filter (the scaffolding was the whole + # "answer") — the ONE bounded recovery: the same + # messages with the correction folded into the single + # system prompt, ``tools=None``, a FRESH filter, the + # same phase-67 retry budget, streamed through the + # same piece loop. A round with real visible content + # needs no recovery (the clean content stands). + if content_chars == 0 and deflected_filter.stripped_chars > 0: + logger.warning( + "chat: deflected reply was pure tool-scaffolding " + "(%d chars stripped) — running the one bounded " + "recovery", + deflected_filter.stripped_chars, + ) + recovery_filter = ScaffoldingFilter() + recovery_stream = chat_stream_retried( + llm, + [ + { + "role": "system", + "content": ( + plan.system_prompt + "\n" + + CORRECTION_INSTRUCTION + ), + }, + *messages[1:], + ], + tools=None, + retries=settings.llm_retries, + delay=settings.llm_retry_delay, + scaffolding=recovery_filter, + ) + async for frame in _pump(recovery_stream): + yield frame + scaffold_stripped += recovery_filter.stripped_chars + if content_chars == 0: + # The second empty reply is terminal (at most + # one recovery per turn) — the dedicated error + # frame below (no done, no query_log row). + logger.warning( + "chat: the recovery reply was still empty " + "(scaffold_stripped=%d) — settling with a " + "malformed-reply error", + scaffold_stripped, + ) + raise MalformedReplyError( + "the deflected model answered in raw " + "tool-scaffolding twice in a row — no " + "clean answer to stream" + ) + else: + # Grounded turns: the agent's rounds + forced final + + # any recovery already accumulated the turn total on + # the holder (the deflected fallback is 0 — the agent + # never runs, so this branch is grounded-only). + scaffold_stripped = holder.scaffold_stripped + except MalformedReplyError as e: + # Phase 71: the recovery policy's terminal signal — + # caught BEFORE the generic LLMError handler (it + # subclasses it), so the dedicated copy reaches the UI; + # the generic "dropped the connection" copy stays for + # transport failures. + logger.error( + "chat: malformed reply after the one bounded recovery " + "question=%r total_ms=%d — %s", + request.message, + int((time.monotonic() - started) * 1000), + e, + ) + settled = True # terminal: the error frame settles the turn + yield sse_event( + ChatErrorEvent( + detail="The model returned a malformed reply — please try again." + ).model_dump() + ) + return except LLMError as e: logger.error( "chat: LLM stream failed question=%r total_ms=%d — %s", @@ -504,7 +637,7 @@ async def chat( logger.info( "question=%r embed_ms=%d top_score=%.3f fts_hits=%d summary_hits=%d tuning=%d " "kb_chars=%d threshold=%.2f deflected=%s sources=%r thinking_chars=%d " - "tool_calls=%d total_ms=%d retries=%d", + "tool_calls=%d total_ms=%d retries=%d scaffold_stripped=%d", request.message, embed_ms, plan.top_score, @@ -519,6 +652,7 @@ async def chat( holder.tool_calls, total_ms, retries_used, + scaffold_stripped, ) settled = True # terminal: the done frame settles the turn yield sse_event( diff --git a/app/rag/agent.py b/app/rag/agent.py index 1558f4b..cc53158 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -95,6 +95,27 @@ task 04): exactly one round. With ``settings.llm_retries=0`` every request is a single plain attempt (the pre-phase-67 path). +Scaffolding guardrail (phase 71, deterministic only — owner permission +2026-09-03: "deterministic guardrails only right now, forget using a +model for that"): every model request (each round, the forced final, +and any recovery) runs its ``delta.content`` through a fresh caller- +owned :class:`app.rag.scaffolding.ScaffoldingFilter`, so raw +``<|tool_call_start|>…<|tool_call_end|>`` tokens can never reach the +user as answer text. A round that ends with NO visible content AND a +non-empty strip (the scaffolding was the whole "answer") gets exactly +ONE bounded recovery: one extra request with ``tools=None``, the same +messages with :data:`CORRECTION_INSTRUCTION` folded into the original +single system message, a fresh filter, and the same phase-67 retry +budget. A recovery that also comes back empty — or a round with no +strip and no content (today's empty/thinking-only answer) — settles as +before; a second empty reply raises :class:`MalformedReplyError` (the +API layer turns it into the dedicated error frame). A round with real +visible content plus scaffolding needs no recovery (the clean content +stands), and a scaffolding-only round that also carried tool calls +needs none either (the tool ran) — the policy keys on the no-calls +exit only. No model participates in detection or repair: the +registry + the fixed retry policy are the whole guardrail. + The DB accessors (:func:`list_catalog`, :func:`list_source_names`, :func:`find_document`, :func:`all_documents`) and the :func:`grep_document` line matcher are module-level functions so unit @@ -116,11 +137,13 @@ from app.models import Document from app.rag.git_sources import effective_sources from app.rag.llm import ( LLMClient, + LLMError, RetryPiece, StreamPiece, ToolCallPiece, chat_stream_retried, ) +from app.rag.scaffolding import ScaffoldingFilter from app.rag.source_removal import resolve_source_name logger = logging.getLogger("app.agent") @@ -228,6 +251,33 @@ UNKNOWN_TOOL = "Unknown tool." MISSING_READ_ARGS = "read requires a string argument 'path'." MISSING_SEARCH_ARGS = "grep requires a string argument 'pattern'." +#: 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, +#: the user message stays last). Verbatim constant: the E2E mock +#: (task 05) keys on a stable substring of it, so it must not drift. +CORRECTION_INSTRUCTION: str = ( + "Your previous reply contained raw tool-call markup, which is not " + "interpreted here. Answer the user's question directly in plain " + "text — no tool syntax." +) + + +class MalformedReplyError(LLMError): + """The model kept replying in raw tool-scaffolding (phase 71). + + Raised ONLY by the recovery policy — :func:`run_agent` (grounded + path) and ``app.api.chat`` (deflected path) — when the one bounded + ``tools=None`` recovery still comes back with no visible content. + It is never raised from inside a stream, so + :func:`app.rag.llm.chat_stream_retried`'s retry-before-first-piece + rule never sees it. The API layer catches it BEFORE the generic + :class:`LLMError` handler and settles the turn with the dedicated + "malformed reply" error frame (no ``done``, no ``query_log`` row). + Deterministic only (owner permission 2026-09-03): no model + participates in detection or repair. + """ + #: Search caps (owner-locked A5, phase 68): a global per-call match cap #: (across documents, in catalog order) and a per-match-line char limit. SEARCH_MAX_MATCHES = 20 @@ -352,10 +402,16 @@ class AgentHolder: rejected calls (unknown tool, unknown/missing arguments or document, already-in-context) do not count. Drives the per-turn log line's ``tool_calls=N`` field (task 04). + ``scaffold_stripped``: how many chars of tool-scaffolding the + turn's filters removed across the turn's requests (rounds + the + forced final + any recovery, phase 71) — drives the per-turn log + line's ``scaffold_stripped=N`` field on grounded turns (the + deflected path computes its own total in ``app.api.chat``). """ read_docs: list[Document] = field(default_factory=list) tool_calls: int = 0 + scaffold_stripped: int = 0 def _execute_tool( @@ -476,6 +532,24 @@ async def run_agent( ``settings.llm_retry_delay``); a round that already streamed pieces fails the turn as before. + Scaffolding recovery (phase 71, deterministic only): every request — + each round, the forced final, and any recovery — runs its content + through a fresh :class:`app.rag.scaffolding.ScaffoldingFilter`. A + round that ends with NO visible content but a non-empty strip (the + scaffolding was the whole "answer") gets exactly ONE recovery: + ``tools=None``, :data:`CORRECTION_INSTRUCTION` folded into the + original single system message (the rest of the history — user + message and tool results — unchanged), a fresh filter, the same + retry budget. A clean recovery ends the turn; a second empty reply + raises :class:`MalformedReplyError` (terminal — the API layer turns + it into the dedicated error frame). A round with visible content + plus scaffolding needs no recovery (the clean content stands), and + a scaffolding-only round that also carried tool calls needs none + (the tool ran) — the policy keys on the no-calls exit only. The + per-span strip warning log (each span truncated to 200 chars) is + the capture mechanism for new registry entries; *holder* accumulates + the turn's ``scaffold_stripped`` total for the API layer's log line. + ``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 @@ -494,6 +568,12 @@ async def run_agent( rounds = 0 while True: calls: list[ToolCallPiece] = [] + # Phase 71: one fresh filter per round (one per model request — + # the retry attempts of this logical request share it: a restart + # only happens while the filter was never fed). Content-only: + # thinking pieces pass through raw. + round_filter = ScaffoldingFilter() + round_content = 0 # visible (clean) content chars this round # Phase 48: bind the round's stream so a consumer abandon # (GeneratorExit into the yield below) tears down the in-flight # model stream deterministically — not GC-dependent. Phase 67: @@ -511,16 +591,97 @@ async def run_agent( tools=tools, retries=settings.llm_retries, delay=settings.llm_retry_delay, + scaffolding=round_filter, ) try: async for piece in stream: if isinstance(piece, ToolCallPiece): calls.append(piece) + elif isinstance(piece, StreamPiece) and piece.kind == "content": + round_content += len(piece.text) yield piece finally: await stream.aclose() + # Phase 71: the per-strip-event capture log — one warning per + # stripped span, truncated to 200 chars (how a new scaffolding + # format gets captured and added to the registry) — and the turn + # total for the API layer's ``scaffold_stripped=N`` log field. + holder.scaffold_stripped += round_filter.stripped_chars + for span in round_filter.stripped_spans: + logger.warning( + "agent: stripped %d chars of tool-scaffolding in round %d: %r", + len(span), + rounds + 1, + span[:200], + ) if not calls: - return # the answer was streamed + if round_content > 0: + return # the answer was streamed (the clean content stands) + if round_filter.stripped_chars == 0: + # Empty/thinking-only answer — today's behavior, unchanged + # (the UI handles it); the guardrail keys on a strip. + return + # Phase 71: the scaffolding was the whole "answer" — the ONE + # bounded recovery (a fixed policy, not a conversation): + # ``tools=None``, the correction folded into the ORIGINAL + # single system message (provider-safe — the user message and + # any tool history stay in place), a fresh filter, the same + # phase-67 retry budget. + logger.warning( + "agent: round %d was pure tool-scaffolding (%d chars stripped) " + "— running the one bounded recovery", + rounds + 1, + round_filter.stripped_chars, + ) + messages_recovered = [ + { + "role": "system", + "content": system_prompt + "\n" + CORRECTION_INSTRUCTION, + }, + *messages[1:], + ] + recovery_filter = ScaffoldingFilter() + recovered = chat_stream_retried( + llm, + cast("list[dict[str, str]]", messages_recovered), + tools=None, + retries=settings.llm_retries, + delay=settings.llm_retry_delay, + scaffolding=recovery_filter, + ) + recovery_content = 0 + try: + async for piece in recovered: + if isinstance(piece, StreamPiece) and piece.kind == "content": + recovery_content += len(piece.text) + yield piece + finally: + await recovered.aclose() + holder.scaffold_stripped += recovery_filter.stripped_chars + for span in recovery_filter.stripped_spans: + logger.warning( + "agent: stripped %d chars of tool-scaffolding in the " + "recovery after round %d: %r", + len(span), + rounds + 1, + span[:200], + ) + if recovery_content > 0: + return # the recovery answered — the turn ends + # The second empty reply is terminal (at most one recovery per + # turn). Raised OUTSIDE the stream, so chat_stream_retried's + # retry rule never sees it; the API layer catches it before + # the generic LLMError handler. + logger.warning( + "agent: the recovery reply was still empty " + "(scaffold_stripped=%d) — settling with a malformed-reply error", + holder.scaffold_stripped, + ) + raise MalformedReplyError( + f"the model answered in raw tool-scaffolding twice in a row " + f"(round {rounds + 1} plus one recovery) — no clean answer " + "to stream" + ) call = calls[0] # a stream can carry several calls; run the first result = _execute_tool(db, call, seed_docs, holder) rounds += 1 # every call the model emits consumes a round @@ -558,17 +719,46 @@ async def run_agent( # teardown as the loop rounds (consumer abandon mid-final # answer must still close the model's stream). Phase 67: the # forced call retries under the same locked-A2 rule as the - # loop rounds. + # loop rounds. Phase 71: the forced final runs through a + # fresh filter too — raw scaffolding can never reach the + # user from ANY grounded request. + final_filter = ScaffoldingFilter() final = chat_stream_retried( llm, cast("list[dict[str, str]]", messages), tools=None, retries=settings.llm_retries, delay=settings.llm_retry_delay, + scaffolding=final_filter, ) + final_content = 0 try: async for piece in final: + if isinstance(piece, StreamPiece) and piece.kind == "content": + final_content += len(piece.text) yield piece finally: await final.aclose() + # Phase 71: the same capture log + turn total; a + # scaffolding-only forced final (this turn used no recovery, + # so nothing is doubled up) settles with the same terminal + # malformed-reply error rather than a silently empty answer. + holder.scaffold_stripped += final_filter.stripped_chars + for span in final_filter.stripped_spans: + logger.warning( + "agent: stripped %d chars of tool-scaffolding in the " + "forced final answer (round %d): %r", + len(span), + rounds + 1, + span[:200], + ) + if final_content == 0 and final_filter.stripped_chars > 0: + logger.warning( + "agent: the forced final answer was pure tool-scaffolding " + "— settling with a malformed-reply error" + ) + raise MalformedReplyError( + "the forced final answer was raw tool-scaffolding — no " + "clean answer to stream" + ) return diff --git a/app/rag/llm.py b/app/rag/llm.py index af39427..1e54143 100644 --- a/app/rag/llm.py +++ b/app/rag/llm.py @@ -23,13 +23,18 @@ import json import logging from collections.abc import AsyncGenerator from dataclasses import dataclass -from typing import Any, Literal, cast +from typing import TYPE_CHECKING, Any, Literal, cast from openai import AsyncOpenAI, AsyncStream from openai.types.chat import ChatCompletionChunk, ChatCompletionMessageParam from app.config import Settings, get_settings +if TYPE_CHECKING: + # Phase 71: the filter type is only needed for typing (the module + # stays import-graph-clean; callers pass their own instances). + from app.rag.scaffolding import ScaffoldingFilter + logger = logging.getLogger("app.llm") @@ -338,6 +343,7 @@ class LLMClient: self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, ) -> AsyncGenerator[StreamPiece | ToolCallPiece, None]: """Stream assistant pieces from the chat model (PLAN A5/A15, phase 17). @@ -365,13 +371,26 @@ class LLMClient: ``id`` and ``function.name`` on the first partial and ``function.arguments`` in fragments — which are accumulated into one :class:`ToolCallPiece` per call, yielded in index order at - stream end (or immediately once a chunk carries - ``finish_reason="tool_calls"``). Malformed ``arguments`` JSON - raises :class:`LLMError`. Wire convention verified live against + stream end (after the stream's chunks are exhausted — aipi ends + the stream at ``finish_reason="tool_calls"``, so this is the + wire's emission point). Malformed ``arguments`` JSON raises + :class:`LLMError`. Wire convention verified live against aipi's ``turbo`` on 2026-08-26 via ``uv run python -m scripts.llm_probe --tools`` (phase 37, task 01: ``probe: turbo tool_calls=supported 2026-08-26``). + Scaffolding guardrail (phase 71): the caller may pass a + ``ScaffoldingFilter`` — one per request, caller-owned (this + method never creates or resets one). When present, only + **content** is filtered: ``delta.content`` is fed through the + filter and only the clean text is yielded (an empty clean + result yields **no** piece — no empty ``delta`` frames); thinking + pieces are never filtered (the scratchpad stays raw, phase 17). + At stream end the filter's held tail is flushed to a content + piece **before** any tool-call materialization (content- + before-tools wire convention). ``None`` (the default) keeps + today's byte-identical raw path for callers that opt out. + Any failure (network, HTTP, malformed stream) surfaces as :class:`LLMError` so the API layer can turn it into an SSE ``error`` event instead of a hung request. @@ -405,7 +424,6 @@ class LLMClient: await self._client.chat.completions.create(**kwargs), ) calls: dict[int, _ToolCallSlot] = {} - emitted = False async for chunk in stream: if not chunk.choices: continue @@ -436,16 +454,22 @@ class LLMClient: yield StreamPiece("thinking", reasoning) content = delta.content if content: - yield StreamPiece("content", content) - if ( - calls - and not emitted - and getattr(choice, "finish_reason", None) == "tool_calls" - ): - for piece in _materialize_tool_calls(calls): - yield piece - emitted = True - if calls and not emitted: + if scaffolding is not None: + # Phase 71: content only — an empty clean result + # yields nothing (no empty delta frames). + cleaned = scaffolding.feed(content) + if cleaned: + yield StreamPiece("content", cleaned) + else: + yield StreamPiece("content", content) + # Phase 71: flush the filter's held tail at stream end, BEFORE + # any tool-call materialization — flushed-tail content precedes + # ToolCallPieces (content-before-tools wire convention). + if scaffolding is not None: + tail = scaffolding.flush() + if tail: + yield StreamPiece("content", tail) + if calls: for piece in _materialize_tool_calls(calls): yield piece except LLMError: @@ -469,6 +493,7 @@ async def chat_stream_retried( tools: list[dict[str, Any]] | None = None, retries: int = 0, delay: float = 0.0, + scaffolding: ScaffoldingFilter | None = None, ) -> AsyncGenerator[StreamPiece | ToolCallPiece | RetryPiece, None]: """Stream a chat turn, retrying a dead endpoint (phase 67). @@ -493,6 +518,13 @@ async def chat_stream_retried( The request is restarted byte-identical: ``chat_stream`` is stateless, so every attempt is opened with the SAME *messages*/*tools*. + Scaffolding guardrail (phase 71): *scaffolding* is passed through to + every attempt's ``chat_stream``. The SAME caller-owned filter object + across the retry attempts of one logical request is safe by + construction: a restarted attempt only happens while no piece was + emitted, i.e. the filter was never fed (its pending buffer is still + empty). + Teardown (phase 48, extended): every attempt's stream is explicitly closed in a ``finally`` — normal exhaustion, a terminal :class:`LLMError`, and a consumer abandon (``GeneratorExit`` mid-attempt @@ -502,7 +534,7 @@ async def chat_stream_retried( max_attempts = retries + 1 for attempt in range(1, max_attempts + 1): emitted = False - stream = llm.chat_stream(messages, tools=tools) + stream = llm.chat_stream(messages, tools=tools, scaffolding=scaffolding) try: async for piece in stream: emitted = True diff --git a/app/rag/prompts.py b/app/rag/prompts.py index 53416dc..a9c2448 100644 --- a/app/rag/prompts.py +++ b/app/rag/prompts.py @@ -30,7 +30,19 @@ Agent tools (phase 37; phase 70: the copy teaches the harness-aligned 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 and stays byte-identical to the pre-phase text. +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 +line — "Reply in plain text only — you have no tools in this mode." +— appended to the ``DEFLECT_MODE`` body: a deflected turn offers no +tools, so any tool markup there is always wrong, and the line closes +the door at the prompt (the deterministic filter + one bounded +recovery in :mod:`app.rag.scaffolding` / :mod:`app.rag.agent` is the +backstop). The ``DEFLECT_MODE`` marker and everything else in the +prompt stay put — the E2E mock LLM keys on the marker's *presence*, +not the wording, so that contract is unchanged. """ from __future__ import annotations @@ -83,9 +95,10 @@ _KB_INTRO = ( #: 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 — and stays byte-identical to the -#: pre-phase text. The E2E mock keys off the ```` marker's -#: *presence*, not this wording. +#: 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. TOOLS_SECTION: str = ( "\n" "You may extend your context with three tools. `ls` lists the " @@ -224,7 +237,10 @@ def build_deflect_prompt( Section order (phase 31): ```` → ```` → ```` → ``DEFLECT_MODE`` body; empty steering/overview omit - their section, keeping the prompt byte-identical to the pre-phase text. + their section, keeping the prompt byte-identical to the pre-phase + text. The body ends with the phase-71 plain-text line (owner- + permitted 2026-09-03 — the LOW prompt's only change): a deflected + turn offers no tools, so any tool markup there is always wrong. """ weak = "\n".join(f"- {t}" for t in titles) if titles else "(nothing close at all)" mid = "\n".join( @@ -239,5 +255,9 @@ def build_deflect_prompt( + "DEFLECT_MODE: retrieval was weak — the titles below are the closest " "your notes come to the question. They are titles only; do not pretend " "they answer it. Use them to propose 2-3 alternative questions.\n" + # Phase 71 (owner-permitted 2026-09-03): the one plain-text line + # — prevention at the prompt. The E2E mock keys on the + # DEFLECT_MODE marker's presence, so appending is safe. + "Reply in plain text only — you have no tools in this mode.\n" + weak ) diff --git a/app/rag/scaffolding.py b/app/rag/scaffolding.py new file mode 100644 index 0000000..3927d42 --- /dev/null +++ b/app/rag/scaffolding.py @@ -0,0 +1,170 @@ +"""Deterministic tool-scaffolding guardrail — pattern registry + streaming +filter (phase 71, task 01). + +The ``lite`` chat model occasionally emits its own chat-template +tool-scaffolding as plain answer text (incident 2026-09-03: a deflected +round streamed ``<|tool_call_start|>[read(path='…')]<|tool_call_end|>`` +into the UI although no tools were offered). This module is the +detection half of the guardrail: a fixed registry of *observed* +scaffolding forms and a streaming state machine that strips them from +``delta.content`` as it flows. Pure Python — no I/O, no logging, no +model: the strip *warning* log and the one bounded recovery are emitted +by the integration layer (phase 71, tasks 02–03), so this module stays +unit-testable in isolation. + +Content only: thinking pieces are the model's raw reasoning by design +(phase 17) and are never filtered — the guardrail protects the answer, +not the scratchpad. +""" +import re + +__all__ = ["SCAFFOLD_PATTERNS", "ScaffoldingFilter"] + +#: The known scaffolding forms. The registry is the extension point: a +#: new entry needs an observed capture (the strip warning log, task 03) +#: + a unit fixture in ``tests/unit/test_scaffolding_filter.py`` — no +#: speculative entries. Every entry here traces to the 2026-09-03 +#: incident (the span form is the deflected round's raw text; the +#: standalone siblings are from the same tokenizer family). +SCAFFOLD_PATTERNS: tuple[re.Pattern, ...] = ( + # The observed span (incident 2026-09-03) — non-greedy, so multiple + # spans in one buffer each strip to their own end token. + re.compile(r"<\|tool_call_start\|>[\s\S]*?<\|tool_call_end\|>"), + # Standalone sibling token (same tokenizer family). + re.compile(r"<\|tool_calls\|>"), + # Standalone sibling token (same tokenizer family). + re.compile(r"<\|tool_call\|>"), +) + +_SPAN_START = "<|tool_call_start|>" +_SPAN_END = "<|tool_call_end|>" +#: The literal openings a live (possibly incomplete) match can begin with. +_OPENINGS: tuple[str, ...] = (_SPAN_START, "<|tool_calls|>", "<|tool_call|>") +#: Boundedness bound: in NORMAL state the held tail is at most one char +#: short of the longest opening (an open span is unbounded, but it is +#: being *stripped*, never emitted). +_MAX_OPENING = max(len(opening) for opening in _OPENINGS) + + +def _leftmost_match(buf: str) -> re.Match | None: + """The leftmost complete match among :data:`SCAFFOLD_PATTERNS`, or + None when the buffer holds no complete scaffolding form. + + (Two patterns cannot match at the same start position — their + literals diverge after ``<|tool_call`` — so the leftmost start is + unambiguous.) + """ + best: re.Match | None = None + for pattern in SCAFFOLD_PATTERNS: + match = pattern.search(buf) + if match is not None and (best is None or match.start() < best.start()): + best = match + return best + + +def _hold_index(buf: str) -> int: + """Index where the live hold-tail begins; everything from it stays + pending (called only after the strip loop found no complete match). + + The tail is the leftmost of two candidates: an **open span** (a + start token with no end token after it — the span may continue in + future chunks, so everything from that start token onward is held) + or a **live prefix** (the longest suffix that is a proper prefix of + any opening literal — the token may complete in future chunks). + Everything before the leftmost candidate is safe to emit: no + complete match remains, and no match can grow from earlier text, + since a complete opening earlier in the buffer would already have + matched (span) or be a candidate in its own right (standalone). + """ + if not buf: + return 0 + hold = len(buf) + start = buf.find(_SPAN_START) + if start != -1 and buf.find(_SPAN_END, start + 1) == -1: + hold = start + for k in range(min(len(buf), _MAX_OPENING - 1), 0, -1): + suffix = buf[-k:] + if any(k < len(opening) and opening.startswith(suffix) for opening in _OPENINGS): + hold = min(hold, len(buf) - k) + break + return hold + + +class ScaffoldingFilter: + """Streaming state machine over a model **content** stream (phase 71). + + One instance per model request (callers create fresh instances — + the phase-67 retry-after-dead-attempt case is safe: a dead attempt + never fed the filter). Feed the raw ``delta.content`` chunks; each + :meth:`feed` returns the clean text safe to emit *now*; :meth:`flush` + emits the tail at stream end. + + ``feed`` appends the chunk to the internal pending buffer, then + (a) repeatedly takes the leftmost complete match among + :data:`SCAFFOLD_PATTERNS` — drops it (counting it into + :attr:`stripped_chars`) and continues — until none remains; then + (b) checks the buffer tail for a **live prefix** via + :func:`_hold_index`: the longest suffix that is a proper prefix of + any opening literal, or an **open span** (a start token with no end + token yet — everything from that start token onward is held, since + the span may continue in future chunks). Everything before the held + tail is emitted; the held tail becomes the new pending state. + Bounded: the held tail in NORMAL state is ≤ the longest opening + token minus one char; in an open span it is unbounded but is being + *stripped*, never emitted. + + ``flush`` (end of stream) emits the pending tail **as-is**: a + partial marker at EOF is content, not scaffolding — a documented, + pinned choice (a stream that ends mid-``<|tool_call_st`` must not + be silently eaten, and a lone ``<|tool_call_end|>`` without a start + is prose the user sent or the model produced outside a span). + + The stripped spans themselves are exposed read-only + (:attr:`stripped_spans`) so the integration layer can log one + warning per strip event (phase 71 task 03) — that log line is how + a *new* scaffolding format gets captured and added to the registry. + """ + + def __init__(self) -> None: + self._pending = "" + self._stripped_chars = 0 + self._stripped_spans: list[str] = [] + + @property + def stripped_chars(self) -> int: + """Total characters removed so far (read by the caller after the + round/turn — the strip warning log, phase 71 task 03).""" + return self._stripped_chars + + @property + def stripped_spans(self) -> list[str]: + """The raw spans removed so far, in stream order (one entry per + strip event — the integration layer's per-span warning log + truncates each to 200 chars). Read-only: callers must not + mutate the filter's state.""" + return list(self._stripped_spans) + + def feed(self, chunk: str) -> str: + """Append *chunk* to the pending buffer; return the clean text + safe to emit now (possibly ``""`` — e.g. while an open span or a + partial marker is still held). An empty chunk is a no-op.""" + if not chunk: + return "" + buf = self._strip_complete(self._pending + chunk) + hold = _hold_index(buf) + self._pending = buf[hold:] + return buf[:hold] + + def flush(self) -> str: + """End of stream: emit the pending tail as-is and reset it.""" + tail = self._pending + self._pending = "" + return tail + + def _strip_complete(self, buf: str) -> str: + """Drop leftmost-complete matches until none remains (step a).""" + while (match := _leftmost_match(buf)) is not None: + self._stripped_chars += match.end() - match.start() + self._stripped_spans.append(match.group(0)) + buf = buf[: match.start()] + buf[match.end():] + return buf diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 40f6e56..a0bd411 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -127,6 +127,35 @@ Implements just enough of the aipi surface: specific phrase — same convention as ``think in paragraphs``); no existing E2E question or fixture file contains the trigger, so every other suite is unaffected. + - user message containing ``emit raw tool markup`` + (``SCAFFOLD_TRIGGER``, phase 71, tool-scaffolding guardrails — the + 2026-09-03 incident where a deflected round streamed the model's + raw ``<|tool_call_start|>…<|tool_call_end|>`` markup into the UI) + **or** ``always emit raw tool markup`` + (``SCAFFOLD_ALWAYS_TRIGGER``, checked FIRST — it contains the + former phrase) -> the deterministic SCAFFOLDING flow, independent + of the ```` marker (both grounded and deflected turns hit + it): + * ``SCAFFOLD_ALWAYS_TRIGGER``: EVERY request (the one bounded + recovery included) streams ONLY ``delta.content`` chunks + carrying the incident span ``SCAFFOLD_SPAN`` — + ``<|tool_call_start|>[read(path='search_docs/reese-notes.md')] + <|tool_call_end|>`` — split across the mock's 12-char chunks + (the filter's boundary path), ``finish_reason: "stop"``, no + structured ``tool_calls``, no reasoning — the terminal + malformed-reply path. + * ``SCAFFOLD_TRIGGER``: request 1 (no ``CORRECTION_INSTRUCTION`` + in the system prompt) streams the same scaffolding-only span; + request 2 (the system prompt carries the harness constant — a + stable substring of ``app.rag.agent.CORRECTION_INSTRUCTION``, + IMPORTED into this module so the mock can never drift from + it: the one bounded recovery, ``tools=None`` with the + correction folded into the single system prompt) streams the + clean ``SCAFFOLD_RECOVERY_ANSWER`` — the recovery path. + Checked BEFORE the ``SEARCH_TRIGGER`` / ``TOOLS_TRIGGER`` flows + (the trigger needs no ```` section); 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 @@ -188,6 +217,8 @@ from typing import Any from fastapi import FastAPI from fastapi.responses import JSONResponse, StreamingResponse +from app.rag.agent import CORRECTION_INSTRUCTION # phase 71: the harness constant + app = FastAPI() DIM = 768 @@ -364,6 +395,51 @@ ALWAYS_FAIL_TRIGGER = "always fail" #: bag-of-words vector — the endpoint's pre-stream embedding retry loop. EMBED_FAIL_TRIGGER = "embed fail once" +# --------------------------------------------------------------------------- +# Phase 71 (tool-scaffolding guardrails, 2026-09-03 incident): +# deterministic raw-markup flows — see the module docstring +# --------------------------------------------------------------------------- + +#: A user message containing this substring (case-insensitive) drives +#: the scaffolding flow: request 1 streams ONLY the incident's raw tool +#: markup as ``delta.content``; the follow-up request carrying the +#: harness correction in the system prompt (the one bounded recovery) +#: streams the clean answer. Independent of the ```` marker — +#: both grounded and deflected turns hit it. Existing E2E questions do +#: not contain the phrase, so every other suite is unaffected. +SCAFFOLD_TRIGGER = "emit raw tool markup" + +#: A user message containing this substring (checked BEFORE +#: ``SCAFFOLD_TRIGGER`` — it contains that phrase) streams the +#: scaffolding-only span on EVERY request, recovery included — the +#: terminal malformed-reply path (the dedicated error frame, no done). +SCAFFOLD_ALWAYS_TRIGGER = "always emit raw tool markup" + +#: The incident span (2026-09-03): the model's chat-template tool +#: syntax, emitted as plain ``delta.content`` although no tools were +#: offered. Streamed through the mock's 12-char chunking, so it always +#: spans ≥2 wire chunks (the filter's boundary path). +SCAFFOLD_SPAN = ( + "<|tool_call_start|>[read(path='search_docs/reese-notes.md')]" + "<|tool_call_end|>" +) + +#: The clean answer the one bounded recovery produces (byte-stable — +#: the dedicated E2E suite asserts the recovered bubble and the wire's +#: delta text against it). +SCAFFOLD_RECOVERY_ANSWER = "Here is the plain-text answer the recovery produced." + +#: The stable substring of the harness-owned correction constant the +#: recovery request carries in its system prompt. Keyed on a substring +#: (not the whole constant) so a re-wrap of the constant cannot silently +#: re-route the mock; the module-level assert below fails loudly if the +#: substring ever leaves the constant (the mock must never drift from +#: ``app.rag.agent.CORRECTION_INSTRUCTION``). +_CORRECTION_MARKER = "no tool syntax" +assert _CORRECTION_MARKER in CORRECTION_INSTRUCTION, ( + "mock drift: the correction marker left CORRECTION_INSTRUCTION" +) + #: 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 @@ -535,6 +611,35 @@ def _search_flow(body: dict[str, Any]) -> tuple[str, ...] | None: return ("search",) +def _scaffold_flow(body: dict[str, Any]) -> str | None: + """Classify a phase-71 scaffolding request (see the module docstring). + + * ``"scaffold"`` — stream ONLY the incident span + (``SCAFFOLD_SPAN``) as ``delta.content`` chunks: ``finish_reason: + "stop"``, no structured ``tool_calls``, no reasoning. EVERY + request for ``SCAFFOLD_ALWAYS_TRIGGER`` (the recovery included), + and the FIRST request of ``SCAFFOLD_TRIGGER`` (no correction in + the system prompt yet). + * ``"recovery"`` — ``SCAFFOLD_TRIGGER`` whose system prompt carries + the harness correction (the one bounded recovery: ``tools=None``, + the constant folded into the single system prompt by + ``app.api.chat`` / ``app.rag.agent``): stream the clean + ``SCAFFOLD_RECOVERY_ANSWER``. + * ``None`` — not the scaffolding flow. The discrimination is + stateless, like the other marker flows: the trigger phrase in + the user message plus the correction's presence in the system + prompt. + """ + user = _user(body).lower() + if SCAFFOLD_ALWAYS_TRIGGER in user: # checked FIRST — it contains SCAFFOLD_TRIGGER + return "scaffold" + if SCAFFOLD_TRIGGER in user: + if _CORRECTION_MARKER in _system(body): + return "recovery" + return "scaffold" + return None + + def _tool_flow(body: dict[str, Any]) -> tuple[str, ...] | None: """Classify a marker request into one step of the tool flow. @@ -1061,6 +1166,29 @@ def chat_completions(body: dict[str, Any]) -> Any: if _chat_dead(RETRY_TRIGGER, RETRY_DEAD_ATTEMPTS): return _llm_500(RETRY_TRIGGER) _fail_posts[RETRY_TRIGGER] = 0 # the answer streamed — restart + # Phase 71 (tool-scaffolding guardrails): the deterministic raw- + # markup flow — checked BEFORE the search/tool marker flows (the + # trigger is independent of the ```` marker, so both + # grounded and deflected turns hit it; SCAFFOLD_ALWAYS_TRIGGER + # is checked first inside the classifier — the more specific + # phrase wins, same convention as THINK_PARAS_TRIGGER). + scaffold_flow = _scaffold_flow(body) + if scaffold_flow is not None: + # Request 1 (or EVERY request for the ALWAYS trigger): the + # incident span as plain delta.content, 12-char chunks + # (the span always spans ≥2 chunks — the filter's boundary + # path), finish_reason "stop", no tool_calls, no reasoning. + # Request 2 of the recovery trigger: the clean answer. + answer = ( + SCAFFOLD_RECOVERY_ANSWER + if scaffold_flow == "recovery" + else SCAFFOLD_SPAN + ) + return StreamingResponse( + _sse_stream(answer, 0.0), + media_type="text/event-stream", + headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, + ) # Phase 68 (search tool): the deterministic search marker flow — # checked BEFORE the phase-37 tool flow (the more specific # trigger phrase wins, same convention as THINK_PARAS_TRIGGER). diff --git a/tests/e2e/test_tool_scaffolding_guardrails.py b/tests/e2e/test_tool_scaffolding_guardrails.py new file mode 100644 index 0000000..80dc4e2 --- /dev/null +++ b/tests/e2e/test_tool_scaffolding_guardrails.py @@ -0,0 +1,438 @@ +"""Phase 71 E2E (Playwright, mock-only): tool-scaffolding guardrails — +the deterministic strip + one bounded recovery, through the real UI. + +Owner request (chat, 2026-09-03): the same incident as phase 70 — a +deflected round streamed the model's raw +``<|tool_call_start|>[read(path='…')]<|tool_call_end|>`` chat-template +markup into the UI although no tools were offered. The guardrail is +DETERMINISTIC ONLY (no model in detection or repair): a streaming +filter strips known scaffolding from ``delta.content`` server-side +(``app/rag/scaffolding.py``), and a reply whose visible content ends +up empty gets exactly ONE bounded recovery (``tools=None``, the +harness correction folded into the system prompt); a second empty +reply settles with the dedicated error frame. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_tool_scaffolding_guardrails.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the gate is the +deterministic scaffolding flows in ``tests/e2e/mock_llm.py`` (phase +71), which are independent of the ```` marker: + +* ``emit raw tool markup`` (``SCAFFOLD_TRIGGER``): request 1 streams + ONLY the incident span as ``delta.content`` (split across the + mock's 12-char chunks — the filter's boundary path), no structured + ``tool_calls``, no reasoning; the follow-up request carrying + ``CORRECTION_INSTRUCTION`` in the system prompt (the one bounded + recovery) streams the clean ``SCAFFOLD_RECOVERY_ANSWER``. +* ``always emit raw tool markup`` (``SCAFFOLD_ALWAYS_TRIGGER``): the + scaffolding-only span on EVERY request (the recovery included) — + the terminal malformed-reply path. + +Both triggers run on an EMPTY knowledge base: with zero chunks the +honesty gate is LOW (cosine 0.0 < the E2E 0.30 threshold, no FTS +hits), so every turn takes the DEFLECTED path — the exact path the +2026-09-03 incident hit — where the filter + recovery live in +``app/api/chat.py``. + +Test → phase mapping (Playwright Mapping Rule): +1. ``test_recovery_strips_scaffolding_and_streams_clean_answer`` — the + recovery case: the turn settles (the composer re-enables, ``done`` + on the wire), the final answer bubble carries the recovery's clean + text, ``document.body.innerText`` contains NEITHER + ``tool_call_start`` nor ``tool_call_end`` (nor the raw + ``[read(path=…`` fragment), and no error banner — and wire-level, + no ``delta`` frame ever carries a scaffolding fragment (the strip + happens server-side, not in the UI). +2. ``test_terminal_scaffolding_lands_on_dedicated_error_app_stays_usable`` + — the terminal case: the existing error state renders with the + dedicated copy ("The model returned a malformed reply — please try + again."), no raw tokens in the DOM, no answer bubble, no ``done`` + and NO ``query_log`` row (the existing terminal-error semantics) — + and the app stays usable: a follow-up plain question in the same + session gets a normal deflected answer and the banner clears. +3. ``test_plain_turn_never_recovers_and_streams_byte_clean`` — no + false positive: a plain deflected question streams its + first-request answer byte-clean (the concatenated delta text is + EXACTLY the mock's deterministic deflection answer — not the + recovery's), with no error state and no recovery request visible + (the turn settles on the first request). +""" +from __future__ import annotations + +import json +import re +import time + +from playwright.sync_api import Page, expect +from sqlalchemy import select, text + +from app.db import SessionLocal +from app.models import QueryLog +from tests.e2e.mock_llm import SCAFFOLD_RECOVERY_ANSWER, compose_answer + +# -------------------------------------------------------------------------- +# Questions + the mock's deterministic expectations +# -------------------------------------------------------------------------- + +#: Carries ``SCAFFOLD_TRIGGER`` (and nothing else — the module-level +#: asserts below pin the trigger exclusions). +RECOVERY_Q = "Please emit raw tool markup in your reply to this." + +#: Carries ``SCAFFOLD_ALWAYS_TRIGGER`` — the scaffolding-only span on +#: EVERY request, recovery included (the terminal path). +TERMINAL_Q = "Please always emit raw tool markup in every reply." + +#: A plain question (the phase-67 deflection shape): no trigger at all. +PLAIN_Q = "How do I bake sourdough bread?" + +for _q in (RECOVERY_Q, TERMINAL_Q, PLAIN_Q): + for _other in ( + "read two documents", + "search your documents", + "write a long answer", + "think in paragraphs", + "think out loud", + "show the end of your notes", + "show me a table", + "fail then answer", + "always fail", + "embed fail once", + "pretend to think slowly", + "use your tools", + ): + assert _other not in _q.lower(), f"{_other!r} unexpectedly in {_q!r}" +assert "emit raw tool markup" in RECOVERY_Q.lower() +assert "always emit raw tool markup" not in RECOVERY_Q.lower() +assert "always emit raw tool markup" in TERMINAL_Q.lower() +assert "emit raw tool markup" not in PLAIN_Q.lower() + +#: The terminal malformed-reply copy (``app.api.chat`` — the phase-71 +#: dedicated error frame; the mock's ALWAYS trigger is what makes the +#: recovery come back empty). +MALFORMED_ERROR_COPY = "The model returned a malformed reply — please try again." + +#: The mock's deterministic deflection phrase (the phase-67 pin). +DEFLECT_PHRASE = r"haven't done anything like that" + +#: The raw scaffolding fragments that must NEVER reach the user — the +#: span's tokens and the incident's argument fragment (``SCAFFOLD_SPAN`` +#: in the mock). +RAW_TOKENS = ("tool_call_start", "tool_call_end", "[read(path=", "<|") + + +def _expected_deflect_answer(question: str) -> str: + """The mock's deterministic DEFLECT_MODE answer for *question*. + + Derived from the mock itself (``compose_answer`` on a synthetic + body: the mode marker in the system prompt, the question as the + user message, no tuning/KB sections — the suite truncates + ``steering_notes`` / ``kb_overview``, so the real prompt carries + none), so the byte-clean pin can never drift from the mock. + """ + return compose_answer( + { + "messages": [ + {"role": "system", "content": "DEFLECT_MODE marker"}, + {"role": "user", "content": question}, + ] + } + ) + + +# -------------------------------------------------------------------------- +# DB reset (EMPTY knowledge base → every turn is deterministically +# deflected: the LOW gate, the incident's path) + query_log reads +# -------------------------------------------------------------------------- + + +def _reset_db_empty() -> None: + """Truncate the KB (plus the prompt-shaping tables): an EMPTY + knowledge base, so the honesty gate is LOW for every question + (no chunks → cosine 0.0 < 0.30, fts_hits 0) — the deflected path + where the phase-71 filter + recovery live, and the prompts stay + byte-stable regardless of leftovers from other suites.""" + with SessionLocal() as db: + db.execute( + text("TRUNCATE chunks, documents, query_log, steering_notes, kb_overview") + ) + db.commit() + + +def _query_log_rows() -> list[QueryLog]: + with SessionLocal() as db: + return list(db.scalars(select(QueryLog)).all()) + + +# -------------------------------------------------------------------------- +# Page hooks (the SSE capture — the house pattern from +# test_agent_document_tools.py / test_llm_retry.py) +# -------------------------------------------------------------------------- + +#: Captures the raw SSE ``data:`` payloads of the /api/chat stream +#: (a response clone read in the background) — wire-level assertions +#: 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, terminal: str = "done") -> list[dict]: + """One turn's SSE frames: wait for that turn's *terminal* frame + (``done`` — or ``error`` for the terminal case), 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: the terminal frame is + observed only in the batch that carries the turn's full frame + sequence.""" + deadline = time.monotonic() + 30.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") == terminal for f in parsed): + return parsed + if time.monotonic() > deadline: + raise AssertionError( + f"SSE hook captured no `{terminal}` frame (frames so far: " + f"{len(parsed)}) — hook install failed?" + ) + time.sleep(0.05) + + +def _delta_text(frames: list[dict]) -> str: + """The turn's answer text exactly as the wire carried it.""" + return "".join(f.get("text", "") for f in frames if f.get("type") == "delta") + + +def _assert_no_raw_tokens_on_wire(frames: list[dict]) -> None: + for frame in frames: + if frame.get("type") != "delta": + continue + text = frame.get("text", "") + for raw in RAW_TOKENS: + assert raw not in text, f"raw scaffolding {raw!r} on the wire: {text!r}" + + +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_raw_tokens(page: Page) -> None: + """The user-visible contract: no raw scaffolding fragment anywhere + in the rendered page (``document.body.innerText``).""" + body = page.locator("body").inner_text() + for raw in RAW_TOKENS: + assert raw not in body, f"raw scaffolding {raw!r} leaked into the DOM" + + +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 recovery case: the span is stripped server-side, the ONE +# bounded recovery answers, and no raw token ever reaches the DOM +# -------------------------------------------------------------------------- + + +def test_recovery_strips_scaffolding_and_streams_clean_answer( + page: Page, app_url: str, db_ready: None +) -> None: + page.set_default_timeout(30_000) + _reset_db_empty() + page.goto(app_url) + _install_sse_hook(page) + + _submit(page, RECOVERY_Q) + _wait_settled(page) + + # The final answer bubble carries the RECOVERY's clean text — the + # mock's deterministic recovery answer, proof the one bounded + # recovery ran (request 2, with the correction in the system + # prompt) and its answer is what the user saw. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(SCAFFOLD_RECOVERY_ANSWER) + # The deflected message state is intact — the turn took the LOW + # path, the incident's path. + expect(page.locator(".msg.brain").last).to_have_class(re.compile(r"is-deflected")) + _assert_no_raw_tokens(page) + _assert_no_error_banner(page) + + # Wire level: the strip happens SERVER-side — no `delta` frame ever + # carries a scaffolding fragment, and the concatenated delta text is + # EXACTLY the recovery answer (request 1's span produced zero delta + # frames). No tool frames, no retry frames, no error frame; the + # turn settled with `done` (deflected). + frames = _drain_frames(page, terminal="done") + assert [f for f in frames if f.get("type") == "delta"], ( + "the recovery answer must have streamed delta frames" + ) + _assert_no_raw_tokens_on_wire(frames) + assert _delta_text(frames) == SCAFFOLD_RECOVERY_ANSWER + assert not [f for f in frames if f.get("type") == "tool"] + assert not [f for f in frames if f.get("type") == "retry"] + assert not [f for f in frames if f.get("type") == "error"] + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is True + + # The recovered turn settles durably: exactly one query_log row + # (deflected — the incident's path). + rows = _query_log_rows() + assert len(rows) == 1, rows + assert rows[0].question == RECOVERY_Q + assert rows[0].deflected is True + + +# -------------------------------------------------------------------------- +# 2. The terminal case: scaffolding twice → the dedicated error frame, +# no done, no query_log row — and the app stays usable +# -------------------------------------------------------------------------- + + +def test_terminal_scaffolding_lands_on_dedicated_error_app_stays_usable( + page: Page, app_url: str, db_ready: None +) -> None: + page.set_default_timeout(30_000) + _reset_db_empty() + page.goto(app_url) + _install_sse_hook(page) + + _submit(page, TERMINAL_Q) + + # BOTH requests (original + the one bounded recovery) came back + # scaffolding-only: no clean content ever streamed, so the turn + # settles with the EXISTING terminal error state — the banner + # (role=alert) with the DEDICATED malformed-reply copy — and the + # send button re-enabled (the banner path settles the state + # machine, cf. the phase-67 exhaustion test). + expect(page.locator("#kb-banner")).to_have_attribute( + "role", "alert", timeout=60_000 + ) + expect(page.locator("#kb-banner")).to_contain_text(MALFORMED_ERROR_COPY) + expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) + expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) + + # No answer bubble was ever rendered (every streamed frame was + # stripped server-side) and no raw token is anywhere in the DOM. + expect(page.locator("#messages > .msg.brain")).to_have_count(0) + _assert_no_raw_tokens(page) + + # Wire: the terminal error frame is LAST — no done, and NO delta + # frame at all (both requests' content was pure scaffolding). + frames = _drain_frames(page, terminal="error") + assert frames[-1]["type"] == "error" + assert MALFORMED_ERROR_COPY in frames[-1]["detail"] + assert not [f for f in frames if f.get("type") == "done"] + assert not [f for f in frames if f.get("type") == "delta"] + + # Terminal semantics (byte-for-byte the existing LLMError shape): + # the turn writes no query_log row. + assert _query_log_rows() == [] + + # The app stays usable: a follow-up plain question (no trigger) in + # the SAME session gets a normal streamed deflected answer, the + # banner clears, and the wire is clean. + _submit(page, PLAIN_Q) + _wait_settled(page) + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(re.compile(DEFLECT_PHRASE, re.IGNORECASE)) + _assert_no_error_banner(page) + _assert_no_raw_tokens(page) + follow_frames = _drain_frames(page, terminal="done") + assert next(f for f in follow_frames if f["type"] == "done")["deflected"] is True + assert not [f for f in follow_frames if f.get("type") == "error"] + _assert_no_raw_tokens_on_wire(follow_frames) + + +# -------------------------------------------------------------------------- +# 3. No false positive: a plain turn streams its first-request answer +# byte-clean — no strip, no recovery request, no error state +# -------------------------------------------------------------------------- + + +def test_plain_turn_never_recovers_and_streams_byte_clean( + page: Page, app_url: str, db_ready: None +) -> None: + page.set_default_timeout(30_000) + _reset_db_empty() + page.goto(app_url) + _install_sse_hook(page) + + _submit(page, PLAIN_Q) + _wait_settled(page) + + # Wire level: the plain deflected answer streams byte-clean from + # the FIRST request — the concatenated delta text is EXACTLY the + # mock's deterministic deflection answer (derived from the mock + # itself above) and NOT the recovery answer (a recovery request + # would have streamed that text instead). + frames = _drain_frames(page, terminal="done") + delta_text = _delta_text(frames) + assert delta_text == _expected_deflect_answer(PLAIN_Q) + assert SCAFFOLD_RECOVERY_ANSWER not in delta_text + _assert_no_raw_tokens_on_wire(frames) + # No recovery request is visible: no error frame, no retry frame + # (the recovery is not a phase-67 retry), and the turn settled on + # the first request with `done`. + assert not [f for f in frames if f.get("type") == "error"] + assert not [f for f in frames if f.get("type") == "retry"] + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is True + + # UI level: the deflected answer rendered, no error banner, no raw + # token in the DOM. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(re.compile(DEFLECT_PHRASE, re.IGNORECASE)) + _assert_no_error_banner(page) + _assert_no_raw_tokens(page) diff --git a/tests/integration/test_agent_tools.py b/tests/integration/test_agent_tools.py index 02a6734..f3cafa4 100644 --- a/tests/integration/test_agent_tools.py +++ b/tests/integration/test_agent_tools.py @@ -23,7 +23,7 @@ import asyncio import uuid from collections.abc import AsyncIterator, Iterator from copy import deepcopy -from typing import Any, cast +from typing import TYPE_CHECKING, Any, cast import pytest from sqlalchemy import delete, text @@ -35,6 +35,9 @@ from app.rag import agent from app.rag.agent import AGENT_TOOLS, AgentHolder, run_agent from app.rag.llm import LLMClient, RetryPiece, StreamPiece, ToolCallPiece +if TYPE_CHECKING: + from app.rag.scaffolding import ScaffoldingFilter + def _doc(db: Session, source: str, path: str, title: str, content: str) -> Document: doc = Document( @@ -168,6 +171,7 @@ class ScriptedToolLLM: self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, # phase 71 pass-through ) -> AsyncIterator[StreamPiece | ToolCallPiece]: self.requests.append((deepcopy(messages), deepcopy(tools))) if len(self.requests) == 1: diff --git a/tests/integration/test_chat_api.py b/tests/integration/test_chat_api.py index acf4139..f57b179 100644 --- a/tests/integration/test_chat_api.py +++ b/tests/integration/test_chat_api.py @@ -18,7 +18,7 @@ import math import re from collections.abc import Iterator from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, cast import pytest from fastapi.testclient import TestClient @@ -33,6 +33,9 @@ from app.rag.agent import AGENT_TOOLS from app.rag.importer import import_sources from app.rag.llm import EmbeddingError, LLMError, StreamPiece, ToolCallPiece +if TYPE_CHECKING: + from app.rag.scaffolding import ScaffoldingFilter + FIXTURES = Path(__file__).resolve().parents[1] / "fixtures" / "docs" QUESTION = "How is my Kubernetes cluster set up?" OFF_TOPIC = "How do I bake sourdough bread?" @@ -63,6 +66,7 @@ class FakeRagLLM: tool_script: list[list[StreamPiece | ToolCallPiece]] | None = None, embed_fail_count: int = 0, stream_fail_count: int = 0, + answer_sequence: list[str] | None = None, ) -> None: self.settings = Settings(_env_file=None) # pyright: ignore[reportCallIssue] self.embed_batches = 0 @@ -71,6 +75,11 @@ class FakeRagLLM: self.embed_error = embed_error self.stream_error = stream_error self.fail_mid_stream = fail_mid_stream + #: Phase 71: per-request canned answers (the recovery matrix): + #: request *i* (0-based, in ``seen_messages`` order) yields + #: ``answer_sequence[i]``; once exhausted it falls back to + #: ``answer``. ``None`` keeps the single-``answer`` behavior. + self.answer_sequence = answer_sequence #: Phase 67: the first N ``embed_one`` calls raise an #: ``EmbeddingError`` (then succeed) — a dead-then-recovered #: embeddings endpoint for the retry loop. @@ -117,17 +126,35 @@ class FakeRagLLM: self.question_embeds.append(text) return _token_vec(text) + def _answer_for_request(self) -> str: + """The canned answer for the request that was just recorded + (phase 71 ``answer_sequence``; ``None`` → the single answer).""" + if self.answer_sequence is None: + return self.answer + index = len(self.seen_messages) - 1 + if index < len(self.answer_sequence): + return self.answer_sequence[index] + return self.answer + async def chat_stream( self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, ): """Typed stream (phase 17): ``thinking`` slices (same 12-char cadence as content) **before** the content pieces. With the default ``thinking=""`` this yields content-only pieces — today's behavior, new yield type. Phase 37: *tools* is the agent loop's ``tools=…`` passthrough (recorded in ``seen_tools``); a request - with tools consumes the next ``tool_script`` entry, if any.""" + with tools consumes the next ``tool_script`` entry, if any. + Phase 71: *scaffolding* mirrors ``LLMClient.chat_stream`` — the + canned content pieces are fed through the caller's filter (an + empty clean result yields nothing) and the held tail is flushed + on normal completion, so a scaffolding-only canned answer streams + zero content pieces and leaves ``stripped_chars`` behind for the + recovery policy to key on. ``None`` (e.g. pre-phase callers) keeps + the byte-identical raw path.""" self.seen_messages.append(messages) self.seen_tools.append(tools) if self.stream_error is not None: @@ -135,17 +162,46 @@ class FakeRagLLM: if self.stream_fail_count > 0: self.stream_fail_count -= 1 raise LLMError("simulated pre-piece endpoint failure") + mid_stream_drop = False + raw: list[StreamPiece | ToolCallPiece] if tools is not None and self.tool_script: - for piece in self.tool_script.pop(0): + raw = self.tool_script.pop(0) + elif self.fail_mid_stream: + raw = [StreamPiece("content", "partial ")] + mid_stream_drop = True + else: + answer = self._answer_for_request() + raw = cast( + "list[StreamPiece | ToolCallPiece]", + [ + StreamPiece("thinking", self.thinking[i : i + 12]) + for i in range(0, len(self.thinking), 12) + ] + + [ + StreamPiece("content", answer[i : i + 12]) + for i in range(0, len(answer), 12) + ], + ) + if scaffolding is None: + for piece in raw: yield piece + if mid_stream_drop: + raise LLMError("mid-stream dropout") return - if self.fail_mid_stream: - yield StreamPiece("content", "partial ") + for piece in raw: + if isinstance(piece, StreamPiece) and piece.kind == "content": + cleaned = scaffolding.feed(piece.text) + if cleaned: + yield StreamPiece("content", cleaned) + else: + yield piece + if mid_stream_drop: + # The tail is NOT flushed on a failed stream — the real + # client only flushes a cleanly completed one. raise LLMError("mid-stream dropout") - for i in range(0, len(self.thinking), 12): - yield StreamPiece("thinking", self.thinking[i : i + 12]) - for i in range(0, len(self.answer), 12): - yield StreamPiece("content", self.answer[i : i + 12]) + tail = scaffolding.flush() + if tail: + yield StreamPiece("content", tail) @pytest.fixture() @@ -621,6 +677,7 @@ def test_grounded_turn_streams_tool_frames_and_cites_read_doc( assert lines and "tool_calls=2" in lines[-1] assert "'docs/homelab/kubernetes.md'" in lines[-1] assert "'docs/homelab/backups.md'" in lines[-1] + assert "scaffold_stripped=0" in lines[-1] # phase 71: uniform clean-turn field def test_grounded_turn_streams_grep_tool_frames( @@ -853,6 +910,7 @@ def test_zero_max_rounds_reproduce_pre_phase_single_request( assert "backups.md" not in row.sources lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] assert lines and "tool_calls=0" in lines[-1] + assert "scaffold_stripped=0" in lines[-1] # phase 71: uniform clean-turn field def test_tool_execution_db_failure_yields_error_event( @@ -929,6 +987,7 @@ def test_embed_failure_retries_then_turn_completes( assert frames[-1]["type"] == "done" lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] assert lines and "retries=1" in lines[-1] + assert "scaffold_stripped=0" in lines[-1] # phase 71: uniform clean-turn field def test_embed_failure_exhausts_retries_then_terminal_error( @@ -988,6 +1047,7 @@ def test_deflected_stream_retries_before_the_first_piece( assert flaky.seen_tools == [None, None] # …byte-identical (no tools key) lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] assert lines and "retries=1" in lines[-1] + assert "scaffold_stripped=0" in lines[-1] # phase 71: uniform clean-turn field def test_deflected_stream_failure_after_first_frame_is_terminal( @@ -1034,3 +1094,135 @@ def test_zero_retries_keep_the_pre_phase_wire_shape( assert frames[0]["type"] == "error" assert "embedding" in frames[0]["detail"] assert not any(f["type"] == "retry" for f in frames) + + +# ---------- phase 71: the deterministic scaffolding guardrail (deflected path) ---------- + + +def _scaffold_span() -> str: + """The raw span from the 2026-09-03 incident (the E2E mock's trigger, + task 05) — a complete span the filter strips in full.""" + return "<|tool_call_start|>[read(path='/homelab/backup-notes.md')]<|tool_call_end|>" + + +def test_deflected_scaffolding_only_reply_recovers_once( + client, + db, + seeded_kb: FakeRagLLM, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """(a) A deflected reply that is pure scaffolding streams ZERO delta + frames (no raw tokens on the wire); the one bounded recovery — + ``tools=None``, the correction folded into the single system prompt, + a fresh filter, the same retry budget — streams the clean answer, the + turn settles with ``done`` + a query_log row, and the log line counts + the stripped chars (the recovery does not bump ``retries=N``).""" + span = _scaffold_span() + clean = "I don't have that on hand — try one of the chips below?" + flaky = FakeRagLLM(answer_sequence=[span, clean]) + live = get_settings() + monkeypatch.setattr(chat_api, "get_settings", lambda: _retry_settings(live)) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: flaky + try: + caplog.set_level(logging.INFO, logger="app.chat") + _, _, frames = _stream_chat(client, OFF_TOPIC) + finally: + fastapi_app.dependency_overrides.clear() + + # The raw tokens never reach the wire; the deltas reassemble to the + # clean recovery answer. + assert span not in json.dumps(frames) + deltas = [f for f in frames if f["type"] == "delta"] + assert "".join(d["text"] for d in deltas) == clean + assert not any(f["type"] == "error" for f in frames) + done = frames[-1] + assert done["type"] == "done" and done["deflected"] is True + + # Exactly two requests: the stripped round + the one recovery, both + # without a tools key… + assert len(flaky.seen_messages) == 2 + assert flaky.seen_tools == [None, None] + # …and the recovery's system prompt is the ORIGINAL deflected prompt + # with the correction folded in (a single system message — the user + # message stays last). + recovered = flaky.seen_messages[1] + assert len(recovered) == 2 + assert recovered[1] == {"role": "user", "content": OFF_TOPIC} + first_system = flaky.seen_messages[0][0]["content"] + assert "DEFLECT_MODE" in first_system + assert recovered[0] == { + "role": "system", + "content": first_system + "\n" + agent.CORRECTION_INSTRUCTION, + } + + # The turn settled normally: one query_log row… + (row,) = db.scalars(select(QueryLog)).all() + assert row.deflected is True + # …and the log line carries the summed stripped count (the clean + # recovery stripped nothing) with retries untouched. + lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] + assert lines and f"scaffold_stripped={len(span)}" in lines[-1] + assert "retries=0" in lines[-1] # the recovery is not an endpoint-retry + + +def test_deflected_scaffolding_twice_settles_malformed( + client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch +) -> None: + """(b) The recovery answer is scaffolding again — a second empty + reply is terminal: the DEDICATED error frame (the exact copy), no + ``done``, no query_log row — byte-for-byte today's ``LLMError`` + terminal shape — and no third request (at most one recovery per + turn).""" + span = _scaffold_span() + dead = FakeRagLLM(answer_sequence=[span, span]) + live = get_settings() + monkeypatch.setattr(chat_api, "get_settings", lambda: _retry_settings(live)) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: dead + try: + _, _, frames = _stream_chat(client, OFF_TOPIC) + finally: + fastapi_app.dependency_overrides.clear() + + assert [f["type"] for f in frames] == ["error"] + assert frames[0]["detail"] == ( + "The model returned a malformed reply — please try again." + ) + assert set(frames[0].keys()) == {"type", "detail"} # the contract shape + assert span not in json.dumps(frames) + assert not any(f["type"] == "done" for f in frames) + assert db.scalars(select(QueryLog)).all() == [] + assert len(dead.seen_messages) == 2 # round + one recovery — no more + assert dead.seen_tools == [None, None] + + +def test_deflected_mixed_scaffolding_and_content_needs_no_recovery( + client, + db, + seeded_kb: FakeRagLLM, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """(c) Real visible content plus scaffolding: the clean remainder + streams (no raw tokens on the wire), NO recovery runs, and the log + line counts the stripped span (``scaffold_stripped>0``).""" + span = _scaffold_span() + mixed = f"I don't have that. {span} Try the chips below?" + flaky = FakeRagLLM(answer=mixed) + live = get_settings() + monkeypatch.setattr(chat_api, "get_settings", lambda: _retry_settings(live)) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: flaky + try: + caplog.set_level(logging.INFO, logger="app.chat") + _, _, frames = _stream_chat(client, OFF_TOPIC) + finally: + fastapi_app.dependency_overrides.clear() + + assert span not in json.dumps(frames) + deltas = [f for f in frames if f["type"] == "delta"] + assert "".join(d["text"] for d in deltas) == "I don't have that. Try the chips below?" + assert not any(f["type"] == "error" for f in frames) + assert frames[-1]["type"] == "done" + assert len(flaky.seen_messages) == 1 # the clean content stands — no recovery + lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] + assert lines and f"scaffold_stripped={len(span)}" in lines[-1] diff --git a/tests/unit/test_agent.py b/tests/unit/test_agent.py index c4850fd..b8afa5e 100644 --- a/tests/unit/test_agent.py +++ b/tests/unit/test_agent.py @@ -32,7 +32,7 @@ import logging import uuid from collections.abc import AsyncGenerator, AsyncIterator from copy import deepcopy -from typing import Any, cast +from typing import TYPE_CHECKING, Any, cast import pytest from sqlalchemy.orm import Session @@ -43,11 +43,15 @@ from app.rag import agent from app.rag.agent import ( AGENT_TOOLS, AgentHolder, + MalformedReplyError, run_agent, ) from app.rag.llm import LLMClient, LLMError, RetryPiece, StreamPiece, ToolCallPiece from app.rag.prompts import TOOLS_SECTION, _base, build_deflect_prompt, build_high_prompt +if TYPE_CHECKING: + from app.rag.scaffolding import ScaffoldingFilter + def _settings(**kwargs: Any) -> Settings: kwargs.setdefault("_env_file", None) @@ -68,7 +72,13 @@ def _doc(source: str, path: str, title: str = "Title", content: str = "CONTENT") class ScriptedLLM: """Canned stream sequences; records every ``chat_stream`` request so - the tests can assert on the messages and the ``tools`` passthrough.""" + the tests can assert on the messages and the ``tools`` passthrough. + Phase 71: when the caller passes a ``ScaffoldingFilter``, the canned + content pieces are fed through it exactly like + ``LLMClient.chat_stream`` (an empty clean result yields nothing; the + held tail is flushed on normal completion) — so a scaffolding-only + canned round streams no content pieces and leaves ``stripped_chars`` + behind for the recovery policy to key on.""" def __init__(self, *streams: list[StreamPiece | ToolCallPiece]) -> None: self.streams: list[list[StreamPiece | ToolCallPiece]] = list(streams) @@ -78,12 +88,26 @@ class ScriptedLLM: 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), tools)) if not self.streams: raise AssertionError("ScriptedLLM ran out of canned streams") - for piece in self.streams.pop(0): - yield piece + pieces = self.streams.pop(0) + if scaffolding is None: + for piece in pieces: + yield piece + return + for piece in pieces: + if isinstance(piece, StreamPiece) and piece.kind == "content": + cleaned = scaffolding.feed(piece.text) + if cleaned: + yield StreamPiece("content", cleaned) + else: + yield piece + tail = scaffolding.flush() + if tail: + yield StreamPiece("content", tail) async def _run( @@ -1102,6 +1126,7 @@ class FailingLLM: self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, # phase 71: fed like the real client ) -> AsyncIterator[StreamPiece | ToolCallPiece]: index = len(self.requests) pieces, error = ( @@ -1112,19 +1137,35 @@ class FailingLLM: self.requests.append( (deepcopy(messages), deepcopy(tools) if tools is not None else None) ) - return self._attempt(index, pieces, error) + return self._attempt(index, pieces, error, scaffolding) async def _attempt( self, index: int, pieces: list[StreamPiece | ToolCallPiece], error: Exception | None, + scaffolding: ScaffoldingFilter | None, ) -> AsyncIterator[StreamPiece | ToolCallPiece]: try: for piece in pieces: - yield piece + if ( + scaffolding is not None + and isinstance(piece, StreamPiece) + and piece.kind == "content" + ): + cleaned = scaffolding.feed(piece.text) + if cleaned: + yield StreamPiece("content", cleaned) + else: + yield piece if error is not None: + # The tail is NOT flushed on a failed attempt — the real + # client only flushes a cleanly completed stream. raise error + if scaffolding is not None: + tail = scaffolding.flush() + if tail: + yield StreamPiece("content", tail) finally: self.closed.append(index) @@ -1402,11 +1443,16 @@ def test_high_prompt_tools_section_with_notes_and_kb() -> None: def test_low_prompt_is_byte_identical_and_tool_free() -> None: + # Phase 71: the LOW prompt carries the owner-permitted plain-text + # line after the DEFLECT_MODE sentence (the marker-keying contract + # is unchanged; the line must not leak into the HIGH prompt — + # pinned in tests/unit/test_prompts.py). expected = ( _base("LOW") + "\nDEFLECT_MODE: retrieval was weak — the titles below are the closest " "your notes come to the question. They are titles only; do not pretend " "they answer it. Use them to propose 2-3 alternative questions.\n" + "Reply in plain text only — you have no tools in this mode.\n" + "- T1\n- T2" ) assert build_deflect_prompt(["T1", "T2"]) == expected @@ -1418,3 +1464,253 @@ def test_low_prompt_is_byte_identical_and_tool_free() -> None: ): assert "" not in prompt assert TOOLS_SECTION not in prompt + + +# ---------- phase 71: the scaffolding recovery policy (deterministic only) ---------- + +#: The raw span from the 2026-09-03 incident (the E2E mock's trigger, +#: task 05) — a complete span the filter strips in full. +_INCIDENT_SPAN = ( + "<|tool_call_start|>[read(path='/homelab/backup-notes.md')]<|tool_call_end|>" +) + + +def test_correction_instruction_is_the_harness_constant() -> None: + """Verbatim constant: the E2E mock (task 05) keys on a stable + substring of it, so it must not drift.""" + assert agent.CORRECTION_INSTRUCTION == ( + "Your previous reply contained raw tool-call markup, which is not " + "interpreted here. Answer the user's question directly in plain " + "text — no tool syntax." + ) + + +def test_malformed_reply_error_subclasses_llm_error() -> None: + assert issubclass(MalformedReplyError, LLMError) + assert not issubclass(LLMError, MalformedReplyError) + + +def test_scaffolding_only_round_gets_exactly_one_recovery( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A round whose visible content is pure scaffolding → exactly TWO + model requests: the normal round, then the ONE recovery — + ``tools=None`` with :data:`CORRECTION_INSTRUCTION` folded into the + original single system message (the user message stays last). The + clean recovery answer ends the turn, the holder is untouched by the + recovery, and the strip was captured in the warning log.""" + holder = AgentHolder() + llm = ScriptedLLM( + [StreamPiece("content", _INCIDENT_SPAN)], + [StreamPiece("content", "The clean recovery answer.")], + ) + with caplog.at_level(logging.WARNING, logger="app.agent"): + pieces = asyncio.run(_run(llm, holder, _settings())) + # Nothing of the round's scaffolding was yielded — only the recovery. + assert pieces == [StreamPiece("content", "The clean recovery answer.")] + assert len(llm.requests) == 2 # the round + the one recovery + # The round: the original system prompt, the tools offered. + assert llm.requests[0][1] == AGENT_TOOLS + assert llm.requests[0][0] == [ + {"role": "system", "content": "SYSTEM_PROMPT"}, + {"role": "user", "content": "QUESTION"}, + ] + # The recovery: no tools, a SINGLE system message carrying the folded + # correction, the user message last. + assert llm.requests[1][1] is None + assert llm.requests[1][0] == [ + { + "role": "system", + "content": "SYSTEM_PROMPT\n" + agent.CORRECTION_INSTRUCTION, + }, + {"role": "user", "content": "QUESTION"}, + ] + # The recovery is a fixed policy, not a conversation: the holder is + # untouched by it. + assert holder.read_docs == [] and holder.tool_calls == 0 + # The turn total feeds the API layer's ``scaffold_stripped=N`` field. + assert holder.scaffold_stripped == len(_INCIDENT_SPAN) + # One strip warning per stripped span, the span truncated to 200 chars. + strip_logs = [ + r + for r in caplog.records + if r.levelno == logging.WARNING and r.getMessage().startswith("agent: stripped") + ] + assert len(strip_logs) == 1 + message = strip_logs[0].getMessage() + assert message.startswith( + f"agent: stripped {len(_INCIDENT_SPAN)} chars of tool-scaffolding in round 1:" + ) + assert _INCIDENT_SPAN[:200] in message + + +def test_scaffolding_twice_settles_with_malformed_reply( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The recovery answer is scaffolding again (a second empty reply) — + terminal: :class:`MalformedReplyError` (an :class:`LLMError` subclass) + after exactly two requests — no third request, no recovery of a + recovery.""" + holder = AgentHolder() + llm = ScriptedLLM( + [StreamPiece("content", _INCIDENT_SPAN)], + [StreamPiece("content", _INCIDENT_SPAN)], + ) + with pytest.raises(MalformedReplyError) as excinfo: + asyncio.run(_run(llm, holder, _settings())) + assert isinstance(excinfo.value, LLMError) + assert len(llm.requests) == 2 # exactly one recovery per turn + assert llm.requests[1][1] is None + assert agent.CORRECTION_INSTRUCTION in llm.requests[1][0][0]["content"] + assert holder.scaffold_stripped == 2 * len(_INCIDENT_SPAN) + + +def test_scaffolding_with_real_content_needs_no_recovery( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A round with real visible content PLUS scaffolding: the clean + content stands — one request only, the clean remainder yielded (no + raw tokens), no correction in any system prompt.""" + holder = AgentHolder() + llm = ScriptedLLM( + [ + StreamPiece("content", "Here it is: "), + StreamPiece("content", _INCIDENT_SPAN), + StreamPiece("content", " hope that helps."), + ], + ) + pieces = asyncio.run(_run(llm, holder, _settings())) + assert [p for p in pieces if isinstance(p, StreamPiece)] == [ + StreamPiece("content", "Here it is: "), + StreamPiece("content", " hope that helps."), + ] + assert len(llm.requests) == 1 # no recovery + for messages, _tools in llm.requests: + assert all(agent.CORRECTION_INSTRUCTION not in m["content"] for m in messages) + assert holder.scaffold_stripped == len(_INCIDENT_SPAN) + + +def test_clean_turn_carries_no_correction_and_no_strip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A clean turn: one request, no correction in any system prompt, + zero stripped (the log field stays 0 — uniform).""" + holder = AgentHolder() + llm = ScriptedLLM( + [StreamPiece("thinking", "hmm "), StreamPiece("content", "a clean answer")] + ) + pieces = asyncio.run(_run(llm, holder, _settings())) + assert pieces == [ + StreamPiece("thinking", "hmm "), + StreamPiece("content", "a clean answer"), + ] + assert len(llm.requests) == 1 + for messages, _tools in llm.requests: + assert all(agent.CORRECTION_INSTRUCTION not in m["content"] for m in messages) + assert holder.scaffold_stripped == 0 + + +def test_empty_round_without_a_strip_keeps_today_behavior( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Round content 0 with NOTHING stripped (an empty/thinking-only + answer) → return as today — no recovery, no error.""" + holder = AgentHolder() + llm = ScriptedLLM([StreamPiece("thinking", "nothing to say, honestly")]) + pieces = asyncio.run(_run(llm, holder, _settings())) + assert pieces == [StreamPiece("thinking", "nothing to say, honestly")] + assert len(llm.requests) == 1 + assert holder.scaffold_stripped == 0 + + +def test_scaffolding_round_with_tool_calls_needs_no_recovery( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A scaffolding-only round that ALSO carried tool calls: the tool + ran, and the policy keys on the no-calls exit only — no recovery (the + next round is a normal tools-offered round carrying the tool + history).""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = ScriptedLLM( + [ + StreamPiece("content", _INCIDENT_SPAN), + ToolCallPiece(id="call_1", name="ls", arguments={}), + ], + [StreamPiece("content", "the answer after the tool")], + ) + pieces = asyncio.run(_run(llm, holder, _settings())) + # The round's scaffolding was stripped (no raw delta), the tool frame + # and the next round's answer flowed on. + assert pieces == [ + ToolCallPiece(id="call_1", name="ls", arguments={}), + StreamPiece("content", "the answer after the tool"), + ] + assert len(llm.requests) == 2 + assert llm.requests[1][1] == AGENT_TOOLS # a normal round, not a recovery + for messages, _tools in llm.requests: + assert all( + m["content"] is None or agent.CORRECTION_INSTRUCTION not in m["content"] + for m in messages + ) + assert holder.tool_calls == 1 + assert holder.scaffold_stripped == len(_INCIDENT_SPAN) + + +def test_recovery_after_tool_rounds_keeps_the_history( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A scaffolding-only answer round after a tool round: the recovery + keeps the SINGLE (folded) system message at the front and the tool + history intact behind it — no second system message, no duplicated + correction.""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = ScriptedLLM( + [ToolCallPiece(id="call_1", name="ls", arguments={})], + [StreamPiece("content", _INCIDENT_SPAN)], + [StreamPiece("content", "recovered after a tool round")], + ) + pieces = asyncio.run(_run(llm, holder, _settings())) + assert pieces == [ + ToolCallPiece(id="call_1", name="ls", arguments={}), + StreamPiece("content", "recovered after a tool round"), + ] + assert len(llm.requests) == 3 # tool round + stripped round + recovery + assert llm.requests[2][1] is None + recovered = llm.requests[2][0] + assert recovered[0] == { + "role": "system", + "content": "SYSTEM_PROMPT\n" + agent.CORRECTION_INSTRUCTION, + } + assert recovered[1] == {"role": "user", "content": "QUESTION"} + assert len(recovered) == 4 + assert recovered[2]["role"] == "assistant" + assert recovered[3] == { + "role": "tool", + "tool_call_id": "call_1", + "content": "1 documents:\nsource: S | path: a.md | title: A", + } + assert sum(1 for m in recovered if m["role"] == "system") == 1 + assert holder.scaffold_stripped == len(_INCIDENT_SPAN) + + +def test_forced_final_scaffolding_only_settles_malformed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The round-cap forced final (``tools=None``) is filtered too: a + scaffolding-only forced answer never reaches the user raw — the turn + settles with :class:`MalformedReplyError` (the same terminal + semantics; this turn used no recovery, so nothing is doubled up).""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = ScriptedLLM( + [ToolCallPiece(id="call_1", name="ls", arguments={})], + [StreamPiece("content", _INCIDENT_SPAN)], + ) + with pytest.raises(MalformedReplyError): + asyncio.run(_run(llm, holder, _settings(agent_max_rounds=1))) + assert len(llm.requests) == 2 + assert llm.requests[1][1] is None # the forced final — no recovery after it + assert holder.scaffold_stripped == len(_INCIDENT_SPAN) diff --git a/tests/unit/test_chat_gate.py b/tests/unit/test_chat_gate.py index 85040bf..f8a64a6 100644 --- a/tests/unit/test_chat_gate.py +++ b/tests/unit/test_chat_gate.py @@ -11,7 +11,7 @@ from __future__ import annotations import json import uuid from collections.abc import Iterator -from typing import Any +from typing import TYPE_CHECKING, Any import pytest from fastapi.testclient import TestClient @@ -25,6 +25,9 @@ from app.rag.llm import StreamPiece from app.rag.retriever import RetrievedChunk, weak_hit_titles from app.rag.suggestions import MAX_SUGGESTIONS, derive_suggestions +if TYPE_CHECKING: + from app.rag.scaffolding import ScaffoldingFilter + ANSWER = "I haven't done anything like that — try one of these instead!" #: A small KB outline standing in for the lite-generated one (phase 31). @@ -332,6 +335,11 @@ def test_low_prompt_has_titles_only_no_content() -> None: assert "LOW" in prompt assert "DEFLECT_MODE" in prompt assert "HONESTY GATE" in prompt # the LOW rule is what the model follows + # Phase 71 (owner-permitted 2026-09-03): the deflection plain-text + # line — the LOW turn offers no tools, so any tool markup there is + # always wrong (prevention at the prompt; the filter + recovery is + # the backstop). + assert "Reply in plain text only — you have no tools in this mode." in prompt assert "- Kubernetes Homelab Cluster" in prompt assert "- Backup Strategy" in prompt assert "ALPHA_DOC_CONTENT" not in prompt @@ -347,6 +355,8 @@ def test_high_path_unaffected() -> None: assert plan.suggestions == [] assert "HIGH" in plan.system_prompt assert "DEFLECT_MODE" not in plan.system_prompt + # Phase 71: the deflection plain-text line never leaks into HIGH. + assert "Reply in plain text only" not in plan.system_prompt assert "ALPHA_DOC_CONTENT" in plan.system_prompt assert "BETA_DOC_CONTENT" in plan.system_prompt assert [d.title for d in plan.docs] == ["Kubernetes Homelab Cluster", "Backup Strategy"] @@ -438,6 +448,7 @@ class _CannedLLM: self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, # phase 71 pass-through ): self.seen.append(messages) self.seen_tools.append(tools) diff --git a/tests/unit/test_llm_client.py b/tests/unit/test_llm_client.py index bf461ae..c6378a2 100644 --- a/tests/unit/test_llm_client.py +++ b/tests/unit/test_llm_client.py @@ -27,6 +27,7 @@ from app.rag.llm import ( ToolCallPiece, chat_stream_retried, ) +from app.rag.scaffolding import ScaffoldingFilter def _settings(**kwargs: Any) -> Settings: @@ -337,13 +338,19 @@ class _FakeCompletions: self.completion = completion self.kwargs: dict | None = None self.chat_kwargs: dict | None = None + #: Every SDK-shaped stream handed out — teardown tests assert the + #: phase-48 ``close()`` on them (phase 71 task 02: with/without + #: a filter, the teardown path is the same object). + self.streams: list[_FakeChatStream] = [] async def create(self, **kwargs) -> _FakeChatStream | _FakeCompletion: self.kwargs = kwargs if self.fail is not None: raise self.fail if kwargs.get("stream"): - return _FakeChatStream(self.chunks) + stream = _FakeChatStream(self.chunks) + self.streams.append(stream) + return stream self.chat_kwargs = kwargs assert self.completion is not None return self.completion @@ -721,6 +728,146 @@ def test_chat_stream_non_object_tool_arguments_raise_llm_error() -> None: asyncio.run(drain()) +# ---------- scaffolding filter integration (phase 71, task 02) ---------- + +#: The incident's raw span (2026-09-03) — the filter's reason to exist. +_INCIDENT_SPAN = "<|tool_call_start|>[read(path='/homelab/backup-notes.md')]<|tool_call_end|>" + + +def _collect_filtered( + llm: LLMClient, + messages: list[dict[str, str]], + scaffolding: ScaffoldingFilter | None, +) -> list[StreamPiece]: + """Collect pieces from a tools-less filtered stream (phase 71): without + tools, no ToolCallPiece can appear (the phase-37 contract).""" + async def run() -> list[StreamPiece]: + pieces = [p async for p in llm.chat_stream(messages, scaffolding=scaffolding)] + assert all(isinstance(p, StreamPiece) for p in pieces) + return cast("list[StreamPiece]", pieces) + + return asyncio.run(run()) + + +def test_chat_stream_with_filter_strips_span_mid_stream() -> None: + """A span mid-stream never reaches the pieces: the clean remainder + flows, ``stripped_chars`` is exact, and no piece carries scaffolding.""" + f = ScaffoldingFilter() + llm, _ = _make_stream_client( + [_chunk("Hello "), _chunk(_INCIDENT_SPAN + " world"), _chunk("!")] + ) + pieces = _collect_filtered(llm, _RETRY_MSGS, f) + assert [(p.kind, p.text) for p in pieces] == [ + ("content", "Hello "), + ("content", " world"), + ("content", "!"), + ] + assert f.stripped_chars == len(_INCIDENT_SPAN) + assert all("<|" not in p.text for p in pieces if p.kind == "content") + + +def test_chat_stream_with_filter_span_split_across_chunks() -> None: + """A span split across two chunks never emits a partial marker — + nothing leaks until the span completes, then the clean text on both + sides flows and the whole span counts as stripped.""" + span = _INCIDENT_SPAN + cut = len("<|tool_call_start|>") # split right after the start token + f = ScaffoldingFilter() + llm, _ = _make_stream_client([_chunk("A " + span[:cut]), _chunk(span[cut:] + " B")]) + pieces = _collect_filtered(llm, _RETRY_MSGS, f) + assert [(p.kind, p.text) for p in pieces] == [ + ("content", "A "), + ("content", " B"), + ] + assert f.stripped_chars == len(span) + + +def test_chat_stream_with_filter_yields_nothing_for_pure_scaffolding() -> None: + """A content stream of pure scaffolding yields ZERO content pieces — + an empty clean result yields nothing (no empty ``delta`` frames).""" + f = ScaffoldingFilter() + llm, _ = _make_stream_client( + [_chunk(_INCIDENT_SPAN), _chunk("<|tool_calls|>")] + ) + pieces = _collect_filtered(llm, _RETRY_MSGS, f) + assert pieces == [] + assert f.stripped_chars == len(_INCIDENT_SPAN) + len("<|tool_calls|>") + + +def test_chat_stream_with_filter_leaves_thinking_raw() -> None: + """Locked (phase 71): the scratchpad stays raw — a span inside + ``reasoning_content`` is yielded verbatim and never counts as + stripped.""" + f = ScaffoldingFilter() + llm, _ = _make_stream_client( + [_chunk("", reasoning=_INCIDENT_SPAN), _chunk("ok")] + ) + pieces = _collect_filtered(llm, _RETRY_MSGS, f) + assert [(p.kind, p.text) for p in pieces] == [ + ("thinking", _INCIDENT_SPAN), + ("content", "ok"), + ] + assert f.stripped_chars == 0 + + +def test_chat_stream_without_filter_is_the_raw_path() -> None: + """``scaffolding=None`` (the default, and the explicit opt-out): a span + in content is yielded verbatim — byte-identical to the pre-phase-71 + raw path for callers that do not pass a filter.""" + llm, _ = _make_stream_client([_chunk(_INCIDENT_SPAN)]) + explicit_none = _collect_filtered(llm, _RETRY_MSGS, None) + default = asyncio.run(_collect(llm, _RETRY_MSGS)) + assert explicit_none == default == [StreamPiece("content", _INCIDENT_SPAN)] + + +def test_chat_stream_flushed_tail_precedes_tool_call_pieces() -> None: + """Content-before-tools wire convention (phase 71): a stream that ends + with a held filter tail (a partial marker at EOF — flushed as-is) + + tool_calls deltas yields the flushed tail content piece BEFORE the + materialized ``ToolCallPiece``.""" + f = ScaffoldingFilter() + llm, _ = _make_stream_client( + [ + _chunk("done <|tool_call_st"), # held: a live prefix of the start token + _chunk( + None, + tool_calls=[_tool_call(0, id="call_t", name="ls")], + finish_reason="tool_calls", + ), + ] + ) + + async def run() -> list[StreamPiece | ToolCallPiece]: + return [ + p + async for p in llm.chat_stream(_RETRY_MSGS, tools=_AGENT_TOOLS, scaffolding=f) + ] + + pieces = asyncio.run(run()) + assert pieces == [ + StreamPiece("content", "done "), + StreamPiece("content", "<|tool_call_st"), + ToolCallPiece(id="call_t", name="ls", arguments={}), + ] + + +def test_chat_stream_abandon_with_filter_closes_stream() -> None: + """Phase-48 teardown is independent of the phase-71 filter: a consumer + abandon mid-filter still closes the endpoint stream exactly once.""" + f = ScaffoldingFilter() + llm, completions = _make_stream_client([_chunk(f"piece {i} ") for i in range(1, 6)]) + + async def run() -> None: + gen = llm.chat_stream(_RETRY_MSGS, scaffolding=f) + first = await gen.__anext__() + assert isinstance(first, StreamPiece) + assert first.text == "piece 1 " + await gen.aclose() # the consumer stops after the first piece + + asyncio.run(run()) + assert completions.streams[0].close_calls == 1 + + # ---------- one-shot chat: LLMClient.chat (phase 30, task 01) ---------- @@ -826,11 +973,15 @@ class _ScriptedClient(LLMClient): ] = [] #: Indices of attempts whose stream teardown has run. self.closed: list[int] = [] + #: The caller-owned filter each attempt's ``chat_stream`` received + #: (phase 71 task 02 — the retry primitive forwards it). + self.scaffoldings: list[ScaffoldingFilter | None] = [] def chat_stream( self, messages: list[dict[str, str]], tools: list[dict[str, Any]] | None = None, + scaffolding: ScaffoldingFilter | None = None, ) -> AsyncGenerator[StreamPiece | ToolCallPiece, None]: index = len(self.request_args) pieces, error = ( @@ -841,15 +992,30 @@ class _ScriptedClient(LLMClient): self.request_args.append( (list(messages), list(tools) if tools is not None else None) ) - return self._attempt(index, pieces, error) + self.scaffoldings.append(scaffolding) + return self._attempt(index, pieces, error, scaffolding) async def _attempt( - self, index: int, pieces: list[StreamPiece | ToolCallPiece], + self, + index: int, + pieces: list[StreamPiece | ToolCallPiece], error: Exception | None, + scaffolding: ScaffoldingFilter | None, ) -> AsyncGenerator[StreamPiece | ToolCallPiece, None]: try: for piece in pieces: - yield piece + if ( + scaffolding is not None + and isinstance(piece, StreamPiece) + and piece.kind == "content" + ): + # Emulate the real contract (phase 71): content feeds + # the filter, an empty clean result yields nothing. + cleaned = scaffolding.feed(piece.text) + if cleaned: + yield StreamPiece("content", cleaned) + else: + yield piece if error is not None: raise error finally: @@ -875,12 +1041,18 @@ def _collect_retried( tools: list[dict[str, Any]] | None = None, retries: int, delay: float, + scaffolding: ScaffoldingFilter | None = None, ) -> list[StreamPiece | ToolCallPiece | RetryPiece]: async def run() -> list[StreamPiece | ToolCallPiece | RetryPiece]: return [ p async for p in chat_stream_retried( - client, messages, tools=tools, retries=retries, delay=delay + client, + messages, + tools=tools, + retries=retries, + delay=delay, + scaffolding=scaffolding, ) ] @@ -1016,6 +1188,40 @@ def test_retried_zero_delay_still_notifies(monkeypatch: pytest.MonkeyPatch) -> N assert sleeps == [0] +def test_retried_forwards_the_filter_to_every_attempt( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Phase 71: the caller-owned filter reaches EVERY attempt's + ``chat_stream`` — the same object on the dead attempt and the + surviving one.""" + client = _ScriptedClient( + [([], LLMError("down")), ([StreamPiece("content", "ok")], None)] + ) + f = ScaffoldingFilter() + _record_sleeps(monkeypatch) + pieces = _collect_retried(client, _RETRY_MSGS, retries=1, delay=0, scaffolding=f) + assert pieces == [RetryPiece(2, 2), StreamPiece("content", "ok")] + assert client.scaffoldings == [f, f] + + +def test_retried_reuses_the_unfed_filter_after_a_dead_attempt( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Phase 71: reusing the SAME filter across attempts is safe by + construction — the dead attempt emitted no piece, so the filter was + never fed; the surviving attempt filters through it as if fresh.""" + span = _INCIDENT_SPAN + client = _ScriptedClient( + [([], LLMError("down")), ([StreamPiece("content", span + " clean")], None)] + ) + f = ScaffoldingFilter() + _record_sleeps(monkeypatch) + pieces = _collect_retried(client, _RETRY_MSGS, retries=1, delay=0, scaffolding=f) + assert pieces == [RetryPiece(2, 2), StreamPiece("content", " clean")] + assert f.stripped_chars == len(span) + assert client.scaffoldings == [f, f] + + def test_retried_abandon_mid_attempt_closes_the_attempt_stream() -> None: """Consumer abandon at a mid-attempt piece (the stop-generation path, phase 48): no exception leaks and the attempt's stream is torn down diff --git a/tests/unit/test_prompts.py b/tests/unit/test_prompts.py index 8535da0..28d11e2 100644 --- a/tests/unit/test_prompts.py +++ b/tests/unit/test_prompts.py @@ -5,6 +5,11 @@ lite-generated KB outline): its own builder contract (empty → ``""``, char budget + ``[…truncated…]`` marker, pathological budgets), its placement between ```` and ```` in both modes, and the byte-identical-when-absent convention (phase 15 precedent). + +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. """ from __future__ import annotations @@ -129,11 +134,15 @@ def test_zero_note_prompt_is_byte_identical_to_pre_steering() -> None: assert build_high_prompt([doc]) == ( _base("HIGH") + "\n\n" + block + "\n" + "\n" + TOOLS_SECTION ) + # Phase 71: the LOW prompt carries the owner-permitted plain-text + # line after the DEFLECT_MODE sentence (the marker-keying contract + # is unchanged — the E2E mock keys on the marker's presence). assert build_deflect_prompt(["T1", "T2"]) == ( _base("LOW") + "\nDEFLECT_MODE: retrieval was weak — the titles below are the closest " "your notes come to the question. They are titles only; do not pretend " "they answer it. Use them to propose 2-3 alternative questions.\n" + "Reply in plain text only — you have no tools in this mode.\n" + "- T1\n- T2" ) assert "" not in build_high_prompt([doc]) @@ -299,11 +308,14 @@ def test_no_overview_prompt_is_byte_identical_to_pre_phase() -> None: docs_block = "\n\n" + block + "\n" + "\n" + TOOLS_SECTION high_plain = _base("HIGH") + docs_block high_steered = _base("HIGH") + "\n" + build_steering_section(["be concise"]) + docs_block + # Phase 71: the owner-permitted plain-text line is part of the + # DEFLECT_MODE body in every LOW build (with or without steering). low_plain = ( _base("LOW") + "\nDEFLECT_MODE: retrieval was weak — the titles below are the closest " "your notes come to the question. They are titles only; do not pretend " "they answer it. Use them to propose 2-3 alternative questions.\n" + "Reply in plain text only — you have no tools in this mode.\n" + "- T1\n- T2" ) low_steered = ( @@ -313,6 +325,7 @@ def test_no_overview_prompt_is_byte_identical_to_pre_phase() -> None: + "\nDEFLECT_MODE: retrieval was weak — the titles below are the closest " "your notes come to the question. They are titles only; do not pretend " "they answer it. Use them to propose 2-3 alternative questions.\n" + "Reply in plain text only — you have no tools in this mode.\n" + "- T1\n- T2" ) for kb in (None, "", " \n\t "): @@ -399,3 +412,67 @@ def test_prompt_kb_section_over_settings_budget_capped_with_marker( close = section.index("") section = section[: close + len("")] assert len(section) <= 200 + + +# ---------- phase 71: the deflection plain-text line (prevention) ---------- + +#: The owner-permitted (2026-09-03) line appended to the ``DEFLECT_MODE`` +#: body — the LOW prompt's only phase-71 change. The E2E mock keys on +#: the ``DEFLECT_MODE`` marker's *presence*, not the wording, so the +#: marker-keying contract is unchanged by the appended line. +PLAIN_TEXT_ONLY_LINE = "Reply in plain text only — you have no tools in this mode." + + +def _pre_phase71_low_body() -> str: + """The ``DEFLECT_MODE`` body exactly as it was before phase 71.""" + return ( + "DEFLECT_MODE: retrieval was weak — the titles below are the closest " + "your notes come to the question. They are titles only; do not pretend " + "they answer it. Use them to propose 2-3 alternative questions.\n" + ) + + +def test_low_prompt_is_pre_phase_plus_exactly_the_plain_text_line() -> None: + """Diff pin: the LOW prompt = pre-phase text + exactly the one new + line, appended to the ``DEFLECT_MODE`` body; the weak-hit title list + follows exactly as before (and the line occurs exactly once).""" + prompt = build_deflect_prompt(["T1", "T2"]) + assert prompt == ( + _base("LOW") + + "\n" + + _pre_phase71_low_body() + + PLAIN_TEXT_ONLY_LINE + + "\n" + + "- T1\n- T2" + ) + assert prompt.count(PLAIN_TEXT_ONLY_LINE) == 1 + assert prompt.endswith("- T1\n- T2") # the title list is untouched + + +def test_low_prompt_carries_the_line_and_keeps_the_mock_marker() -> None: + """The new line is present in the LOW prompt (inside the + ``DEFLECT_MODE`` body, after the marker) and the ``DEFLECT_MODE`` + marker the E2E mock keys on stays put.""" + for titles, tail in ((["T1"], "- T1"), ([], "(nothing close at all)")): + prompt = build_deflect_prompt(titles) + assert "DEFLECT_MODE" in prompt + assert PLAIN_TEXT_ONLY_LINE in prompt + assert prompt.index("DEFLECT_MODE") < prompt.index(PLAIN_TEXT_ONLY_LINE) + # The title list (or the no-titles fallback) follows the line + # exactly as before. + assert prompt.endswith(tail) + + +def test_plain_text_line_never_leaks_into_high_prompt() -> None: + """The line is the LOW prompt's: every HIGH build (with/without + steering/overview) is unchanged and carries none of it.""" + doc = _doc("kubernetes.md", "Talos Linux on three nodes.", "Kubernetes Homelab Cluster") + for notes, kb in ( + (None, None), + (["be concise"], None), + (None, OVERVIEW), + (["be concise"], OVERVIEW), + ): + high = build_high_prompt([doc], notes=notes, kb_overview=kb) + assert PLAIN_TEXT_ONLY_LINE not in high + assert "you have no tools" not in high diff --git a/tests/unit/test_scaffolding_filter.py b/tests/unit/test_scaffolding_filter.py new file mode 100644 index 0000000..877b14f --- /dev/null +++ b/tests/unit/test_scaffolding_filter.py @@ -0,0 +1,280 @@ +"""Unit tests: the deterministic tool-scaffolding filter (phase 71, task 01). + +The matrix from the phase overview: a span in one chunk (exact +``stripped_chars``), the observed incident span split across chunks at +**every** boundary offset of the start token (0..len) plus a mid-span +and an end-token split, multiple spans in one chunk, the standalone +sibling tokens (alone, embedded, char-by-char), look-alikes that must +NOT be stripped, partial markers at EOF (``flush`` emits as-is), and +exact preservation of surrounding text. +""" +from __future__ import annotations + +import pytest + +from app.rag.scaffolding import SCAFFOLD_PATTERNS, ScaffoldingFilter + +# The observed incident text (2026-09-03 — the deflected round that +# streamed raw model tool-scaffolding into the UI). +SPAN_START = "<|tool_call_start|>" +SPAN_END = "<|tool_call_end|>" +SPAN_CONTENT = "[read(path='/homelab/backup-notes.md')]" +SPAN = f"{SPAN_START}{SPAN_CONTENT}{SPAN_END}" +TOOL_CALLS = "<|tool_calls|>" +TOOL_CALL = "<|tool_call|>" + + +def _run(text: str, chunks: list[str] | None = None) -> tuple[str, ScaffoldingFilter]: + """Feed *chunks* (or *text* as one chunk) + ``flush``; return the + (total output, filter) pair the assertions below work on.""" + f = ScaffoldingFilter() + parts = [f.feed(chunk) for chunk in (chunks if chunks is not None else [text])] + parts.append(f.flush()) + return "".join(parts), f + + +# ---------------------------------------------------------------- the registry + + +def test_registry_is_the_three_observed_forms() -> None: + # Every entry traces to the 2026-09-03 incident — no speculative + # entries (the registry is the extension point; new forms need an + # observed capture + a fixture here). + assert len(SCAFFOLD_PATTERNS) == 3 + assert SCAFFOLD_PATTERNS[0].pattern == r"<\|tool_call_start\|>[\s\S]*?<\|tool_call_end\|>" + assert SCAFFOLD_PATTERNS[1].pattern == r"<\|tool_calls\|>" + assert SCAFFOLD_PATTERNS[2].pattern == r"<\|tool_call\|>" + # The span form is non-greedy: two spans each strip to their own end. + assert SCAFFOLD_PATTERNS[0].sub("", SPAN + "x" + SPAN) == "x" + + +# ---------------------------------------------- span in one chunk, split spans + + +def test_span_in_one_chunk_is_stripped_with_exact_count() -> None: + out, f = _run(SPAN) + assert out == "" + assert f.stripped_chars == len(SPAN) + + +def _split_offsets() -> list[int]: + """Every split offset of the start token (0..len) + a mid-span split + and an end-token split (the task-01 matrix).""" + offsets = list(range(len(SPAN_START) + 1)) # 0 .. 19, every start-token offset + offsets += [ + len(SPAN_START) + 1, # one char into the span body + len(SPAN_START) + len(SPAN_CONTENT) // 2, # mid-span + len(SPAN) - len(SPAN_END), # exactly at the end token + len(SPAN) - 5, # inside the end token + len(SPAN) - 1, # the very last char + ] + return offsets + + +@pytest.mark.parametrize("offset", _split_offsets()) +def test_span_split_at_boundary_offsets_emits_nothing_until_complete(offset: int) -> None: + f = ScaffoldingFilter() + first = f.feed(SPAN[:offset]) + second = f.feed(SPAN[offset:]) + third = f.flush() + assert first == "" # nothing ever emits until the span completes + assert second == "" + assert third == "" + assert f.stripped_chars == len(SPAN) + + +def test_span_completed_by_a_later_chunk_then_content_follows() -> None: + f = ScaffoldingFilter() + assert f.feed(f"lead {SPAN_START}part") == "lead " # prose before the span flows + assert f.feed(f"{SPAN_END} tail") == " tail" + assert f.flush() == "" + assert f.stripped_chars == len(SPAN_START) + 4 + len(SPAN_END) # body: "part" + + +def test_span_with_nested_start_token_strips_to_first_end() -> None: + # Non-greedy: the span runs from the FIRST start to the FIRST end. + text = f"{SPAN_START}a{SPAN_START}b{SPAN_END}" + out, f = _run(text) + assert out == "" + assert f.stripped_chars == len(text) + + +def test_spans_across_chunks_with_a_standalone_token_after() -> None: + f = ScaffoldingFilter() + assert f.feed(SPAN[:25]) == "" # inside the span body + assert f.feed(SPAN[25:] + " mid " + TOOL_CALLS) == " mid " + assert f.flush() == "" + assert f.stripped_chars == len(SPAN) + len(TOOL_CALLS) + + +# ------------------------------------------------------- multiple spans / tokens + + +def test_two_spans_in_one_chunk_strip_each_to_their_own_end() -> None: + out, f = _run(f"{SPAN} middle {SPAN}") + assert out == " middle " + assert f.stripped_chars == 2 * len(SPAN) + + +def test_two_spans_with_different_content() -> None: + other = f"{SPAN_START}[grep(pattern='vault')]{SPAN_END}" + out, f = _run(f"before{SPAN}mid{other}after") + assert out == "beforemidafter" + assert f.stripped_chars == len(SPAN) + len(other) + + +def test_repeated_strip_until_no_complete_match_remains() -> None: + # A standalone token embedded in a would-be token: step (a) repeats + # leftmost-complete removals until the buffer is clean. + out, f = _run(f"x<|tool_call{TOOL_CALLS}|>y") + assert out == "xy" + assert f.stripped_chars == len(TOOL_CALLS) + len(TOOL_CALL) + + +# ------------------------------------------------------------ standalone tokens + + +@pytest.mark.parametrize( + ("token", "expected"), + [(TOOL_CALLS, 14), (TOOL_CALL, 13)], +) +def test_standalone_token_alone_is_stripped(token: str, expected: int) -> None: + out, f = _run(token) + assert out == "" + assert f.stripped_chars == expected + + +@pytest.mark.parametrize("token", [TOOL_CALLS, TOOL_CALL]) +def test_standalone_token_embedded_in_a_line_is_stripped(token: str) -> None: + out, f = _run(f"line {token} tail\n") + assert out == "line tail\n" + assert f.stripped_chars == len(token) + + +def test_standalone_tokens_adjacent_to_text() -> None: + out, f = _run(f"a{TOOL_CALL}b{TOOL_CALLS}c") + assert out == "abc" + assert f.stripped_chars == len(TOOL_CALL) + len(TOOL_CALLS) + + +@pytest.mark.parametrize("token", [TOOL_CALLS, TOOL_CALL]) +def test_standalone_token_char_by_char_never_emits(token: str) -> None: + out, f = _run("", list(token)) + assert out == "" + assert f.stripped_chars == len(token) + + +def test_partial_standalone_token_completing_in_a_later_chunk() -> None: + f = ScaffoldingFilter() + assert f.feed("<|tool_ca") == "" + assert f.feed("lls|>") == "" + assert f.flush() == "" + assert f.stripped_chars == len(TOOL_CALLS) + + +def test_partial_token_resolving_to_prose_is_emitted() -> None: + # ``<|tool_cat|>`` is no known form — the held partial prefix must be + # released as content once the input can no longer complete a token. + f = ScaffoldingFilter() + assert f.feed("x" * 50 + "<|tool_ca") == "x" * 50 # live prefix held + assert f.feed("t|> done") == "<|tool_cat|> done" + assert f.flush() == "" + assert f.stripped_chars == 0 + + +# ------------------------------------------------------------------- look-alikes + + +@pytest.mark.parametrize( + "text", + [ + "tool_call_start", # the word as prose, no delimiters + "the tool_call and tool_calls words", # prose words + "<|tool_call_start|", # missing delimiter close — prose at EOF + "<|some_other_token|>", # unknown token + "<|tool_call_end|>", # a lone end token without a start + "a < b", # a lone angle bracket + ], +) +def test_lookalikes_are_not_stripped_and_emit_verbatim(text: str) -> None: + out, f = _run(text) + assert out == text + assert f.stripped_chars == 0 + + +def test_lone_end_token_before_an_open_start_stays_content_until_closed() -> None: + # The lone end token is content (emitted as-is); the trailing start + # opens a span and is HELD — if the span later closes, only the + # span is stripped, the lone end token stays. + f = ScaffoldingFilter() + assert f.feed(f"{SPAN_END}{SPAN_START}") == SPAN_END + assert f.feed(f"body{SPAN_END}") == "" + assert f.flush() == "" + assert f.stripped_chars == len(SPAN_START) + 4 + len(SPAN_END) + + +def test_lone_end_token_at_eof_flushes_as_is() -> None: + f = ScaffoldingFilter() + out = f.feed(SPAN_END) + assert out + f.flush() == SPAN_END # verbatim total (the tail may be split) + assert f.stripped_chars == 0 + + +# ----------------------------------------------------------- EOF / empty / clean + + +def test_partial_start_token_at_eof_flushes_as_is() -> None: + # A partial marker at EOF is content, not scaffolding (pinned choice). + f = ScaffoldingFilter() + assert f.feed("<|tool_call_st") == "" + assert f.flush() == "<|tool_call_st" + assert f.stripped_chars == 0 + + +@pytest.mark.parametrize( + "partial", + [ + "<", + "<|", + "<|tool_ca", + "<|tool_call|", # one char short of the standalone token + "<|tool_calls|", # one char short of the standalone token + "<|tool_call_start|", # one char short of the span opening + ], +) +def test_partial_tokens_at_eof_flush_as_is(partial: str) -> None: + out, f = _run(partial) + assert out == partial + assert f.stripped_chars == 0 + + +def test_empty_chunk_is_a_noop() -> None: + f = ScaffoldingFilter() + assert f.feed("") == "" + assert f.feed("") == "" + assert f.flush() == "" + assert f.stripped_chars == 0 + + +def test_empty_chunk_amid_pending_is_a_noop() -> None: + f = ScaffoldingFilter() + assert f.feed("<|tool_ca") == "" + assert f.feed("") == "" + assert f.feed("lls|>") == "" # completes the standalone token + assert f.flush() == "" + assert f.stripped_chars == len(TOOL_CALLS) + + +def test_flush_on_a_clean_stream_emits_nothing_new() -> None: + f = ScaffoldingFilter() + assert f.feed("hello ") == "hello " + assert f.feed("world") == "world" + assert f.flush() == "" + assert f.stripped_chars == 0 + + +def test_surrounding_text_preserved_exactly() -> None: + # No reflow beyond the removal — both spaces around the span stay. + out, f = _run(f"hello {SPAN} world") + assert out == "hello world" + assert f.stripped_chars == len(SPAN)