PR 19808 review: Cody's process, then ours

"Count not-ready pods separately on the Workloads Overview" by codyrancher · head 132a873 · fixes your issue 19643 · you are the requested reviewer. Reviewed 2026-11-09 against the mf Rancher (v2.16.0).

Where it stands

It works. On the PR build one not-ready pod is counted blue on its own and turns green on the next poll, with no reload. Checked as admin and as a project member. Master reproduces the bug for both.
Both processes found the same one thing: the Pods card now has two rows named "Running" that only the dot color tells apart, which screen readers can't see (S1).
Ours adds one question for you (Q1): your issue allowed a simpler fix, letting the state name decide the color. The PR took the more accurate option, and the split rows come with it.
CI: Branch Milestone is red on purpose: master is frozen as v2.16.0, and every v2.17.0 PR on master fails it until 2.16 is cut. One @components e2e job was cancelled after 6 h; the @explorer2 jobs that hold the overview spec passed for both roles.
Nothing was posted to GitHub. Both drafted reviews are below.

Test it yourself

The PR's fixture is still on the mf local cluster: namespace a-issue-19643 with three ready pods and a-not-ready, which stays Running but not ready until /tmp/ready exists. Open both previews from the bar above as admin (local user).

# make it ready (both pages, no reload: the PR turns green within one 5 s poll, master stays blue)
kubectl --kubeconfig kubeconfig/kube_config_server_mf.yaml -n a-issue-19643 exec a-not-ready -- touch /tmp/ready
# make it not ready again (about 6 s for the probe to fail)
kubectl --kubeconfig kubeconfig/kube_config_server_mf.yaml -n a-issue-19643 exec a-not-ready -- rm -f /tmp/ready
# remove the fixture when done
kubectl --kubeconfig kubeconfig/kube_config_server_mf.yaml delete ns a-issue-19643

As admin with all namespaces, mf has 87 Running pods. Master samples a-not-ready first (confirmed through the API), so all 87 go blue; the PR shows 1 blue and 86 green.

Before and after (the same walk, as a project member)

master: 4 Running pods blue, still blue after the pod is ready

PR: 1 blue and 3 green, then 4 green on the next poll

Pass 1: Cody's process

Run as my-pr-full-review: demos, the base review, the seven passes, then each comment through verify, refine, final pass, remove suggestions, condense and the pre-existing check. The passes ran one after another in this session instead of as separate agents; what replaced his workspace is listed on the process page.

Passes 1 and 2: acceptance criteria and verification

	Criterion	Asked for in	Verdict	How it was checked
AC1	Healthy Running pods are green on the overview (By State, By Type, By Namespace), matching their badges on the Pods list.	Issue body, Expected Result 1stated	Met	PR build, admin, all namespaces: Pods 1 blue and 86 green; the `a-issue-19643` card shows 1 blue and 3 green. Master: 87 blue.
AC2	One transitioning pod does not repaint its whole state.	Issue body, Expected Result 2: "either the state name decides the colour … or transitioning resources are counted separately"stated	Met	Same run. The PR takes the second option.
AC3	A stale color doesn't stay once the resources change; no reload.	Issue body, Expected Result 3; codyrancher, 2026-11-04, step 6stated	Met	All green 11 s after the pod reported Ready, without reloading, for both roles. Master stays blue until a reload.
AC4	The same as a Project Member who sees one project.	codyrancher, 2026-11-04, step 1: "Sign in as a user who can see only one project"from the assignee's comment	Met	A throwaway project member in a project holding only `a-issue-19643`: 1 blue and 3 green, then 4 green. User and project deleted afterwards.
AC5	Every resource is still counted once.	implied: the counts are what the overview is for	Met	PR 1 + 86 + 139 = 226 pods, the same as master's 87 + 139. Unit test "should not count more flagged resources than the summary has".

Out of scope (not raised in the thread): the Cluster Dashboard status cards and the list's state filter chips still sample through useStateColor, which the PR description says. Other workload types: Steve answers 422 "column is invalid [metadata.state.error]" for Deployments on this Rancher, and their state names already say "updating". Open questions: none.

The other passes

Base reviewbugs, checklist

No bugs. The split goes error first, then transitioning with error=false, then the state name's own color, per namespace, capped at what the main summary has. That is the order colorForState uses for the list badge. Checklist items hold; the accessibility box is ticked, see pass 7.

3 · Simplify

Nothing filed. Dropped: stateNameColor repeats the darker-to-disabled mapping useStateColor already has twice (taste, no concept removed). "Let the state name decide" is simpler but does not behave the same, so this pass cannot file it.

4 · Side effects

Nothing filed. Followed out: 4 more summary requests per 5 s poll for Pods and Jobs (in the PR description); a failing filtered request falls back to sampling without redirecting (unit tests); stateCounts to states on WorkloadDashboardEntry has no other user; useStateColor's other callers are unchanged; the overview e2e spec is green for admin and standard user.

5 · Component reuse

No new component.

6 · Design fidelity

No design linked. Dark mode checked anyway: the split renders with the theme's tokens.

7 · Accessibility

1 filed: the split rows (1.4.1, below). Dropped: focus is lost if it sits on the blue row when a poll removes it. Rows already vanish on poll on master, and this needs focus on that exact link at that moment.

The one comment, phase by phase

Two "Running" rows told apart only by color

shell/pages/c/_cluster/explorer/workload-dashboard/composable.ts:253 states: w.states

filed

Master Pods card: one Running row, 87, blue; Completed 139 green — master: one Running row

PR Pods card: Running 1 with a blue dash, Running 86 with a green dash, Completed 139 — PR: two Running rows; the dashes are the only difference

Base / pass 7 draftThe Pods card lists not-ready pods as a second "Running" row and only the dot color separates them; the indicator is aria-hidden, so 1.4.1 fails for the card.
Verify: holds.Playwright ARIA snapshot of the card, light and dark: link "Running" then 1, and link "Running" then 3, both to pod?stateFilter=running. Screenshot attached.
Refine
User impact: A screen reader reads "Running, 1" and "Running, 86" with nothing saying which pods are not ready, and someone who can't tell blue from green sees two identical rows.
Why fix it: Telling not-ready pods apart is what the split is for, and only the color carries it.
Repro: Workloads > Overview with one Running pod not ready, By Type > Pods.
Final pass
The not-ready pods get a second row also named "Running" on the Pods card, and only the dot color, which is aria-hidden, tells the two apart. A screen reader reads "Running 1" and "Running 86", so the split this PR adds is lost on anyone who can't see the color.
Remove suggestionsNothing to remove.
Condense: 226 characters
The not-ready pods get a second Running row on the Pods card, told apart from the ready one only by the dot color, which screen readers skip, so anyone who can't see the colors can't tell which count is not ready (WCAG 1.4.1).
Pre-existing: introduced, kept.Master renders one Running row for the state (left screenshot, master build of the same moment); the duplicate label only exists with this PR.

Pass 1 output: the pending review it would leave

Review body (PR-level comment, the fixed demo format)

Before this change: [before.mp4]

With this change: [after.mp4]

Inline · composable.ts:253

The not-ready pods get a second Running row on the Pods card, told apart from the ready one only by the dot color, which screen readers skip, so anyone who can't see the colors can't tell which count is not ready (WCAG 1.4.1).
[pods-card.png]

Pass 2: our process

The checklist in prompts/code-review.prompt.md plus the review rules we keep in memory: only what the PR introduces, the simplest fix over edge cases, look-and-feel goes to the designer, and the review shape (intro line, inline comments, no code, no preview link). Same previews and evidence as pass 1.

Summary

The overview used to color each state from one sampled resource and cache it, so one not-ready pod turned every Running pod blue until a reload. For Pods and Jobs the PR now asks Steve for two more summaries per poll (in error, and transitioning but not in error) and splits each state's per-namespace counts by those, coloring the rest by the state name. Other workload types keep the sampled color, since Steve rejects those filters for them. Thirteen new unit cases cover the split, the cap, the fallback and the next poll.

Findings

Two "Running" rows told apart only by color

composable.ts:253 · Accessibility: "information is never conveyed by color alone"

Suggestion

Same finding as pass 1. Our wording describes the fix in a sentence; what the split row should be called is a question for the designer.

Inline comment

The not-ready pods now get their own "Running" row on the Pods card, and only the dot color, which is aria-hidden, tells it apart from the ready one, so a screen reader reads "Running 1" and "Running 86" with nothing saying which is which. A label on the split row, or naming it in the badge's accessible name, would fix it; the wording is probably one for the designer.

Your issue allowed a simpler fix

composable.ts:36-49, 352-381 · "Simple fix over edge cases"

Your call

Your Expected Result 2 said either the state name decides the color, or transitioning resources are counted separately. The PR took the second. The first would be a few lines (drop the sampling for these types and color by name, as stateNameColor already does).

	The PR (count separately)	State name decides
Extra requests	4 more summaries every 5 s	none, and the sampling requests go too
Pods card	Two "Running" rows (S1)	One row per state
Blue link	Opens every Running pod, ready ones too (in the PR description)	No blue row to explain
A not-ready Running pod	Blue here and blue on the Pods list	Green here, blue on the Pods list
Other screens	Unchanged	Could fix the Cluster Dashboard cards too, if done in `useStateColor`

Only the last-but-one row favors the PR, and it is the row the PR exists for. If the overview should flag not-ready pods, the PR is right and S1 is the cost. Suggested question if you want to ask it: "Did you weigh the simpler option from the issue, coloring by the state name? This one is more accurate; I'm asking because of the extra requests and the split rows."

FYI, not for the review

FYI

Branch Milestone fails because branches-metadata.json still maps master to v2.16.0, now with codeFreeze: true. That is the gate keeping 2.17 work out of 2.16; the six newest v2.17.0 PRs on master all fail it. Leave it red.
e2e standard_user @components was cancelled at 6 h in "Run user tests". The overview spec lives in @explorer2, green for both roles; master's e2e workflow has also failed on its last six runs.
Follow-up, same on master: the Cluster Dashboard status cards and the list state chips still use the sampled color. Not this PR's to fix.

Nits dropped (3)

The commit subject is 56 characters, over 50; it will be squashed.
The new ExtraResponses test type uses any, like the helper it extends.
stateNameColor repeats a mapping useStateColor has.

Checklist

Section	Result	Notes
1 Vue components	n/a	No `.vue` file changed.
2 TypeScript and code quality	OK	Typed, no `any` in source, aliases, names, every new export used.
3 Security	OK	The new URL parts are constants.
4 Store and API	OK	`cluster/request`, `cluster/urlFor`, types from `@shell/config/types`; failures fall back and are tested; polling unchanged.
5 i18n	OK	No new strings.
6 Styling	n/a	None.
7 Comments	OK	Two short JSDoc comments on new exports; no ticket refs.
8 Testing	OK	13 new cases in `__tests__`, `toStrictEqual` throughout. 264 tests in the 9 related suites pass at 132a873.
9 General	OK	Fixes 19643, based on master; CI as in FYI.
Performance	OK	4 more requests per poll, stated in the PR; fine on mf's 226 pods.
Accessibility	S1	Color-only split rows.
Error handling	OK	The silent fallback is deliberate and tested.

Pass 2 output: suggested review (our shape)

Body

Tested it on the overview as admin and as a project member: the not-ready pod is counted on its own and goes green on the next poll, no reload. One thing on the Pods card inline.

Inline · composable.ts:253

S1's comment above, with the master and PR card screenshots side by side.

Q1 only if you want to ask it.

Comparing the two passes

	Cody's process	Ours
What drives it	The issue thread, turned into numbered criteria, then seven passes with one question each	A code-conventions checklist plus the rules in memory
Found here	1 comment (S1)	The same S1, plus Q1 and three FYIs
Bugs	None	None
Roles tested	Admin and project member, because the assignee's repro in the thread used a project member	Admin by default; the checklist names no role. The member run here came from pass 1
Evidence	Before and after clips in the review body; screenshot and AX tree on the comment	PR and rebuilt master previews in the bar; screenshots on this page; never the preview link on GitHub
Comment text	About 230 characters, the problem only, no fix	A sentence or two, the fix in words, no code
The simpler-option question	Can't come out: simplify needs the same behavior, and suggestions are stripped	Raised as Q1
Code conventions	Not checked (left to lint and CI)	Checked, all clean here
Already on master	A pass of its own, run last, with a record of what it removed	A rule from memory; same result here
CI	Not part of the review	Checked: Branch Milestone is the expected freeze gate, the 6 h job is unrelated
Cost as designed	About 11 agent runs (2 demos, base review, 7 passes, plus 2 per comment)	One pass

What this one shows

On a small, well-tested PR both processes land on the same point. Neither found a bug.
CodyThe criteria step is what put the project member into the test. Our checklist has no step that reads the issue thread for roles or for what "done" means.
Cody"Already on master" is a step that runs every time, not a rule to remember.
OursQ1 only exists in ours. Cody's pipeline removes suggestions by design, so a trade-off between two valid fixes has nowhere to go.
OursConventions and CI were clean here, so they cost nothing, but nothing in Cody's review would have caught them if they weren't.
That matches the 2026-11-04 comparison: take his acceptance-criteria and pre-existing steps into our prompt, keep our checklist, previews and page.