Code Review Bench PR #26556 - feat: limit badges to 2 with hover/click popover in UserListTable by ketkarameya · Pull Request #8 · gitarcode/cal.com-combined-gitar

ketkarameya · 2026-02-10T20:34:47Z

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#8

feat: limit badges to 2 with hover/click popover in UserListTable

Summary by Gitar

New component:
- LimitedBadges in components/ui/LimitedBadges.tsx consolidates badge limiting logic with responsive popover (hover on desktop, click on mobile)
Badge display refactor:
- Replaced HoverCard implementations in UserListTable teams/attributes columns and ResponseValueCell with LimitedBadges
Type safety improvements:
- Added explicit CellContext<UserTableUser, unknown> and HeaderContext types throughout UserListTable.tsx
Code quality:
- Simplified conditional logic from nested ternaries to explicit if statements for better readability

_{This will update automatically on new commits.}

gitar-bot-staging · 2026-02-10T20:43:12Z

Code Review ⚠️ Changes requested 0 resolved / 5 findings

Reusable LimitedBadges component is a nice consolidation, but has three important bugs: hidden popover items don't wire up onClick handlers (breaking click-to-filter for overflow teams), attribute badge labels unconditionally append a trailing space when weights are disabled, and the compound badge visual styling for weight suffixes is lost.

⚠️

Bug: Popover hidden items don't fire onClick handlers

📄 apps/web/components/ui/LimitedBadges.tsx:92

The LimitedBadges component accepts onClick in each BadgeItem, but only the visible badges wire up onClick. The hidden items rendered inside the popover are plain <span> elements with no click handler, despite having cursor-pointer styling that suggests they should be interactive.

This is a functional regression in the teams column of UserListTable, where clicking a team badge triggers table.getColumn("teams")?.setFilterValue([team.name]). If a user has 3+ teams, the overflow teams in the popover won't be clickable to filter — breaking the existing click-to-filter behavior for those items.

Suggested fix

                {hiddenItems.map((item, index) => (
                  <span
                    key={item.label}
                    className="text-default cursor-pointer text-sm hover:text-emphasis"
                    onClick={item.onClick}>
                    {item.label}
                  </span>
                ))}

⚠️

Bug: Attribute weight label has trailing space when weights disabled

📄 apps/web/modules/users/components/UserTable/UserListTable.tsx:252

When attribute.isWeightsEnabled is false, weight remains "". The label is then built as:

label = `${label} ${weight}`;  // e.g. "Engineering " (trailing space)

This unconditionally appends a space + empty weight to every attribute badge label, producing labels like "Engineering " instead of "Engineering". The original code only appended the weight suffix when it existed (it used a separate badge).

This is a visual regression that will be visible on every attribute badge when weights are not enabled.

Suggested fix

                    let label = attributeValue.value;
                    if (weight) {
                      label = `${label} ${weight}`;
                    }
                    label = `${label}${groupIndicator}`;

⚠️

Bug: Attribute weight suffix loses its distinct visual styling

📄 apps/web/modules/users/components/UserTable/UserListTable.tsx:240

The original code rendered the attribute weight as a separate, visually distinct badge with:

A darker background (color-mix(in hsl, var(--cal-bg-emphasis), black 5%))
rounded-l-none on the weight badge and rounded-r-none on the value badge, creating a compound pill effect

The refactored code flattens this into a single text string ("Engineering 75%"), completely losing the compound badge visual treatment. This is a UI regression that removes an intentional design distinction between the attribute value and its weight percentage.

If the compound badge style is intentionally being simplified, this should be called out explicitly in the PR description as a design change rather than a pure refactor.

💡 Edge Case: Duplicate labels cause React key collisions in LimitedBadges

📄 apps/web/components/ui/LimitedBadges.tsx:69 📄 apps/web/components/ui/LimitedBadges.tsx:94

Both the visible and hidden badge lists use key={item.label} for React reconciliation. If two items share the same label (e.g., duplicate attribute values or team names), React will log a warning and may incorrectly reuse DOM nodes, potentially causing rendering glitches.

Using the array index as part of the key (e.g., key={\${item.label}-${index}`}) or just key={index}would avoid this issue, since the list is derived from a stableitems` array.

💡 Quality: Unused useEffect import in LimitedBadges

📄 apps/web/components/ui/LimitedBadges.tsx:7

useEffect is imported from React but never used in the LimitedBadges component. This is a dead import that should be removed to keep the code clean.

Suggested fix

import { useCallback, useMemo, useState } from "react";

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: PR description was minimal (~40 chars). Added 4-category technical summary with component creation, refactoring, type safety improvements, and code quality enhancements.

_{5 rules not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

gitar-bot-staging · 2026-02-10T20:45:20Z

apps/web/components/ui/LimitedBadges.tsx

+            onMouseEnter={handleMouseEnter}
+            onMouseLeave={handleMouseLeave}>
+            <div className="flex flex-col gap-1">
+              {hiddenItems.map((item, index) => (


⚠️ Bug: Popover hidden items don't fire onClick handlers

The LimitedBadges component accepts onClick in each BadgeItem, but only the visible badges wire up onClick. The hidden items rendered inside the popover are plain <span> elements with no click handler, despite having cursor-pointer styling that suggests they should be interactive.

This is a functional regression in the teams column of UserListTable, where clicking a team badge triggers table.getColumn("teams")?.setFilterValue([team.name]). If a user has 3+ teams, the overflow teams in the popover won't be clickable to filter — breaking the existing click-to-filter behavior for those items.

_{Was this helpful? React with 👍 / 👎}

Suggested change

{hiddenItems.map((item, index) => (

{hiddenItems.map((item, index) => (

<span

key={item.label}

className="text-default cursor-pointer text-sm hover:text-emphasis"

onClick={item.onClick}>

{item.label}

</span>

))}

Apply suggested fix

gitar-bot-staging · 2026-02-10T20:45:21Z

apps/web/modules/users/components/UserTable/UserListTable.tsx

+                      groupIndicator = " (group)";
+                    }
+                    let label = attributeValue.value;
+                    label = `${label} ${weight}`;


⚠️ Bug: Attribute weight label has trailing space when weights disabled

When attribute.isWeightsEnabled is false, weight remains "". The label is then built as:

label = `${label} ${weight}`; // e.g. "Engineering " (trailing space)

This unconditionally appends a space + empty weight to every attribute badge label, producing labels like "Engineering " instead of "Engineering". The original code only appended the weight suffix when it existed (it used a separate badge).

This is a visual regression that will be visible on every attribute badge when weights are not enabled.

_{Was this helpful? React with 👍 / 👎}

Suggested change

label = `${label} ${weight}`;

let label = attributeValue.value;

if (weight) {

label = `${label} ${weight}`;

}

label = `${label}${groupIndicator}`;

Apply suggested fix

gitar-bot-staging · 2026-02-10T20:45:22Z

apps/web/modules/users/components/UserTable/UserListTable.tsx

              return (
-                <div className={classNames(isNumber ? "flex w-full justify-center" : "flex flex-wrap")}>
-                  {attributeValues?.map((attributeValue) => {
+                <LimitedBadges


⚠️ Bug: Attribute weight suffix loses its distinct visual styling

The original code rendered the attribute weight as a separate, visually distinct badge with:

A darker background (color-mix(in hsl, var(--cal-bg-emphasis), black 5%))

rounded-l-none on the weight badge and rounded-r-none on the value badge, creating a compound pill effect

The refactored code flattens this into a single text string ("Engineering 75%"), completely losing the compound badge visual treatment. This is a UI regression that removes an intentional design distinction between the attribute value and its weight percentage.

If the compound badge style is intentionally being simplified, this should be called out explicitly in the PR description as a design change rather than a pure refactor.

_{Was this helpful? React with 👍 / 👎}

Apply suggested fix

gitar-bot-staging

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

kayagokalp · 2026-02-24T22:38:57Z

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 4/4 = 100.0%)

ID	Severity	Title	Found?
GT-8-001	Important	Hidden popover items render without onClick handlers, breaki	Yes
GT-8-002	Important	Using non-unique labels as React keys causes key collision w	Yes
GT-8-003	Minor	Unconditional string concatenation produces trailing whitesp	Yes
GT-8-004	Minor	Attribute weight loses distinct visual styling by being conc	Yes

Extra Gitar Findings

File	Line	Valid Bug?	Notes
apps/web/components/ui/LimitedBadges.tsx	7	No	Unused useEffect import in LimitedBadges

Summary

Recall: 100.0% (4/4)
Precision: 80.0% (4/5)
Precision counts ground truth matches + valid extra findings as true positives

tomerqodo added 2 commits January 25, 2026 12:02

Apply PR changes (squashed due to cherry-pick conflicts)

a35a098

update pr

8b8cbd5

gitar-bot-staging bot reviewed Feb 10, 2026

View reviewed changes

gitar-bot-staging bot requested changes Feb 10, 2026

View reviewed changes

kayagokalp closed this Feb 25, 2026

kayagokalp deleted the corrupted_pr_26556_20260125_2421 branch February 25, 2026 02:05

kayagokalp restored the corrupted_pr_26556_20260125_2421 branch February 25, 2026 02:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code Review Bench PR #26556 - feat: limit badges to 2 with hover/click popover in UserListTable#8

Code Review Bench PR #26556 - feat: limit badges to 2 with hover/click popover in UserListTable#8
ketkarameya wants to merge 2 commits intobase_pr_26556_20260125_2421from
corrupted_pr_26556_20260125_2421

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading

Gitar Rules

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

gitar-bot-staging bot left a comment

Uh oh!

kayagokalp commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ketkarameya commented Feb 10, 2026 • edited by gitar-bot-staging bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Gitar Rules

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot-staging bot left a comment

Choose a reason for hiding this comment

Uh oh!

kayagokalp commented Feb 24, 2026

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 4/4 = 100.0%)

Extra Gitar Findings

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading