Frozen behavior Delete candidate: independent acceptance review

Reviewed 2026-09-12 by /root/behavior_delete_acceptance_review, GPT-6 Astra high, read-only production review. Scope: the nine candidate files in .local/behavior-delete-20260912, branch codex/behavior-delete-956atf-20260912, uncommitted over dc1506f; implementation/design handoffs; applicable AGENTS and PROJECT_GOAL; surrounding dispatch, projection, focus, mounted harness and core deletion code. Root owns issue 15, integration, execution and delivery. This review did not use Gitea, Cargo, rustc, native UI or WSL and did not edit production files.

Result

No blocking or high-severity implementation defect was confirmed by source review. The candidate is suitable for integration and executable acceptance work. This is not runtime or whole-product acceptance: candidate tests had not been executed by this reviewer, confirmation prose was deliberately awaiting parent integration, and the focused tests do not establish every required input/recovery workflow.

Evidence supporting the implementation

Exact identity: process Graph eligibility checks rendered node/edge IDs, rejects owner, and requires the ID's actual behavior owner (behavior/topology/direct.rs:512). App rechecks full semantic context, current selected ID, tool, modal/dirty state, projection eligibility and qualified-selection state (app/process_canvas.rs:100). Scenario uses actual scene box/line IDs plus owner membership (behavior.rs:1193, behavior/scenario.rs:92, app.rs:1665). This does not substitute referenced definitions or implied path endpoint IDs.
Qualified process previews: the outer reference remains the App's selected UUID, but has_qualified_selection is checked before opening confirmation. The rejection explains how to open the shared definition or select the actual outer use. The qualified predicate resolves the owner/address against the current topology index. The mounted test uses a real preview coordinate and verifies a SelectQualified message before attempting Delete; it does not seed the qualified state.
Capture and replay: app/diagram_removal.rs:179 captures the derived owner after retiring the pre-modal canvas epoch. Confirmation requires the same owner at line 196 as well as the existing document/revision/epoch/page/layer snapshot and captured selection. Derived placement is consistently normalized to None in request, capture and validation (:37). Cancel/new-token replay and changed-owner/same-selection checks target the relevant lifetime risks.
Semantic preservation: confirmation executes ordinary Command::Delete on the captured UUID. oa-core/src/references.rs follows owned children, incident/dependent relations and references into removed objects, rather than following an occurrence's reusable target outward. Lifeline removal clears participant references/bindings on retained occurrences. Call removal clears retained reply correlation. The candidate does not introduce a second deletion implementation.
Input lifetime: nonrepeating plain/Shift Delete is admitted only for editable Select with no pending press or source. Middle/space pan blocks semantic keyboard forwarding. The new focus_pending immediately retires keyboard admission after viewport Tab/F6/Escape while keeping is_focused true for the deferred Collect/Apply operation. Real focus/unfocus clears it. This addresses the stale focused child during an Iced event batch without turning cursor presence into a keyboard ownership test.
Harness validity: canvas_workspace/tests.rs:155 builds the actual App.view() and calls pinned Iced UserInterface.update with the whole ordered event slice, then applies App messages and real widget operations. Pinned iced_runtime-0.14.0/src/user_interface.rs processes each base event before returning its messages, supporting the same-batch premise. Geometry probes calculate production projection coordinates; pointer tests assert the selected target afterward. Release followed by Delete in a subsequent build without a redraw is a meaningful check of stale child press state. Bundle compilation and workspace equality check the normal source/history path, although these checks still need execution.

Acceptance gaps to close or explicitly retain

Actual text ownership and keyboard recovery are not established by the two new focus tests. app/behavior_removal_tests.rs:253 chooses an arbitrary real text field and uses FocusText (:255, :307) to explicitly unfocus every other widget. That validly tests programmatic exclusive focus, but can conceal click/Tab ownership defects and does not identify the Inspector field. The boundary test (:200) checks lack of removal after Tab/F6/Escape but reacquires with pointer clicks; it does not assert that Tab reached a real control or that F6 can restore the viewport through the new pending flag. Suggested closure: select a real occurrence; click a named clean Inspector field, Delete at end-of-text, and assert no removal plus field ownership; then F6 and Delete with no pointer reacquisition. Separately assert the focused target after Tab, Shift+Tab and Escape, and keyboard-only F6 recovery.
Remaining input variants are source-covered, not newly demonstrated. The handoff correctly lists repeats, armed creation, message-source, connect/reconnect/reorder, contextual persisted links and legacy implied connectors as acceptance gaps. A small set of mounted cases should exercise these distinct mechanisms. For an implied connector, make the selected UUID and visible highlight explicit before and after clicking: unchanged selection of a previously selected occurrence must not be misreported as successful connector selection.
Remaining scenario preservation variants are not all explicit in assertions. The lifeline case proves messages are retained and sender/receiver references are cleared, but not scenario-use binding cleanup or action/state lifeline cleanup. The message case proves the reply survives but does not assert its reply_to was cleared. Core source supports those semantics; retain this distinction from executed behavior evidence.
Confirmation prose and actual rendered recovery remain parent gates. diagram_removal::explanation is intentionally not wired in the frozen candidate. Parent must use the captured object, inspect the rendered explanation for occurrence/link/lifeline/fragment cases, and inspect the incomplete-message state after lifeline removal. The supplied prose correctly distinguishes lifeline cleanup from deleting messages and reusable definitions.

These are verification gaps, not confirmed reproducible production defects. No new production fix is requested from this source review. Root's separate state relation-row visibility and F2 regression changes were outside this frozen candidate and must be included in its combined verification.

Checks performed

Read scoped diffs, affected surrounding implementation, core cascade/reference cleanup, and installed Iced input/operation source. git diff --check passed in the frozen candidate worktree. A Git CRLF normalization warning appeared on an earlier diff read of viewport.rs; no content defect follows from that warning. No executable pass, GUI success, upstream coordination update or publication is claimed.

Authorized evaluator follow-up

After the source review, parent authorized edits only to root crates/oa-desktop/src/app/behavior_removal_tests.rs to close acceptance gap 1. The frozen worker code is now c2e651b455fbacda25d7840f6bbdaa79e594bcba (parent reports CRLF normalization only), integrated as 49a4cb1. Parent reports 12b2237 supplies confirmation explanation/rendered button tests and state row handling; root 2d9e9c1 adds interaction specification/help. Those later parent changes are not retroactively claimed as reviewed here.

The root test file now separately covers actual Inspector-name pointer focus and Escape followed by ordinary Tab entry, asserts actual focused widget geometry and /name binding via End + ShiftDelete while clean, then asserts F6 exclusively transfers focus to the viewport and permits subsequent Delete without another pointer event. The existing boundary test now checks completed Tab traversal to one real control and inverse ShiftTab return; F6/Escape exit and keyboard-only reentry; same-batch F6-entry/Delete refusal; and successful subsequent Delete. These expectations follow the existing production process/scenario keyboard tests: canvas Tab cycles canvas members, while Escape followed by ordinary Tab enters Inspector. The separate programmatic FocusText test remains and is not presented as evidence of pointer/Tab behavior.

Frozen root file SHA256: EF09843A7DAEB91AFAB0B76E83C5679351A1635A84A26D80A194DEA744EA5874. Scoped installed rustfmt (--edition 2024 --config skip_children=true) and scoped git diff --check passed. No Cargo, compiler, native, WSL, Gitea or Git/index mutation was run by this reviewer. Parent owns executable acceptance of this test-only follow-up; writing the tests does not close the runtime gap until they pass against the integrated implementation.

9.0 KiB Raw Blame History