feat(viewer): add review comment tag filters

[amh1k]

Description

Adds client-side filtering for review comments in the WebUI Viewer.

Users can now filter comments by severity or category, including Critical, High, Bug, Security, Maintainability, Test, and Other. Nonmatching comments and empty file groups are hidden, with an empty-state message when no comments match.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• go test ./...
• go test -race ./internal/viewer
• go vet ./...
• node --check internal/viewer/static/session.js
• git diff --check
• Manual testing with a temporary session containing categorized review comments

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of the code
• I have added tests that prove this feature works
• New and existing unit tests pass locally
• I have updated the documentation accordingly (not required for this UI-only change)
• I have signed the CLA

Related Issues

Fixes #778

[github-actions]

🔍 OpenCodeReview found 2 issue(s) in this PR.

• ✅ Successfully posted inline: 2 comment(s)

[github-actions]

[maintainability · low]
The "All" button only appears in the severity filter bar. If the severity filter bar happens to be hidden (e.g., in a future refactor) or if a user is focused on the category bar, the only way to reset from a category filter is to click the already-active category chip again. Consider adding an "All" chip to the category filter bar as well for consistency and discoverability, or alternatively moving the "All" button outside both {{with}} blocks so it's always visible.

[lizhengfeng101]

Nice feature — the filter chips feel clean and the severity normalization is a solid catch. A couple of small things I noticed:

1. Leftover .severity-bar CSS. The template no longer renders .severity-bar (it's been replaced by .comment-filter-bar .severity-filters), but the rule is still sitting in style.css around line 882 with no references anywhere in the branch. Probably worth deleting it so it doesn't linger as dead code.

2. No "All" chip on the category bar. The severity bar has its All chip, but the category bar doesn't, so clearing a category filter is a bit non-obvious — you either click the active chip again to toggle it off, or reach over to the severity All. Adding a matching All chip to the category bar would make resetting more discoverable and keep the two bars consistent.

Neither is blocking — just some polish. Thanks for the work here!

[amh1k]

Nice feature — the filter chips feel clean and the severity normalization is a solid catch. A couple of small things I noticed:

1. **Leftover `.severity-bar` CSS.** The template no longer renders `.severity-bar` (it's been replaced by `.comment-filter-bar .severity-filters`), but the rule is still sitting in `style.css` around line 882 with no references anywhere in the branch. Probably worth deleting it so it doesn't linger as dead code.

2. **No "All" chip on the category bar.** The severity bar has its `All` chip, but the category bar doesn't, so clearing a category filter is a bit non-obvious — you either click the active chip again to toggle it off, or reach over to the severity `All`. Adding a matching `All` chip to the category bar would make resetting more discoverable and keep the two bars consistent.

Neither is blocking — just some polish. Thanks for the work here!

Done

[lizhengfeng101]

Thanks for adding the category All chip! Playing with it a bit more though, I think I steered you slightly wrong with that suggestion — sorry about that. The deeper issue is that the filter keeps a single global state, so the two All chips aren't really independent:

let activeKind = 'all';
function cardMatches(card){
  if (activeKind === 'all') return true;
  return card.dataset[activeKind] === activeValue; // only ever one dimension at a time
}

A few consequences of that:

• Clicking either All runs the same kind === 'all' reset and clears everything, so the two chips do the exact same thing.
• You can't combine dimensions — e.g. there's no way to see "Critical and Bug".
• Once you pick, say, Bug, activeKind becomes category, so the severity row loses its highlight entirely and looks like nothing is selected there.

I think the cleaner model is two independent dimensions AND-ed together, each row with its own All:

let activeSeverity = 'all';
let activeCategory = 'all';
function cardMatches(card){
  return (activeSeverity === 'all' || card.dataset.severity === activeSeverity)
      && (activeCategory === 'all' || card.dataset.category === activeCategory);
}

That makes each All reset only its own row, lets you stack filters like Critical + Bug, and keeps both rows independently highlighted.

One small thing on top: it'd help to prefix each row with a little header label (Severity: / Category:) so it's obvious at a glance what each bar controls. Right now the two chip rows sit back-to-back with nothing distinguishing them.

Not blocking, but I think it'd make the interaction feel a lot more predictable. Thanks again!

[lizhengfeng101]

One more small visual nit on the filter chips: the active state currently stacks two selection indicators, which reads as a double ring.

.comment-filter-chip.is-active {
    border-color: currentColor;                                  /* ring #1: border at the edge */
    box-shadow: 0 0 0 2px var(--surface), 0 0 0 3px currentColor; /* ring #2: a surface-colored gap, then a colored outer ring */
}

The first box-shadow layer uses --surface (same as the background), so it renders as a gap between the border and the outer colored ring — you end up with two concentric circles around the selected chip.

Collapsing it to a single ring looks a lot cleaner:

.comment-filter-chip.is-active {
    border-color: transparent;
    box-shadow: 0 0 0 2px currentColor;
}

I tried this locally and it reads much better — one crisp ring, no gap, and no layout shift since the chip already reserves a 1px transparent border. Totally optional polish, but figured I'd pass it along. Thanks!

[lizhengfeng101]

@amh1k

[lizhengfeng101]

LGTM