Skip to content

feat(chat): surface background streams as per-tab status dots#409

Open
sneumannb5 wants to merge 2 commits into
byte5ai:mainfrom
sneumannb5:feat/lume-stream-toasts-conflict
Open

feat(chat): surface background streams as per-tab status dots#409
sneumannb5 wants to merge 2 commits into
byte5ai:mainfrom
sneumannb5:feat/lume-stream-toasts-conflict

Conversation

@sneumannb5

Copy link
Copy Markdown
Contributor

What

Replace the floating StreamToasts overlay with per-tab status dots on each chat tab, so background streams surface
in-context. Adds ADR 0006. Closes #286

Why

Floating toasts stacked up and detached the stream state from the tab it belonged to. Per-tab dots put liveness where
the user is already looking.

Test plan

  • npm run lint && npm run typecheck && npm run test in web-ui (0 errors, 173 tests pass)

Risk / blast radius

UI-only. No schema, API, or env-var changes. i18n de/en updated.

@sneumannb5
sneumannb5 force-pushed the feat/lume-stream-toasts-conflict branch from fea9770 to 9665ed4 Compare July 3, 2026 06:28

@ConnysCode ConnysCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Request changes

Two things to change before merge — both non-cosmetic. The implementation itself is sound: I verified the removal is clean, the done-dismiss is safe, and i18n parity holds (details below).

Must change before merge

🟠 a11y — error vs done dots are distinguished by colour alone. All three dots share shape and size; running carries a non-colour cue (animate-pulse), but done (accent) and error (danger) differ only in hue. The aria-label/title cover screen-reader and hover users, but a colour-blind operator glancing at the tab strip can't tell "ready" from "failed" without hovering — and ADR-0006 elevates §8 ("colour is never the sole signal") to ship-blocking. Please give the error dot a non-colour cue (e.g. a ring, or a hollow-vs-filled shape) so the distinction survives without hover. Detail inline on ChatTabs.tsx:106.

🟠 The new state logic ships untested. tabStreamState and the done-only dismiss in handleSelect (chat/page.tsx:441) encode the subtlest part of this change — no dot on the active tab, aborted → no marker, forget only done while keeping error/aborted/running. The repo already unit-tests pure logic (29 test files, e.g. i18n-parity.test.ts). Please add a test covering the tabStreamState matrix (active / no-record / running / done / error / aborted) and the handleSelect dismiss rule before merge, so the contract ADR-0006 describes can't silently regress. Detail inline on ChatTabs.tsx:83.

Verified against the head clone

  • Clean removal. No dangling references to StreamToasts or its streamToasts i18n namespace anywhere (grep across the head clone — only docs/** mention it now). No orphaned useTranslations('streamToasts').
  • dismiss('done') on select is safe. Messages render from activeSession.messages and the liveness pill from message.liveness — both independent of the stream record. The only per-session store read on the active tab is agentUnavailableSlug (chat/page.tsx:547), which a done record never carries. Deleting the record on select breaks nothing inline; error / aborted / running are correctly kept so the recovery banner, stop button, and composer lock stay correct.
  • i18n parity holds. streamToasts.* removed from both de.json and en.json; three chatTabs.* keys added to both with matching {title} placeholders, so i18n-parity.test.ts (key-set + placeholder equality) stays green.
  • tabStreamState matrix is correct — active → no dot, aborted → no dot, running / done / error as documented in the ADR.

PR description — factcheck

Claim Reality
"UI-only. No schema, API, or env-var changes." ✅ Only web-ui/** .tsx, messages/{de,en}.json, and docs/** touched.
"i18n de/en updated." ✅ Both locales updated symmetrically; parity test unaffected.
"0 errors, 173 tests pass." Not re-run here (heavy Next.js install skipped). Consistent on inspection — no dangling refs, parity test stays green, types align.
"Closes #286." ✅ Option 2. The issue's illustrative "inline status row in the affected session" is already covered by the existing inline streaming UI + inline errors, per ADR-0006.
Nits (non-blocking)
  • web-ui/app/_components/ChatTabs.tsx:215 — the dot's aria-label / title repeat the session title ("{title}: response ready") while the dot sits inside a tab that already exposes that title, so a screen reader announces the title twice. Consider dropping {title} from the dot label (just "response ready"); the tab context already supplies it.

(Reviewed against head 9665ed4.)

* each dot carries an aria-label + title (§8). */
function streamDotClass(state: TabStreamState): string {
const base = 'ml-1 inline-block size-1.5 shrink-0 rounded-full';
if (state === 'error') return `${base} bg-[color:var(--danger)]`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error and done dots are distinguished by colour alone. All three dots share the same shape and size; running carries a non-colour cue (animate-pulse), but done (accent) and error (danger) differ only in hue. The aria-label / title cover screen-reader and hover users, but a colour-blind operator glancing at the tab strip can't tell "ready" from "failed" without hovering — and ADR-0006 itself elevates §8 ("colour is never the sole signal") to ship-blocking. Please give the error dot a non-colour cue too (e.g. a ring, or a hollow-vs-filled shape) so the distinction survives without hover — that is what makes the §8 claim in the ADR actually hold visually.

* (active tab, no record, or a user-aborted turn). */
type TabStreamState = 'running' | 'done' | 'error';

function tabStreamState(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new state logic ships without tests. tabStreamState here (and the done-only dismiss in handleSelect, chat/page.tsx:441) encode the subtlest part of this change: no dot on the active tab, aborted → no marker, and forget only done while keeping error / aborted / running. The repo already unit-tests pure logic (29 test files, e.g. i18n-parity.test.ts), and both of these are pure and trivially testable. Please add a test covering the tabStreamState matrix (active / no-record / running / done / error / aborted) and the handleSelect dismiss rule, so the contract ADR-0006 describes can't silently regress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lume: stream toasts conflict with the no-toasts rule (visual-spec §7.6)

2 participants