Files
ARCADIA-Studio/harness/scenario-group-spacing-evidence-20260924/review.md
T

20 KiB

Scenario group spacing implementation review

2026-09-24. Read-only review of the local, uncommitted extension outside prerelease #37. No Cargo, Git, network or GUI operation. Tests authored independently have not been executed by this reviewer. Reviewed the frozen pure implementation and the completed controller click correction. No unresolved material finding in this reviewed implementation; build/test and native evidence remain separate.

Addressed material finding

[P2] Preserve stationary click-to-collapse for mixed and unsupported groups. behavior.rs:2236-2248 classifies a saved-view hit as a group spacing hit solely by coordinate capability and the pressed role. Therefore an existing mixed selection containing a local lifeline and local message, plain-clicked on either selected spacing-role object, bypasses ScenarioGroupPress. The widget immediately publishes spacing Begin (:2330-2349). spacing/controller.rs routes the original set to capture_group, which correctly rejects mixed membership. app/scenario_spacing.rs consumes that failed Begin by advancing canvas_epoch, making the later release inert. The result is refusal without collapsing to the clicked member, whereas the prior deferred group-press path emits Intent::Collapse on a stationary release (behavior.rs:2384-2404).

Rejecting the drag as a whole is required, but eligibility for moving the group should not remove the established selection-only click operation. Preserve a pending click/collapse route for ineligible groups, using exact original Context/Snapshot and Euclidean movement tracking; actual motion should refuse the whole group. A whole-set role preflight that retains the existing group_press path is one possible bounded fix. Add mounted tests for a lane+message group and a message+unsupported occurrence group: stationary click on each selected spacing-role member selects only it without a source/history change; motion retains the whole set and refuses; stale release after Context/selection change is inert.

Correction reinspected: the final controller stores group_refusal with the full original Snapshot. An eligibility refusal may use a successfully captured pressed singleton only to establish the unchanged full-scene geometry and actual camera/hit witness. Session::update returns before Drag::prepare whenever that refusal exists, so neither a partial preview nor a singleton command can be constructed from this fallback. Finish returns Click if the Euclidean threshold was never crossed; after crossing it returns the retained whole-set refusal, including off-axis/out-and-back motion. Invalid actual baseline, camera or hit still fails Begin and retires its epoch through App. Group paint retains original membership and primary. This resolves the identified click regression without making movement eligibility a selection requirement. The newly read App tests cover mixed stationary clicks on both roles, whole-set drag refusal/replay, malformed Begin retirement and source-write failure; execution is pending outside this review.

Adapter findings without a defect so far

  • App admission checks complete Scenario Context and camera epoch, then shared selection context/generation before any status, session or model side effect. Non-Begin packets additionally require exact session token/stamp and Selection::matches against the immutable captured Snapshot, including ordered typed targets and primary.
  • A valid Begin captures the complete current Snapshot. A selected nonprimary spacing-role hit reaches Begin without replacing the group or its primary. Group capture receives the original typed target slice rather than an eligible subset.
  • Cancel, Finish, and otherwise admitted failed Begin retire the canvas epoch; old mismatched packets return before touching a newer session. Current disabled input cancels its own admitted session. This preserves the camera visit rather than treating a selection-only change as a camera navigation event.
  • Session preview configures the full group Snapshot and captured primary in both original and prepared scenes. The workspace retains the Canvas structure and displays selected count with spacing feedback.
  • Group threshold is Euclidean in screen pixels while spacing delta remains axis-specific. ever_crossed survives a return to origin: off-axis or out-and-back motion can finish with no prepared command rather than collapse. An actually stationary supported group click collapses to the pressed member.
  • Finish takes the session and retires its epoch before execution. Group success restores the captured primary before synchronization/reconciliation; only successful document execution accepts the frozen camera. Document::execute prepares the candidate and source update before committing, so source-update failure leaves model/source/history uncommitted while the gesture is retired.

Frozen pure implementation review

No additional concrete finding in behavior/scenario/spacing.rs. The typed group entrypoint rejects empty, duplicate, wrong-target-type, mixed-role, missing, and unpressed-member sets before planning. Every captured origin then requires exact local ownership, appropriate semantic role, and a unique actually rendered header/message; the message path checks actual sequence/operand membership and blocks cycles or foreign references. Cross-operand local groups remain permitted without changing parents.

Planning uses frozen original display origins and the original Project, applies one common axis delta, deterministically orders commands independently of primary/pressed member/insertion order, and validates the final batch geometry. Any invalid member or final geometry refuses the whole plan. Skipping a member whose f32 coordinate is unchanged is a representational no-op, not an eligibility subset; it preserves missing/explicit overrides rather than manufacturing equivalent records. Zero/out-and-back input returns no command. Final candidate execution and source synchronization remain atomic through Document::execute.

Challenged the newly shared singleton origin checks: direct local lifeline ownership and unique rendered message path are stricter than merely selecting a drawing by ID, but I found no valid preexisting local singleton spacing workflow excluded. They intentionally reject malformed, repeated, referenced or unrendered input; rendered unassigned local headers remain supported. This conclusion is static and still depends on the singleton regression gates passing.

Verification still needed

Run the parent's focused evaluators. In particular retain coverage for first-press nonprimary group drag, primary/order preservation, mixed/unsupported full-set refusal, current failed-Begin retirement, stale terminal packets against newer sessions, selection/representation ABA, Euclidean off-axis and out-and-back behavior, source-failure atomicity, and unchanged camera/visit. Supported groups return Noop after off-axis/out-and-back motion; movement-ineligible groups refuse the whole set after crossing, and neither collapses. The Scene's existing cached-pointer event policy is retained: this review does not claim a new position source for a release lacking CursorMoved. This review is not build, native usability, or release acceptance.

Test-only correction review at 67534ce4

Independently compared 67534ce4a5fab29cb69b461f00d447eca2e7d12b with failed source 3b113e5316724eca001cd92c5b8efed0a48ccc86 and read the retained run-1 failures (exit 101, unchanged inputs; 286 passed, 3 failed, 1 ignored). Only two test files changed. No concrete concern that these three corrections hide a production defect. No Cargo was run; the parent's run-2 result was not assumed.

  • The complete-group movement test previously assigned fixed expected X values to randomly generated lane identities. Its failure shows exactly those expected X values reversed between the two lane IDs; actual automatic lane positions follow identity order (behavior.rs:1468). The correction authors explicit starting lane/message positions before capture. The expected complete Project remains independently constructed from those literal positions plus the screen-to-world delta; it does not copy the production result. Both pressed members, lane/message cases, preview nonmutation, captured Snapshot, primary/set preservation, exact Undo/Redo source restoration and reopen checks remain. This particular App test now covers explicit initial layout; unchanged pure tests still cover absent overrides and group movement from automatically resolved origins.
  • The Properties cancellation test used the controller-only packet helper's generation zero against a live App selection. Filling its stamp from the current Snapshot matches real workspace packet construction and the existing App packet helper; it does not bypass production admission. The test still requires a real held session and moved preview before opening Properties, then checks session cancellation, unchanged stored camera, inert late Finish, pointer-identical document snapshot and no history entries.
  • The mounted modifier test now expects a spacing Finish for a stationary saved group click instead of the retired direct Intent::Collapse transport. It retains the actual singleton target assertion immediately afterward and the later exact primary, representation/visit, camera, original document/source and empty history/session assertions. Shift toggle counts and absence of spacing/reorder for Shift input remain checked. This changes an internal message expectation while preserving the selection-only behavior oracle.

Numeric annotation and evidence reuse at 4a27c8ef

Compared final 4a27c8ef0b8f4954e208fdfda5efd2b41c240577 with 67534ce4: only the two test-oracle variables x and y acquire explicit f32 annotations. Previously inferred f64 literal values 148, 240, 160 and 400 are exactly representable in f32, and f64::from then restores the same exact f64 values before the unchanged f64 delta addition and f32 result cast. There is no numeric change in the oracle and no production change. No material finding.

Read the actual closed run-2 receipt/log: 289 passed, 0 failed, 1 ignored at 67534ce4, exit 0, inputs unchanged. Read the preserved clippy-1 failure at 67534ce4: two useless_conversion errors, exit 101, inputs unchanged. Final format-3 passes at 4a27c8ef. Clippy-2 had no closed receipt at review time. Reusing the executed 67534ce4 test evidence with final-source all-target lint (which checks the edited test code), product build and native exercise is proportionate for this exact annotation-only correction. Those final checks must actually pass and retain their own source pins; this conclusion neither invents their results nor represents the 289 tests as executed at 4a27c8ef. Further production edits or a concrete gate failure would require reassessing reuse. No Cargo was run by this reviewer.

Copy-only follow-up at 8d6fac47

Reviewed all four changed production files between 4a27c8ef and 8d6fac4701933c7ada13cc7e6e07060662aa7465 (7 insertions, 4 deletions). Three refusal/menu strings replace the obsolete blanket movement-unavailable wording with unavailable reordering/removal or current-selection movement refusal. The fourth adds a saved-group Select tooltip; its new conditional selects text only. No operation admission, captured-state lifecycle, command or geometry path changes. No material defect found, and reuse of the 67534ce4 executed-test result remains proportionate with final source gates and native verification kept distinct.

Nonblocking wording observation sent to the parent: the saved-group tooltip is also shown for mixed/unsupported/fallback selections. “Drag a group of messages vertically or lifelines horizontally” would make homogeneous-group intent clearer than the current generic “Drag messages vertically or lifelines horizontally”; the actual full-set refusal remains enforced.

Read the newly closed clippy-2 receipt: exit 0, unchanged inputs at 4a27c8ef. Format-4 likewise closed exit 0 at 8d6fac47. Build-1 has started for cargo build -p oa-desktop -p oa-cli --message-format=json at 8d6fac47; no closed build, final lint or native result is claimed here. The 289 tests remain an execution result at 67534ce4 only.

Native run-1 and helper audit

Reviewed native-evidence-package.py, native-evidence-verify.py (the file copied as portable verify.py), run-1 launch/close/build/fixture records and selected original screenshots. No material helper/schema or run-1 evidence mismatch found. Run-2 fresh reopen was still separate ongoing work, and no final portable package had yet been audited.

Independently rehashed every run-1 prepared immutable input, including the copied executable and DLL. The GUI is 119,703,552 bytes, SHA256 e74342f3e06a28f91d378a429661fc88e5b009ce60549b3a2611160d48d16cdc; it equals the uniquely identified non-test Studio artifact in the closed successful unchanged build at 8d6fac47. Source-proof/result/input-map bindings and the 811-input count agree. Started binds the exact prepared receipt. Finished records normal exit 0, immutable inputs unchanged, empty stdout/stderr, and final working source 3,554 bytes, SHA256 360b9e24564a10cce3379a097bf30f33db5c583cceaaaf595fc595f662b556d5.

All nine actual source files outside immutable evidence (eight saved snapshots plus final working file) pass their declared phases. The checker permits only exact Decimal-equivalent numeric spelling at the predeclared changed A coordinates; baseline remains byte-exact, and every other byte, including B, must match the pinned expected fixture. This is explicit numeric spelling handling, not float tolerance or a claim that the whole moved source has identical raw bytes to the integer-spelled expectation.

Exercised the complete offline verifier against an in-memory package assembled from actual run-1 records, both preflights, fixtures and helpers with the intended schema. It passed with no run problems and 52 capture records, including validation of all three current raw CLI check reports and retention of the differently shaped old failed preflight. This rehearsal wrote no archive/files and is not a final-package integrity result. The packager is create-only; actual final archive/member equality and run-2 origin binding still require their delivery audit.

Viewed full original screenshots 15/16: both selected message lines move down while count two and Alpha primary remain; unselected Beta stays in place and the final status reports two-object spacing. Screenshot 43 shows count two, mixed Intake primary, and full-set refusal feedback. Screenshot 45 shows count one and Alpha primary after stationary collapse. These establish visible endpoints; precise first-press input sequence is the executing agent's observation, and exact model/source claims come from the byte checks. No native Shift, Escape key, full preview timing, or fresh-reopen outcome is inferred by this review.

The original helper README/HARNESS/EVIDENCE files retain historical “prepared only/not run” statements. Keep those raw notes unchanged, but label them as preparation history in the final package's current report so they are not mistaken for the final run status. Executed tests remain 289/0/1 at 67534ce4; the final 8d6fac47 binary incorporates only the already reviewed test annotations and UI copy beyond that tested source. No product launch, build, or production edit occurred during this audit.

Final native reopen and delivered-package audit

No material package integrity or final-report overclaim found within this bounded audit. Reviewed the final implementation report and root evidence index. They distinguish the 289 executed tests at 67534ce4 from final 8d6fac47 lint/build/native evidence, retain the two failed gates and obsolete-CLI preflight, explain historical helper names/preparation notes, and explicitly exclude combined integration, release and broad usability acceptance.

Correction to the run-1 paragraph above: the retained directory contains nine .arcadia snapshots (baseline, final-after-refusal-cancel, lane-move, lane-redo, lane-undo, message-move, message-offaxis, message-redo and message-undo), plus the working file: ten actual source files. The earlier eight-snapshot/nine-file count was a review wording error. The final package verifier checks all nine snapshots and working source; all declared phases pass. Run 2 adds one reopened-save snapshot.

Run 2 started at 2026-09-24T23:25:23.494008Z as PID 1036, after run 1 had closed, and finished normally at 23:27:42.098383Z with exit 0 and empty stdout/stderr. Independently rehashed its immutable files and checked the prepared/start bindings, distinct process, reopen-origin source/copy pins, snapshot sidecar and final receipt. The copied origin, launch input, reopened-save snapshot and final working source match run 1's actual saved 3,554-byte source SHA256 360b9e24564a10cce3379a097bf30f33db5c583cceaaaf595fc595f662b556d5. Both edited A groups therefore survive this fresh-process reopen/save without further source changes.

Viewed run-2 original actual-size screenshot 14. Saved A visibly retains the two moved message/lane groups at 100%; exact coordinates are established by the source comparison. The Review-to-Archive gap is now 228, and Beta's white label backing visibly interrupts a short Archive lifeline section above Beta. Labels remain ellipsized. These are bounded visual/readability limitations, not evidence that stored group coordinates failed; the report discloses them and appropriately avoids broad readability/accessibility acceptance. Screenshot 09 shows the intended run-2 path after filename refocus/retry. The failed local binding attempt before any action is the executing agent's reported tool recovery; neither that nor the recorded dialog focus retry supports a product-failure claim.

Ran both delivered portable verifiers successfully without launching Studio or Cargo. Checks: 10 closed runs, 2 preserved failures, 10 source proofs, 4 deduplicated source capsule pairs, 83 logical files and 71 stored members, with 811 inputs per run. Native: 2 accepted closed runs, 67 capture records (52 + 15), 319 ZIP members and 6 explicitly excluded executable/DLL pins. Independently compared all 80 archived raw Cargo/run files with their actual .local/scenario-group-gates originals. Independently verified all native member hashes and exact byte equality of its 318 retained raw/helper files to their local originals; the remaining member is generated review configuration matching the manifest. Also rehashed all six locally retained excluded binaries against their pins. This local binary rehash is additional review evidence; portable replay cannot rehash omitted bytes. Source proofs and artifact links were checked for internal agreement; this audit does not claim a new Git comparison of every captured input.

Audited archive identities:

  • checks/evidence.zip: 12,897,498 bytes, SHA256 d3de224502a2b7f8236d2ad82af6d6c57bb3a349468aebeea7369c3c3c267342.
  • native/native-evidence.zip: 6,069,072 bytes, SHA256 27e0e189053fdfdbfcecd8c91ae718849448b090aab557afc7d9445af1ceaba8.

The root index resolves the nonblocking historical checks-README title/example mismatch by identifying reused schema names and listing the actual group-spacing runs; original pinned bytes remain untouched. The archived preparation notes are explicitly historical. Review remains limited to source reasoning, selected visual endpoints, retained receipts/source bytes and packaging consistency; it does not independently reproduce the executing agent's complete input sequence or exercise native Shift/Escape, intermediate preview frames, accessibility, or another platform. No production edits, builds, native actions, Git mutations or network operations were performed for this final audit.