Skip to content

fix(web): keep activity in front of composer notices

MacroscopeApp / Macroscope - UI Consistency failed Aug 29, 2026 in 7m 8s

UI Consistency: 4 issues found

Four findings posted as inline review comments.

  • apps/web/src/components/chat/ChatComposer.tsx (form className, line 3417) — --chat-composer-drawer-inset was deleted from index.css but is still read via getComputedStyle in ComposerCommandMenuLayer (line 200); the inset now lives only as the literal 2.75rem in ComposerBanner.Attachment, so the command-menu geometry can silently drift from the banner inset.
  • apps/web/src/components/chat/ComposerBannerStack.tsx (line 68) — the stack gained real state (priority ordering, focusable peek button, pointer/focus expansion, Escape collapse) in the same change that deletes ComposerBannerStack.test.tsx, ComposerStashBadge.test.tsx, and ComposerTasksBadge.test.tsx, leaving ordering and expansion behavior uncovered.
  • apps/web/src/components/chat/ComposerStashBadge.tsx (line 65) — chat-composer-stash-tab is now a dead class: no CSS rule, selector, or query references it after the .chat-composer-shoulder-tab block was removed.
  • apps/web/src/components/chat/ComposerTasksBadge.tsx (line 149) — chat-composer-tasks-tab is likewise dead after shoulderTabReserve (its only JS consumer) and the shoulder-tab CSS were removed.

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.

Verified as consistent, no findings raised:

  • ComposerBanner primitive composition: rows/icons/content/actions share grid columns, disclosure rows preserve cursor-pointer, focus-visible outlines, aria-expanded, and labels; ToggleIcon is a styled span so no button nests inside a row button.
  • Long lists moved to the shared ScrollArea (ComposerBanner.Scroll) with scrollFade, preserving the previous bounded height, overscroll-contain, and keyboard-focusable viewport instead of a static mask.
  • Attachment geometry matches the deleted .chat-composer-drawer-slot rules (22px inset, 45.25rem cap, calc(1rem + 1px) overlap), and the :has(.chat-composer-top-drawer, .chat-composer-shoulder-tab) selectors were consistently replaced by [data-composer-banner-surface="attached"]. chat-composer-shoulder-tab is still required by ChatView's group-has- padding hook and is retained at every tab call site.
  • Activity/tasks/stash rendering paths were traced across banner-items, top-drawer, tasks-drawer, dock, and inline states; no duplicate activity rows and no stash badge rendered with a zero count (ComposerStashBadge returns null at count === 0).
  • deleteStashEntry closes the stash menu only when the deleted entry was the last one, matching the documented behavior.
  • dismissVersionMismatch(null) is a no-op, so the widened dismiss condition for failed updates is safe.

Not raised (aesthetic/borderline): the failed server-update banner uses variant="error" with an InfoIcon, and ComposerServerUpdateStatus nests role="alert"/role="status" inside the stack item's own role="alert" surface.


🔍 Agent activity — 24 tool calls

💭 I'm scanning the diff for UI consistency issues, focusing on the new ComposerBanner, ComposerActivityStatus, and ComposerServerUpdateStatus files alongside changes in ChatComposer, ComposerBannerStack, and ComposerStashMenu. I'm checking whether ComposerBanner.Root properly forwards refs, since ComposerStashMenu passes a drawerRef into it. I'm noticing a potentially missing aria-label on the Dismiss button in ComposerStashMenu compared to before, and wondering if removing the useEffect that reset highlightedId when entries change could leave a stale highlight. I'm also examining a condition in ChatComposer that checks whether any other stash candidate exists before closing the stash menu, which looks like it might have a logic issue. Wait, I notice the .chat-composer-shoulder-tab CSS block seems to have been deleted from index.css, yet the class name is still referenced in both ChatComposer and ComposerTasksBadge. I need to verify whether .chat-composer-stash-tab and .chat-composer-tasks-tab are still defined somewhere, since leaving dead class references would be a small but real nit if the styling truly moved to chat-composer-drawer-surface. I should grep for any remaining :has(...) selectors that reference .chat-composer-shoulder-tab, since the diff seemed to replace those occurrences with a data-composer-banner-surface="attached" attribute selector instead — I want to confirm nothing was missed. I'm also checking that the fill-width behavior on ComposerBanner.Root in the tasks badge still works correctly through the Dock's flex styling, and noting that ComposerServerUpdateStatus uses role="alert" on a span. Now I'm also noticing nested alert roles — ComposerBannerStackAlert applies role="alert" on every banner Root regardless of type, which seems to carry over from the previous Alert component's built-in role rather than being a new choice, so it's probably fine. I'm also flagging a potential accessibility issue: in ComposerStashMenu, the header button sets aria-expanded="true" as a string literal instead of a real boolean, which should be fixed. I'm also noting that keyboard navigation in ComposerStashMenu calls scrollIntoView unconditionally, even though focus is only moved when the drawer contains the active element—worth checking. More significantly, this PR deletes several test files (for ComposerBannerStack, ComposerStashBadge, ComposerTasksBadge, ThreadSyncStatusPill) without adding coverage for the new components like ComposerBanner, ComposerActivityStatus, and ComposerServerUpdateStatus, which conflicts with the guideline requiring focused tests when primitive composition changes behavior. I'm flagging that ComposerBannerStack now carries meaningful new behavioral state—stack expansion, keyboard Escape handling, priority-based sorting—yet none of it is tested anymore since the old test file was removed. I'm also checking whether Array.prototype.toSorted is safe to use given the project's target runtime, and noting a useEffect that collapses the stack whenever fewer than two items remain, which could interact unexpectedly with the expansion state. Now I'm tracing a possible logic gap in ChatComposer's hasShoulderTab calculation: it can become true purely from activityStatus being set, independent of whether there's anything in the stash queue, but the render logic then conditionally shows ComposerStashBadge based on !showInlineStashBadge rather than checking the queue length directly. I want to confirm whether ComposerStashBadge safely no-ops when the count is zero, since the previous version's condition apparently required a non-empty stash queue before showing shoulder tabs at all — if that check was dropped, the badge could render with a stale or zero count. I should verify this directly against the source rather than reasoning from memory, and also check whether ComposerTasksBadge's inline variant is being rendered as a bare row without its usual wrapping root component, since that could affect layout or accessibility. Let me run a few greps to verify.

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

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

💭 I'm reviewing the stash menu structure—the row uses a full-overlay restore button with the dismiss button layered above via z-index, and I'm checking how the delete button's visibility behaves on hover versus touch input. Also checking keyboard focus handling, an aria-expanded string, and the Row component's cursor styling, before moving on to check the ChatComposer's shoulder tabs and dock rendering conditions.

3. git_tools — showShoulderTabs|hasShoulderTab|inlineTasksBadge|showInlineTasksBadge|showInlineStashBadge|inlineStashBadge

💭 Something's off with the output, possibly a revision issue. Let me look directly at the ChatComposer file around those specific line ranges to see what's happening.

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

💭 Let me look at the render area around lines 3400-3600 in the head version.

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

💭 I'm tracing through the Dock's shoulder tab logic to confirm it correctly handles cases where activityStatus exists but tasks and stashQueue are both empty, verifying ComposerStashBadge safely returns null when count is zero. Now I'm noticing the CSS class chat-composer-shoulder-tab was removed from index.css but is still referenced in ChatComposer and the badge components, so I need to grep index.css for remaining references to the shoulder-tab, stash-tab, tasks-tab, drawer-slot, and top-drawer classes.

6. git_tools — chat-composer-shoulder-tab|chat-composer-stash-tab|chat-composer-tasks-tab|chat-composer-drawer-slot|chat-composer-top-drawer|chat-composer-drawer-inset|chat-composer-attachment-overlap

💭 I found that the code reads the --chat-composer-drawer-inset CSS variable via getComputedStyle, but the diff removed the actual rule that defines this variable on the relevant selectors—so the value would now be missing entirely. I'm also noticing that the .chat-composer-shoulder-tab class rule got deleted from index.css, but it's still referenced as a marker class in a group-has selector elsewhere—which is fine since the class is still applied by badge components, though its visual styling now comes from a different surface class. I want to check index.css directly to confirm both of these deletions.

7. browse_code — apps/web/src/index.css

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

💭 This PR removed the --chat-composer-drawer-inset custom property from index.css, but ChatComposer.tsx:200 still reads it with a || 1.375 fallback, so the numeric value stays the same since 1.375rem equals 22px, matching the new Attachment inset's 44px total split evenly. This is now a dangling variable read that duplicates the same value as a magic number in Tailwind and in the TS fallback -- not a behavior regression, but a minor ownership/dead-code inconsistency worth flagging at low severity: either restore the CSS variable or drop the getComputedStyle read. Now I'm checking ComposerBanner.tsx for similar selector and arbitrary-variant concerns, looking at how the attribute selectors and descendant combinators are structured in the Tailwind classes. I'm cross-checking the drawer's computed max-width and margin values against the old CSS variable formulas, and they line up correctly with the new arbitrary Tailwind values. I'm also verifying whether Tailwind v4's --spacing() function is valid inside arbitrary properties and calc expressions, since it's being used both in padding calculations and as a raw value for a custom property, which raises a question about whether Tailwind actually transforms that function when used this way. I can use --spacing() inside calc() in arbitrary values, which seems safe, but using it bare in arbitrary properties is riskier without verification. The responsive variable setup gives 28px on mobile and 24px on sm+, which matches the docs, so I move on to checking how ComposerBanner.Row is used inside ComposerPendingUserInputPanel with the CollapsibleTrigger. Since Row uses base-ui's useRender and merges props like onClick and aria-expanded from CollapsibleTrigger through the render prop pattern, I trace how those props flow through mergeProps into the final rendered element. Now I notice the previous hover/focus styling on CollapsibleTrigger — the hover background — has disappeared in the new implementation, leaving only a focus-visible outline; this is a minor cosmetic regression rather than a functional bug. I'll flag this as low-severity. More significant is the removal of dismiss functionality for tasks in ComposerTasksBadge — but that seems like a deliberate product decision matching the docs about clicking to expand/collapse instead. I'm checking that Dismiss button's prop spreading is safe, then moving to review the Row component's structure. Working through the grid layout logic — how icon, content, and actions columns adjust when the icon is missing, and whether the column spans behave correctly in that case. It looks fine, though I want to double check the generated CSS for potential conflicts with the conditional selectors. That combination with the group prefix should be fine syntactically, though I can't actually run the build to verify the emitted CSS here. Let me move on to reviewing the full new ComposerBannerStack.tsx file for the bigger behavioral concerns.

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

💭 I'm checking that the sorting logic correctly places the activity item after urgent items but before notices, and examining how the stack cap button is positioned within the reversed flex container relative to the front item. Since the cap sits inside the notices container whose collapsed height is zero, its bottom-aligned position still visually matches the previous top-offset placement above the front banner, which confirms the layout math checks out. I'm also noting the peek button is now clickable rather than purely decorative, which is a nice usability fix, though I want to verify hover interactions still work given the zero-height container. I'm checking a few more details: the pointer-enter bubbling still triggers expansion correctly, the pre-existing role="alert" behavior on all notices (including non-urgent ones) matches prior tests so it's not a regression, and hidden stacked notices remain properly excluded from the accessibility tree via visibility hidden. Touch devices should still work fine since tapping the peek button expands the stack directly. The bigger concern I'm circling back to is that the stack now has real interactive state and priority ordering, but the corresponding test file was deleted with nothing added to cover this new behavior. I'm confirming this is a legitimate gap — the PR removes ComposerBannerStack.test.tsx and several other badge/pill test files without adding replacement coverage for the new ComposerBanner primitives, priority-based stack ordering, or the expansion state machine, which directly violates the requirement to test behavior changes, prop forwarding, and state transitions. I want to double check whether any new test file was actually added for ComposerBanner before finalizing this as a finding.

10. browse_code — apps/web/src/components/chat/*.test.tsx

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

💭 I'm checking prop ordering in ComposerBanner's Root—data attributes are placed before the spread so consumers can still override them, which looks fine. I'm also noticing ToggleIcon renders a non-interactive span styled with button-like ghost hover variants rather than an actual button element, which seems like a potential accessibility concern worth flagging. I'm scanning through Dismiss buttons and Row components across ComposerStashMenu, ComposerStashBadge, ComposerTasksBadge, and ComposerPendingUserInputPanel to check for any nested-interactive-element issues, particularly where Row is rendered as a button or list item containing other clickable controls. So far most look correctly structured, but I want to verify the restore button inside ComposerStashMenu's list item Row doesn't create a nesting problem. I'm also checking ComposerActivityStatus and its ActivityIcon logic — when the status kind isn't "sync", no icon slot renders, which shifts the Content layout, but that's expected. The working timer's status label lacks a role="status" attribute unlike the sync variant, though this matches prior behavior so it isn't a regression. Now I'm moving into ComposerServerUpdateStatus to look at its markup. I'm also noticing there's a nested duplicate role="alert" between the parent and the failed-state span, which could cause redundant screen reader announcements — a minor accessibility smell. And the separator styling targets direct children via a data-slot selector, so I'm checking whether it still applies correctly within the button's nesting. I recall it renders inside a div with a drawer data attribute — I want to check lines 140-250 of the file to see if the wrapper div still has that top-drawer className or just the collapsed-controls attribute.

12. browse_code — apps/web/src/components/chat/ComposerTasksBadge.tsx

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

💭 Checking that the composer banner list uses proper semantics with the ul/li structure inside the scroll area, and that the aria-label on the ul is valid since it has implicit list role. Now looking at how TaskSegments is used within TaskSummary, checking its className and steps prop. Working through how the tailwind classes merge on TaskSegments — since hidden and flex are both display utilities, tw-merge should keep hidden as the base and preserve sm:flex since it's a different responsive variant, resulting in the segments being hidden by default but shown as flex at the sm breakpoint. I'm also confirming that ScrollArea properly passes through data attributes and works with an auto height container. Now I'm flagging a potential regression: the scroll container previously had tabIndex={0} with a focus ring so keyboard users could scroll the region, which matters for accessibility since scrollable areas need to be keyboard-focusable.

14. browse_code — apps/web/src/components/ui/scroll-area.tsx

💭 I'm checking whether tailwind-merge correctly resolves the conflict between the size-full utility and the h-auto override on the scroll area root, since that determines which height class actually wins. The viewport inherits its height fine given the auto-height parent, and I confirm Base UI's focus behavior on the viewport is preserved with the ring styling, matching the old tabIndex handling. I'm also noting that overscroll-contain carried over correctly, and now I'm turning to compiling the actual findings to report — starting with the deleted --chat-composer-drawer-inset CSS variable that's still being read via getComputedStyle, creating a fragile sync issue between the composer and the attachment banner's hardcoded width calculation. I want to suggest either restoring the shared token or removing the dead read, and I'm working out the best place to leave this as an inline comment — likely on ComposerBanner.tsx line 17 since that's where the replacement magic number lives and it's on the diff's addable side. Now I need to double check the deletion really did remove the only definition of that variable. Confirmed via grep that no --chat-composer-drawer-inset definitions remain anywhere in the CSS, so ChatComposer.tsx:200 is the only remaining reader — this is a solid, real finding. Separately, I'm now looking at whether the .chat-composer-shoulder-tab class removal broke the ChatView padding logic, since that selector was written assuming the tab was absolutely positioned and overlaying rather than in-flow within the Dock as it seems to be now. Checking the grep results, it confirms chat-composer-tasks-tab only appears in ComposerTasksBadge.tsx and chat-composer-stash-tab only in ComposerStashBadge.tsx, with chat-composer-shoulder-tab used as the actual shared consumer class across ChatView.tsx and ChatComposer. So those two specific tab classes are effectively orphaned markers now. I'll treat this as a minor point rather than a blocking issue, since it's low-severity dead code introduced alongside the change—probably worth folding into a single combined comment on those two badge components rather than flagging separately. I've also verified chat-composer-banner-stack-cap is still properly defined in index.css. Now I'm shifting attention to the ComposerStashMenu component, where the effect that used to reset highlightedId was removed. That effect syncing highlightedId was replaced by a derived value picking the matching entry or defaulting to the first one, which is actually a cleaner approach that avoids stale state, so no issue there. Checking the arrow-key navigation, the indexing into the restore buttons still lines up correctly with the entries order. Now I'm looking at the stash-menu-close condition in ChatComposer, which checks whether any other candidate besides the current entry remains in the queue before closing the menu. I need to view the surrounding code to confirm this logic sits inside a delete handler rather than a restore handler, since the semantics of "close if no other entries remain" depend on whether the deleted entry has already been removed from the queue at that point. I'm also flagging a nested role="alert" inside another role="alert" container in the ComposerServerUpdateStatus component — for the failed state, the outer stack alert and an inner span both carry alert roles, which can cause duplicate announcements in some screen readers. It's a minor but real accessibility concern in new code, compounded by the inner span also carrying role="status" while the outer element differs. Weighing severity, I'm deciding whether this nested-role issue is worth surfacing as a low-severity inline comment or whether to drop it in favor of stronger, more concrete findings. Two other concrete issues stand out more: a deleted CSS token still being referenced, creating silent drift, and four deleted test files covering behavior that actually became more complex — banner stack expansion, priority ordering — with no replacement tests, which directly violates the rule requiring focused tests for state-machine and behavior changes. There's also the dead marker classes left behind. Now I'm checking whether removing the shoulder-tab reserve and its MutationObserver from ChatView introduces a layout regression, since tabs used to be absolutely positioned above the composer and now sit in-flow inside the Dock, changing how clearance was calculated. I'm also tracing the padding class conditioned on the shoulder-tab presence to confirm the reduced bottom padding is still intentional now that the tab is in-flow, checking that thread-keyed remounting of the banner stack is fine, and noting that the condition controlling the inline stash badge visibility switched from checking composer-collapsed-on-mobile to just checking mobile viewport — which changes when that badge actually appears. The inline stash badge still renders separately within the composer rows, so that's fine. I'm checking whether activity can render twice — when the top drawer is a non-blocking plan follow-up, the inline tasks badge shows in the strip correctly, and I'm now tracing whether tasksHostActivityStatus could trigger a duplicate render alongside the shoulder tabs or inline badge condition. Checking the case where banner items exist with blocking drawer active and tasks present: the activity stack skips tasks entirely since blocking wins, showing only the activity row when applicable. When banner items exist and tasks aren't blocked, the tasks content renders expanded in the stack, and even though the inline badge condition is technically true, the activity strip stays suppressed since it's guarded by the activityStackItem check. I'm also confirming the composer banner row markup nests properly as a list item inside the scroll area without wrapper conflicts, and re-verifying that the CSS variable for drawer inset is only defined in one place across the whole repo via a fresh grep. Since getComputedStyle would return an empty string for the missing variable, the fallback of 1.375rem kicks in and matches today's actual value, so there's no visible regression -- but the contract between the CSS variable and the runtime reader is effectively broken since the inset is now hardcoded as a literal in ComposerBanner. I'm flagging this as a low-to-medium ownership finding, suggesting either restoring the CSS variable on the Attachment component and deriving the width from it, or removing the dead getComputedStyle read entirely. Now sketching the suggested inline fix for that ComposerBanner line. Wait, though -- ChatComposer actually reads the variable off the form element, not the attachment, and since attachment is a child of the form, setting the variable there wouldn't make it visible to the form anyway. So the better fix is really to define the variable on the form element in ChatComposer itself. That line is part of the diff already, so I can suggest restoring the custom property there directly, keeping it in sync with the value used in Attachment. But ComposerBanner still hardcodes its own value separately, so I should flag both spots -- either restore the token everywhere consistently or remove the now-unused read entirely. I'll leave an inline suggestion on the ChatComposer line, noting it needs to stay synced with the Attachment side. Now for the missing tests, I need to decide whether to leave a top-level review comment or attach it inline to ComposerBannerStack.tsx, since the diff there also touches the new stackExpanded state around that area. Checking the guidance, this really does warrant flagging since the PR removes the ComposerBannerStack test that covered ordering and expansion while adding new state transitions, keyboard Escape handling, and priority ordering logic — that's exactly the kind of behavior change the instructions say should require tests, so I'll report it. I also want to check whether the other deleted test files correspond to components that were fully removed, which would make their deletion fine rather than a gap. I confirm it's genuinely dead by rechecking the repo-wide grep, which already covered tests and returned no other usages. I also verify chat-composer-shoulder-tab still has a live consumer in ChatView, so that one should stay. Then I look again at where ComposerServerUpdateStatus sits within the banner structure, noticing it creates a nested role="alert" inside another role="alert" element. This nesting is worth flagging as a minor accessibility concern since it's new code and could cause screen readers to double-announce, and I also notice a role="status" inside role="alert" for the running state, which may assertively interrupt announcements when progress text changes — something the old ServerUpdateProgress component likely didn't do since its title was plain text. I want to check ServerUpdateProgress's roles to confirm whether this is actually a behavioral change.

15. git_tools — role=|aria-live|serverUpdateStageLabel

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

💭 The deleteStashEntry logic looks fine, closing when the deleted entry was the last one, though there's a minor issue if takeStashEntry fails since the menu still closes. I'm also noticing an accessibility nesting problem: the failed banner uses role="alert" while the running state has no role in Settings, but the new ComposerServerUpdateStatus nests a role="status" running state inside the stack's role="alert" Root, meaning the failed banner ends up being an alert containing another alert. I'll treat this as a minor consistency note rather than a full inline comment, since I already have a few stronger findings. Given that only inline-posted findings actually surface on the PR, I'm settling on two to flag directly: the deleted --chat-composer-drawer-inset token that's still read in ChatComposer but now duplicated as a literal value in ComposerBanner.Attachment, and the new interactive banner stack state machine in ComposerBannerStack that shipped without its corresponding test coverage for priority ordering and expansion behavior. I also want to flag ComposerStashBadge and ComposerTasksBadge — their tab classes now have no matching CSS rules after this PR strips the shoulder-tab block, so I'm suggesting they just reuse the still-active chat-composer-shoulder-tab class instead. Now I'm cross-checking each of these against the actual diff hunks to confirm the line numbers land on added lines so the inline comments will attach correctly. I'm also weighing whether the ToggleIcon-as-span with buttonVariants deserves a flag, but since that's an intentional pattern to avoid nested buttons, I'll skip it. Last thing to verify is whether swapping div to span in ComposerPendingApprovalPanel causes any issue with block-level children now sitting inside an inline span.

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

💭 The nesting seems fine since code is inline with display block styling. I notice there's a test file, ComposerPendingApprovalPanel.test.tsx, that might assert on the outer element's markup — I should check whether it still passes if the PR changed that structure without updating the test.

18. browse_code — apps/web/src/components/chat/ComposerPendingApprovalPanel.test.tsx

19. browse_code — apps/web/src/components/chat/ComposerPendingUserInputPanel.test.tsx

💭 That test case should still pass since the pending user input panel renders its button correctly through Row. Now I want to double-check how CollapsibleTrigger's render prop interacts with ComposerBanner.Row when nesting a button element—base-ui needs to clone that Row element while properly merging its className and data attributes like data-panel-open. The className merging should work fine since it's typically resolved to a plain string before being passed to cn(), and mergeProps in base-ui handles concatenating classNames when combining the Row's own props with the CollapsibleTrigger's provided props (including onClick and ref). I'm also verifying that ComposerBanner.Icon still keeps its aria-hidden attribute as before in the tasks list. Checking the Root/Row rendering hierarchy in ComposerTasksContent and ComposerTasksDrawer, both properly provide a Root wrapper so a bare Row placement works correctly in either context. The more interesting concern is whether ChatComposer's new bannerItems array prop is memoized upstream in ChatView -- if it's rebuilt fresh on every render without useMemo, it'll break ChatComposer's memo comparison and cause unnecessary re-renders, so I want to trace where composerBannerItems is constructed to confirm it's wrapped in useMemo.

20. git_tools — const composerBannerItems

💭 Good, memoization looks fine. I'm deciding to skip the timeline entry removal since those changes come with updated tests already. Now I'm checking the banner priority logic in ChatView.tsx around the update banner icon, verifying it shows consistently regardless of failure state.

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

💭 I'm spotting a potential bug around the dismiss handler logic: previously dismiss was only offered when versionMismatchDismissKey was truthy, but the new condition changes behavior so that when updateFailed is true and the dismiss key is null/empty, the button visibility might shift unexpectedly compared to the original logic. I also want to check whether calling dismissVersionMismatch with a null key is actually safe, and I'm noticing an icon inconsistency—an error-variant banner showing an InfoIcon instead of a warning/error icon like other failed states in this stack use. I should look at the icons used by nearby items around line 2150-2240 to confirm this mismatch and verify how dismissVersionMismatch(null) behaves.

22. browse_code — apps/web/src/versionSkew.ts

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

💭 Resetting the dismissed key to null should be safe since it just clears state, though I want to check whether the banner visibility logic correctly rechecks the dismissal condition against the current mismatch key before re-showing anything. I also notice the update-failed banner uses an info icon rather than an error/warning icon, unlike other error-variant banners that use something like a wifi-off icon. That's a small inconsistency worth flagging as a minor, low-severity note since the icon doesn't match the error severity of the state. Actually, an inline suggestion to swap in a triangle-alert icon runs into an import issue and risks reading as a purely aesthetic preference, so I'll skip proposing that fix inline and just mention it as a non-blocking observation while I finalize the remaining findings — starting with the drawer inset token drift, and noting that even the proposed fix there only partially resolves the underlying inconsistency since the literal width value elsewhere wouldn't be updated. Now I'm working through whether the token should live on the form itself, given that some Attachment instances render outside the composer form's DOM subtree, which means a shared ancestor is needed for the custom property to actually work. The cleanest options seem to be either removing the now-unused computed-style lookup in favor of one shared constant, or re-declaring the custom property at whatever level actually wraps all the attachment instances. It's not perfect since it duplicates a magic value elsewhere, but I'll present it as one option alongside the alternative fix. For finding 2, I'll leave an inline comment on ComposerBannerStack.tsx line 68 without a diff. For finding 3, I'm weighing whether to comment separately on the stash badge and tasks badge dead-class issues or consolidate into one comment on ComposerStashBadge.tsx line 65 with a suggestion, then decide how to handle the tasks badge line separately. I want to confirm the shoulder-tab classes aren't referenced anywhere in e2e or playwright tests outside apps/web -- since the grep was repo-wide, I've confirmed they're truly dead, except chat-composer-shoulder-tab itself, which is still needed for ChatView's padding hook and should stay. So my suggested fix keeps that class on both badges while dropping the dead ones, using ml-auto for the stash variant. Now I'm weighing severity: finding 1 is a real drift issue but currently harmless, while finding 2 is a coverage gap worth flagging. Since the check defaults to failure and there are genuine findings, even if none is a hard regression, I'll lean toward reporting failure rather than neutral, since the instructions favor failure whenever findings exist. I'm double-checking finding 2 by scanning the diff for any replacement tests -- new component files were added but no corresponding render tests appeared, only updates to existing test files like versionSkew and session-logic, confirming the banner stack lost its coverage. Now I'm drafting the actual review comments, starting with one about a CSS variable that's no longer defined but is still referenced in ChatComposer.tsx. The rule defining that inset variable was removed from index.css, but ComposerCommandMenuLayer still reads it off the form and silently falls back to a hardcoded value, while the real inset now lives elsewhere as a literal width calculation -- meaning the two can drift out of sync if that literal ever changes. I'm noting this as worth flagging so the token gets re-declared or the dead style read gets removed in favor of a single source of truth. Separately, I'm looking at ComposerBannerStack, where its test file was deleted in the same change that added real behavior -- banner priority ordering, a focusable peek button, hover/focus expansion, and Escape-to-collapse -- along with the Stash and Tasks badge tests. None of that new logic is currently covered, so I want to suggest adding a focused test for the ordering and expand/collapse behavior. I'm also verifying two other suggestions: one to remove an unused chat-composer-stash-tab class in ComposerStashBadge, and one checking the indentation and class names on line 149 of ComposerTasksBadge to make sure the suggested diff matches exactly.

24. modify_pr — UI consistency review of the composer banner migration. The new ComposerBanner primitive set, the ComposerBannerStack ordering/expansion rework, and the shared ScrollArea reuse all look sound; a few ownership and coverage issues below.


Agent Credits: 301 credits

Token usage: 36 uncached input · 2,154,559 cache-read input · 164,890 cache-write input · 30,297 output
Agent Credits may also include non-token charges from external tools such as web research.