Whether two picker rows collide cannot depend on a third row's width

[blooop]

Found by a retrospective wf-review of #336/#350/#363/#370 — four iterations on the picker landed inside three hours, none of them reviewed (Sourcery was out of quota for all four).

The defect

ec5c882 added a guard for the cross-shape collision: a two-column whole-name row <owner> | <name> can be drawn identically to a three-column split row <owner> | <repo> | <ref> when the name is those last two columns. chosen maps a label back to a workspace, and dl rm is one of the verbs this picker opens for, so two rows drawn alike is one workspace deleted in place of another.

The guard asked all_distinct of the padded labels. Padding is a fact about the widest row in the list. So a third workspace — nothing to do with either of the two — whose repo is one character wider than devlaunch widens the repo column, the split row gains one trailing space the whole-name row does not, the two labels stop being equal as strings, and the guard, which is looking for equal strings, does not fire.

Reproduced on main as a failing test before any fix:

["blooop | devlaunch  | main", "blooop | devlaunch | main", "blooop | wayfinderx | main"]
                    ^^                      ^

Two rows one space apart. Nobody can tell those apart on a terminal, and the row you pick is the workspace that gets deleted.

There were two notions of "same row" in the file and neither was the one that matters: shared_key ignored padding but was blind across shapes, and all_distinct saw across shapes but counted padding.

The fix

shared_key becomes the row's own unpadded label — label(naming, 0, 0). Asking label rather than restating its format is the point: the separator, the column count and which fields are drawn are decided in one place, so a change to how a row is drawn cannot leave the key describing the old drawing. It also retires the NUL delimiter, since | is the delimiter and it is the one on screen.

That makes the existing key pass in named() see every shape of collision, which has a second benefit: it de-splits only the rows that collided. The old fallback put every split row in the listing back to its id over one ambiguous pair — which contradicted README.md:211, promising that "both rows go back to their full ids", and the principle a_collision_only_pulls_in_the_rows_that_collide already pins for the same-shape case. This brings the code to the doc rather than the other way round.

offered()'s whole-list fallback and all_distinct are deleted. Not on taste — measured: with the key fixed, stubbing the fallback to if false && … leaves all 23 select:: tests green. It cannot fire, and an unreachable safety net that no test can pin is worse than no net.

Verified

• Red before green. a_wider_third_row_cannot_hide_the_cross_shape_collision fails on main with the output above and passes after. It asserts on the labels with runs of whitespace squashed, deliberately — the raw strings do differ, by exactly the difference a person cannot see, so assert_ne! on them would pass while the bug stands.
• Scoping pinned. a_cross_shape_collision_also_only_pulls_in_the_rows_that_collide — the collided pair goes back to ids, a third row of the same repository keeps its columns.
• The fallback is dead. Measured as above before deleting it.
• cargo test -p dl 114 lib + every test binary green, cargo clippy --locked --all-targets -- -D warnings clean, cargo fmt --check clean.

Not in this PR

The same review confirmed several non-blocking items I have left for separate changes: no column budget (one long repo name pushes the branch column off-screen), widest counting scalar values while its doc claims terminal cells, the picker pty harness having no three-column row in it at all, and a CHANGELOG.md:535 claim that --stop opens a multi-select picker when it is a retirement refusal. Full report on #370.

🤖 Generated with Claude Code

Summary by Sourcery

Make picker collision handling depend only on the rows being compared and align disambiguation with their visible labels.

Bug Fixes:

• Fix picker row collision detection so visually identical whole-name and split rows remain distinguishable regardless of unrelated rows’ widths.
• Limit fallback to expanding only the rows involved in a collision, preserving compact labels for unaffected rows.

Enhancements:

• Derive collision keys from each row’s unpadded displayed label so collision detection matches the actual rendered text.

Tests:

• Add regression coverage for cross-shape collisions hidden by a wider third row and for collision handling scoped to the affected rows.

Chores:

• Remove the redundant whole-list padded-label collision fallback and its associated distinctness helper.

[sourcery-ai]

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]

Reviewer's Guide

Fix picker row collision detection by using each row’s canonical unpadded display label as its shared key, allowing named() to catch cross-shape collisions and de-split only affected rows; remove the unreachable list-wide padded-label fallback and add focused regression tests.

Flow diagram for picker collision detection

flowchart TD
    A["Workspace rows"] --> B["named(workspaces, cache_dir)"]
    B --> C["shared_key(naming)"]
    C --> D["label(naming, 0, 0)"]
    D --> E{"Canonical labels collide?"}
    E -->|No| F["Keep row's split or whole-name display"]
    E -->|Yes| G["De-split only colliding rows"]
    G --> H["drawn(namings)"]
    F --> H
    H --> I["offered labels used by chosen"]

Loading

File-Level Changes

Change	Details	Files
Make collision detection depend on each row’s unpadded rendered text rather than field boundaries or list-wide padding.	Derive shared keys by calling the row-label formatter with zero padding. Detect collisions across whole-name and split-row shapes while ignoring alignment whitespace. Remove the NUL-delimited field-based key construction and obsolete all-distinct fallback.	rust/dl/src/select.rs
Scope de-splitting to only the rows involved in a rendering collision.	Retain the existing collision handling in named() as the sole fallback mechanism. Stop reverting every split row when any cross-shape collision exists. Preserve column formatting for unrelated rows.	rust/dl/src/select.rs
Add regression coverage for padding-independent cross-shape collisions and collision scoping.	Add a test where a wider unrelated repository previously hid a collision. Add a test confirming only the whole-name/split pair is expanded back to ids.	rust/dl/src/select.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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.20%. Comparing base (a5f7ed8) to head (c051084).

Additional details and impacted files

Flag	Coverage Δ
python	42.98% <ø> (ø)
rust	95.56% <100.00%> (+<0.01%)	⬆️

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

Components	Coverage Δ
shipped code (rust)	95.56% <100.00%> (+<0.01%)	⬆️
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.