feat(chat): stop an in-flight answer — Send becomes Stop, the partial is kept and persisted, the model stream is torn down
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