Skip to content

fix(grid): shift selection indices on step insert/remove - #146

Merged
mrcsin merged 1 commit into
masterfrom
selection-index-shift
Jul 22, 2026
Merged

fix(grid): shift selection indices on step insert/remove#146
mrcsin merged 1 commit into
masterfrom
selection-index-shift

Conversation

@mrcsin

@mrcsin mrcsin commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

The grid caches the current selection as a list of step indices
(SelectedStepIndices). When a step was inserted or removed at or before a
selected step, the control's own selection model shifted to keep the same
items selected, but the cached indices were left stale. Consumers that read
the cache — copy, cut, delete, and the paste insert position — then operated
on the wrong steps.

The bug is a correctness/data-integrity defect. It is largely latent in the
current single-window UI, because every consumer command reselects after
mutating and undo/redo clears the selection, so no clickable gesture leaves a
stale cache for a later read. It becomes reachable the moment an "insert
above" command, a second window on the same recipe, or a consumer without a
follow-up reselect is added. The fix makes the invariant hold unconditionally.

What changed

RecipeGridSurfaceBase.OnMutation now maintains the cache itself instead of
relying on a control-level signal:

  • A pure ShiftSelection(selection, signal) shifts the cached indices by the
    mutation signal — insert shifts survivors up, remove drops the removed set
    and shifts survivors down, a full rebuild clears the selection.
  • The selection is snapshotted at the top of OnMutation and re-applied after
    the collection mutates, guarded by a sequence-equality check, so a
    mid-mutation push from the transposed list cannot double-shift.
  • The shift is scoped to the four index-affecting signals; identity signals
    (including StepActionChanged, whose item reference is swapped in place)
    deliberately leave the cache untouched, so the existing reselect path governs
    them.
  • The old drop-only ReconcileSelectionWithItems clamp is removed.

This works for both grids, including the canonical DataGrid, which exposes no
index-shift signal, and for the orientation-flipped surface whose view is
detached.

How it was verified

  • 22 targeted SelectionShift tests across both grids: insert/remove before,
    after, and at the selection; multi-select block shifts; multi-insert +count
    and the multi-remove survivor offset; RecipeReplaced clears; ChangeStepAction
    non-clobber; and a consumer end-to-end test that deletes through the shifted
    cache and asserts the originally selected step is the one removed, by content.
  • One transposed live-control test asserts the cache equals the control's own
    selection indices after a remove that pushes mid-mutation.
  • Full suite green: 1466 passed, 0 failed.
  • dotnet format --verify-no-changes clean; analyser clean on the changed file.

No clickable manual repro exists in the shipped single-window UI (see Problem);
the automated tests are the evidence.

Per-task commits before collapse
2585a8e docs(plans): archive stale-selection-shift plan
3095118 docs(plans): record exec branch and progress file
eedd215 test: cover multi-insert and multi-remove offset arithmetic
52a382c docs: note OnMutation selection-shift reconciliation
c33d74f test: verify acceptance criteria for selection shift fix
6d0eaa1 test: cover selection shift both grids, consumers, action-change
483e84d fix: shift cached selection indices on index-changing mutations
948d9ce test: red tests for stale selection index shift
cf0f9fa docs(plans): plan stale-selection-index shift fix

🤖 Generated with Claude Code

The grid caches the current selection as a list of step indices. When a
step was inserted or removed at or before a selected step, the control's
own selection model shifted to keep the same items selected, but the
cached indices were left untouched and went stale. Consumers that read
the cache (copy, cut, delete, paste insert position) then acted on the
wrong steps.

Maintain the cache in the shared surface instead of relying on a control
signal. OnMutation already runs on every mutation and carries the signal
payload, so shift the cached indices arithmetically there: insert shifts
survivors up, remove drops the removed set and shifts survivors down, and
a full rebuild clears the selection. This holds for both the canonical
DataGrid, which exposes no index-shift signal, and the transposed list,
and it keeps the invariant even for the orientation-flipped surface whose
view is detached.

The selection is snapshotted at the top of OnMutation and re-applied
after the collection mutates, guarded by a sequence-equality check, so a
mid-mutation push from the transposed list does not double-shift. The
drop-only clamp it replaces is removed.
@mrcsin
mrcsin marked this pull request as ready for review July 22, 2026 14:15
@mrcsin
mrcsin merged commit deee4ad into master Jul 22, 2026
1 check passed
@mrcsin
mrcsin deleted the selection-index-shift branch July 22, 2026 14:16
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