Repository navigation
UI Consistency: 2 issues found
Two findings posted as inline review comments.
apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx— the disclosure panel lost its horizontal/bottom padding in the migration toComposerBanner.Body(which pads only the inline start).CollapsiblePanelclips withoverflow-hidden, so thefocus-visible:ring-1on thew-fulloption buttons is now shaved on the end edge of every option and the bottom edge of the last one — the exact regression the removed comment documented. Suggested fix:<CollapsiblePanel className="pe-1 pb-1">.apps/web/src/components/chat/ComposerBanner.tsx—Attachmenthardcodes the drawer geometry (w-[calc(100%-2.75rem)],max-w-[45.25rem]) while the PR deletes the--chat-composer-drawer-insetdeclaration fromindex.css. The token still has an imperative consumer atChatComposer.tsx:200(getComputedStyle(...).getPropertyValue(...)for the command-menu layer), which now permanently falls back to a duplicated1.375literal. Suggested fix: keep one owner — either consume the custom property here or remove the dead read and share a constant.
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.
Methodology and scope
- Reviewed all changed files matching the check's include patterns (
apps/web/src/**/*.{ts,tsx,css}), plus directly affected call sites (ChatView.tsx,ChatComposer.tsx,index.css) at head74d4ba6. - Traced deleted CSS for live consumers before treating anything as dead:
.chat-composer-shoulder-tabis still a functional marker (ChatView.tsx:7180group-has-[.chat-composer-shoulder-tab]/composer-stack:pb-4);.chat-composer-drawer-slot/.chat-composer-top-drawerhave no remaining literal, dynamic, or imperative consumers;data-thread-sync-drawerandshoulderTabReserveare fully removed with their call sites. Only--chat-composer-drawer-insetwas deleted while agetComputedStyleconsumer remained (reported). - Checked the
:has(:is(.chat-composer-top-drawer, .chat-composer-shoulder-tab))→:has([data-composer-banner-surface="attached"])rewrite against the new markup: every attached surface (ComposerBanner.Rootdefault placement) emits that attribute, andComposerBannerStack, the top drawer, tasks drawer, stash menu, and shoulder dock all route through it, so the glass-shell split still triggers even thoughexternalComposerDrawerAttachedno longer includes thread sync.
Checked and not flagged
- Scroll ownership: the hand-rolled
overflow-y-auto overscroll-contain tabIndex={0}task list was replaced by the sharedScrollArea(ComposerBanner.Scroll), which keepsoverscroll-contain, viewport focus ring, and dynamic (not always-on) edge fades viascrollFade— this matches the check's ScrollArea rule. - Primitive reuse: raw elements introduced by
ComposerBannerimplement semantic rows/list items rather than recreatingButton/Input;DismissandToggleIconroute throughButton/buttonVariants.ComposerBanner.Rowpreserves cursor, focus-visible outline, andrender/mergePropsforwarding for the Base UICollapsibleTriggercomposition. items.toSortedtypechecks (apps/weblib: ["ES2023", ...]), and the newbannerPriorityordering keeps background liveness (priority: "activity") ahead of passive notices, preserving the Stop affordance rule the deleted comment described.- Deleted test files (
ComposerBannerStack,ComposerStashBadge,ComposerTasksBadge,ThreadSyncStatusPill) cover components that were restructured or removed; the stack's new stateful expand/collapse has no replacement coverage, but per the check's guidance this is a request-for-tests judgment rather than a concrete violation, so it was not raised as a finding.
🔍 Agent activity — 35 tool calls
💭 I'm scanning the diff for UI consistency issues and spot a likely logic bug in ChatComposer.tsx: the condition checking whether to close the stash menu inverts the intended check, since
!stashQueue.some(c => c.id !== entry.id)only closes the menu when there are no other entries left, which seems backwards from what the code probably intends. I also want to verify whyhasBlockingComposerTopDrawerswitched from checking mobile-collapsed composer state to just viewport width, check whetherComposerBanner.Root's ref forwarding still works correctly under the React version in use, and confirm whetherComposerBanner.Dismisslost anaria-labelthat was present in the original stash menu. I'm also noting a few smaller inconsistencies: the working-status label is missingrole="status"unlike the sync one, the stash badge row correctly gets a pointer cursor when rendered as a button, and the banner stack's use oftoSortedshould be fine given the app's Node/ES target. Looking further, the peek button in the banner stack now expands viaonPointerEnteron the notices container rather than the whole group, which seems reasonable, and it's wired up with properaria-controls. TheInfoIconusage in ChatView looks fine too. I'm now checking what was removed alongsideshoulderTabReserveand its associated test. For index.css, the shoulder-tab class rules got removed and replaced with adata-composer-banner-surface="attached"selector in a few places, but I notice the actual class names are still being applied in the TSX for the stash and tasks badges — so I need to verify those class references aren't now dangling with no matching CSS definition. Also flagged: deleted tests for ComposerBannerStack, ComposerStashBadge, ComposerTasksBadge, and ThreadSyncStatusPill. Since this PR swaps hover-based expansion for state-driven expansion with pointer, focus, and Escape handling, deleting all coverage without replacement is a real gap worth calling out under the rule requiring focused tests for behavioral and keyboard-handling changes. I want to check what tests currently exist in the repo, then look at ComposerStashMenu -- it dropped Command/CommandList in favor of a banner list, which seems like an intentional swap rather than a primitive violation. But I'm concerned about keyboard navigation: the effect that ensured a default highlighted entry was removed, so if highlightedEntry starts null, arrow-key navigation might compute an index of -1 and behave unpredictably. I need to pull up the actual file to verify.
1. browse_code — apps/web/src/components/chat/ComposerStashMenu.tsx
2. browse_code — apps/web/src/components/chat/ComposerBanner.tsx
💭 I'm checking that the stash row structure correctly overlays a clickable restore button beneath a dismiss button, and confirming the row's mouse/focus handlers merge properly through props. Now I'm verifying the highlightedId fallback logic finds a valid entry when items change, and confirming the CSS selectors for banner rows and the close button's accessibility attributes are correct. Next I want to check ChatComposer.tsx for logic controlling top drawer blocking, shoulder tabs, and inline stash/task badge visibility.
3. git_tools — showShoulderTabs|hasShoulderTab|showInlineTasksBadge|showInlineStashBadge|inlineTasksBadge|inlineStashBadge|hasBlockingComposerTopDrawer|isMobileViewport|isComposerCollapsedMobile
💭 That grep warning about a binary file is odd — the file might contain a null byte. I should look at the specific lines directly instead.
4. browse_code — apps/web/src/components/chat/ChatComposer.tsx
💭 I'm checking whether the shoulder-tab class for the tasks badge still has its styling defined in index.css, since it looks like that rule may have been removed.
5. git_tools — chat-composer-shoulder-tab|chat-composer-tasks-tab|chat-composer-stash-tab|chat-composer-drawer-slot|chat-composer-top-drawer|chat-composer-drawer-attached
💭 The shoulder-tab class is actually still referenced as a styling hook in ChatView.tsx, so that one's not dead code after all. Now I want to check whether
chat-composer-stash-tabandchat-composer-tasks-tabhad CSS rules at some earlier point that got removed, or if they've always been unused—I'll look at the merge base version of index.css to compare.
6. git_tools — chat-composer-stash-tab|chat-composer-tasks-tab
7. browse_code — apps/web/src/index.css
💭 I'm checking which composer-related CSS classes still have consumers after the changes—
chat-composer-shoulder-tabremains used by ChatView, butchat-composer-tasks-tabandchat-composer-stash-tabappear to have no remaining consumers now that shoulderTabReserve and its tests were removed. I'm verifying the shoulder tab stays properly nested within the composer-stack group hierarchy in ChatView, and confirming ComposerBanner.Root's default width behavior correctly applies flex-1 as a direct child. I'm also double-checking whether ComposerTasksBadge's inline rendering now uses a bare button through Row's render prop instead of the previous Button component. I'm confirming that decorative icons remain properly hidden from accessibility trees, and that the working-status row correctly omits an icon slot while padding adjusts via the group-not-has selector. I'm also verifying that the sync status pill's role="status" and label are preserved equivalently even though the markup structure changed slightly, then moving on to check what tests were removed. Looking at the added files—ComposerActivityStatus.tsx, ComposerBanner.tsx, ComposerServerUpdateStatus.tsx—against the deleted test files, I don't see any replacement tests in the diff for the chat components despite a real behavioral shift toward stateful stack expansion with keyboard and pointer handling. That's worth flagging, so I want to check whether any tests still exist in the chat directory at head.
8. browse_code — apps/web/src/components/chat/*.test.tsx
9. browse_code — apps/web/src/components/chat/ComposerTasksBadge.tsx
💭 I'm tracing how tailwind-merge resolves the conflicting className props here—since
flex/hiddenbelong to the same display group, the later class wins while the responsivesm:flexvariant stays intact, and similarlyw-20overridesw-10since they're both width utilities. The aria attributes on TaskSegments and the list markup look fine—ul with aria-label is valid even without an explicit list role. Now I want to check the full head of ComposerBannerStack.tsx to see the interaction changes there.
10. browse_code — apps/web/src/components/chat/ComposerBannerStack.tsx
💭 I'm tracing through the ComposerBannerStack layout: the peek button sits absolutely positioned within a container that collapses to zero height when notices are hidden, so its bottom-anchored 12px height ends up overlapping the front banner beneath it—I need to check how flex-col-reverse affects that stacking order. There's also a keyboard accessibility concern: the peek button stays mounted in the DOM even when expanded, just with opacity-0 rather than pointer-events-none, meaning it could still be tabbed to and might intercept clicks over the bottom 12px of the expanded content area. I'm also checking that pointer/touch interactions collapse and expand correctly through the pointerEnter, pointerLeave, and pointerdown handlers, and confirming that group-hover-based expansion behavior might regress compared to the previous implementation. I need to verify
toSortedis safe given the TS lib target, and it looks likefindLastis already used elsewhere so ES2023 support should be fine. I'm also noting thatrole="alert"moved to the outer Root container, which seems like an acceptable change, and I want to check what replaced the previousAlertprimitive component for banner items. Actually, using local ComposerBanner subcomponents built on Button/ScrollArea seems intentional as a new shared primitive rather than a violation of preferring core UI. I want to check React version compatibility for ref forwarding on the Root component's div props, then move on to scanning ChatComposer's rendering logic near where inline badges were removed, looking for missing elements like inlineTasksBadge. I'm now tracing the conditional that renders inlineTasksBadge inside an activity strip only when there's no activityStackItem, checking that the gating logic (showInlineTasksBadge depending on hasBannerItems, showComposerTopDrawer, etc.) still correctly matches the badge's visibility conditions after the refactor. Now I'm scanning the new Tailwind classes on ComposerBanner.Attachment, noticing hardcoded spacing values replacing what should probably be design system tokens. The stacked items live inside the Attachment wrapper, so that's fine either way. I notice the hardcoded negative margin duplicates the samecalc(1rem + 1px)value already defined as the CSS variable on the Root's surface class, which is a bit redundant but not broken — though the variable isn't scoped where the Attachment could reference it directly, so switching to the variable form isn't straightforward. Now I want to check ChatView.tsx around line 7180 for the composer stack, specifically the padding-bottom group-has selector and whether banner stack items still render inside the form. It looks like ChatView previously had a conditional branch renderingComposerBannerStackeither standalone or with a relative z-0 class, and that branch seems to have been removed entirely in favor of passing bannerItems into ChatComposer directly — I should pull up that region to confirm.
11. browse_code — apps/web/src/components/ChatView.tsx
💭 I'm noting that the attached-drawer condition now covers multiple banner sources beyond the composer drawer—activity strip, tasks drawer, top drawer, stash menu, shoulder tabs—and confirming the shell-attached class still works correctly since the CSS selector matches any element with the attached banner surface data attribute, not just the specific class. Now I want to check the ChatComposer render region around lines 3400-3600 to see the form and stack ordering, and to locate where ComposerStashMenu actually renders.
12. browse_code — apps/web/src/components/chat/ChatComposer.tsx
💭 I notice the mobile-collapsed layout got unified with the desktop one, dropping the separate compact styling in favor of a single layout with a data attribute controlling focus-capture behavior—seems like an intentional consolidation. Now I'm checking how the pending-approval branch behaves in the previously-collapsed mobile state. Then I'm scanning the stash badge shoulder tab for regressions: its pointer handlers, aria-label, and cursor styling all carry over fine, but the hit target height shrank from 32px to 28px on mobile, which is below the 44px coarse-pointer guidance and slightly worse than before—worth flagging as a marginal regression. Next I look at the dismiss button, which now stays visible in each stash row instead of appearing only on hover, an intentional visual change. I check keyboard navigation and scroll behavior in the stash menu for edge cases, then start digging into ChatComposer.tsx's stash queue logic to look for an actual defect. Stash queue check confirms it's correct as analyzed. I move on to the dismissServerUpdateFailure WeakSet logic in versionSkew.ts and how ChatView tracks dismissed server update state, which looks fine. Then I question whether ComposerServerUpdateStatus's narrowed prop type actually matches how ChatView passes serverUpdateState into the banner, and I need to check the surrounding branch logic to confirm it's properly narrowed.
13. browse_code — apps/web/src/components/ChatView.tsx
💭 I'm checking whether TypeScript can actually narrow the type based on those derived boolean consts like
updateInProgressandupdateFailed, since that's not the same mechanism as inferred type predicates on functions—need to figure out what's really happening here. Actually the real issue I'm spotting is in the dismiss logic:dismissServerUpdateFailurealso callsdismissVersionMismatchwith a key that could be null, and the condition guarding whetheronDismissgets passed changed between versions in a way that might let a null key slip through. I need to check thedismissVersionMismatchsignature in versionSkew.ts to see if it actually accepts null or if this would be a type error.
14. browse_code — apps/web/src/versionSkew.ts
💭 I'm noting that dismissVersionMismatch accepts null now. There's a subtle edge case where identity-based dismissal via WeakSet could fail if the same failed state gets re-emitted as a new object, but that's more of a behavioral concern than a UI consistency issue, so I'll set it aside. Now I want to circle back to the specific concerns raised in the check. Looking at the first candidate rule violation: ComposerBanner.ToggleIcon renders a decorative span styled with button variants inside an actual button, which seems fine since it's non-interactive and nested properly. The more interesting case is ComposerStashBadge, which overrides a Button's micro size with a call-site class that forces text-xs at both base and sm breakpoints — likely to defeat the primitive's smaller sm:text-[10px] default. That looks like exactly the kind of call-site typography override the rule flags, so I want to check whether this same override pattern shows up at other call sites too. I'm also noticing that removing the
.chat-composer-shoulder-tabglass styling from index.css while keeping the class name in TSX as a pure marker for agroup-has-[]selector in ChatView is a bit fragile but seems intentional. More importantly, I'm now suspecting a real layout bug: shoulder tabs used to be absolutely positioned and overlap the composer top, but now the Dock sits in normal flow and adds real height above the composer, which could clash with how the draft-hero headline is positioned at the bottom. I'm also spotting a genuine accessibility issue inComposerBannerStack— the peek button stays clickable and focusable even when hidden viaopacity-0, and when collapsed, the peek cap is only 12px tall, which is far too small a touch target and the only way to reveal hidden banners on touch devices since hover doesn't apply there. It's actually better than before, when touch had no affordance at all, but this is still worth flagging since the codebase seems to already have a convention for coarse-pointer hit targets (likepointer-coarse:after:min-h-11) that could be applied here. I want to check whether that pattern exists elsewhere in the codebase to ground this as a concrete, actionable suggestion.
15. git_tools — pointer-coarse
16. browse_code — apps/web/src/components/chat/ComposerStashBadge.tsx
💭 I notice the inline branch of ComposerStashBadge only changed its className line, keeping the icon and count intact. The inline Button also got a new className adding fixed text sizing, which overrides the micro size setting from before. Font size isn't something the check explicitly flags for large call-site overrides, so this seems like a low-priority observation. The real reason for forcing text-xs is likely that Count previously had an explicit small size that's now inherited from the button's micro variant, so they're compensating. I'm also noting that the CSS custom property for icon column width isn't defined locally on this inline Button since it's outside ComposerBanner.Root, but it still works because of CSS inheritance and the fallback value. But if this inline badge is actually nested inside a banner's Root elsewhere (like the top drawer's Body at line 3506), then it would pick up the Root's actual defined value of 1.75rem instead of the fallback, giving the count an unexpectedly wide 28px minimum width inside what's meant to be a compact micro button — a context-dependent inheritance quirk worth flagging, though it's plausible the intent was to align counts to the icon column in banner rows generally. I want to step back and focus on the strongest, most defensible findings rather than speculative inheritance claims I can't verify without rendering. Looking at a few more likely-problematic spots: in ComposerActivityStatus.tsx there's an asymmetry where the sync branch has role="status" but the working branch doesn't, though that seems intentional and consistent with prior behavior; and in ComposerServerUpdateStatus.tsx there's a nested role="alert" span sitting inside a ComposerBanner.Root that's already role="alert", which could be a real duplication issue worth examining further. I should verify whether the Tooltip component actually supports a
triggerIdprop by checking its implementation. Separately, I'm eyeing a possible regression inChatComposeraroundhasBlockingComposerTopDrawer— I need to check whether its logic checking active approvals and pending user inputs actually changed from before, or whether the diff just moved code around.
17. git_tools — triggerId|closeOnClick
💭 I notice tooltip.tsx doesn't have
triggerIdorcloseOnClickdefined. Let me check that file directly.
18. browse_code — apps/web/src/components/ui/tooltip.tsx
💭 I'm checking whether base-ui's Tooltip.Root actually exposes a
triggerIdprop, recalling that newer versions added support for multiple triggers viahandleand related APIs. I'm also unsure whetherTooltip.TriggersupportscloseOnClick, so I should grep the codebase for existing usages of these props to confirm the actual API.
19. git_tools — closeOnClick|triggerId
💭 I notice line 1043 in PullRequestCodeTab.tsx uses closeOnClick set to false, and I want to check what element that's attached to — possibly another TooltipTrigger — so I'll look at that spot.
20. browse_code — apps/web/src/components/pullRequest/PullRequestCodeTab.tsx
💭 I'm questioning whether
closeOnClickis actually a valid prop on Base UI's Tooltip.Trigger, since it looks like it might be borrowed from Menu.Item instead. I want to verify by checking the base-ui package version in the project and recalling its documented Tooltip.Trigger props. Actually, I now recall Base UI's Tooltip.Trigger does have acloseOnClickprop to control whether clicking dismisses the tooltip, so I won't flag that as an issue. Now I want to focus on what actually matters for this review and narrow down to a few solid findings. I'm looking back at the removed CSS controlling the composer drawer inset and width calculations, checking whether the removed variables (inset, attachment overlap, max-width/margin formulas) are actually replaced equivalently elsewhere or if their removal breaks the drawer layout. I'm checking whether--chat-composer-drawer-insetis still referenced anywhere else in the codebase before concluding it's safe to remove.
21. git_tools — chat-composer-drawer-inset
22. git_tools — chat-composer-attachment-overlap
💭 The line numbers don't match what I saw earlier, so those attachment-overlap references at 712, 739, 746 must belong to a different section, maybe the context strip rather than the banner stack. More importantly, I'm noticing that ChatComposer.tsx line 200 reads the drawer inset CSS variable via getComputedStyle, which matters for whatever the PR is changing. The PR actually deletes the CSS rule defining that variable on the composer form data attribute, and grepping for it elsewhere in index.css turns up nothing else — so this variable would now be undefined, making getPropertyValue return an empty string at runtime. This looks like exactly the kind of missed downstream consumer the deletion check warns about, so I want to look at ChatComposer.tsx around lines 140-230 to understand how that read is used.
23. browse_code — apps/web/src/components/chat/ChatComposer.tsx
💭 There's a fallback of 1.375 in the code, so removing the CSS variable doesn't break anything—it just falls back to the same value, though now it's silently relying on a dead reference that happens to match. The geometry still checks out: attachment width equals 100% minus twice that value, matching the command menu inset, so it's consistent but worth flagging as a low-severity note about the unused variable. This is a genuine ownership gap worth flagging as finding #1: the token was deleted but its consumer remains, meaning attachment and menu geometry could silently drift apart since they're now hardcoded independently rather than sharing a source of truth. It's a concrete, actionable fix—either restore the shared variable or remove the dead read—and it's within scope of the composer and stylesheet files I'm reviewing. Now I want to check whether ChatComposer has other computed-style or DOM reads pointing at removed classes or data attributes, so I'm planning to search the codebase for similar querySelector usages tied to hooks that may no longer exist.
24. git_tools — data-chat-composer-top-drawer|data-thread-sync-drawer|data-composer-banner-drawer|data-composer-tasks-badge|data-composer-tasks-list|data-chat-composer-tasks-drawer|data-composer-stash-drawer|data-chat-composer-activity-strip|data-composer-banner-stack-expanded-items
💭 I see the attribute is actually set in ChatComposer, but git grep missed it because that file apparently contains a null byte, which causes grep to stop scanning after the first match — probably unrelated to this PR. I should also check other consumers like e2e tests that might reference the removed elements even if they're outside the scoped search patterns.
25. browse_code — Search the whole repo (including e2e/playwright/desktop test directories) for any remaining references to the selectors or class names removed in this change: chat-composer-top-drawer, chat-composer-drawer-slot, chat-composer-shoulder-tab, data-thread-sync-drawer, ThreadSyncStatusPill, ComposerTasksDrawer onDismiss, shoulderTabReserve. List file paths and line numbers for each hit. Repo: pingdotgg/t3code at commit 74d4ba6.
💭 I realize the research agent missed that ChatView.tsx actually uses that class as a Tailwind arbitrary group-variant selector, so it's still functional as a marker rather than dead CSS. Now I want to double-check whether that selector still correctly targets the shoulder tab element given the group hierarchy. The
--chat-composer-drawer-insetfinding holds up: the definition was removed but ChatComposer.tsx still references it with a hardcoded fallback, and Attachment hardcodes matching pixel values elsewhere—this is a real ownership drift worth reporting. I'm now verifying the Attachment and Dock width/padding math checks out against the form's max-width constraints, and confirming that the shoulder-tab class is applied to the activity and tasks Roots but want to check if it's also applied to the stash tab. Now I'm confirming the stash badge tab does include the shoulder-tab class alongside its own classes. I want to double-check the TypeScript lib target supportsArray.prototype.toSortedon readonly arrays, since the codebase already relies on other ES2023 features likefindLast, so I should verify the tsconfig lib setting to be sure this usage is safe.
26. browse_code — In apps/web (and root tsconfig it extends), what is the TypeScript lib/target setting? Does it include ES2023 (needed for Array.prototype.toSorted)? Also, does the codebase already use toSorted anywhere else in apps/web/src? Quote the tsconfig lib/target lines.
💭 toSorted's stability means ties in bannerPriority keep their input order, which is fine. I'm also noting that the front item's exit transition can change mid-dismiss after sorting, but that's a pre-existing issue, not a new regression. Now I want to check how ComposerPendingU handles this. Now I'm tracing how CollapsibleTrigger's
renderprop works when passed a ComposerBanner.Row element -- Base UI clones the element and merges props, including refs, so I need to verify the ref actually reaches the underlying button through Row's prop forwarding. I'm also noticing the previous trigger had hover and focus-visible ring styling that seems to be missing now, since Row only applies an outline-based focus style with no hover background treatment. More significantly, I'm tracing how horizontal padding moved: the card's padding and CollapsiblePanel padding were removed along with a comment explaining that padding exists specifically to prevent option buttons' focus rings from being clipped during the height animation. Now the panel has no horizontal padding and the Body component only applies inline-start padding, leaving the end side potentially exposed to that same clipping issue. Root does have p-1 (4px) padding, but since the panel's overflow-hidden clip boundary sits inside that padding, focus rings on the option buttons near the edges could still get clipped by the panel's own box during the collapse animation, even though there's some outer spacing from Root. I should check whether the Collapsible.Panel actually applies overflow hidden only during transitions or persistently, so I need to look at the collapsible primitive and this component's source directly.
27. browse_code — apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx
28. browse_code — apps/web/src/components/ui/collapsible.tsx
💭 I'm noticing that CollapsiblePanel keeps overflow-hidden permanently, and since the panel dropped its horizontal padding in favor of only inline-start padding from ComposerBanner.Body, the full-width option buttons' focus rings will likely get clipped at the inline-end edge where there's now zero padding. So the last button's bottom edge also lands right on the panel's bottom edge, clipping that ring too — this is a real regression the deleted comment had warned about, and it's fixable by restoring some end/bottom padding, like adding
pe-1 pb-1to CollapsiblePanel so the 1px ring has room to breathe. Now I want to look at the--chat-composer-drawer-insetfinding. Checking the stash menu's row markup, I'm verifying whether the restore button's expanded hit area (before:absolute before:inset-0) could overlap other interactive elements like the delete button or timestamp within the same row, since z-index layering matters here to keep the delete button clickable above the invisible overlay. Time and image thumbnails would also get covered, but since they're non-interactive, clicking them just restores the entry, which seems intentional for whole-row-click behavior. I'm also checking ComposerBanner.Dismiss elsewhere in the stack, and it looks consistent. Now I'm verifying whether the pending-approval branch renders correctly in the top drawer regardless of mobile-collapsed state, since previously that arrangement differed. This consolidation seems fine. Next I want to check the stash menu closing logic in ChatComposer around line 2589, so I'll look at that section for correctness and readability.
29. browse_code — apps/web/src/components/chat/ChatComposer.tsx
💭 That logic checks out fine, minor readability nitpick only, not worth flagging. Now I'm checking whether there's a scroll-related issue to look into, examining how the ComposerBanner scroll wrapper handles max-height and fade behavior for its scroll area. Comparing that against the previous tasks drawer implementation, which used a native scrollable div with focus-ring styling and a list role, versus the new Base UI ScrollArea component — I'm weighing two possible regressions: whether the new viewport still supports keyboard-driven scrolling via tabIndex like before, and whether overscroll-contain behavior was preserved to prevent scroll chaining to the parent page. Also worth checking whether the stash menu's command list, which previously had a fixed max-height and scroll padding, still behaves the same way under the new ScrollArea. I want to look at the actual scroll-area component source to confirm these details.
30. browse_code — apps/web/src/components/ui/scroll-area.tsx
💭 I'm checking that the ScrollArea viewport's focus-visible ring styling aligns with it being keyboard-focusable when content overflows, which confirms this migration is consistent. Then I'm working through ComposerBanner.Scroll's height overrides — the root uses auto height with a max-height cap while the viewport uses full height and inherits that max-height, which should still work correctly since a full-height child of an auto-height parent resolves to content-based height. I decide not to dig further into precedent for this pattern and instead move on to check the activity/priority area and MessagesTimeline, noting a change there where something was removed that I want to look into next. Now I'm tracing how MessagesTimeline groups non-error, non-spawn entries into a single collapsible summary toggle, verifying that both branches of the row-deriving logic push the correct row kind with a non-null summary string, and confirming the expansion condition still checks both "work-toggle" and "work-live" row kinds correctly. I'm also checking whether the toggle button component still retains its
aria-expandedattribute after losing its chevron indicator in favor of a tool icon — need to confirm that accessibility attribute wasn't accidentally dropped in the refactor.
31. browse_code — apps/web/src/components/chat/MessagesTimeline.tsx
💭 The timeline no longer shows the working timer row since it moved to the composer, and the logic was relocated rather than duplicated. I should double check whether anything still references the old max visible work log constant. I also need to verify
ThinkingActivityRowwas fully removed withLiveActivityRowtaking over, and grep for any leftover references to the old work log entries limit, working step label, or active turn started timestamp.
32. git_tools — MAX_VISIBLE_WORK_LOG_ENTRIES|workingStepLabel|activeTurnStartedAt|ThinkingActivityRow|onlyToolEntries
💭 I want to verify the composerBannerItems assembly in ChatView now that ordering relies on the stack's own sorting, making sure the parked-thread banner still lands last given its default priority in the new bannerPriority scheme. I'm confirming that the old comment about careful ordering—ensuring a passive "update available" notice never covers the background liveness Stop button—still holds under the new sort. Since liveness items now carry
priority: "activity"(rank 1), they'll sort ahead of notices (rank 2) but behind urgent items (rank 0), which matches the original intent, so I want to look at lines 4980-5030 to confirm the final assembly reflects this.
33. browse_code — apps/web/src/components/ChatView.tsx
34. browse_code — apps/web/src/components/ChatView.tsx
💭 I'm tracing through the stack sort logic: both the composer's activity item and the background-liveness item share priority "activity," so a stable sort keeps the liveness banner first since it appears earlier in bannerItems, meaning the composer's working/tasks row could end up hidden behind it. I think I have two solid findings so far. Checking the peek cap: it's now a real button reachable via Tab, which is a genuine accessibility improvement, though it stays focusable and clickable even while expanded (clicking it then is just a no-op). The real issue I'm spotting is in the collapse behavior — when expanded via the peek button's click handler, focus lands on that button, which sits inside noticesRef, so onFocusCapture keeps things expanded and onPointerLeave never fires collapse since the active element is still contained within it. Tracing through the Escape key, touch, and pointer-leave paths: Escape from the peek button works fine and refocuses/collapses correctly; on touch, tapping outside the stack doesn't collapse it since there's no document-level listener and focus remains on the peek button, though since the stack stays in-flow rather than overlaying content, I'd call this low severity. I think I'll settle on two solid findings, plus maybe a minor third one about a removed feature. I'm settling on my findings: first, the panel's padding removal clips focus rings under
overflow-hidden, and notably it deletes a comment that explicitly warned against this exact issue — a clear violation of preserving intentional exceptions. Second,--chat-composer-drawer-insetgets deleted from CSS while an imperative consumer viagetComputedStylestill depends on it, meaning the drawer geometry is now duplicated across three places and can silently drift out of sync. Both issues fall squarely within scope for CSS ownership and accessibility concerns, so I want to re-verify the first one more carefully. So the last option's ring is clipped at the bottom, plus every option's ring clipped on the end side. The fix is to add breathing room inside the panel, like padding on the panel or the Body component (pe-1 pb-1), giving enough space for the 1px ring not to get clipped. I'll suggest adding it to the panel since that's the element owning the overflow clip. Now I need to figure out the correct diff line numbers for the inline review comment, checking whether line 203 falls within the new file's hunk to reference<CollapsiblePanel>correctly. For finding 2, deleting the CSS variable requires commenting on the LEFT side, but it's simpler to instead comment on the hardcoded width value in the new ComposerBanner.tsx file, since that's an added line and clearly part of the diff. This matches the flagged category exactly since the deleted property is consumed by an imperative getComputedStyle read elsewhere—it won't break due to the fallback, but leaves duplicated magic numbers, so I'll report it as a modest-severity finding. Since the check defaults to failure state when any findings exist, and this is a concrete (if small) visual regression risk, I'll mark this as a failure rather than "all clear." The end edge has zero padding while the start has some, confirming padding needs to be added there, and I should verify the panel's overflow-hidden is unconditional (it is). I'll finalize the review with two inline comments, suggesting the fix of adding "pe-1 pb-1" classes to the CollapsiblePanel element. For the second finding, I'm considering whether the minimal fix should be removing the unused getComputedStyle read since it always defaults, or keeping a single source of truth for the style — I'll mention both as options. I confirm line 17 of ComposerBanner.tsx matches the margin/width styling I expected, and both files fall within the review's include patterns.
35. modify_pr — UI consistency review of the composer banner migration. Two findings, both about behavior/ownership preserved-through-migration rather than styling taste. The rest of the migration (banner primitives composed from Button/ScrollArea, ScrollArea replacing the hand-rolled overflow-y-auto task list, [data-composer-banner-surface="attached"] replacing the per-class :has() selectors, .chat-composer-shoulder-tab retained as the marker ChatView still keys group-has-[...] off) looks consistent.
Agent Credits: 822 credits
Token usage: 100 uncached input · 6,315,670 cache-read input · 531,330 cache-write input · 53,777 output
Agent Credits may also include non-token charges from external tools such as web research.