10 KiB
Shared behavior canvas corrections — independent source review
Codex-956atf /root/canvas_review, 2026-09-11. Three unresolved P2 findings in the correction freeze. Do not integrate as accepted functionality yet. The original stale-pan cancellation and missing keyboard widgets are substantially addressed, but toolbar admission and the verification helper still need correction.
Reviewed stopped isolated codex/behavior-canvas-20260911, base e511c4153fb5bb484652434340e0e4c7000c93d0, against the first independent review. Correction handoff: isolated harness/release-behavior-canvas-phase1-corrections-20260911.md, SHA256 3949C3774C244E04D14DD5C017711D729AEE53F85A707F4A0F4B1BDA7FDE2204; author reports ordered manifest 2EF8CED44BF26C43CEEDC296EBDF8A0E6573EB07424B106399D58DBF2E2A8CD5. Independently rehashed all 19 listed Rust files: every actual hash matches the report. The ordered manifest is an author-supplied identifier; the 19 individual matches are this review's direct observation.
Read issue 2 through comment 360, current Wiki and empty open-PR inventory. Review ownership is comment 361. Only this new root report was written. No production edits, compiler/rustc, tests, renderer/native execution, WSL/Linux checks, networking changes, Git mutation or delegation occurred. No previous runtime result is transferred to these bytes. All failures below are inferred from actual code paths and installed Iced primary source; none is an executed failure log. Locations below are relative to crates/oa-desktop/src in the isolated freeze.
R1 — P2: scenario rejects a real toolbar action after its own batched pan
app.rs:1530 still requires exact scenario_context() == context before handling scenario Fit, Choose, Focus and Shelf. Process has the special toolbar admission at app/process_canvas.rs:93–104; scenario does not.
Reproducer: start a middle drag, retain middle, then move onto Fit and left-click it in a batch containing CursorMoved, ButtonPressed and ButtonReleased. Viewport emits a nonzero Pan for the movement; Control/Button emits the Fit Zoom with that same view's semantic context. App applies Pan and increments canvas_epoch, then rejects Zoom at line 1530. The click therefore fails to Fit. A tool or panel action has the same ordering problem.
The prepared production_scenario_camera_external_controls_bounds_and_retained_deltas_cancel_old_drags at scenario/workspace/tests.rs:788 already includes this sequence and requires pan to become ZERO after Fit. Keep this assertion unchanged. The correction report's claim that both toolbar paths accept their own uninterrupted camera batch is not supported by the scenario adapter.
Correction must also satisfy R2; copying the current process epoch-range exception alone would be insufficient.
R2 — P2: toolbar intents are not stamped with the presentation lifetime
Camera packets carry the new camera_epoch, but toolbar Zoom/Choose/Focus/Shelf intents still carry only semantic Context. Process admission at app/process_canvas.rs:97–104 accepts an exact semantic epoch or camera_toolbar_epoch; the latter (app.rs:1109–1111) compares only the inclusive semantic-epoch range. Scenario's exact-epoch arm has the same missing-lifetime problem when an external boundary does not advance semantic epoch.
Concrete retained-intent falsifier for either owner: capture a clean toolbar Choose or Zoom; open the real Inspector reference chooser; close it with Escape without writing a field; replay the retained toolbar intent. app/reference_authoring.rs:106–152 changes chooser ownership without changing canvas_epoch. The new App wrapper correctly increments camera_epoch on both ownership transitions, but camera_boundary_epoch remains equal to the old semantic epoch. The old toolbar Context therefore still passes exact equality (and the process range), despite belonging to the previous camera lifetime. Dirty-to-clean or busy-to-ready transitions with unchanged semantic epoch have the same distinction.
Use a presentation-lifetime stamp on toolbar intent, validate it before the action, and allow semantic epoch differences only from that same still-valid camera lifetime. Preserve admission for R1's own ordered pan; reject retained toolbar packets across a real chooser cycle as well as Fit, tool, caller and bounds boundaries. Do not treat the absence of a source/history write as proof of correct input ownership.
R3 — P2: mounted Task execution differs materially from native Iced
canvas_workspace/tests.rs:101–111 runs App::update(message) and immediately awaits its Task before handling the next message in the same input batch. Installed iced_winit-0.14.0/src/lib.rs:1300–1335 drains all App messages and collects ready actions first; lines 1214–1251 rebuild the final UI and then execute those actions.
This changes the result of a concrete production path: Control publishes ControlFocus on press (canvas_workspace/control.rs:167), then Button publishes the toolbar action on release. For a same-batch click, native Iced applies the toolbar action and advances camera lifetime before the earlier focus Task runs. Collect's old lifetime then has no live members. The helper instead applies focus before that lifetime change. Thus it can preserve pointer-control focus which the real same-batch path loses. Add a runtime-equivalent same-batch pointer activation → Tab assertion; require the intended final focus, rather than relying on preparatory F6.
Also, perform at lines 185–195 only calls root operate. Installed iced_runtime-0.14.0/src/user_interface.rs:539–578 applies operations to the root and nested overlays. Updating/drawing an overlay in other helper methods does not make its search input participate in a focus operation. The chooser/Review regression at topology/layout_tests.rs:531 therefore cannot establish the complete claimed overlay focus behavior.
Correct batching and operation traversal together, preferably using Iced UserInterface/Cache for the tested behavior. Retain every original source/history/geometry assertion and fail on unavailable backends. The helper should execute the operations on the final App view exactly as the runtime does; fixes must not weaken fixtures to accommodate the current helper.
Supported corrections and limits
- The new App boundary compares semantic epoch, document/revision/owner/page/window, editing/chooser/modal availability and camera presentation. Non-camera changes end the presentation lifetime; Camera's own ordered deltas retain it. Both Camera adapters explicitly validate that lifetime and clean ownership. Viewport now cancels pan/Space on lifetime changes, disabled input, changed layout bounds and different/outside button presses. These paths address the original retained-pan defect, subject to R1/R2 and execution.
- Control registers a real focusable target, handles Enter/Space, and draws a dark two-pixel border. F6 provides deliberate canvas entry; Tab/Shift+Tab cycle live members; Escape/F6 leave. Collect requires an enabled current-lifetime viewport before focusing or clearing another field. Tool activation preserves the control's stable ID. This is an actual C2 implementation, not merely new labels around pointer buttons.
- Palette reveal uses unscrolled member geometry to calculate an absolute Scrollable offset, matching installed
Scrollable::operategeometry. No sign error was found. The prepared keyboard tests target OR/Action and Fit without asserting that a target initially lies below an overflowing palette. Add an explicit overflowing-last-tool reveal falsifier before claiming offscreen keyboard access is verified. - The pixel helper now draws App::view and an overlay into WGPU and reads screenshot bytes.
assert_drawn_focusmeasures changed border pixels at the selected tool and Fit. This can establish that those border pixels change when run; it does not establish transformed scene clipping, visible operand-refusal text, native active-button contrast, accessibility or all popup geometry. No pixel execution occurred. Preserve the missing-backendexpect. - Focus Apply (
canvas_workspace/focus.rs:106–145) itself has no lifetime/owner recheck after Collect. A deliberately retained Chain applied after opening Review would clear other focus. Installed Iced executes Chain passes synchronously (iced_winitlines 1742–1759), so this review does not claim a demonstrated native interleaving between those passes. The current test delays the initial Task, not a retained Apply. If the stricter retained-Apply invariant used by root's I1/I2 tests is required for this group, it needs its own guarded test/correction. - Shared
behavior/form_session.rsmatches root's authorizedF5027FB3A5A5599A182E7D1A10DBA56DD66EB1F23A4BA4C4A1744C9A6A5B6E61exactly. Both editor paths use it with their existing session/invocation. No duplicate implementation or new model/source persistence was introduced.
Handoff
Root should correct R1/R2/R3 before integration, then review the resulting source and run coherent Windows gates under its sole execution lease. There are 13 prepared production App-root tests plus two shared tests; they remain uncompiled/unexecuted here. Root's earlier passing scenario/chooser/workspace gates identify its other freeze, not this correction.
Mandatory later union remains root's I1/I2 live root marker, session-versus-mount Name/FormShown identity, Back/Use/Escape rearm, current zero-axis focus correction, five parent-focus tests and scenario replacement-scroll test. Their deliberate absence here is not a new finding. Keep one shared form_session module. B4 remains ytr83r-owned; transform both its clip rectangle and clipped child text by camera pan plus scale and preserve its oracles.
Linux builds/checks remain explicitly deferred until the user says networking is resolved. Persistent process placement/lane arrangement, source round trips, broader domain manipulation and full issue #2/PROJECT_GOAL acceptance remain outside this source review. Root owns the common Wiki/ledgers and publication.