Security: filter chips, side-by-side Overview, trend range fixes by msogin · Pull Request #105 · Smartling/glooker

msogin · 2026-10-26T17:18:42Z

What changes

Filter chips. Each filter that isn't at its default shows as a removable chip under the dropdowns, like on Projects and Team. That includes Repo, which used to be set from the repo list with nothing in the bar showing it. × resets that filter and returns focus to its dropdown. "Clear all" replaces "Reset filters". The chip row's height is reserved, so adding or removing a chip never moves the page.

Teams panel. It's sized from the org's total number of teams: 5 to 12 rows of 40px, never from the filtered list. An org with 10 teams no longer has to scroll. Filtering never resizes the panel.

Overview layout. From 1100px wide, the teams panel (60%) and the trend card (40%) sit side by side at the same height. Below 1100px they stack as before.

Trend range (existing bugs found while testing on real data):

Only measurements inside the selected range reach the chart, so the y axis fits the range.
A range longer than the history starts at the first measurement instead of squeezing the lines against the right edge.
The trend response now carries historyStart. A range that the history fits inside would draw the same chart as All time, so the Range select greys it out as "(not enough history yet)".

docs/vulnerabilities-page.md ("Layout stability" and the trend card) is updated to match.

Spec: docs/superpowers/specs/2026-10-22-security-overview-layout-design.md
Plan: docs/superpowers/plans/2026-10-22-security-overview-layout.md

Testing

npm test: 3430 pass, 2 fail. Both failures are on main too: chart-no-literal-colors, and the docs "Page modules" check, which trips on an untracked duplicate file in my checkout.
I checked it in a browser on mock data at 1440px and 1024px: card positions and sizes stay the same when filters change.
It's deployed locally against real org data (podman + MySQL).

Known follow-up

In the side-by-side layout, the team table's Total row is too narrow for four-digit Resolved totals: the Change column's "0" overlaps them. Long team names are also cut off with "…".

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… 5-12 rows) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

…Reset filters Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

… Overview Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

…d indent) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Only the points inside the range reach the chart, so the y axis rescales and lines no longer run off the left edge on Last year / 90 / 30 days. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…easurement Last year / 90 days drew a 365- or 90-day axis even when the history is a few weeks, squeezing every line against the right edge. The axis now never starts before All time does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The trend response carries historyStart (the first measured day, whatever since is). A range that would cover all of it draws the same chart as All time, so the Range select disables it as "(not enough history yet)". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

msogin

Smartling review

Reviewed with the Smartling org review prompt (fullstack). No critical issues found. Findings:

🟡 queries.ts:325 historyStart ignores the team filter, so a team whose history starts later can still be offered ranges that look identical to All time.
🔵 vulnerabilities-content.tsx:142: with few teams the trend card's minimum height is taller than the ownership card's body, so the 'same height' layout leaves empty space under the teams table.
🔵 trend-model.ts:30: when historyStart is null (no measurements), every range except the current one shows '(not enough history yet)'. That is accurate but worth a test or a note in the comment.
🟣 filter-bar.tsx:77: removing the Repo chip moves focus to Codebase. Should focus go to the next chip, or to Clear all, instead?

Assessment: OK to merge, with the warning fixed or explicitly accepted.

msogin · 2026-10-26T17:21:17Z

  const snaps = await loadTrendSnapshotRows(p.org, chosen);
+  // The first measured day whatever `since` is, from the set index alone: lets the page tell a range that cuts history from one
+  // that covers all of it (and would draw the same chart as All time).
+  const historyStart = pickTrendSets(index, { codebase: f.codebase, severity }).reduce<string | null>(


🟡 warning: historyStart is computed from the set index for the codebase and severity only. The team filter is ignored. With a team selected, the chart draws only that team's line, so if the team's first measurement is later than the org's, a range can still look the same as All time and stay enabled. The opposite can't happen, because the org history always starts at or before the team's. If greying out ranges should work per team, take the min date from computeTrend's points for the team, or document that historyStart is org-wide. It also runs pickTrendSets over the index a second time, which is cheap but could be folded into one pass.

msogin · 2026-10-26T17:21:17Z

-            <TrendCard {...viewProps} />
+            {/* Below 1100px (OVERVIEW_STACK_BP) the cards stack; from 1100px they sit side by side, the same height: the ownership
+                card's body (sized to the org's teams) sets the row, and the trend card stretches to it. */}
+            <div data-testid="overview-grid" className="grid grid-cols-1 items-stretch min-[1100px]:grid-cols-[minmax(0,3fr)_minmax(0,2fr)]" style={{ gap: PAGE_GAP }}>


🔵 suggestion: Side by side, the trend card has a minimum height. That is the header in column layout, the plot at least 160px, the 112px legend, and the rule and footnote, which comes to roughly 420px or more. ownershipBodyH(5..6) plus the card's tab row comes to less than that. For orgs with about 6 teams or fewer, the trend card sets the row height and the ownership card stretches with blank space under its fixed-height body. Consider letting the ownership body grow to fill the stretched card (flex-1 with min-height: bodyH), or check visually with a 5-team fixture.

msogin · 2026-10-26T17:21:17Z

+ */
+export function rangeCutsHistory(range: TrendRange, historyStart: string | null, today: string): boolean {
+  if (range === 'all') return true;
+  return historyStart !== null && historyStart < addDays(today, -RANGE_DAYS[range]);


🔵 suggestion: The doc comment says unknown history (null) 'cuts nothing'. As a result, an org with no measurements gets every range except the current one disabled as '(not enough history yet)'. That is reasonable, but no test covers it. Add a case to vuln-trend-card.test.tsx for historyStart: null so the behaviour is pinned.

msogin · 2026-10-26T17:21:17Z

+  const chips: Array<{ key: string; label: string; remove: () => void; focus: () => void }> = [];
+  if (!url.isDefault.codebase) chips.push({ key: 'codebase', label: `Codebase: ${CODEBASE_LABELS[url.codebase]}`, remove: () => url.setCodebase(CODEBASE_SCHEMA.default as (typeof CODEBASE_GROUPS)[number]), focus: () => codebaseRef.current?.focus() });
+  if (!url.isDefault.team && url.team) chips.push({ key: 'team', label: `Team: ${url.team}`, remove: () => url.setTeam(null), focus: () => teamRef.current?.focus() });
+  if (!url.isDefault.repo && url.repo) chips.push({ key: 'repo', label: `Repo: ${url.repo}`, remove: () => url.setRepo(null), focus: () => codebaseRef.current?.focus() }); // no repo select exists, so focus falls back to Codebase


🟣 question: There's no Repo select, so removing the Repo chip moves focus to the Codebase select, which is unrelated. Would focus on the next chip, or on 'Clear all' if it's still visible, be more predictable for keyboard users?

msogin

Review-loop (2 personas): Sr. Frontend Eng + Correctness, Sr. Architect + UX/A11y/Maintainability. 1 loop.

No Critical issues. Findings:

I trend-model.ts:55: trendDomain clamps to the first point of a series the server already cut at since, so a ranged axis can start later than the window when there's a gap at the start of the window. Use historyStart instead.
I filter-bar.tsx:182: the chip row has no group label or list semantics.
I (follow-up) filter-chip.tsx:20: the new FilterChip copies the inline chip markup in Projects, Team and Settings, whose × buttons have no accessible name. Open a ticket to move them onto FilterChip.
S trend-model.ts:55: add a test for a ranged domain with a gap at the start of the window.
S filter-bar.tsx:76: removing the Team or Codebase chip also clears Repo, so one × removes two chips. Is that intended?
S filter-bar.tsx:192: Clear all shows at 1 or more active filters here, but Projects shows it only above 1. Pick one rule.
S trend-card.tsx:106: while a new key loads, the disabled range options use the previous key's historyStart. It's cosmetic and short-lived.

Checked with nothing found: historyStart is additive and backward compatible (MCP passes it through), ownershipBodyH clamping, focus return after ×, hidden Clear all a11y, and docs match the code.

msogin · 2026-10-26T17:22:10Z

+  const allStart = dates[0] && dates[0] < floor ? dates[0] : floor;
+  if (range === 'all') return { start: allStart, end };
+  const windowStart = addDays(today, -RANGE_DAYS[range]);
+  return { start: windowStart > allStart ? windowStart : allStart, end };


Important: allStart comes from series, but the server already dropped sets before since (pickTrendSets). With a 200-day history, 90d range and no data between day -90 and day -30, the axis starts at day -30, not at the window start, which contradicts the docs. Clamp against the response's historyStart instead. Also worth adding a test for a long history with a sparse window start.

msogin · 2026-10-26T17:22:10Z

-          >
-            Reset filters
-          </button>
+        <div data-testid="chip-row" className="flex min-w-0 flex-nowrap items-center overflow-hidden" style={{ height: FILTER_CHIP_ROW_H, marginTop: FILTER_CHIP_ROW_GAP, gap: FILTER_ROW_GAP }}>


Important (a11y): The chip row is a plain div, so a screen reader hears the "Remove … filter" buttons with no context. Add role="group" aria-label="Active filters" (or use ul/li).

msogin · 2026-10-26T17:22:11Z

+  return (
+    <span
+      data-testid="filter-chip"
+      className="inline-flex min-w-0 shrink items-center gap-1.5 bg-accent/20 text-accent-lighter text-xs font-medium px-2.5 py-1 rounded-lg border border-accent/30"


Important (follow-up): This class string is identical to the inline chips in projects-content.tsx, report/[id]/team/page.tsx and settings, whose bare × buttons have no accessible name. Suggest a ticket to move those pages onto FilterChip.

msogin · 2026-10-26T17:22:11Z

+  // One chip per non-default filter, in the selects' order; × resets that filter and puts focus on its select (the chip is gone).
+  const chips: Array<{ key: string; label: string; remove: () => void; focus: () => void }> = [];
+  if (!url.isDefault.codebase) chips.push({ key: 'codebase', label: `Codebase: ${CODEBASE_LABELS[url.codebase]}`, remove: () => url.setCodebase(CODEBASE_SCHEMA.default as (typeof CODEBASE_GROUPS)[number]), focus: () => codebaseRef.current?.focus() });
+  if (!url.isDefault.team && url.team) chips.push({ key: 'team', label: `Team: ${url.team}`, remove: () => url.setTeam(null), focus: () => teamRef.current?.focus() });


Suggestion/question: setTeam and setCodebase also null repo, so one × removes two chips. Is that intended? If so, a test or note would help.

msogin · 2026-10-26T17:22:11Z

+            <button
+              type="button"
+              onClick={() => { url.resetFilters(); codebaseRef.current?.focus(); }}
+              disabled={url.isDefault.all}


Suggestion: Clear all shows at 1 or more active filters here, while Projects uses activeFilterCount > 1. Worth settling one rule before FilterChip spreads.

msogin · 2026-10-26T17:22:11Z

+              {/* A range the history fits inside would draw the same chart as All time: greyed out, unless it is the current value
+                  (a disabled selected option would read as broken). Until the trend loads, every range is offered. */}
+              {TREND_RANGES.map(([v, label]) => {
+                const empty = historyStart !== undefined && v !== url.range && !rangeCutsHistory(v, historyStart, today);


Suggestion (minor): Under keepPreviousData, historyStart belongs to the previous codebase/severity while the new one loads, so options can briefly grey out against the wrong history. It's cosmetic.

- Trend axis starts from the response's historyStart, not the series' first point (already cut at since), so a gap at the start of a window no longer starts the axis late. - Chip row is a group labelled "Active filters". - Pin the no-measurements case of the Range select; note that historyStart is org-wide, never per team. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

msogin · 2026-10-26T17:37:23Z

I've addressed the review in 84472d2.

Fixed

Axis starts late after a gap at the start of the window (trend-model.ts:55, Important): trendDomain now uses the response's historyStart, not the first point of the series. The server has already cut the series at since, so its first point can be later than the real history start. I added a test for a long history with a gap at the start of the window.
Chip row has no label (filter-bar.tsx:182, a11y): the row is now role="group" aria-label="Active filters", and a test checks it.
No measurements (trend-model.ts:30): the case where historyStart is null is now pinned by a card test. With no measurements, every range except the current one is greyed out.

Accepted and documented

historyStart ignores the team (🟡 queries.ts:325): it applies to the whole org for the selected codebase and severity. By design, the trend request is never filtered by team: the page filters the series itself, so each team keeps its colour. The comment on rangeCutsHistory now says so. With a team whose history starts later than the org's, a range can stay enabled even though it draws the same chart as All time. That's harmless, and the opposite can't happen. Running pickTrendSets a second time only scans the set index, so I left it as is.
Disabled options use the previous history while new data loads (trend-card.tsx:106): this is cosmetic and lasts only until the new data arrives. Left as is.

Answers

Focus after removing the Repo chip (filter-bar.tsx:77): it stays on Codebase. "Clear all" can vanish when the last chip goes, and the next chip shifts position, so Codebase is the one control guaranteed to exist. Repo is also narrowed by codebase and team, so that's where a keyboard user would go next.
One × also removes the Repo chip (filter-bar.tsx:76): yes, that's intended. A repo is only valid inside the selected codebase and team, so changing either one clears it. This is existing security-state behaviour; the chips just make it visible.
When "Clear all" shows (filter-bar.tsx:192): it shows with one or more active filters. It's the only reset on this page, since it replaced "Reset filters", and its slot is reserved, so showing it never moves anything. Projects should adopt the same rule when it moves onto FilterChip.

Follow-ups, not in this PR

Move Projects, Team and Settings onto FilterChip (filter-chip.tsx:20): this gives their × buttons an accessible name. I'll open a ticket.
Blank space under the teams table with 6 or fewer teams (vulnerabilities-content.tsx:142): the fix is to let the table body grow to fill the card. I'll check it with a 5-team fixture in the same follow-up as the Total-row overlap.

msogin and others added 12 commits October 22, 2026 21:42

Spec: Security tab filter chips and side-by-side Overview

13e4332

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Plan: Security tab filter chips and side-by-side Overview

1f2f9d1

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Security: size the ownership card to the org's team count (40px rows,…

d6e72b2

… 5-12 rows) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Security: show active filters as removable chips; Clear all replaces …

81cb776

…Reset filters Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Security Overview: ownership and trend cards side by side from 1100px

e2b3813

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Security Overview: fix the trend legend's height from 1100px

1915992

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Docs: Security layout stability for filter chips and the side-by-side…

043f6b0

… Overview Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Security: tidy deferred review minors (test title, orgTeamCount, Fiel…

61e8346

…d indent) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Security: Clear all uses the page link style; docs describe the chip row

0c9f9ef

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Security trend: scale the chart to the selected range

5129817

Only the points inside the range reach the chart, so the y axis rescales and lines no longer run off the left edge on Last year / 90 / 30 days. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

msogin commented Oct 26, 2026

View reviewed changes

msogin merged commit f5291a2 into main Oct 26, 2026
1 check passed

Conversation

msogin commented Oct 26, 2026

What changes

Testing

Known follow-up

Uh oh!

msogin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msogin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msogin commented Oct 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant