PR 1255: make the ignore rule dialog and the ignored checks clearer

Your PR, reviewed twice at 1ca1526: Cody's process (codyrancher/ai-skills c04c11a) and ours, then merged and re-checked. Nothing was posted to GitHub.

Preview: harvesterignoreuxr, built from 1ca1526, backend Harvester .105 (v1.10.0-rc1 with the resource health controller). Its certificate is still waiting on Let's Encrypt's rate limit, so the browser warns. Baseline: harvester-main at f3ca9e4, same backend.

Where it stands: everything the issue asks for works, 3 things to fix first

Every point of issue 12277 works on the preview in both themes, and lint, unit tests and CI are green.

  1. The grey text on the yellow rows is under 4.5:1 (both passes)
  2. The yellow rows don't highlight on hover
  3. Chips show twice when a check becomes ignored as a whole (Copilot's, reachable with the default rule)

Then one optional tidy in the same function (4), one call for you (5), one for the designer (6), and the docs PR to update after the merge (7).

How the two passes compare

#FindingPass A (Cody)Pass B (ours)Lands as
1Grey text on the yellow rows under 4.5:1Accessibility passSuggestionFix
2Yellow rows don't highlight on hoverIn the probe data, not filedSuggestionFix
3Chips twice when the check becomes ignored as a wholeCopilot's, reproducedCopilot's, dropped (premise doesn't hold)Fix
4The ignored-name filter on huge checksCopilot's, measuredCopilot's, not practicalOptional
5The banner stays after a new expiration-QuestionYour call
6The expired-rule dialog scrolls at 1280x720FYI-Designer
7Docs PR 1172 describes the old dialog and tableSide effects-Follow-up
Pass A (Cody's process)Pass B (our process)
Verdict2 comments: contrast, docsReady once the contrast is fixed
Checked by bothEvery point of the issue on the preview against .105, in light and dark, with fake ignored states in the browser and every write aborted. The contrast drop, measured the same way (backgrounds composited) and landing on the same numbers.
Only one pass didBefore/after demo videos. A sourced acceptance table. His a11y probe, over CDP, with a control run on an active rule to set shell findings aside. The docs and test-steps side effects. Copilot's duplicate reproduced in jest.The main-vs-PR contrast table in both themes. The lost hover colour. Copilot's duplicate in the browser in both data shapes. Full lint and test:ci. A coverage grid.
OutputNotes per pass, 2 videos, probe JSONreview.md, coverage.md, checked.md, draft.md, 26 screenshots
CostIn this sessionA separate agent: 71 tool calls, about 210k tokens, 25 minutes

Findings

1

The grey text on the yellow rows is under 4.5:1

Found by Pass A (Cody)Found by Pass B (ours)
Fix

The yellow is the warning banner's background, but the rows keep the greys picked for a plain row. The banner itself puts body text on it, not muted text. Both passes measured it, with the row colour composited over what is behind it:

Text on a yellow rowThememainPR
Muted: API version; name and message of a check ignored as a wholedark4.623.6
Samelight5.234.58
Label: resource kind ("Upgrades")light5.034.41
Samedark7.355.73

All of it is 12-14px normal weight, so WCAG 1.4.3 asks for 4.5:1. The dashed ignored chips are disabled tags, which are exempt.

Dark theme, the whole-ignored check on main (top) and on the PR (bottom): the grey text is fainter on the yellow.
Dark, main on top, the PR below: the same grey text is fainter on the yellow.

Ways out, for you or the designer: body text instead of muted text on those rows (what the banner does), a lighter tint, or the yellow on a narrower element than the whole row. Each changes the look, so I haven't picked one.

2

The yellow rows don't highlight under the pointer

Found by Pass B (ours)Found by Pass A (Cody)
Fix

The new rule wins over the table's hover colour. On main every check row turns light grey under the pointer; on the PR the yellow rows stay yellow, so they are the only ones with no hover feedback. Pass B saw it from the computed colours; Pass A's probe measured the same background at rest and on hover.

3

Chips show twice when the check becomes ignored as a whole

Raised by CopilotFound by Pass A (Cody)
Fix

Copilot's point holds. Open "+ N ignored" on a check, then widen its rule to cover every resource: the controller marks the check ignored as a whole and keeps each name's own flag, the row then lists all names and appends the ignored ones again, and the link that would close the list is gone. Vue also warns about duplicate keys.

Pass B dropped it, reading that this shape needs a second rule on the check. It doesn't: the dialog's default Create is "Selected resources" with every resource selected, and for such a rule the controller sets the check ignored once every resource is (ignore.go, "a check is ignored as a whole once every affected resource of it is ignored"). Pass A reproduced it in jest, Pass B in the browser by changing the store.

UpgradeLogNotReady ignored as a whole with hvst-upgrade-xv5nq listed twice and no link to close the list.
hvst-upgrade-xv5nq twice, four chips under a count of 3, no link left.
4

The ignored-name filter is slow on huge checks

Raised by Copilot
Optional

Copilot's other point. The list of ignored names checks every name against the list of reported ones, so its cost grows with the square of the names, and only while that check's list is open. Pass A: 79 ms at 2,000 names, 640 ms at 10,000, 4.2 s at 30,000 (drawing the chips included). Pass B: 0.36 ms at 1,000 for the filter alone. .105's checks have at most 5 names. It sits in the same function as 3, so it costs nothing extra to tidy while fixing that.

5

The banner stays after a new expiration is chosen

Found by Pass B (ours)
Your call

After picking 30 days the banner still says "To make any changes, set a new expiration", although that has just been done and "Will expire on ..." shows under the radios. I kept it because your note said the banner should always show; the issue only describes it when the dialog opens. Hiding it once a new expiration is picked is one line.

6

The expired-rule dialog scrolls at 1280x720

Found by Pass A (Cody)
Designer

The banner makes the dialog taller, so at a 720px-high window Reason is cut off until you scroll inside the dialog. On main the same dialog fits. Layout only.

7

Docs PR 1172 describes the old dialog and table

Found by Pass A (Cody)
Follow-up

harvester/docs PR 1172 is still open and says "Entire check", "Ignore for", "+ 6 ignored", that the Show ignored label counts checks and resources, and that changing an expired rule without a duration shows a warning. Its dialog screenshot shows the old labels. It needs those lines and that image updated once this merges. PR 1225's Test Steps, which QA follows for issue 9747, also say "Entire check".

Pass A: Cody's process

Followed my-pr-full-review in this session, mapped to our setup: the PR preview for his dev server, harvester-main (rebuilt at f3ca9e4) for his master build, local files instead of his harness, our recorder for the demos, his a11y-probe.mjs run from the clone over CDP (axe not run: the probe looks for it under /workspace only). The ignored states were added in the browser on top of .105's real checks and your two real rules; every write was aborted.

Before this change: main.
With this change: the PR.

Acceptance, from issue 12277 (written by us from your message, so a weaker standard)

CriterionVerdict
AC1"Ignore for" reads "Expiration"Met
AC2"Entire check" reads "All resources"Met
AC3Expired rule in Edit: the warning first in the dialog, with the issue's textMet
AC4Only Expiration enabled, Save too, until a new one is setMet
AC5"+ N ignored" is a link like "+ N more", right after it, and shows the ignored resourcesMet
AC6"Show ignored checks" with a badge of the ignored checksMet
AC7Checks with anything ignored get the warning banner's backgroundMet, but see 1 and 2
AC8Create, View and non-expired rules unchanged apart from the labelsMet

Pass B: our process

A separate agent that didn't see Pass A. Verdict: ready once the contrast is fixed; 0 blockers, 1 suggestion (findings 1 and 2), 0 nits, 1 question (finding 5). It read Copilot's two points as not worth posting; the merge keeps the duplicate (finding 3) because the shape it needs comes from the dialog's default rule.

Also seen

What I'd do

It's your PR, so nothing to post as a review. I'd fix 1 to 3 (and 4 with 3) in one more commit, once you pick how the yellow rows should look, then reply to Copilot's two threads and resolve them, and update the docs PR after the merge.