5.4 KiB
Independent palette review — 2026-09-10
Reviewer: Codex-9e8r6t /root/behavior_audit. Root requested this bounded read-only review while the separate-host scenario patch is being coordinated. Reviewed the current uncommitted architecture delta over 1797516, its new/existing tests, guide wording, implementation evidence, and original native acceptance. Read Gitea issue #1, including Codex-956atf's stale-submission finding in comment #12. No production files, Cargo commands or native input were used in this review.
Confirmed finding
P2 — stale submission hides the retained inline name.
crates/oa-desktop/src/app/architecture_authoring.rs:400–416 validates nonempty input, then sets saved.placing = true before calling commit_architecture. The commit guard at 447–460 rejects stale document/revision/diagram, dirty source/properties/behavior or invalid point, but it only sets the status string and returns. It does not restore placing = false or set the draft error. State::inline at 95–99 requires !placing, and inline_editor at 540–541 filters on that predicate.
Consequently the model remains unchanged and the bytes of the name remain in memory, but the name editor disappears on this rejection. The status says the name is retained while asking the user to cancel/restart. The old Edit action was removed in this delta; no direct repair action restores the editor. A later canvas click can reopen naming, but cannot repair the stale document/revision binding. This is a defect in the stale-context recovery path, not an observed ordinary native workflow failure or accepted-model corruption. Most normal navigation/model edits are already blocked during naming by app.rs:1075–1097.
Minimal correction: on the guarded rejection, retain all raw fields and set saved.placing = false plus error = Some(status.clone()), as the actual execution-failure branches already do at 474–477 and 534–536. Keep rejection of the stale commit; do not silently refresh its document/revision or accept it against another model. Ensure the retained editor can be read/copied/cancelled. This mirrors the separate-host finding but was independently confirmed in our current implementation.
Focused falsifier to add: for both named Create-at-point and F2 Rename, capture the raw name, accepted bundle and camera; invalidate the saved document/revision binding in the test fixture; send Event::Submit. Assert the bundle and camera are exact, no new history is created, the same raw name/point remains, inline() is true, and error is populated. Repeat Submit without repair and assert it still rejects. Cancel must remove only the draft. Restore ordinary naming afterward to ensure the failed draft does not poison the next operation. The existing stale test changes a Click event's revision (architecture_authoring_tests.rs:65–86), so it does not cover this Submit path. The four new tests cover blank Submit and invalid Click, not stale Submit.
Areas reviewed without another confirmed source defect
- The new F2 test drives a real Iced canvas event, requires canvas focus and verifies exact source/history plus unchanged placements/camera. The production Arc borrow fix is correct and does not alter selection identity.
- Named OA/SA allocated work is root-owned with a distinct allocation and placement in one Batch. New tests compare exact source bundles before/after Undo/Redo; no semantic-parent/allocation conflation was found.
- Nonfinite Click rejection precedes saved geometry, and State restoration also validates point finiteness, compatible type/performer and accepted fingerprint. Existing checkpoint tests retain raw names, phase and point with a fresh process binding. These safeguards should stay intact.
- Name length is bounded, whitespace-only Submit keeps the editor/error, and Escape cancels without accepting a placeholder. Choosing another creation tool is allowed only while the first tool is unnamed; named inputs are protected by the App gate.
- The new card height bound and scrollable wrapper directly address a long title/error overflowing the canvas. Source review does not prove native edge placement, focus, scrolling or expanded-label readability. There is no new measured layout assertion for this wrapper; retain root's minimum/wide native acceptance.
- Current guide text teaches side-palette creation, point then inline name/Enter, Escape, F2, explicit allocated work and palette connections. Those statements match the inspected architecture implementation. Scenario guide labels will need synchronization after the separate clicked-slot/layout work; that is outside this palette review.
Validation boundary
The upstream palette report records desktop588, GUI build, formatting and strict desktop Clippy. This reviewer did not rerun them. Root reports actual tool→point→inline→Enter with stable screen origin and one exact source Undo; that evidence belongs in root's native report. It does not exercise stale submission, long names at the lower/right edge, existing insertion/Locate, recovered inline focus or all OA/SA allocation cases.
Fix and run the focused stale-submission falsifier before claiming that rejection paths retain a usable inline editor. No other blocking finding was established in this bounded review. Scenario production remains unchanged pending root's source-coordination clearance.