Repository navigation
UI Consistency: 1 issue found
apps/web/src/components/pullRequest/PullRequestListFilters.tsx — The new "Clear selected labels" MenuItem inherits Base UI's closeOnClick: true, so it dismisses the entire filters menu, while every other control in the same panel (MenuCheckboxItem label toggles, MenuRadioItem state/involvement/author/project choices) keeps it open. Suggested fix posted inline: pass closeOnClick={false}, matching the existing opt-outs in PreviewMoreMenu.tsx and PullRequestCodeTab.tsx.
Previously reported issues that are now resolved and were not re-flagged:
CompactFilterMenu'soutlinedmode now renders throughButton variant="outline"instead of rebuilding the outline treatment on a rawMenuTrigger.- The author search field sizes the control (
size="compact") rather than theInputGroupwrapper, and now shields only printable keys from the menu typeahead. PullRequestRowLabelsis guarded at the call site, soPullRequestMetaLineno longer draws a stray separator on rows without labels.- The project filter tests route through
findLabeledGroup(view, "Project")and again cover the submenu composition. - The duplicated
labelDotColor/dot markup was removed fromPullRequestRow.tsx.
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.
Scope reviewed
apps/web/src/components/pullRequest/PullRequestListFilters.tsxapps/web/src/components/pullRequest/PullRequestListFilters.test.tsxapps/web/src/components/pullRequest/PullRequestRow.tsxapps/web/src/components/pullRequest/pullRequestList.logic.tsapps/web/src/routes/_chat.pull-requests.tsx
Checks performed
- Compared the two new commits (
9db0641,8654504) against the previously reviewed revision to confirm which prior findings were addressed rather than re-flagging them. - Verified primitive contracts against
components/ui:Buttonsize/variant classes for both new outlined triggers,Input'scompactsize,InputGroupaddon padding,MenuPopup's width-defaulting helper (min-w-72,w-80,w-72all correctly suppressmin-w-32), andMenuItem/MenuCheckboxItem/MenuRadioItemclose-on-click defaults across existing call sites. - Traced the project filter's move into the shared
PullRequestFilterRadioGroup: disabled state, tooltip reason and unavailable-first ordering are preserved;ProjectFaviconcarries its ownshrink-0, so the icon is not squashed by the shared item markup. - Re-ran the project filter test paths by hand through
findLabeledGroup/findValueChangeto confirm both assertions now resolve the submenu's radio group. - Checked the row's right column still renders two lines (relative time + size), so the
[contain-intrinsic-block-size:54px]estimate is unaffected.
🔍 Agent activity — 32 tool calls
💭 I want to check the current state of the PR at head to see what's changed since the previous run (7589c7d) and whether earlier findings were addressed, so I'll fetch the relevant files.
1. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.tsx
2. git_tools — 7589c7d..8654504
💭 Since the prior commits might have been force-pushed away, I'm checking the head files against the earlier findings to see what's been addressed: the outlined mode in CompactFilterMenu, onKeyDown propagation in the author search input, and the InputGroup sizing issue which now looks fixed since it uses size="compact" instead of h-8. I'm now checking whether the labelDotColor duplication between PullRequestRow and the summary has been resolved too. The PullRequestMetaLine guard and the test findValueChange both look fixed in the diff now. I want to view the current author filter section and the CompactFilterMenu to confirm.
3. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.tsx
4. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
💭 The outline variant, InputGroup height, and onKeyDown findings all look resolved now — the printable-key-only check is exactly the guard that was suggested. Still need to check the labelDotColor duplication, which appears to still be present here and in PullRequest. Checking further, the PullRequestRow copy has been removed since labelDotColor no longer appears there, so the duplication is now just between filters and the summary tab, which was already flagged and doesn't need re-raising. I'm now scanning the current diff for genuinely new issues, starting with the filters trigger button where the icon-only size was dropped in favor of a labeled "Filters" span, leaving a now-unused
relativeclass from the removed dot indicator — worth checking that region more closely.
5. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.tsx
6. browse_code — apps/web/src/components/ui/menu.tsx
💭 I'm checking that the ProjectFavicon icon in the project filter submenu correctly receives its sizing className, then recalling how the previous project menu handled this to compare against the current implementation. Now I need to check whether project options passing
unavailableare actually handled by PullRequestFilterRadioGroup, so I should look at lines 150-205 to confirm.
7. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.tsx
8. browse_code — apps/web/src/components/ui/input-group.tsx
💭 I need to check whether Input's size prop supports "compact" in input.tsx, and verify how the menu submenu handles keyboard navigation with the embedded input. I'm also flagging the Filters trigger button's composition of icon, label span, and count as a potential issue. Other things to verify: how PullRequestRow's diff-stat wrapping interacts with the meta line's child-counting logic, whether empty group labels in the pull-requests grouping render an empty header bar, and whether the sort menu's Button variant handles icon color correctly through the control-icon-color CSS variable versus explicit text-color overrides. Let me pull up PullRequestRow at head to check the meta line contract.
9. browse_code — apps/web/src/components/pullRequest/PullRequestRow.tsx
💭 I want to check whether PullRequestDiffStat already handles the zero-case by returning null, since the new row showing "1,234 lines" alongside the diff stat might be redundant rather than a real UI issue. Let me look at pullRequestPresentation to confirm.
10. git_tools — export function PullRequest(DiffStat|MetaLine|ActorAvatar|ActorLabel)
💭 The PullRequestActorAvatar inside MenuRadioItem should be fine since the size selector targets svg elements, not the avatar span/img, and MenuRadioItem's children wrapper has no indicator conflicts. Now I want to check the route file that handles group rendering with an empty label and the "others" key.
11. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
12. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
💭 I'm checking where sortMenu renders in PullRequestsColumn, looking at the control row to see if Filters and Sort are siblings and how they behave responsively.
13. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
14. browse_code — apps/web/src/components/ui/button.tsx
💭 I notice the sort menu's chevron icon uses a hardcoded muted color instead of the
--control-icon-colorvariable, which seems intentional since the outlined Button variant opts out of that indirection for its icon styling. More notably, the accessible name mismatch stands out:aria-label="Sort pull requests"overrides the visible text "Sort: Recently updated", so screen readers never announce the current sort value. This violates WCAG 2.5.3 Label in Name since the visible text isn't contained in the accessible name — though it's a pre-existing pattern from the compact version, it's now surfacing in this new visible-label outlined control too. Moving on, I'm checking the Filters trigger button and its icon sizing — the size is now unset rather than "icon", matching the search input's responsive height, and the icon has an explicit size class that was already there before this diff, so it's not a new issue. I'm also noticing the count badge is built as a raw span with manually applied background, padding, and text styles rather than using the existing Badge primitive that's already used elsewhere in the codebase — checking the badge variants to see if one matches this styling. The row label chips also look like a reconstructed Badge with a border, muted background, and small rounded pill styling, so I want to check badge.tsx to see whether an existing variant already covers this pattern.
15. browse_code — apps/web/src/components/ui/badge.tsx
16. git_tools — labelDotColor|label.color
💭 I want to compare the summary tab's label chip against the row label chip, so I'm checking PullRequestSummaryTab around lines 60-80 and 610-640 to line up their geometry.
17. browse_code — apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx
💭 There are now three distinct label treatments across the summary tab chip, row chip, and filter menu dot, and I've already flagged this duplication twice before. Since the row chip changed in response by dropping the dot, the underlying chip geometry duplication between row and summary tab still hasn't been resolved. Re-flagging this again feels redundant, so I'd rather focus on finding genuinely new issues in this revision instead. I want to diff the prior reviewed commit against head, but since the prior head commits are gone due to a force-push and aren't fetched locally, I need to check the PR's commit list directly.
18. github_api_read_only — repos/{owner}/{repo}/pulls/{pull_number}/commits
💭 I see two new commits since the prior run - one for size edge cases and one aligning filter controls. I want to diff against the earlier commit but the tool couldn't locate that short SHA, so it's probably not fetched; I'll try using the full SHA instead.
19. git_tools — 7589c7d..8654504
💭 With those four findings resolved, I want to confirm the
cnimport is still needed elsewhere in the route file, which it likely is. I should also verify that the Input component actually supports a "compact" size variant.
20. browse_code — apps/web/src/components/ui/input.tsx
💭 Compact size looks valid, and the addon padding tweak in InputGroup only applies to sm, so compact keeps its default padding. I want to check PullRequestReviewerPicker to see if there's an established pattern for a searchable field inside a popup menu that I can reuse for the author search.
21. git_tools — InputGroup
22. git_tools — size="compact"
23. git_tools — autoFocus
💭 Let me check PullRequestReviewerPicker as a precedent, since it's a popover/menu with a search input and list — I want to see how it handles keyboard interaction and whether it uses an Input inside a Popover rather than a Menu.
24. browse_code — apps/web/src/components/pullRequest/PullRequestReviewerPicker.tsx
💭 The author filter's use of InputGroup primitives looks fine, matching precedent. I'm now going through remaining candidate findings one by one, starting with the width utilities on the submenu popups which look okay since they suppress the min-width issue, and moving on to check the author submenu wrapper next. The submenu's search field padding stacks with the parent popup's own padding, giving slightly more spacing than the reviewer picker uses, but this is purely cosmetic. On keyboard handling, I'm checking whether autoFocus and typeahead interactions inside the Base UI menu composite are consistent with the reviewer picker precedent, and whether Home/End key hijacking is worth flagging—it's a minor nitpick I'll likely skip since my earlier suggestion already addressed a related direction on this. Looking at the filterCount badge, I'm weighing whether reconstructing a pill with custom classes instead of the existing Badge component violates the primitive-reuse guideline, but since no matching variant exists, custom styling seems justified—marginal enough to skip flagging. I'm also noting the row duplicates line-count info alongside PullRequestDiffStat, which seems fine, though there's a documented edge case about hosts that don't report line counts where showing +0/-0 could misleadingly read as "no changes" rather than "data missing," so I want to check how changedLines is computed for that scenario. I'm also checking the fixed row height against the new "N lines" text being added—since it sits in the same flex line as the diff stat rather than adding a new row, the intrinsic 54px sizing should still hold. Then I'm considering whether using "others" as a fallback group key could collide with an actual group genuinely named "others" in the data. I'm noting that sort mode collapses all priority-based groupings ("Yours", "Needs review") into a single unlabeled bucket, which silently removes that grouping distinction—more of a product decision than a bug. I'm also confirming that
facetTargetswithlimit: PAGE_SIZEis a data-layer concern rather than a UI consistency issue. Now I'm looking at theCompactFilterMenu'soutlinedprop switching to render an actual Button component, and checking how the button's icon color styling interacts with its variant when rendered this way. I'm checking icon color inheritance across triggers—Filters uses ListFilterIcon with no explicit text class so it inherits the control icon color, while Sort's chevron opts out via its own muted-foreground class, a pre-existing inconsistency. I'm also noting the chevron's explicitsize-3versus the Button's default sizing, plus confirming theMenu onOpenChangewiring and removed separators between filter groups are both fine, and that the Project submenu drops its "Unavailable" badge but retains the tooltip and disabled state through the radio group. Losing that badge is a minor visual regression since the tooltip already covered the explanation, so it's a deliberate consolidation rather than a bug—though I want to double check whether the radio group's disabled logic properly reads the unavailable option flag, since that could be a real issue worth flagging. I'm noticing the ProjectFavicon lost itsshrink-0class compared to before, which could let it shrink in a flex row with a long title, and the option label text isn't wrapped in a truncating span either — worth checking if that causes layout issues withmin-w-0. A raw text node in a flex container becomes an anonymous flex item that can shrink and wrap, so a long title would wrap across lines instead of truncating, while the icon without shrink-0 gets squeezed. I'm also checking whether the submenu popup has a max-width constraint or just relies on min-width, which would let it grow unbounded with content. Now I'm confirming that the shared radio group markup, which existed before this change for host/server/state options, is the same code path project options were newly routed through — meaning long project titles genuinely lost their truncation and the favicon lost its shrink protection as a side effect of reuse, rather than something newly broken in isolation. That feels like a real but modest regression worth flagging. I'm also noting that while the[&_svg]:shrink-0rule protects lucide-style icon elements, ProjectFavicon might render an<img>tag instead of an svg, which wouldn't be covered by that shrink rule — so I need to check how ProjectFavicon actually renders.
25. browse_code — apps/web/src/components/ProjectFavicon.tsx
💭 Since ProjectFavicon already handles shrink-0, losing truncate on the title is a minor issue—long titles just wrap the submenu, so I'll deprioritize that. But I notice the Project trigger passes
aria-hiddento ProjectFavicon, which likely doesn't accept that prop in its type signature, so I need to check that. That confirms this pattern already works elsewhere, so it's not a new issue, and the inline component just ignores aria-hidden anyway. Now I want to double check the Sort control's filter count logic — specifically whether switching sort back to "updated" properly clears the query param in updateSearch, so I should look at that function around lines 430-500.
26. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
27. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
💭 These logic-level items look mostly fine and outside strict UI consistency concerns. I'm checking that the Filters and Sort triggers use consistent Button styling with the refresh control, and heights line up correctly across variants. Now I'm considering how condensed mode handles the header with the CompactFilterMenu. Looking at the label filter's checkbox item structure, I find the layout consistent with the grid template used elsewhere. But there's a subtle interaction inconsistency: checking/unchecking labels keeps the menu open (correct for multi-select), while clicking "Clear selected labels" closes the entire menu since MenuItem closes on click by default — creating an inconsistency where one action in the submenu behaves differently from the others. I also notice the pattern of using a disabled MenuItem to show an empty-state message like "No labels in this view," and I want to check whether this precedent matches how other pickers handle empty states. The closeOnClick inconsistency on the "Clear selected labels" item feels like a genuine finding worth flagging as a suggestion, so I want to double-check how other MenuItems across the codebase set closeOnClick to make sure my assumption about the default holds.
28. git_tools — closeOnClick
💭 I've confirmed that MenuItem closes by default, since the codebase explicitly passes closeOnClick={false} elsewhere. This means the "Clear selected labels" item inconsistently closes the whole filters menu unlike other selections in the same menu—a real bug I should flag with the fix being to add closeOnClick={false}. Checking similar cases: the author submenu's radio items behave consistently by keeping the menu open, so that's fine. There's a minor concern about a disabled MenuItem being nested inside a MenuRadioGroup when no authors are found, which could theoretically affect arrow-key navigation, but it's likely harmless. I'm also noting the filterCount badge's text sizing looks a bit ad-hoc without using a Badge component, though probably not worth flagging. Now I want to check the test file to see if it needs updates for things like the removed dot indicator or the "filtered" state.
29. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx
💭 I'm tracing through the test case for "project chosen again" — following how the labeled group finder recurses through nested rendered components (submenu then radio group, both labeled "Project") to locate the right MenuRadioGroup element. Once found, calling onValueChange("all") should trigger the submenu's onChange, which finds no matching project and falls back to calling onProject(undefined, undefined) since projectId was set. Re-selecting the same project value confirms no call happens since next equals value. Then I move to the second test case, where projects have a duplicate id and projectId is undefined, so projectValue defaults to the "all" constant — selecting a specific project key should trigger onChange to find and select that project. I confirm this resolves to onProject(projectId, "env-2") correctly, so both tests now align with expected behavior, noting the previous no-op-on-reselect logic is now handled by the radio group's own guard. Now I'm looking at a few smaller UI details: the author filter trigger text, the submenu popup width relative to its parent's min-width, and the padding structure around the search input inside the popup wrapper — these all seem acceptable as is. I'm also considering whether autoFocus on the search input could cause unwanted focus stealing when a submenu opens via hover rather than keyboard navigation, since hovering into a submenu while the mouse is already over the parent menu might trigger the same focus behavior unintentionally. This raises a potential concern: hovering to open the submenu could steal focus away from the user's current context, and moving focus outside the composite item list might interact poorly with Base UI's hover/mouse-out handling for submenus — but since I can't verify this without actually running the interaction, I don't want to assert it as a confirmed bug. I'll hold off on flagging it definitively. Looking at the sizing conventions, the
text-[10px]arbitrary value on the label chip matches usage elsewhere already in the file, so that's not an issue. My most concrete finding remains thecloseOnClickinconsistency on "Clear selected labels" — I need to weigh whether that alone is a strong enough regression to fail the check, since it seems more like a minor interaction quirk than a clear violation. Digging further, I'm also noticing that therelativeclass in thecn("relative", ...)call is now redundant since the dot indicator it was positioning was removed and the Button's base class already includesrelative— a trivial but real leftover. Checking the[--control-icon-color:currentColor]override on the Filters trigger, I confirm it's intentional pre-existing behavior tied to the outline variant's icon color token, not a new issue. I'm now looking at whether the count pill'sbg-primary/10 text-primarystyling matches similar token pairings used elsewhere in the codebase.
30. git_tools — bg-primary/1[06]
31. git_tools — text-primary\b
💭 That local pattern checks out fine, so I'm confirming it's not an issue. I'm also verifying the empty state filtering logic looks correct, and checking that the min-w-72 class properly triggers the width-detection helper's explicit width class match. Now I'm reconsidering whether dropping truncate on project titles is actually a problem, since host/server labels could be long too, but since that radio group already existed for host/server, I'll let it go. However, I spot a real issue: the Project trigger's Icon is defined as an inline component within an array that gets rebuilt every render, meaning React treats it as a new component type each time and remounts the favicon — causing it to flicker or reload on every re-render of the filters menu, since projectOptions isn't memoized. Though I realize the module-level favicon cache mostly mitigates this by restoring the cached image synchronously after remount, so cached favicons won't visibly flicker; only uncached ones would restart their load on every render, which could be a subtle issue during frequent re-renders but isn't a strict UI-consistency bug, just a code-quality nit worth flagging. Given my discipline of only posting solid, high-confidence findings, I'm weighing this against the clearer issue that "Clear selected labels" closes the entire filters menu on click while every other control in that menu is designed to stay open — that inconsistency feels like a stronger candidate to actually raise. Any factory approach still creates a new function identity per render, so memoizing projectOptions with useMemo is the more realistic fix, though it only helps when the projects list itself is stable. I'm weighing whether this remount issue is severe enough to flag given the module cache likely masks it — leaning toward skipping it since it risks reading as a nitpick rather than a real regression. Instead I want to check whether PullRequestFilterRadioGroup inside MenuSubPopup duplicates the submenu trigger label via MenuGroupLabel. That duplication looks harmless. I'm also verifying the filters file doesn't have unused imports — Tooltip, MenuRadioItem, MenuGroupLabel, cn, and LayersIcon are all actually used, so that's clean. Now checking the pull-requests chat file for unused MenuSeparator or other leftover imports. Deciding whether the closeOnClick inconsistency warrants failing the check. Since a menu unexpectedly closing is a genuine interaction issue rather than just an aesthetic preference, this leans toward being a real regression risk worth flagging. Tracing the label filter flow further, I confirm the checkbox-to-route-to-search pipeline stays consistent, mapping label names into AND-groups correctly at each stage, and cleared filters get properly dropped via undefined filtering. I also verify the filters passed to the menu component come only from menuFilters, not from any typed query syntax, which matches the earlier comment about labels being handled separately. Checking filterCount, it sums each selected label individually which seems reasonable, and the author filter follows the same undefined-stripping pattern as labels. One edge case I spot: the author radio item compares against the URL value directly, so a hand-typed
?author=Foowith different casing than the list options wouldn't match and show as unchecked — a minor edge case I'll set aside for now. I'm confirming the trigger display for the author submenu falls back to "Anyone" when unset, consistent with the other submenu triggers using their current label. Now I'm locating the exact line for the "Clear selected labels" MenuItem in the diff so I can attach a suggestion comment there, confirming it's on line 333 of the head file and part of the added diff hunk. Writing suggestion diff... Confirming the disabled empty-state MenuItems aren't affected since disabled items don't fire click handlers. Also verifying that Base UI's Menu.Item inside a SubmenuRoot popup closes the whole menu tree by default, which still achieves the intended effect of closing the submenu the user is working in. Time to write up the comment concisely.
32. modify_pr — One finding this round. The earlier notes on the outlined sort trigger, the author search field sizing/key handling, the meta-line separator guard, and the project submenu tests all look resolved.
Agent Credits: 248 credits
Token usage: 46 uncached input · 1,911,014 cache-read input · 137,117 cache-write input · 21,767 output
Agent Credits may also include non-token charges from external tools such as web research.