ARCADIA-Studio/harness/reviews/multiselect-composition-20260923.md

Ordinary saved-view multiselect: source/composition review

Reviewed 2026-09-23, source only. Delivery: 7e348d967d1787c052a145b7448636613a6a7054; final executable source: f6a6010baed9e6164a1fcc7ecd58256a562827d7, following 12a7352ddef5f69a144ead8219a50916233d63a9, above b8685416f56e53e30bd11ecf93a54cea10ac8458. Intended integration base: f82e29d6b8ed2b5fd23ac806675d3eafe851d104. Inspected Git objects, actual production/test changes, and harness/multiselect-saved-view-20260923.md. No merge, compiler, GUI, subject edits, or Gitea writes were performed. Peer runtime receipts remain evidence for the peer pin, not a compiled combined candidate.

Disposition

Suitable for a bounded integration attempt after the constructor repair below. I found no established production functional blocker in this source review. This is not acceptance of the combined executable or completion of all multiselect surfaces. Preserve the final fixed-height strip fix; integrating only the first feature commit reinstates a peer-observed canvas resize/refit defect.

Definite integration repair

New example does not construct the newer Diagram model. At delivery crates/oa-core/examples/multiselect_native.rs:36–46, the Diagram literal has no layout. The combined model requires layout: Option<DiagramLayout> at crates/oa-core/src/model.rs:1523. Add layout: None to this ordinary fixture when integrating. Serde defaults do not supply missing fields in a Rust literal. This is a source-established compilation incompatibility for the example/all-target gate, not a demonstrated runtime defect in the peer's own base. Check new literals against the final combined types rather than assuming clean textual application implies type compatibility.

Shared seams to preserve

A read-only comparison of the two ranges from b868541 found five production paths changed on both sides: oa-core/src/lib.rs and oa-desktop/src/{app.rs,diagram.rs,main.rs,views.rs}. Their edited base-line intervals did not overlap. This is a hunk observation, not a performed merge or a guarantee about Git conflict resolution.

1. App admission and reconciliation. Delivery app.rs:1293–1396 adds the multiselect boundary, early DiagramSelection envelope refusal, and post-action reconciliation. Keep the combined early Text handling and Scenario spacing/epoch admission (f82 app.rs:1367–1427) ahead of their side effects, plus the newer saved-Scenario architecture-action refusal. The new envelope check must coexist with these checks. app/multiselect.rs rejects dirty/busy/active-edit operations, requires all selected targets to be placements for Move/Hide, and rechecks full selection snapshot, context, and visible membership. It does not silently move the placement subset of a mixed selection. Nested Text dispatch calls self.update, so its mutating path can receive the new reconciliation; do not bypass that recursion when combining branches.
2. Surface identity. Retain the combined route_context exclusion for saved Scenarios and the saved-Scenario picker/view guard (f82 views.rs:391–408). The new adapter obtains its context only on Page::Diagram through route_context; Scenario/Behavior should never acquire an ordinary saved-view group merely because a Diagram record exists. State, allocation-group semantics, derived Process, and Scenario adapters remain explicitly unfinished in the delivery.
3. Canvas and rendering. Retain the added selection field and explicit None values in unrelated canvas fixtures; retain the combined SVG unsupported-Scenario result/error path (f82 diagram.rs:1881–1901). Keep delivery views.rs:614–684, especially the always-reserved .height(56) at line 682. The final mounted regression checks empty, singleton, group, and mixed selections, including a long primary name, against the same canvas bounds and fitted camera. This directly addresses the earlier conditional-strip defect.
4. Properties and Text. Keep the combined Properties Apply/Revert action row outside the scrolling body, Connections/Properties tab and scroll ownership, and the focused Text SourceIndent binding (f82 views.rs:1209). The new group controls belong above the diagram and explicitly say Properties edits only the primary; replacing the whole older views.rs would lose unrelated accepted fixes. The delivery does not change the source writer; retain combined compact/default/comment preservation and explicit-none handling in full.
5. Transient lifetime and focus. Keep the delivery's selection generation/scope checks, ordered pointer handling, modifier restoration through the mounted focus owner, and pending-group cancellation on viewport/context changes. Group placement moves are one execute(Command::Batch(...)); the core planner unions descendants and computes route translation once against the original diagram. The App rejects queued singleton Move/Delete paths while a group owns the action. These are meaningful protections, not UI-only selection decoration.

Targeted remaining checks, not proven production bugs

Group preview cache depends on event ordering. Delivery diagram.rs:549–555 caches by route context and singleton preview/route/port state, with no group snapshot or current selection/viewport validity key. diagram/multiselect.rs:267–295 applies a crossed-threshold group preview without independently checking live selection, editable state, or viewport. A direct scene read after changing those inputs can therefore retain the old preview until input cancellation clears the cache. However, native RedrawRequested is handled by DiagramCanvas::update and reaches group_input, whose validity checks run first; I have not established an actual native path that bypasses that ordering. This is not a proven stale-commit or native rendering bug.

The existing tests changed_viewport_or_context_retires_group_without_a_release_intent (delivery diagram/multiselect_tests.rs:596) and changed_selection_generation_rejects_press_even_after_membership_is_restored (line 646) dispatch a release before inspecting the cleared preview. A discriminating additional check should warm the real scene cache during a group drag, change viewport or selection generation, deliver the normal mounted redraw, then assert committed geometry and no release intent. Include dirty/read-only or overlay retirement if the combined owner can suppress child updates. Harden the scene validity contract only if that trace exposes a reachable gap; a bare draw-before-update unit call alone is insufficient to claim a native failure.

After constructor repair, verify the combined application retains: ordinary group selection and fixed canvas across empty/single/group/mixed states; one-command move/hide with full-Project and source round trips, Undo/Redo, and unchanged non-active views; stale group events across Text Apply/navigation; saved-Scenario refusal and stale Scenario events without selection/history/deferred-reveal side effects; and Properties action-row/tab return behavior. Exercise OA and SA placement identities, including duplicate semantic occurrences, alongside an existing saved Scenario layout and explicit route attachments in the same project. The peer's own source/history and geometry tests are reusable evidence, but do not cover all those newer composed records.

Evidence limits and oracle assessment

The core planner tests use explicit expected geometry/complete models for descendant union and route behavior; the App tests also compare source and history. Selection snapshots preserve exact placement/port/edge identity and a generation, preventing membership ABA from reviving a captured gesture. These checks support the claimed ordinary saved-view slice.

The native example openly derives a bounded common delta from saved placement A, then independently enumerates the expected moved set/routes and compares the complete Project. That is a useful invariant oracle, not an independent expected pointer-to-world delta. A uniformly wrong displacement could satisfy it. The handoff correctly labels this limitation; keep that wording when reusing its receipts. Native context-menu Add exercises selection through a different input path from Shift-click; mounted Shift tests provide separate evidence, and neither should be relabeled as native Shift-click observation. The final layout/refit regression and native fixed-strip observation are specifically worth preserving; passing core geometry tests alone would not have detected the original UI defect.