From d114a271c5ecffcd7960fb8aa44d45989c10dc33 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Fri, 11 Sep 2026 01:21:24 -0700 Subject: [PATCH] Refresh scenario viewport measurement when its identity changes --- .../src/behavior/scenario/workspace.rs | 2 + .../src/behavior/scenario/workspace/tests.rs | 86 ++++++++++++++++++- .../release-scenario-workspace-20260910.md | 13 ++- ...o-workspace-independent-review-20260910.md | 42 +++++++++ 4 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 harness/release-scenario-workspace-independent-review-20260910.md diff --git a/crates/oa-desktop/src/behavior/scenario/workspace.rs b/crates/oa-desktop/src/behavior/scenario/workspace.rs index ea63b579..71280bea 100644 --- a/crates/oa-desktop/src/behavior/scenario/workspace.rs +++ b/crates/oa-desktop/src/behavior/scenario/workspace.rs @@ -649,6 +649,8 @@ fn canvas_pane<'a, R: iced::advanced::text::Renderer + 'a>( .width(Fill) .height(Fill), )) + // A new owner/document needs its first measurement even at the same size. + .key((context.document, context.scenario)) .on_show(move |size| Message::ScenarioCanvas(CanvasEvent::Viewport(context, size))) .on_resize(move |size| Message::ScenarioCanvas(CanvasEvent::Viewport(context, size))), ) diff --git a/crates/oa-desktop/src/behavior/scenario/workspace/tests.rs b/crates/oa-desktop/src/behavior/scenario/workspace/tests.rs index 8829a024..e1dade5b 100644 --- a/crates/oa-desktop/src/behavior/scenario/workspace/tests.rs +++ b/crates/oa-desktop/src/behavior/scenario/workspace/tests.rs @@ -341,6 +341,85 @@ fn scenario_workspace_fit_uses_measured_inner_viewport_and_rejects_nonfinite_inp ); } +#[test] +fn scenario_workspace_fit_reobserves_same_sized_owner_and_document_changes() { + use super::super::direct_tests::{navigation_event, navigation_fixture}; + let (project, [a, b, _], [reference, _, _]) = navigation_fixture(); + let mut app = App::with_project(project.clone()); + let _ = app.update(Message::Select(a)); + let _ = app.update(Message::Page(crate::app::Page::Behavior)); + let size = Size::new(500., 570.); + let graph = Size::new(1800., 2400.); + let mut view = fixture_view( + &app.behavior.scenario, + app.scenario_context().unwrap(), + false, + 68., + ); + let mut tree = widget::Tree::new(view.as_widget()); + redraw(&mut view, &mut tree, size, &mut app); + let (_, first) = geometry(&mut view, &mut tree, size); + let measured = first.scrolls["scenario-canvas"].0.size(); + assert_eq!(app.behavior.scenario.presentation.viewport, Some(measured)); + + let accepted = app.document.snapshot(); + navigation_event(&mut app, |ctx| CanvasEvent::OpenUse(ctx, reference)); + assert_eq!(app.scenario_context().unwrap().scenario, b); + assert_eq!(app.behavior.scenario.presentation.viewport, None); + view = fixture_view( + &app.behavior.scenario, + app.scenario_context().unwrap(), + false, + 68., + ); + tree.diff(view.as_widget()); + redraw(&mut view, &mut tree, size, &mut app); + let (_, second) = geometry(&mut view, &mut tree, size); + assert_eq!(second.scrolls["scenario-canvas"].0.size(), measured); + assert_eq!(app.behavior.scenario.presentation.viewport, Some(measured)); + navigation_event(&mut app, |ctx| CanvasEvent::Zoom(ctx, None)); + let fitted = app.behavior.scenario.presentation.scale(graph); + assert!( + fitted < 1. + && graph.width * fitted <= measured.width - 16. + && graph.height * fitted <= measured.height - 16. + ); + assert!(std::sync::Arc::ptr_eq(&accepted, &app.document.snapshot())); + assert!(!app.document.can_undo()); + + let document = app.scenario_context().unwrap().document; + let mut replacement = App::with_project(project); + let _ = replacement.update(Message::Select(b)); + let _ = replacement.update(Message::Page(crate::app::Page::Behavior)); + assert_ne!(replacement.scenario_context().unwrap().document, document); + assert_eq!(replacement.behavior.scenario.presentation.viewport, None); + let accepted = replacement.document.snapshot(); + view = fixture_view( + &replacement.behavior.scenario, + replacement.scenario_context().unwrap(), + false, + 68., + ); + tree.diff(view.as_widget()); + redraw(&mut view, &mut tree, size, &mut replacement); + let (_, replaced) = geometry(&mut view, &mut tree, size); + assert_eq!(replaced.scrolls["scenario-canvas"].0.size(), measured); + assert_eq!( + replacement.behavior.scenario.presentation.viewport, + Some(measured) + ); + navigation_event(&mut replacement, |ctx| CanvasEvent::Zoom(ctx, None)); + assert_eq!( + replacement.behavior.scenario.presentation.scale(graph), + fitted + ); + assert!(std::sync::Arc::ptr_eq( + &accepted, + &replacement.document.snapshot() + )); + assert!(!replacement.document.can_undo()); +} + #[test] fn scenario_workspace_open_and_back_restore_owner_and_mounted_scroll_after_preselection() { use super::super::direct_tests::{navigation_event, navigation_fixture}; @@ -694,10 +773,9 @@ fn scenario_workspace_scaled_and_scrolled_scene_keeps_clicked_slots_and_message_ &canvas::Event::Mouse(event), bounds, mouse::Cursor::Available(cursor + scene.scroll_offset), - ) { - if let Some(message) = action.into_inner().0 { - messages.push(message); - } + ) && let Some(message) = action.into_inner().0 + { + messages.push(message); } } assert!( diff --git a/harness/release-scenario-workspace-20260910.md b/harness/release-scenario-workspace-20260910.md index 44468041..ceda99db 100644 --- a/harness/release-scenario-workspace-20260910.md +++ b/harness/release-scenario-workspace-20260910.md @@ -43,4 +43,15 @@ After root integrated the upstream explicit-enum F1 seed repair as `81668f9`, th Root received an immutable **631-test** executable in `.local/scenario-workspace-20260910/tests-81668f9/arcadia-studio-tests.exe`, 104,007,680 bytes, SHA256 `FF5999466032E4FBDD615969E349720EC6B29A5945F7E3A57CCF976D8A2736E7`. The sibling `libunwind.dll` is 190,976 bytes, SHA256 `F45604A12E8BCD839FD86F667C1BE1CC7C4F9DA2B3C69FF381AC708F0E1AE7D4`. `manifest.json` records the source HEAD and 252 working-tree input hashes (including clearly marked present but undeclared creation modules), SHA256 `D06C72EBBE9B39A7CE2CCCFF888D2DE7892DFD07EA166FC308155642D37708EE`. Root owns the full run of this immutable artifact while this worker builds the GUI and performs fmt/strict Clippy under the coordinated source freeze. -Scoped rustfmt and diff checks pass. No current GUI binary or native acceptance is claimed at this checkpoint. Root's earlier immutable artifact had 615 passes and three failures; all three failing cases now pass the focused current gates above. +The GUI build passed in **4m17s** (`gui-build.log` and `gui-build-status.json`). Before final handoff, palette_finish's independent review found a real missing case: `State::enter` clears measured viewport on owner/document changes, while an unkeyed Iced Sensor retains its last size and does not re-emit when the new workspace is the same size. Root staged this build only as a known-defect native candidate, not scenario acceptance. + +The correction keys the actual canvas Sensor by `(document, scenario)`. A new mounted regression opens a different owner with exactly the same pane dimensions, then replaces the document while retaining the owner UUID and dimensions; both must re-observe the viewport and make Fit work immediately. Existing scrollable/observer identity and camera restoration remain unchanged. The corrected complete scenario filter passed **53/53**, 579 filtered (**632 total**), in 30.90s after a 2m17s compile. It includes eleven workspace tests. Evidence: `scenario-sensor-tests.log` and matching status JSON. + +Scoped rustfmt and diff checks pass. After this gate, root explicitly transferred shared App/behavior/checkpoints integration and the sole Cargo writer lease to the creation worker. Exact shared boundaries were handed over; this worker has stopped Rust edits. Root owns the sensor correction commit and final combined GUI/fmt/strict Clippy after the creation focused gate. The corrected/combined GUI and native acceptance remain pending. Root's earlier immutable artifact had 615 passes and three failures; all three failing cases now pass the focused current gates above. The full 631-test run belongs to root and must not be duplicated. + +## Native handoff and remaining limits + +- At 1100×700 and 1440×900, open a scenario, insert at a visible root/ALT position, scroll a long message/value form and reach the pinned Create/Cancel actions. Verify canvas rows remain usable while the form is open; Focus and shelves must retain raw input and canvas position. +- Use Fit after switching to another scenario with unchanged window/pane size, and after opening another document. It must act immediately, with no resize workaround. Manual scale must survive form disclosure and resize; returning to a caller must recover its tool, scale, shelf, focus and actual scroll offsets. +- Create remains one existing validated command batch; Save/Undo/Redo/reopen and exact source/ordered-occurrence identity remain the semantic acceptance oracle. This layout slice introduces no new domain/schema/REST contract, local owner/source creation, reuse semantics or lifecycle inference. +- Test rendering uses actual Iced layout/Scrollable operations with a null renderer and explicit header-height cases; root must verify real font wrapping, hit targets and readability. The first GUI candidate intentionally predates the Sensor correction and cannot establish corrected Fit acceptance. diff --git a/harness/release-scenario-workspace-independent-review-20260910.md b/harness/release-scenario-workspace-independent-review-20260910.md new file mode 100644 index 00000000..9d3488f1 --- /dev/null +++ b/harness/release-scenario-workspace-independent-review-20260910.md @@ -0,0 +1,42 @@ +# Independent scenario workspace review — 2026-09-10 + +Codex-9e8r6t /root/palette_finish. Reviewed commit +`3978bbe23c3d50563ad87fea5552de3d52f39d6b` against its parent, the new tests, +App context/modal gates, existing scroll observer and actual cached +`iced_widget-0.14.2/src/sensor.rs`. No source edits, Cargo or native input. + +## Actionable finding: viewport observation loses owner identity + +**P2 — Fit overview can stop working after same-size scenario navigation.** + +Affected code: `crates/oa-desktop/src/behavior/scenario.rs`, `State::enter`, +and `crates/oa-desktop/src/behavior/scenario/workspace.rs`, `canvas_pane`. + +Reproduction condition: mount scenario A and obtain its viewport measurement; +navigate to scenario B at the same canvas viewport size, without opening a +shelf or changing focus/header height; then choose Fit overview on a scene +larger than that viewport. `enter` resets presentation to defaults for B, +including `viewport = None`. The Sensor wraps the identity-resetting scroll +observer but has its own stable outer widget state and default unit key. +Iced's Sensor retains `has_popped_in` and `last_size` through `diff`; unchanged +size and key produce neither on_show nor on_resize. B therefore never receives +the current measurement. Its fit scale falls back to 1.0 until another action +changes the canvas dimensions. The same identity-lifetime issue can preserve a +historical caller viewport on Back after resizing in a callee. + +The existing mounted A-to-B tests verify actual scroll reset/restoration and +manual camera values, but do not assert B obtains a measured viewport at an +unchanged allocation. Pure Presentation tests manually provide that measurement +and therefore cannot expose this failure. + +Requested fix: key the Sensor by `(context.document, context.scenario)` or +otherwise re-observe its current size at that identity boundary, while preserving +the existing child scroll restoration contract. Add a mounted same-size A-to-B +regression that proves B gets a measurement and Fit produces a scale below one +without a resize. The author acknowledged the finding and is implementing this +fix and regression under their lease. No executed fix result is claimed here. + +No second actionable counterexample was established in this bounded review of +stale pointer epochs, passive scroll/viewport guards, invalid Back normalization, +raw pending forms, stable disclosure/focus slots or scale/hit-coordinate paths. +This does not replace root's immutable full-suite run or native acceptance.