Skip to content

fix(session-ui): expose streaming assistant steps - #51084

Open
aimansour wants to merge 1 commit into
anomalyco:devfrom
aimansour:streaming-steps-a11y
Open

fix(session-ui): expose streaming assistant steps#51084
aimansour wants to merge 1 commit into
anomalyco:devfrom
aimansour:streaming-steps-a11y

Conversation

@aimansour

@aimansour aimansour commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Issue for this PR

Fixes #54652

Continuation of #33137 / #46396 and the streaming-content part of #41408. Supersedes #48597.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Assistant rows are hidden from screen readers while a response is generating. Removing the active-turn aria-hidden makes the visible text and tool steps available to browse during generation.

The regression covers text and a tool button in an incomplete response. The existing grouped background-shell test also uses normal role navigation, removing its obsolete includeHidden workaround. Automatic status announcements remain a separate follow-up.

Updated against dev at ed74ec82ce1fe3a36da6c0815a935473ef4bb0fb on 2026-10-26.

How did you verify your code works?

  • Restoring the old aria-hidden makes both affected cases fail; they pass with the fix.
  • Production-build accessibility and timeline-stream suites: 60 passed. Both affected cases repeated 10 times at 5 workers: 20 passed, no retries.
  • bun run check and the affected packages’ typecheck:tsc pass. Package tests: session-ui 313; app 881 unit + 231 browser, all passing.
  • Production streaming benchmark before/after: all 40 deltas delivered, no row or Markdown replacements.
  • typecheck:e2e has the same 11 existing diagnostics on unmodified dev and this change; no new diagnostics.

Manual NVDA validation on Windows: assistant text and the shell button/output remain readable and operable while the response is still generating.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72" role="img" aria-labelledby="title">
  <title id="title">Streamed text and tool steps remain accessible during generation</title>
  <style>@keyframes stream{50%{stroke-dashoffset:18}}@media(prefers-reduced-motion:no-preference){.stream{animation:stream 2s ease-in-out infinite}}</style>
  <g fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
    <rect x="10" y="9" width="151" height="54" rx="8" opacity=".25"/>
    <path d="m22 24 5 4-5 4m11 0h12" opacity=".8"/>
    <path d="M56 28h91M23 44h122M23 53h89" opacity=".65"/>
    <path class="stream" d="M56 28h91" stroke-dasharray="6 6"/>
    <path d="m181 29 11-8v30l-11-8h-8V29z" opacity=".8"/>
    <path d="M202 28q10 8 0 16m7-22q17 14 0 28" opacity=".55"/>
    <path d="m211 57 5 5 12-13"/>
  </g>
</svg>

Screenshots / recordings

Visual rendering is unchanged. The regression checks the accessibility tree during an active response.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

github-actions Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@aimansour aimansour mentioned this pull request Sep 27, 2026
Closed
3 of 6 tasks
@thdxr
thdxr changed the base branch from v2 to dev October 10, 2026 19:16
This was referenced Oct 12, 2026
Open
Closed
Closed
Closed
@aimansour

aimansour commented Oct 12, 2026

Copy link
Copy Markdown
Author

Updated this PR to Fixes #54652, the open V2 continuation of #33137 / #46396 and Problem 1 of #41408. The latest V2 source still contains the same aria-hidden barrier; the affected renderer is identical on current dev.

AnimatedNumber is tracked separately in #54653 and is outside this patch. The three closed V1 reports now point to the corresponding V2 issues. Kept the maintainer-selected dev base and removed the outdated non-default-base / needs:issue explanation from the description. Existing validation remains documented as historical validation of the submitted change.

@aimansour aimansour mentioned this pull request Oct 12, 2026
Closed
@pranam-s

pranam-s commented Oct 20, 2026

Copy link
Copy Markdown

Thanks for taking this on. The aria-hidden removal is the part I was missing.

Two notes from testing with NVDA on the web UI, in case they affect the PR:

  1. The role="status" "Working" chip at packages/app/src/session/timeline/message-timeline.tsx:411 cannot announce anything as it stands. It is inside a <Show>, so the region is created along with its text, and the text itself is in aria-hidden spans from TextShimmer, which leaves the live region with empty text. That is independent of this change, but it means "Working" is currently the only thing a screen reader hears before going silent.

  2. The includeHidden: true in packages/app/e2e/regression/timeline-stream.spec.ts:722 is there because of the aria-hidden this PR removes, so the flag can probably go once this lands.

I'm not opening a separate PR for #54652 since this one covers it. I can extend this PR, or open a follow-up that adds a polite status live region outside the virtualized rows. Which would you prefer?

@pranam-s pranam-s mentioned this pull request Oct 20, 2026
Open
1 task done
@aimansour
aimansour force-pushed the streaming-steps-a11y branch from 08d809a to 14cafab Compare October 26, 2026 09:08
@aimansour

aimansour commented Oct 26, 2026

Copy link
Copy Markdown
Author

@pranam-s Removed the obsolete includeHidden workaround from the grouped background-shell test. I agree the “Working” live region needs a separate follow-up; this PR stays focused on exposing the streamed content for browsing.

Rebased onto current dev (ed74ec82c). Local checks pass, including 60 production E2E cases and 20 repeated runs of the affected cases. Manual NVDA testing also confirmed the text and shell output are readable and operable during generation.

Could a maintainer approve the pending check, check-test, and check-nix workflow runs and review this for merge? This accessibility fix has been waiting since September, and the barrier is still affecting screen-reader users.

@digitARTI digitARTI mentioned this pull request Nov 13, 2026
Closed
3 tasks done

This branch has not been deployed

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

Reviewers

@Hona Hona Awaiting requested review from Hona
@Brendonovich Brendonovich Awaiting requested review from Brendonovich

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(session-ui): V2 still hides streaming assistant content from screen readers

2 participants

@aimansour @pranam-s