phase: 159_edit_and_resend_message
All completion criteria verified. Final report:
**Phase 159 — Edit the last question and re-ask it: VERIFIED COMPLETE** (all 4 tasks already in `complete/`; this pass verified end-to-end)
**Verified (implementation landed + tests green):**
- `app.js`: `EDIT_ICON`, `lastUserWrap`, `appendEditButton`/`markLastEditable` (single-target discipline, in-flight guard), `openEditor` (labelled textarea, `maxlength=4000`, Escape/Ctrl+⌘+Enter, empty guard), save path: replace → splice tail → DOM removal → `lastBrainWrap=null` → `saveConversation`+`persistConversation` → `runTurn(text,{reask:true})` — matches README exactly
- `styles.css`: `.edit-btn`/`.edit-form`/`.edit-error`/`.is-editing` + mobile 44px; unit pins 40, E2E 7, integration `test_chats_api.py` extended
**Test/lint results (all green):**
- `uv run pytest` — 3,661 tests, exit 0
- `uv run pytest --cov=app --cov-report=term-missing` — TOTAL **99%** (>90%)
- `uv run ruff check . && uv run pyright` — all checks passed; 0 errors
- `uv run pytest tests/e2e/test_edit_and_resend.py -v --no-cov` (DB up) — **7/7 passed**
- Regressions in isolation: `test_retry_answer.py` 4/4, `test_failed_turn_retry.py` 4/4, `test_chat_persistence.py` 4/4
**Completion criteria:** all 7 ✓ — Edit on last bubble + prefill + Escape/Cancel restore (E2E #3); replace+truncate+re-ask with no duplicated question (E2E #1); refresh survival (E2E #4); failed/“question too long” turn editable + trim re-asks (E2E #5); earlier bubbles un-editable + byte-identical Retry/Stop/persistence (E2E #2 + regressions + unit pins); suite/coverage/lint ✓; E2E + regressions isolated ✓. Commit + phase-dir move: harness's job (no git ops by me, per protocol).
**Documentation:** README.md — full review; phase 159 section cross-checked against `app.js` source (cap, keys, truncate, image survival) — consistent, no changes needed; all 30 `.env.example` vars documented. AGENTS.md — full review; all rules/quick-ref commands valid — verified consistent, no changes needed.
**Notable:** none — no defects found in prior work; no live-infrastructure changes required.
**Next pending phase:** `160_tool_budget_telemetry`
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"
|
||||
```
|
||||
+37
@@ -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).
|
||||
Reference in New Issue
Block a user