Skip to content

fix(web): address composer banner review follow-ups

MacroscopeApp / Macroscope - UI Consistency failed Aug 31, 2026 in 4m 37s

UI Consistency: 1 issue found

1 finding

  • apps/web/src/components/chat/ChatComposer.tsx:1,738 — the shipped composer now runs its props through ComposerPlayground.local.tsx, a committed review-only fixture. The import is unconditional (so the module ships in the production bundle and the hook executes on every ChatComposer render), and in dev it silently replaces real props (bannerItems, threadSyncPhase, task progress/steps, onSend/onInterrupt, approval, plan and pending-question state) for any thread id starting with banner-playground-. Suggested fix: remove the import and call, and drop ComposerPlayground.local.tsx (plus the publicly served apps/web/public/composer-review-followups.local.html, which embeds internal host URLs) from the branch.

Inline comment posted on ChatComposer.tsx:738.

Details

Note

Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.

Reviewed with no findings

  • ComposerBanner.tsx — Attachment switching to w-[calc(100%-2*var(--chat-composer-drawer-inset))] and dropping max-w-[45.25rem] is equivalent: the only ComposerSurface.Shell call site (ChatView.tsx:7338) is max-w-3xl (48rem), so 100% - 2 × 1.375rem = 45.25rem. Nested attachments inside ComposerBanner.Column are already forced to w-full. Same reasoning for ComposerSurface.ContextStrip losing max-w-181.
  • ComposerBanner.ToggleIcon gaining pointer-events-none — all three call sites (ComposerTasksBadge, ComposerStashMenu, ComposerPendingUserInputPanel) render it as decorative content inside a button/trigger row, so activation still reaches the real control while the borrowed 44px touch hit area no longer extends past the header.
  • ComposerBannerStack.tsx — the Escape handler is now guarded by stackExpanded, which addresses the stale pendingFocusRef case flagged on the previous commit. aria-hidden/tabIndex={-1} on the peek pair with invisible, and focus is moved out of it before it is hidden, so no focus-inside-aria-hidden path was found.
  • ChatComposer.tsx command-menu inset now reads --chat-composer-drawer-inset without the || 1.375 fallback; the variable is declared on the single Shell ancestor of the form, so no reachable NaN path today.
  • ComposerServerUpdateStatus.tsx — ComposerServerUpdateIcon keeps aria-hidden, uses motion-safe:animate-spin for reduced motion, and the ChatView.tsx call site passes the real serverUpdateState.status.
  • role="list" additions on the task and stash <ul> elements are semantics-only and match the Safari list-marker caveat described in the PR.

Out of check scope (not commented on): apps/web/public/composer-review-followups.local.html, referenced in the inline comment only because removing it is part of the same fix.


🔍 Agent activity — 18 tool calls

💭 I'm reviewing the diff and noticing that ChatComposer.tsx now imports a dev-only playground file into what looks like production code, which is concerning, along with a removed fallback value in the drawer inset calculation. I'm tracing whether ComposerBanner.tsx's CSS variable for the drawer inset is actually available if it renders outside the ComposerSurface Shell, then noting that both the attachment banner and context strip previously had a max-w-[45.25rem]/max-w-181 constraint that's now been dropped entirely — a potential layout regression at wide viewports unless the parent form already constrains width. I also want to check if removing the NaN fallback in the drawer inset parsing could break things if the CSS variable is empty. I also see the Escape key handler in ComposerBannerStack was already fixed to check stackExpanded before closing, addressing a previous review comment, so I don't need to raise that again. I'm going to pull the actual current file contents to verify these concerns against the real code.

1. browse_code — apps/web/src/components/chat/ComposerSurface.tsx

2. browse_code — apps/web/src/components/chat/ComposerBannerStack.tsx

💭 I'm noticing an accessibility issue: when the peek is expanded and becomes aria-hidden with tabIndex -1, the toggle control disappears from the accessibility tree, so a screen reader user who just expanded it loses access to the control that would let them collapse it again. On the width calculation: 45.25rem equals exactly 48rem minus 2.75rem, which means the max-width was mathematically redundant when the parent is capped at max-w-3xl — so removing it is fine in that context, but I need to check whether Attachment ever renders under a different parent width, like in ComposerBanner. Then there's the ToggleIcon pointer-events-none change — I should check if that icon is ever used as an actual interactive trigger. But the bigger red flag is the .local.tsx playground fixture being imported directly into the production ChatComposer file, along with a committed HTML fixture in the public folder — the report claims these aren't committed, but they clearly are, and the DEV-gate is only internal to the module, not around the import itself. Also concerning: the playground code runs unconditionally when bundled, calling several hooks and reading location.search on every render, plus pulling in contracts and promptStashStore — real production overhead for what's supposed to be a dev-only tool. Since the include patterns only cover .tsx files under apps/web/src, the committed HTML file falls outside scope, so I'll focus comments on the two TypeScript files. I'm also checking whether the drawer inset parsing in ChatComposer has any fallback — it doesn't, so if the CSS custom property is ever missing, the parsed value becomes NaN and silently breaks positioning. That depends on whether the anchor/form always sits inside the Shell where the variable is declared, which I need to verify before flagging it as a real bug versus a minor robustness gap.

3. browse_code — apps/web/src/components/chat/ChatComposer.tsx

4. browse_code — apps/web/src/components/chat/ComposerBanner.tsx

💭 I'm now checking whether ChatComposer actually imports and calls ComposerPlayground.local as expected.

5. git_tools — ComposerPlayground|useComposerPlayground|.local

6. git_tools — local

💭 I notice *.local.tsx isn't gitignored, so the playground fixture file ended up committed and imported directly into the production ChatComposer — dev-only code shipping in the real bundle and wired into the composer's props pipeline. The report even claims these fixtures aren't committed, which contradicts reality, so this looks like a clear accidental inclusion worth flagging. Next I'm looking at the aria-hidden toggle on Peek — since Peek controls aria-expanded/aria-controls, hiding it from assistive tech while expanded strips the disclosure control from the accessibility tree, so screen reader users can't perceive or toggle the expanded state. I'm also tracing the focus-management logic: after a click, focus moves to the trigger, then a layout effect shifts focus to the first control in the expanded group — but if that group has no focusable elements, focus should fall back to the container itself, so I'm checking whether that fallback actually happens correctly given the timing of the pending-focus effect relative to pointer/click updates. Actually since the peek button is already visually hidden via invisible, it's already excluded from the accessibility tree and tab order, so the aria-hidden/tabIndex marking is redundant rather than a real finding — I'll skip that one. Then there's the focus ring question: the focus-visible outline sits on the collapsing grid container, and since programmatic .focus() calls after a keydown typically still satisfy focus-visible heuristics in Chrome, this seems like a minor concern rather than a real bug. Now I'm checking whether ToggleIcon's pointer-events-none could be an issue if it's actually the clickable element at some call site, so I want to grep for its usages, and I'm also examining whether Attachment/ContextStrip's new width calculation relying on the parent's max-w-3xl still holds everywhere it's rendered, since it's used across multiple composer banner locations that might not share the same Shell wrapper. I should also consider the removed max-w-181 and max-w-[45.25rem] values — with Shell's max-w-3xl these seem equivalent, but if Shell has a className overriding max-width somewhere for a wider layout, that previous clamp mattered. I want to grep for Shell usages to check for such overrides.

7. git_tools — ComposerSurface.Shell|ComposerBanner.Attachment|ComposerBanner.Dock|ToggleIcon

💭 I should check if Shell is used elsewhere, like within ChatComposer itself, and search more broadly for ComposerSurface usage and composer max-width settings. I also want to see whether the banner stack or dock renders outside the Shell, so I'll look at ChatView's structure around that area.

8. browse_code — apps/web/src/components/ChatView.tsx

9. git_tools — ComposerSurface.

💭 With only one Shell, the inset variable is consistently available to nested components, and I'm checking whether ComposerBanner.Attachment could ever render outside that Shell elsewhere, like in the desktop app. I'm now wondering about the command menu's portal into document.body — since that content sits outside the Shell, the CSS variable wouldn't be inherited there, potentially breaking the width calculation if an Attachment renders inside it. I need to check what actually gets rendered inside ComposerCommandMenuLayer to see if this is a real issue.

10. git_tools — ComposerCommandMenuLayer

💭 I'm curious about that NUL byte warning in ChatComposer.tsx — it's making the file appear binary during search. I should check whether this was already present at the merge base rather than something introduced by the PR.

11. git_tools — ComposerCommandMenuLayer

12. browse_code — In apps/web/src/components/chat/ChatComposer.tsx, where is ComposerCommandMenuLayer rendered (its JSX usage) and what children does it receive? Show the surrounding JSX. Also show the JSX where ComposerBannerStack and ComposerTasksBadge/ComposerStashMenu are rendered relative to the composer form element with data-chat-composer-form.

💭 I'm checking whether ComposerStashMenu renders a ComposerBanner.Attachment inside the portal, since that would break inheritance of the --chat-composer-drawer-inset CSS variable since portals attach to document.body outside the normal DOM tree. Actually tracing through the computed style resolution, the drawerInset calculation uses the form or anchor element, both of which live inside Shell, so the variable resolves correctly and won't produce NaN in the normal path—only Shell ever renders the composer, so there's no other host case to worry about. Let me look at ComposerStashMenu directly to confirm.

13. browse_code — apps/web/src/components/chat/ComposerStashMenu.tsx

💭 I confirm ComposerStashMenu doesn't depend on the width variable since it uses ComposerBanner.Root, and the pre-existing padding calc there relies on a globally defined variable, which isn't a concern here. The pointer-events-none on ToggleIcon is just decorative styling inside a button, so that's fine too. Checking the bannerItems order change with activityStackItem prepended to the array — since bannerPriority sorting is stable and the activity item likely has the lowest priority anyway, it should still sort first regardless. Now I'm focusing on the real issue: ComposerPlayground is imported unconditionally at the top of ChatComposer, meaning it gets bundled into production even though DEV checks guard the runtime logic — the hook, its state, effects, and playground cases still ship. Worse, this could hijack the real composer for any thread whose id matches the playground prefix, and a URL query parameter might apply review-case behavior more broadly than intended if the threadId check isn't scoped tightly enough. I also notice the file is named with a .local.tsx suffix, suggesting it was meant to stay untracked, and the PR's own notes say fixtures weren't supposed to be committed — so this looks like an accidental commit. There's also a publicly served HTML file with internal hostnames and file-drop URLs that falls outside the strict file patterns I'm reviewing inline, but worth flagging briefly in the overall summary rather than as an inline comment. I'm also noting a small UI inconsistency in the playground file where a raw span with Tailwind classes is used instead of the shared dot component. Now I'm tracing through the focus-state logic more carefully — checking whether the pendingFocusRef can end up stale when pointer-enter sets it to "notice" while the stack is expanding, and whether the Escape handler's condition properly resets it in every case. Since the peek is invisible and blurred when expanded, this stale-focus scenario seems unlikely in practice given the activeElement guard. But I notice a real edge case with touch: tapping the peek triggers onClick which sets focus then pendingFocus to "notice" before expanding, potentially moving focus unexpectedly on tap interactions rather than keyboard ones. I'm also checking whether mouse clicks on the peek could double-trigger the expansion logic, but since pointer-events are disabled on the peek once expanded, that path seems safe. Now I'm examining the group container's tabIndex and focus-visible outline behavior to see if it interacts correctly with Escape-triggered collapse. Checking the ancestor chain confirms no overflow clipping issue there, and the tabIndex-only focus on the collapsed container seems fine. I'm now noticing a separate accessibility concern: when the peek stays visible while collapsed (not aria-hidden), it still carries aria-expanded and aria-controls, and I need to check whether screen reader users get consistent information in that state. Then I look at the min-h-3 class applied conditionally when expanded on the notices container -- it seems meant to reserve 12px of height, but since the peek is absolutely positioned with bottom-0, I'm working through whether a zero-height collapsed container actually causes the peek to render correctly or get clipped. I also weigh whether to flag the removed || 1.375 fallback as a minor robustness issue. Since --chat-composer-drawer-inset is always defined on the Shell ancestor and there's only one call site, the fallback seems truly dead code, so I lean toward not raising it as noise. Turning to the max-width removal, I'm tracing whether 100% could ever exceed 45.25rem given how Attachment nests inside ComposerBanner.Column and Dock, where child widths get overridden to w-full and the Dock itself is constrained by the form's max-w-3xl — so the width chain seems bounded in practice. Working through ContextStrip's parent chain confirms the same equivalence, so neither width change looks like a real regression. The one finding worth flagging is the committed playground/dev-fixture wiring that overrides the composer's props in production — that's a genuine consistency and regression risk in the changed UI code. More importantly, in dev mode any thread whose id starts with banner-playground- gets its props hijacked, which is only meant for internal review. There's also the code smell of reassigning the props parameter itself via useComposerPlayground, but the bigger concern is committing this at all — I should check whether the repo has any convention around .local. files or existing examples of that pattern.

14. browse_code — **/.local.

15. git_tools — 30175a8..e70ff8e

💭 Those .local.* files are the only new ones, confirming this was accidental. Now I want to diff the head commit e70ff8 against 558f358 to see what's changed since my prior comment, so I can verify whether the escape fix addressed it and check for any new issues.

16. git_tools — 558f358..e70ff8e

💭 This confirms the head commit is the one that added the playground wiring and fixtures, so it's new and worth flagging. I'll post an inline comment on ChatComposer.tsx at line 738, where the playground override call happens, pointing out the import and usage. Since removing this cleanly requires touching multiple files (dropping the import, the call, and deleting the fixture files), there's no clean single-line diff suggestion I can offer — a partial fix would leave an unused import and break lint. So I'll just leave a plain comment describing what needs to be removed rather than a suggested diff, placed at the call site since that's the actual production entry point, and note the other files that should go with it.

17. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 I'm drafting the review comment for line 738, flagging that the dev-only fixture import is unconditional and ships into production, silently overriding real composer props during every render. I catch myself misstating the activation condition — reviewing again, ?reviewCase= alone doesn't trigger it; the thread id must start with banner-playground-, so I correct that before finalizing. Given the clear risk of shipping code that hijacks production composer props, I decide to mark this as a failing check and post the corrected comment.

18. modify_pr — One finding: the production composer now routes its props through a committed dev-only review fixture. Details inline.


Agent Credits: 231 credits

Token usage: 30 uncached input · 1,000,085 cache-read input · 198,082 cache-write input · 18,278 output
Agent Credits may also include non-token charges from external tools such as web research.