phase: 160_tool_budget_telemetry
All green. Final report: **Phase 160 completion pass — verified (final verification; all 5 tasks already in `complete/`)** - **Verified, no code defects found:** budget trailers (`TOOL_BUDGET_REMAINING/LAST/SPENT`), cap-path rejection + `ROUND_CAP_FALLBACK_ANSWER` + `ROUND_CAP_WRAPUP_INSTRUCTION`, `done.budget_exhausted` (ABSENT-when-false), `rounds=/cap_hit=` log fields, `agent_max_rounds=10` default, frontend Continue button + restore + shared-page exclusion, mock markers. - **Tests/lint:** `uv run pytest` 153/153 green (EXIT 0); `uv run pytest --cov=app --cov-report=term-missing` TOTAL **99%** (>90%); `uv run ruff check .` clean; `uv run pyright` 0 errors/0 warnings. - **Criteria:** ① decreasing trailer pinned by unit tests (9→…→spent) ✅ ② cap path ends readable, no tool lines after cap (E2E) ✅ ③ `budget_exhausted` true-only-on-cap, absent otherwise (integration) ✅ ④ Continue button → one fresh `continue` turn, fresh budget, record intact (E2E) ✅ ⑤ flag survives refresh; shared chat never renders it (`shared.js` has no `budget_exhausted` path) ✅ ⑥ `max_rounds=0` byte-identical single `tools=None` request; default still 10 ✅ ⑦ full suite/coverage/lint green ✅ ⑧ `test_tool_budget_telemetry.py` **7/7** in isolation; all 6 regression E2E suites green in isolation ✅ ⑨ commit + phase move = harness (I make no git history/index changes). - **Docs:** README.md — fixed: the "Continue researching" button and cap behavior were undocumented (added a Chat-section paragraph + an Agent-tools-section paragraph, incl. the phase-71 recorded-revision note); env-var table, commands, and architecture all verified consistent otherwise. AGENTS.md — verified consistent, no changes needed. - **Decisions:** no code changes required — phase was fully implemented; only README docs added. No live-infrastructure touched. - **Next pending phase:** `161_reasoning_bubble`.
This commit is contained in:
@@ -0,0 +1,64 @@
|
||||
# Phase 160 — The agent knows its tool-call budget (and never dumps tool calls into the answer)
|
||||
|
||||
**Source:** `TODO.md` L4 — "The LLM need to know how many tool calls it has left. During long research sessions it will sometimes end the conversation with tool calls that just get dumped into chat because it ran out of turns. Every tool call should also return 'you have x tool calls remaining' or some other helpful message and then on the last one attempted tool calls should be rejected rather than spit out to chat and the LLM should be encouraged to come up with a reply or tell the user to continue the session by allowing more tool calls (or something similar)."
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-10-05)
|
||||
**Context:** `app/rag/agent.py` (2,322 lines) — `run_agent` (L1995): `max_rounds = settings.agent_max_rounds` (L2103), `tools = agent_tools(settings) if max_rounds > 0 else None` (L2104), `rounds = 0` (L2106), the round loop (L2107) whose pieces are all yielded verbatim (L2136-2142: `if isinstance(piece, ToolCallPiece): calls.append(piece)` … `yield piece`), `call = calls[0]` (L2224 — "a stream can carry several calls; run the first"), `result = _execute_tool(tool_db, call, seed_docs, holder, settings)` (L2227, a plain string), `rounds += 1` (L2229), the `agent tool=%s args=%s round=%d/%d` INFO line (L2230), `messages.append({role: assistant, tool_calls: […]})` + `messages.append({"role": "tool", "tool_call_id": …, "content": result})` (L2237-2251), the phase-95 `ToolResultPiece` yields (L2258), and the **round-cap block** (L2270-2322): the `agent round cap reached (rounds=%d) — forcing a final no-tools answer` warning, the forced `chat_stream_retried(llm, messages, tools=None, …, scaffolding=final_filter)` whose loop yields EVERY piece (`ToolCallPiece` included), and the phase-71 terminal `MalformedReplyError` when the forced final carried no visible content but stripped scaffolding.
|
||||
Refusal/teaching constants (the house pattern for teaching the model in-band): `ALREADY_IN_CONTEXT` (L537), `UNKNOWN_TOOL` (L541), `READ_TRUNCATION_NOTICE` (L578), `REREAD_PARTIAL` (L628), `CORRECTION_INSTRUCTION` (L732, folded into the single system message as `system_prompt + "\n" + CORRECTION_INSTRUCTION` at L2178 / L1044 in `app/api/chat.py`). `AgentHolder` (L1482: `read_docs`, `tool_calls`, `scaffold_stripped`, `read_truncations`, `shown_images`, `read_coverage` — the API layer reads it after the stream).
|
||||
`app/config.py` `agent_max_rounds: int = 10` (+ its loud validator, ~L229-240 / L694 pattern), `.env.example` L69 (`# BOR_AGENT_MAX_ROUNDS=10 … (0 = no tools)`).
|
||||
`app/api/chat.py` — the grounded vs deflected dispatch (L840-900), `tools=None` for the deflected turn (L870) and the kill switch, `_pump(pieces)` (L893, the ONE piece→SSE loop: `ToolCallPiece` → `tool` frame at L909, `ToolResultPiece` → `tool_result`, `thinking`/`delta`), the per-turn log line (L1196-1219: `… thinking_chars=%d tool_calls=%d total_ms=%d retries=%d scaffold_stripped=%d`), the `ChatDoneEvent` yield (~L1265).
|
||||
`app/schemas.py` — `ChatDoneEvent` (L316: `type`/`deflected`/`sources`/`related`/`suggestions`/`answer_image` + the `@model_serializer(mode="wrap")` omission rule that drops `answer_image` when `None` — "ABSENT, never null"), `ChatErrorEvent.hint` (the phase-114 additive precedent), `ChatMessage` (L1264ff, `extra="forbid"`, the optional `failed`/`error`/`stopped` markers).
|
||||
`app/rag/scaffolding.py` — `SCAFFOLD_PATTERNS` (the observed-forms registry: `<|tool_call_start|>…<|tool_call_end|>`, `<|tool_calls|>`, `<|tool_call|>`) + `ScaffoldingFilter` (content only, never thinking).
|
||||
Tests: `tests/unit/test_agent.py` (the always-calling fake LLM: `_run(llm, holder, _settings(agent_max_rounds=N))`, the round-cap cluster L4561-4944, the forced-final scaffolding case L5241-5255 asserting `MalformedReplyError`, `llm.requests[i][1] is None` as the "no tools offered" pin), `tests/integration/test_chat_api.py`, `tests/e2e/mock_llm.py` (`TOOLS_TRIGGER` L1114 "use your tools", `MULTI_READ_TRIGGER` L1123 "read two documents", `SCAFFOLD_TRIGGER` L1375 "emit raw tool markup", `LONG_ANSWER_TRIGGER` L1035), `tests/e2e/test_agent_unlimited_tools.py` (the multi-read flow + the TRUNCATE-then-seed KB fixture).
|
||||
PLAN §4 (SSE contract), §9 (the required per-turn line), A15 (event types `thinking, delta, tool, retry, done, error, tool_result`), A10 (`POST /api/chat` stateless), A16/A17.
|
||||
|
||||
## Objective
|
||||
Every tool result tells the model how many tool calls it has left; when the budget is gone a further attempted call is **rejected instead of reaching the chat** (no tool lines, no raw markup, never an empty answer); the forced final is explicitly coached to answer with what it has or tell the user it can keep going; and the user gets a one-click **Continue researching** action that runs a fresh turn with a whole new budget. `BOR_AGENT_MAX_ROUNDS` stays 10 (owner-locked).
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Telemetry is model-facing (LOCKED):** the remaining count rides the tool-result string the loop already appends as `{"role": "tool", …}` — no new SSE event type, A15 untouched. Trailer constants live with the other teaching strings in `app/rag/agent.py`:
|
||||
`TOOL_BUDGET_REMAINING = "\n\n[tool calls remaining: {n}]"` for `n >= 2`, `… 1 call remaining` wording at `n == 1`, and `TOOL_BUDGET_SPENT = "\n\n[No tool calls remain — answer now from what you have, or tell the user to say \"continue\" to keep researching.]"` at `n == 0`. Appended in `run_agent` (NOT inside `_execute_tool`, which stays a pure `(db, call, seed_docs, holder, settings) → str` seam and keeps its existing unit tests byte-identical), after `rounds += 1`, so a truncated read's `ToolResultPiece` and the `read_coverage` bookkeeping are unaffected.
|
||||
- **`agent_max_rounds = 0` (the kill switch) is untouched:** no tools are offered, so no tool result and no trailer exist — the single `tools=None` request stays byte-identical (the phase-45 pin, `tests/unit/test_agent.py::test_zero_max_rounds_is_one_request_without_tools`).
|
||||
- **The bug class this phase kills:** a request made WITHOUT tools can still come back with structured `tool_calls` deltas. Today those `ToolCallPiece`s become SSE `tool` frames (`app/api/chat.py::_pump`, L909) and render as tool lines in the chat — a round-capped or deflected turn "ending with tool calls dumped into chat", exactly the owner's report. New invariant: **a request with `tools=None` never produces a `tool` frame, and never leaves the answer empty.**
|
||||
- **The cap path (task 02):** the forced final's loop stops yielding `ToolCallPiece`s (they are REJECTED: not run, not shown, not counted in `holder.tool_calls` — the refusal style: "counts in nothing"), counts them in a new `AgentHolder.rejected_calls`, logs one warning (`agent: %d tool call(s) attempted after the round cap — rejected, not shown`), and — when the forced final streamed no visible content — yields the deterministic `ROUND_CAP_FALLBACK_ANSWER` content piece (an honest "I've used up this turn's research budget — here's where I got to; say continue to dig further" line) instead of dying. The model is coached (next bullet) to produce the "say continue to keep going" line itself; `ROUND_CAP_FALLBACK_ANSWER` is the deterministic backstop for the case where it emits only calls or only scaffolding.
|
||||
- **Coaching the forced final (task 02):** `ROUND_CAP_WRAPUP_INSTRUCTION` (a module constant beside `CORRECTION_INSTRUCTION`) is folded into the single system message for the forced final — the exact L2178 pattern (`system_prompt + "\n" + ROUND_CAP_WRAPUP_INSTRUCTION`) — spelling out: no tools remain, NEVER emit tool-call markup or a tool call, answer from the results you already have, and if the answer is incomplete say so and tell the user they can say "continue" to keep researching.
|
||||
- **The same rule for the other tool-less requests (task 02):** `app/api/chat.py::_pump` drops `ToolCallPiece`s while the request had no tools — the deflected turn (A8 `DEFLECT_MODE`, L870) and the kill-switch turn — so the phase-71 incident class is closed at the transport too. The `ScaffoldingFilter` keeps stripping textual markup forms; this task only stops STRUCTURED calls from becoming UI lines.
|
||||
- **User-visible signal (tasks 03/04):** one additive `done` key `budget_exhausted: bool` (serialized ABSENT when false, the `answer_image` omission precedent — every pre-phase frame stays byte-identical), driven by `AgentHolder.cap_reached`, plus the per-turn log fields `rounds=%d` and `cap_hit=%d`. The frontend answers it with a `Continue researching` button that sends a plain next turn (`runTurn("continue")`): the budget is per-turn, so a new turn has `agent_max_rounds` again — **no client-supplied budget in the request body** (a client-tunable round cap is a DoS surface and would break the server-side kill switch, A10-adjacent).
|
||||
- **NOT touched:** `BOR_AGENT_MAX_ROUNDS`'s default and validator, the `tool` SSE event shape (A15), `done.sources`/`related`/`suggestions`/`answer_image`, the retry primitive (A2/phase 67), `ScaffoldingFilter`'s pattern registry (a new form gets an observed capture, not a guess), the phase-71 recovery path's `MalformedReplyError` (non-cap: a deflected/round turn whose SECOND reply is scaffolding-only still errors).
|
||||
|
||||
## Tasks
|
||||
1. `01_remaining_in_tool_results.md` — the budget trailer on every executed tool result (model-facing), + the docstring/env-comment updates.
|
||||
2. `02_reject_calls_at_the_cap.md` — no `tool` frames from a `tools=None` request (the forced final, the deflected turn, the kill switch), `AgentHolder.rejected_calls`, the cap-path fallback answer, and `ROUND_CAP_WRAPUP_INSTRUCTION` on the forced final.
|
||||
3. `03_budget_exhausted_flag_and_log.md` — `AgentHolder.rounds` / `cap_reached`, the additive `done.budget_exhausted` key, the `rounds=` / `cap_hit=` log fields, and the one PLAN.md §9/§4 revision note.
|
||||
4. `04_continue_researching_button.md` — the UI: the button, the persisted record marker, the restore path, CSS.
|
||||
5. `05_budget_tests.md` — the mock's round-cap marker + unit + integration + the isolated `tests/e2e/test_tool_budget_telemetry.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_agent.py` (extend) — the trailer on every tool result (the fake LLM's recorded `messages`: `[n] remaining` decreasing 9, 8, … 1, then the spent line at 0; `agent_max_rounds=1` → the spent line on the only result; `agent_max_rounds=0` → NO tool message at all), the forced final's system message carries `ROUND_CAP_WRAPUP_INSTRUCTION`, a `ToolCallPiece` on the forced final is not yielded and does not reach the API layer (`holder.rejected_calls` counts it, `holder.tool_calls` does not), the cap path with a call-only / scaffolding-only forced final yields `ROUND_CAP_FALLBACK_ANSWER` instead of raising, and the existing cap cluster (`llm.requests[i][1] is None`, the `round cap reached (rounds=N)` warning) still passes.
|
||||
- Integration: `tests/integration/test_chat_api.py` (extend) — a cap-hitting mock turn streams NO `tool` frame after the cap and ends with `done` carrying `budget_exhausted: true`; a normal grounded turn's `done` frame is byte-identical to today (the key is absent, not false); a deflected turn emits no `tool` frame even when the mock returns structured calls.
|
||||
- Coverage: **>90%** on `app/` (AGENTS.md rule 9) — the new branches (cap-path rejection, the fallback answer, the serializer omission) each need a test.
|
||||
- E2E (mandatory, A16): `tests/e2e/test_tool_budget_telemetry.py`, run **in isolation** (`uv run pytest tests/e2e/test_tool_budget_telemetry.py -v --no-cov`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A grounded turn's tool messages carry a decreasing `[tool calls remaining: N]` trailer and the last one says the budget is spent (visible in a live turn's server log or a captured request).
|
||||
- [ ] A turn that hits the cap ends with a readable answer — zero tool lines after the cap, zero raw tool markup in the answer, no empty answer, no `MalformedReplyError` on the cap path.
|
||||
- [ ] `done.budget_exhausted` is `true` exactly on cap-reached turns and ABSENT (never `false`) everywhere else.
|
||||
- [ ] The exhausted answer offers **Continue researching**; clicking it sends one new turn and streams a new answer with a fresh tool budget; earlier answers' buttons are gone.
|
||||
- [ ] A refresh keeps the Continue button on a flagged answer; a shared chat never shows it.
|
||||
- [ ] `agent_max_rounds=0` still sends exactly one `tools=None` request and no trailer; deflected turns are unchanged; `BOR_AGENT_MAX_ROUNDS` default is still 10.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `uv run pytest tests/e2e/test_tool_budget_telemetry.py -v --no-cov` green in isolation (DB up); regressions green in isolation: `test_agent_unlimited_tools.py`, `test_agent_document_tools.py`, `test_chat_rag.py`, `test_harness_aligned_tools.py`, `test_grep_regex_teaching.py`, `test_smoke.py`.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **Owner-locked (2026-10-05): `BOR_AGENT_MAX_ROUNDS` stays 10** — this phase teaches the model and the user about the budget; it does not raise it.
|
||||
- **Owner-locked (2026-10-05): Continue lives on the answer** — a button, not a live counter in the tool lines (the "you have x remaining" half of the TODO is model-facing).
|
||||
- **A15 holds — no new SSE event type:** the count rides the tool result text; the user-facing signal is one additive `done` key (the `related`/`answer_image` precedent, PLAN §4's additive rule).
|
||||
- **The boundary call still runs:** the call the model emitted while budget remained is executed and its result carries `[tool calls remaining: 0]`; only calls attempted AFTER the budget is spent are rejected (never executed, never shown) — no in-flight research is thrown away and the OpenAI wire stays valid.
|
||||
- **Recorded revision of phase 71's cap-path terminal (AGENTS.md rule 3, owner-confirmed 2026-10-05):** a round-capped forced final that carries no visible content no longer raises `MalformedReplyError` — it yields `ROUND_CAP_FALLBACK_ANSWER`, because an honest "budget spent" line beats an error frame for the exact symptom this TODO reports. The non-cap recovery path (a round or deflected reply that is pure scaffolding twice in a row) keeps `MalformedReplyError` unchanged. The PLAN.md revision note in task 03 records it (the phase-45 pattern: a recorded revision, never a silent deviation).
|
||||
- **A10 holds** — Continue is an ordinary stateless `POST /api/chat` with the client's history; no session state, no client-controlled budget, no migration.
|
||||
- **A16/A17 honoured** — one E2E file, one atomic commit.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agents/ app/ frontend/ tests/ && git commit --no-gpg-sign -m "feat(agent): teach the tool-call budget and reject calls past the cap instead of dumping them into chat"
|
||||
```
|
||||
Reference in New Issue
Block a user