feat(rag): unbounded agent tool calls behind a round cap (owner revision)
Phase 45 (owner permission 2026-08-27, TODO.md L8: "allow the LLM
to make as many tool calls as it wants"): the phase-37 per-turn tool
budgets (BOR_AGENT_LIST_CALLS / BOR_AGENT_READ_CALLS, default 1 each)
and their exhaustion refusals are removed — a grounded turn now offers
list_documents / read_document for the whole turn (re-lists included),
bounded only by the round cap:
- app/config.py: agent_max_rounds (BOR_AGENT_MAX_ROUNDS, default 10,
negative rejected) replaces agent_list_calls / agent_read_calls;
.env.example + README document the single knob; app/rag/prompts.py
docstrings follow.
- app/rag/agent.py: the loop runs tools until the model answers or
rounds >= max_rounds, at which point it forces one final no-tools
answer (the cap is the only forced exit); 0 = no tools — exactly one
tools=None request, byte-identical to the pre-phase-37 path (the
kill switch). Rejected calls (unknown tool / missing args /
already-in-context / unknown path) still consume a round, so
pathological rejected-call streams are bounded by the cap. The
per-call log line is now tool/args/round=N/M; the per-turn
tool_calls=N field and the tool SSE event are unchanged.
- tests/e2e/mock_llm.py: MULTI_READ_TRIGGER ("read two documents") —
the deterministic list -> read #1 -> read #2 -> forced-answer flow
(byte-stable "I read <sp1> and <sp2>." line), classified by the
count of tool-role read results; the phase-37 single-read flow stays
byte-identical (unit-pinned in tests/unit/test_mock_tool_flow.py).
- tests/e2e/test_agent_unlimited_tools.py (new, story suite,
mock-only): three tool frames/lines in order (one list, two reads —
the second read is what the old read budget refused) + the
both-named non-deflected answer; done.sources + chips = retrieval
doc + both reads, deduped; no budget refusal rendered; the
single-read marker flow regression (exactly one read, single tool
pair).
- .agent/PLAN.md: the phase-45 SSE revision note (owner-locked, R2) —
the only PLAN edit this phase; the phase-37 note's budget clause is
marked removed.
Unit/integration rewrites (test_agent.py round-cap matrix incl. the
kill switch and rejected-call spam, test_config.py, test_chat_api.py
agent_max_rounds=0 fixtures) landed with the server core so every gate
stays green.
uv run pytest: 756 passed, app/ coverage 99%; ruff + pyright clean;
story E2E 4/4 in isolation (ran twice); regression E2E suites
(agent_document_tools unmodified, chat_rag, smoke) green in isolation.
Also records the 45_agent_unlimited_tools todo/ -> complete/ task-file
moves (00/01/02 pending in the working tree, task 03 moves on success).
This commit is contained in:
@@ -1,52 +0,0 @@
|
||||
# Task 01 — Server core: round cap replaces the budgets (config + loop + unit/integration)
|
||||
|
||||
**Phase:** `45_agent_unlimited_tools` · **Source:** `TODO.md:8` — "Allow the LLM to make as many tool calls as it wants, remove the restrictions, they're causing problems getting correct answers"
|
||||
**Story:** `.agent/user_stories/agent-unlimited-tools.md`
|
||||
|
||||
## Objective
|
||||
One coherent server-side change, landed atomically so the suite is green at the checkpoint: `agent_max_rounds` (`BOR_AGENT_MAX_ROUNDS`, default 10; `0` = no tools) replaces both per-tool budgets in config, the agent loop, and every test that pins them.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` —
|
||||
- **delete** the `agent_list_calls` and `agent_read_calls` fields (with docstrings);
|
||||
- **add** in their place, same "RAG tuning" section:
|
||||
```python
|
||||
#: Hard cap on the agent tool rounds per grounded turn (phase 45,
|
||||
#: revising phase 37's per-tool budgets — owner permission
|
||||
#: 2026-08-27, TODO L8: "allow the LLM to make as many tool calls
|
||||
#: as it wants"). Every tool call the model emits consumes a
|
||||
#: round; at the cap the loop forces one final no-tools answer.
|
||||
#: ``0`` disables the tools entirely — the turn is a single
|
||||
#: request with ``tools=None`` (the pre-phase-37 path — the kill
|
||||
#: switch).
|
||||
agent_max_rounds: int = 10
|
||||
```
|
||||
- optional: a `field_validator` rejecting negative values (note it in the docstring if added).
|
||||
2. `app/rag/agent.py` —
|
||||
- `run_agent`: `max_rounds = settings.agent_max_rounds`; `tools = AGENT_TOOLS if max_rounds > 0 else None` (the kill switch — at 0 the loop makes exactly one request with `tools=None`, byte-identical to the pre-phase-37 path);
|
||||
- delete `list_left` / `read_left` and the budget-driven `tools = None if (list_left == 0 and read_left == 0) else AGENT_TOOLS` transition — `tools` stays `AGENT_TOOLS` while rounds remain;
|
||||
- after each executed call: `rounds += 1`; the existing cap branch becomes the **only** forced-exit: `if rounds >= max_rounds:` → the `logger.warning("agent round cap reached …")` + final `chat_stream(messages, tools=None)` (update the warning text: it is no longer belt-and-braces — it is the cap);
|
||||
- `_execute_tool(db, call, seed_docs, holder)`: drop the `list_left` / `read_left` parameters and the `LIST_EXHAUSTED` / `READ_EXHAUSTED` early returns; keep the `ALREADY_IN_CONTEXT`, `UNKNOWN_TOOL`, `MISSING_READ_ARGS` rejections (non-budget — a repeated rejected call still consumes a *round* in the loop, so a pathological stream is bounded by `max_rounds`); return type simplifies to `str`;
|
||||
- delete the `LIST_EXHAUSTED` / `READ_EXHAUSTED` constants;
|
||||
- `AGENT_TOOLS`: `read_document` description "Add the full content of exactly one more indexed document to your context" → "Add the full content of one more indexed document to your context";
|
||||
- module docstring: the budget paragraph (points 1, 3, 4) rewritten for the round cap (owner revision 2026-08-27, `TODO.md` L8); `run_agent` docstring updated (`seed_docs` note unchanged); `AgentHolder` unchanged (`tool_calls` still counts executed calls — now including re-lists);
|
||||
- the per-call `logger.info("agent tool=… budget list_left=… read_left=…")` line becomes `logger.info("agent tool=%s args=%s round=%d/%d", …)` (or equivalent — the per-turn `tool_calls=N` field in `app/api/chat.py` is untouched).
|
||||
3. `tests/unit/test_agent.py` — **rewrite** the budget tests around the round cap (keep the file's fake-LLM harness):
|
||||
- an always-`list_documents`-calling mock with `agent_max_rounds=3`: exactly 3 tool rounds execute, then one forced `tools=None` request streams the answer; `holder.tool_calls == 3`;
|
||||
- `agent_max_rounds=0`: exactly one request, `tools=None`, no tool lines, `holder.tool_calls == 0` (kill switch);
|
||||
- an always-`read_document`-with-unknown-path mock (every call rejected — `No document at …`): the loop runs to `max_rounds` and forces the final answer (rejections no longer end the loop early via budgets, the cap bounds them);
|
||||
- the existing rejections tests (`Unknown tool.`, `MISSING_READ_ARGS`, `Already in your context.`) keep passing — update their `_settings(...)` calls (`agent_max_rounds=…` instead of the budget kwargs);
|
||||
- a **re-list** test: `list_documents` called twice in one turn executes both (the second returns the catalog again) and counts 2 in `holder.tool_calls`.
|
||||
4. `tests/unit/test_config.py` — default 10; `BOR_AGENT_MAX_ROUNDS=0` / `=5` overrides; (negative validator, if added); delete the old budget assertions.
|
||||
5. `tests/integration/test_chat_api.py` — the `agent_list_calls=0, agent_read_calls=0` fixture kwargs (~line 661) become `agent_max_rounds=0`; any other budget kwarg in the file the same; the tool SSE-event and `done.sources` assertions stay untouched.
|
||||
6. `.env.example` — the two `BOR_AGENT_*_CALLS` lines become one: `# BOR_AGENT_MAX_ROUNDS=10 # hard cap on agent tool rounds per turn (0 = no tools)` (file's optional-setting comment style).
|
||||
7. Grep the repo for `agent_list_calls|agent_read_calls|BOR_AGENT_(LIST|READ)_CALLS|LIST_EXHAUSTED|READ_EXHAUSTED` — zero hits outside `.agent/phases/complete/**` (history).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit + integration: full `uv run pytest` green at this checkpoint (the mock/E2E multi-read flow lands in task 02 — the existing 3-step mock flow still works unmodified, so `test_agent_document_tools.py` E2E is not yet run by the gate).
|
||||
- Coverage: **>90%** on `app/` — `agent.py` + `config.py` fully covered by the rewritten tests.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] No per-tool budgets anywhere in `app/` or `tests/`; `agent_max_rounds` is the single knob (default 10, `0` = kill switch).
|
||||
- [ ] Re-lists execute; non-budget rejections intact; the cap bounds pathological streams; `tool_calls=N` log field and `tool` SSE event unchanged.
|
||||
- [ ] `uv run pytest` + coverage gate green at this checkpoint.
|
||||
@@ -1,27 +0,0 @@
|
||||
# Task 02 — Mock: deterministic multi-read tool flow
|
||||
|
||||
**Phase:** `45_agent_unlimited_tools` · **Source:** `TODO.md:8` — "Allow the LLM to make as many tool calls as it wants, remove the restrictions, they're causing problems getting correct answers"
|
||||
**Story:** `.agent/user_stories/agent-unlimited-tools.md`
|
||||
|
||||
## Objective
|
||||
The E2E mock gains a deterministic **multi-read** agent flow (list → read #1 → read #2 → answer) so "as many tool calls as it wants" is provable statelessly, without disturbing the existing 3-step flow.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` —
|
||||
- the existing phase-37 flow (documented in the module docstring and `_tool_flow`): marker `TOOLS_TRIGGER` ("use your tools") + `<tools>` system section → step classification **statelessly from the messages**: no tool results yet → `list`; one `tool`-role message with the catalog prefix → `read` (first catalog doc, parsed from the listing via the `rsplit("/", 1)` convention); one `tool`-role message with the `Document <source/path>:` prefix → forced answer.
|
||||
- add a **multi-read variant**: when the user message contains **both** `TOOLS_TRIGGER` and a new marker `MULTI_READ_TRIGGER = "read two documents"`, the classifier reads the *count* of `tool`-role messages whose content starts with `"Document "` (the read-result prefix, `app.rag.agent`'s `_execute_tool` output):
|
||||
- 0 read results (+ no catalog yet) → `list`;
|
||||
- 0 read results (catalog present) → `read` the **first** catalog doc;
|
||||
- 1 read result → `read` the **second** catalog doc (the listing minus the already-read doc — parse the catalog lines the same way the existing read step does, skipping the path already read);
|
||||
- 2 read results → forced answer: the existing answer shape (tail echo) plus a deterministic line naming **both** read paths (e.g. `"I read <path1> and <path2>."` — byte-stable) so the E2E can assert the model actually used both;
|
||||
- the single-read flow (no `MULTI_READ_TRIGGER`) stays byte-identical — the variant must be a strict superset (the existing `test_agent_document_tools.py` E2E keeps passing unmodified).
|
||||
- update the module docstring's tool-flow documentation (the multi-read steps + the marker).
|
||||
2. `uv run pytest` green (mock-only change; the existing 3-step E2E is not run by the unit gate but must stay conceptually intact — the regression run in task 03 proves it).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: full suite green; if a mock-specific unit test file exists (check `tests/unit/`), add the multi-read classification case there (catalog → read #1 → read #2 → answer) so the new branch is unit-covered; otherwise the E2E (task 03) covers it.
|
||||
- Coverage: **>90%** on `app/` (unchanged — `tests/` only).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `TOOLS_TRIGGER` + `MULTI_READ_TRIGGER` → deterministic 4-step flow (list, read #1, read #2, answer naming both paths); the 3-step flow is unchanged for marker-less turns.
|
||||
- [ ] Full unit/integration suite green.
|
||||
Reference in New Issue
Block a user