feat(chat): save by default + share anonymously — auto-saved chats, guest-facing Share, success toast, action row
This commit is contained in:
@@ -1,27 +0,0 @@
|
||||
# Task 01 — Anonymous save/share API surface
|
||||
|
||||
**Phase:** `55_save_share_ux` · **Source:** `TODO.md:3` — "Share chat should work anonymously without login"
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The `/api/chats` save/share **write** surface works without a session: the router-wide `require_admin` (A1) moves off the router and onto the four management routes only (list, detail, delete, unshare).
|
||||
|
||||
## Work
|
||||
1. `app/api/chats.py` — remove `dependencies=[Depends(require_admin)]` from `router = APIRouter(...)` (line ~69; keep `prefix="/chats"` and `tags`). Add `dependencies=[Depends(require_admin)]` to exactly four route decorators: `GET ""` (list, line ~128), `GET /{chat_id}` (detail, line ~180), `DELETE /{chat_id}` (line ~219), `POST /{chat_id}/unshare` (line ~267). The write surface — `POST ""` (create, **including the save-then-share `share: true` branch** which mints the token in the same commit), `PUT /{chat_id}`, `POST /{chat_id}/share` — becomes public (no dependency).
|
||||
2. `app/api/chats.py` — module docstring: rewrite the phase-16 "router-wide admin" paragraph into the split contract and its WHY — the write surface is the visitor's own conversation (row id is an unguessable `uuid4`, same trust model as the phase-51 share token: the id/token IS the credential); the management surface (list/detail/delete/unshare) is the owner's History surface and stays admin-only. Update the per-endpoint docstrings where they still say "admin-only" (the create/update/share docs must now state "public — no session required").
|
||||
3. `tests/integration/test_chats_api.py` — extend (update the existing pins that assert 403 on the now-public write endpoints; the admin pins stay):
|
||||
- Guest (no session cookie): `POST /api/chats` with `messages` → 201 + id + auto-title; `PUT /api/chats/<id>` → 200 (messages replaced, `updated_at` moved); `POST /api/chats/<id>/share` → 200 + `share_url`, a second POST returns the **same** token (idempotent); `POST /api/chats` with `{ messages, share: true }` → 201 + `share_url` in one action (the save-then-share contract, now guest-reachable).
|
||||
- Guest 403s: `GET /api/chats`, `GET /api/chats/<id>`, `DELETE /api/chats/<id>`, `POST /api/chats/<id>/unshare` → all 403 (the router-wide gate moved, it did not disappear).
|
||||
- Revocation still end-to-end: after an admin unshares, the public `GET /api/shared/<token>` read 404s (guest or admin — the public read is unaffected by this task).
|
||||
- Admin session still fully works: the pre-existing admin create/list/detail/delete/unshare pins stay green unchanged.
|
||||
- Do NOT change: `public_router` (`GET /api/shared/<token>`) and `shared_page_router` (already public, phase 51), the token minting (128-bit `uuid4`, same commit as save-then-share), `app/core/auth.py`, the models, or any migration (no schema change).
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: the guest write surface (create, create+share, update, share, idempotent share), guest 403s on the management surface, admin flow byte-for-byte unchanged.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/integration/test_chats_api.py -v` green with the new guest-surface pins.
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] No migration; no change in `public_router` / `shared_page_router` / `app/core/auth.py`.
|
||||
@@ -1,42 +0,0 @@
|
||||
# Task 02 — Auto-save every conversation (retire the Save pill)
|
||||
|
||||
**Phase:** `55_save_share_ux` · **Source:** `TODO.md:4` — "Save shouldn't be a button, every chat should be saved by default"
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The Save pill is gone. Every conversation upserts itself into `saved_chats` at the existing persistence save points (create on the first user message, update on each brain-done and the pagehide partial), and the row link (`currentChatId`) survives a page reload so the same conversation never spawns a second row.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html` — remove the `#save-chat-btn` element and its Phase-50 comment block (the button currently sits between `#new-chat-btn` and `#share-chat-btn`). Do **not** touch `#new-chat-btn` or `#share-chat-btn` here (task 03/05 own those).
|
||||
2. `frontend/assets/app.js` — convert the on-demand `saveCurrentChat()` (line ~1121) into a **headless** `persistConversation()` helper (rename + strip the button UI):
|
||||
- Drop `saveBtn` references, the `saveBtn?.addEventListener(...)` binding (line ~1687), and the `if (saveBtn) saveBtn.hidden = !isAdmin;` boot line (line ~1723).
|
||||
- Keep the exact upsert semantics (they are load-bearing, phase 50): empty conversation → no-op; linked (`currentChatId` set) → `PUT /api/chats/<id>`, and a **404 on PUT unlinks and retries as a create** (`currentChatId = null` then `POST /api/chats`) so a row deleted from History can't wedge the conversation; unlinked → `POST /api/chats`; on `201` capture `currentChatId = String(created.id)`.
|
||||
- Change the failure feedback from the error banner to the **A2 quiet contract**: on a non-ok or network failure set `sendStatus.textContent` to a one-line note (e.g. "Couldn't save automatically — will try on the next message.") and return — **no** `showErrorBanner`, the turn never blocks. On success, stay silent (A2: no status text; the History page is the visible proof).
|
||||
- Keep the idempotent double-fire guard: the save points can overlap (pagehide during a stream), so a module-level `persisting` flag + the existing per-turn ordering prevents concurrent upserts of the same conversation.
|
||||
3. `frontend/assets/app.js` — wire the helper into the save points:
|
||||
- **Save point 1** (user send, inside `runTurn`'s `if (!reask)` block, line ~1443, right after `saveConversation()`): call `persistConversation()`. This is where an unlinked conversation **creates** its row (auto-title from the first question, server-side, unchanged) and a linked one refreshes.
|
||||
- **Save point 2** (`rememberBrainTurn`, line ~1271, after `saveConversation()`): call `persistConversation()` — updates the row with the new brain turn + metadata.
|
||||
- **Pagehide partial** (the `window.addEventListener("pagehide", ...)` handler, line ~1700): it already calls `rememberBrainTurn`, which now carries the `persistConversation()` — so the partial rides the same path with no extra wiring (do **not** add a second call there).
|
||||
4. `frontend/assets/app.js` — **persist the link** across reloads (the dedupe guarantee):
|
||||
- Extend the `bor.chat.v1` record shape (line ~915/932) to `{ v: 1, chatId: string | null, messages: [...] }`. `saveConversation()` (line ~973) must write the current `currentChatId` (null when unlinked). The restore reader (line ~940) must read it back.
|
||||
- At boot, **after** `restoreConversation()` (line ~1725) and **after** `restoreSavedChatFromUrl()` (which already sets `currentChatId`, line ~1101), hydrate `currentChatId` from the restored record. `restoreSavedChatFromUrl` (the `?chat=<id>` admin path, A3 stays admin-gated) keeps its `currentChatId = chatId` and now also persists it into the record.
|
||||
- `startNewChat()` (line ~1330) already sets `currentChatId = null` — keep it, so "New chat" unlinks and the next conversation creates a fresh row on its first message.
|
||||
- **Old-record safety:** a pre-existing `bor.chat.v1` record that lacks `chatId` reads as `null` (unlinked) — its first save point after upgrade creates a row; never throw on the missing field.
|
||||
5. `frontend/assets/styles.css` — remove the `.save-chat-btn` base block (line ~319–339), the ≤640px `.save-chat-btn`/`.save-chat-label` overrides (line ~2637–2639), the `.chat-shell .save-chat-label`/`.chat-shell .save-chat-btn svg` overrides (line ~2649–2650), and drop `.save-chat-btn` from the ≤900px combined padding rule (line ~2532: `.new-chat-btn, .save-chat-btn, .auth-link` → `.new-chat-btn, .auth-link`).
|
||||
6. `tests/e2e/test_chat_history.py` + `tests/e2e/test_share_chat.py` — adapt the **existing phase-50/51 pins** to the new contract (assertion edits limited to the Save-pill removal; every other pin stays byte-for-byte):
|
||||
- `test_chat_history.py` — the file-local `_save()` helper (click `#save-chat-btn` + wait for "Conversation saved.") becomes an **auto-save waiter**: poll `GET /api/chats` (admin cookies, the file's existing `_chats`/`_admin_cookies` helpers) until a row with the conversation's auto-title appears — auto-saves are silent (A2: no status text to wait on). `test_save_and_see_history`: drop the admin Save-pill visibility + click + status assertions — the row now exists right after the question + answer settle (auto-save at the save points). The anonymous-state block (the "phase-50 surface is absent for anonymous" section, line ~415): `#save-chat-btn` now asserts **absent** (`to_have_count(0)` — the element no longer exists in the DOM); `#nav-history` stays hidden (unchanged); the "NEVER calls `/api/chats`" History-page pin stays (list remains admin-only, task 01).
|
||||
- `test_share_chat.py` — `test_share_from_history_and_unshare`: replace the `#save-chat-btn` click + "Conversation saved." wait (line ~375) with the same auto-save row wait (the conversation is already saved by the time the answer settles); the rest of that test (History's Share column, Create link / Copy / Unshare) is untouched. The `/shared/<token>` zero-controls pin (line ~333: `#save-chat-btn, #share-chat-btn` count 0 on the **shared page**) stays valid — the shared page gets no pills.
|
||||
- Do NOT change: `restoreSavedChatFromUrl`'s **admin gate** (`!isAdmin` → `false`, line ~1069, A3), the localStorage `v: 1` key (shape extends in place, old records valid), `saveConversation()`'s localStorage mirror contract, the composer/`#send-status` markup, or the phase-53 stale Regenerate's auto re-save — it must keep working (if it calls `saveCurrentChat` by name, re-point it at the renamed `persistConversation()`; the stale-banner logic is untouched).
|
||||
|
||||
## Testing & Quality
|
||||
- Frontend source pins (house style, `tests/unit/test_save_chat_ui.py` / `test_history_page.py` pattern): `index.html` has **no** `#save-chat-btn`; `app.js` has no `saveBtn`/`saveCurrentChat` symbol, a `persistConversation` referenced from the user-send save point, `rememberBrainTurn`, and the record read/write of `chatId`; `styles.css` has no `.save-chat-btn`.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate) — the server is untouched here, so this task's gate is the frontend pins + a green suite.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] No `#save-chat-btn` in `frontend/index.html` and no `.save-chat-btn` in `frontend/assets/styles.css`; no `saveBtn`/`saveCurrentChat` symbol left in `app.js`.
|
||||
- [ ] A signed-out visitor who sends one question produces exactly one `saved_chats` row (auto-title, both messages); the next brain answer updates the **same** row (no second row).
|
||||
- [ ] Reloading `/` restores the conversation **and** the link — the next message updates the same row, never a duplicate.
|
||||
- [ ] "New chat" unlinks: the following conversation creates a fresh row on its first message.
|
||||
- [ ] A failed auto-save (5xx / network) leaves the conversation fully usable — one status-line note, no error banner, and the next save point retries.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] Regression E2E green in isolation: `test_chat_persistence.py`, `test_smoke.py`, and the two adapted suites `test_chat_history.py`, `test_share_chat.py`.
|
||||
@@ -1,27 +0,0 @@
|
||||
# Task 03 — Share pill visible to every visitor
|
||||
|
||||
**Phase:** `55_save_share_ux` · **Source:** `TODO.md:3` — "Share chat should work anonymously without login"
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The Share pill ships **visible to all visitors** — the admin reveal gate goes away (the API it calls is public since task 01), and its error copy stops presuming a signed-in user.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html` — `#share-chat-btn` (line ~155): remove the `hidden` attribute. Rewrite the Phase-51 comment above it: no longer "Admin-only — ships HIDDEN exactly like Save (absent-not-hidden)" — since phase 55 the pill is static, always-visible markup (task 01 opened the save/share write surface); the save-then-share contract in the comment stays accurate (unsaved → `POST /api/chats` with `share: true`; linked → idempotent `POST /{id}/share`; clipboard with the inline-link fallback; unsharing lives on the admin's History page).
|
||||
2. `frontend/assets/app.js`:
|
||||
- Boot IIFE (line ~1724): **remove** `if (shareBtn) shareBtn.hidden = !isAdmin;` — no reveal step; the `shareBtn` const (line ~203) and the `shareBtn?.addEventListener("click", shareCurrentChat)` binding (line ~1690) stay.
|
||||
- `shareCurrentChat()` (line ~1222): update the two error-banner texts that say "check you're still signed in and try again" → neutral "try again" (with a public API the 403/5xx path is no longer a sign-in problem for a guest). Keep the network-failure banner ("…is the app reachable?").
|
||||
- Update the module-header Share contract comment (line ~158–177): drop "admin-only, SHIPS HIDDEN, revealed at boot" — now "visible to every visitor"; the empty-conversation no-op guard ("Nothing to share yet.") and the one-share-at-a-time double-click guard stay.
|
||||
3. `tests/e2e/test_chat_history.py` — extend the anonymous-state block (the same section task 02 adapted: "the phase-50 surface is absent for anonymous", line ~415) with the new-contract assertion: the anonymous visitor at `/` now sees the Share pill — `expect(page.locator("#share-chat-btn")).to_be_visible()` (alongside the existing `#save-chat-btn` count-0 and `#nav-history` hidden pins). No other pin in that file changes in this task.
|
||||
- Do NOT change: the save-then-share request contract, `copyShareLinkWithFallback` / `absoluteShareUrl`, the inline fallback field, unshare (History page, admin-only, unchanged), or the `#new-chat-btn` (module-owned by `header.js`).
|
||||
|
||||
## Testing & Quality
|
||||
- Frontend source pins: `index.html` — `#share-chat-btn` present **without** a `hidden` attribute; `app.js` — no `shareBtn.hidden` assignment anywhere; the neutral error copy present.
|
||||
- E2E: the extended anonymous-state block in `test_chat_history.py` green in isolation.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A signed-out visitor at `/` sees the Share pill (next to New chat) without signing in; a signed-in admin sees the same pill (no admin-only divergence).
|
||||
- [ ] Clicking Share on an empty conversation still no-ops with the live-region line "Nothing to share yet."
|
||||
- [ ] A failed share shows the neutral error banner (no "signed in" wording).
|
||||
- [ ] `uv run pytest` green; `uv run pytest tests/e2e/test_chat_history.py -v --no-cov` green in isolation (the extended anonymous block); `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -1,34 +0,0 @@
|
||||
# Task 04 — Share-success toast
|
||||
|
||||
**Phase:** `55_save_share_ux` · **Source:** `TODO.md:5` — "Need feedback (probably dropdown notification toast) to show share worked"
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
A top-right slide-down toast confirms a successful share (both success paths). It is **visual only** — the existing `#send-status` live region remains the accessibility announcer, so there is no double screen-reader read.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/styles.css` — add a `.toast` component near the banner/status rules:
|
||||
- Position: `position: fixed; top: <header height + small offset>; right: 1rem; z-index` above the header; a small `max-width` so long text wraps.
|
||||
- Look: theme-consistent with the app's surfaces — solid, high-contrast (text on fill ≥ 4.5:1, WCAG AA, matching the `.new-chat-btn` brand-fill family is the natural choice), rounded, a subtle shadow, one line of icon-optional text.
|
||||
- Entry: slide-down + fade (`transform: translateY(-8px) → 0` + `opacity 0 → 1`, ~200ms). Provide a `.toast.is-visible` (or equivalent) state class the JS toggles.
|
||||
- **Reduced motion:** inside the existing `@media (prefers-reduced-motion: reduce)` convention — drop the transform, keep the opacity fade (or make it instant).
|
||||
- Hidden by default (`opacity: 0; pointer-events: none;` or `display` toggle) so it never intercepts clicks when idle.
|
||||
2. `frontend/assets/app.js` — a `showToast(message: string)` helper (page-script-local, house style — do **not** create a cross-page module):
|
||||
- Lazy-create **one** `<div class="toast">` appended to `document.body`; set text via `textContent` (XSS-safe, never `innerHTML`); mark it `aria-hidden="true"` (A4: visual only — `#send-status` is the announcer).
|
||||
- Re-trigger the entry: clear any pending dismiss timer, force a reflow, toggle the visible state class.
|
||||
- **Single instance:** a new toast replaces a pending one (clear the prior timer; reuse the same node — no stacking).
|
||||
- **Auto-dismiss ~4000ms.**
|
||||
- Call it from `shareCurrentChat()`'s success path (line ~1222): after a successful copy → `showToast("Share link copied.")`; after the fallback field is offered → `showToast("Share link ready — copy it from the field.")`. **Leave the `sendStatus.textContent` updates exactly as they are** (they drive the live region — never stale). Do **not** call `showToast` on any failure branch (the error banner is the failure UI).
|
||||
- Do NOT change: the `#send-status` markup/live region, `copyShareLinkWithFallback` / the inline fallback field, `showErrorBanner`, or any other page (the toast is chat-page only for this phase).
|
||||
|
||||
## Testing & Quality
|
||||
- Frontend source pins: `app.js` — a `showToast` defined and called from the two `shareCurrentChat` success branches (and **not** from any failure branch); the toast node is `aria-hidden`. `styles.css` — a `.toast` rule with a reduced-motion override.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A successful share (clipboard path) shows the toast top-right; it auto-dismisses in ~4s.
|
||||
- [ ] A share that falls back to the inline field shows its own toast text.
|
||||
- [ ] A second share while the first toast is still up replaces it (exactly one toast node in the DOM, no stacking).
|
||||
- [ ] A **failed** share shows the error banner and **no** toast.
|
||||
- [ ] The toast never blocks pointer interaction when idle; with `prefers-reduced-motion: reduce` it appears without a transform.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -1,25 +0,0 @@
|
||||
# Task 05 — New chat / Share action row (horizontal on desktop, stacked on mobile)
|
||||
|
||||
**Phase:** `55_save_share_ux` · **Source:** `TODO.md:6` — "New Chat and Share buttons should only be vertically stacked when in mobile, otherwise they should be horizontally next to each other"
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The New chat and Share pills sit **horizontally next to each other** on desktop and stack **vertically only at ≤640px** (mobile) — today they are direct children of the vertical `.chat-shell` column, so they stack at every width.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html` — wrap `#new-chat-btn` and `#share-chat-btn` in a single `<div class="chat-actions">` row element, keeping the current DOM order (**New chat first, then Share**). The Save pill is already gone (task 02). The wrapper replaces the two pills as direct children of `.chat-shell`; the steering announcer / kb-banner / messages structure around them is untouched.
|
||||
2. `frontend/assets/styles.css`:
|
||||
- **Base (desktop):** `.chat-actions { display: flex; flex-direction: row; align-items: center; gap: 0.6rem; }`. As a flex **item** of `.chat-shell` (a column), the row must NOT stretch the pills to full width — `align-items: center` (not the column default `stretch`) keeps each pill at its intrinsic content width, so they read as two pills side by side, left-aligned in the column.
|
||||
- **≤640px** (the existing mobile block, line ~2536): `.chat-actions { flex-direction: column; align-items: stretch; gap: 0.5rem; }` — the pills stack full-width, New chat above Share. The **existing** ≤640px pill rules (`.new-chat-btn`/`.share-chat-btn` padding line ~2632–2644, the icon/label handling, and the `.chat-shell` label overrides line ~2647–2652) stay and apply to the stacked pills unchanged.
|
||||
- **No horizontal overflow at 360px:** the two pills + gap must fit `360px − 2 × 0.9rem` container padding. Verify the intrinsic widths; if the pair is tight at 360px, reduce the base `gap` (0.6rem → 0.5rem) or the ≤900px padding rule (line ~2532) rather than shrinking the 44px touch floor.
|
||||
- Do NOT change: the pill styling (`.new-chat-btn` / `.share-chat-btn` declarations), the header/nav bar layout, or the `.chat-shell` centered-46rem column contract (PLAN §7 — the wrapper is a normal column child, the column width is untouched).
|
||||
|
||||
## Testing & Quality
|
||||
- Frontend source pins: `index.html` — `#new-chat-btn` and `#share-chat-btn` both inside a single `.chat-actions` wrapper (New chat before Share). `styles.css` — a `.chat-actions` base rule (row) and a ≤640px override (column).
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Desktop (1280×800): both pills on **one horizontal row** — Share's bounding box is to the right of New chat's (same `y` band, `Share.x > New.x + New.width`), each at intrinsic width (not full-column).
|
||||
- [ ] Mobile (390×844): the pills **stack vertically** — Share below New chat (`Share.y > New.y + New.height`), full-width.
|
||||
- [ ] No horizontal page overflow at a 360px viewport.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
Reference in New Issue
Block a user