ARCADIA-Studio/harness/multiselect-desktop-review-20260923.md

Independent ordinary saved-view multi-selection review — 2026-09-23

Read-only review of the in-progress ordinary saved-view adapter against multiselect-contract-20260923.md. The owning session was actively wiring UI during inspection; references describe the inspected code and must be rechecked after fixes. No production edits, Cargo, GUI, or runtime/test acceptance claims. Missing State, Allocation, derived Process/Scenario and aggregate model-delete adapters are explicitly outside this review, not findings below.

Reviewed primary files: crates/oa-desktop/src/app/multiselect.rs, diagram/multiselect.rs, diagram/group_gesture.rs, diagram_selection.rs, plus relevant app.rs, diagram.rs, diagram/focus.rs, views.rs and menu paths.

Verified findings

P2 — surviving selection is discarded when legacy sync removes the primary

app/multiselect.rs:78–97 decides whether the group held the primary by comparing the old selection against post-sync selected_node/selected. But sync_with_alias_editor clears nonexistent semantic selection and calls refresh_draft, which recomputes/clears the placement (app.rs:800–810, :829–853). Consequently the reducer's correct survivor fallback is overwritten.

Concrete path: select placement A and B, with B primary; undo the model operation that created B, or remove B's placement through a singleton-target action. sync first makes selected_node = None (and may select the project). At final reconciliation, held_primary is false. Selection::reconcile prunes B and chooses A, but legacy_target is None, so line 95 clears A too. The same problem occurs when an external accepted model/source change removes the primary while other exact selected placements remain.

Preserve provenance of the pre-change primary across sync, and distinguish genuine explicit navigation from model/history reconciliation. The latter should prune only invalid members and inspect its surviving fallback. Do not recover by mapping an old semantic UUID to an arbitrary replacement placement. Regression: select A/B, undo B creation; expect A selected and inspected, no additional history. Repeat for placement deletion and semantic deletion, plus deletion of a non-primary member and ordinary group-move Undo/Redo.

P2 — mixed selection cannot collapse through the specified stationary click

diagram/multiselect.rs:83–97 tests whole-set movement capability on pointer press. For a legal selected set containing placement A and edge E (or a port), a plain press on either member immediately publishes Refuse, stores no click gesture, and captures the press. A stationary release therefore cannot execute the contract's "select that member alone" behavior. This is a selection defect inside ordinary saved views, not a request to implement mixed-target movement.

Track a deferred click for every selected exact target, independently of movement capability. On a stationary release replace with that target; only crossing the drag threshold should refuse unsupported movement while retaining the set. Regression: A+E, click A, expect exactly A and no model/history mutation; recreate A+E and drag A, expect refusal and both retained. Repeat with a port and with pointer displacement that never exceeds the threshold.

P2 — a left press can start group movement while middle-button pan owns the pointer

The new group_input runs before the existing singleton left-press branch (diagram.rs:724 versus the middle-pan guard around :858). It has no state.pan_start admission check. Reproducible event path from inspection: select A/B, middle-button press sets pan_start; while holding middle, left-button press on selected A enters GroupGesture::new; subsequent cursor motion is consumed by the group and left release emits a model move even though the middle pan remains active. The legacy singleton handler explicitly captures this left press without moving nodes.

Keep gesture ownership exclusive before starting or toggling group selection. Reject/defer left presses while middle pan is active, and ensure starting a group cannot coexist with drag, route_drag, port_drag, or work gesture state. Regression should replay middle-down → left-down on selected node → move → left-up → middle-up and assert no geometry/history/selection mutation attributable to a group drag.

P2 — Delete hides the group during an active drag

The active-group dispatcher handles only cursor movement and left release (diagram/multiselect.rs:27–45). Delete falls through into the unconditional multi-selection Delete branch at :54–63, emitting Hide(snapshot) while the live preview exists. App validates that snapshot and hides the placements. By contrast the established singleton Delete branch requires every move/route/port/pan/work gesture to be idle (diagram.rs:803–813). Group selection should preserve this input-safety convention.

Require no live pointer gesture before publishing Hide, or define and test an explicit cancel-then-delete policy. Do not silently delete while the drag's release is still pending. Regression: A/B selected, start group drag and move, press Delete before release; expect no deletion/model history, retain the selection, and verify the subsequent release follows the chosen policy. Cover Delete during middle pan too: the same new branch also lacks pan_start gating.

Performance observation (P3, no latency measurement)

Every accepted App::update calls reconcile_multiselect (app.rs:1383), which unconditionally calls projection::project_view (app/multiselect.rs:76) while on Page::Diagram. That rebuilds ViewIndex::new, including the global BehaviorIndex/evidence and all edge facts (oa-core/src/view_options.rs:484–543). Thus each pan delta, inspector keystroke and unrelated asynchronous completion now incurs global semantic indexing plus projection, even when surface/model visibility and selection are unchanged. Explicit Choose additionally projects before calling reconciliation and again at end of update.

Reuse document.view_index() via project_view_with_index, and separate revision/surface/visibility reconciliation from explicit legacy selection changes. Cache or skip the former when authority and visibility inputs have not changed. This is a verified unnecessary work path; responsiveness has not been measured and no timing claim is made.

Sound portions and boundaries

• Selection stores insertion-ordered exact typed targets, maintains primary membership, prevents Whole admission, and uses a generation plus exact snapshot/context comparison to reject remove/re-add and primary ABA cycles. The reducer's survivor fallback itself is correct; the defect is App's later legacy reconciliation.
• placements collects an Option<BTreeSet<_>> over the full target set, so movement/hide cannot silently filter ports or edges. Selection events are checked before App's general allocation/camera boundaries, and operation events require the exact captured selection generation.
• GroupGesture freezes viewport and press geometry, consumes release position, latches threshold crossing, preserves a group after return-to-origin or snap-to-zero, and permanently retires invalid/outside pointers. Ordinary group input checks current selection snapshot and viewport before processing it. Preview applies only the geometry planner's emitted records, avoiding the full session-per-frame identity work identified in the core review.
• The existing ordered state.pointer normalization occurs before group_input, preserving native event-order handling rather than using the final batch cursor for earlier presses/releases.
• Escape, viewport/context invalidation, window focus loss, cursor leave, wheel, and right/middle press cancel an existing group before committing. App independently denies dirty drafts, busy/modal ownership, active architecture editors and connections. The newly passive modifier path is appropriate for Shift held before canvas focus; it does not grant keyboard command focus.
• Model/source transactions still flow through App::execute and Document::execute. Successful group movement restores the exact primary after the command's implementation-selected last record; zero movement returns before execution. Source failure is not accepted as a successful group operation.

Future verification should use mounted App and native-ordered canvas-event tests for the four findings, then actual GUI exercise. Reducer and gesture unit tests alone cannot expose the cross-layer sync and competing-gesture paths above. Existing known incomplete adapters are neither accepted nor reopened by this review.

Follow-up inspection while desktop-1 gate runs

The owner has addressed the reported paths in source. This status is static inspection only; the review findings above remain useful regression definitions and are not asserted to be outstanding in the updated source:

• App reconciliation now detects an invalid exact primary before pruning and retains the reducer's survivor fallback when legacy sync has no target.
• A typed deferred multiselect::Press preserves mixed-selection stationary clicks; movement refusal occurs only after threshold crossing.
• An active group captures keyboard input, so Delete does not fall through to Hide; existing Escape cancellation still precedes that capture.
• Group admission yields to an existing pan_start, preserving the legacy middle-pan exclusion and Delete guard.
• App reconciliation is conditional on its selection/model/surface boundary and uses the document's cached view index.

Eleven production-canvas event regression tests have been drafted in the local-only .local/multiselect-canvas-tests.rs for later authorized incorporation into diagram/multiselect_tests.rs. They exercise actual DiagramCanvas::update, including ordered native batches, captured status, preview records, snapshots and independent literal geometry expectations. Coverage includes Shift hit identity, 0.5/1/2 zoom grouped movement, mixed click versus drag, Escape, Delete during drag, middle-pan exclusivity, seven viewport/context changes, membership ABA, release-only displacement and return-to-origin. The parent owns App-level survivor tests, module registration, compilation and native verification. No draft test has been compiled or run by this reviewer; no production test file was created during the frozen-input gate.