Independent source review: rename Preview / Full details scroll

2026-09-13. Reviewer /root/source8_s2a_review, GPT-6 Astra. Source-only review; no compiler, Rust test execution, GUI, candidate edits, Git staging, publication, or Gitea changes. Root controls integration and acceptance. This ignored local report is not yet available to remote peers.

Reviewed sources and disposition

Initial candidate 022d8cda2ad218ef85a98cce2e2f80a0af71e073 atop 357da24542eaa519c2a01c465be6c7d1acbc8126: reviewed its complete two-file diff, surrounding rename controller/admission/reset code, shared App update path, actual mounted test harness, and installed Iced implementation. One P2 finding below required correction. Follow-up bebf9f594c6722200e95e92be2734acbea0dd700 was reviewed as an exact twelve-line addition atop022; clean worktree observed. That finding is resolved at source level in bebf. No remaining actionable source defect identified within this scope; suitable to proceed to compilation and runtime gates, not native acceptance.

Read author handoff .local/navigation-preview-details-scroll-handoff-20260913.md and prior native finding, and used previously independently inspected native440 frames043–045: returning from Full details moved the same preview from its deliberate nonzero offset back to the top. Those failed frames remain evidence of440; source correction does not change their result.

F1 — P2: accepted viewport reports cross shared source boundaries (022; corrected in bebf)

Proof: App::update in crates/oa-desktop/src/app.rs:1276 rejects stale rename envelopes, but022 then passes accepted PreviewScrolled through reveal/allocation/camera reconciliation and update_inner. The latter's source-interaction exclusions do not contain this new event; source_interaction() (source_symbols.rs:143) rotates the interaction ID and clears request/reveal/source_jump. After the event's single f32 assignment, update_inner calls refresh_source_rename_context (app.rs:1697 at022), which calls rename_context_matches and rename_admission (source_rename.rs:225–300). Healthy admission compares accepted and workspace source files and materializes active editor text. Thus every changed viewport report incurs document-dependent checks and changes unrelated source-interaction state. Before adding this callback, ordinary preview wheel motion was widget-local. No measured latency or observed data loss is claimed.

The risk also includes reports due to changed viewport geometry: Iced0.14.2 scrollable.rs:1084,1586–1634 notifies on redraw when bounds/content/relative/absolute offsets change; redundant identical viewports are suppressed. This is not limited to one report per explicit wheel gesture.

Repair: retain the envelope rejection, then consume only an accepted PreviewScrolled before shared boundaries. Keep one existing offset handler and return its empty task. Add an invariant to the actual-wheel regression that fails if a valid display report becomes a new source interaction.

Falsifier: if022 routed accepted reports before update_inner, or the interaction exclusions included them and refresh did not run, this finding would not hold. Direct source traversal disproves those alternatives. The new interaction equality assertion would fail on022 because a fresh UUID is unconditionally assigned by source_interaction; tests remain UNRUN, so this is source reasoning rather than an executed failure.

Follow-up disposition: bebf adds the narrow five-line early return immediately after the rename envelope and before diagram reveal/allocation/camera work. It calls the existing source_rename_event arm, whose only action is assigning the validated offset and returning Task::none. No second mutation implementation or broad bypass is introduced. Seven added regression lines compare source interaction, camera/allocation boundaries, and full existing evidence around actual wheel-report delivery. Other rename events retain their existing update/admission route. F1 is resolved in the reviewed source.

Other reviewed properties

The event envelope requires a live form, current invocation and approval, Preview visible, current destination visit, and finite nonnegative offset. Reports from hidden Details, prior approval, closed/reopened form, or prior visit fail before shared App boundaries. The callback captures those exact IDs rather than consulting a later form. At bebf valid measurements update display state only.
Details entry and return rotate the destination. Return captures the stored vertical offset in an absolute scroll_to aimed at the fresh preview ID, preserving file/hunk/line/segment and approval. Repeated visibility requests remain no-ops. A held operation from another visit cannot target the current widget; old reports are likewise rejected. This is the same explicit operation mechanism already used for paging resets, rather than relying on IDs to reset retained widget state.
Explicit changed file/hunk/line/segment/filter navigation and new approvals call reset_preview_position, now clearing the stored offset as well as issuing the existing START operation when visible. A new approval also clears the offset when Details is mounted. Input clears approval; any later valid replacement resets before use. Search with no match still resets the visit. No new path changes model, save, acceptance, or Undo representation.
Iced0.14.2 stores an absolute offset and resolves it against current content/viewport extent; preserving the same pixel position after a resize is bounded by the available range. The patch promises vertical retention only. Real wheel, scrollbar/touch, and geometry updates use the same Iced viewport notification path. No claim of unchanged horizontal state or impossible beyond-end offset is needed.
The mounted regression sends a real wheel event to the actual dialog Scrollable, requires event consumption and exactly one report, then records measured nonzero geometry and feeds that message through App.update. It exercises both a genuinely replaced Details subtree and a rapid round trip without the intermediate mount. The offset is measured immediately after the restore operation, before another scroll; that matters because the older round-trip test scrolled again and masked the native failure. It also rejects an old report while hidden/after return and checks an explicit file navigation returns to zero. Existing four mounted reset/held-operation tests remain.
The harness diffs and lays out the current widget tree before operating. Installed iced_runtime-0.14.0/src/user_interface.rs:539 operates against the built interface layout, and iced_winit-0.14.0/src/lib.rs:1742 dispatches widget tasks through that interface. These support the operation choice but do not establish real-window task/redraw ordering or final pixel appearance. Native rechecking remains necessary.

Limits and required next evidence

All five preview-scroll tests and the combined candidate remain UNRUN by this reviewer. Author-reported rustfmt/diff checks are not compiler evidence. Require an actual source-bound listing and five-test execution, appropriate related rename/modal/selection and full/strict checks under the parent's serialized gates, then native reproduction at a nonzero preview offset: Details -> Preview with unchanged proposal/file/hunk/page and visible offset, repeated visibility, explicit navigation reset, and Cancel/save preservation. Include a resize or changed viewport if making a claim beyond the original fixed-size workflow. The source review does not prove rendered scroll accuracy, event timing under real input, or performance latency.

The new tests are proportionate and expose the observed restoration bug plus F1; absence of exhaustive forged numeric-event cases is not treated as a blocker given the direct finite/nonnegative and identity checks. No settled paging, modal, rename grammar, acceptance, or focus behavior was redesigned. Relative to357, bebf affects three existing Rust files; one new test, expected scroll membership five, remains an expectation until the actual listing. Original440 receipts must not be relabeled as bebf.

Exact source bytes

SHA256 below is over git show COMMIT:PATH bytes, not working-tree newline conversion. The original controller file is unchanged by the twelve-line follow-up.

Pin	Path	Bytes	SHA256
`022d8cd`	`crates/oa-desktop/src/app.rs`	235017	`61a82339a4dffe60145141722d797338d02dd57ae43e41f0bc0777fbb650a3e3`
`022d8cd`	`crates/oa-desktop/src/app/source_rename.rs`	47140	`ae6cda4befaa6eaebbcb176f50a2da5f4ccd94940257c178d70dd3614bd79fbe`
`022d8cd`	`crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs`	18853	`161f2e156b3cc24b645eeb29879c6b2a5c030db9564c2194ae3fed19ec412fb9`
`bebf9f5`	`crates/oa-desktop/src/app.rs`	235342	`814747ebc9b9ffe7286a8efc8669eb8a9c578e2988de43326c02446f8fb94748`
`bebf9f5`	`crates/oa-desktop/src/app/source_rename.rs`	47140	`ae6cda4befaa6eaebbcb176f50a2da5f4ccd94940257c178d70dd3614bd79fbe`
`bebf9f5`	`crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs`	19224	`642d2f3f98e3ed88ac3d9285532867f7ab2d35327b5285935067ef4aefa6e2e9`

9.1 KiB Raw Blame History Unescape Escape