From 203f65d03f56236928977df3f8c6b6ea6bfad8d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Thu, 6 Aug 2026 23:34:44 +0000
Subject: [PATCH] add a walkthrough for the chat ui changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 docs/chat-ui-walkthrough.md | 226 ++++++++++++++++++++++++++++++++++++
 1 file changed, 226 insertions(+)
 create mode 100644 docs/chat-ui-walkthrough.md

diff --git a/docs/chat-ui-walkthrough.md b/docs/chat-ui-walkthrough.md
new file mode 100644
index 00000000..54a34b2f
--- /dev/null
+++ b/docs/chat-ui-walkthrough.md
@@ -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.