Skip to content

fix: keep the picker's highlighted row across refreshes - #80

Merged
idmitriev merged 1 commit into
mainfrom
fix/picker-refresh-keeps-selection
Oct 17, 2026
Merged

idmitriev merged 1 commit into
mainfrom
fix/picker-refresh-keeps-selection

Conversation

@idmitriev

Copy link
Copy Markdown
Owner

Problem

Seen live in v0.10.0. Type "pr" with a calendar event a few minutes away: "Pairing session" (with a Zoom link) ranks first, then "Search Pull Requests". Press ↓ and wait a moment, and the highlight jumps back to the top row. Return then opens the meeting's Zoom link, and on a Mac without Zoom that's a "No application set to open the zoommtg URL" alert.

Cause

The countdown isn't the cause. The calendar row's "In 4 min" badge is computed only when the row renders, and no calendar timer exists. The event matters only because imminence ranking pins it to the top row.

The picker kept the selection only when a list update was identical: same IDs in the same order. Several things re-rank the list for the same text while the picker is open:

  • a moduleActionsChanged dynamic refresh: a Notes listing finishing about a second after open, a timer firing, or a theme change;
  • refreshInPlace after a hotkey re-show;
  • an initial load landing after a fast keystroke run.

ModuleRegistry.fanOut collects module results in completion order, so tied rows reorder on almost every refetch. The list was almost never "identical", and the highlight snapped back to the first row.

Fix

PickerState now records listQuery, the search text the rows on screen were ranked for. updateActions(_:cache:rankedFor:) applies the rule:

  • Same text (a refresh): the highlighted row stays highlighted wherever it moved, matched by action ID. If it's gone, the row now at its index takes over, clamped to the last row. The list doesn't jump to the top.
  • New text, pushed rows, or the first ranking after the cached list shown at open: start on the first row. The cached list is the last session's, and this session's ranking may put a meeting that's about to start first.
  • When a refresh keeps the selected row but moves it, a new selectionRevealToken makes ActionListView scroll it back into view. Its ID doesn't change, so the selection onChange can't do this.
  • CoveredList saves and restores listQuery, so a list restored from under a message keeps refreshing in place.
  • The unused QueryOptions.preservingSelection is removed; every preset left it at its default.

Parameter options are unchanged: they only reload when the parameter query changes, which should reset the selection.

Tests

  • PickerSelectionTests covers the rule on PickerState: a reorder keeps the row and asks for a reveal, a dropped row falls back to its neighbour, new text resets, the first ranking after the cached list resets, and a list restored from under a message still refreshes.
  • PickerCoordinatorTests+Refresh rebuilds the report end to end. A ReshufflingCatalogModule offers an event four minutes away with a Zoom link and two rows that swap on every fetch. ↓ and then refreshActions() keep the highlight on Search Pull Requests; new text afterwards starts on the first row. Before the fix, this test failed with the selection on event.pairing.
  • make test: 591 tests, 0 failures. make lint: 0 violations.

Not reproduced live: that needs a real calendar event with a Zoom link.

Follow-up, handled separately: rank tied rows in a deterministic order, so refreshes stop visibly shuffling them.

🤖 Generated with Claude Code

A refresh re-ranks the list for the same text: a module reporting new
actions, the picker re-shown with its state, or an initial load landing
after a keystroke run. The selection survived only an identical list,
but the catalog fan-out collects module results in completion order, so
tied rows reorder on almost every refetch and the highlight snapped back
to the top row. With an imminent calendar event ranked first, Return
then opened its Zoom link instead of the row the user had picked.

PickerState now remembers the text the rows on screen were ranked for.
A list ranked for the same text keeps the highlighted row by ID; if
that row is gone, the row now in its place takes over. New text, the
first ranking after the cached list shown at open, and pushed rows
start on the first row. When a kept row moves, the list scrolls it
back into view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 14:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The selection lifecycle is implemented consistently and covered by focused state and coordinator regression tests.

0 open findings

What changed in this PR

Preserves picker selection across same-query refreshes while resetting it for new rankings.

Changes:

  • Tracks the query associated with displayed results.
  • Preserves and reveals moved selections during refreshes.
  • Adds state-level and coordinator regression tests.
File Description
VibeshedTests/​PickerSelectionTests.swift Tests selection stability rules.
VibeshedTests/​PickerCoordinatorTests+Refresh.swift Adds end-to-end refresh regression coverage.
VibeshedTests/​PickerCoordinatorTests.swift Exposes shared async test helper.
Vibeshed/​Picker/​PickerView.swift Passes the reveal token to the list.
Vibeshed/​Picker/​PickerState.swift Implements query-aware selection preservation.
Vibeshed/​Picker/​PickerCoordinator+Support.swift Removes obsolete selection configuration.
Vibeshed/​Picker/​PickerCoordinator.swift Associates ranked results with their query.
Vibeshed/​Picker/​ActionListView.swift Reveals selections moved by refreshes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@idmitriev
idmitriev merged commit a395391 into main Oct 17, 2026
2 checks passed
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.

2 participants