28 lines
4.9 KiB
Markdown
28 lines
4.9 KiB
Markdown
# State multiple-selection independent review
|
|
|
|
Scope: State adapter above `7e348d967d1787c052a145b7448636613a6a7054` on `codex/multiselect-state-9e8r6t-20260923`, subsequently committed as `2a3b9ba822a49ead233c4b5f98412a07c1c1da12`. This is branch-specific review evidence, not integrated acceptance. The reviewer was a fresh GPT-6 Astra agent and did not author the adapter or checker. Parent owns verification and integration.
|
|
|
|
The read-only review covered App admission, exact State row identity, context-menu ownership, the unsnapped gesture policy, diagram-only simultaneous preview, fixed State controls, and the independent native checker. No Cargo or native GUI execution was performed by the reviewer.
|
|
|
|
## Findings
|
|
|
|
1. **Checker rounding false negative.** Recovering the displacement from A's nonzero `(30,100)` origin can round it differently from the original `f32` displacement. For actual `(0.01f32,10f32)`, saved A's X is `30.0100002289`, giving recovered `0.010000229`; an inner node initially at 55 actually becomes `55.0099983215`, while replaying the recovered delta predicts `55.0100021362`. The reviewer reproduced this with single-precision arithmetic. The strict whole-Project oracle should retain exact comparison and use an origin anchor, with nonbinary fractional positive cases. This is a checker defect, not evidence of incorrect product movement.
|
|
|
|
2. **Late refusal changes unrelated input state.** Same-context legacy State `Select`, `Move`, or `Remove` events under a multiple selection were refused in `edit_state_diagram`, after `App::update` had reconciled pending reveal and retired allocation input. Static tracing showed that the rejected event could clear navigation focus/pending reveal, advance allocation epoch, and clear `arrange_next`. Refusal must occur before those hooks. Model/source/history-only assertions do not cover this failure; the regression must include the shared state and a queued `Select`.
|
|
|
|
The reviewer found the inspected exact-row visibility, typed menu target checks, planner-backed preview, authored boundary-symbol anchors and fixed-height control design consistent with the intended contract. This bounded review does not establish correctness by itself.
|
|
|
|
## Required verification identified by review
|
|
|
|
Run current-State App and mounted focus tests, existing State bend/reconnect/attachment tests, and singleton route interactions with `StateCanvas.selection = Some(singleton)`. Legacy canvas constructors using `None` bypass the new selection admission path. Verify actual native group movement, Hide, history, source preservation and reopen on the recorded Windows build. Linux verification remains user-paused.
|
|
|
|
## Follow-up review and first corrected gate
|
|
|
|
The same independent reviewer inspected both corrections and found them addressed statically: the main fixture positions, pins and recorded header points shift consistently by `(-30,-100)`; A is at the origin; `(0.01,10)` and `(0.1,-0.2)` are real-command positive cases. The legacy State rejection now precedes the shared hooks, with a regression comparing valid navigation markers, full selection, model/source, camera, epochs, feedback and history. Three singleton-selection route tests cover snapping, commit/cancel, stationary reconnect and Shift priority. No new blocking finding was reported. A minor fidelity note remains: the legacy queued Select probe uses a placement UUID rather than the semantic UUID normally emitted by the canvas; the admission rejects the whole legacy variant independently of that UUID.
|
|
|
|
Parent Windows `core-2` ran all 11 State native-checker tests successfully with 733 unchanged captured inputs (input-map SHA-256 `9cf85879dfb0bb5ec3b15c66b23df316e6bfe8451b6cf0c895bf7b9b5fbdde4a`). This includes the new nonbinary fractions. Full `desktop-2` passed 1,286 tests with six existing ignored on the same inputs, now compared exactly to committed Git blobs at `2a3b9ba`. Native acceptance remains separate.
|
|
|
|
Strict Clippy then found the optional exact menu target enlarged the message enum. Commit `3480b54201a5f361cca0bcd0d45dd6546a7a7bc9` boxes that field without changing its matching rules. Strict core/desktop all-target lint, formatting and all 122 focused State/menu tests pass on 735 unchanged inputs exactly equal to that commit. The revised capture scope explicitly includes the existing attachment and textual-authoring compile-time fixtures outside the usual source directories; historical receipts remain unchanged. Native verification remains separate.
|
|
|
|
The subsequent [native result and limitations](multiselect-state-20260923.md) record parent execution and the critic's independent screenshot/receipt/source-history review. No new blocker was identified for the State contribution. Shared picker Escape behavior is tracked as issue 35; multiline transition-label overlap and the distinction between recovery-choice visibility and actual recovery through the picker remain explicit limitations.
|