fix(agent): teach the document-identity contract on ls/read/grep refusals — end the post-harness tool-loop rambling
Phase 72 (72_teaching_refusals) — completed under the 2026-09-04 controlled methodology (owner directive: stop clearing/re-importing the homelab KB per iteration; measure tool-calling accuracy on a controlled fixture KB, target >90%). Real-model gate verdicts (live, configured chat model 'lite', fixture KB): - Controlled fixture battery (the new methodology's pass condition — contract accuracy >= 90%): PASS, 4 consecutive runs: gate: lite PASS turns=10 answered=10 caps=0 tool-turns=10 calls 8/11 executed (73%) contract 11/11 (100%) 2026-09-04 (wall 43.4s) gate: lite PASS turns=10 answered=10 caps=0 tool-turns=10 calls 8/13 executed (62%) contract 12/13 (92%) 2026-09-04 (wall 50.6s) gate: lite PASS turns=10 answered=10 caps=0 tool-turns=10 calls 7/11 executed (64%) contract 11/11 (100%) 2026-09-04 (wall 46.8s) gate: lite PASS turns=10 answered=10 caps=0 tool-turns=10 calls 9/15 executed (60%) contract 14/15 (93%) 2026-09-04 (wall 54.8s) - Locked derived battery (phase-72 task 05, executed >= 90% bar, run unchanged on the same fixture KB): gate: lite FAIL turns=10 answered=10 caps=0 tool-turns=10 calls 5/15 executed (33%) contract 12/15 (80%) 2026-09-04 (wall 47.7s) The teaching works — every bare-path trap self-corrects in exactly one round, zero cap hits, zero repeat loops, 10/10 answered. The locked executed bar is blocked by ALREADY_IN_CONTEXT dedupe refusals on the corrected re-reads (the trap question seeds its target, so the correct combined-form read is refused for redundancy) — a copy-invariant model behavior (five copy variants, 0/15 re-reads flipped, 2026-09-03 -> 04) and an app-semantics decision for the owner (TOOL_CALLING_TESTING.md sections 5 and 7), not a copy lever. Copy changes this phase owns (unit pins updated to follow): - app/rag/agent.py: ls teaching refusals (path-like scope -> document-path line; unknown source -> no-source line with the source-name parenthetical), read/grep 'did you mean source/path?' teaching (find_path_candidates: exact or suffix path match, catalog order, cap 3), ALREADY_IN_CONTEXT naming the correct action (answer from the text already in the prompt), read tool description front-loaded with the do-not-read rule (the 2026-09-04 controlled telemetry: the re-read is the only remaining refusal class; contract accuracy 92-100% across runs) - app/rag/prompts.py: TOOLS_SECTION states the document-identity contract up front (ls path = source name; read/grep = combined source/path including the source name; do-not-read for <documents> documents placed next to the read teaching; one-call-per-reply and never-repeat rules) - tests: refusal pins (unit + integration), new dedicated E2E suite tests/e2e/test_tool_path_teaching.py (mock misuse flow, green in isolation), regression suites green in isolation (harness_aligned_tools, agent_document_tools, agent_unlimited_tools, search_tool, chat_rag). Gates: uv run pytest green (1501); coverage TOTAL 99% (>90%); ruff + pyright clean. Carries the still-uncommitted phase-71 todo/ -> complete/ move and both phases' .agent/reports/ (AGENTS.md 8).
This commit is contained in:
@@ -0,0 +1,152 @@
|
||||
# Phase 71 — Tool-Scaffolding Guardrails: Deterministic Strip + One Bounded Recovery
|
||||
|
||||
**Source:** owner request (chat, 2026-09-03) — same incident as phase 70: the
|
||||
deflected answer streamed the raw model text
|
||||
`<|tool_call_start|>[read(path='/homelab/backup-notes.md')]<|tool_call_end|>` into
|
||||
the UI (the model's own `<|…|>` chat-template tool syntax, emitted as plain
|
||||
`delta.content` even though no tools were offered). Owner direction: "We also need
|
||||
guardrails for situations like this… **deterministic guardrails only** right now,
|
||||
forget using a model for that" — no model of any kind (no `lite` classifier, no
|
||||
model-authored repair) in the guardrail path; `lite` stays the chat model for
|
||||
everything (it's faster).
|
||||
**Story:** n/a (owner request from chat — tool-scaffolding guardrails, 2026-09-03)
|
||||
**Context:**
|
||||
- `app/rag/llm.py` — `chat_stream` yields `StreamPiece("content", delta.content)`
|
||||
verbatim (L~`content = delta.content; if content: yield`); thinking pieces
|
||||
(`delta.reasoning_content`) pass through raw by design; tool-call materialization
|
||||
happens at stream end (after the `async for`); phase-48 teardown closes the
|
||||
endpoint stream on every exit (must stay intact). `chat_stream_retried` (phase 67)
|
||||
wraps it with the retry-before-first-piece rule.
|
||||
- `app/rag/agent.py` — `run_agent`'s round loop: a round with no `ToolCallPiece`s
|
||||
ends the turn (`if not calls: return`); the round cap forces one final
|
||||
`tools=None` request (the "forced final answer" pattern this phase reuses for
|
||||
recovery); `AgentHolder` (read_docs / tool_calls) is unchanged.
|
||||
- `app/api/chat.py` — the piece loop (thinking/tool/retry/delta handling +
|
||||
`thinking_chars` counter), the deflected path (`chat_stream_retried(...,
|
||||
tools=None)` directly), the `LLMError` → terminal error-frame handler, the
|
||||
per-turn log line (ends `…total_ms=N retries=N`).
|
||||
- `app/rag/prompts.py` — `build_deflect_prompt`'s `DEFLECT_MODE` body (the E2E mock
|
||||
keys on the marker's *presence*, not the wording — an appended line is safe).
|
||||
- `tests/e2e/mock_llm.py` — the trigger-string flow table (task 05 adds the
|
||||
scaffolding triggers; phase-70 names apply).
|
||||
- Tests to extend: `tests/unit/test_llm_client.py`, `test_agent.py`,
|
||||
`tests/integration/test_chat_api.py` (log-line + error-frame pins),
|
||||
`tests/unit/test_prompts.py`.
|
||||
|
||||
## Objective
|
||||
Raw tool-scaffolding tokens can never reach the user as answer text: a deterministic
|
||||
streaming filter strips known scaffolding from `delta.content` as it flows, and a
|
||||
round/turn whose visible content ends up empty (scaffolding was the whole "answer")
|
||||
gets **one** bounded, deterministic recovery (same turn, `tools=None`, a fixed
|
||||
harness-owned correction line in the system prompt); if the recovery also comes back
|
||||
empty, the turn settles with a dedicated structured error frame. No model is used to
|
||||
detect or repair anything.
|
||||
|
||||
## Dependencies
|
||||
- `70_harness_aligned_tools` (todo) — runs first: the mock/prompt/code state this
|
||||
phase builds on (new tool names in the mock flows and prompt).
|
||||
- `67_llm_retry` (complete) — `chat_stream_retried`, the primitive every request
|
||||
(including recoveries) goes through.
|
||||
- `48_stop_generation` (complete) — the phase-48 stream-teardown contract the
|
||||
filter integration must not break.
|
||||
|
||||
## Tasks
|
||||
1. `01_filter.md` — `app/rag/scaffolding.py` (new): the pattern registry +
|
||||
`ScaffoldingFilter` streaming state machine, with the boundary test matrix.
|
||||
2. `02_llm_integration.md` — `app/rag/llm.py`: `chat_stream`/`chat_stream_retried`
|
||||
accept the filter; content deltas are filtered, the tail flushed before tool
|
||||
materialization; `None` = byte-identical raw path.
|
||||
3. `03_recovery_policy.md` — `app/rag/agent.py` + `app/api/chat.py`: the empty-round
|
||||
recovery (one per turn, `tools=None`, correction constant),
|
||||
`MalformedReplyError`, the dedicated error copy, and the per-turn log line's
|
||||
`scaffold_stripped=N` field.
|
||||
4. `04_deflect_prompt.md` — the plain-text line in the `DEFLECT_MODE` body
|
||||
(prevention; owner-permitted LOW-prompt change).
|
||||
5. `05_e2e_commit.md` — the mock scaffolding triggers, the dedicated E2E suite,
|
||||
full gates, commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (new `tests/unit/test_scaffolding_filter.py`): the filter matrix — span in
|
||||
one chunk; span split across chunks at **every** boundary offset of the start
|
||||
token; multiple spans in one chunk; standalone `<|tool_calls|>` / `<|tool_call|>`
|
||||
tokens stripped; look-alikes **not** stripped (prose containing the words
|
||||
"tool_call" or `tool_call_start` without the `<|…|>` delimiters, an unknown
|
||||
`<|some_other_token|>`, a lone `<|tool_call_end|>` without a start); a partial
|
||||
start token at stream end → `flush()` emits it as-is (no false-positive strip);
|
||||
`stripped_chars` accounting; empty chunks.
|
||||
- Unit: `tests/unit/test_llm_client.py` — `chat_stream` with a filter (content
|
||||
filtered, thinking raw, `None` = raw pass-through pinned byte-identical, flush
|
||||
order: flushed tail content precedes tool-call pieces, phase-48 teardown intact).
|
||||
- Unit: `tests/unit/test_agent.py` — grounded recovery matrix (scaffolding-only
|
||||
round → exactly one recovery request: `tools=None` + correction line in the
|
||||
system prompt + fresh filter → clean answer ends the turn; scaffolding twice →
|
||||
`MalformedReplyError`; scaffolding + real content → clean answer, **no**
|
||||
recovery; round cap and kill switch unchanged).
|
||||
- Integration: `tests/integration/test_chat_api.py` — deflected-path recovery matrix
|
||||
(same shapes over the SSE endpoint: clean recovery → `done` frame; terminal → the
|
||||
dedicated error frame, no `done`, no `query_log` row — same terminal semantics as
|
||||
today's `LLMError`); log line carries `scaffold_stripped=N` (0 when nothing was
|
||||
stripped — the field is uniform, the phase-67 `retries=N` pattern).
|
||||
- E2E (mandatory, house rule): NEW dedicated suite
|
||||
`tests/e2e/test_tool_scaffolding_guardrails.py`, run in isolation — recovery case
|
||||
(raw tokens never in the DOM, clean answer shown) and terminal case (error state,
|
||||
no raw tokens, the app stays usable).
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `rg "tool_call_start" frontend/` → no matches (no scaffolding rendering
|
||||
path); the filter lives in `app/rag/scaffolding.py` as a pure module (no I/O,
|
||||
no model calls).
|
||||
- [ ] A content stream of pure scaffolding yields zero `delta` frames; a mixed
|
||||
stream yields the clean remainder; thinking frames are never filtered.
|
||||
- [ ] Exactly one recovery per turn (grounded and deflected paths); the recovery
|
||||
request is `tools=None` with the fixed correction line in the system prompt;
|
||||
a second empty reply settles with the error frame
|
||||
"The model returned a malformed reply — please try again."
|
||||
- [ ] The per-turn log line ends `…retries=N scaffold_stripped=N`; `scaffold_stripped=0`
|
||||
on clean turns (uniform field).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app` TOTAL **>90%**;
|
||||
`uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `uv run pytest tests/e2e/test_tool_scaffolding_guardrails.py -v --no-cov`
|
||||
green in isolation; regression suites green in isolation:
|
||||
`test_harness_aligned_tools.py`, `test_chat_rag.py`, `test_agent_document_tools.py`.
|
||||
- [ ] One `--no-gpg-sign` commit (message in the Commit block); phase dir moved to
|
||||
`.agent/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **Owner (chat, 2026-09-03): deterministic only.** No model — `lite` or any other —
|
||||
participates in detection or repair. The guardrail is a fixed pattern registry + a
|
||||
fixed retry policy. (A model-based classifier/repair was proposed and explicitly
|
||||
rejected for now — if it is ever wanted, it is a later phase with its own
|
||||
permission.)
|
||||
- **`lite` stays the chat model** (owner: "I want to use lite for everything since
|
||||
it's way faster") — the guardrail is what protects the UX while `lite` is the
|
||||
model; no `.env` change in this phase.
|
||||
- **The pattern registry is the extension point.** Initial entries: the observed
|
||||
span form `<|tool_call_start|>…<|tool_call_end|>` (non-greedy, any text between)
|
||||
plus the standalone sibling tokens `<|tool_calls|>` and `<|tool_call|>` from the
|
||||
same tokenizer family. Every strip logs a warning with the stripped span
|
||||
(truncated to 200 chars) — that log line is how a new format gets captured and
|
||||
added (pattern + unit fixture), keeping the registry honest (every entry traces
|
||||
to an observed capture or the initial incident).
|
||||
- **Content only, thinking never filtered.** The Thinking block is the model's raw
|
||||
reasoning by design (phase 17) and stays raw (collapsible); the guardrail protects
|
||||
the answer, not the scratchpad.
|
||||
- **Recovery is a fixed policy, not a conversation.** One extra request per turn,
|
||||
same messages with the harness-owned constant folded into the system prompt
|
||||
(single system message — provider-safe), `tools=None`, a fresh filter, the same
|
||||
phase-67 retry budget. At most one recovery; the second empty reply is terminal.
|
||||
A round with real visible content plus scaffolding needs no recovery (the clean
|
||||
content stands).
|
||||
- **Terminal semantics follow the existing error pattern.** A terminal malformed
|
||||
turn settles with a structured `error` frame (dedicated copy), writes no
|
||||
`query_log` row, and the UI shows the existing error state — byte-for-byte the
|
||||
same shape as today's `LLMError` terminal path.
|
||||
- **`MalformedReplyError` subclasses `LLMError`** and is raised only by the recovery
|
||||
policy (never from inside a stream, so `chat_stream_retried`'s retry rule never
|
||||
sees it); `chat.py` catches it before the generic `LLMError` handler.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agent/ app/ tests/ frontend/ && git commit --no-gpg-sign -m "feat(agent): strip raw tool-scaffolding from streamed answers — deterministic filter with one bounded recovery"
|
||||
```
|
||||
@@ -0,0 +1,77 @@
|
||||
# Task 05 — Mock Triggers, Dedicated E2E Suite, Gates, Commit
|
||||
|
||||
**Phase:** `71_scaffolding_guardrails` · **Story:** n/a (owner request from chat, 2026-09-03)
|
||||
|
||||
## Objective
|
||||
Prove the guardrail end-to-end through the real UI: a mock-LLM scaffolding flow
|
||||
(recovery case + terminal case), a dedicated Playwright suite pinning that raw
|
||||
tokens never reach the DOM, then the full quality gates and the phase commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` — two new deterministic flows (checked in the flow
|
||||
table **before** the plain `TOOLS_TRIGGER` flow, after `SEARCH_TRIGGER`
|
||||
ordering rules as they fit — the triggers are independent of the `<tools>`
|
||||
marker, so both grounded and deflected turns hit them):
|
||||
- `SCAFFOLD_TRIGGER = "emit raw tool markup"` — request 1 (no correction in
|
||||
the system prompt): stream ONLY `delta.content` chunks carrying the incident
|
||||
text `<|tool_call_start|>[read(path='search_docs/reese-notes.md')]<|tool_call_end|>`
|
||||
(split across ≥2 chunks to exercise the boundary path), `finish_reason:
|
||||
"stop"`, no structured `tool_calls`, no reasoning. Request 2 (system prompt
|
||||
contains the stable substring of `CORRECTION_INSTRUCTION` — import it from
|
||||
`app.rag.agent` so the mock can never drift from the constant): stream a
|
||||
clean plain answer ("Here is the plain-text answer the recovery produced.")
|
||||
+ `finish_reason: "stop"`.
|
||||
- `SCAFFOLD_ALWAYS_TRIGGER = "always emit raw tool markup"` — every request
|
||||
(recovery included): the same scaffolding-only stream, forever.
|
||||
- Update the module docstring's flow table + the phase-71 note.
|
||||
2. `tests/e2e/test_tool_scaffolding_guardrails.py` (NEW — the phase's dedicated
|
||||
suite, house pattern, run in isolation; DB up, mock LLM):
|
||||
- **Recovery case** — ask a question containing `SCAFFOLD_TRIGGER`: the turn
|
||||
settles (the composer re-enables, `done` observed); the final answer bubble
|
||||
contains the recovery's clean text; `document.body.innerText` contains
|
||||
**neither** `tool_call_start` nor `tool_call_end` (nor the raw
|
||||
`[read(path=…]` fragment); no error banner.
|
||||
- **Terminal case** — ask a question containing
|
||||
`SCAFFOLD_ALWAYS_TRIGGER`: the existing error status renders with the
|
||||
dedicated copy ("The model returned a malformed reply — please try
|
||||
again."); no raw tokens in the DOM; no answer bubble with the scaffolding;
|
||||
the app stays usable — a follow-up plain question (no trigger) gets a
|
||||
normal streamed answer in the same session.
|
||||
- **No false positive** — a plain question (existing `CHAT_TRIGGER`-style
|
||||
flow, no tools needed): the answer streams byte-clean, no error state, no
|
||||
recovery request visible (the turn settles on the first request).
|
||||
- The SSE wire itself: in the recovery case, no `delta` frame ever carries a
|
||||
`tool_call_start`/`tool_call_end` fragment (the existing SSE-capture house
|
||||
pattern) — the strip happens server-side, not in the UI.
|
||||
3. Gates + commit:
|
||||
- `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing`
|
||||
TOTAL **>90%**; `uv run ruff check . && uv run pyright` clean.
|
||||
- E2E in isolation (DB up): `test_tool_scaffolding_guardrails.py`, then the
|
||||
regression suites `test_harness_aligned_tools.py`, `test_chat_rag.py`,
|
||||
`test_agent_document_tools.py`.
|
||||
- One atomic commit (message below); move
|
||||
`.agent/phases/todo/71_scaffolding_guardrails/` →
|
||||
`.agent/phases/complete/71_scaffolding_guardrails/`.
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: the new dedicated suite (recovery / terminal / no-false-positive / wire
|
||||
pins) + the three regression suites (isolation runs).
|
||||
- Coverage: **>90%** on `app/` (phase-level gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `rg "tool_call_start|tool_call_end" frontend/` → no matches.
|
||||
- [ ] Raw scaffolding is never visible in the DOM in any case (recovery,
|
||||
terminal, mixed) — pinned by the dedicated suite.
|
||||
- [ ] Exactly one recovery per malformed turn (mock request counts implied by the
|
||||
flows); clean turns never carry the correction line (pinned by the unit
|
||||
suites from task 03).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app` TOTAL **>90%**;
|
||||
`uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `uv run pytest tests/e2e/test_tool_scaffolding_guardrails.py -v --no-cov`
|
||||
green in isolation; regression suites green in isolation.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agent/ app/ tests/ frontend/ && git commit --no-gpg-sign -m "feat(agent): strip raw tool-scaffolding from streamed answers — deterministic filter with one bounded recovery"
|
||||
```
|
||||
Reference in New Issue
Block a user