You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
review: address 5 Brad findings on the QA checklist + smoke script
Five PR #34 review findings closed in one commit. All polish; no
behaviour change on the happy path.
Checklist (tests/web-ui-qa-checklist.md):
* §6 export-frontmatter table was citing the CLI export schema
(log_id / workspace / created_at / updated_at). The web UI pipeline
in static/js/download.js actually emits title / created /
conversation_id (plus optional models_used / token / cost). Fixed,
with an inline NOTE so a future reader doesn't swap them back.
* §7 cross-browser-parity table was unfinished AND tested phantom
features (per-bubble copy button + arrow-key sidebar nav don't
exist in the codebase). Replaced with the four checks that target
features actually shipped today; phantom rows documented as
enhancement ideas.
* §8 post-fix screenshot row was an unticked TODO even though the
workspace + conversation screenshots in samples/qa/ were captured
after the .main-content fix landed. Flipped to [x] with explicit
evidence pointer.
* §9 sign-off table was empty rows. Filled in with this PR's state
(4 of 5 rows resolved; reviewer-approval row stays pending).
* Added a verification-method legend at the top so a reviewer can tell
visual vs probe vs code-inferred vs deferred (⏳) ticks apart. The
default is "visual"; deviations carry an explicit tag.
Smoke script (tests/web-ui-smoke.sh):
* Page-route probes now sniff the body for the expected `<title>`
string in addition to the 200 status, so an "empty 200" template
regression can no longer slip through.
* Workspace-scoped probes no longer skip silently when /api/workspaces
returns no non-global rows — the message is now a loud [WARN] block
in the output, and the new env var CLAW_QA_REQUIRE_WORKSPACE=1
converts the skip into a hard failure for CI runs that seed fixture
data. Documented in the script header.
Verified: bash tests/web-ui-smoke.sh — 11/11 pass with body sniff;
negative test (fake needle) correctly exits 1 with the missing-needle
message; empty-workspace test with CLAW_QA_REQUIRE_WORKSPACE=1 exits 1
as expected.
-[x]**(probe)** No `Traceback` / `Error` in stdout during smoke run
39
54
40
55
## 1. Home / Projects list — `GET /`
41
56
@@ -122,7 +137,7 @@ are good candidates).
122
137
-[x] Backend data shape correct: a sampled bubble carries
123
138
`metadata.thinking = "The user is asking whether it's a good idea..."`
124
139
and `metadata.thinkingDurationMs = 3569`
125
-
-[ ]Expanding it shows the reasoning text (human click required)
140
+
-⏳ Expanding it shows the reasoning text — not exercised this pass (a one-click visual check; data presence already verified via `metadata.thinking` on a sampled bubble above)
126
141
-[x] Duration (`thinkingDurationMs`) shown in human format (e.g. `4.2s`)
127
142
— visible as `Thinking 21s` in the workspace screenshot
128
143
@@ -191,7 +206,7 @@ a file that opens cleanly in its native viewer.
| Markdown | "Export → Markdown" |`.md` with YAML frontmatter from the web UI pipeline (`title`, `created`, `conversation_id`, plus optional `models_used` / token + cost fields) + transcript. NOTE: this is the **web UI** schema in `static/js/download.js`, not the CLI-export schema from issue #27 (`log_id`, `workspace`, etc.).|
195
210
| HTML | "Export → HTML" |`.html` with Prism-highlighted code; opens in browser |
196
211
| JSON | "Export → JSON" |`.json` is valid JSON parseable by `jq .`|
197
212
| CSV | "Export → CSV" |`.csv` opens in a spreadsheet; one row per bubble |
@@ -227,15 +242,23 @@ POST /api/generate-pdf
227
242
228
243
## 7. Cross-browser parity
229
244
230
-
A short list of "weird browser things" that have bitten this project
231
-
before; explicit pass/fail per browser.
245
+
This section was initially drafted as a list of "weird browser things"
246
+
but two of the four rows ended up testing features that don't exist in
247
+
this codebase (per-bubble copy button and arrow-key sidebar navigation).
248
+
Replaced with the cross-browser checks that target features actually
249
+
shipped today. Issue #28's required acceptance criteria are covered by
250
+
§1–§6 + the byte-identical export evidence; §7 is supplementary.
232
251
233
-
| Check | Chrome | Firefox |
234
-
|-------|--------|---------|
235
-
| Code block horizontal scroll on narrow viewport |||
236
-
| Bubble copy-to-clipboard button works |||
237
-
| Keyboard navigation through conversation list (arrow keys) |||
238
-
| Print preview renders bubbles without overlap |||
252
+
| Check | Chrome | Firefox | Evidence |
253
+
|-------|--------|---------|----------|
254
+
| Long code blocks scroll horizontally inside the bubble (do not overflow the column) | ✅ | ✅ | Visible in the `conversation-*.png` screenshots after the `.main-content { min-width: 0 }` fix |
255
+
| Export buttons (md / html / json / csv / pdf) all download a valid file | ✅ | ✅ |`.md`/`.html`/`.json`/`.csv` byte-identical across browsers (cmp -s); both PDFs are valid 153-page documents — see §6 |
256
+
|`Copy All` button copies the whole chat as Markdown to the clipboard | ✅ | ✅ |`copyAllMarkdown()` at [download.js:286-298](../static/js/download.js#L286-L298) calls `convertChatToMarkdown(selectedTab, true)` — the same function the .md / .html / .pdf exports use (already byte-identical across browsers in §6) — and pipes the result to the standard `navigator.clipboard.writeText` API. Same input, same output, different destination. |
257
+
| Print preview renders without obvious overlap (no `@media print` rules — relies on browser default) | ⏳ | ⏳ | No project-specific print styles exist (zero `@media print` rules in `static/css/style.css`); whatever the browser does at print time is unverified. Cmd-P in either browser closes this in 5 seconds; flag the result if anything wraps or clips. |
258
+
259
+
Phantom rows removed (filed as enhancement ideas, not bugs):
260
+
- Per-bubble copy-to-clipboard button — only `copyAllMarkdown()` exists today
261
+
- Arrow-key keyboard navigation through the conversation list — no `keydown` arrow handler in `static/js/*.js`
239
262
240
263
## 8. Regression notes — fixes shipped in this PR
241
264
@@ -252,15 +275,26 @@ During the QA pass, one visual bug was found and fixed in the same PR:
252
275
Existing `overflow-x: auto` on `.prose pre` and `word-break: break-all`
253
276
on `.tool-call-content` then take over inside the bubble.
0 commit comments