Improve accessibility, style, and code-splitting

[Kaiohz]

• Add skip link, ARIA roles/labels, and keyboard navigation
• Lazy-load secondary routes with Suspense fallback
• Extract shared SegmentedToggle component
• Enhance focus rings, status badges, and breadcrumb semantics
• Use proper ellipsis character and hide decorative icons

[Kaiohz]

Review summary

Solid transverse refactor — a11y, code-splitting, and design-system consolidation done with real care (skip link, ARIA roles, focus rings, <time>, <article>, combobox/menu patterns, memo on committed chat messages). One real bug, one fragile pattern, and a few gaps. Score: 7.5/10 — would approve after the inputs fix and a few tests for the new a11y contracts.

Strengths

• Code-splitting: App.tsx now lazy-loads AgentsPage and RagPage with a polite aria-live fallback. Net win on initial bundle.
• SegmentedToggle: clean 3-pattern API (tab / radio / toggle) with proper ARIA propagation, generic over V extends string. Good candidate for the design system.
• Memo on ChatMessage: explicit comparator on agentName / threadId / message ref — exactly the right place to avoid re-rendering committed messages while streaming. This is the kind of thing that's easy to add too early; here it lands at the right scope.
• A11y details that show real intent:
  • Skip link in MainLayout with tabIndex={-1} + outline-none + scroll-mt-24 on <main>
  • aria-label on the logo (Composables logo) + width/height to prevent CLS
  • <time dateTime="..."> wrapping timestamps
  • <article> + min-w-0 + break-words on message bodies
  • aria-current="page" on the active breadcrumb and the active thread
  • aria-live="polite" on loading/processing states
  • Decorative material-symbols-outlined spans consistently aria-hidden="true"
  • Mobile nav with aria-expanded + aria-controls
  • Combobox pattern on WorkspaceSelector with full keyboard nav (Arrow, Enter, Escape) and roving tabindex
  • menu + menuitem roles on IndexActionMenu with arrow-key cycling and Escape-to-close returning focus to the trigger
• Design system harmonization: dialogs now have a consistent header / body / footer pattern with sticky footer; bg-surface-container-lowest baseline; rounded-full buttons matching the brand voice; the … ellipsis character replaces ... everywhere.
• StatusBadge taxonomy is much more useful now (green/orange/blue/red/slate by intent) and the formatLabel helper is the right place to centralize snake_case → Title Case.
• Test updates reflect behavior changes (ellipsis text, menuitem role, badge label casing, heading-level disambiguation).

Issues — must fix before merge

1. Bug: invalid Tailwind class in ui/input.tsx

- "flex h-10 w-full rounded-md border border-input bg-background ..."
+ "flex h-10 w-full rounded-lg border border-outline-variant/60 bg-surface-container-lowest px-3 py-2 text-sm text-on-surface placeholder:text-outline placeholder:text-sm placeholder:end-with-… ..."

placeholder:end-with-… is not a Tailwind class — it'll be silently dropped, so the intent (ellipsizing overflowing placeholder text) doesn't ship. Either implement the ellipsizing via CSS (e.g. text-overflow: ellipsis requires overflow: hidden and a max-width) or drop the class. Same on the flex flex-1 — the flex-1 was lost in the rewrite, so placeholder width behavior changed unintentionally.

2. Missing trailing newline (POSIX)

App.tsx, AppRoot block, AgentConfigForm.tsx, AgentConfigViewer.tsx, CreateAgentDialog.tsx, MainLayout.tsx, ThreadSidebar.tsx, TopNav.tsx, BreadcrumbBar.tsx, FileContentPanel.tsx, PipelineToggle.tsx, RagTabBar.tsx, SegmentedToggle.tsx, StatusBadge.tsx, ui/select.tsx all end with \ No newline at end of file. The repo would benefit from a .editorconfig (insert_final_newline = true) and/or a pre-commit hook (prettier --check or lint-staged). Not a blocker per file, but the volume here is a signal that the toolchain is missing this guard.

3. Tests don't cover the new a11y contracts

The hard work was put into ARIA patterns, but only behavior-text tests were updated. Specifically:

• SegmentedToggle: no test file. Add unit tests for the three patterns (tab / radio / toggle) to lock in role, aria-selected / aria-checked / aria-pressed, and the radiogroup roving tabindex. This is the kind of component that will silently break a11y on the next refactor.
• IndexActionMenu: no test for arrow-key cycling, Escape-to-close returning focus to the trigger, or outside-click closing. The role="menu" change is good but unverified.
• WorkspaceSelector: no test for combobox keyboard nav (ArrowUp/Down, Enter commits, Escape closes).
• ChatMessage memo: add a test that asserts committed messages don't re-render when the parent rerenders with a new message ref appended.
• App.tsx Suspense fallback: no test for the aria-live="polite" "Loading…" surface.
• MainLayout skip link: no test asserting the link targets #main-content and is sr-only by default.

Without these, future refactors will erode the a11y gains.

4. Fragile cross-form submit via form="..." attribute

CreateAgentDialog and AgentConfigViewer both rely on <button form="agent-config-form"> / <button form="agent-yaml-form"> to bind a submit button to a form living in a sibling subtree. This is valid HTML5 but couples the dialog to the form IDs by string match — renaming either ID will silently break submit with no type error.

• AgentConfigForm defines const FORM_ID = "agent-config-form" ✅
• CreateAgentDialog defines id="agent-yaml-form" inline, and references form="agent-config-form" by string ⚠️
• AgentConfigViewer references form="agent-config-form" by string ⚠️

Either lift both form IDs to a shared constants.ts (and import the same constant in the dialog/viewer) or use a ref + requestSubmit() pattern. The current shape will bite a future refactor.

Nits / nice-to-haves

• TopNav desktop nav: <NavLink> doesn't render aria-current="page" on the active link. Add it to match the breadcrumb / thread sidebar pattern and your mobile nav already shows the active state via class only.
• ThreadSidebar Escape listener: redundant. The new Dialog primitive already closes on Escape. Drop the useEffect + document.addEventListener block.
• MemoChatMessage comparator: prev.message === next.message compares by reference. If Message is ever mutated in place upstream (e.g. streaming state updates the same object), the memo will incorrectly skip rerender. Worth a comment in the file explaining the immutability contract, or a structural-equality comparator on a stable id field.
• AgentConfigForm mobile submit footer: the comment "Mobile submit footer is rendered by the standalone footer below" is misleading — it's rendered in AgentConfigViewer, not in the form. Tighten the comment or move the button.
• CreateAgentDialog onSuccess now calls handleClose() instead of onOpenChange(false): good consolidation, but the form name input and file input reset should also be unit-tested. Right now, if someone removes setName("") from handleClose, no test will fail.
• StatusBadge taxonomy: no tests for the new variants (approved, streaming, in_progress, error, failed, standby, off). Easy add and worth locking in.
• Dialog overlay: bg-black/40 backdrop-blur-sm is a nice touch, but be aware that some screen readers consider fully-opaque overlays better for focus trap signaling. Worth a quick a11y audit with NVDA/VoiceOver before shipping to all users.
• App.tsx: the double-default-export pattern (App for routes, AppRoot wrapping in Suspense for default export) is a little confusing. A short comment near the exports explaining the boundary would help.

Nice work on the non-code stuff

• Branch is 1 commit, 0 behind main — first PR in a while that doesn't need a rebase. 👏
• PR body is concrete and honest about what changed. Don't lose that habit.
• The aria-label="..." on the mobile menu button that flips between "Open menu" / "Close menu" is exactly the right pattern.

Bottom line

Approve once the placeholder:end-with-… bug is fixed and at least the SegmentedToggle tests are added. The rest can land in follow-ups. This is the kind of PR that compounds — the a11y infrastructure here will pay back on every feature built on top of it.

[Kaiohz]

Thanks for putting this together — a substantial and well‑scoped PR. 140 files, two commits, real semantic value (lazy routes, a11y, focus management, SegmentedToggle extraction, ChatMessage memoization), and a clean reformat on top via .editorconfig + .prettierrc. CI is green. Below is my review.

Highlights 💪

• App.tsx — lazy routes + PageFallback. Splitting AgentsPage / RagPage into React.lazy chunks is a clear win for the initial bundle, and exporting PageFallback lets the dedicated test file assert on its role="status" / aria-live="polite" contract. Nice.
• SegmentedToggle extraction. Generic over the value type, three ARIA patterns (tab / toggle / radio), and a 158‑line test suite that covers the relevant ARIA attributes and roving tabindex. PipelineToggle now reads as a 12‑line wrapper — exactly what a shared primitive should look like.
• MainLayout — skip link + focusable <main>. id={MAIN_ID} + tabIndex={-1} + outline-none is the right combo. The sr-only focus:not-sr-only skip link is properly layered above (z-[100]).
• TopNav — mobile menu. The aria-expanded / aria-controls / aria-label toggling between "Open menu" and "Close menu" is correct, and the menu auto‑closes on link click. The active state moved to bg-secondary-brand/5 rather than a border — that was probably needed to keep the drawer height consistent.
• BreadcrumbBar. <ol> semantics, aria-current="page" on the last segment, flex-wrap + truncate for narrow viewports. Clean.
• StatusBadge. Now maps 12+ statuses to 6 semantic colors, formats snake_case → "Awaiting Hitl", and accepts a className for layout. The "default" fallback moved from slate-100/600 to surface-container-high/on-surface-variant — better integration with the theme.
• IndexActionMenu — combobox‑style menu with full keyboard support. aria-haspopup="menu", role="menu" / role="menuitem", roving tabindex, Arrow / Home / End (wraparound), Escape returns focus to the trigger, outside click closes. 151 lines of test cover the full path.
• WorkspaceSelector — listbox combobox. role="combobox" + role="listbox" + role="option", aria-expanded is false when filtered results are empty (important), Arrow keys move focus, Enter commits, Escape closes, focus returns to the input. The useEffect on [value] is the right fix for the parent‑controlled input drift.
• ChatMessage memoization. memo(ChatMessageImpl, customAreEqual) keyed on the whole Message reference is the right call here: MessageList re‑renders on every streaming tick, but committed messages don't need to. The agentName + threadId comparators are belt‑and‑suspenders — given the test setup it would be worth a one‑line comment in the code too.
• Tests. ~25 test files updated to match the new ARIA contracts (and a lot of Prettier reformat — see "Caveats" below). New tests for PageFallback, SegmentedToggle, WorkspaceSelector, IndexActionMenu. Quality of the new tests is high — they assert on roles, focus, and ARIA state, not just text presence.

Suggestions / nits 🔧

1. WorkspaceSelector — requestAnimationFrame focus pattern is subtle. The keyboard handler reads activeIndex (the old value) to compute the next one, then defers focus() via rAF. It works, but it relies on React batching the state update before the next paint. A safer pattern is the roving tabindex approach used in SegmentedToggle (radio): set tabIndex={idx === activeIndex ? 0 : -1} on each option, and let the user agent's Tab + the setActiveIndex + focus() triple handle it. Or compute nextIdx once, before the state update, and use it for both setActiveIndex and the rAF call:

   if (e.key === "ArrowDown") {
     e.preventDefault();
     const next = activeIndex === count - 1 ? -1 : activeIndex + 1;
     setActiveIndex(next);
     requestAnimationFrame(() => itemRefs.current[next]?.focus());
   }

   The current code recomputes nextIdx from the old activeIndex inside rAF, which is fine when there are no intermediate renders — but it's a code smell worth refactoring.

2. IndexActionMenu — aria-current is the wrong attribute on menuitem. The WAI‑ARIA menu pattern uses aria-checked (for checkable items) or nothing (for commands). For a plain action menu, aria-current on a menuitem will trigger false positives in screen readers — e.g. some will announce "current menu item" on every item that has it. Drop it; focus management alone is sufficient.

3. SegmentedToggle — aria-pressed default for "toggle" pattern is OK, but consider an explicit tabIndex story. For a toggle group of buttons (not a radiogroup), there is no roving tabindex, so each option is in the natural tab order. That's fine for binary/2‑option toggles, but for 4+ items, consider documenting the recommendation in the JSDoc (e.g. "for >3 options, prefer the radio pattern with roving tabindex"). Right now both patterns look symmetric in the docs.

4. StatusBadge — normalization duplicates itself. normalized === "in_progress" || normalized === "in progress" and awaiting_hitl || "awaiting review". If you want to be defensive, normalize the input first (split on _ / space, lowercase) before the switch — saves 4 lines and handles "awaiting-hitl", "Awaiting Hitl", etc. the same way. Tests still pass; lower risk of drift.

5. ChatMessage memo — reference equality on Message. It works if the upstream code path (TanStack Query / WebSocket) returns the same object reference when nothing changed, which is typical, but worth a one‑line comment in the file explaining the assumption. Otherwise a future "normalize the timestamp" refactor could silently break memoization.

6. ThreadSidebar — useEffect for Escape is redundant. The <Dialog> from your ui/dialog.tsx already handles focus trap + Escape; the manual keydown listener duplicates it and adds a useEffect for nothing. Remove it, and trust the primitive. (If your Dialog doesn't actually trap focus yet, that's a separate follow‑up — but then fix the primitive, not this consumer.)

7. Prettier reformat mixed into semantic PR. About 50% of the 140 files are pure whitespace / import‑order changes. Two commits is already a good start, but for future PRs consider making the reformat the first commit (auto‑mergeable, no review needed) and keeping the second commit purely functional. It also makes git blame and the diff review much easier to follow.

8. App.tsx — ChatPage is not lazy. That's intentional (default landing page → no point deferring it), but it would be worth a one‑line comment so the asymmetry doesn't read as an oversight to the next person.

9. Accessibility — MainLayout skip link target is the <main> element. Make sure that no part of the page later changes the focus order in a way that puts interactive widgets before the <main> (e.g. an inline notification that auto‑focuses). Not an issue today, but the scroll-mt-24 you added suggests you're already thinking about it.

10. Minor — … (U+2026) is now used in 4+ places. Good. Make sure your .editorconfig charset is enforced in CI (Prettier's endOfLine: "lf" is set; maybe add a eslint rule for the ellipsis if you have one).

Caveats

• A handful of test files have large negative deltas (-119 for AgentConfigViewer.test.tsx, -48 for AgentConfigForm.test.tsx, etc.). I spot‑checked them and they are all Prettier reflow (multi‑line screen.getByX(...) collapsed to a single line, import lists collapsed) — no test cases were dropped. Worth a quick git diff --stat to confirm, but the CI passing is the strongest signal.
• I did not run the project locally; review is based on the diff + your CI run.

Score

8.5 / 10 — solid a11y work, real architectural improvements (lazy routes, SegmentedToggle, memoization, primitive extraction), and tests that prove the new contracts. The two nits that cost half a point each are the IndexActionMenu aria-current misuse and the WorkspaceSelector rAF dance. Both are easy to fix in a follow‑up commit on the same branch.

Overall: ✅ ready to merge after the aria-current fix on IndexActionMenu. The other items are nice‑to‑haves you can pick up in follow‑ups.