MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-128

2.5.17 Bug — filter-bar facet check marks de-sync on rapid multi-select (finding #58)

Done
Description

Estimate: 6m · Depends on: 2.5.4

Resolution (PR #116) — corrected root cause. The real bug was not the rapid-multi-select stale-closure race described below — that theory was wrong (a faithful round-trip test passed on main). Filter selection was not optimistic: the check marks / facet counts / trigger badge rendered straight off the server-round-tripped filter prop, so a clicked status showed no check until the router.push → Server re-read settled — and a status matching nothing (e.g. "Blocked") made the empty read make it look permanently broken.

Fix: mirror the filter into local optimistic state that updates the instant a facet is toggled; render every check mark + count from it; reconcile to the prop on identity change (navigation landed / external reset), guarded like the urlText sync. A synchronous filterRef stays the compose source so back-to-back toggles still stack (the original race concern, kept covered). Regression test: click a status → aria-selected + badge update with NO round-trip (fails on main). The stale-closure framing in the sections below is retained as the audit trail of the mis-diagnosis.

Regression introduced by 2.5.4 (logged as finding #58). In the /issues filter popover, checking two or more Status values in quick succession silently reverts the first one's check mark — the visible ticks, the active-count badge, and the actually-filtered tree disagree. It's intermittent ("sometimes"): toggling slowly always works. The same latent race lives on the Kind and Assignee facets.

Root cause — stale-closure clobber. IssueFilterBar is a Client Component whose filter is a prop derived from the URL: each selection round-trips through router.push → Server-Component re-read → new filter prop. The facet OptionRow toggles compute next-state from the render-time filter closure (apply(toggleStatus(filter, s.key))). A second click that lands before the first navigation settles still sees the old filter, so its push drops the in-flight first selection. The text quick-filter already solved exactly this — it threads the latest filter through an effect-synced filterRef.current (the "must not be clobbered" comment) — but the KIND/STATUS/ASSIGNEE toggles were never migrated to that ref.

Fix: route every facet toggle through filterRef.current (e.g. apply(toggleStatus(filterRef.current, s.key)) for kind / status / assignee / unassigned), so each push composes onto the freshest filter regardless of in-flight navigations. Pure reducers in lib/issues/issueListFilter.ts are correct and unchanged — the bug is in the caller only.

Acceptance criteria

  • Toggling two+ statuses (or kinds, or assignees) in quick succession — before navigation flushes — keeps EVERY selected row's check mark; the pushed URL carries all selected keys; the active-count badge matches the visible ticks.
  • All four facet toggles (kind · status · assignee · unassigned) read filterRef.current, not the render-time filter closure; the pure reducers in lib/issues/issueListFilter.ts stay untouched.
  • Regression test in tests/components/issue-filter-bar.test.tsx: fire two status toggles synchronously (no await between clicks, navigation not yet flushed) → assert the pushed href contains BOTH status keys and both rows render aria-selected="true" (the current code drops the first).
  • No behavioural change to slow/single toggles, Clear, or the text debounce; tsc / eslint / prettier clean.

Context refs

  • Finding #58 in PRODECT_FINDINGS.md (full root-cause writeup)
  • app/(authed)/issues/_components/IssueFilterBar.tsx — the OptionRow onToggle handlers (KIND/STATUS/ASSIGNEE) + the existing filterRef / text-debounce precedent (~L143–167) to mirror
  • lib/issues/issueListFilter.ts (toggleKind/toggleStatus/toggleAssignee/toggleUnassigned — pure, reused as-is); tests/components/issue-filter-bar.test.tsx (where the regression test lands)
Status
Done
Type
Bug