ARCADIA-Studio/harness/experiments/diagram-removal-windows-20260912/diagram-delete-acceptance-review-20260912.md

Diagram Delete candidate independent acceptance review

Reviewed 7dcfb3cb107a8f2fc28664fe5b7ad9cda16f4dda against 5a70181 in .local/diagram-delete-20260912. Read all 13 changed Rust files and the candidate handoff. Read-only source review; no Cargo/compiler, native application, Gitea or WSL operations. Parent owns integration and executable verification. This review does not establish issue 15 completion.

Finding

P2 — Delete and Shift+Delete silently fail after choosing Placement properties. Primary location: crates/oa-desktop/src/app/diagram_removal.rs:46.

Reproduction using existing UI paths:

1. In an OA or SA architecture diagram, right-click a placed node and choose Placement properties.
2. Click and release an empty canvas area without moving, retaining the selected placement and reacquiring canvas keyboard focus.
3. Press Delete. The placement remains. Shift+Delete similarly fails to open confirmation.

Source evidence: model_menu.rs:416 emits Message::Select(node.id). app.rs:1006 retains the supplied placement identity as selected; refresh_draft at app.rs:707 derives selected_node from that placement identity. The empty-background branch at diagram.rs:931 starts a pan rather than changing selection, and its release clears the pan. selection_context_request at diagram.rs:248 explicitly accepts n.id == self.selected and emits the node's semantic element as the removal intent's object. Admission at diagram_removal.rs:46 then compares that semantic object directly with the selected placement ID and returns.

Suggested fix: resolve the current selected identity against the exact captured placement when validating a canvas removal. Retain raw selection identity separately for lifetime validation. Also pass the captured semantic object explicitly to the confirmation initializer: begin_model_delete currently constructs its target from self.selected at diagram_removal.rs:92. Merely relaxing line 46 would make Shift+Delete target the placement, contrary to its semantic-removal contract. Preserve existing generic Message::Delete semantics for explicit placement-property actions.

Add mounted coverage for both keys through Placement properties → empty-canvas click. Assert Delete removes exactly the placement, Shift+Delete names and confirms the semantic element, Cancel preserves everything, and Undo restores the source bundle. Existing new tests only use semantic selections and do not cover this identity representation.

Evidence scope: deterministic source trace, not executed reproduction. Finding and normalization implications were sent to parent promptly; parent acknowledged ownership of the fix and regression.

Other reviewed behavior

No additional concrete defect established in the reviewed subset. The focus wrapper registers a real Iced Focusable, standard focus operations explicitly unfocus it, and its transient Tab flag denies later keys in the same native batch while preserving the original index for the queued operation. Checked pinned iced_core-0.14.0/src/widget/operation/focusable.rs, iced_widget-0.14.2/src/{row,stack,text_input}.rs, runtime/widget operations, and project reference-form traversal. Inspector traversal intentionally stays within its own form; this is existing behavior, not evidence of a new Tab regression. The mounted tests exercise real operations rather than private focus booleans.

Captured confirmation stores semantic target and token for ordinary semantic selection, checks document/revision/epoch/page/layer/diagram/selection/placement, and clears on Cancel/Escape. Keyboard removal does not call apply(); dirty source, property and behavior editors refuse admission. Ordinary placement removal uses the document command path. Bend deletion precedes whole-item removal; unsupported modifiers and active canvas gestures are denied. Allocation cleanup is limited to an epoch-retired retained press; live same-epoch presses remain guarded. No changes were found to process/scenario removal paths that could accidentally reinterpret their occurrences as reusable actor/function deletion.

Executable evidence remains the parent's responsibility. The authored ten tests are not evidence of passing behavior until run. They do not exercise actual rendered confirmation-button clicks, every gesture refusal, or state-action label removal. Process/scenario occurrence/link/fragment removal, Explorer keyboard removal and state-action shortcuts remain explicitly outside this candidate; no whole-issue acceptance should be inferred.