fix(dashboard): surface refused regenerate/switch-variant via ErrorNotice (#4202) - #4238
Conversation
Design Review (Fable 5) — ✅ PASSAdvisory design-level review of Design-Verdict: PASS Real information-loss bug, fixed at its root (no rendered surface) with a small generalized helper rather than a third one-off — proportionate and well-evidenced. Watch
Suggestions
[DESIGN-REVIEWED] 196c516 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSAdvisory premise-level review of Contract read; evidence gathered. I verified the key mechanical claims: First-Principles-Verdict: PASS A reported silent-failure defect (#4202) fixed at the cause the author names — no rendered surface — by reusing the existing What this change shipsIntent: make a server-refused regenerate or switch-variant press tell the user why, instead of silently un-disabling the button. This is a FIX.
Watch
[FIRST-PRINCIPLES-REVIEWED] 196c516 |
UX Review (Fable 5) — 🟡 CONCERNSAdvisory UX-level review of UX-Verdict: CONCERNS The refusal finally reaches the screen, but its body is raw backend prose — "slot is running", "no variants" — internal jargon the title's own vocabulary contradicts. Watch
Suggestions
[UX-REVIEWED] 196c516 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsBoth No findings. [OPUS-REVIEWED] 196c516 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
…tice (#4202) A regenerate or switch-variant press the server refuses under the slot lock (a turn already running, a stop in progress, a pending approval, a readiness probe that timed out) died in console.warn: the control flicked to disabled and straight back with nothing on screen. Generalize the refusal surface: one refused-press ErrorNotice above the composer, fed by any refusable press through showRefusedPress with a per-action title (pages.chatPage.could_not_regenerate / could_not_switch_variant across the 12 shipped locales + generated en-XA), showing the server's own message. The notice retires when a turn starts, the session changes, or it is dismissed. The continue press keeps its own path (open PR #4194 owns that surface). Tests lock in: refused regenerate renders title + server reason; refused switch-variant renders and dismiss clears; a starting turn retires the notice. Capture harness + temp-screenshots included.
ea17989 to
196c516
Compare
|
Disposition for UX Review CONCERNS (head ea17989, addressed at 196c516): Screenshots showed invented refusal prose -- fixed at 196c516. The finding is verified legitimate: the wired endpoints return Friendly-copy mapping in |
|
Disposition for First Principles Review CONCERNS (head ea17989, applies unchanged to 196c516): "Fold Continue into the map here / retire one of the two surfaces" -- rebutted for THIS PR, with the convergence path stated. The deferral is deliberate collision avoidance, not scope-dodging: #4194 is another author's open, in-review branch that already carries the continue wiring, its own i18n key, tests, and evidence frames. Landing a competing continue implementation here would guarantee whichever PR merges second rebases through a semantic conflict in the same ChatPage region and re-arms every reviewer on both PRs -- strictly worse than sequencing. The generalized map is exactly what makes convergence cheap: whichever PR lands second, migrating continue onto |
|
Disposition for UX Review CONCERNS on 196c516 (round 2): Raw backend prose as the notice body -- rebutted for this PR (previously dispositioned; restating with the round-2 specifics). The finding is legitimate as a copy-quality observation, and the screenshots now honestly show it. The prescribed prose-keyed map remains the wrong mechanism: matching on server sentence strings drifts silently the day the backend rewords, and per-surface maps fork the copy problem instead of solving it. The repo's own contract (AGENTS.md: non-2xx bodies MUST carry a machine-readable
Title contraction style ("Couldn't" vs sibling "Could not") -- rebutted with precedent. The chosen style matches the newest key in this exact surface family: the continue-refusal title shipped by the sibling PR uses "Couldn't continue", and these two titles will sit in the same notice. Consistency within the refused-press surface outranks consistency with an older, unrelated settings-error key; a catalog-wide style pass is copy-editing outside this PR's scope. |
…tice (kirodotdev#4202) (kirodotdev#4238) A regenerate or switch-variant press the server refuses under the slot lock (a turn already running, a stop in progress, a pending approval, a readiness probe that timed out) died in console.warn: the control flicked to disabled and straight back with nothing on screen. Generalize the refusal surface: one refused-press ErrorNotice above the composer, fed by any refusable press through showRefusedPress with a per-action title (pages.chatPage.could_not_regenerate / could_not_switch_variant across the 12 shipped locales + generated en-XA), showing the server's own message. The notice retires when a turn starts, the session changes, or it is dismissed. The continue press keeps its own path (open PR kirodotdev#4194 owns that surface). Tests lock in: refused regenerate renders title + server reason; refused switch-variant renders and dismiss clears; a starting turn retires the notice. Capture harness + temp-screenshots included.
Problem / Motivation
Two presses in the dashboard chat swallow a server refusal into
console.warn, so the button flicks to disabled and straight back with nothing on screen:ChatPage.tsxregenerate catch) -- also reachable via the slow-probe timeout described in Readiness gate reads a timed-out probe as signed-out, locking out 5 endpoints on a slow host #4201, which reaches this endpoint as an indefinite503 kiro_prerequisite_requiredChatPage.tsxswitch-variant catch)Both endpoints re-check under the slot lock and refuse for ordinary, user-actionable reasons: a turn already running, a stop in progress, a pending approval. The user's only theory is "the click did nothing", and the natural next move is pressing the button again.
The third such press -- Continue -- is being fixed by #4194 with a titled
ErrorNoticeabove the composer. This PR does not touch the continue path; #4194 owns that surface.Why it matters
A control that promises recovery (regenerate a failed answer, flip to another variant) and then says nothing at all teaches the user the dashboard is broken. The server already names the reason in its error body; throwing that prose away in the console is pure information loss at the exact moment the user needs it.
What changed (motivation -> approach -> change)
Symptom: refusals die in
console.warn. Root cause: neither press has any rendered error surface. Change: generalize the surface rather than adding a second one-off --ErrorNoticerendered above the composer, fed by a singleshowRefusedPress(action, error)helper. Any refusable press added later inherits the behavior by adding one entry to theREFUSED_PRESS_TITLE_KEYSmap instead of re-discoveringconsole.warn.pages.chatPage.could_not_regenerate("Couldn't regenerate") andpages.chatPage.could_not_switch_variant("Couldn't switch version", matching the variant switcher's own "Next/Previous version" labels) -- across the 12 shipped locales, withen-XAregenerated byscripts/gen-pseudolocale.mjs. The key map isas constso the catalog-key gate statically resolves every member.friendlyErrTextchokepoint, soErrorNotice's journal lookup recovers the structured context (endpoint, status, backendcode).Tests
website/src/test/ChatPage.refusedPress.test.tsx(mounted ChatPage, mutation-verified -- all 3 fail with the ChatPage change reverted):Full local battery on the rebased head:
npx tsc -bclean, website vitest 1354 files / 21240 tests green,npm run i18n:checkfully green, eslint 0 errors (no new warnings), brand gate clean.Manual verification
Capture harness (
website/capture/refused-press.{html,tsx}+website/scripts/capture-refused-press.mjs, same pattern as the repo's othercapture/entries) mounts the REALErrorNotice+ChatInputagainst the real stylesheet and live i18n catalog; each scene asserts its rendered text before writing a frame, so a frame cannot contradict the diff.Screenshots / video
Before (dark) -- a refused press shows nothing above the composer:
After -- refused regenerate (dark):
After -- refused switch-variant (dark):
After -- refused regenerate (light)
Related Issues
Closes #4202
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)