8.6 KiB
Scenario parent-focus correction — independent source review
Codex-956atf /root/focus_review, 2026-09-11. No unresolved material source finding in the bounded I1/I2 correction. Runtime acceptance remains pending. This reviews the working patch over HEAD 8d55d2d2eb05aed826b96a917cf776e1d8c3b690, frozen with author scenario_focus_fix, including the final test-only strengthening below. The pre-reboot integration review remains unchanged history. See the implementation handoff.
Read AGENTS, current Gitea issue 1 coordination, empty open PR inventory, BehaviorAuthoring20260911 and ContextualSourceDrafts20260911 Wiki pages. Review ownership was posted at issue 1/comment 302. Only this new report was written. No production edits, Cargo, rustc, rustfmt, tests, renderer creation, native UI, install or commit occurred. git diff --check passed for tracked working changes; it is not compilation or runtime evidence.
Findings and resolution
I1, previous P2 — correction supported by source. scenario.rs:317, :490, :669 and :710 now request a fresh parent mount on successful Escape/Back/Use return. This changes only form_mount and form_focus_pending; form_session remains stable. Source staging and its existing admission/validation are retained. In workspace.rs:362 the Name ID includes the current stamp; at :642 the parent-only sensor is keyed by mount, while editor_panel remains keyed by editing session. The child still replaces the parent branch. An ordinary parent whose initial focus has already been consumed can therefore publish a fresh FormShown after the child returns. Raw semantic identities are not rotated to obtain focus.
I2, previous P2 — correction supported by source. App::view (app.rs:1029) wraps the final reference/hosted content with the scenario owner marker. scenario_focus_owner (:1387) computes the live document/scenario/revision/session/mount stamp and excludes busy, hosted, workflow, architecture, model-menu, file-prompt, deletion and pending-confirmation ownership. The marker is published from the current view on every operation traversal, rather than retained widget state. FormShown checks the context and consumes the exact mount before constructing the guarded task (:1410). Geometry epoch is deliberately absent from the stamp, so a same-mount viewport notification does not invalidate first focus.
focus.rs:84–119 collects without changing any focusable or scrollable. A mismatched owner or missing exact Name target ends with Outcome::None. Only a valid target yields Apply, initially invalid; Apply rechecks the current root owner before changing focus or scroll (:129–145). Thus a saved S1 action operated after replacement, child opening, same-session remount, document/owner/revision change or modal ownership change cannot act on S2 or clear another field. The scoped Name ID also prevents the former global-ID collision.
R1, P2 test weakness — corrected during review, execution pending. The initial reveal test scrolled to 1000 and asserted only a final offset at most 64. Without proving actual overflow, that sequence could pass with a broken reveal. Author changed the production viewport to 1100×700, requires measured pre-focus offset greater than 64, and measures the Name's effective bounds inside the actual form viewport afterward (workspace/tests.rs:217). Existing raw/history assertions remain. No claim is made that this stronger fixture has passed; missing overflow will now fail explicitly. Author also added a direct chained-Apply owner-change falsifier (:602) after review feedback.
Prepared regression scope
Five scenario_form_focus_* tests are present in workspace/tests.rs; four use the production headless renderer and actual App::view. Backend creation uses expect, with no absent-renderer/GPU skip. Tests remain under the existing debug-test module gate.
- The current-focus test retains stale-dispatch, repeated-dispatch, busy/hosted, epoch-only-change, raw snapshot, accepted-Project pointer and history assertions; it now executes the operation chain on the mounted production view and checks real reveal geometry.
- The return test (
:507) first runs and consumes ordinary initial focus, mounts and focuses the actual source Name, then uses Back, valid Use and Escape independently. It checks stable session, changed mount, retained raw name/invocation/owner/slot/source-origin fields, focused parent, unchanged accepted Project/source bundle/history, and an unchanged returned recovery snapshot after focus. Use intentionally stages its source; the focus step must preserve that resulting snapshot. - The saved-action matrix (
:553) extracts S1's actual Widget action before eight live-state changes: replacement session, open source child, real Review candidate modal, document token, document revision, owner, same-session source-return mount, and hosted visibility. It operates on the rebuilt currentApp::view, preserving the focused child/Review or other actual input, source bundle, recovery state and Undo/Redo availability. This exercises delayed execution, not merely rejection of stale FormShown messages. - The chained-Apply test (
:602) successfully collects on the current parent, saves its Chain result, mounts/focuses a real Review candidate, and applies the saved operation there. It requires unchanged focus, recovery/source and history. - Missing exact target (
:626) deliberately uses a small generic marker/input tree with a valid stamp and an unrelated focused input. This is a narrow missing-target test, not claimed to mount the entire App.
focus_action extracts the first Widget action without waiting for task-stream completion while that action retains the channel. The execution helper follows Outcome::Chain. This matches the relevant synchronous chain behavior in the locally read Iced 0.14 runtime; no event-loop execution is claimed.
Scroll, chooser and integration boundaries
Compared behavior/scenario/focus.rs with forms/reference_chooser/trigger/focus.rs. Both translate bounds by accumulated ancestor offsets, restore traversal state when leaving ancestors, calculate reveal from inner to outer scrollables, clamp shifts to content capacity, and apply shifts by traversal index. Local Iced Scrollable::operate reports its translation before traversing content, matching that calculation. Scenario Apply focuses the exact stamped Name; chooser focus has separate form scope/relative navigation/modal policy. Their root marker types are distinct. No concrete sign, ancestor-scope or retained-child conflict was found in this readback; native pixels and broader nested-scroll behavior remain execution work.
The production test helper calls App::view's widget operation traversal. It does not invoke Iced UserInterface's additional overlay traversal. reference_chooser::Popup::operate (widget.rs:252) traverses its content, while its separate search input belongs to the overlay operation (:484). This report does not claim generic content traversal reaches that separate input. Existing child-open focus tasks and other focus systems were not redesigned or exhaustively audited.
Root owns scoped formatting, compilation, all five focus gates, related workspace/chooser/contextual-source tests, broader checks, matching GUI and native acceptance. Canvas integration must preserve the outer live owner marker, stamped Name, parent-only mount sensor and session/mount distinction. Root/author should link this review from their shared Wiki evidence; source review alone does not close the release or replace the frozen pre-reboot report.
Reviewed byte fingerprints
SHA-256, actual pre-format freeze. Paths are relative to crates/oa-desktop/src. Formatting or further changes require fresh gate provenance.
| File | SHA-256 |
|---|---|
app.rs |
4B700DBC6F8C0E8258B0F409C0B8F28266838F66ADAEABBEACE6B37E6DCAF736 |
behavior/scenario.rs |
6E26E0EDD44D11D473B67E9DFE634185D77AE080E26776E9F55230D587ABA083 |
behavior/scenario/workspace.rs |
D58CD2EA3F4603801BBD268906141076C5F70A83FB87A102B19A8806B083DEAE |
behavior/scenario/focus.rs |
48E1F227527E66CF0B56D75B23E05A1F4F89C3D0681760BE471D3E31362F2FBA |
behavior/scenario/workspace/tests.rs |
CBA10D3A5C2EC0DF9E5FA67EA2B2570412DACE45BBD1D351431CC1579970C56D |
forms/reference_chooser/trigger/focus.rs |
D489FA00F445BC603B3326CAD8721E2B608456ECF543954538904BF188C6C5FA |
forms/reference_chooser/widget.rs |
D66CC17AF7A48BB8598BE683F084BB08FA7AF7304F9636F0A1BEA94C5CF6C727 |