phase: 115_doc_draft_discard
All green — this was the final verification pass; everything from the four completed tasks was already in the working tree and verified. **Phase 115 — Doc drafts: Discard + DELETE route + title fix — verification report** - Verified all 4 task deliverables present: DELETE route (`app/api/doc_drafts.py`), Discard UI (`doc-edit.html` + `doc-edit.js` + `.discard-draft` CSS), title fix (`defaultDocTitle(wrap)` pairing + `saveAsDoc` call site), and all test pins (integration, frontend unit, E2E). No code changes needed. - **Completion criteria:** 1. ✅ Orphaned draft discardable from edit screen; row gone — `test_delete_removes_row_and_invalidates_token` (204 → GET 404), unknown-token 404, admin-gate 403 on all routes, E2E `test_discard_draft_from_edit_screen` all pass. 2. ✅ Title after retry redo = redone answer's own question — E2E `test_save_title_is_the_redo_question_after_retry` passes. 3. ✅ Push flow byte-identical — `git diff` shows only the new DELETE route + module docstring; all 7 existing push tests green. 4. ✅ `uv run pytest --cov=app` → **2457 passed**, app coverage **99%** (>90%); `uv run pytest tests/e2e/test_save_doc_session.py -v --no-cov` → **4 passed**; `uv run ruff check .` → clean; `uv run pyright` → 0 errors. 5. ⏳ Commit + phase-dir move left to the harness (per executor rules, no `git` run; all changes left in the working tree). - No defects found; no deviations. - Next pending phase: none in `todo/` other than this one (`115_doc_draft_discard` is the last).
This commit is contained in:
@@ -0,0 +1,45 @@
|
||||
# Phase 115 — Doc drafts: Discard + DELETE route + title fix (TODO L7)
|
||||
|
||||
**Source:** `TODO.md` L183–209 — "L7 — 'Save as doc' has no Discard: orphan drafts are invisible and un-deletable (2026-09-15, brain-of-reese interactive test)"
|
||||
**Story:** n/a (interactive-test follow-up fix; extends the phase-59/75 doc-draft and save-as-doc assets).
|
||||
**Context:** "Save as doc" (`frontend/assets/app.js::saveAsDoc`, L720) POSTs `/api/doc-drafts` (201) → `/doc-edit.html?draft=<token>`. The edit screen offers exactly one action: **"Push to docs branch"**. The drafts router (`app/api/doc_drafts.py`) has POST/GET/PUT/POST-push only — no DELETE, no TTL/pruning; a draft created by an accidental click (or a tester) sits orphaned in the DB forever — invisible (no UI lists drafts) and only consumable by actually pushing a doc to the repo. Side observation: the draft's default title comes from `defaultDocTitle()` (app.js:619) — the **last user record** in the conversation — and after a Retry redo-in-place (phase 49) the redone answer sits at the end, so its title came from an unrelated trailing question (a junk 4,000-char test question, not the question the answer answered). The body (full session transcript) is correct by design; only the title derivation mismatches.
|
||||
|
||||
## Objective
|
||||
An orphaned doc draft can be discarded from the edit screen (a new admin-gated `DELETE /api/doc-drafts/{token}` + a Discard control), and the draft's default title is the question the saved answer actually answered (its paired user record) — fixing the retry-redo mismatch.
|
||||
|
||||
## Dependencies
|
||||
- `114_embed_question_length` (todo) — pipeline predecessor (execution order) only; no code dependency.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **DELETE route (task 01):** `DELETE /api/doc-drafts/{token}` → **204**. The whole router already sits behind `require_admin` (phase 59, `dependencies=[Depends(require_admin)]` at L63) — the new route inherits it; the uuid4 token is the screen's credential (same trust model as GET/PUT/push). Unknown token → 404 via the existing `_get_draft_or_404` helper. No migration (a row delete); no push-side state (the git push happens only on push).
|
||||
- **Discard UI (task 02):** the doc-edit screen — a "Discard draft" control next to "Push to docs branch" (secondary/danger treatment per the theme). `frontend/assets/doc-edit.js`: `confirm()` (destructive + irreversible — no undo exists), `DELETE /api/doc-drafts/${token}` (the same token the screen already uses for GET/PUT) → 204 → `location.assign("/")` (back to the chat page). Non-204 → the page's existing inline-error pattern, no navigation.
|
||||
- **Title fix (task 03):** `defaultDocTitle(wrap)` — takes the saved brain bubble's wrap (the `.save-as-doc-btn`'s bubble); the title is the text of the user bubble **paired** with that brain bubble (the nearest preceding user message in the DOM conversation flow), falling back to the current last-user-record-in-`conversation` logic when no wrap is given or no paired user bubble is found (first-turn edge / DOM mismatch). The `DOC_TITLE_MAX` slice + "Note" fallback are unchanged.
|
||||
- **NOT touched:** the push flow (byte-identical), the draft body (`buildSessionTranscript` — full session, correct by design), the drafts schema (no field change), no TTL/pruning (locked A1 — out of scope).
|
||||
|
||||
## Tasks
|
||||
1. `01_delete_draft_route.md` — `DELETE /api/doc-drafts/{token}` (204 / 404 / admin-gated).
|
||||
2. `02_discard_ui.md` — the Discard control on the doc-edit screen wired to the route.
|
||||
3. `03_draft_title_fix.md` — the title from the answer's own question (the paired user record).
|
||||
4. `04_draft_discard_tests.md` — the API + frontend + E2E pins.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: `tests/integration/test_doc_drafts_api.py` (existing — extend, task 04): DELETE removes the row (204; subsequent GET 404); unknown token → 404; the admin gate applies (same assertions the sibling routes use).
|
||||
- Unit (frontend, house source-assertion style, task 04): the Discard control's presence/handler (confirm → DELETE → 204 → redirect; non-204 → inline error, no navigation); the `defaultDocTitle` pairing logic + the call site.
|
||||
- E2E: `tests/e2e/test_save_doc_session.py` (existing — extend, task 04): the discard flow (save → edit screen → discard → confirm → back on the chat, draft gone); a save-as-doc after a Retry redo-in-place → the title matches the redone answer's own question. Run in isolation: `uv run pytest tests/e2e/test_save_doc_session.py -v --no-cov`.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] An orphaned draft can be discarded from the edit screen; the draft row is gone afterward (integration).
|
||||
- [ ] The title of a save-as-doc after a retry redo matches the redone answer's own question (E2E).
|
||||
- [ ] The push flow is byte-identical (regression green).
|
||||
- [ ] `uv run pytest` green; coverage >90%; the e2e file green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — scope: the Discard control + the DELETE route + the title fix; NO TTL/pruning (owner-confirmed 2026-09-14, roadmap confirmation — the discard covers the acceptance; pruning can be a future phase).**
|
||||
- **A2 — the DELETE route is admin-gated by the router-level `require_admin` (phase 59) with the uuid4 token as the credential — same trust model as the sibling routes (owner-confirmed 2026-09-14).**
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ tests/ frontend/ .agents/phases/ && git commit --no-gpg-sign -m "feat(docs): discard doc drafts from the edit screen (DELETE /api/doc-drafts/{token}) + derive the draft title from the answer's own question"
|
||||
```
|
||||
@@ -0,0 +1,37 @@
|
||||
# Task 01 — DELETE /api/doc-drafts/{token}
|
||||
|
||||
**Phase:** `115_doc_draft_discard` · **Source:** `TODO.md:185–199, 201–204` — "Clicking 'Save as doc' on an answer POSTs `/api/doc-drafts` (201) and navigates to `/doc-edit.html?draft=<token>`. The edit screen offers exactly one action: **'Push to docs branch'**. There is no Discard/cancel control, the drafts API has no DELETE route (`app/api/doc_drafts.py`: POST, GET, PUT, POST /push only) and no TTL/pruning. A draft created by an accidental click (or a tester) sits orphaned in the DB forever — invisible (no UI lists drafts) and only consumable by actually pushing a doc to the repo." + "Suggested fix: add a Discard control to `/doc-edit.html` wired to a new `DELETE /api/doc-drafts/{token}` (admin-gated, token = the screen's credential) …"
|
||||
|
||||
## Objective
|
||||
A new admin-gated `DELETE /api/doc-drafts/{token}` removes a draft row (204) so an orphaned draft can be discarded instead of only pushed.
|
||||
|
||||
## Work
|
||||
1. `app/api/doc_drafts.py` — add (after the `update_draft` PUT route, before the push route — or at the file's route-order idiom):
|
||||
```python
|
||||
@router.delete("/{token}", status_code=status.HTTP_204_NO_CONTENT)
|
||||
def delete_draft(
|
||||
token: uuid.UUID,
|
||||
db: Session = Depends(get_db), # noqa: B008
|
||||
) -> None:
|
||||
"""Discard a draft (the edit screen's Discard control, phase 115).
|
||||
|
||||
Admin-gated like the whole router (phase 59); the uuid4 token is
|
||||
the screen's credential — after a successful discard, GET/PUT/push
|
||||
all 404.
|
||||
"""
|
||||
row = _get_draft_or_404(db, token)
|
||||
db.delete(row)
|
||||
db.commit()
|
||||
```
|
||||
Match the file's existing import style (check whether `status` from `fastapi` is already imported; use the file's idiom for the 204 response).
|
||||
2. No migration, no schema change, no other route touched.
|
||||
3. ASSUMPTION: 204 No Content (no body) — the token is a one-way credential; nothing else references the row (no FK targets, no push-side state).
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: extended in task 04 (`tests/integration/test_doc_drafts_api.py`).
|
||||
- Coverage: **>90%** on `app/` including the new route.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `DELETE /api/doc-drafts/{token}` → 204; the subsequent `GET` → 404.
|
||||
- [ ] Unknown token → 404; the admin gate applies exactly like the sibling routes.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,33 @@
|
||||
# Task 02 — Discard control on the doc-edit screen
|
||||
|
||||
**Phase:** `115_doc_draft_discard` · **Source:** `TODO.md:201–204` — "Suggested fix: add a Discard control to `/doc-edit.html` wired to a new `DELETE /api/doc-drafts/{token}` (admin-gated, token = the screen's credential), or a TTL/prune for stale drafts; …" + `TODO.md:207–208` — "Acceptance: an orphaned draft can be discarded from the edit screen; the draft row is gone afterward (API test) …"
|
||||
|
||||
## Objective
|
||||
The doc-edit screen offers a Discard control: confirm → DELETE → back to the chat.
|
||||
|
||||
## Work
|
||||
1. The doc-edit template (find the exact HTML file — the edit screen that renders "Push to docs branch"; `frontend/doc-edit.html` or the template it uses): add next to the push control:
|
||||
```html
|
||||
<button type="button" id="discard-draft" class="discard-draft"
|
||||
title="Delete this draft permanently — this cannot be undone">Discard draft</button>
|
||||
```
|
||||
visually secondary to the push button (the theme's muted/danger treatment).
|
||||
2. `frontend/assets/doc-edit.js` — a handler near the push handler:
|
||||
- `if (!confirm("Discard this draft? This cannot be undone.")) return;`
|
||||
- `fetch(\`/api/doc-drafts/${token}\`, { method: "DELETE" })` (the same token the screen already uses for GET/PUT, per its existing load code ~L136–143);
|
||||
- 204 → `location.assign("/")` (back to the chat page);
|
||||
- non-204 → the page's existing inline-error pattern (message + no navigation, no crash).
|
||||
3. `frontend/assets/styles.css` — `.discard-draft`: the secondary/danger button style (contrast ≥4.5:1, `focus-visible` ring per the theme), laid out next to the push button.
|
||||
4. Frontend unit tests (house source-assertion style, shipped in task 04's `tests/unit/test_frontend_doc_draft_discard.py`): the button's presence; `confirm(...)` before the DELETE; 204 → redirect; non-204 → inline error, no navigation.
|
||||
5. ASSUMPTION: after a successful discard the user lands on the chat page (`/`) — the draft has no other home (no drafts list exists).
|
||||
6. ASSUMPTION: a native `confirm()` is acceptable for this one destructive action (the codebase has no custom dialog asset — if the executor finds one in the theme, use it instead).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the frontend source-assertion tests (task 04).
|
||||
- E2E: the discard flow (task 04, `tests/e2e/test_save_doc_session.py`).
|
||||
- Coverage: n/a (frontend) — the `app/` gate stays green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The edit screen shows "Discard draft" next to the push control; the confirm dialog appears; a 204 returns the user to `/`.
|
||||
- [ ] A failed DELETE (e.g. a 404 race) shows the inline error; no navigation; no crash.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,26 @@
|
||||
# Task 03 — Draft title from the answer's own question
|
||||
|
||||
**Phase:** `115_doc_draft_discard` · **Source:** `TODO.md:193–199` — "Side observation (edge case, same test): the draft's default title is the user record *immediately preceding* the saved answer. After a Retry redo-in-place (L-see app.js `retryLastTurn`), the redone answer sits at the end of the conversation, so its save-as-doc title came from an unrelated trailing question (the doc was titled with a junk 4,000-char test question, not the question the answer answered). Body is the full session transcript (correct by design); title derivation is just the mismatch." + `TODO.md:203–204` — "consider deriving the default title from the question the answer actually answered (its paired user record) rather than the preceding record."
|
||||
|
||||
## Objective
|
||||
The draft's default title is the user question paired with the saved brain bubble (its own question) — after a Retry redo-in-place the title matches the redone answer's question.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — `defaultDocTitle()` (L619) → `defaultDocTitle(wrap)`:
|
||||
- when *wrap* (the brain bubble) is given, walk the DOM conversation flow backwards from *wrap* to the **nearest user message bubble** (confirm the exact user-bubble class — the `.msg` variant used for user turns) and use its text;
|
||||
- fall back to the current last-user-record-in-`conversation` logic when *wrap* is absent or no paired user bubble is found (first-turn edge / DOM mismatch);
|
||||
- the `DOC_TITLE_MAX` slice + whitespace collapse + "Note" fallback are unchanged.
|
||||
2. `saveAsDoc(btn)` (L720) — pass the bubble: `const title = defaultDocTitle(btn.closest(<the bubble class>))` — the save button lives in the bubble's meta (the `addSaveAsDocButton` code ~L704–710 shows the exact ancestor; use the same class `lastBrainWrap` uses at L577).
|
||||
3. The `docSlug`/path logic is unchanged (it derives from the title).
|
||||
4. Frontend unit tests (house source-assertion style, task 04's file): `defaultDocTitle` takes a wrap arg and prefers the paired user bubble over the last conversation record; the `saveAsDoc` call site passes the bubble ancestor.
|
||||
5. ASSUMPTION: the pairing is DOM-structural (nearest preceding user bubble), not index-based — the redo-in-place reorders the DOM, and the structural pair IS the answer's question by construction.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the frontend source-assertion tests (task 04).
|
||||
- E2E: the title-after-retry pin (task 04, `tests/e2e/test_save_doc_session.py`).
|
||||
- Coverage: n/a (frontend) — the `app/` gate stays green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A save-as-doc on a normal (non-redone) answer → the same title as pre-phase (no regression).
|
||||
- [ ] A save-as-doc after a Retry redo-in-place → the title is the redone answer's own question (E2E).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Task 04 — API, frontend, and E2E pins for the discard + title
|
||||
|
||||
**Phase:** `115_doc_draft_discard` · **Source:** `TODO.md:207–209` — "Acceptance: an orphaned draft can be discarded from the edit screen; the draft row is gone afterward (API test); title of a save-as-doc after a retry redo matches the redone answer's own question."
|
||||
|
||||
## Objective
|
||||
The acceptance is pinned at all three layers: the API (the row is gone), the frontend (the control + handler + the pairing), and the E2E (the full discard flow + the title after a retry redo).
|
||||
|
||||
## Work
|
||||
1. `tests/integration/test_doc_drafts_api.py` (existing — extend):
|
||||
- DELETE an existing draft → 204; the subsequent GET → 404.
|
||||
- DELETE an unknown token → 404.
|
||||
- the admin gate: the same assertions the sibling routes use (mirror the existing test's auth fixtures — anonymous → 401 / non-admin → 403 per the router's `require_admin`).
|
||||
2. `tests/unit/test_frontend_doc_draft_discard.py` (new, house source-assertion style):
|
||||
- the doc-edit template carries `#discard-draft`; `doc-edit.js` calls `confirm(...)` before the `DELETE /api/doc-drafts/` fetch; 204 → the redirect; non-204 → the inline error, no navigation.
|
||||
- `app.js`: `defaultDocTitle` takes a wrap arg and prefers the paired user bubble; the `saveAsDoc` call site passes the bubble ancestor.
|
||||
3. `tests/e2e/test_save_doc_session.py` (existing — extend):
|
||||
- **the discard flow:** ask (mock LLM) → save as doc → the edit screen → click Discard → confirm → back on the chat page; an API check (test client) confirms the draft row is gone (GET 404).
|
||||
- **the title after a retry:** ask → the answer → Retry (redo-in-place) → save as doc on the redone answer → the edit screen's title field value == the redone question (not an unrelated trailing question).
|
||||
4. Run in isolation: `uv run pytest tests/e2e/test_save_doc_session.py -v --no-cov` (DB up).
|
||||
5. Regression: `tests/integration/test_doc_drafts_api.py` (all), the existing `tests/e2e/test_save_doc_session.py` tests green.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: the DELETE contract (the acceptance: "the draft row is gone afterward (API test)").
|
||||
- Unit: the frontend pins.
|
||||
- E2E: the flow + the title (the acceptance).
|
||||
- Coverage: **>90%** on `app/`.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All the acceptance pins green (API row gone; title after a retry).
|
||||
- [ ] `uv run pytest` green; coverage >90%; the e2e file green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||
Reference in New Issue
Block a user