docs(phases): convert TODO L3/L4 into phases 159 (edit & resend) and 160 (tool-call budget)
TODO.md's two open items become executable phase directories under .agents/phases/todo/: - 159_edit_and_resend_message (5 tasks): inline edit affordance on the last user bubble, in-place resend that truncates the replaced turn, an editable error turn after an oversize question, and the Playwright suite tests/e2e/test_edit_and_resend_message.py. - 160_tool_budget_telemetry (5 tasks): remaining tool-call count in every tool result, tool calls rejected (never shown) once the budget is spent, the additive done.budget_exhausted flag + rounds=/cap_hit= log fields, the Continue researching affordance, and tests/e2e/test_tool_budget_telemetry.py. No application code changed; the TODO lines are recoverable from git history. LLM-Generated: true
This commit is contained in:
@@ -0,0 +1,58 @@
|
||||
# Phase 159 — Edit the last question and re-ask it
|
||||
|
||||
**Source:** `TODO.md` L3 — "Need to be able to edit and resend a message. Got 'FailedQuestion too long — trim it and re-ask' but I can't trim it because I can't edit it."
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-10-05)
|
||||
**Context:** `frontend/assets/app.js` (4,007 lines) — `getActionsRow(wrap)` (L498, the `.msg-meta.msg-actions` `role=list` row every bubble action joins as a `listitem`), `appendTuneButton` (L517) + `openTuneForm` (L866, the inline-form precedent: `form.tune-form` + labelled `textarea[maxlength]` + `.tune-form-actions` Save/Cancel + a `role=alert` status line + "one form at a time" via `document.querySelectorAll(".tune-form").forEach(f => f.remove())`), `appendRetryButton` (L617) + `markLastRetryable()` (L650, the single-target discipline: remove every `.retry-btn`, re-append on exactly one bubble, gated on the record invariant), `addMessage(who, html, scroll, image)` (L969, the ONE bubble renderer — `.msg.<who> > .avatar + .msg-body > .bubble`), `renderStoredMessage(m)` (L2254; the user branch L2262 renders `renderMarkdown(m.text)` + the optional `m.image`), `restoreConversation()` (L2410; ends with `markLastRetryable()`), `MAX_QUESTION_CHARS = 4000` (L1689, mirrors `ChatRequest.message max_length=4000`) + `updateCharCount()` (L1702, the `len/4000 — character limit` copy), `conversation` + `trimToBudget` (L2128) + `saveConversation()` (L2143, localStorage `bor.chat.v1`) + `persistConversation()` (L2532, the phase-55 auto-save), `retryLastTurn(wrap)` (L2984, the redo-in-place precedent: pop the last brain record, keep the question as the last entry, `runTurn(text, { reask: true })`), `runTurn(text, { reask, image })` (L3260; `reask` skips the user append + save point 1, `history = conversation.slice(0, -1)`), `finalizeFailedTurn` (L3209, phase 120), `UI_STATE` + `setUiState` (the thinking/streaming in-flight gate every action obeys), `lastBrainWrap`.
|
||||
Server side (read-only for this phase): `app/schemas.py` `ChatRequest.message` (`max_length=4000`, L77) and `ChatMessage.text` (`max_length=32_000`, L1264, `extra="forbid"`), `app/api/chats.py` (the raw `bor.chat.v1` `messages` JSONB round-trip — logic untouched), `app/api/chat.py` L705–733 (the phase-114 terminal `error` frame `detail="Question too long — trim it and re-ask."` + its `hint` — deterministic, no retry frames: the exact turn this phase gives a remedy for).
|
||||
CSS: `frontend/assets/styles.css` `.msg-meta.msg-actions` (L795), `.tune-btn` (L980), `.retry-btn` (L1010, the pill family), `.tune-form` + `label` + `textarea` + `.tune-form-actions` (L1116–1141), the mobile `min-height: 44px` action rules (L5708–5709).
|
||||
|
||||
## Objective
|
||||
The last question the user sent carries an **Edit** action: click it, the bubble becomes a textarea prefilled with the question, trim it, Save — the answer is re-asked from the edited text. A "Question too long — trim it and re-ask" turn (or any mistyped / half-formed question) is fixed by editing, never by retyping.
|
||||
|
||||
## Dependencies
|
||||
- `158_thinking_timeline` (complete) — pipeline predecessor (execution order) only.
|
||||
- Code dependencies (all complete): phase 49/53 redo-in-place retry (`retryLastTurn` + `runTurn({reask:true})`), phase 120 failed-turn records (`appendFailedNote`, `finalizeFailedTurn` — the driver case leaves a failed brain record after the question), phase 14 persistence (`conversation` / `saveConversation`), phase 55 auto-save (`persistConversation`), phase 130 `.msg-actions` row, phase 114 the "question too long" error frame, phase 123 the user-bubble image (`m.image`).
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Target set (LOCKED, owner 2026-10-05): exactly ONE editable bubble** — the most recent user message. `markLastEditable()` mirrors `markLastRetryable()`: remove every `.edit-btn` first, then append to the last user bubble, but only when the last **user** record exists in `conversation` (scan newest-first for `who === "user"`; the records after it are the answers it produced — that index is `editIdx`). The bubble itself is tracked in a new `lastUserWrap` module variable (the exact mirror of `lastBrainWrap`, assigned in `runTurn`'s fresh-send `addMessage("user", …)` and in `renderStoredMessage`'s user branch, cleared by `startNewChat()`). Earlier bubbles are never editable, so no mid-session branch exists and the "what happens to the turns after it" question never arises for an old message.
|
||||
- **Editing happens IN the bubble (LOCKED, owner 2026-10-05)** — `openEditor(wrap, idx)` is the `openTuneForm` family: `form.edit-form` in the bubble's `.msg-body`, `<label for="edit-input-N">`, `textarea[maxlength="4000"]` prefilled from `conversation[idx].text`, Save (submit) + Cancel, a `role=alert` `.edit-error` line. The rendered `.bubble` is hidden (`.is-editing`) while the form is open; Cancel / `Escape` restores it and returns focus to the toggle button; `Ctrl/Cmd+Enter` submits; only one editor open at a time.
|
||||
- **Save = replace + truncate + re-ask (LOCKED)** — the edited text replaces `conversation[editIdx].text` in place, **every record after it is dropped** (the answers to the old text are stale by construction), `saveConversation()` + `persistConversation()` persist the new shape, the dropped bubbles are removed from the DOM, `lastBrainWrap = null`, `markLastRetryable()` re-targets, then `runTurn(newText, { reask: true })`. Because the edited question ends up as the LAST record, `runTurn`'s `history = conversation.slice(0, -1)` invariant and `reask` (skip the user append + save point 1) hold **unchanged** — this phase adds no API, no schema field, no migration (**A10 stateless holds**).
|
||||
- **Empty guard:** a whitespace-only save is rejected in-form (the `.edit-error` line, the form stays open) — `ChatRequest.message` / `ChatMessage.text` both require `min_length=1`; the >4000 case is unreachable because the textarea carries `maxlength`.
|
||||
- **The image survives, the re-ask is text-only** (the phase-49/123 A7 rule): the record keeps its `image` key and its bubble keeps the attachment; `runTurn(newText, { reask: true })` sends no `image`, exactly like `retryLastTurn`.
|
||||
- **In-flight discipline:** `markLastEditable()` no-ops while `uiState` is `thinking`/`streaming` (the same guard `retryLastTurn` opens with), and `saveEditor` re-checks it — a save can never start a second turn.
|
||||
- **NOT touched:** `runTurn` (its `reask` path is consumed as-is), `retryLastTurn`, `handleSend`, `rememberBrainTurn`, `trimToBudget`/`persistConversation` (called, not changed), `app/` server code, the SSE contract (A15), the shared-chat page.
|
||||
|
||||
## Tasks
|
||||
1. `01_edit_affordance_on_user_bubbles.md` — the Edit button (`appendEditButton` / `markLastEditable`) + the inline editor (`openEditor`, Save updates the record + the bubble text + localStorage only).
|
||||
2. `02_resend_in_place.md` — Save also truncates the records after the edited question (DOM + `conversation`) and re-asks through `runTurn({reask:true})`; the in-flight and empty guards.
|
||||
3. `03_editable_error_turn.md` — the driver case: the question stays editable when its turn failed (the "Question too long" frame, the phase-120 failed record, a stopped answer) and after a refresh.
|
||||
4. `04_edit_resend_tests.md` — unit source-pins + the `test_chats_api` edited-record round-trip + the isolated `tests/e2e/test_edit_and_resend.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (house style = source pins on `frontend/assets/app.js`, e.g. `tests/unit/test_frontend_failed_turn.py`): the `.edit-btn` join-the-actions-row rule, `markLastEditable`'s remove-then-append single target, the inline form's labelled textarea + `maxlength=4000` + Cancel/`Escape`/`Ctrl+Enter` paths, the replace-and-truncate record edit, the `runTurn(text, { reask: true })` call, the in-flight no-op, and `retryLastTurn`/`runTurn` byte-unchanged.
|
||||
- Integration: `tests/integration/test_chats_api.py` (extend) — a `PUT /api/chats/<id>` whose last user record carries the edited text and whose later records were dropped round-trips byte-identically (no schema change is required; this pins the claim).
|
||||
- Coverage: **>90%** on `app/` (this phase changes no `app/` code — the gate must stay green, not drop).
|
||||
- E2E (mandatory, A16): `tests/e2e/test_edit_and_resend.py`, run **in isolation** (`uv run pytest tests/e2e/test_edit_and_resend.py -v --no-cov`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The most recent user bubble carries Edit; opening it shows a textarea prefilled with the question; Cancel/`Escape` restores the original bubble text.
|
||||
- [ ] Saving trimmed text updates the bubble + the `bor.chat.v1` record in place, drops every turn after the question (DOM + storage + the saved chat), and streams a fresh answer from the edited text with no duplicated question.
|
||||
- [ ] A refresh after an edit shows the edited question and the new answer (no stale tail resurrected).
|
||||
- [ ] The "Question too long — trim it and re-ask" turn's question is editable, so trimming + Save re-asks successfully.
|
||||
- [ ] Retry, Stop, the failed-turn path, and every successful turn behave byte-identically to before; earlier user bubbles show no Edit button.
|
||||
- [ ] `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_edit_and_resend.py -v --no-cov` green in isolation (DB up); regressions green in isolation: `test_retry_answer.py`, `test_failed_turn_retry.py`, `test_chat_persistence.py`.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **Owner-locked (2026-10-05, roadmap confirmation): last question only** — one editable bubble, managed by the `markLastRetryable()` single-target discipline. Mid-conversation branching is explicitly out of scope.
|
||||
- **Owner-locked (2026-10-05): inline edit in the bubble** — the `.tune-form` family, not the composer (the composer keeps its own draft untouched by editing).
|
||||
- **Save truncates the tail and re-asks** (the phase-49 redo-in-place rule, one level up: the question is replaced instead of kept verbatim) — no message versioning, no "edited" history trail, one new API-free flow.
|
||||
- **A10 untouched** — `POST /api/chat` stays stateless; the edited question is just the client's next `message`. No migration, no `ChatMessage` field (the ≤4000 / ≤32 000 caps already cover it).
|
||||
- **Edit is available to every visitor who can chat** — no admin gate, the phase-49 Retry precedent.
|
||||
- **A16/A17 honoured** — one E2E file, one atomic commit.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agents/ frontend/ tests/ && git commit --no-gpg-sign -m "feat(chat): edit the last question and re-ask it — fixes trim-and-re-ask with no way to trim"
|
||||
```
|
||||
@@ -0,0 +1,37 @@
|
||||
# Task 01 — Edit button + inline editor on the last question
|
||||
|
||||
**Phase:** `159_edit_and_resend_message` · **Source:** `TODO.md:3` — "Need to be able to edit and resend a message. Got 'FailedQuestion too long — trim it and re-ask' but I can't trim it because I can't edit it."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The most recent user bubble carries an **Edit** action that swaps the bubble for a textarea prefilled with the question; Save writes the new text into the bubble and the `bor.chat.v1` record, Cancel restores the original. (The re-ask itself lands in task 02 — this task ships shippable: an editable question that persists.)
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — add `EDIT_ICON` beside `RETRY_ICON` (L606) and `TUNE_ICON` (L475): an inline `aria-hidden` pencil SVG in the same stroke family (`fill="none" stroke="currentColor" stroke-width="1.7"`, 24×24 viewBox) — no icon font, no CDN (A11).
|
||||
2. `frontend/assets/app.js` — `appendEditButton(wrap)`, directly after `appendRetryButton` (L617) and shaped exactly like it: `getActionsRow(wrap)` → bail if the row is missing → one-per-bubble guard on `.edit-btn` → `btn.type = "button"`, `btn.className = "edit-btn"`, `role="listitem"` when the row reports `role="list"`, `btn.innerHTML = EDIT_ICON + "<span>Edit</span>"`, click → `openEditor(wrap, btn)`. A visible "Edit" label carries the accessible name (no ellipsis clipping needed at this width).
|
||||
3. `frontend/assets/app.js` — `markLastEditable()`, the sibling of `markLastRetryable()` (L650): `messagesEl.querySelectorAll(".edit-btn").forEach(b => b.remove())` FIRST (an older bubble's button is stale the moment a newer question lands), then bail when `uiState === UI_STATE.thinking || uiState === UI_STATE.streaming` (the `retryLastTurn` in-flight guard, L2985), then scan `conversation` newest-first for `who === "user"` through a small helper `lastUserRecordIndex()` (returns `-1` when the conversation holds no user record); bail on `-1`; append the button on `lastUserWrap`. Task 02 reuses `lastUserRecordIndex()` for the truncation.
|
||||
- ASSUMPTION: the last-user-record index comes from that newest-first `conversation` scan (the `markLastRetryable` last-brain-index rule, one `who` over), NOT from a DOM↔record index map — the DOM and the record list are parallel today but re-deriving the index from the record keeps the editor correct after a task-02 truncation.
|
||||
- The wrap is tracked in a new module variable `lastUserWrap`, the exact mirror of `lastBrainWrap`: assigned where a user bubble is created — `runTurn`'s fresh-send `addMessage("user", …)` (L3270) and `renderStoredMessage`'s user branch (L2262, which is what makes the restore path work for free) — cleared by `startNewChat()` (L2886). `markLastEditable()` appends only when `lastUserWrap` exists AND its record really is the last user record (the `editIdx` scan above), so a stale wrap can never carry the button. Task 02 reuses the same variable.
|
||||
- Call sites — pair every existing `markLastRetryable()` call (there are exactly six: `restoreConversation()` L2415, `restoreSavedChatFromUrl()` L2482, `finalizeFailedTurn()` L3247, the `done` settle L3649, the zero-frame fallback L3734, the stop finalize L3769): call `markLastEditable()` immediately after each. `startNewChat()` (L2886) needs no call: resetting `messagesEl` removes the buttons with the list (the phase-49 reasoning). Task 03 verifies the error-path pairs (L3247 / L3734 / L3769) end-to-end.
|
||||
4. `frontend/assets/app.js` — `openEditor(wrap, toggleBtn)`, the `openTuneForm` (L866) family, module-level `editSeq` for unique ids (the `tuneSeq` precedent):
|
||||
- `document.querySelectorAll(".edit-form").forEach(f => f.remove())` — one editor at a time.
|
||||
- `form.className = "edit-form"`, `form.noValidate = true`; `<label for="edit-input-N">Edit your question</label>`; a `textarea` with `id="edit-input-N"`, `rows="3"`, `maxlength="${MAX_QUESTION_CHARS}"` (L1689 — the 4000 cap stays single-sourced), `value = conversation[editIdx].text` assigned with `.value` (never an HTML string — the house no-injection rule), `placeholder` reused from the composer if one exists.
|
||||
- `.edit-form-actions` with a `submit` `.edit-save` ("Save") and a `type="button"` `.edit-cancel` ("Cancel"); a `<p class="edit-error" role="alert" hidden>` for the empty-text rejection.
|
||||
- While open: `wrap.querySelector(".bubble")` gets `.is-editing` (CSS-hides the rendered question so the question appears ONCE); on close the class is removed.
|
||||
- Keys: `Escape` in the textarea → cancel; `Ctrl/Cmd+Enter` → submit; cancel restores the bubble, removes the form, returns focus to `toggleBtn` (the `openTuneForm` cancel rule).
|
||||
- Submit (`e.preventDefault()`): re-check the in-flight guard (no-op), `const text = textarea.value.trim()`; empty → set the `.edit-error` copy ("Ask something first — the question can't be empty."), unhide, keep the form open, return; otherwise this task's scope: `conversation[editIdx].text = text`, `saveConversation()`, re-render the bubble text through `renderMarkdown(text)` (the `addMessage` renderer, so markdown rules stay in one place), close the editor, announce through the existing `#steering-announcer`-style pattern only if a generic announcer already exists — otherwise the visible re-render is the feedback (AGENTS.md rule 10 is satisfied by the bubble change; do NOT invent a new live region).
|
||||
- Leave a clearly-marked `// task 02: truncate the tail + re-ask` seam where the close happens, so the next task adds the resend without restructuring.
|
||||
5. `frontend/assets/styles.css` — `.edit-btn` in the `.retry-btn` family (L1010: same pill size/spacing, the global `:focus-visible` ring applies — verify it is not scoped to `.tune-btn`/`.retry-btn` only and extend the selector list if it is), `.edit-btn:hover` in the same tint; `.msg.user .bubble.is-editing { display: none; }`; the `.edit-form` card, label, textarea and actions row cloned from the `.tune-form` block (L1116–1141) and `.edit-error` cloned from `.tune-error`; add `.edit-btn { min-height: 44px; }` beside `.tune-btn`/`.retry-btn` in the mobile block (L5708–5709) and honor `prefers-reduced-motion` if the family transitions anything.
|
||||
6. `frontend/index.html` — no change (the chat shell, the `.messages` live region and the composer already exist). Confirm no new `<script>`/`<link>` is introduced (AGENTS.md rule 6, the no-CDN test).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_frontend_edit_message.py` (new) — house-style source assertions: `.edit-btn` is appended through `getActionsRow` and joins `role=list` as `listitem`; `markLastEditable` removes before appending and bails on `UI_STATE.thinking`/`streaming`; `openEditor` renders a `<label for>` + `textarea[maxlength]` bound to `MAX_QUESTION_CHARS` and prefills from `conversation`; `Escape`/Cancel restore, `Ctrl/Cmd+Enter` submits; an empty trim shows `.edit-error` and keeps the form; the bubble text is re-rendered with `renderMarkdown`; `retryLastTurn` and `runTurn` are byte-unchanged (read the file and pin the two function bodies' current text).
|
||||
- Coverage: **>90%** on `app/` — unchanged (frontend-only task).
|
||||
- Manual check (visible browser or `page.goto` in a scratch Playwright run): send a question, Edit → Cancel leaves the original text; Edit → Save updates the bubble and survives a reload.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `grep -n "edit-btn\|edit-form\|markLastEditable" frontend/assets/app.js frontend/assets/styles.css` shows the button, the form and the CSS family.
|
||||
- [ ] Exactly one bubble (the newest question) shows Edit at rest; after a new send the button moved to the new bubble.
|
||||
- [ ] Save updates the bubble text and the `bor.chat.v1` localStorage record; a reload shows the edited question.
|
||||
- [ ] Cancel / `Escape` leave the question byte-identical; an empty trim is refused in-form.
|
||||
- [ ] `uv run pytest` green; coverage TOTAL unchanged (>90%); `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Task 02 — Save re-asks: replace the question, drop the stale tail, stream the new answer
|
||||
|
||||
**Phase:** `159_edit_and_resend_message` · **Source:** `TODO.md:3` — "Need to be able to edit and resend a message. Got 'FailedQuestion too long — trim it and re-ask' but I can't trim it because I can't edit it."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
Saving an edited question **resends** it: the answers that followed the old text are removed (DOM + `conversation` + the saved chat), the edited question becomes the last record, and a fresh answer streams from it — the phase-49 redo-in-place semantics one level up (the question is replaced instead of kept verbatim).
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — in `openEditor`'s submit handler (task 01's seam), after the record text is written and BEFORE the editor closes:
|
||||
- `const editIdx = lastUserRecordIndex()` (the newest-first `who === "user"` scan from task 01) and `conversation.splice(editIdx + 1)` — every record the OLD question produced is dropped outright (never rewritten, the phase-114 whole-turn rule).
|
||||
- Remove the matching DOM bubbles: every `.msg` after `lastUserWrap` in `messagesEl` (`let n = lastUserWrap.nextSibling; while (n) { const next = n.nextSibling; n.remove(); n = next; }`).
|
||||
- `lastBrainWrap = null` (the removed answer is no longer a retry target) and clear `leavePartialIndex` / the phase-73 pagehide partial pointer if it points past `editIdx` (a partial saved from the dropped turn must not be re-persisted into the new shape — read `runTurn`'s `leavePartialIndex` usage before deciding the exact reset; `= -1` is the safe value).
|
||||
- `saveConversation()` (localStorage, `trimToBudget` rides it unchanged) + `persistConversation()` (the phase-55 auto-save pushes the truncated shape to `PUT /api/chats/<id>` — fire-and-forget, never blocks the turn).
|
||||
- `markLastRetryable()` — the Retry button re-targets the (now removed) tail: after the drop there is no brain record, so the call simply clears every `.retry-btn`.
|
||||
2. `frontend/assets/app.js` — same handler, then `closeEditor()` and `return runTurn(text, { reask: true })`:
|
||||
- `reask: true` is the whole point — `runTurn` (L3260) skips the user-bubble append and save point 1, so the question is never duplicated; `history = conversation.slice(0, -1)` (L3340) is exactly the prior turns because the edited question is the LAST record.
|
||||
- Return the promise (the `retryLastTurn` L2984/`regenerateStaleChat` convention — `runTurn` never rejects; a failure lands in the phase-120 failed-turn path).
|
||||
- No `image` is passed: a re-ask sends text only (the phase-49/123 locked A7 rule); the record keeps its `image` key and the bubble keeps its attachment.
|
||||
3. `frontend/assets/app.js` — guards, in this order at the top of the submit handler: in-flight (`uiState === UI_STATE.thinking || UI_STATE.streaming` → no-op, a save can never start a second turn), then the empty trim (`Ask something first — the question can't be empty.` in `.edit-error`, form stays open). Re-check the in-flight state a second time immediately before `runTurn` (the same double-check `retryLastTurn` gets by being a single synchronous stretch — keep the handler synchronous from the guard to the `runTurn` call except for nothing: no `await` between the guard and the call).
|
||||
4. `frontend/assets/app.js` — keep the composer out of it: editing never touches `input.value`, `autoGrow()` or `updateCharCount()` (the user's unsent draft survives an edit), and `MAX_QUESTION_CHARS` stays the textarea's `maxlength` so a >4000 trim is unreachable server-side (A10: the server's `ChatRequest.message max_length=4000` remains the real bound).
|
||||
5. No server change: `app/api/chat.py` sees an ordinary `POST /api/chat` with the edited `message` and a shorter `history`; `app/api/chats.py` stores the shorter `messages` array through the existing `SavedChatUpdate` path (no new field, no migration).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_frontend_edit_message.py` (extend) — the splice is `conversation.splice(editIdx + 1)` (the whole tail, nothing before it), the DOM removal starts at `lastUserWrap.nextSibling`, `lastBrainWrap = null` precedes the re-ask, `saveConversation()` + `persistConversation()` both run before `runTurn`, and the call is exactly `runTurn(text, { reask: true })` with no `image`; the in-flight guard precedes the record edit; `retryLastTurn` stays byte-unchanged.
|
||||
- Integration: `tests/integration/test_chats_api.py` (extend) — a `PUT /api/chats/<id>` carrying the edited last user record and a truncated `messages` list round-trips byte-identically (pins "no schema change is needed"; a shortened array is already legal).
|
||||
- Coverage: **>90%** on `app/` (no `app/` code changes — the gate must hold).
|
||||
- Manual: ask a question, get an answer, Edit → change one word → Save: the old answer disappears, the new answer streams in its place, the question appears once.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Save removes every turn after the edited question (DOM + `bor.chat.v1` + the saved-chat row) and streams exactly one fresh answer from the edited text.
|
||||
- [ ] The question is never duplicated in the DOM, in localStorage, or in the request `history`.
|
||||
- [ ] Retry on the new answer, Stop during the new turn, and "New chat" all behave as before.
|
||||
- [ ] Editing while a turn is in flight is impossible (no Edit button, and a queued Save no-ops).
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Task 03 — The driver case: a failed or rejected question stays editable
|
||||
|
||||
**Phase:** `159_edit_and_resend_message` · **Source:** `TODO.md:3` — "Need to be able to edit and resend a message. Got 'FailedQuestion too long — trim it and re-ask' but I can't trim it because I can't edit it."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The exact turn the owner hit — the terminal `error` frame `Question too long — trim it and re-ask.` — leaves its question **editable**, so trimming + Save re-asks successfully; the same holds for every phase-120 failed turn, a stopped partial, and after a page refresh.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — verify and, where missing, add the `markLastEditable()` pairing on the paths that settle WITHOUT a `done` frame, so the question bubble always ends up with its Edit button once the turn is terminal:
|
||||
- the `runTurn` non-OK-HTTP path (L3359: a 422 from `ChatRequest` — e.g. `message` over 4000 — throws with the server `detail` and lands in the error catch) and the SSE `error` frame path (the phase-114 "Question too long" frame arrives as an `error` event, L3675/L3806) — both settle through the error catch → `finalizeFailedTurn` (L3209), which already calls `markLastRetryable()` at L3247: `markLastEditable()` must be the very next statement;
|
||||
- the zero-frame fallback (L3734) and the phase-48 stop finalize (L3769) — same pairing;
|
||||
- the 300s guard abort (`armTurnTimeout`, L3283 → `setUiState(UI_STATE.error, …)`) — confirm the catch's `aborted` branch reaches a `markLast*()` call; if it settles without one, call `markLastRetryable()` + `markLastEditable()` there too (a timed-out turn must not strand the question).
|
||||
2. `frontend/assets/app.js` — the error state must not lock the editor: `markLastEditable()`'s guard bails only on `UI_STATE.thinking` / `UI_STATE.streaming`, never on `UI_STATE.error` / `UI_STATE.idle` (a failed turn IS idle-or-error at rest — read `setUiState` and pin the two states the guard checks). `saveEditor`'s in-flight guard mirrors it exactly.
|
||||
3. `frontend/assets/app.js` — the failed answer record is part of the stale tail: after an edit-save the phase-120 `{ who: "brain", failed: true, error }` record is dropped by task 02's `conversation.splice(editIdx + 1)` like any other answer, and its bubble (with `.failed-note`) is removed with it. Confirm `lastBrainWrap = null` happens on that path so no stale Retry target survives, and that `rememberBrainTurn`'s internal `trimToBudget`-driven index shift (its `brainIdx -= trimDrop` bookkeeping, L2861) can never be confused by the truncation (the truncation happens before the new turn starts, so the new turn's `rememberBrainTurn` sees the post-splice array — add the one-line comment saying so).
|
||||
4. `frontend/assets/app.js` — the restore paths: a reload after a failed turn renders the failed bubble (phase 120) and the question above it; `renderStoredMessage`'s user branch sets `lastUserWrap` (task 01) and the paired `markLastEditable()` after `markLastRetryable()` (L2415 / L2482) puts Edit back on that question. Verify the shared-chat page (`?chat=` admin restore) does NOT get the button for a read-only view — ASSUMPTION: the `?chat=` restore is an editable working copy exactly like the local restore (it already is Retry-able, so Edit follows); if the code shows a genuinely read-only render path, gate on it and record the finding here.
|
||||
5. `frontend/assets/app.js` — the banner: `showErrorBanner(detail, opts)` (L2918) is NOT changed (the phase-120 rule — its `opts.retryable && lastBrainWrap` condition stays byte-identical). Edit is an in-bubble affordance beside Retry, never a banner control.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_frontend_edit_message.py` (extend) — `markLastEditable()` follows every `markLastRetryable()` call site (assert the six pairings by counting both calls), the guard names only `UI_STATE.thinking`/`UI_STATE.streaming` (no `UI_STATE.error`), `showErrorBanner` is byte-unchanged, and the failed-turn splice claim (a `failed: true` record after the edited question is dropped, not kept).
|
||||
- Integration: none new (the phase-120 failed-record round-trip in `tests/integration/test_chats_api.py` already covers the shape this task reads).
|
||||
- Coverage: **>90%** on `app/` (unchanged — no `app/` change).
|
||||
- Manual: with the mock LLM's embed-failure trigger (`tests/e2e/mock_llm.py` `EMBED_FAIL_TRIGGER`, L1321) or a >4000-char question, confirm the error frame's question is editable and a trimmed Save answers successfully.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] After the "Question too long — trim it and re-ask." error: the question bubble shows Edit, the failed bubble shows Retry, trimming + Save produces a real answer.
|
||||
- [ ] The same holds for a network-error turn, a mid-stream drop, a stopped answer, and a 300s timeout.
|
||||
- [ ] A page refresh after any of the above restores the question with a working Edit.
|
||||
- [ ] `showErrorBanner` and the phase-120 failed-record shape are byte-unchanged.
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,38 @@
|
||||
# Task 04 — Tests for edit-and-resend (unit + integration + isolated E2E)
|
||||
|
||||
**Phase:** `159_edit_and_resend_message` · **Source:** `TODO.md:3` — "Need to be able to edit and resend a message. Got 'FailedQuestion too long — trim it and re-ask' but I can't trim it because I can't edit it."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The phase's contract is pinned: one editable bubble, the inline editor, replace-and-truncate-and-re-ask, the editable failed turn, and byte-identical Retry/Stop/persistence behavior — with the phase's own Playwright file run in isolation (AGENTS.md rule 9).
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_frontend_edit_message.py` (new) — house-style source pins against `frontend/assets/app.js` (+ `styles.css`), one focused assertion cluster per behavior:
|
||||
- `EDIT_ICON` exists and is an `aria-hidden` inline SVG; `.edit-btn` is created in `appendEditButton` through `getActionsRow` with the one-per-bubble guard and the `role=list` → `listitem` rule;
|
||||
- `markLastEditable()` removes `.edit-btn` before appending, bails on `UI_STATE.thinking`/`UI_STATE.streaming` (and NOT on `UI_STATE.error`), uses `lastUserRecordIndex()` + `lastUserWrap`, and is called after all six `markLastRetryable()` call sites;
|
||||
- `openEditor`: `<label for>` + `textarea` + `maxlength` bound to `MAX_QUESTION_CHARS`, prefilled from `conversation`, `.edit-form` uniqueness, `Escape`/Cancel restore, `Ctrl/Cmd+Enter` submit, the empty-trim `.edit-error` branch;
|
||||
- save path order: guard → `conversation[editIdx].text = text` → `conversation.splice(editIdx + 1)` → DOM removal from `lastUserWrap.nextSibling` → `lastBrainWrap = null` → `saveConversation()` → `persistConversation()` → `runTurn(text, { reask: true })` (no `image` key);
|
||||
- regressions pinned as byte-unchanged: `retryLastTurn`, `runTurn`, `showErrorBanner`, `handleSend`, `rememberBrainTurn`;
|
||||
- `styles.css` carries `.edit-btn`, `.edit-form`, `.edit-error`, `.is-editing` and the 44px mobile rule.
|
||||
2. `tests/integration/test_chats_api.py` (extend) — two cases: (a) a `PUT /api/chats/<id>` whose `messages` array is the post-edit shape (edited last user record, later records dropped) round-trips byte-identically on `GET`; (b) the same shape on a shared chat still serves through `SharedChatOut.messages` — pins "the edited/truncated conversation needs no schema change".
|
||||
3. `tests/e2e/test_edit_and_resend.py` (new) — mock-LLM suite on the `app_server`/`page`/`db_ready` fixtures, the phase-120 file as the template (TRUNCATE-then-seed `saved_chats` in an autouse fixture; `auth_helpers.login` for the authed chat; seed `tests/fixtures/docs/homelab/kubernetes.md` as `docs`/`homelab/kubernetes.md` so retrieval grounds and the mock answers with `MOCK_ANSWER_MARKER`):
|
||||
1. `test_edit_last_question_reasks_in_place` — send "How is my Kubernetes cluster set up?" → grounded answer → the user bubble has Edit and the OLD brain bubble count is 1 → click Edit → the textarea is prefilled with the exact question → append " in one line" → Save: exactly ONE user bubble (the edited text), the old answer bubble is gone, the new answer streams with the mock marker, `bor.chat.v1` holds `[…user(edited), brain]` with no duplicate question, and `saved_chats.messages` matches (poll the row).
|
||||
2. `test_only_the_last_question_is_editable` — two turns: exactly one `.edit-btn` exists and it is on the SECOND user bubble; the first user bubble has none.
|
||||
3. `test_edit_cancel_is_a_no_op` — open the editor, press `Escape`: the bubble text, the record, and the answer are unchanged (byte-identical `localStorage` payload before/after); the button returns focus (assert `document.activeElement` is the Edit toggle).
|
||||
4. `test_edit_drops_a_stale_tail_and_survives_refresh` — after an edit-save, `page.reload()`: the edited question + the new answer restore, the dropped answer never reappears, and Edit is back on the question.
|
||||
5. `test_failed_turn_question_is_editable_and_trim_reasks` — the driver case: send a question carrying the mock's terminal-failure trigger (`ALWAYS_FAIL_TRIGGER`, "always fail") so the turn settles with an `error` frame and a failed bubble (the phase-114 "question too long" frame takes the identical settle path — see the docstring note: its exact detail string is real-endpoint-only), assert Edit exists on the question beside the failed bubble's Retry, then Edit → Save with the trigger stripped out and watch the grounded answer stream.
|
||||
6. `test_edit_is_inert_while_the_turn_runs` — start a long answer (`LONG_ANSWER_TRIGGER`, "write a long answer"), assert no `.edit-btn` while streaming, wait for `done`, assert Edit lands.
|
||||
7. negative: `test_composer_draft_survives_an_edit` — type an unsent draft in the composer, edit + save the last question, the draft text is still in `#composer`'s input.
|
||||
- MOCK-ONLY: the suite sets no `E2E_REAL_LLM`; the file's docstring states the trigger names, the seed, and the per-test contract mapping (the phase-120 file's shape).
|
||||
4. `tests/e2e/test_edit_resend.py` is NOT created (one file per phase, A16) — regression runs stay in the phase's final gate: `test_retry_answer.py`, `test_failed_turn_retry.py`, `test_chat_persistence.py`, `test_stop_generation.py`, `test_smoke.py`, each in isolation.
|
||||
|
||||
## Testing & Quality
|
||||
- Run in this order: `uv run pytest tests/unit/test_frontend_edit_message.py -v --no-cov` → `uv run pytest` (full unit + integration) → `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%) → `uv run pytest tests/e2e/test_edit_and_resend.py -v --no-cov` (DB up) → the regression files above, isolated.
|
||||
- No `app/` code changes in this phase: coverage must not move; if a `tests/`-only change drops the TOTAL below 90%, the gate failure is a phase bug (investigate, do not add filler tests).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest` green (unit + integration) with TOTAL coverage >90%.
|
||||
- [ ] `uv run pytest tests/e2e/test_edit_and_resend.py -v --no-cov` green in isolation.
|
||||
- [ ] Every regression E2E file listed above green in isolation.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] The phase dir moves to `.agents/phases/complete/` with the single `--no-gpg-sign` commit (`00_phase.md` §Commit).
|
||||
@@ -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"
|
||||
```
|
||||
@@ -0,0 +1,32 @@
|
||||
# Task 01 — Every tool result tells the model how many calls remain
|
||||
|
||||
**Phase:** `160_tool_budget_telemetry` · **Source:** `TODO.md:4` — "The LLM need to know how many tool calls it has left… Every tool call should also return 'you have x tool calls remaining' or some other helpful message."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The model sees its remaining round budget in every tool result it receives — so it plans its last reads and wraps up on its own instead of hitting the wall mid-research.
|
||||
|
||||
## Work
|
||||
1. `app/rag/agent.py` — add the trailer constants beside the other in-band teaching strings (the `READ_TRUNCATION_NOTICE` L578 / `REREAD_PARTIAL` L628 family, before `CORRECTION_INSTRUCTION` L732):
|
||||
- `TOOL_BUDGET_REMAINING: str = "\n\n[tool calls remaining: {n}]"` — the `.format(n=…)` call site is the only place that builds it;
|
||||
- `TOOL_BUDGET_LAST: str = "\n\n[1 tool call remaining — make it count, then answer]"`;
|
||||
- `TOOL_BUDGET_SPENT: str = "\n\n[No tool calls remain — answer now from what you have, or tell the user to say \"continue\" to keep researching.]"`;
|
||||
- one small pure helper `def _budget_note(remaining: int) -> str` (n ≥ 2 → the formatted remaining line, 1 → `TOOL_BUDGET_LAST`, ≤ 0 → `TOOL_BUDGET_SPENT`) so the wording lives in exactly one place and the unit tests hit a pure function.
|
||||
2. `app/rag/agent.py::run_agent` — after `result = _execute_tool(...)` (L2227) and `rounds += 1` (L2229), append the note before the message goes on the wire: `messages.append({"role": "tool", "tool_call_id": call.id, "content": result + _budget_note(max_rounds - rounds)})` (L2251). Keep `_execute_tool` itself unchanged — its return value is the raw tool result and its existing unit tests stay byte-identical.
|
||||
- The note rides EVERY executed call, including a **rejected** call's refusal text (`UNKNOWN_TOOL`, `ALREADY_IN_CONTEXT`, …) — a refusal consumes a round, so the budget line is exactly as true there.
|
||||
- A truncated read is unaffected: the phase-95 `ToolResultPiece` yields (L2258) read `holder.read_truncations`, not the result string, so the SSE `tool_result` frame and the UI marker stay byte-identical.
|
||||
- `agent_max_rounds == 0` never reaches this code (no tools offered, `tools=None` at L2104) — the kill-switch request stays byte-identical.
|
||||
3. `app/rag/agent.py` — extend the module docstring's loop contract (the L31-60 block, and the phase-45 round-cap paragraph at L214-235): one sentence naming the trailer, its three forms, and the rule "the count is what the model has LEFT after the call it is reading" (so the first result of a 10-round budget reads 9).
|
||||
4. `.env.example` — extend the `BOR_AGENT_MAX_ROUNDS` comment (L69) to mention the telemetry: `# BOR_AGENT_MAX_ROUNDS=10 # hard cap on agent tool rounds per turn (0 = no tools); every tool result carries the model's remaining count (phase 160)`. No new setting, no default change (LOCKED).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (`tests/unit/test_agent.py`, extend the round-cap cluster at L4561): a 3-round always-calling fake LLM produces three tool messages whose contents end with `[tool calls remaining: 2]`, `[1 tool call remaining…`, `[No tool calls remain…` respectively (assert on the recorded `llm.requests[i][0]` message list — the fake already records the exact messages sent); `agent_max_rounds=1` → the single tool message ends with `TOOL_BUDGET_SPENT`; `agent_max_rounds=0` → the request carries no `role: "tool"` message at all (the existing kill-switch pin, extended with one assert); a rejected call (unknown tool) still ends with its refusal + the note; `_budget_note` boundary table (`2`, `1`, `0`, `-1`).
|
||||
- The existing cap/retry/scaffolding tests must keep passing unchanged (the trailer is appended content, not a new message).
|
||||
- Coverage: **>90%** on `app/` — `_budget_note` + the `run_agent` append line fully covered.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A live grounded turn's second model request contains a `role: "tool"` message ending in `[tool calls remaining: N]` with `N == agent_max_rounds - rounds_consumed`.
|
||||
- [ ] The final tool result of a capped turn carries the "no tool calls remain … say \"continue\"" line.
|
||||
- [ ] No SSE frame, `done` shape, `tool_calls=` log field, `read_coverage` span or `tool_result` frame changed (`uv run pytest tests/integration/test_chat_api.py -v --no-cov` green).
|
||||
- [ ] `agent_max_rounds=0` still issues exactly one `tools=None` request with no trailer anywhere.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,37 @@
|
||||
# Task 02 — Past the cap: reject the call, coach the wrap-up, never dump into chat
|
||||
|
||||
**Phase:** `160_tool_budget_telemetry` · **Source:** `TODO.md:4` — "…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
|
||||
|
||||
## Objective
|
||||
A tool call attempted after the budget is spent is rejected — it never runs, never becomes a `tool` frame, never renders in the chat — the forced final is explicitly coached to answer or hand off, and a cap-path turn always ends with readable text.
|
||||
|
||||
## Background (the actual defect — read before coding)
|
||||
The round-cap path already forces one `tools=None` request (L2270-2322), but that request's pieces are yielded **verbatim**, and `app/api/chat.py::_pump` (L893) turns every `ToolCallPiece` into an SSE `tool` frame (L909). A model that answers a tool-less request with structured `tool_calls` anyway — the documented behavior of the hosted `lite` family, same family as the phase-71 scaffolding incident — therefore lands its tool calls in the user's chat as tool lines with no answer behind them. That is the reported symptom. The same holds for the deflected turn (A8, `tools=None` at L870) and the `agent_max_rounds=0` kill switch.
|
||||
|
||||
## Work
|
||||
1. `app/rag/agent.py` — new module constants next to `CORRECTION_INSTRUCTION` (L732):
|
||||
- `ROUND_CAP_WRAPUP_INSTRUCTION: str` — the forced final's coaching, in the CORRECTION_INSTRUCTION voice, covering exactly four points: no tool calls remain; do NOT emit a tool call, tool-call JSON, or any `<|tool_call…|>`-style markup (a tool call now has no effect at all); answer from the results already in the conversation, stating plainly what is still unchecked; if the answer is incomplete, end by telling the user they can say **"continue"** to let you keep researching.
|
||||
- `ROUND_CAP_FALLBACK_ANSWER: str` — the deterministic backstop line (one short sentence in the brand voice: the research budget for this turn ran out before an answer came together, what was gathered still stands above, say **"continue"** to keep digging). Keep it free of markdown decoration (the UI renders it through the normal markdown path).
|
||||
2. `app/rag/agent.py` — `AgentHolder` (L1482): add `rejected_calls: int = 0` (+ a docstring paragraph in the `tool_calls` style: calls the model attempted after the round cap — never executed, never shown, counted here only; the refusal style, "counts in nothing").
|
||||
3. `app/rag/agent.py::run_agent`, the forced-final block (L2270-2322):
|
||||
- build the coached message list the way the phase-71 recovery does it (L2172-2180): `messages_capped = [{"role": "system", "content": system_prompt + "\n" + ROUND_CAP_WRAPUP_INSTRUCTION}, *messages[1:]]` and pass THAT to `chat_stream_retried(..., tools=None, …)` (the retry/restart semantics of locked A2 are unchanged);
|
||||
- inside the consumption loop, intercept before yielding: `if isinstance(piece, ToolCallPiece): holder.rejected_calls += len(…); continue` — one counter increment per attempted call, nothing yielded (no `tool` frame, no visible markup);
|
||||
- keep the existing `final_content` tally for `StreamPiece(kind="content")` and the existing `holder.scaffold_stripped` / per-span warning block;
|
||||
- after the stream closes, when `final_content == 0`: log one warning (`agent: round cap hit with no visible reply (rejected_calls=%d, scaffold_stripped=%d) — answering with the budget fallback`) and `yield StreamPiece(kind="content", text=ROUND_CAP_FALLBACK_ANSWER)`; **delete the `MalformedReplyError` raise on this path only** (the recorded revision, `00_phase.md` §Locked decisions — an honest "budget spent" line beats an error frame for this symptom), while every non-cap `MalformedReplyError` (the round recovery, the deflected recovery, `app/api/chat.py` L1060ff) stays exactly as it is;
|
||||
- keep the `agent round cap reached (rounds=%d) — forcing a final no-tools answer` warning (its existing unit pin at `tests/unit/test_agent.py` L4944 must not change), and add the attempted-calls warning beside it: `agent: %d tool call(s) attempted after the round cap — rejected, not shown`.
|
||||
- Retries still obey locked A2: the coached request goes through `chat_stream_retried` with the same `settings.llm_retries` / `llm_retry_delay` (the phase-67 pin at `tests/unit/test_agent.py` L4800 keeps passing — update its expectation only if it asserts the exact system text).
|
||||
4. `app/api/chat.py::_pump` (L893) — the transport half of the same invariant: add a keyword `tools_offered: bool = True` to the piece loop and drop `ToolCallPiece`s (counting them into a local so one debug line can name them) when `tools_offered is False`. Pass `tools_offered=False` for the deflected/kill-switch request (the `chat_stream(..., tools=None)` call at L860-875 and its recovery at L1040-1060) and `True` for the grounded first pass — `run_agent` already suppresses the cap-path calls internally, so the grounded loop keeps its current behavior. A15 is untouched: the event TYPE list in PLAN §4 does not change, a frame is simply no longer emitted for a call that cannot run.
|
||||
5. `app/rag/agent.py` — the `run_agent` docstring's round-cap paragraph (L214-235) gains the rejection rule + the fallback-answer rule in two sentences, and `app/api/chat.py`'s module docstring gains one line for the no-`tool`-frame-without-tools invariant.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (`tests/unit/test_agent.py`, extend the phase-71 cluster at L5241 and the cap cluster at L4561): a fake LLM whose forced final (`llm.requests[i][1] is None`) streams `ToolCallPiece`s only → the run yields NO `ToolCallPiece`, yields the `ROUND_CAP_FALLBACK_ANSWER` content piece, `holder.rejected_calls == N`, `holder.tool_calls` unchanged, no exception; the same shape with tool calls + real content → the content streams and no fallback is appended; a scaffolding-only forced final → the fallback replaces the old `MalformedReplyError` (update that existing test and name the revision in its docstring); the forced final's recorded system message contains `ROUND_CAP_WRAPUP_INSTRUCTION` while `messages[1:]` (user + tool history) is unchanged; the `round cap reached (rounds=N)` and the new `attempted after the round cap` warnings both appear in `caplog`.
|
||||
- Integration (`tests/integration/test_chat_api.py`, extend — the scripted `tool_script` fake at L84/L125 drives this without touching `tests/e2e/mock_llm.py`): a grounded turn with `agent_max_rounds=1` whose forced final returns a `ToolCallPiece` streams `tool` ×1 (the executed round) and then NOTHING of the sort — no second `tool` frame, no `error` frame — and settles with `done` whose delta text is the answer or the fallback; a deflected turn whose mock returns a `ToolCallPiece` emits no `tool` frame at all.
|
||||
- Coverage: **>90%** on `app/` — every new branch (intercept, fallback, `tools_offered=False`, `final_content == 0`) has a named test.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A cap-hitting turn produces ZERO `tool` frames after the cap and ZERO raw tool markup in the answer.
|
||||
- [ ] That turn always ends with readable content (`done`, not `error`) — the fallback line appears when the model gave nothing but calls/scaffolding.
|
||||
- [ ] `holder.rejected_calls` counts the attempts; `tool_calls=N` in the per-turn log still counts executed calls only.
|
||||
- [ ] Non-cap `MalformedReplyError` behavior is byte-identical (deflected twice-scaffolding integration test still green).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Task 03 — Surface it: `done.budget_exhausted` + the `rounds=` / `cap_hit=` log fields
|
||||
|
||||
**Phase:** `160_tool_budget_telemetry` · **Source:** `TODO.md:4` — "…tell the user to continue the session by allowing more tool calls…" (the server-side half: the turn must REPORT that it ran out, so the UI can offer it and the log can prove it).
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
A cap-reached turn reports itself: one additive `done` key (absent on every other turn) plus two per-turn log fields — so the frontend can act on it (task 04) and the owner can grep how often the cap actually bites.
|
||||
|
||||
## Work
|
||||
1. `app/rag/agent.py` — `AgentHolder` (L1482): add `rounds: int = 0` and `cap_reached: bool = False`, documented in the existing field-doc style — `rounds` is the number of rounds CONSUMED (an executed call, a refusal, or a rejected attempt's round; today it moves only with `rounds += 1`), `cap_reached` is set exactly where the forced no-tools final runs (L2270's block).
|
||||
- Mirror the loop-local `rounds` into `holder.rounds` at the same statement (`holder.rounds = rounds` beside `rounds += 1`) AND once before the forced final, so a turn that ends at the cap reports the full count; do not re-derive it in the API layer.
|
||||
2. `app/schemas.py` — `ChatDoneEvent` (L316): add `budget_exhausted: bool = False` with a docstring paragraph in the `answer_image` style (phase 160, TODO L4 — true ONLY on a turn that hit `BOR_AGENT_MAX_ROUNDS`; the UI's Continue affordance keys on it) and EXTEND the existing `@model_serializer(mode="wrap")` rule so the key is popped when `False`: frames of every non-cap turn serialize byte-identically to today ("ABSENT, never null" — the `answer_image` / `SourceRef.image_url` / phase-123 `image` precedent, and the byte-identical-off rule in PLAN §13).
|
||||
3. `app/api/chat.py` — the `ChatDoneEvent(...)` construction (~L1265): pass `budget_exhausted=holder.cap_reached` (the grounded turn only; the deflected path builds its own done frame and never sets it — a deflected turn runs no agent, so the default `False`/absent is correct by construction).
|
||||
4. `app/api/chat.py` — the per-turn log line (L1196-1219): insert `rounds=%d cap_hit=%d` immediately after `tool_calls=%d` (PLAN §9's field order is `… tool_calls=N …`, the agent fields stay grouped), with `holder.rounds` and `int(holder.cap_reached)`. Keep every existing field, its order and its format byte-identical (several tests grep this line).
|
||||
5. `.agents/PLAN.md` — the ONLY plan edit of this phase (the phase-45 pattern, owner permission = the 2026-10-05 roadmap confirmation for this phase; AGENTS.md rule 3 — a recorded revision, never a silent deviation):
|
||||
- §4's SSE line: `done` `{deflected, sources[], suggestions[], budget_exhausted?}` + one sentence: a `tools=None` request never emits a `tool` frame, and a round-capped turn whose final reply carries no visible content answers with the budget fallback instead of erroring (revising the phase-71 cap-path note);
|
||||
- §9's required line: add `rounds=N cap_hit=0|1` after `tool_calls=N`;
|
||||
- §12 is left alone (the roadmap table lists shipped phases by directory).
|
||||
6. No migration, no new table, no new setting — `agent_max_rounds` keeps its default of 10 (LOCKED).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_schemas*.py` (extend the `ChatDoneEvent` serialization pins — the phase-124 answer-image tests are the template): `budget_exhausted=False` → the key is ABSENT (assert `"budget_exhausted" not in payload`), `True` → present and `true`; a full done frame with sources + related + suggestions + no flag serializes byte-identically to the pre-phase expectation (the existing golden payload test must pass untouched). `tests/unit/test_agent.py` — `holder.rounds` counts refusals as rounds today (an unknown-tool call bumps `rounds` but not `tool_calls`), and `cap_reached` is `False` on a turn that answered inside the cap.
|
||||
- Integration (`tests/integration/test_chat_api.py`, extend): a cap-hitting scripted turn ends with `data: …"budget_exhausted": true…` and the captured `caplog` line contains `rounds=N cap_hit=1`; an ordinary grounded turn's `done` frame has no `budget_exhausted` key and its log line reads `cap_hit=0`; the deflected turn's line reads `rounds=0 cap_hit=0`.
|
||||
- Coverage: **>90%** on `app/` (the serializer branch + both holder fields covered).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `done.budget_exhausted` is `true` exactly on cap-reached turns and absent (never `false`) on every other turn — including all deflected turns.
|
||||
- [ ] The per-turn line reads `… tool_calls=10 rounds=10 cap_hit=1 …` on a capped turn; every pre-existing field is unchanged in value and order.
|
||||
- [ ] `.agents/PLAN.md` §4/§9 carry the additive revision note and nothing else changed in the plan.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Task 04 — "Continue researching" on an answer that ran out of tool calls
|
||||
|
||||
**Phase:** `160_tool_budget_telemetry` · **Source:** `TODO.md:4` — "…tell the user to continue the session by allowing more tool calls (or something similar)."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
When a turn ends because its tool budget was spent, the answer bubble offers **Continue researching**; one click sends the next turn (fresh history + a whole new round budget) so a long research session keeps going without typing.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — `CONTINUE_ICON` beside `TUNE_ICON` (L475) / `RETRY_ICON` (L606): an inline `aria-hidden` SVG in the same stroke family (a forward/arrow-on form, 24×24, `fill="none" stroke="currentColor" stroke-width="1.7"`) — no icon font, no CDN (A11).
|
||||
2. `frontend/assets/app.js` — `const CONTINUE_QUESTION = "continue";` and `appendContinueButton(wrap)` directly after `appendRetryButton` (L617), same shape: `getActionsRow(wrap)` (L498), one-per-bubble guard on `.continue-btn`, `role="listitem"` when the row reports `role="list"`, `btn.innerHTML = CONTINUE_ICON + "<span>Continue researching</span>"`, click → `continueTurn()`.
|
||||
3. `frontend/assets/app.js` — `continueTurn()`, the `retryLastTurn` (L2984) discipline:
|
||||
- no-op while `uiState === UI_STATE.thinking || uiState === UI_STATE.streaming` (one turn at a time);
|
||||
- clear the button from every bubble first (`messagesEl.querySelectorAll(".continue-btn").forEach(b => b.remove())`) so it can never be clicked twice into two turns;
|
||||
- `return runTurn(CONTINUE_QUESTION)` — a plain send (`reask` defaults to `false`): the user bubble appears, `conversation.push({who:"user", text:"continue"})` + `saveConversation()` + `persistConversation()` are the ordinary save points, and the request carries the full `history`, so the model resumes with everything it already read and gets a fresh `agent_max_rounds`. A10 holds: nothing server-side knows or cares that this is a continuation.
|
||||
- ASSUMPTION: the continuation question is the literal word `continue` (what the model is taught to ask the user for in `ROUND_CAP_WRAPUP_INSTRUCTION` / `TOOL_BUDGET_SPENT`, so the visible bubble, the typed equivalent and the model's own suggestion all agree).
|
||||
4. `frontend/assets/app.js` — the `done` settle (L3600-3652): add `budget_exhausted: ev.budget_exhausted || undefined` to the `rememberBrainTurn(…)` meta object (the `related` / `answer_image` optional-key precedent — `undefined` drops the key, so every non-cap record stays byte-identical), and after the existing `markLastRetryable()` (L3649) add `if (ev.budget_exhausted) appendContinueButton(wrap);` (so the row order is Tune → Retry → Continue → Save-as-doc, the insertion order phase 130 relies on).
|
||||
5. `frontend/assets/app.js` — the restore: `renderStoredMessage`'s brain branch (after L2281) gets `if (m.budget_exhausted) appendContinueButton(wrap);` — a reload keeps the affordance (a restored `history` is exactly what the continuation turn needs). `startNewChat()` (L2886) needs nothing (resetting `messagesEl` drops the buttons with the list).
|
||||
6. `app/schemas.py` — `ChatMessage` (L1264ff, `extra="forbid"`): add the optional marker `budget_exhausted: bool | None = None` documented in the phase-120 `failed` / phase-48 `stopped` style (the persisted answer that ran out of tool calls — the client's restore hint, never a server behavior). No migration: `saved_chats.messages` is raw JSONB (phase 50), and `app/api/chats.py`'s logic is unchanged.
|
||||
7. `frontend/assets/styles.css` — `.continue-btn` in the `.retry-btn` family (L1010: identical pill size/spacing/color, the shared `:focus-visible` ring — extend the selector list if the ring rule is per-class) + `min-height: 44px` beside `.retry-btn` in the mobile block (L5708-5709) + any `prefers-reduced-motion` parity the family needs (PLAN §7 feedback rules).
|
||||
8. Verify the shared page (`/shared/<token>`, rendered server-side, not by `app.js`) shows no Continue control for a flagged record — the key is ignored there by construction; record the check in the task's commit note (no code change expected).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (house-style source pins; new `tests/unit/test_frontend_tool_budget.py`): `appendContinueButton` joins the actions row with the one-per-bubble guard and `listitem` role; `continueTurn` bails on `thinking`/`streaming`, removes every `.continue-btn` before sending, and calls `runTurn(CONTINUE_QUESTION)` with no `reask`; the `done` handler passes `budget_exhausted` through `rememberBrainTurn` and appends the button only when flagged; the restore branch keys on `m.budget_exhausted`; `retryLastTurn`, `markLastRetryable` and `rememberBrainTurn` bodies are byte-unchanged.
|
||||
- Integration (`tests/integration/test_chats_api.py`, extend): a saved chat whose brain record carries `budget_exhausted: true` round-trips byte-identically on `GET /api/chats/<id>` (and on the shared endpoint) — pins the additive `ChatMessage` field.
|
||||
- Coverage: **>90%** on `app/` (the only `app/` change is the schema field — covered by the round-trip test).
|
||||
- Manual: with `BOR_AGENT_MAX_ROUNDS=1` (or the task-05 mock marker) confirm the answer offers Continue, one click runs a fresh turn, and a reload restores the button.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] An answer from a cap-reached turn carries a working **Continue researching** button; clicking it sends exactly one new turn whose request carries the prior `history`, and its answer streams normally.
|
||||
- [ ] A non-cap answer shows no such button, and neither does any earlier answer once a newer turn lands.
|
||||
- [ ] The button is inert while a turn is in flight; no double-send is possible.
|
||||
- [ ] A page refresh keeps the button on a flagged answer; the shared page never shows it.
|
||||
- [ ] `bor.chat.v1` records of non-cap turns are byte-identical to pre-phase (no `budget_exhausted` key).
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,38 @@
|
||||
# Task 05 — Budget tests: the mock's round-cap flow, unit/integration pins, isolated E2E
|
||||
|
||||
**Phase:** `160_tool_budget_telemetry` · **Source:** `TODO.md:4` — "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…"
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The whole contract is pinned end to end: a deterministic mock flow that runs a turn to the cap and then tries to dump tool calls into chat, the unit/integration assertions for the trailer / rejection / flag, and the phase's own Playwright file run in isolation (AGENTS.md rule 9).
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` — a new deterministic flow, documented in the module docstring's marker table:
|
||||
- `ROUND_CAP_TRIGGER = "run out of tool calls"` — detected on the LAST user message only (the mock's existing invariant: the last `user` message is the current question, the earlier non-system messages are prior turns — see the `last_user` scan around L3186);
|
||||
- while tools are offered (`"tools"` present on the request) and no tool result answers the last emitted call: stream ONLY `tool_calls` deltas — `ls` with no arguments, incrementing id `call_cap_1`, `call_cap_2`, … , `finish_reason: "tool_calls"`, no content — i.e. the model never stops asking, so the turn is guaranteed to hit `BOR_AGENT_MAX_ROUNDS`;
|
||||
- on the request WITHOUT tools (the forced final) in this flow: stream a `tool_calls` delta anyway (`read` on the seed path, id `call_cap_dump`) and NO content — the exact dumped-call shape the phase kills, and the shape `SCAFFOLD_TRIGGER` (L1375) does not cover (that one emits textual markup, this one is structured);
|
||||
- when the last user message is exactly `continue` and an earlier message carries the trigger: stream a stable answer `ROUND_CAP_CONTINUE_ANSWER` (a fixed sentence naming the seed document, so the E2E asserts a real continuation answer rather than a tool call).
|
||||
- The new branch must not disturb any existing trigger (the phase-45 multi-read flow, `TOOLS_TRIGGER`, `SCAFFOLD_TRIGGER`, the failure markers) — the existing mock-only suites are the regression gate.
|
||||
2. `tests/e2e/test_tool_budget_telemetry.py` (new, MOCK-ONLY — no `E2E_REAL_LLM` support, the phase-45/120 files are the template: `app_server` / `page` / `db_ready` fixtures, `auth_helpers.login`, the TRUNCATE-then-seed KB pattern over `tests/fixtures/docs/homelab/kubernetes.md`, the autouse `saved_chats` truncate):
|
||||
1. `test_tool_results_carry_the_remaining_count` — server-side pin at the wire level: run the cap flow and read the mock's recorded requests (the mock exposes its request log; otherwise assert through the app's captured log lines) that each successive tool message ends with `[tool calls remaining: N]` counting down and the last says no tool calls remain.
|
||||
2. `test_capped_turn_never_shows_tool_lines_after_the_cap` — with `BOR_AGENT_MAX_ROUNDS=3` (the `app_server` env override the existing suites already use for tuning): the chat shows at most 3 tool lines total, the dump call produces NO tool line, the answer bubble holds non-empty readable text (the fallback or the coached answer), the turn settles with `done` (never the error banner), and the SSE capture has no `error` frame.
|
||||
3. `test_done_flags_budget_exhausted` — the captured SSE stream's `done` frame carries `budget_exhausted: true`; a normal grounded turn's `done` carries no such key (byte-identical control case in the same file).
|
||||
4. `test_continue_button_runs_a_fresh_turn` — the capped answer bubble shows **Continue researching**; clicking it appends exactly one user bubble `continue`, the mock answers with `ROUND_CAP_CONTINUE_ANSWER`, the earlier tool lines do not duplicate, and `bor.chat.v1` holds `[…brain(flagged), user("continue"), brain]`.
|
||||
5. `test_continue_is_inert_while_streaming_and_single_shot` — clicking Continue twice fast (or during the streamed turn) starts ONE turn (count the `/api/chat` requests via `page.on("request")`).
|
||||
6. `test_flag_survives_a_refresh` — `page.reload()` after a capped turn: the flagged answer restores WITH the Continue button and clicking it still works.
|
||||
7. negative `test_deflected_turn_gets_no_continue_button` — a deflected question (A8 LOW mode) settles deflected with no `budget_exhausted` key and no Continue button.
|
||||
- The file's docstring states: the markers, the `BOR_AGENT_MAX_ROUNDS` override used, the seed, and this numbered test → contract mapping (the phase-120 file's shape).
|
||||
3. `tests/unit/test_tool_budget_note.py` (new, if `tests/unit/test_agent.py` gets too long for the reviewer's taste — otherwise extend that file's round-cap cluster L4561): `_budget_note` boundaries, the trailer's presence on executed AND refused calls, `agent_max_rounds=0` producing no tool message at all.
|
||||
4. `tests/unit/test_frontend_tool_budget.py` — already written in task 04; extend it with anything task 04 left unpinned rather than duplicating it here.
|
||||
5. Regression gate (each file in isolation, `--no-cov`): `test_agent_unlimited_tools.py` (the multi-read flow must still stream its 3 tool lines and answer), `test_agent_document_tools.py`, `test_harness_aligned_tools.py`, `test_grep_regex_teaching.py`, `test_big_read_progress.py` + `test_read_range_tool_lines.py` (the `tool_result` frame path the trailer rides next to), `test_chat_rag.py`, `test_scaffolding_guardrails.py` (or its current name in `tests/e2e/`), `test_smoke.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Order: `uv run pytest tests/unit -v --no-cov` → `uv run pytest` (unit + integration) → `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%) → `uv run pytest tests/e2e/test_tool_budget_telemetry.py -v --no-cov` (DB up) → the regression list above, isolated.
|
||||
- Every new branch of tasks 01-04 must be attributable to a named test in the coverage report (`--cov-report=term-missing` shows no new red lines in `app/rag/agent.py`, `app/api/chat.py`, `app/schemas.py`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_tool_budget_telemetry.py -v --no-cov` green in isolation.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90% with no uncovered new lines in the touched modules.
|
||||
- [ ] Every regression E2E file listed above green in isolation.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `.agents/PLAN.md` shows exactly the task-03 revision note; the phase dir moves to `.agents/phases/complete/` with the single `--no-gpg-sign` commit (`00_phase.md` §Commit).
|
||||
Reference in New Issue
Block a user