Skip to content

Post-launch cleanup: review-queue lanes, bots, snooze, mobile#486

Open
jarstelfox wants to merge 10 commits into
masterfrom
review-queue-fixes
Open

Post-launch cleanup: review-queue lanes, bots, snooze, mobile#486
jarstelfox wants to merge 10 commits into
masterfrom
review-queue-fixes

Conversation

@jarstelfox

@jarstelfox jarstelfox commented Jul 25, 2026

Copy link
Copy Markdown
Member

Post-launch cleanup for the v2 board.

The rewrite has been in prod for a while, and living in it day to day turned up a batch of rough edges. Almost none of these are new features. They are places where the board was technically working but doing the wrong thing for how we actually review. Grouped by the problem each one solves, since the diff already shows the how.

The board was hiding work it exists to surface

  • Starved PRs got buried. With a repo priority set, the Starving section pulled starved PRs out of the queue into a capped fold. A high-volume repo (ops) filling that fold pushed a starved PR from a quieter repo (ifixit) behind "show more," and because it had been removed from its own repo block, it never came back. The fairness backstop was doing the opposite of its job. Starving and code regions are non-exclusive highlights now: the card stays in the queue and the section is a copy, the same relationship a claimed PR and "Recently updated" already have with their lanes.
  • Recently updated was an activity feed, not a review surface. It listed anything that moved since you last cleared, including PRs you had already stamped and bumps from over a week ago. That pulls against the whole point of the board (what is my move, not what happened). It is bounded now: recent, and only work you might still review.

Bot activity was leaking into human-review signals

Bots author a lot of our PRs and claude[bot] reviews most of them, so the board reading that activity as human work is a steady low-grade wrongness.

  • A bot's comment or review read as "changes requested" or a re-review you owed, and once auto-unsnooze landed it would wake a PR you had snoozed. Bot activity stays out of the nudge and the snooze now. The server sends a human-only comment count, since the client can't tell a bot's comment from a human's in a bare comment_count.
  • Hiding bots also stripped them from the CI lens and Ready-to-merge, where you still want a bot's red build and its one-press-from-done PR. Hiding now empties the queue and the bot fold but keeps CI and Ready, and a saved filter, author:, or is:bot brings a named bot back.
  • Our own machine accounts (ifixit-systems, Copilot) carry no [bot] suffix, so nothing recognized them as bots. They are in the config now.

Small things that only real use surfaces

  • The snooze tooltip already promised "until it changes," but comments and reviews arrive on a path that never moves updated_at, so they never woke a snooze. They do now.
  • On a phone, tapping a PR title both opened the PR and, a beat later, popped its hover preview on the board you had left. The hover-arm skips touch now (mouse and pen keep it).
  • The "shipped while you were away" toast threw the titles away and showed a bare count; it names the PRs now, and it plus the notifications stopped truncating.
  • Ten to fifteen pinned filters wrapped the whole filter bar and shoved the controls around. They got their own scrollable row.

QA

  • tsc, 446 vitest, vite build; 61 backend tests after build:shared; root eslint ./ reports 0 errors.
  • Reviewed by a Fable-model pass over the full git diff master...HEAD; every finding addressed.
  • Not yet eyeballed on a real device (the in-session browser renderer was wedged). The mobile popover and the bot behaviors are the ones to poke.

Deploy notes

  • No DB changes.
  • human_comment_count is an additive wire field; an older client ignores it and the snooze falls back to comment_count.
  • Prod and dev config.js both need ifixit-systems + Copilot in their bots list. Both are done (see the deploy comment below).

🤖 Generated with Claude Code

jarstelfox and others added 10 commits July 25, 2026 12:44
Reviewers deliberately pin many saved searches (every personal roster auto-pins one, plus hand-saved filters), which overflowed the single filter row: the pins wrapped to two or three lines and shoved the dimension triggers and search box down with them. That is the badge-soup shape PRODUCT.md warns against, moved into the header.

Move the pins to their own dedicated line, a horizontally-scrollable strip reusing the lens tab strip's overflow-x-auto no-scrollbar pattern. Chips are shrink-0 so a long set scrolls instead of compressing, every pin stays one click away, and a growing set can never wrap the triggers again. The divider follows whichever row leads so the header keeps one separator.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With a repo priority set, starved pulls were extracted into a Starving fold and removed from their repo block, and code-region matches were pulled out of the queue and QA lanes. A high-volume repo (ops) filling the Starving fold's cap could bury a starved PR from another repo (ifixit) behind 'show more' with no way back onto the board, since it no longer appeared in its own block either.

Make both non-exclusive: the Review queue keeps everything, and 'Starving' and 'In your code regions' show a copy of a slice rather than removing it. This matches how 'Recently updated' and a claimed PR already relate to their lanes. The four state lanes (Ready, Waiting on you/others, Needs QA) stay mutually exclusive, so position still encodes state where it matters.

Also fixes a pull you already hold a live CR stamp on showing in 'your code regions': the QA-pool leg feeding regionMatches never checked crBy, so a needs_qa pull you had already reviewed sailed in. A needs_recr pull whose stamp is only yours-and-stale still shows, since that stamp has dropped out of the live crBy set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recently updated was an unbounded activity feed: it listed anything that moved since you last hit Clear, including PRs you had already CR'd and bumps from over a week ago when you had not looked in a while. That cuts against the board's actionability-first ethos and was the noisiest lane.

Bound it two ways and narrow it: within the last 3 days (not only since Clear), drop PRs you hold a live CR stamp on (finished for you) and non-reviewable ones (others' drafts, dev-blocked), and keep your own PRs since activity on them is worth seeing. It now reads as what changed on your review work since last look. The lane still lives in the component; moving it into buildReviewLanes for unit coverage is a fair follow-up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When more than one PR landed while you were away, the shipped catch-up toast threw the titles away and collapsed to a bare count ('2 yours · 1 you reviewed'). It now lists the top 3 as links with a '+N more' line, so the catch-up names real work. The board's Recently-closed fold already did this; only the toast lagged.

Also stops the toast and bell-panel text from truncating (DESIGN.md: nothing truncates except the wait-word door; density comes from geometry, not from hiding text), so cards grow to fit the full title and body.

And fixes the bell panel dropping the leading number on the two count-bearing nudges (milestone 'reviews this sitting', top-of-board 'stamps in view'): it rendered the title without the count the live toast shows via its Odometer, leaving a number-less fragment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On a phone, tapping a PR title both opened the PR and, about 250ms later, popped its description preview uninvited. The title is a real anchor, so a tap navigates; the tap also synthesizes a mouseenter that armed the hover-open timer, and nothing cancelled it because the title's trigger deliberately drops its click handler (a click on the title means open the PR). The avatar (tap filters) and the fold-gloss eyebrow share the same defect class without the navigation.

Switch usePopover's hover handlers from mouse events to pointer events and only arm the preview when pointerType is 'mouse'. Touch taps no longer arm a hover-preview, so those elements do their primary action cleanly, while desktop hover is unchanged and every button-triggered door still opens on tap through the click toggle. This is 'touch has no hover' applied in JS to match the CSS the board already uses, and it reads pointerType per interaction so a hybrid mouse-and-touch device behaves correctly either way.

The title's description preview is now desktop-only by design; on mobile the title simply navigates.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The snooze tooltip promised 'off your Review tab until tomorrow or until it changes,' and the code voided a snooze once updated_at moved. But a new comment or review does not move updated_at: those arrive on their own webhook path that re-derives the pull without touching it. So commenting on or reviewing a snoozed PR never woke it, and the promise was half true.

The snooze record now captures the comment and review counts at snooze time, and isSnoozed wakes when either climbs, alongside the existing updated_at check. Counts only, since the wire carries no per-event author, so your own comment wakes it too, the same way your own push already does. Clean cut: a pre-baseline snooze entry (a bare epoch number) is dropped on load, so the pull reappears once and you re-snooze it if you still want to.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
These are machine accounts GitHub does not tag with a [bot] suffix, so isBotLogin's suffix check missed them and the board treated them as human authors. ifixit-systems is our dependency-bumper and AI-agent account (dozens of PRs across fixbot, ifixit, and DocHarvestor); Copilot is GitHub's coding agent (user.type Bot, but no suffix). Listing them in config gives them bot treatment: folded or skipped, and out of the reviewer leaderboard and rotation. claude[bot] and the other suffixed apps were already detected.

This updates the example template only. The deployed config.js on the host needs the same two entries added to its bots list to take effect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A bot's comment or review, the auto claude[bot] review or a CI bot, shouldn't drive the human-review nudge or wake a PR you snoozed. claude[bot] only posts COMMENTED reviews, so the status bucket was already safe, but two paths still leaked bot activity into what a human reads: the changes-requested nudge copy, and the auto-unsnooze added earlier on this branch.

Client: changesRequestedBy (shared derive) and lastChangesRequestedAt now filter bot reviewers, matching the isSuffixBot filter engagedNoStamp and the 'Answer the review' branch already use, so a bot's changes-requested can't demand a re-review or skew whether the author answered a human. The snooze-wake now excludes bot-authored reviews (allCR/allQA/unstamped_reviewers filtered by isBotLogin, so config bots count too).

Server: a bot's plain COMMENT can't be told apart on the client (comment_count is a bare aggregate), so the server now sends human_comment_count with bot authors excluded (reusing review-model's config-aware isBot), and the snooze prefers it, falling back to comment_count for an older server.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'Ignore bot PRs' setting hid bots at one global chokepoint, so turning it on also stripped them from the CI lens and Ready-to-merge, and no filter could bring a specific bot back. When you hide bots you still want their fleet CI health, and a green signed-off bot PR that's one merge press from done.

Split boardHidden into every-hide-except-bots plus a reveal-aware bot rule (hideBots && isBot && !revealedAuthor && !is:bot token), so a bot named by the active saved/pinned filter, author:, or is:bot still shows. A bots-bypassing pool that still respects every other hide feeds only the CI lens and Ready-to-merge's bot portion; the queue tail and the bot fold stay emptied when hideBots is on. Snoozing a bot straight off Ready still parks it in 'Snoozed by you' so it never vanishes without an undo.

Adds an is:bot query token (and appends it to the filter-box hint) so the escape hatch is discoverable like every other is: token.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A review of the branch surfaced several small gaps, fixed here:

- Ready-to-merge's empty check ignored botsForReady, so a merge-ready bot could hide behind 'All clear' once hide-bots emptied the queue.
- is:bot matched only the [bot] suffix, so with hide-bots on it revealed suffix-bots but not config bots (ifixit-systems, Copilot), and even narrowed CI/Ready; matchesQuery now takes the extra-bots set.
- The popover hover gate excluded a stylus; it now excludes only touch, so mouse and pen keep hover.
- The snooze wake summed CR+QA+unstamped into one count, so a reviewer going COMMENTED to APPROVED (unstamped down, CR up) washed out and left the snooze asleep; the three counts are now tracked separately, and the load guard drops half-shaped records.
- Recently updated's sub-line no longer overpromises (it names the 3-day bound).

Plus comment accuracy (the changes-requested filter stays suffix-only per the shared-layer convention; the hidden-ledger note calls out the CI/Ready exception) and a toast singular-format nit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jarstelfox
jarstelfox marked this pull request as ready for review July 26, 2026 00:28
@jarstelfox

Copy link
Copy Markdown
Member Author

Deploy record (2026-07-25) 🤖

Deployed straight from this branch to both instances; this PR is the review trail. No DB changes.

instance image status
dev pulldasher-dev.cominor.com pulldasher-dev:10249968 live
prod pulldasher.cominor.com pulldasher:latest (built from 1024996) live, 598 open pulls loaded

Both config.js files got ifixit-systems + Copilot added to bots. Smoke on both: / 302, /api/v1/me 401, no errors in the logs.

Rollback (seconds, nothing to unwind):

# prod
docker rm -f pulldasher && docker run -d --name pulldasher --restart=always \
  --publish 8090:8090 --env-file ~/pulldasher-prod/.env.db pulldasher:pre-1024996-20260725
# dev
docker rm -f pulldasher-dev && docker run -d --name pulldasher-dev --restart=always \
  --publish 51515:8090 --env-file ~/pulldasher/.env.db pulldasher-dev:ac72d931

Note

Both deploy checkouts (~/pulldasher-prod, ~/pulldasher) are pinned to this branch right now instead of master. Merging this restores them to master-tracking on the next routine deploy.

@jarstelfox jarstelfox changed the title Review-queue polish: non-exclusive lanes, bot handling, snooze, mobile popovers Post-launch cleanup: review-queue lanes, bots, snooze, mobile Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant