Skip to content

The fuzzy selector takes several workspaces for the verbs that can use them - #336

Merged
blooop merged 3 commits into
mainfrom
multi-select-picker
Aug 22, 2026
Merged

The fuzzy selector takes several workspaces for the verbs that can use them#336
blooop merged 3 commits into
mainfrom
multi-select-picker

Conversation

@blooop

@blooop blooop commented Aug 22, 2026

Copy link
Copy Markdown
Owner

The fuzzy selector took one workspace and only ever one. For the verbs that
apply per workspace and return, that made dl rm a five-visit job to clear five
dead workspaces. It now takes as many as you mark, the way fzf --multi and the
vs launcher this borrows from do.

What changed

Multi-select for the verbs that finish on their own — up, stop, rm,
code, dotfiles, and the --rm/--stop flag spellings. TAB marks any number
of rows, Enter applies the verb to each in turn.

Single-select, unchanged, for everything that ends in an interactive session:
attach, -- <command>, restart, recreate, reset. Several of those would be
sessions queued behind each other's exit, which is not what marking five rows
asks for. Verb::several_at_once is an exhaustive match, so a verb added later
has to answer the question rather than inheriting single-select by omission.

A batch does not stop at the first refusal. Every marked workspace is
attempted whatever happened to the ones before it — one rm refused over unsaved
work must not silently drop the other four — and the command's ending is the
first that was not Done, so a script still learns something failed. The
completion-cache latch is re-armed between workspaces, the same reasoning
--autorm re-arms for: each workspace after the first is one more state change
after whatever refresh the last one spawned.

The invitation line names what the picker will take, since it is the only place
TAB is discoverable:

Select workspaces (type to filter, TAB to mark several):

The bug the self-review caught

The first working version degraded to last-toggle-wins: mark two workspaces,
Enter, and only one was acted on — the exact failure the feature exists to
prevent.

skim keys every marked row by (run, get_index()), and the SkimItem trait's
get_index() defaults to 0 "for retro-compatibility". Row never
implemented it, so every row collided on one key and each TAB removed the
previous mark instead of adding to it. Proven by driving the built binary
through a pty against a fake devpod, and by instrumenting a vendored skim
(TOGGLE idx=(1, 0) text="alpha" now=[(1,0)] then
TOGGLE idx=(1, 0) text="bravo" now=[]). skim's own sk -m returns both rows,
which is what placed the defect in the embedding rather than in skim.

Each row now carries its position as its index, and a test pins that the indices
are distinct — the property the accumulation depends on.

Testing

  • Full workspace suite green (1205 core + 93 dl unit + integration), clippy --all-targets -D warnings clean, cargo fmt --check clean, codespell clean.
  • The integration test that pins the selector's stdout now asserts each verb gets
    the right invitation line.
  • End-to-end through a pty against a fake devpod: marking two workspaces issues
    devpod stop alpha and devpod stop bravo, in the order marked.

Worth knowing: the pty harness exercised stop. rm's unsaved-work refusal
mid-batch is covered by unit tests and by reading, not by a live run.

🤖 Generated with Claude Code

Summary by Sourcery

Allow the fuzzy workspace selector to batch self-contained operations while retaining single selection for interactive commands.

New Features:

  • Enable multi-select workspace picking for self-contained verbs, applying the selected operation to every marked workspace in order.
  • Keep interactive-session verbs single-select to avoid queuing multiple sessions.

Bug Fixes:

  • Ensure marked picker rows have distinct indices so multiple selections accumulate correctly.
  • Continue processing a batch after an individual workspace fails while preserving the first failure as the command result.

Enhancements:

  • Re-arm workspace refresh state between batch operations and expose whether the picker accepts one or several workspaces.

Documentation:

  • Document multi-workspace selection behavior and distinguish the picker prompts for single- and multi-select verbs.
  • Add changelog coverage for multi-selection, batch failure handling, and retained single-selection behavior.

Tests:

  • Add coverage for distinct picker row indices and ordered multi-workspace mapping.
  • Update selector integration assertions for the appropriate invitation prompt.

blooop and others added 3 commits August 22, 2026 11:10
…can use them

A verb that applies per workspace and returns — up, stop, rm, code,
dotfiles, and the --rm/--stop spellings — now opens the picker in skim's
multi-select: TAB marks any number of rows and Enter applies the verb to
each in turn, so dl rm clears five dead workspaces in one visit. Every
marked workspace is attempted whatever happened to the ones before it,
and the command's ending is the first that was not Done. The forms that
end in an interactive session (attach, --, restart, recreate, reset)
still take exactly one — several of those would just be sessions queued
behind each other's exit.

The completion-cache latch is re-armed between workspaces, the same
reasoning as --autorm's re-arm: each workspace after the first is one
more state change after whatever refresh the last one spawned.
skim keys every marked row by (run, get_index()), and the SkimItem
trait's get_index() defaults to 0 — so rows that carry no index of
their own all collide on one key, and each TAB after the first removed
the previous mark instead of adding to it. Observed live through a pty:
mark two workspaces, Enter, and only one is acted on; skim's own sk -m
binary returns both because its item type implements the index.

Every row now carries its position among the offers as its index, and
the test pins that the indices are distinct — the property the
accumulation depends on.
several_at_once was a matches! over the batching arms, so a verb added
to Verb later would silently fall to single-select instead of breaking
the build until somebody answers the question. An exhaustive match is
this module's own rule for that obligation. Not testable — the failure
is a compile that should have happened.

@sourcery-ai sourcery-ai Bot 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.

Sorry @blooop, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR teaches the embedded fuzzy selector to return multiple workspaces for verbs that operate per workspace and finish on their own, wires verb-controlled arity through the selection pipeline, and fixes a skim multi-select embedding bug by giving each row a distinct index and mapping multiple selected rows back to workspace IDs in order.

Sequence diagram for multi-workspace selector execution

sequenceDiagram
    participant User
    participant Selector as FuzzySelector
    participant Commands as render_select
    participant Workspace as render_workspace
    participant Refresh as RefreshLatch

    User->>Selector: TAB marks workspaces
    User->>Selector: Enter
    Selector-->>Commands: Pick::Chose(workspace_ids)
    loop Each selected workspace in selection order
        alt Not the first workspace
            Commands->>Refresh: rearm()
        end
        Commands->>Workspace: render_workspace(workspace_id, verb)
        Workspace-->>Commands: Ending
    end
    Commands-->>User: First non-Done ending
Loading

File-Level Changes

Change Details Files
Introduce verb-controlled picker arity (single vs multi-select) and propagate it through the fuzzy selector and command dispatch.
  • Add Verb::several_at_once to explicitly declare which verbs may receive several workspaces at once and document the rationale.
  • Extend Command::Select and render_select to respect verb arity, compute a select::Arity value, and pass it into the picker.
  • Adjust the selector’s invitation line to differentiate single-select vs multi-select modes so TAB discoverability is explicit.
  • Iterate over multiple selected workspace IDs in render_select, rearming completion cache between runs, preserving the first non-Done Ending as the overall command result.
rust/dl/src/cli.rs
rust/dl/src/commands.rs
rust/dl/tests/read_side.rs
Extend the selection module to support multi-selection with skim, maintain ordering, and fix the index collision bug that prevented accumulation of marks.
  • Introduce select::Arity enum to represent single vs multiple row selection and thread it through pick and run_skim.
  • Change Pick::Chose to carry a NonEmpty of workspace IDs instead of a single ID, modeling ordered batches.
  • Implement rows_of helper that builds skim Row items with distinct indices; implement get_index/set_index on Row to avoid default-0 collisions under multi-select.
  • Update run_skim to enable skim multi-select based on Arity and to return all selected rows instead of just the first.
  • Update chosen to map multiple labels back to workspace IDs, drop labels that don’t map, and return Quit when no mapped choices exist.
  • Add tests covering distinct indices per row, multi-row mapping and ordering, behavior with no offers and both arities, and Quit semantics when no rows or unmapped rows are chosen.
rust/dl/src/select.rs
Update user-facing documentation to describe multi-select semantics, affected verbs, batch behavior, and unchanged single-select cases.
  • Document multi-select behavior, affected verbs, and batch semantics in CHANGELOG.md for the Unreleased section.
  • Update README.md to explain which verbs allow multi-select, which remain single-select, and how selector behavior changes when no workspace is named.
  • Revise CLI help text in cli.rs to describe interactive selection in terms of single vs multi-select and to mention TAB marking for applicable verbs.
CHANGELOG.md
README.md
rust/dl/src/cli.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@blooop blooop mentioned this pull request Aug 22, 2026
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.80645% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.01%. Comparing base (207c238) to head (358c082).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
rust/dl/src/commands.rs 33.33% 18 Missing ⚠️
rust/dl/src/select.rs 86.95% 12 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
python 42.98% <ø> (ø)
rust 95.38% <75.80%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
shipped code (rust) 95.38% <75.80%> (-0.02%) ⬇️
harness and tooling (python) 42.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@blooop
blooop merged commit 9125cf0 into main Aug 22, 2026
14 checks passed
@blooop
blooop deleted the multi-select-picker branch August 22, 2026 10:47

@blooop blooop left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This was generated by AI during review.

Retrospective wf-review. #336, #350, #363 and #370 were reviewed together as one surface — the full two-axis report is on #370: #370 (review) — and every finding there was verified against current main (a5f7ed8), not against the intermediate state this PR left behind.

Specific to #336:

  • Nothing broken in the multi-select itself. Verb::several_at_once is an exhaustive match, Arity::One sets multi: false so TAB is inert rather than misleading, and the get_index fix is genuinely pinned (removing the impl turns the test red). I tried hard to break the batch's first-failure latch with the Ending::Child(Exit::Code(0)) case — a success that is not Ending::Done would latch and swallow every later failure — but DevpodRefused is only constructed behind !exit.is_success() in both workspace_stop and workspace_delete, and up/code/dotfiles return Ending::Done via Launched::Ready. The contract holds. It holds by coincidence of the other arms though; if matches!(ending, Ending::Done) would be sturdier as "latch the first ending whose code() is non-zero".
  • P1 (non-blocking) — CHANGELOG.md:535-537, shipped in [0.6.0] and still on main, is wrong. The entry says multi-select applies to the five verbs "(and the --rm/--stop spellings)". Verified by driving cli::resolve on main: dl --rm resolves to Select { Attach { rm: Yes } } with several_at_once = false (single-select — and cli.rs:171 says so outright: "This is --rm, and Verb::Remove is rm — docker's split"), and dl --stop is Err(RetiredFlag(Stop)), which never opens a picker. README gets it right; drop the parenthesis from the CHANGELOG.
  • Non-blockingrefresh.rearm() runs once per workspace, so a batch of ten spawns up to ten detached dl --update-cache children, nine of them describing a world the batch is still changing. No corruption risk (write_atomically stages through a pid-unique temp name), but re-arming once after the loop is what the entry's own reasoning asks for.

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