Files
ARCADIA-Studio/harness/multiselect-native-review-20260923.md
T

12 KiB
Raw Blame History

Independent native evaluator review — 2026-09-23

Read-only review of crates/oa-core/examples/multiselect_native.rs at 12a7352ddef5f69a144ead8219a50916233d63a9, the local native preparation, snapshot and capture helpers, and the owning session's proposed UI sequence. Only this note was edited by the reviewer. No builds, tests, UI actions or Gitea operations were run. The parent reports core, full desktop and lint gates passed; no native run was available for acceptance at this review checkpoint.

Finding and correction

P2, corrected in the helper during review: the original .local/multiselect-native/prepare-native.py:38–41 recorded the GUI exit code and binary/fixture immutability booleans, then returned normal helper success even if the GUI crashed or a recorded input changed. This could mislead a consumer using process exit status as the gate. The parent added assertions at lines 42–43 requiring GUI exit zero and all three immutability conditions after writing the durable receipt. Re-reading the helper confirmed the correction. No production source or build input changed for this correction, and it was not runtime-tested by this reviewer.

No additional actionable checker correctness defect was established. A native acceptance result still requires the actual UI sequence and evidence below; successful launcher completion alone is insufficient.

Independence and falsification coverage

  • The expected Project starts from the loaded baseline and changes explicitly enumerated fixture records. Movement expects A, B and the child to move once, translates only the internal route's pins, and preserves C, one-ended route pins, attachments, labels, all semantic records, traces and other views. Hide removes only A/B placements. It preserves the unselected child, stored routes and model objects. Neither expectation invokes the group planner, descendant traversal or routing implementation.
  • equal compares the complete Project, not a selected geometry subset. Validation, JSON roundtrip and source compilation/roundtrip supplement that comparison. Completeness warnings in this deliberately partial fixture are allowed; source discovery errors and compilation failures are rejected.
  • The ten checker tests include positive generation/source/command examples and multiple valid-model corruption falsifiers. The negative cases cover an omitted or double-moved child, stranded internal pins, moved one-ended pins, attachment/label drift, semantic and trace drift, other-view and fixed node changes, invalid movement evidence, destructive Hide and unchanged-mode semantic drift. Corruptions are validated before comparison and most tests require the specific whole-Project mismatch, avoiding a fake pass caused only by malformed input rejection. Planner use is confined to generating positive/corrupted test candidates, not expected evaluator output.
  • Fixture UUIDs are deterministic, generation refuses an existing output directory, and evidence writes are exclusive. Preparation validates binary hashes against successful unchanged-input build receipts and requires the GUI and checker input-manifest hashes to match. The inspected checker receipt names the reviewed commit; the GUI's then-current started receipt records the same input hash. Final build/run results remain the parent's gate.
  • The snapshot helper copies source into a unique directory, compares the source file set and hashes before/after copying and after checking, pins the checker and baseline, and propagates evaluator failure. Source snapshot evaluation is separate from UI capture, as it should be.

Acceptance limits for the planned sequence

The parent plans native context Add for A/B, actual drag, saved-model checks, one-step Undo/Redo with exact source-byte comparison, undo back to baseline, group Hide with Undo/Redo, and reopen. That is appropriate evidence for the ordinary saved-view path with these explicit limits:

  1. The movement checker is an invariant evaluator. It infers the common delta from saved A and checks bounds, nontrivial length and anchor grid alignment. A wrong drag direction or magnitude can pass if all required records share the same admissible delta. The code and manifest already state this limit. Record native press/release and viewport observations if claiming intended displacement accuracy; otherwise claim only the verified common translation and preservation invariants. Existing pointer/gesture tests provide separate evidence and are not native coordinate evidence.
  2. Model equality does not prove an action happened. Capture one actual Undo or Redo and completed Save between the appropriate snapshots. An unchanged check alone cannot distinguish successful Undo from never making an edit. Reopen must actually load the saved working source in the GUI and demonstrate its state; checker roundtrip alone does not verify GUI reopening.
  3. Exact source-byte restoration is a separate comparison. The snapshot helper records each source inventory but does not compare different stages. Explicitly compare all relative paths and hashes: Undo movement to the original fixture source, Redo movement to the first moved snapshot, and corresponding baseline/hidden inventories for Hide history. A Project PASS does not establish identical source formatting or file layout.
  4. Join provenance rather than assuming it. Tie the captured native window to the launched PID/executable in started.json, identify the opened working entry, retain the build/input receipts, and require the final fixture and binary immutability checks. The checker verifies the expected fixture project ID, not a baked-in baseline hash; baseline authenticity comes from the generated fixture inventory and unchanged final receipt. Capture JSON and screenshots alone do not establish which binary produced saved source.
  5. This fixture is OA Activities on an ordinary Overview. Native context Add verifies that explicit alternative, not held-Shift injection (unavailable through the chosen native tool). Mounted focus tests separately exercise Shift before canvas focus. The native run does not establish an SA Function workflow, selected ancestor-plus-child deduplication, mixed-port/edge refusal, busy suspension behavior or every cancellation path. Existing targeted tests remain separate evidence for those cases; no additional broad native claims should be inferred.
  6. Scope remains incomplete beyond ordinary saved views. State, Allocation, Process/Scenario and aggregate model deletion are not accepted by this checker or planned native sequence. Their absence is not a new finding in this bounded review.

These limits qualify what the evidence proves. They do not require replacing the independently enumerated Project oracle or broadening this native fixture into the unfinished adapter families.

Native layout follow-up: selection changes move the canvas

P2 — group feedback changes the diagram viewport while merely selecting. The parent subsequently supplied native run-1 captures. I independently viewed .local/multiselect-native/run-1/06-select-a-0.jpg and 08-group-selected-0.jpg; both capture the same native window at 1442 by 932. The canvas begins near y=314 in the singleton capture and y=370 after forming the group. A's header moves from approximately y=484 to y=511. The group count, Hide/Clear actions and primary-properties explanation are readable, but adding them has moved the diagram under the user's pointer. This is a presentation finding; it does not assert that selecting changed saved geometry.

The source cause is explicit: views.rs:434–461 conditionally appends a two-row group block above the View content control and canvas when the target count exceeds one. The additional block is about 55 pixels high. The observed node shift is smaller than the viewport-top shift, consistent with the canvas resize and current fitted camera reacting to the reduced height. Removing the final group member or clearing selection reverses the same conditional layout.

The ordinary idle hint (views.rs:654) also mentions right-click, drag and zoom but gives no visible Shift-click discovery cue; neither inspected screenshot shows such a cue. Context Add is usable, but the user's requested modifier gesture should be discoverable directly in the ordinary selection UI.

Smallest space-conscious correction

Move the group feedback into an always-mounted, fixed-height selection/status strip at the existing fixed 30-pixel hint location near views.rs:658. Remove the conditional group block from the toolbar. A roughly 56-pixel strip can contain:

  • A fixed approximately 32-pixel first row with selection count, the existing group Hide/Clear controls when applicable, and the short explicit disclosure Properties edit <primary> only. Use a bounded/nonwrapping primary label so long names or a mixed selection cannot increase strip height. Empty and singleton states keep the same row allocation even with fewer controls.
  • A short second line: Shift-click to add/remove · drag selected to move · right-click for actions. Existing active connection, allocation and route instructions retain precedence when those modes own input; ordinary idle selection should expose the modifier hint.

This reuses the existing status location and consumes about 26 more vertical pixels than today's singleton layout, while recovering about 29 pixels from today's group layout. The shown central workspace is approximately 860 pixels wide, sufficient for these compact controls and a bounded primary label without adding a third row. A fixed outer height, fixed first-row height and nonwrapping text keep the canvas rectangle identical for zero, one and multiple targets. Keep this strip outside the canvas; an overlay would obscure diagram content and complicate hit testing. Simply reserving the current extra 55-pixel toolbar block would also stop the jump, but spends more permanent vertical space than reusing the existing status strip.

The narrow verification target is mounted canvas-bounds equality across 0→1→2→1→0 selection transitions, including mixed selection and a long primary name, with unchanged model/history for those selection changes. A native before/after capture should confirm stationary node screen positions and readable controls at the same 1442-pixel window width. No source changes, build or native interaction were performed by this reviewer for this follow-up.

Delegated layout correction

The parent subsequently authorized edits to views.rs, app/diagram_focus_tests.rs and this note. The conditional toolbar group block is removed. Every active ordinary diagram now reserves a 56-pixel strip in the former hint location with a fixed 32-pixel controls row. Count is always shown; the existing group Hide/Clear intents and primary-only property explanation are shown for groups. The primary name remains shortened and all strip text is nonwrapping. Hide still requires whole-set placement admission and all App event guards are unchanged.

Idle selection hints expose Shift-click add/remove. Supported ordinary placement groups advertise dragging selected elements together; mixed groups and Allocation groups receive selection guidance without claiming a movement adapter they do not have. Active allocation feedback and connection instructions retain priority; singleton route/promoted-edge hints are preserved. The strip remains outside the canvas with clipping, and no overlay or global layout policy was introduced.

One mounted regression was added in source: mounted_selection_strip_keeps_canvas_and_fitted_camera_fixed_for_empty_single_group_and_mixed. It mounts the real App at 1442 by 932 with fitting enabled, a very long primary name and a visible edge, then checks 0→1→2→3 mixed→2→1→0 selection transitions. Each step must retain the exact measured canvas rectangle, zoom, pan, fit flag, observed canvas size, complete Project, source bundle and empty history. The previous implementation changes the canvas rectangle on the 1→2 transition, so this verifies the observed regression through production layout and sensors.

Only a static inspection and git diff --check were performed for these edits. Formatting, compilation, regression execution and a fresh native before/after capture remain owned by the parent. Native run-1 continues against its unchanged pinned binary and cannot serve as verification of this subsequent source edit.