[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​132	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​131
Prod	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​38	$\color{#cf222e}{\Huge{\mathbf{−}}}$​22	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​16

ELI5

You spun the scroll wheel over the workspace-cleanup filter panel. Your cursor happened to be over a number box you'd clicked into. The browser quietly counted that as "change this number", so a filter you never turned on started hiding 253 of your 799 workspaces — and it got saved. This stops the wheel from being able to do that.

What Changed

• FacetNumberField renders type="text" inputMode="numeric" instead of type="number". All five numeric facets (idle days, min/max size, ahead/behind) share this one field, so one change covers them.
• patchFilters now writes through a functional update against the latest store state instead of closing over the render's snapshot. toggleSortField and clearFilters had the same defect and are fixed with it.

Why

The wheel. Chromium mutates a focused <input type="number"> on every wheel tick. Before #14629 the facet panel could not scroll, so the natural response — put the cursor in the panel and spin — walked activity.idleMinDays up and persisted it. The filter bar did render "Showing 546 of 799" the whole time, so the effect was visible; what was missing was any attribution telling you which filter, or that it came from a previous session.

preventDefault() on a focused number input also stops the value changing, and I rejected it: preventDefault blocks the wheel's default action, which includes scrolling the nearest scrollable ancestor. It would re-break the panel scrolling #14629 just fixed, in exactly the gesture that caused this. A text input can't be wheel-mutated at all and keeps focus and scrolling intact. parseWorkspaceCleanupFacetNumber already parses strings and clamps negatives, so nothing downstream changes.

The patch drop. patchFilters read browse from the render, so two patches in the same tick both started from the same snapshot and the second overwrote the first. Nothing in today's UI writes two groups at once, so this is latent rather than user-visible — but the per-group apply checkboxes that follow write several fields per toggle, and this is the seam they'd land on.

Linked Issue

Reported directly by @BrennanKB5 in-session with a screenshot of the 20-day filter applied on open. No tracking issue was filed; the design work is in docs/workspace-cleanup-filter-apply-plan.md.

Visual Proof

N/A — no rendered change. The input looks and behaves identically for keyboard and mouse entry; only the wheel gesture's effect is removed, and the native spinner arrows go with it (deliberate: they were also a mis-click surface in a dense panel). The measurable behaviour is covered by tests below rather than a screenshot, since "a wheel no longer does something" does not photograph.

Testing

Both new tests were confirmed to fail against the unfixed source before being kept — I reverted each source file, re-ran, saw red, and restored:

• use-workspace-cleanup-browse-state.test.tsx (new, 3 cases): two different groups patched in one tick both land; the same group patched twice in one tick keeps both fields; sort direction flips against latest state. All 3 red before the fix.
• workspace-cleanup-facet-controls.test.tsx: asserts every numeric facet is off type="number" and on inputMode="numeric". Red before the fix.

On the wheel test specifically — I assert the input's type rather than dispatching a wheel event on purpose. The spinner is a Chromium behaviour happy-dom does not implement, so a dispatched wheel changes nothing in the test environment either way and such a test would pass without the fix. The type is the property that makes the class impossible; the comment in the test says so, so nobody later "improves" it into a vacuous one.

Gates run locally on the rebased branch: pnpm typecheck, npx oxlint, pnpm lint:react-doctor:changed, pnpm verify:localization-catalog, and the full workspace-cleanup suite (36 files, 248 tests) — all clean. No strings added or removed.

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

Review

Worth a look at the type="number" → type="text" trade specifically. It drops native min={0} enforcement; typing -5 now yields null (filter off) rather than being blocked at the input, which I judged acceptable since the parser already clamped. If you'd rather keep the spinners, the alternative is a blur-on-wheel guard in the shared Input primitive — that would also cover the 8 other type="number" inputs across settings and SSH forms, which have the identical exposure and are deliberately out of scope here.

Agent skill upstream boundary

• Not applicable.

Notes

• Cross-platform: the wheel behaviour is Chromium's, so the fix applies identically on macOS, Linux, and Windows. No platform branches.
• SSH / remote: filter state is renderer-local and persisted through the existing ui.set path; no wire change, no host involvement.
• Backwards compatibility: no schema or wire change. patchFilters keeps its signature; only its read is now functional.
• Performance: the functional update removes browse from the callback's dependency array, so patchFilters stops re-creating on every browse change.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test pass locally

Author

• X / Twitter: @BrennanKB5

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 25e62d12-ee65-4253-a3aa-55897ceff690

📥 Commits

Reviewing files that changed from the base of the PR and between b89b58b and e7278b7.

📒 Files selected for processing (5)

• src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.test.tsx
• src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.ts
• src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-controls.test.tsx
• src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-controls.tsx
• src/renderer/src/store/slices/workspace-cleanup-browse.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Workspace cleanup browse actions now use functional store updates for filter patches, sort toggles, and filter clearing. The store accepts complete browse state values or updater functions. Tests cover successive updates. Numeric threshold facets now use text inputs with inputMode="numeric" instead of number inputs. Tests verify the input semantics and numeric threshold updates.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes both main fixes: preventing wheel-induced filter changes and preserving every filter patch.
Description check	✅ Passed	The description is detailed and mostly complete, with clear rationale, testing evidence, visual-proof rationale, review notes, and checklist coverage.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

Superseding review complete: PASS / ready for merge after CI.

• Rebased the PR onto current origin/main and tightened the new tests: all five real numeric facet labels must resolve as type="text" with inputMode="numeric", and each hook test now starts from a fresh browse state.
• Review loop: round 1 found only those two test-quality gaps; after the fixes and push, a fresh round 2 returned CLEAN. Production behavior, dropped native min={0}, persistence, concurrency, cross-platform behavior, SSH/folder-workspace use, and performance were all clean.
• Readiness: initial PASS and final PASS, with zero P0/P1/P2 candidates. No mobile-facing surface, wire/schema change, dependency change, security exposure, retry/growth risk, or platform-specific branch is involved. The reference check found the approach conventional and did not call for a different design.
• Local gates all passed: pnpm run typecheck; npx oxlint on the changed renderer/store directories; separate pnpm run audit:code-quality:type-aware; pnpm run lint:react-doctor:changed; pnpm run verify:localization-catalog; and workspace-cleanup Vitest with the required explicit config (29 files, 175 tests).
• Red-test proof repeated during review: restoring the stale hook implementation made all 3 functional-update tests fail; restoring type="number" made the wheel-guard test fail. Both fixes were restored before the gates and push.
• Electron CDP validation confirmed all five thresholds render as text-backed numeric inputs; a focused value of 20 survived a real Chromium wheel event, and the filter viewport remained scrollable while the field stayed focused.

Owner decision: none beyond normal merge timing; no broader product scope was added.

[brennanb2025]

Electron QA — running-app validation

Validated in the dev app launched from this PR's worktree (pr-15298-wheel-guard, throwaway profile, real orca repo registered → fleet of 728 discovered workspaces). All interaction via $electron / Playwright CDP against the app identity pr-15298-wheel-guard @ brennanb2025/cleanup-filter-apply-checkboxes-impl. No rows were deleted.

Core claim: wheel over a focused numeric threshold

Value protection: PASS. The field renders type="text" inputmode="numeric". With the "Idle for at least" input focused (value 20) and the cursor over it, two real wheel events (240px each) changed nothing: DOM value 20 → 20, store filters.activity.idleMinDays 20 → 20. The before/after screenshots are byte-identical (cmp clean).

"…and the Filters panel scrolls instead": FAIL — but pre-existing, not this PR. Wheel events over the Filters popover arrive defaultPrevented: true from the parent dialog's scroll-lock (body[data-scroll-locked="1"]): the popover portals outside the Radix dialog subtree, so react-remove-scroll cancels wheel there, which suppresses both the spinner mutation and ancestor scrolling. Measurements:

• Wheel over the input and over panel background: panel viewport scrollTop 0 → 0.
• Control: the main candidate list (inside the dialog subtree) wheel-scrolls normally (0 → 400).
• Live-flipping the input back to type=number and wheeling: value still unchanged — under the lock even the old bug cannot fire in the current dialog structure.
• Scrollbar drag works (916 → 1544), as does focus-driven scrolling.
• Not a regression here: this branch is rebased on yesterday's main and ui/popover.tsx + workspace-cleanup-filter-bar.tsx are identical to origin/main. The fix(workspace-cleanup): make the Filters panel scrollable #14629 wheel shim only engages for popover-scroll-content popovers, which this panel's PopoverContent doesn't use.

The type="text" change is still correct defense-in-depth — if the popover ever regains native wheel scrolling (e.g. gets the shim class or the lock is sharded), the value stays protected. But "the panel scrolls instead" does not hold in today's app, on this branch or on main. Worth a follow-up: give this popover the popover-scroll-content treatment so the panel wheel-scrolls again.

Check results

#	Check	Verdict
1	Filter bar shows a chip naming the applied filter	N/A — no chip row exists in this branch; the bar shows only a count badge on the Filters button (chips belong to a sibling PR)
2	One removal clears one thing	PASS (via panel; exercises this PR's patchFilters write-through): idle 20d + branch "cleanup" → Showing 11 of 728; clearing the branch field left idle=20 intact in store and input, count back to 728 of 728
3	Clear filters reachable from the bar	N/A — Clear filters exists only in the popover footer and the no-matches empty state; no bar-level control in this branch
4	Filters panel still scrolls	PARTIAL — scrolls via scrollbar drag and focus-scroll; does not wheel-scroll (pre-existing on main, see above). No chip row exists to combine with
5	Nothing pre-selected on open, Delete disabled	PASS — 0 selected, Delete selected [disabled] immediately after open
6	Numeric fields ignore the scroll wheel	PASS on value protection (measured, byte-identical screenshots); the "panel scrolls instead" half fails for the pre-existing reason above
7	Empty state: no chips, no stray Clear	PASS — with no filters: no chips, no Clear control anywhere in the bar

Also verified: Clear filters resets both active groups in one action (idleMinDays → null, branchQuery → "", 728 of 728) — the clearFilters half of the store fix.

Observations (not blocking)

• At superhuman typing speed (no inter-key delay) one keystroke dropped in "Branch contains" (cleanup → ceanup; DOM and store stayed consistent with each other). At 90 ms/key everything lands. Controlled-input re-render race, same code as main; low priority.
• In this fresh-profile rig every row is Git status unknown, so the select-all checkbox stays disabled and I could not tick a row to prove Delete enables; "disabled until ticked" is proven only on the disabled side.
• Light theme and narrow-width wrapping were not exercised in this run.

Evidence

Checks 5 + 7 — pristine open, 0 selected, Delete disabled, no chips/Clear:

Check 2 — two constraints active, Showing 11 of 728:

Check 2 — branch constraint removed, idle survives, Showing 728 of 728:

Check 4 — panel scrolled (viewport at 916px via drag/focus):

Check 6 — after two wheel ticks over the focused field: value still 20, panel unmoved (byte-identical to the before shot):

Clear filters — both groups reset in one action: