Reset alias preview scroll for each explicit destination

crates/oa-desktop/src/app/source_rename.rs

@@ -111,6 +111,7 @@ struct Form {
     hunk: usize,
     line: usize,
     segment: usize,
+    preview_destination: Id,
     details: bool,
     detail_segment: usize,
     detail_editor: text_editor::Content,
@@ -311,6 +312,7 @@ impl App {
     }
 
     pub(super) fn source_rename_event(&mut self, event: Event) -> Task<Message> {
+        let mut reset_preview_scroll = false;
         match event {
             Event::Begin(_) => return self.begin_source_rename(),
             Event::Close(_) => self.close_source_rename(),
@@ -359,7 +361,11 @@ impl App {
             }
             Event::Search(_, value) => {
                 let form = self.source_rename.form.as_mut().unwrap();
+                if form.search == value {
+                    return Task::none();
+                }
                 form.search = value;
+                reset_preview_scroll = form.approval.is_some();
                 if let Some(approval) = &form.approval
                     && let Some(index) = approval
                         .preview
@@ -379,6 +385,7 @@ impl App {
             | Event::Segment(_, _, next) => {
                 let form = self.source_rename.form.as_mut().unwrap();
                 let ready = form.approval.as_ref().unwrap();
+                let previous_page = (form.file, form.hunk, form.line, form.segment);
                 match event {
                     Event::File(..) => {
                         let matching: Vec<_> = ready
@@ -414,6 +421,8 @@ impl App {
                     }
                     _ => form.segment = step(form.segment, segment_total(form), next),
                 }
+                reset_preview_scroll =
+                    previous_page != (form.file, form.hunk, form.line, form.segment);
             }
             Event::Details(_, value) => {
                 let form = self.source_rename.form.as_mut().unwrap();
@@ -441,6 +450,9 @@ impl App {
                 }
             }
         }
+        if reset_preview_scroll && let Some(form) = &mut self.source_rename.form {
+            return reset_preview_position(form);
+        }
         Task::none()
     }
 
@@ -492,6 +504,7 @@ impl App {
             hunk: 0,
             line: 0,
             segment: 0,
+            preview_destination: Id::new_v4(),
             details: false,
             detail_segment: 0,
             detail_editor: text_editor::Content::new(),
@@ -591,6 +604,7 @@ impl App {
         }
         let form = self.source_rename.form.as_mut().unwrap();
         form.pending = None;
+        let mut reset_preview_scroll = false;
         match completion.payload {
             Payload::Target(Ok(target)) => {
                 form.replacement = target.alias.clone();
@@ -619,6 +633,7 @@ impl App {
                     form.line = 0;
                     form.segment = 0;
                     form.search.clear();
+                    reset_preview_scroll = true;
                 }
             }
             Payload::Target(Err(error)) | Payload::Preview(Err(error)) => form.notice = error,
@@ -626,6 +641,9 @@ impl App {
         if form.details {
             refresh_details(form);
         }
+        if reset_preview_scroll {
+            return reset_preview_position(form);
+        }
         Task::none()
     }
 
@@ -920,6 +938,31 @@ fn input_id(invocation: Id) -> iced::widget::Id {
     iced::widget::Id::from(format!("source-rename-{invocation}"))
 }
 
+// Each navigation gets a fresh destination even when returning to the same page
+// or changing a filter that leaves the page tuple unchanged (the ABA case).
+fn reset_preview_position(form: &mut Form) -> Task<Message> {
+    form.preview_destination = Id::new_v4();
+    if !form.details
+        && let Some(ready) = &form.approval
+    {
+        iced::widget::operation::snap_to(
+            preview_scroll_id(form, ready.id),
+            iced::widget::scrollable::RelativeOffset::START,
+        )
+    } else {
+        Task::none()
+    }
+}
+
+// The operation targets this visit, not a later page, visit or approval.
+// A new widget id alone does not reset Iced's retained Scrollable state.
+fn preview_scroll_id(form: &Form, approval: Id) -> iced::widget::Id {
+    iced::widget::Id::from(format!(
+        "source-rename-diff-{}-{approval}-{}-{}-{}-{}-{}",
+        form.invocation, form.preview_destination, form.file, form.hunk, form.line, form.segment
+    ))
+}
+
 /// Complete indexed preview with bounded rendering: eight admitted source rows
 /// and one UTF-8 segment per side at a time. No candidate content is discarded.
 pub(crate) fn dialog(app: &App) -> Element<'_, Message> {
@@ -1064,7 +1107,7 @@ where
             pager(format!("Row segments {}/{} · both sides", form.segment + 1, segments.max(1)),
                 (form.segment > 0).then(|| message(Event::Segment(invocation, preview_id, false))),
                 (form.segment + 1 < segments).then(|| message(Event::Segment(invocation, preview_id, true)))),
-            scrollable(displayed).id("source-rename-diff").height(Fill).width(Fill).spacing(8),
+            scrollable(displayed).id(preview_scroll_id(form, preview_id)).height(Fill).width(Fill).spacing(8),
         ].spacing(6).height(Fill).into()
     } else {
         scrollable(column![

crates/oa-desktop/src/app/source_rename/preview_glyph_tests.rs

@@ -152,6 +152,7 @@ impl shaped::Renderer for GlyphRecorder {
 
 #[derive(Default)]
 struct ScrollGeometry {
+    id: Option<widget::Id>,
     bounds: Option<Rectangle>,
     content: Option<Rectangle>,
     offset: Vector,
@@ -168,7 +169,7 @@ impl widget::Operation for ScrollGeometry {
         offset: Vector,
         _: &mut dyn widget::operation::Scrollable,
     ) {
-        if id == Some(&widget::Id::new("source-rename-diff")) {
+        if id == self.id.as_ref() {
             self.bounds = Some(bounds);
             self.content = Some(content);
             self.offset = offset;
@@ -262,7 +263,10 @@ fn assert_page_glyphs_visible(app: &App, host: Size) {
     let node =
         view.as_widget_mut()
             .layout(&mut tree, &renderer, &layout::Limits::new(Size::ZERO, host));
-    let mut geometry = ScrollGeometry::default();
+    let mut geometry = ScrollGeometry {
+        id: Some(preview_scroll_id(form, ready.id)),
+        ..ScrollGeometry::default()
+    };
     view.as_widget_mut()
         .operate(&mut tree, Layout::new(&node), &renderer, &mut geometry);
     let bounds = geometry.bounds.unwrap();
@@ -279,7 +283,7 @@ fn assert_page_glyphs_visible(app: &App, host: Size) {
             Layout::new(&node),
             &renderer,
             &mut widget::operation::scrollable::scroll_to(
-                widget::Id::new("source-rename-diff"),
+                preview_scroll_id(form, ready.id),
                 widget::operation::scrollable::AbsoluteOffset {
                     x: None,
                     y: Some(offset),

crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs

@@ -0,0 +1,310 @@
+//! Execute the controller's scroll operations on a retained, scrolled dialog.
+use super::*;
+use crate::views::source_tests::FontLayout;
+use iced::advanced::{Layout, layout, widget};
+use iced::futures::StreamExt;
+use iced::{Rectangle, Size, Vector};
+
+const HOST: Size = Size::new(720., 660.);
+
+fn paged_fixture() -> App {
+    let mut source = bundle();
+    let name = "Long engineering name 雨 ".repeat(18);
+    source.files.get_mut("defs.arcadia").unwrap().replace_range(
+        ..,
+        &format!(
+            "oa_activity pump \"{name}\" id \"00000000-0000-0000-0000-000000000002\" {{}}\r\n\
+            oa_activity backup \"Backup\" id \"00000000-0000-0000-0000-000000000004\" {{}}\r\n"
+        ),
+    );
+    let mut uses = String::new();
+    for index in 0..16 {
+        if index == 12 {
+            uses.push_str(&"// unchanged gap\r\n".repeat(10));
+        }
+        uses.push_str(&format!(
+            "interaction use_{index} \"{name}\" id \"00000000-0000-0000-0001-{index:012}\" {{ source @pump; target @backup; }}\r\n"
+        ));
+    }
+    source.files.insert("uses.arcadia".into(), uses);
+    let mut app = fixture();
+    app.window_size = HOST;
+    app.document = Document::from_bundle(source, LIMITS).unwrap();
+    app.source_workspace = Workspace::new(app.document.bundle(), true);
+    app.source_workspace.active = "uses.arcadia".into();
+    app.show_source_file(false);
+    let start = app.source_workspace.text().find("source @pump").unwrap() + "source @".len();
+    app.source_editor
+        .move_to(cursor(app.source_workspace.text(), start, start + 4));
+    begin(&mut app);
+    input(&mut app, "pump_long");
+    let _ = preview(&mut app);
+    app
+}
+
+fn scroll_id(app: &App) -> widget::Id {
+    let form = app.source_rename.form.as_ref().unwrap();
+    preview_scroll_id(form, form.approval.as_ref().unwrap().id)
+}
+
+struct Geometry {
+    id: widget::Id,
+    offset: Option<Vector>,
+}
+impl widget::Operation for Geometry {
+    fn traverse(&mut self, visit: &mut dyn FnMut(&mut dyn widget::Operation)) {
+        visit(self);
+    }
+    fn scrollable(
+        &mut self,
+        id: Option<&widget::Id>,
+        _: Rectangle,
+        _: Rectangle,
+        offset: Vector,
+        _: &mut dyn widget::operation::Scrollable,
+    ) {
+        if id == Some(&self.id) {
+            self.offset = Some(offset);
+        }
+    }
+}
+
+struct Mounted {
+    editor: text_editor::Content<FontLayout>,
+    tree: widget::Tree,
+}
+impl Mounted {
+    fn new(app: &App) -> Self {
+        let editor = text_editor::Content::<FontLayout>::new();
+        let tree = widget::Tree::new(dialog_content(app, &editor).as_widget());
+        Self { editor, tree }
+    }
+    fn operate(&mut self, app: &App, operation: &mut dyn widget::Operation) {
+        let mut view = dialog_content(app, &self.editor);
+        // Preserve the actual scroll state across the new page's view diff.
+        self.tree.diff(view.as_widget());
+        let node = view.as_widget_mut().layout(
+            &mut self.tree,
+            &FontLayout,
+            &layout::Limits::new(Size::ZERO, HOST),
+        );
+        view.as_widget_mut()
+            .operate(&mut self.tree, Layout::new(&node), &FontLayout, operation);
+    }
+    fn offset(&mut self, app: &App) -> f32 {
+        let mut geometry = Geometry {
+            id: scroll_id(app),
+            offset: None,
+        };
+        self.operate(app, &mut geometry);
+        geometry.offset.expect("mounted preview scrollable").y
+    }
+    fn scroll_down(&mut self, app: &App) -> f32 {
+        self.operate(
+            app,
+            &mut widget::operation::scrollable::scroll_to(
+                scroll_id(app),
+                widget::operation::scrollable::AbsoluteOffset {
+                    x: None,
+                    y: Some(10_000.),
+                },
+            ),
+        );
+        let offset = self.offset(app);
+        assert!(offset > 10., "fixture must really be scrolled");
+        offset
+    }
+    async fn execute(&mut self, app: &App, task: Task<Message>) -> usize {
+        let mut count = 0;
+        if let Some(mut actions) = iced_runtime::task::into_stream(task) {
+            while let Some(action) = actions.next().await {
+                let iced_runtime::Action::Widget(mut operation) = action else {
+                    panic!("preview paging must only schedule a widget operation");
+                };
+                self.operate(app, operation.as_mut());
+                count += 1;
+            }
+        }
+        count
+    }
+}
+
+#[tokio::test]
+async fn explicit_navigation_resets_retained_scroll_but_id_change_alone_does_not() {
+    let mut app = paged_fixture();
+    let before = evidence(&app);
+    let form = app.source_rename.form.as_ref().unwrap();
+    let invocation = form.invocation;
+    let approval = form.approval.as_ref().unwrap().id;
+    let mut mounted = Mounted::new(&app);
+    mounted.scroll_down(&app);
+    let old_id = scroll_id(&app);
+    let task = app.update(Message::SourceRename(Event::File(
+        invocation, approval, true,
+    )));
+    assert_ne!(old_id, scroll_id(&app));
+    // Negative control: even after diff/layout with the new id and new file,
+    // Iced retains the previous offset until the returned operation executes.
+    assert!(mounted.offset(&app) > 10.);
+    assert_eq!(mounted.execute(&app, task).await, 1);
+    assert_eq!(mounted.offset(&app), 0.);
+
+    for navigation in [
+        Event::Line(invocation, approval, true),
+        Event::Segment(invocation, approval, true),
+        Event::Hunk(invocation, approval, true),
+        Event::File(invocation, approval, false),
+        Event::Search(invocation, "uses".into()),
+        Event::Search(invocation, "no-path-match".into()),
+        Event::Search(invocation, String::new()),
+    ] {
+        mounted.scroll_down(&app);
+        let task = app.update(Message::SourceRename(navigation));
+        assert_eq!(mounted.execute(&app, task).await, 1);
+        assert_eq!(mounted.offset(&app), 0.);
+        assert_eq!(evidence(&app), before);
+    }
+    // Passive redraw and unchanged filter input preserve deliberate scrolling.
+    let offset = mounted.scroll_down(&app);
+    assert_eq!(mounted.offset(&app), offset);
+    let task = app.update(Message::SourceRename(Event::Search(
+        invocation,
+        String::new(),
+    )));
+    assert_eq!(mounted.execute(&app, task).await, 0);
+    assert_eq!(mounted.offset(&app), offset);
+    let task = app.update(Message::SourceRename(Event::Input(
+        invocation,
+        "pump_long".into(),
+    )));
+    assert_eq!(mounted.execute(&app, task).await, 0);
+    assert_eq!(mounted.offset(&app), offset);
+    let task = app.update(Message::SourceRename(Event::File(
+        invocation, approval, false,
+    )));
+    assert_eq!(
+        mounted.execute(&app, task).await,
+        0,
+        "clamped first-page request is a no-op"
+    );
+    assert_eq!(mounted.offset(&app), offset);
+    assert_eq!(evidence(&app), before);
+}
+
+#[tokio::test]
+async fn delayed_page_and_approval_operations_cannot_reset_a_new_destination() {
+    let mut app = paged_fixture();
+    let before = evidence(&app);
+    let form = app.source_rename.form.as_ref().unwrap();
+    let invocation = form.invocation;
+    let old_approval = form.approval.as_ref().unwrap().id;
+    let mut mounted = Mounted::new(&app);
+    let old_page_task = app.update(Message::SourceRename(Event::File(
+        invocation,
+        old_approval,
+        true,
+    )));
+    let next_page_task = app.update(Message::SourceRename(Event::Line(
+        invocation,
+        old_approval,
+        true,
+    )));
+    assert_eq!(mounted.execute(&app, next_page_task).await, 1);
+    let offset = mounted.scroll_down(&app);
+    assert_eq!(mounted.execute(&app, old_page_task).await, 1);
+    assert_eq!(mounted.offset(&app), offset, "old page target is absent");
+
+    let old_approval_task = app.update(Message::SourceRename(Event::Search(
+        invocation,
+        "defs".into(),
+    )));
+    input(&mut app, "pump_new");
+    let _ = preview(&mut app); // Real new approval in the same invocation/page.
+    assert_ne!(
+        app.source_rename
+            .form
+            .as_ref()
+            .unwrap()
+            .approval
+            .as_ref()
+            .unwrap()
+            .id,
+        old_approval
+    );
+    let offset = mounted.scroll_down(&app);
+    assert_eq!(mounted.execute(&app, old_approval_task).await, 1);
+    assert_eq!(
+        mounted.offset(&app),
+        offset,
+        "old approval target is absent"
+    );
+    let task = app.update(Message::SourceRename(Event::File(
+        invocation,
+        old_approval,
+        true,
+    )));
+    assert_eq!(
+        mounted.execute(&app, task).await,
+        0,
+        "stale envelope is rejected before scheduling"
+    );
+    assert_eq!(mounted.offset(&app), offset);
+    assert_eq!(evidence(&app), before);
+}
+
+#[tokio::test]
+async fn repeated_page_visits_filters_and_fast_preview_completion_have_distinct_destinations() {
+    let mut app = paged_fixture();
+    let before = evidence(&app);
+    let form = app.source_rename.form.as_ref().unwrap();
+    let invocation = form.invocation;
+    let approval = form.approval.as_ref().unwrap().id;
+    let mut mounted = Mounted::new(&app);
+
+    let first_visit = app.update(Message::SourceRename(Event::File(
+        invocation, approval, true,
+    )));
+    let next_page = app.update(Message::SourceRename(Event::Line(
+        invocation, approval, true,
+    )));
+    assert_eq!(mounted.execute(&app, next_page).await, 1);
+    let return_visit = app.update(Message::SourceRename(Event::Line(
+        invocation, approval, false,
+    )));
+    assert_eq!(mounted.execute(&app, return_visit).await, 1);
+    let offset = mounted.scroll_down(&app);
+    assert_eq!(mounted.execute(&app, first_visit).await, 1);
+    assert_eq!(
+        mounted.offset(&app),
+        offset,
+        "old visit to the identical page is stale"
+    );
+
+    let old_filter = app.update(Message::SourceRename(Event::Search(
+        invocation,
+        "uses".into(),
+    )));
+    let new_filter = app.update(Message::SourceRename(Event::Search(
+        invocation,
+        "us".into(),
+    )));
+    assert_eq!(mounted.execute(&app, new_filter).await, 1);
+    let offset = mounted.scroll_down(&app);
+    assert_eq!(mounted.execute(&app, old_filter).await, 1);
+    assert_eq!(
+        mounted.offset(&app),
+        offset,
+        "old filter on the same page is stale"
+    );
+
+    input(&mut app, "pump_new");
+    let job = start_preview(&mut app);
+    let completion = preview_completion(&app, job);
+    // No intermediate frame mounts the temporary no-approval view. The old
+    // Scrollable survives even though the new approval has a different id.
+    let ready = app.update(Message::SourceRenameReady(completion));
+    assert!(mounted.offset(&app) > 10.);
+    assert_eq!(mounted.execute(&app, ready).await, 1);
+    assert_eq!(mounted.offset(&app), 0.);
+    assert_eq!(evidence(&app), before);
+}

crates/oa-desktop/src/app/source_rename/tests.rs

@@ -6,6 +6,10 @@ use std::collections::BTreeMap;
 #[path = "preview_glyph_tests.rs"]
 mod preview_glyph_tests;
 
+#[cfg(debug_assertions)]
+#[path = "preview_scroll_tests.rs"]
+mod preview_scroll_tests;
+
 fn bundle() -> SourceBundle {
     SourceBundle { entry: "project.arcadia".into(), files: BTreeMap::from([
         ("project.arcadia".into(), concat!("arcadia 7;\r\nproject model \"救援\" id \"00000000-0000-0000-0000-000000000001\" {\r\n",
@@ -976,6 +980,7 @@ fn actual_dialog_real_paragraph_geometry_keeps_input_preview_and_footer_reachabl
     use iced::advanced::{Layout, layout, widget};
     #[derive(Default)]
     struct Bounds {
+        diff_id: Option<widget::Id>,
         input: Option<iced::Rectangle>,
         preview: Option<iced::Rectangle>,
         footer: Option<iced::Rectangle>,
@@ -1004,7 +1009,7 @@ fn actual_dialog_real_paragraph_geometry_keeps_input_preview_and_footer_reachabl
             _: iced::Vector,
             _: &mut dyn widget::operation::Scrollable,
         ) {
-            if id == Some(&widget::Id::new("source-rename-diff")) {
+            if id == self.diff_id.as_ref() {
                 self.diff = Some(bounds);
             }
         }
@@ -1042,7 +1047,11 @@ fn actual_dialog_real_paragraph_geometry_keeps_input_preview_and_footer_reachabl
                 &FontLayout,
                 &layout::Limits::new(iced::Size::ZERO, host),
             );
-            let mut bounds = Bounds::default();
+            let form = app.source_rename.form.as_ref().unwrap();
+            let mut bounds = Bounds {
+                diff_id: Some(preview_scroll_id(form, form.approval.as_ref().unwrap().id)),
+                ..Bounds::default()
+            };
             view.as_widget_mut()
                 .operate(&mut tree, Layout::new(&node), &FontLayout, &mut bounds);
             let input = bounds.input.unwrap();