add a walkthrough for the chat ui changes

twelve items in click-through order, each with where to look and what the old behaviour was — several are only visible if you know what was broken. states plainly at the top that none of it has been rendered in a browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-06 23:34:44 +00:00
parent 9eb8fa1c17
commit 203f65d03f
1 changed files with 226 additions and 0 deletions
@@ -0,0 +1,226 @@
 # Chat UI walkthrough — 2026-08-06
 A guided tour of the twelve changes made to the web chat on 2026-08-06, in the order they are easiest
 to click through. Each item says **where to look**, **what to do**, and **what changed** — and, where
 it matters, what the old behaviour actually was, because several of these are only visible if you know
 what was broken.
 **Before starting:** `pm2 restart officer`, then hard-refresh the browser (Ctrl/Cmd-Shift-R). The
 frontend is served bundled from `index.gen.html`; without the hard refresh you will be looking at the
 old JS and none of this will be there.
 **Honesty note up front: none of this has been rendered in a browser.** `bunx tsgo` is clean and
 prettier is clean after every change, and the diff algorithm has real unit-test coverage run under
 `bun`. Everything visual is reasoned from the source and the CSS token values, not seen. Treat this
 document as a list of claims to check, not a list of things known to work.
 Commits, oldest first: `d7f5cd5`, `8c27901`, `306def1`, `9daf420`, `9eb8fa1`.
 ---
 ## 1. Dark mode: the white-on-white labels
 **Where:** any chat session that has already started — the provider label beside the model selector,
 and the active provider tab.
 **What to do:** switch to dark mode and open an existing conversation.
 **What changed:** the label was `bg-duck-dark/80 text-white`. `--duck-dark` is `#14532d` in light mode
 but **`#f1f5f9` in dark** — it inverts to near-white. So the label was white text on a near-white
 background for every started session, which is why it looked like the label had simply gone missing.
 This turned out to be a whole class of defect rather than one bug, so `duck-dark` was swept out of the
 chat entirely — 13 files — and replaced with the semantic tokens that carry their own dark variants:
 `foreground`, `foreground/80`, `muted-foreground`, `border`, `border-input`, `bg-muted`. Raw
 `text-red-500` / `bg-red-500` went to `destructive` at the same time.
 Two things were deliberately **not** swept: `--duck-teal` (it has a real dark override, so it works),
 and the `bg-gray-900 text-green-400` terminal look on bash output and log tails, which is meant to look
 like a terminal in both themes.
 ---
 ## 2. Dead air between sending and the first token
 **Where:** the transcript, immediately after you press send.
 **What to do:** send a message with thinking turned on, and watch the gap.
 **What changed:** the streaming bubble returned `null` when its text was empty, so the entire wait
 between send and the first token — tens of seconds with extended thinking — rendered _nothing_. No
 bubble, no spinner, no acknowledgement that the message went anywhere. It now shows the bubble
 immediately with three pulsing dots.
 ---
 ## 3. Text that broke in the wrong places
 **Where:** any tool call with a long shell command; any user message containing a pasted URL.
 **What changed:** `break-all` → `break-words`. `break-all` splits mid-identifier, so a path or a flag
 would break across lines in the middle of a word and become unreadable. The user bubble also gained
 `break-words`, so a pasted URL now stays inside the pane instead of pushing it wide.
 Sub-12px labels also went up to `text-xs`. There were several `text-[10px]` and `text-[11px]` labels
 that were legible on the machine they were written on and not much else.
 ---
 ## 4. Edit and Write tool calls render as real diffs
 **Where:** the transcript, any `Edit` or `Write` tool call. Click it to expand.
 **What to do:** ask for a small edit to a file and expand the tool row.
 **What changed:** the biggest single change of the day. Expanding an `Edit` used to print a `key: value`
 dump — `old_string: …`, `new_string: …` — as raw text, and reading what actually changed meant diffing
 two blobs by eye. It now renders a proper diff: added lines tinted with `bg-success/10`, removed with
 `bg-destructive/10`, context in muted grey.
 **Collapsed rows now carry the stat** — `+12 −4` in the row itself, so you can see the size of an edit
 without opening it.
 Details worth knowing:
 - **No line numbers, deliberately.** An `Edit`'s `old_string`/`new_string` are fragments with no file
  position attached. Any number printed beside them would be invented, and a plausible-looking wrong
  line number is worse than none.
 - **Long diffs clamp at 40 lines** in the view and the differ refuses anything over 800 lines outright,
  falling back to the old input dump rather than locking the tab computing an LCS over a huge file.
 - **Copy gives you the code, not the diff.** The `+`/`−` gutter is `select-none`, so selecting a diff
  and copying gets the source lines. The copy button on a `Bash` row gives the command; on a `Write`
  row it gives the resulting file text — never the `key: value` dump.
 - The differ is 85 lines of LCS in
  `src/workspaces/officerdev/src/apps/Chat/components/line-diff.ts`, written rather than pulled in —
  jsdiff would be a runtime dependency shipped to the browser to run a textbook algorithm. It has unit
  tests; they caught a real bug (`''.split('\n')` is `['']`, not `[]`, so every new-file diff opened
  with a phantom deleted blank line).
 ---
 ## 5. The session list is a list of links now
 **Where:** the left pane of `/chat`.
 **What to do:** **cmd-click a session.** It should open in a new tab. Middle-click it. Tab to it with
 the keyboard.
 **What changed:** rows were `<div onClick>` — the "opaque click" anti-pattern `docs/navigation-audit.md`
 names. The id lived in a closure, not the DOM, so there was no cmd-click, no middle-click, no
 link-focus, and nothing to copy the address of. Rows are now real `<Link>`s built on the shared
 `DataRow`, and they carry the query string, so `?cwd=` survives the click.
 The whole list was rebuilt on the shared data primitives (`DataList`, `DataRow`, `RelativeTime`,
 `LoadingBlock`, `EmptyBlock`, `ErrorBlock`) — the same vocabulary the other rebuilt screens use.
 Two files went away with it: `SessionBar.tsx` and `SessionContextMenu.tsx`, both unused.
 **Note the row action buttons are siblings of the anchor, not inside it.** A `<button>` inside an `<a>`
 is invalid HTML and breaks cmd-click on the row — so rename and delete sit next to the link in a flex
 wrapper, revealed on hover _and on keyboard focus_ (`focus-within:opacity-100`, which the old
 hover-only version did not have).
 ---
 ## 6. "No sessions" versus "the read failed"
 **Where:** the session list, when the transcript directory cannot be read.
 **What changed:** those two states rendered identically — an empty list. One of them is your fault and
 the other one you can act on. `useClaudeSessions` now returns the query's `error`, and the list renders
 an `ErrorBlock` with a retry, not an empty state.
 ---
 ## 7. Rename and delete say when they fail
 **Where:** the session list row actions.
 **What changed:** both were fire-and-forget. A failed rename left the row showing the old title with no
 explanation, which reads as "the rename didn't take" rather than "the rename failed". Both now toast the
 real error.
 This is subtler than it sounds: `useClient` rejects with a plain `{ status, message }` object, **not an
 `Error`**, so the reflex `err instanceof Error ? err.message : 'unknown'` reports every API failure in
 the app as "unknown error". That check now lives in `helpers/error-text.ts` and gets it right.
 ---
 ## 8. Deep links that point at a deleted transcript
 **Where:** open `/chat/<some-id-that-no-longer-exists>`.
 **What changed:** the fetch failed, the code fell back to a bare id, and you got an empty chat pane —
 indistinguishable from a new conversation. It still falls back (the pane is usable), but now it says
 why. Most often the id is stale: the transcript was deleted or pruned out from under the link.
 ---
 ## 9. The read-aloud button
 **Where:** the speaker icon on an assistant message.
 **What to do:** if TTS is down, press it.
 **What changed:** every error was swallowed. The spinner stopped, the speaker icon came back, and a TTS
 service that was simply not running looked exactly like a button that did nothing. Both the synthesis
 failure and the playback failure now say so.
 ---
 ## 10. The background task tray
 **Where:** above the composer, whenever a session has background tasks.
 **What changed, three things:**
 - **Failed tasks had the same icon as stopped ones** — both `CircleSlash`. The two outcomes you most
  need to tell apart were the one glyph. Failed is now an `X`.
 - **Status colours now come from the shared `tone.ts`.** A completed task was green in the transcript
  and grey in the tray; same status, two vocabularies.
 - **The expanded panel is capped against the viewport**, `max-h-[min(16rem,25vh)]`. The composer is
  `shrink-0` and the transcript above it is `flex-1 min-h-0`, so every pixel the panel takes comes out
  of the conversation — a flat `max-h-64` could leave almost no transcript visible in a short panel.
 ---
 ## 11. The attach menu did half of what it offered
 **Where:** the paperclip, in both the composer and the launcher.
 **What changed:** the menu had four entries and two of them — **Text File** and **PDF** — had no
 `onSelect` at all. Clicking them closed the menu and did nothing.
 **Text File now works.** It reads the file and inlines it into the composer as a fenced block with the
 filename on the fence, which is the form an agent reads best. Guards: 256 KB limit, a NUL-byte check
 that catches a binary file whatever its extension claimed, and a four-backtick fence when the file
 contains a three-backtick fence of its own (otherwise the block closes early and the rest of the file
 renders as prose).
 **PDF was removed rather than fixed.** There is no PDF text extraction anywhere in the platform, client
 or server, so that entry could not be made honest without building one first. Worth a decision: it can
 be built, but it is its own piece of work.
 ---
 ## 12. Contrast on the New Chat button
 **Where:** top of the session list.
 **What changed:** it was `bg-duck-teal text-duck-yellow` — roughly 2:1 contrast, in _both_ themes.
 `--duck-yellow` has no dark override at all. It is now `bg-primary text-primary-foreground`.
 ---
 ## Things noticed and deliberately left alone
 - **`useChatWebSocket` silently ignores unparseable frames.** That one is intentional and the comment
  says so — leave it.
 - **`useAttachments`** already toasts on both its failure paths and rolls back the optimistic row.
  Nothing needed.
 - **An earlier audit claim of mine was wrong** and I want it on the record rather than quietly dropped:
  I said `pt-2` inside an `h-full` container caused an 8px overflow on the chat screen. With
  `border-box`, percentage heights resolve against the content box, so there is no overflow. I did not
  change it.
 - **Mobile edit-in-invisible-panel** (dashboards) is unrelated to this work and still open; it is
  recorded in `nav-test-checklist.md` in the workspace root.