Independent source review — canvas focus follow-up — 2026-09-12

Reviewed source: 40154983cc86ac0a45b062d7381124f262adbb3d on codex/behavior-authoring-20260910; working tree was clean at start and end. Focus: 24bb248, e2c027e, b25065c, 4015498. No compiler, test runner, native application, tracked edits, or Gitea writes were performed by this reviewer. Root owns execution, integration and acceptance.

Read local AGENTS/PROJECT_GOAL, relevant STATE and both requested release evidence notes; read current main/AGENTS.md through Gitea (blob c8140593ef02025c96d900a87607590bf2b77eaf, matching local instructions). The cached origin/HEAD points at an old branch and was not used as current default-branch authority. Read the user-authored issue bodies directly: #15 requires Delete to remove from model/diagram; #25 says Canvas mode should always be active without an activation button.

Findings

No actionable blocking or high/medium source defect found in the bounded corrections. This is source-review approval of the design and regression targets, not execution or native acceptance.

Owner continuity is explicit and bounded

crates/oa-desktop/src/diagram/focus.rs:69 separates logical owner identity (document, diagram, epoch) from revision. A revision change clears pointer continuity while keeping whichever real Iced widget already owns keyboard input. New document/diagram/epoch, a modal/draft boundary, a window focus loss and an outside press revoke the owner.
focus.rs:87 replaces only the wrapped canvas child when passive busy state begins, discarding held gestures while retaining the wrapper's actual focus state. focus.rs:19 refuses new focus acquisition during suspension. focus.rs:118 continues exposing a suspended existing owner to ordinary unfocus operations. focus.rs:170 rejects keyboard/mouse dispatch during suspension.
crates/oa-desktop/src/app/diagram_removal.rs:20 keeps busy command admission disabled independently of the retained owner; the App busy guard also rejects authoring messages. There is no invented canvas-focus boolean restored after a save, so a clean Inspector can take real ownership without Save completion granting it back.
Underlying architecture and state canvases independently reset gestures when their full revision context changes (diagram.rs:580, behavior/state_canvas.rs:777), supporting the wrapper's owner/gesture distinction.

State picker slot is structurally stable

crates/oa-desktop/src/behavior/state_view.rs:146 always creates an empty-or-populated Column, and state_view.rs:181 keeps it ahead of the focus owner inside a fixed two-child parent. I checked the installed Iced 0.14.2 implementation: column! uses with_children/push, but it drops only children whose size hint includes Fixed(0). The empty Column uses Shrink, so it remains a child. Thus this is not the previously documented zero-width Row slot mistake. The incoming state header/control structure is fixed for an existing state-machine canvas.

Obsolete activation/panel mode removal is complete in production source

The Canvas and Panels controls, event variants, state booleans, toggle methods, message whitelist branches, conditional sidebar/Inspector layout and active help guidance are removed together. Searching production desktop source found no remaining scenario/topology CanvasEvent::Focus, Camera::Focus, presentation.focused, topology.focused, Focus canvas or Restore panels path. Unrelated semantic-review Event::Focus is a different concept and correctly remains.

crates/oa-desktop/src/views.rs:230 now gives behavior workspaces the ordinary sidebar/main/Inspector structure. F6/Tab/control ownership remains via Viewport and Collect/Apply; deleting the old Event::Focus arm does not remove ToggleFocus, Traverse, ControlFocus or Leave. Pointer admission remains independent of initial keyboard ownership.

Evaluator adequacy and required execution boundary

The updated evaluators are appropriate to the reported regressions and do not merely assert internal booleans:

app/diagram_removal_tests.rs:581 checks the mounted real canvas or clean Inspector focus through actual edit/history and passive-save tasks, including F2 ownership.
:659 denies focus acquisition while busy and checks window-lost ownership; :695 checks both pointer and programmatic Inspector ownership during passive Save; :740 retires a held architecture drag and then requires a fresh drag to work.
:992 retains actual state placement Delete → Save → Undo → Shift+Delete and now checks focus at picker appearance, Save and picker removal. It verifies the real tree before the next shortcut and would expose the original positional-tree replacement.
Scenario workspace tests at :1457 and process layout tests at :323 retain actual F6 entry/exit, Tab/Shift+Tab traversal, Enter/Space action, focus-ring rendering and Inspector text ownership. Removed-control assertions check IDs absent in the production tree, and direct-pointer workflows start with no keyboard owner.
The shared Mounted runner constructs app.view(), runs UserInterface input, applies App tasks including widget operations, and carries the UI cache across rebuilds. It is meaningfully stronger than seeding a private focus field.

Acceptance hold — verification, not a newly discovered code defect: the existing recorded focused gate is 22 passes plus a real state-focus failure at the earlier correction, and strict lint later failed on the two obsolete variants. Neither old result becomes passing because this source looks correct. Root must retain those failures and obtain the new full Windows test/lint results and matching native build checks before accepting this correction.

The architecture held-drag and clean-Inspector busy tests do not directly exercise the persisted-state canvas during suspension. The common wrapper makes their transfer reasonable source evidence, but native state checks should include an in-progress drag, Save, release and a fresh drag if practical. This is a bounded coverage limitation, not evidence of a defect.

Native acceptance should directly reproduce the original architecture click → Delete → Save → Undo → Save → Redo → Save → Undo → Shift+Delete sequence without an extra canvas click, and the state placement/picker sequence above. Verify exact saved Project/SourceBundle results, text ownership, window focus loss, and direct behavior editing plus retained F6/Tab at the required window size. Compilation, mounted rendering and this report cannot establish native event ordering or minimum-size usability.

This review does not establish process/scenario revision/Save keyboard-owner continuity: those workspaces retain a different Viewport lifetime policy (behavior/canvas_workspace/viewport.rs:82). It also does not establish nested activation-bar rendering, general free placement/resizing, Details usability, or complete closure of #15/#25 beyond the bounded behaviors reviewed.

6.9 KiB Raw Blame History