feat(ActiveFilterPills): inactive-state pill styling and a11y polish

[elizabetdev]

Summary

Refreshes ActiveFilterPills so it can render filters that are present in state but not currently applied to the query (e.g. the column doesn't exist on the active source) in a clearly muted, "inactive" style — without changing any behavior for existing call sites.

• Inactive pill style: dashed --color-border-emphasis border, transparent background, 0.75 opacity, strikethrough label/operator/value, and a multi-line tooltip explaining why the filter isn't applied (with an invalidFieldReason override hook).
• Excluded (!=) pill refresh: now uses the --color-bg-danger / --color-text-danger design tokens instead of raw Mantine red, so the variant follows the theme.
• A11y / testability:
  • data-testid="active-filter-pill-<field>" on every pill for deterministic E2E selection.
  • data-invalid="true" on inactive pills.
  • aria-label="Remove filter" on the close button.
• API: two new optional props on ActiveFilterPills — invalidFields?: Set<string> and invalidFieldReason?: (field) => string. Existing callers don't change.

This PR is purely the component-level work. The feature wiring (search page state, query stripping, source-switch flow) lives in #2360, which is moved to draft for reference.

Test plan

• yarn ci:unit in packages/app still passes.
• Visual check: with invalidFields passed in, pills render dashed / strikethrough / muted and the tooltip explains why.
• Visual check: excluded (!=) pills still read as the "danger" variant via the new tokens.
• Existing call sites (no invalidFields prop) behave identically to before.

Made with Cursor

[changeset-bot]

⚠️ No Changeset found

Latest commit: 72ceb91

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 28, 2026 2:21pm

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 1
• Production lines changed: 95
• Branch: design/filter-pills-inactive-state
• Author: elizabetdev

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

Deep Review

Scope: packages/app/src/components/ActiveFilterPills.tsx (single file, ~80 changed lines). Adds optional invalidFields / invalidFieldReason props, refreshes != styling to semantic tokens, and adds data-testid / data-invalid / aria-label for testability and a11y. No call-site changes. All semantic tokens used (--color-bg-danger, --color-text-danger, --color-bg-hover, --color-border-emphasis) verified present in both themes.

✅ No critical issues found. The diff is additive, well-scoped, and behaviorally backward-compatible for the existing DBSearchPage.tsx caller. P2 items are real but non-blocking.

🟡 P2 -- recommended

• packages/app/src/components/ActiveFilterPills.tsx:84 -- data-testid is keyed only on pill.field, so multi-value filters render N siblings with identical testids, defeating the "deterministic E2E selection" the testid was added for; the very test fixture for this component already creates status=200 and status=404 to exercise that case.
  • Fix: Include pill.type and pill.value (or the composite already used as the React key at line 224) so each pill is uniquely addressable.
  • _{correctness, testing, api-contract}
• packages/app/src/components/__tests__/ActiveFilterPills.test.tsx -- the new isInvalid branch (strikethrough, dashed border, opacity, transparent background, data-invalid="true", default fallback tooltip, invalidFieldReason override) has zero coverage, and the existing excluded-pill tests only assert getByText('500') so the Mantine-red → --color-bg-danger / --color-text-danger swap is invisible to the suite.
  • Fix: Add tests that render with invalidFields={new Set(['status'])} asserting data-invalid="true", that the default tooltip fires when no invalidFieldReason is provided, that the override replaces it, and that excluded pills still carry the danger token styling.
  • _{testing, correctness, maintainability, kieran-typescript, api-contract, project-standards}

🔵 P3 nitpicks (8)

• packages/app/src/components/ActiveFilterPills.tsx:74-77 -- the 130-char default invalid tooltip is inlined and uses invalidReason ?? '...' so an empty-string override silently produces an empty tooltip.
  • Fix: Hoist the default to a module-level constant and use || if empty strings should also fall back.
• packages/app/src/components/ActiveFilterPills.tsx:60-70,220-232 -- isInvalid and invalidReason are two props that only make sense together; invalidReason with isInvalid=false is meaningless and the parent already gates one on the other.
  • Fix: Collapse to invalid?: { reason?: string } so the invariant is expressed in the type.
• packages/app/src/components/ActiveFilterPills.tsx:113-123,133-146 -- the operator <Text> and remove <ActionIcon> set both a Mantine color prop (c="dimmed" / color="gray") and an inline style.color override, forcing the reader to compute CSS precedence to know the rendered color.
  • Fix: Pick one source per element by precomputing the final color and applying via style only.
  • _{maintainability, project-standards}
• packages/app/src/components/ActiveFilterPills.tsx:107,119,129 -- textDecoration: isInvalid ? 'line-through' : undefined is set via inline style on three <Text> children where Mantine's td prop is used elsewhere in this same file.
  • Fix: Switch to td={isInvalid ? 'line-through' : undefined} for consistency with the codebase convention.
• packages/app/src/components/ActiveFilterPills.tsx:81-149 -- six inline ternaries across the wrapper and four children encode three variants (default / excluded / invalid), with textDecoration duplicated three times.
  • Fix: Derive a single variant value once and look styles up from a small map at module scope.
  • _{maintainability, kieran-typescript}
• packages/app/src/components/ActiveFilterPills.tsx:133-146 -- the remove <ActionIcon variant="transparent" color="gray"> overlays var(--color-text-danger) via inline style only when excluded, so the Mantine color prop is dead weight when the override is on and a bare token when it is off.
  • Fix: Drop the color prop and drive the icon color entirely from the inline token, or compute the token once.
• packages/app/src/components/ActiveFilterPills.tsx:165 -- invalidFields?: Set<string> makes callers allocate a Set and does not signal read-only intent.
  • Fix: Tighten to ReadonlySet<string>, or accept readonly string[] and build the lookup internally with useMemo.
• packages/app/src/components/ActiveFilterPills.tsx:82 -- switching every tooltip to multiline maw={280} also changes wrapping for normal pills whose ${field}${op}${value} exceeds 280px, even though the PR scope is only the inactive state.
  • Fix: Apply multiline maw={280} only to the invalid branch, or confirm the wrap behavior is acceptable for normal long-label pills.

---

Reviewers (6): correctness, testing, maintainability, project-standards, kieran-typescript, api-contract.

Testing gaps:

• New invalidFields / invalidFieldReason branches (visual props, data-invalid, default vs. overridden tooltip) are untested.
• Existing excluded-pill tests only assert text presence, so the Mantine-red → semantic-token migration has no regression coverage.
• New data-testid and aria-label="Remove filter" are not exercised by the suite (existing tests still query via getAllByRole('button') by index).

[github-actions]

E2E Test Results

✅ All tests passed • 192 passed • 3 skipped • 1248s

Status	Count
✅ Passed	192
❌ Failed	0
⚠️ Flaky	3
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →