ARCADIA-Studio/harness/release-reference-chooser-production-review-20260911.md

Independent production reference chooser review — 2026-09-11

Reviewer: Codex-956atf /root/semantics_critic. Scope: Inspector and active semantic Review scalar live-reference chooser; read-only production inspection. The reviewer did not run Cargo, the desktop application, or native input, and did not edit production, prior reports, or Git state. Root owns integration and execution; behavior_audit owns corrections. Scenario FormShown and canvas scrolling are separate work.

Status: the three concrete P2 findings below are corrected at source-review level after independent read-back. No remaining source blocker was found in the assigned chooser slice. The corrected host suite is still uncompiled/unexecuted at this review; this is not production or native acceptance. Gitea issue 1 comments 209 and 219 record ownership and the initial findings.

Findings in the first production candidate

These locations refer to the formatted candidate inspected before the focus correction: forms/reference_chooser/trigger.rs SHA256 529C36791272346C166064C02BB659FD0BE6C2A76376CC6C5D440B6BA22495F7; app/reference_authoring.rs SHA256 A8FEA3D252C01C849CE5B0ADE4FBBB9D3CF3A34E58C062FBEBF152710377F8E8. Paths below are relative to crates/oa-desktop/src unless stated otherwise. They are source-derived falsifiers, not executed failure logs.

K1 — P2: keyboard focus can enter a clipped field that cannot open

forms/reference_chooser/trigger.rs:185 runs ordinary Iced focus traversal across all focusable descendants. FromField/SetFocus around lines 408–449 also select by traversal index alone. Neither reveals the selected field. The root Measure correctly clips reference anchors to nested viewport intersections at lines 357–391, and activation at line 236 publishes an Open only when such an anchor exists.

Minimal falsifier: put the reference trigger immediately below a 180-pixel form viewport; focus the last visible text input, press Tab, then Enter. The reference becomes focused below the viewport, but its anchor is None, so Enter publishes no chooser. Text inputs below the fold can likewise receive unseen typing. Escape/Tab return can select an offscreen neighbor. The original mounted scroll test snaps the reference into view before interaction, so it does not test this sequence.

The installed Iced 0.14 widget/operation/focusable.rs implements focus selection and traversal without using scroll geometry; there is no implicit reveal to complete this workflow. The author confirmed the defect. Resolution: select within the active form, minimally reveal the destination through its enclosing scrollables, and remeasure before activation; add a mounted test without a preparatory scroll.

K2 — P2: fresh/boundary Tab loses form entry and can reach an inactive host

The same scope branch handles Tab only if Count.focused is true (trigger.rs:185). Iced focus_next unfocuses the last item and focus_previous unfocuses the first. Subsequent Tab sees no focused child and does nothing; a newly mounted unfocused form cannot enter its first field through this handler. Custom chooser return at trigger.rs:430 also selects None beyond either boundary and clears all focus.

Minimal falsifiers: mount an unfocused form and press Tab; or focus its last field and press Tab twice; or close a chooser on its final field with Tab. No active field is recovered. Additionally, opening Review while Inspector retains focus leaves two mounted scopes: Iced opaque captures only mouse presses (iced_widget-0.14.2/src/helpers.rs:660–682), and Stack stops descending only after event capture (stack.rs:231–263). An unfocused Review can therefore let Tab reach the underlying Inspector. This is a real host-routing consequence of the same missing active entry policy, not a claim that an obsolete chooser passes the App's write guard.

The author confirmed the defect. Resolution: put keyboard routing at the stable root with the exact active form session, explicit first/last entry and boundary policy, and a distinction between a modal-owned Review form and an ordinary Inspector alongside other editable controls. Regression coverage must include an inactive mounted scope.

The correction must also prevent simultaneous inactive text focus: if modal entry focuses a Review reference trigger while an underlying Inspector text input stays focused, a character key unhandled by that trigger can pass through Stack/opaque into Inspector. Check retained Inspector raw text after modal entry, reference focus and character input, including chooser dismissal. Correct Tab routing alone does not prove general keyboard isolation.

Additional ordering case required by the correction

An asynchronous Escape/Choose focus Task can be created while Inspector is active, then run after Review or another modal opens. The same Inspector field and generation can remain mounted underneath that modal. A traversal that only proves target existence can still focus the inactive host. Require current root ownership as well as exact field/session/generation before applying return focus. The original stale-focus test replaces the generation, so its target disappears and does not establish this stronger condition. The author has received this concrete test case.

K3 — P2: the first focus correction disables a legitimate sibling Review input

The first correction's Popup::update calls own_input before every input while Review owns a Candidate form. Its focus::exclusive clears every focusable outside that form. This protects Inspector but also clears controls inside the same Review modal. A concrete existing control is semantic_review/view.rs:333, the required engineering rationale for explicit SA→OA technical-operation omission. It is mounted above facts_view, outside the Candidate detail scope around line 873.

Minimal falsifier: review such an Exchange occurrence in SA→OA direction, enable operation omission, select a Candidate facet, click its required rationale, then type a character. own_input unfocuses the rationale before dispatching the character, so its required text cannot be entered. The first correction's modal fixture contains only the reference trigger, so it cannot detect this regression. Resolution: distinguish the entire owning modal from the narrower field/form scope, clear only background focus, and add a mounted sibling-input regression. This finding was sent to the author and root at correction read-back before acceptance.

Source checks with no counterexample found

• app/reference_authoring.rs:52–89 checks Inspector versus exact active Review ownership and binds opening to document instance, project identity, document revision, page/layer, draft session, generation and live path. Unrelated modal, hosted, source-dirty, pending and busy states invalidate the chooser. This prevents the keyboard defects above from becoming authority to write an obsolete draft.
• semantic_review/batch.rs:299–365 requires the current active row and Facts tab, checks the frozen project, rejects unaddressed writes and stops Open/Event messages before normal update/planning. semantic_review/row.rs:529–545 requires the selected Candidate facet and authorized path roots, then resolves the correct scalar/group/attachment draft. The App validates again before mutation.
• forms/reference_chooser/adapter.rs derives candidates from the existing typed reference_ids, excludes nil and historical paths, retains invalid/missing/incompatible current text, and rechecks actual eligibility and optionality immediately before a scoped setter. Candidate metadata covers the actual Project::identities variants; the chooser does not introduce a second eligibility authority.
• The App's Outcome::Browsed branch at reference_authoring.rs:140–144 restores only transient chooser state; it does not call the Review planner or the draft setter. Only explicit Choose/Clear uses shape_scoped, preserving unrelated raw inputs and constructor caches. Apply, accepted Project/source and history remain their existing separate boundary.
• The popup is persistently mounted above the ordinary view/context-menu wrapper and below hosted UI. Browser input is captured; the controller blocks global save/undo/open/new/source-navigation/guide shortcuts while open, and Escape closes the picker before its owning Review. Absolute row actions are invocation/result stamped; relative navigation/query ordering is intentional. Press and release must match the same pointer/action/stamp before a row write.
• The original root measurement traversal matches installed Iced's Scrollable::operate call order: each scrollable reports its offset immediately before child traversal. It accumulates nested offsets and viewport clipping with stack restoration. No coordinate contradiction was found by source inspection. The prepared mounted test establishes only a single already-scrolled case when executed; nested/clipped/full-app behavior still needs evidence.

Evidence boundaries

The earlier 13/13 isolated chooser tests passed under root's test-only declaration and do not establish the new host adapter. The first production handoff adds six App tests and three retained-widget tests, initially unexecuted. They cover 1,000-entry eligibility, literal invalid sibling preservation, source/Project/Undo/Redo after Apply, invalid current/Clear, stale contexts, constructor/array changes, Review scalar/group/attachment behavior, banked rows and mounted opening/return/scrolling. The mounted renderer is a null text renderer with a purpose-built form, not the actual complete Inspector/Review view.

The correction must be compiled and its new focus/reveal tests executed by root, followed by the relevant existing forms/history/Review gates. Native acceptance remains necessary for actual font clipping, long details, 1100×700 layout, nested scrollers, modal priority, keyboard focus, gesture cancellation and measured 1,000-entry query/eligibility latency. This report does not turn authored tests, source reasoning, or the earlier isolated pass into production/native evidence.

Corrected-source read-back

Finding	Resolution independently inspected	Evidence still required
K1: clipped focus	trigger/focus.rs::Collect retains enclosing scroll geometry, chooses the exact form destination, computes bounded reveal from inner viewport to outer viewport, then applies scrolling and focus. Root measurement runs again before Enter activation.	Execute the prepared fresh offscreen, one/two nested scroll, Enter, and return-after-scrolling-away test. The test starts at zero scroll and checks the reference really begins below the fold.
K2: entry, boundaries and ownership	Root Popup receives the current App form session. Fresh Tab/Shift+Tab enter first/last; boundaries cycle. Ordinary Inspector traversal respects another focused control. Exact asynchronous return requires both current root ownership and the original generation-scoped field. Review deactivates background input before ordinary input, including pointer opening and Outside dismissal.	Execute the bidirectional cycles, still-mounted old Inspector under actual Review ownership, character non-leak and Outside/no-focus-task tests.
K3: sibling rationale	A separate layout-neutral trigger::modal boundary wraps the entire batch::State::view, including header, rationale, facts and footer. Ordinary-input cleanup preserves everything inside that modal; Candidate form identity remains the narrower explicit Tab/return scope.	Execute the added SA→OA operation-omission rationale test and confirm the actual full view in native use.

The final mounted rationale fixture loads a real operation-bearing SA occurrence, creates an OA counterpart, opens reverse Review, enables omission, and activates a Candidate parent facet. Its rationale is inside the modal but outside the Candidate form. It asserts every character emits the correct addressed rationale event, the final literal is Engineering reason, the rationale retains focus, and Candidate recovery, Inspector raw input, SourceBundle and Undo remain unchanged. The Inspector-under-Review and Outside fixtures likewise retain the actual old input and use real Stack/Opaque plus the reference adapter; they do not substitute a no-background modal.

The corrected host suite contains 14 prepared tests: six App tests and eight mounted tests. None was run by this reviewer or the author at handoff. The separate earlier isolated 13/13 remains exactly that earlier executed checkpoint. Installed Iced Winit source also confirms that synchronous App updates rebuild user interfaces before returned widget operations (iced_winit-0.14.0/src/lib.rs around 1214–1251), supporting the new-generation/root-owner return design; it does not replace runtime acceptance.

Root can proceed with the compiled integration gates against this corrected candidate. A compile/test failure or native focus/geometry failure must be resolved before treating these findings as execution-closed. No redesign of typed eligibility, raw staging, semantic planning, or source/history contracts is indicated by this review.

Final correction hashes were independently read from disk and match the author's final freeze. Paths remain relative to crates/oa-desktop/src:

File	SHA256
forms/reference_chooser/trigger.rs	4932CCE196DB20B7EC444C42AB01B3217D9B9AA0B9203D0E7466759B04722552
forms/reference_chooser/trigger/focus.rs	C59FC25096829B86632BE78979F54178D28653FB1E7447E5122FC5C1243456AE
forms/reference_chooser/widget.rs	D380A574F381C0AA6A5CEABC57F5879487623C816C120F2C3ADFE4C7737F03DC
app/reference_authoring.rs	81CFC1057082451C70E12DB3F5DCDE9FEC820F2B101592894B6CBAF132A5013C
app/reference_authoring/tests/mounted.rs	D50F89A1998DD6B37C841D63B33C1F72CD844FEB3CEA2F75C9294D3935416F9E
semantic_review/batch.rs	29F498265FC12C0B33B8186911ECC4D7C7DC806CC36BCBA51862436EAB1E12A1

These identify the inspected source before root formatting/build. They do not identify an executable. The author reports a clean git diff --check; the reviewer checked only the new review artifact for whitespace and preserved all frozen reports and production files.