From fe637dcd277b17653139ffa234e601b29775c565 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Thu, 24 Sep 2026 20:09:26 -0700 Subject: [PATCH] Add atomic Process group removal with explicit reuse and ordering impact --- crates/oa-desktop/src/app.rs | 47 +- crates/oa-desktop/src/app/diagram_removal.rs | 2 + crates/oa-desktop/src/app/escape.rs | 15 + crates/oa-desktop/src/app/process_removal.rs | 229 +++++ .../src/app/process_removal/tests.rs | 640 ++++++++++++ .../src/app/process_removal/view.rs | 275 ++++++ crates/oa-desktop/src/app/scenario_removal.rs | 1 + crates/oa-desktop/src/behavior/topology.rs | 9 + .../src/behavior/topology/direct_selection.rs | 67 +- .../topology/direct_selection_tests.rs | 100 +- .../src/behavior/topology/group_removal.rs | 322 +++++++ .../behavior/topology/group_removal/tests.rs | 909 ++++++++++++++++++ .../src/behavior/topology/removal_input.rs | 84 ++ .../oa-desktop/src/behavior/topology/view.rs | 10 + crates/oa-desktop/src/model_menu.rs | 42 +- crates/oa-desktop/src/views.rs | 7 + docs/language.rst | 20 + .../process-removal-baseline-20260925.arcadia | 22 + .../process-removal-removed-20260925.arcadia | 19 + harness/multiselect-contract-20260923.md | 5 + .../process-group-removal-policy-20260925.md | 129 +++ 21 files changed, 2916 insertions(+), 38 deletions(-) create mode 100644 crates/oa-desktop/src/app/process_removal.rs create mode 100644 crates/oa-desktop/src/app/process_removal/tests.rs create mode 100644 crates/oa-desktop/src/app/process_removal/view.rs create mode 100644 crates/oa-desktop/src/behavior/topology/group_removal.rs create mode 100644 crates/oa-desktop/src/behavior/topology/group_removal/tests.rs create mode 100644 crates/oa-desktop/src/behavior/topology/removal_input.rs create mode 100644 harness/fixtures/process-removal-baseline-20260925.arcadia create mode 100644 harness/fixtures/process-removal-removed-20260925.arcadia create mode 100644 harness/process-group-removal-policy-20260925.md diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 6ab7a854..3e462aed 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -19,6 +19,7 @@ mod hosted; pub(crate) mod multiselect; #[cfg(test)] mod multiselect_tests; +pub(crate) mod process_removal; mod reference_authoring; #[cfg(test)] mod review_navigation_tests; @@ -445,6 +446,7 @@ pub enum Message { ScenarioCanvas(crate::behavior::scenario::CanvasEvent), ScenarioReorder(Box), ScenarioRemoval(Box), + ProcessRemoval(Box), TopologyCanvas(crate::behavior::topology::CanvasEvent), PlaceSelected, HideSelected, @@ -1364,7 +1366,19 @@ impl App { && self .delete_target .as_ref() - .is_some_and(|target| target.scenario.is_some()) + .is_some_and(|target| target.scenario.is_some() || target.process.is_some()) + { + return Task::none(); + } + if let Message::ProcessRemoval(event) = message { + return self.process_removal_event(*event); + } + // Process review Escape is captured with its token by the root widget. + if matches!(message, Message::Escape) + && self + .delete_target + .as_ref() + .is_some_and(|target| target.process.is_some()) { return Task::none(); } @@ -1387,6 +1401,14 @@ impl App { self.confirm_scenario_removal(*token); return Task::none(); } + if self + .delete_target + .as_ref() + .is_some_and(|target| target.process.is_some()) + { + self.confirm_process_removal(*token); + return Task::none(); + } } // Captured reorder packets must be admitted before shared navigation, // camera or source boundaries can disturb a newer operation. @@ -1400,14 +1422,14 @@ impl App { // A delayed Scenario menu cannot retire the current reveal/gesture even // when it uses the same semantic owner after a representation round trip. if let Message::Menu(crate::model_menu::Event::Open(request)) = &message - && request.scenario.is_some() + && (request.scenario.is_some() || request.process.is_some()) && crate::model_menu::Opened::new(self, request.clone(), self.menu_sequence).is_none() { return Task::none(); } if let Message::Menu(crate::model_menu::Event::Invoke(token, _)) = &message && let Some(menu) = &self.model_menu - && menu.request.scenario.is_some() + && (menu.request.scenario.is_some() || menu.request.process.is_some()) && (menu.token != *token || !menu.valid(self)) { return Task::none(); @@ -1423,6 +1445,17 @@ impl App { .into(); return Task::none(); } + if self.topology_context().is_some() + && self + .multiselect_snapshot() + .is_some_and(|s| s.targets().len() > 1) + && matches!(&message, Message::Delete) + { + self.status = + "Use Remove selected above the canvas to review the complete Process selection." + .into(); + return Task::none(); + } let preview_read_only = self.process_preview_primary().is_some(); let selection_boundary = self.multiselect_boundary(); let scenario_selection_before = self.scenario_context().and_then(|context| { @@ -2260,6 +2293,7 @@ impl App { Message::Hosted(event) => return self.hosted_event(event), Message::ScenarioReorder(event) => return self.scenario_reorder_event(*event), Message::ScenarioRemoval(event) => return self.scenario_removal_event(*event), + Message::ProcessRemoval(event) => return self.process_removal_event(*event), Message::ScenarioCanvas(event) => { use crate::behavior::scenario::{self, CanvasEvent}; let context = event.context(); @@ -2802,7 +2836,10 @@ impl App { } if matches!(*action, Message::DiagramSelection(_)) || (menu.request.process.is_some() - && matches!(*action, Message::TopologyCanvas(_))) + && matches!( + *action, + Message::TopologyCanvas(_) | Message::ProcessRemoval(_) + )) || (menu.request.scenario.is_some() && matches!( *action, @@ -3724,7 +3761,7 @@ impl App { .is_some_and(|s| s.targets().len() > 1) { self.status = if self.topology_context().is_some() { - "Group removal needs an aggregate review. Select one contribution to remove it individually." + "Use Remove selected above the canvas to review the complete Process selection." } else { "Group model-deletion review is not available yet. Select one element or use Hide selected." }.into(); diff --git a/crates/oa-desktop/src/app/diagram_removal.rs b/crates/oa-desktop/src/app/diagram_removal.rs index 4cc0334f..9820d563 100644 --- a/crates/oa-desktop/src/app/diagram_removal.rs +++ b/crates/oa-desktop/src/app/diagram_removal.rs @@ -16,6 +16,7 @@ pub struct Confirmation { pub placement: Option, pub behavior_owner: Option, pub(crate) scenario: Option, + pub(crate) process: Option, } pub fn enabled(app: &App) -> bool { @@ -211,6 +212,7 @@ impl App { placement: placement(self), behavior_owner: behavior_owner(self), scenario: None, + process: None, } }); self.delete_pending = self.delete_target.is_some(); diff --git a/crates/oa-desktop/src/app/escape.rs b/crates/oa-desktop/src/app/escape.rs index 54f09957..3c78d2ca 100644 --- a/crates/oa-desktop/src/app/escape.rs +++ b/crates/oa-desktop/src/app/escape.rs @@ -24,6 +24,15 @@ pub(crate) fn root<'a, R: renderer::Renderer + 'a>( && app.architecture.is_none())); Element::new(Bridge { content, + removal_cancel: app + .delete_target + .as_ref() + .filter(|target| app.delete_pending && target.process.is_some()) + .map(|target| { + Message::ProcessRemoval(Box::new(super::process_removal::Event::Cancel { + token: target.snapshot.token, + })) + }), reorder_active: app.scenario_reorder_enabled() && app.scenario_reorder.session.is_some(), reorder_apply: crate::behavior::scenario::reorder_view::apply_message(app), messages: [ @@ -43,6 +52,7 @@ pub(crate) fn root<'a, R: renderer::Renderer + 'a>( struct Bridge<'a, R> { content: Element<'a, Message, Theme, R>, messages: [Option; 2], + removal_cancel: Option, reorder_active: bool, reorder_apply: Option, } @@ -147,6 +157,11 @@ impl Widget for Bridge<'_, R> { // Escape would otherwise reach focused fields underneath it. Offer only // Escape to the current overlay before updating the ordinary tree. if escape { + if let Some(cancel) = &self.removal_cancel { + shell.publish(cancel.clone()); + shell.capture_event(); + return; + } if let Some(element) = self.content.as_widget_mut().overlay( &mut tree.children[0], layout, diff --git a/crates/oa-desktop/src/app/process_removal.rs b/crates/oa-desktop/src/app/process_removal.rs new file mode 100644 index 00000000..7bebf25a --- /dev/null +++ b/crates/oa-desktop/src/app/process_removal.rs @@ -0,0 +1,229 @@ +//! One captured Process selection, one impact review, one document transaction. +use super::multiselect::process_context; +use super::*; +use crate::behavior::topology::{self, group_removal}; +use crate::diagram_selection::Snapshot; +use crate::diagram_usage::Target; +use crate::model_menu; + +#[cfg(test)] +mod tests; +pub(crate) mod view; + +#[derive(Debug, Clone)] +pub(crate) enum Event { + Begin { + input_generation: u64, + context: topology::Context, + snapshot: Box, + }, + Query { + token: u64, + query: String, + }, + Page { + token: u64, + page: usize, + }, + Cancel { + token: u64, + }, +} + +pub(crate) struct Review { + input_generation: u64, + pub(crate) context: topology::Context, + pub(crate) snapshot: Snapshot, + pub(crate) plan: group_removal::Plan, + pub(crate) query: String, + pub(crate) page: usize, +} + +pub(crate) fn begin(app: &App) -> Option { + let input_generation = app.behavior.topology.removal_generation()?; + let context = app.topology_context()?; + let snapshot = app.multiselect_snapshot()?; + (snapshot.targets().len() > 1).then_some(Message::ProcessRemoval(Box::new(Event::Begin { + input_generation, + context, + snapshot: Box::new(snapshot), + }))) +} + +impl App { + pub(crate) fn process_removal_enabled(&self) -> bool { + diagram_removal::enabled(self) + && !self.source_rename.open() + && self.behavior.topology.selecting() + && self.behavior.topology.removal_generation().is_some() + && self.topology_context().is_some() + } + + fn process_removal_matches(&self, context: topology::Context, snapshot: &Snapshot) -> bool { + if self.topology_context() != Some(context) + || !self + .diagram_selection + .as_ref() + .is_some_and(|selected| selected.matches(snapshot, process_context(context))) + { + return false; + } + let visible = self + .behavior + .topology + .selection_targets(self.project(), context.owner); + snapshot + .targets() + .iter() + .all(|target| visible.contains(target)) + } + + pub(super) fn process_removal_event(&mut self, event: Event) -> Task { + match event { + Event::Begin { + input_generation, + context, + snapshot, + } => { + // Admission precedes every shared input/selection boundary. A + // stale packet must not prune the set or retire a newer editor. + if self.behavior.topology.removal_generation() != Some(input_generation) + || !self.process_removal_matches(context, &snapshot) + || !self.process_removal_enabled() + || snapshot.targets().len() < 2 + { + return Task::none(); + } + let plan = + match group_removal::prepare(self.project(), context.owner, snapshot.targets()) + { + Ok(plan) => plan, + Err(reason) => { + self.status = reason; + return Task::none(); + } + }; + let Some(primary_target) = snapshot.primary().cloned() else { + return Task::none(); + }; + let primary = match &primary_target { + Target::ProcessOccurrence(endpoint) => endpoint.occurrence, + Target::ProcessLink(id) => *id, + _ => return Task::none(), + }; + self.source_interaction(); + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + let context = self.topology_context().expect("admitted Process"); + let snapshot = self.multiselect_snapshot().expect("admitted selection"); + let mut request = model_menu::Request::object(primary, iced::Point::ORIGIN); + request.process = Some(Box::new(context)); + request.selection_target = Some(Box::new(primary_target)); + self.menu_sequence = self.menu_sequence.wrapping_add(1); + self.delete_target = model_menu::Opened::new(self, request, self.menu_sequence) + .map(|opened| diagram_removal::Confirmation { + snapshot: opened, + selection: self.selected, + placement: None, + behavior_owner: Some(context.owner), + scenario: None, + process: Some(Review { + input_generation, + context, + snapshot, + plan, + query: String::new(), + page: 0, + }), + }); + self.delete_pending = self.delete_target.is_some(); + } + Event::Query { token, query } => { + if let Some(target) = self + .delete_target + .as_mut() + .filter(|target| target.snapshot.token == token) + && let Some(review) = target.process.as_mut() + { + review.query = query; + review.page = 0; + } + } + Event::Page { token, page } => { + if let Some(target) = self + .delete_target + .as_mut() + .filter(|target| target.snapshot.token == token) + && let Some(review) = target.process.as_mut() + { + review.page = page; + } + } + Event::Cancel { token } => { + if self.delete_target.as_ref().is_some_and(|target| { + target.snapshot.token == token && target.process.is_some() + }) { + self.delete_target = None; + self.delete_pending = false; + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + self.status = "Removal cancelled. Model and selection unchanged.".into(); + } + } + } + Task::none() + } + + pub(crate) fn process_removal_review_valid( + &self, + target: &diagram_removal::Confirmation, + ) -> bool { + let Some(review) = &target.process else { + return false; + }; + self.delete_pending + && !self.busy + && target.snapshot.valid(self) + && self.behavior.topology.removal_generation() == Some(review.input_generation) + && self.process_removal_matches(review.context, &review.snapshot) + && self.selected == target.selection + && !self.source_dirty() + && !self.draft_dirty() + && !self.behavior.has_pending() + && !self.source_rename.open() + && self.behavior.topology.selecting() + } + + pub(super) fn confirm_process_removal(&mut self, token: u64) { + let Some(target) = self + .delete_target + .as_ref() + .filter(|target| target.snapshot.token == token && target.process.is_some()) + else { + return; + }; + let valid = self.process_removal_review_valid(target); + let target = self.delete_target.take().expect("captured review"); + self.delete_pending = false; + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + if !valid { + self.status = + "Removal review expired. Nothing was removed; review the current selection again." + .into(); + return; + } + let review = target.process.expect("Process review"); + match self.document.execute(review.plan.command) { + Ok(_) => { + self.selected = review.context.owner; + self.selected_node = None; + self.behavior.active_owner = Some(review.context.owner); + self.sync(); + self.reconcile_multiselect(); + self.status = format!( + "Removed {} selected Process contributions or links in one change. Referenced definitions preserved. Undo restores the model.", + review.snapshot.targets().len() + ); + } + Err(error) => self.status = format!("Nothing removed: {error}"), + } + } +} diff --git a/crates/oa-desktop/src/app/process_removal/tests.rs b/crates/oa-desktop/src/app/process_removal/tests.rs new file mode 100644 index 00000000..54a8d22b --- /dev/null +++ b/crates/oa-desktop/src/app/process_removal/tests.rs @@ -0,0 +1,640 @@ +use super::*; +use crate::behavior::topology::{CanvasEvent, CanvasTool, direct_tests}; +use oa_core::source_project::SourceBundle; + +fn local(id: Id) -> Target { + Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![], + occurrence: id, + }) +} +fn choose(app: &mut App, target: Target, choice: multiselect::Choice) { + let context = app.multiselect_context().unwrap(); + let _ = app.update(Message::DiagramSelection(multiselect::Event::Choose( + context, target, choice, + ))); +} +fn open_project(project: Project, owner: Id) -> App { + let mut app = App::with_project(project); + let _ = app.update(Message::Select(owner)); + let _ = app.update(Message::Page(Page::Behavior)); + app.reconcile_multiselect(); + app.window_size = iced::Size::new(1600., 1000.); + app +} +fn fixture(layer: Layer) -> (App, direct_tests::Fixture) { + let f = direct_tests::fixture(layer); + let mut app = open_project(f.p.clone(), f.owner); + choose( + &mut app, + Target::ProcessLink(f.link), + multiselect::Choice::Replace, + ); + choose(&mut app, local(f.nodes[0]), multiselect::Choice::Add); + assert!(app.process_removal_enabled()); + assert_eq!(app.multiselect_snapshot().unwrap().targets().len(), 2); + (app, f) +} +fn send(app: &mut App, event: Event) { + let _ = app.update(Message::ProcessRemoval(Box::new(event))); +} +fn open(app: &mut App) -> u64 { + let message = begin(app).unwrap(); + let _ = app.update(message); + let target = app.delete_target.as_ref().expect("aggregate review"); + assert!(app.process_removal_review_valid(target)); + target.snapshot.token +} +fn native_bundle(source: &str) -> SourceBundle { + SourceBundle { + entry: "project.arcadia".into(), + files: [("project.arcadia".into(), source.into())] + .into_iter() + .collect(), + } +} +const BASELINE: &str = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../harness/fixtures/process-removal-baseline-20260925.arcadia" +)); +const REMOVED: &str = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../harness/fixtures/process-removal-removed-20260925.arcadia" +)); +fn native() -> App { + let document = Document::from_bundle(native_bundle(BASELINE), Default::default()).unwrap(); + let owner = alias(document.project(), "assessment"); + let mut app = open_project(document.project().clone(), owner); + app.document = document; + app.sync(); + app.reconcile_multiselect(); + app +} +fn alias(project: &Project, name: &str) -> Id { + project + .identities() + .into_iter() + .find(|(_, alias)| *alias == name) + .unwrap() + .0 +} + +#[test] +fn process_group_removal_literal_native_model_and_source_oracle_undoes_once() { + let mut app = native(); + for (index, name) in ["a", "c", "approved"].iter().enumerate() { + let id = alias(app.project(), name); + let target = if *name == "approved" { + Target::ProcessLink(id) + } else { + local(id) + }; + choose( + &mut app, + target, + if index == 0 { + multiselect::Choice::Replace + } else { + multiselect::Choice::Add + }, + ); + } + let before = app.document.snapshot(); + let selected = app.multiselect_snapshot().unwrap(); + let camera = app.behavior.topology.camera_boundary(); + let token = open(&mut app); + let review = app + .delete_target + .as_ref() + .unwrap() + .process + .as_ref() + .unwrap(); + assert_eq!(review.snapshot.targets(), selected.targets()); + assert_eq!(review.snapshot.primary(), selected.primary()); + let rows = view::matching_rows(app.project(), review); + assert!( + rows.iter().any(|row| row.label.contains("@caller") + && row.disposition.contains("Uses changed definition")) + ); + assert!( + rows.iter().any(|row| row.label.contains("@outer") + && row.disposition.contains("Uses changed definition")) + ); + assert!(rows.iter().any(|row| { + row.label + .contains("Implicit listed order; 1 adjacent ordered pairs") + && row.disposition.starts_with("After") + })); + assert_eq!(app.document.bundle(), &before.bundle); + let expected = Document::from_bundle(native_bundle(REMOVED), Default::default()).unwrap(); + let _ = app.update(Message::ConfirmDelete(token)); + assert_eq!(app.project(), expected.project()); + assert_eq!( + app.document.bundle(), + expected.bundle(), + "literal independently authored source oracle" + ); + assert_eq!(app.source_workspace.bundle(), *expected.bundle()); + assert_eq!(app.selected, alias(app.project(), "assessment")); + assert!(app.multiselect_snapshot().unwrap().targets().is_empty()); + assert!(app.behavior.topology.camera_boundary() == camera); + let removed = app.document.bundle().clone(); + let _ = app.update(Message::Undo); + assert_eq!(app.project(), &before.decoded.project); + assert_eq!(app.document.bundle(), &before.bundle); + assert!(!app.document.can_undo()); + let _ = app.update(Message::Redo); + assert_eq!(app.document.bundle(), &removed); +} + +#[test] +fn process_group_removal_mixed_card_link_is_one_exact_transaction_in_both_layers() { + for layer in [Layer::Oa, Layer::Sa] { + let (mut app, f) = fixture(layer); + let before = app.document.snapshot(); + let token = open(&mut app); + let review = app + .delete_target + .as_ref() + .unwrap() + .process + .as_ref() + .unwrap(); + assert_eq!( + review.plan.removed, + [f.nodes[0], f.link].into_iter().collect() + ); + let _ = app.update(Message::ConfirmDelete(token)); + let mut expected = f.p.clone(); + expected.elements.remove(&f.nodes[0]); + expected.relations.remove(&f.link); + *expected + .elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() = vec![f.nodes[1], f.nodes[2]]; + assert_eq!(app.project(), &expected); + for id in f.work.into_iter().chain([f.exchange]) { + assert!(app.project().contains(id)); + } + let removed = app.document.bundle().clone(); + assert_eq!( + Document::from_bundle(removed.clone(), Default::default()) + .unwrap() + .project(), + &expected + ); + let _ = app.update(Message::Undo); + assert_eq!(app.project(), &before.decoded.project); + assert_eq!(app.document.bundle(), &before.bundle); + assert!(!app.document.can_undo()); + let _ = app.update(Message::Redo); + assert_eq!(app.document.bundle(), &removed); + } +} + +#[test] +fn process_group_removal_cancel_stale_packets_and_failed_commit_preserve_redo_source_and_selection() +{ + for failure in [false, true] { + let (mut app, f) = fixture(Layer::Oa); + app.document + .execute(Command::Rename(f.nodes[2], "Future name".into())) + .unwrap(); + let future = app.document.bundle().clone(); + assert!(app.document.undo()); + app.sync(); + app.reconcile_multiselect(); + let baseline = app.document.snapshot(); + let source = app.source_editor.text(); + let selected = app.multiselect_snapshot().unwrap(); + let camera = app.behavior.topology.camera_boundary(); + let old_begin = begin(&app).unwrap(); + let first = open(&mut app); + send(&mut app, Event::Cancel { token: first }); + let second = open(&mut app); + let epoch = app.canvas_epoch; + for message in [ + old_begin, + Message::ConfirmDelete(first), + Message::CancelDelete, + Message::Escape, + Message::ProcessRemoval(Box::new(Event::Cancel { token: first })), + Message::ProcessRemoval(Box::new(Event::Query { + token: first, + query: "stale".into(), + })), + Message::ProcessRemoval(Box::new(Event::Page { + token: first, + page: 90, + })), + ] { + let _ = app.update(message); + assert_eq!(app.delete_target.as_ref().unwrap().snapshot.token, second); + assert_eq!(app.canvas_epoch, epoch); + } + if failure { + // Force a genuine atomic engine failure after the admitted deletion. + // Neither the successful first candidate command nor history leaks. + let review = app + .delete_target + .as_mut() + .unwrap() + .process + .as_mut() + .unwrap(); + review.plan.command = Command::Batch(vec![ + Command::Delete(f.nodes[0]), + Command::Delete(Id::new_v4()), + ]); + let _ = app.update(Message::ConfirmDelete(second)); + assert!(app.status.starts_with("Nothing removed:")); + } else { + send(&mut app, Event::Cancel { token: second }); + } + assert!(std::sync::Arc::ptr_eq(&baseline, &app.document.snapshot())); + assert_eq!(app.project(), &baseline.decoded.project); + assert_eq!(app.document.bundle(), &baseline.bundle); + assert_eq!(app.source_editor.text(), source); + assert_eq!(app.source_workspace.bundle(), baseline.bundle); + assert_eq!( + app.multiselect_snapshot().unwrap().targets(), + selected.targets() + ); + assert_eq!( + app.multiselect_snapshot().unwrap().primary(), + selected.primary() + ); + assert!(app.behavior.topology.camera_boundary() == camera); + assert!(app.document.can_redo() && !app.document.can_undo()); + let _ = app.update(Message::Redo); + assert_eq!(app.document.bundle(), &future); + } +} + +#[test] +fn process_group_removal_stale_secondary_and_context_are_rejected_before_any_boundary() { + for change in 0..4 { + let (mut app, f) = fixture(Layer::Sa); + let message = begin(&app).unwrap(); + match change { + 0 => { + app.diagram_selection + .as_mut() + .unwrap() + .remove(&Target::ProcessLink(f.link)); + } + 1 => { + app.canvas_document = Id::new_v4(); + } + 2 => { + app.canvas_epoch = app.canvas_epoch.wrapping_add(1); + } + _ => { + app.document + .execute(Command::Rename(f.nodes[2], "New revision".into())) + .unwrap(); + } + } + let source = app.document.bundle().clone(); + let selected = app.multiselect_snapshot(); + let epoch = app.canvas_epoch; + let _ = app.update(message); + assert!(app.delete_target.is_none()); + assert_eq!(app.canvas_epoch, epoch); + assert_eq!(app.multiselect_snapshot(), selected); + assert_eq!(app.document.bundle(), &source); + } + let (mut app, f) = fixture(Layer::Oa); + let token = open(&mut app); + app.diagram_selection + .as_mut() + .unwrap() + .remove(&Target::ProcessLink(f.link)); + let current = app.document.snapshot(); + let _ = app.update(Message::ConfirmDelete(token)); + assert!(app.status.contains("expired")); + assert!(std::sync::Arc::ptr_eq(¤t, &app.document.snapshot())); +} + +#[test] +fn process_group_removal_pending_inputs_and_legacy_singleton_messages_preserve_all_members() { + for pending in ["clean", "properties", "source", "tool", "busy", "form"] { + let (mut app, f) = fixture(Layer::Oa); + match pending { + "properties" => { + let _ = app.update(Message::Field("/name".into(), "Unapplied".into())); + } + "source" => { + let text = format!("{}\n// pending", app.source_workspace.text()); + let _ = app.update(Message::SourceEdit(text_editor::Action::SelectAll)); + let _ = app.update(Message::SourceEdit(text_editor::Action::Edit( + text_editor::Edit::Paste(std::sync::Arc::new(text)), + ))); + } + "tool" => app.behavior.topology.choose(CanvasTool::Connect), + "busy" => app.busy = true, + "form" => { + let context = app.topology_context().unwrap(); + let _ = app.update(Message::TopologyCanvas(CanvasEvent::Begin { + context, + tool: topology::Tool::Work, + control: ControlKind::Choice, + insertion: topology::Insertion::default(), + })); + assert!(app.behavior.has_pending()); + } + _ => {} + } + let baseline = app.document.snapshot(); + let selected = app.multiselect_snapshot().unwrap(); + let epoch = app.canvas_epoch; + let source = app.source_workspace.bundle(); + if pending != "clean" { + let message = begin(&app).unwrap(); + let _ = app.update(message); + } + let _ = app.update(Message::Delete); + let context = app.topology_context().unwrap(); + let _ = app.update(Message::TopologyCanvas(CanvasEvent::Remove( + context, f.nodes[0], + ))); + assert!(app.delete_target.is_none()); + assert_eq!(app.canvas_epoch, epoch); + assert_eq!(app.multiselect_snapshot().unwrap(), selected); + assert_eq!(app.document.bundle(), &baseline.bundle); + assert_eq!(app.source_workspace.bundle(), source); + assert!(!app.document.can_undo()); + } +} + +#[test] +fn process_group_removal_qualified_member_refuses_whole_set_without_path_stripping() { + let mut app = native(); + let owner = alias(app.project(), "caller"); + let root = alias(app.project(), "r1"); + let other = alias(app.project(), "r2"); + let leaf = alias(app.project(), "a"); + let _ = app.update(Message::Select(owner)); + let context = app.topology_context().unwrap(); + let _ = app.update(Message::TopologyCanvas(CanvasEvent::Expand(context, root))); + let qualified = Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![root], + occurrence: leaf, + }); + choose(&mut app, qualified.clone(), multiselect::Choice::Replace); + choose(&mut app, local(other), multiselect::Choice::Add); + let selected = app.multiselect_snapshot().unwrap(); + assert_eq!(selected.targets(), &[qualified, local(other)]); + let before = app.document.snapshot(); + let epoch = app.canvas_epoch; + let message = begin(&app).unwrap(); + let _ = app.update(message); + assert!(app.delete_target.is_none()); + assert_eq!(app.canvas_epoch, epoch); + assert_eq!(app.multiselect_snapshot().unwrap(), selected); + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(!app.document.can_undo()); +} + +#[test] +fn process_group_removal_unselected_context_subject_does_not_replace_the_group() { + for unselected in [false, true] { + let (mut app, f) = fixture(Layer::Oa); + let selection = app.multiselect_snapshot().unwrap(); + let clicked = if unselected { f.nodes[2] } else { f.nodes[0] }; + let mut request = model_menu::Request::object(clicked, iced::Point::ORIGIN); + request.process = app.topology_context().map(Box::new); + request.selection_target = Some(Box::new(local(clicked))); + let _ = app.update(Message::Menu(model_menu::Event::Open(request))); + let token = app.model_menu.as_ref().unwrap().token; + let message = begin(&app).unwrap(); + let _ = app.update(Message::Menu(model_menu::Event::Invoke( + token, + Box::new(message), + ))); + let review = app + .delete_target + .as_ref() + .unwrap() + .process + .as_ref() + .unwrap(); + assert_eq!(review.snapshot.targets(), selection.targets()); + assert_eq!(review.snapshot.primary(), selection.primary()); + assert!(!review.plan.removed.contains(&f.nodes[2])); + } +} + +#[test] +fn process_review_lists_transitive_callers_even_when_all_modes_stay_explicit() { + let mut app = native(); + let a = alias(app.project(), "a"); + let link = alias(app.project(), "approved"); + choose(&mut app, local(a), multiselect::Choice::Replace); + choose( + &mut app, + Target::ProcessLink(link), + multiselect::Choice::Add, + ); + let token = open(&mut app); + let review = app + .delete_target + .as_ref() + .unwrap() + .process + .as_ref() + .unwrap(); + assert!( + review.plan.interpretation.is_empty(), + "remaining Control keeps the definition explicit" + ); + let rows = view::matching_rows(app.project(), review); + for alias in ["@caller", "@outer"] { + assert!( + rows.iter().any(|row| row.label.contains(alias) + && row.disposition.contains("Uses changed definition")) + ); + } + send( + &mut app, + Event::Query { + token, + query: "@outer".into(), + }, + ); + let review = app + .delete_target + .as_ref() + .unwrap() + .process + .as_ref() + .unwrap(); + let rows = view::matching_rows(app.project(), review); + assert_eq!(rows.len(), 1); + assert!(rows[0].label.contains("@assessment")); + assert!(rows[0].label.contains("03 Mission using Response")); +} + +#[tokio::test] +async fn mounted_process_group_delete_and_shift_delete_capture_group_escape_is_stamped() { + use crate::behavior::canvas_workspace::tests::{Mounted, key}; + use iced::{ + keyboard::{self, key::Named}, + mouse, + }; + for shift in [false, true] { + let (mut app, _) = fixture(Layer::Oa); + let selected = app.multiselect_snapshot().unwrap(); + let source = app.document.bundle().clone(); + let mut mounted = Mounted::new(&mut app).await; + mounted + .batch(&mut app, &[key(Named::F6)], mouse::Cursor::Unavailable) + .await; + let mut remove = key(Named::Delete); + if let iced::Event::Keyboard(keyboard::Event::KeyPressed { modifiers, .. }) = &mut remove { + *modifiers = if shift { + keyboard::Modifiers::SHIFT + } else { + keyboard::Modifiers::default() + }; + } + let output = mounted + .batch(&mut app, &[remove.clone()], mouse::Cursor::Unavailable) + .await; + assert!(output.iter().any(|message| matches!(message, Message::ProcessRemoval(event) if matches!(&**event, Event::Begin { snapshot, .. } if snapshot.targets() == selected.targets()))), "{output:?}"); + let token = app.delete_target.as_ref().unwrap().snapshot.token; + if let iced::Event::Keyboard(keyboard::Event::KeyPressed { repeat, .. }) = &mut remove { + *repeat = true; + } + let output = mounted + .batch(&mut app, &[remove], mouse::Cursor::Unavailable) + .await; + assert!( + !output + .iter() + .any(|message| matches!(message, Message::ConfirmDelete(_))) + ); + assert_eq!(app.delete_target.as_ref().unwrap().snapshot.token, token); + let output = mounted + .batch(&mut app, &[key(Named::Escape)], mouse::Cursor::Unavailable) + .await; + assert!(output.iter().any(|message| matches!(message, Message::ProcessRemoval(event) if matches!(&**event, Event::Cancel { token: captured } if *captured == token))), "{output:?}"); + assert!(app.delete_target.is_none()); + assert_eq!(app.document.bundle(), &source); + assert_eq!( + app.multiselect_snapshot().unwrap().targets(), + selected.targets() + ); + assert!(!app.document.can_undo()); + } +} + +#[tokio::test] +async fn mounted_process_group_removal_queued_delete_is_revoked_by_later_press_in_same_batch() { + use crate::behavior::canvas_workspace::tests::{Mounted, key}; + use iced::{Event as Input, keyboard::key::Named, mouse}; + let (mut app, f) = fixture(Layer::Oa); + let mut mounted = Mounted::new(&mut app).await; + mounted + .batch(&mut app, &[key(Named::F6)], mouse::Cursor::Unavailable) + .await; + let bounds = mounted.geometry(&app).bounds("process-viewport"); + let local = app + .behavior + .topology + .removal_test_point(&app, f.owner, f.nodes[0], None); + let at = bounds.position() + iced::Vector::new(local.x, local.y); + let before = app.document.snapshot(); + let selected = app.multiselect_snapshot().unwrap(); + let epoch = app.canvas_epoch; + let messages = mounted + .batch( + &mut app, + &[ + key(Named::Delete), + Input::Mouse(mouse::Event::CursorMoved { position: at }), + Input::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), + ], + mouse::Cursor::Available(at), + ) + .await; + assert!(messages.iter().any(|message| matches!(message, Message::ProcessRemoval(event) if matches!(&**event, Event::Begin { .. }))), "the first event queues a real review packet: {messages:?}"); + assert!( + app.delete_target.is_none(), + "later press must revoke that packet before App consumes it" + ); + assert_eq!( + app.canvas_epoch, epoch, + "refusal does not retire the active gesture" + ); + assert_eq!(app.multiselect_snapshot().unwrap(), selected); + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(!app.process_removal_enabled()); + assert!( + begin(&app).is_none(), + "active gesture cannot capture another review" + ); + mounted + .batch( + &mut app, + &[key(Named::Escape)], + mouse::Cursor::Available(at), + ) + .await; + // Escape leaves viewport focus; F6 restores keyboard ownership and sends + // the canvas cancellation boundary before a fresh Delete. + mounted + .batch(&mut app, &[key(Named::F6)], mouse::Cursor::Unavailable) + .await; + assert!(app.process_removal_enabled()); + // Cancelling the pointer gesture retains the group and permits a fresh review. + assert_eq!( + app.multiselect_snapshot().unwrap().targets(), + selected.targets() + ); + mounted + .batch(&mut app, &[key(Named::Delete)], mouse::Cursor::Unavailable) + .await; + assert!(app.delete_target.is_some()); + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); +} + +#[tokio::test] +async fn mounted_process_group_removal_queued_delete_is_revoked_by_camera_pan_start() { + use crate::behavior::canvas_workspace::tests::{Mounted, key}; + use iced::{Event as Input, keyboard::key::Named, mouse}; + for button in [mouse::Button::Middle, mouse::Button::Left] { + let (mut app, _) = fixture(Layer::Sa); + let mut mounted = Mounted::new(&mut app).await; + mounted + .batch(&mut app, &[key(Named::F6)], mouse::Cursor::Unavailable) + .await; + let bounds = mounted.geometry(&app).bounds("process-viewport"); + let at = iced::Point::new( + bounds.x + bounds.width - 12., + bounds.y + bounds.height - 12., + ); + let before = app.document.snapshot(); + let messages = mounted + .batch( + &mut app, + &[ + key(Named::Delete), + Input::Mouse(mouse::Event::CursorMoved { position: at }), + Input::Mouse(mouse::Event::ButtonPressed(button)), + ], + mouse::Cursor::Available(at), + ) + .await; + assert!(messages.iter().any(|message| matches!(message, Message::ProcessRemoval(event) if matches!(&**event, Event::Begin { .. }))), "real Delete is queued: {messages:?}"); + assert!( + app.delete_target.is_none(), + "camera input retires earlier review packet" + ); + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(!app.document.can_undo()); + } +} diff --git a/crates/oa-desktop/src/app/process_removal/view.rs b/crates/oa-desktop/src/app/process_removal/view.rs new file mode 100644 index 00000000..eafeba77 --- /dev/null +++ b/crates/oa-desktop/src/app/process_removal/view.rs @@ -0,0 +1,275 @@ +//! Review the complete captured removal impact before one explicit confirmation. +use super::{Event, Review}; +use crate::app::{App, Message, diagram_removal}; +use crate::diagram_usage::Target; +use crate::style; +use iced::widget::{button, column, container, row, scrollable, text, text_input}; +use iced::{Element, Length::Fill}; +use oa_core::{Id, Project}; +use std::collections::{BTreeMap, BTreeSet}; + +const PAGE_SIZE: usize = 6; + +pub(super) struct ImpactRow { + pub(super) label: String, + pub(super) disposition: String, +} + +pub(super) fn matching_rows(project: &Project, review: &Review) -> Vec { + let aliases: BTreeMap = project.identities().into_iter().collect(); + let label = |id: Id| { + let name = project.named(id); + let identity = aliases + .get(&id) + .map_or_else(|| id.to_string(), |alias| format!("@{alias}")); + if name == "Missing object" || name.is_empty() { + identity + } else { + format!("{name} ({identity})") + } + }; + let selected = |id: Id| { + review.snapshot.targets().iter().any(|target| match target { + Target::ProcessOccurrence(endpoint) => { + endpoint.references.is_empty() && endpoint.occurrence == id + } + Target::ProcessLink(link) => *link == id, + _ => false, + }) + }; + let mut rows = vec![]; + for id in &review.plan.removed { + rows.push(ImpactRow { + label: label(*id), + disposition: if selected(*id) { + "Removed · selected" + } else { + "Additional removed" + } + .into(), + }); + } + for id in &review.plan.changed { + rows.push(ImpactRow { + label: label(*id), + disposition: "Updated · preserved".into(), + }); + } + // Interpretation is separate from stored record deltas: callers may keep + // identical records while their reused definition or analysis changes. + for impact in &review.plan.interpretation { + let owner = label(impact.owner); + let shared_order = impact.before.local_order == impact.before.expanded_order + && impact.after.local_order == impact.after.expanded_order; + for (dimension, before, after, shown) in [ + ( + if shared_order { + "Listed and reused order" + } else { + "Local listed order" + }, + &impact.before.local_order, + &impact.after.local_order, + true, + ), + ( + "Order inside reused definitions", + &impact.before.expanded_order, + &impact.after.expanded_order, + !shared_order, + ), + ] { + if !shown || before == after { + continue; + } + for (when, order) in [("Before", before), ("After", after)] { + let summary = match order { + None => "Explicit topology; no implicit listed order".to_owned(), + Some(ids) if ids.is_empty() => { + "Empty order; no ordered pair; contextual flow may be incomplete".to_owned() + } + Some(ids) if ids.len() == 1 => { + "One item in this order; no ordered pair".to_owned() + } + Some(ids) => format!( + "Implicit listed order; {} adjacent ordered pairs", + ids.len() - 1 + ), + }; + rows.push(ImpactRow { + label: format!("{owner}: {summary}"), + disposition: format!("{when} · {dimension}"), + }); + if let Some(ids) = order { + for (index, id) in ids.iter().enumerate() { + rows.push(ImpactRow { + label: format!("{owner}: #{} {}", index + 1, label(*id)), + disposition: format!("{when} · {dimension} · listed position"), + }); + } + } + } + } + if impact.before.contextual_analysis != impact.after.contextual_analysis { + rows.push(ImpactRow { + label: format!( + "{owner}: contextual analysis {} → {}", + if impact.before.contextual_analysis { + "enabled" + } else { + "disabled" + }, + if impact.after.contextual_analysis { + "enabled" + } else { + "disabled" + } + ), + disposition: + "Analysis mode changes; this is not a claim of complete graph connectivity" + .into(), + }); + } + } + let mut affected = review.plan.affected_uses.clone(); + for impact in &review.plan.interpretation { + affected + .entry(impact.owner) + .or_default() + .extend(impact.callers.iter().copied()); + } + for (definition, callers) in affected { + for caller in callers { + rows.push(ImpactRow { + label: format!("{} uses {}", label(caller), label(definition)), + disposition: "Uses changed definition · affected caller definition".into(), + }); + } + } + let query = review.query.trim().to_lowercase(); + rows.into_iter() + .filter(|row| { + query.is_empty() + || row.label.to_lowercase().contains(&query) + || row.disposition.to_lowercase().contains(&query) + }) + .collect() +} + +pub(crate) fn view<'a>( + app: &'a App, + target: &'a diagram_removal::Confirmation, +) -> Element<'a, Message> { + let Some(review) = target.process.as_ref() else { + return text("Process removal review is unavailable.").into(); + }; + let token = target.snapshot.token; + let valid = app.process_removal_review_valid(target); + let rows = matching_rows(app.project(), review); + let callers: BTreeSet<_> = review + .plan + .affected_uses + .values() + .flatten() + .chain( + review + .plan + .interpretation + .iter() + .flat_map(|impact| impact.callers.iter()), + ) + .copied() + .collect(); + let semantic_summary = if review.plan.interpretation.is_empty() && callers.is_empty() { + "No listed-order or analysis-mode changes; no affected caller definitions.".to_owned() + } else { + format!( + "Order / analysis changes in {} definitions · {} caller definitions affected. Review their effects below.", + review.plan.interpretation.len(), + callers.len() + ) + }; + let pages = rows.len().div_ceil(PAGE_SIZE).max(1); + let page = review.page.min(pages - 1); + let mut impacts = column![].spacing(12).width(Fill); + for impact in rows.iter().skip(page * PAGE_SIZE).take(PAGE_SIZE) { + impacts = impacts.push( + column![ + text(impact.disposition.clone()) + .size(12) + .color(style::MUTED), + text(impact.label.clone()) + .size(13) + .width(Fill) + .wrapping(iced::widget::text::Wrapping::WordOrGlyph), + ] + .spacing(3) + .width(Fill), + ); + } + if rows.is_empty() { + impacts = impacts.push(text("No matching effects. Try a name, alias or effect.").size(13)); + } + let previous = button("Previous").on_press_maybe((page > 0).then(|| { + Message::ProcessRemoval(Box::new(Event::Page { + token, + page: page - 1, + })) + })); + let next = button("Next").on_press_maybe((page + 1 < pages).then(|| { + Message::ProcessRemoval(Box::new(Event::Page { + token, + page: page + 1, + })) + })); + let mut footer = column![].spacing(8).width(Fill); + if !valid { + footer = footer.push( + text("This review expired. Cancel and review the current selection again.") + .size(13) + .width(Fill) + .wrapping(iced::widget::text::Wrapping::WordOrGlyph), + ); + } + footer = footer.push( + row![ + container( + button("Cancel") + .padding(10) + .on_press(Message::ProcessRemoval(Box::new(Event::Cancel { token }))) + ) + .id("delete-cancel"), + container( + button("Remove selected contributions / links") + .style(button::danger) + .padding(10) + .on_press_maybe(valid.then_some(Message::ConfirmDelete(token))) + ) + .id("delete-confirm"), + ] + .spacing(10), + ); + column![ + text(format!("Remove {} selected contributions / links?", review.snapshot.targets().len())).size(21), + text(format!("{} records removed · {} records updated", review.plan.removed.len(), review.plan.changed.len())).size(14), + text(semantic_summary).size(13).width(Fill).wrapping(iced::widget::text::Wrapping::WordOrGlyph), + text_input("Search affected names, aliases or effects", &review.query) + .id("process-removal-query") + .on_input(move |query| Message::ProcessRemoval(Box::new(Event::Query { token, query }))) + .size(13), + row![previous, text(format!("Page {} of {} · {} matches", page + 1, pages, rows.len())).size(12), next] + .spacing(10).align_y(iced::Alignment::Center), + scrollable(column![ + text("Removes selected local contributions or maintained links and the additional records listed below. Referenced definitions, work and exchanges are preserved. Reused topology, entry/exit connections and diagnostics may change. Review every listed order and affected caller definition; this review does not compute complete caller graphs. One Undo restores the entire change.") + .size(13).width(Fill).wrapping(iced::widget::text::Wrapping::WordOrGlyph), + impacts, + ].spacing(14).width(Fill)) + .width(Fill).height(Fill).spacing(8).id("process-removal-impact"), + footer, + ] + .spacing(12) + .width(Fill) + // Keep confirmation and search outside the flexible, scrollable impact body. + .height((app.window_size.height - 160.).clamp(260., 570.)) + .into() +} diff --git a/crates/oa-desktop/src/app/scenario_removal.rs b/crates/oa-desktop/src/app/scenario_removal.rs index 1328a6e5..d3c2e3e4 100644 --- a/crates/oa-desktop/src/app/scenario_removal.rs +++ b/crates/oa-desktop/src/app/scenario_removal.rs @@ -107,6 +107,7 @@ impl App { selection: self.selected, placement: None, behavior_owner: Some(context.scenario), + process: None, scenario: Some(Review { context, snapshot, diff --git a/crates/oa-desktop/src/behavior/topology.rs b/crates/oa-desktop/src/behavior/topology.rs index 5381f47f..d9159df6 100644 --- a/crates/oa-desktop/src/behavior/topology.rs +++ b/crates/oa-desktop/src/behavior/topology.rs @@ -5,7 +5,9 @@ use iced::widget::{Column, text_input}; use std::collections::BTreeSet; pub(crate) mod contextual; mod direct; +pub(crate) mod group_removal; pub(crate) mod group_reorder; +mod removal_input; mod usages; mod view; #[cfg(test)] @@ -80,6 +82,7 @@ impl std::fmt::Display for ControlChoice { } #[derive(Default)] pub struct State { + removal_input: removal_input::Lifetime, identity: Option<(Id, Id)>, tool: CanvasTool, expanded: BTreeSet, @@ -128,6 +131,10 @@ struct Draft { contextual: Option, } impl State { + pub(crate) fn removal_generation(&self) -> Option { + self.removal_input.idle() + } + pub(crate) fn camera_boundary(&self) -> impl PartialEq + use<> { ( self.tool, @@ -1104,6 +1111,7 @@ const PAINT_PASSES: [PaintPass; 5] = [ ]; mod contextual_routing; struct Graph<'a> { + removal_input: removal_input::Lifetime, project: &'a Project, interaction: Option<(Context, CanvasTool, bool)>, camera_pan: iced::Vector, @@ -1401,6 +1409,7 @@ impl<'a> Graph<'a> { } Self { project: p, + removal_input: removal_input::Lifetime::default(), interaction: None, camera_pan: iced::Vector::ZERO, pending_insertion: None, diff --git a/crates/oa-desktop/src/behavior/topology/direct_selection.rs b/crates/oa-desktop/src/behavior/topology/direct_selection.rs index 7d233e29..80ae9a71 100644 --- a/crates/oa-desktop/src/behavior/topology/direct_selection.rs +++ b/crates/oa-desktop/src/behavior/topology/direct_selection.rs @@ -6,6 +6,7 @@ use crate::diagram_selection::Snapshot; use crate::diagram_usage::Target; pub(super) struct GroupPress { + _removal_lease: super::super::removal_input::Lease, snapshot: Snapshot, target: Target, start: Point, @@ -108,6 +109,12 @@ impl Graph<'_> { cursor: mouse::Cursor, ) -> Option> { use iced::keyboard::{Event as KeyEvent, Key, key::Named}; + if matches!( + event, + canvas::Event::Window(iced::window::Event::Unfocused | iced::window::Event::Resized(_)) + ) { + self.removal_input.revoke_idle(); + } match event { canvas::Event::Keyboard(KeyEvent::ModifiersChanged(modifiers)) | canvas::Event::Keyboard(KeyEvent::KeyPressed { modifiers, .. }) @@ -205,21 +212,46 @@ impl Graph<'_> { return None; } let publish = |event| Some(Action::publish(Message::DiagramSelection(event)).and_capture()); - if matches!( - event, - canvas::Event::Keyboard(KeyEvent::KeyPressed { - key: Key::Named(Named::Delete), - repeat: false, - .. - }) - ) && (snapshot.targets().len() > 1 - || snapshot.targets().iter().any(|t| match t { - Target::ProcessPreviewLink { .. } => true, - Target::ProcessOccurrence(endpoint) => !endpoint.references.is_empty(), - _ => false, - })) + if let canvas::Event::Keyboard(KeyEvent::KeyPressed { + key: Key::Named(Named::Delete), + modifiers, + repeat, + .. + }) = event + && (snapshot.targets().len() > 1 + || snapshot.targets().iter().any(|t| match t { + Target::ProcessPreviewLink { .. } => true, + Target::ProcessOccurrence(endpoint) => !endpoint.references.is_empty(), + _ => false, + })) { - return publish(SelectionEvent::Refuse(snapshot.clone(), "Remove is unavailable for this selection. Open the referenced definition to edit qualified contributions; group removal needs an aggregate review.".into())); + if *repeat + || tool != CanvasTool::Select + || !(modifiers.is_empty() || *modifiers == iced::keyboard::Modifiers::SHIFT) + || state.press.is_some() + || state.source.is_some() + { + return Some(Action::capture()); + } + if snapshot.targets().len() > 1 { + let Some(input_generation) = self.removal_input.idle() else { + return Some(Action::capture()); + }; + return Some( + Action::publish(Message::ProcessRemoval(Box::new( + crate::app::process_removal::Event::Begin { + input_generation, + context, + snapshot: Box::new(snapshot.clone()), + }, + ))) + .and_capture(), + ); + } + return publish(SelectionEvent::Refuse( + snapshot.clone(), + "Open the referenced definition to remove qualified contributions.".into(), + )); } let pointer = match event { canvas::Event::Mouse(mouse::Event::CursorMoved { position }) => Some(*position), @@ -228,6 +260,9 @@ impl Graph<'_> { if !bounds.contains(pointer) { return None; } + if matches!(event, canvas::Event::Mouse(mouse::Event::ButtonPressed(_))) { + self.removal_input.revoke_idle(); + } let world = Point::new( (pointer.x - bounds.x - self.camera_pan.x) / self.scale, (pointer.y - bounds.y - self.camera_pan.y) / self.scale, @@ -310,7 +345,11 @@ impl Graph<'_> { state.tool = tool; state.bounds = Some(bounds); state.pointer = Some(pointer); + let Some(lease) = self.removal_input.acquire() else { + return Some(Action::capture()); + }; state.group = Some(GroupPress { + _removal_lease: lease, snapshot: snapshot.clone(), target, start: pointer, diff --git a/crates/oa-desktop/src/behavior/topology/direct_selection_tests.rs b/crates/oa-desktop/src/behavior/topology/direct_selection_tests.rs index 48e41de2..46c30cb7 100644 --- a/crates/oa-desktop/src/behavior/topology/direct_selection_tests.rs +++ b/crates/oa-desktop/src/behavior/topology/direct_selection_tests.rs @@ -171,7 +171,7 @@ fn selected_anchor_noop_and_irreversible_threshold_never_collapse_after_motion() } #[test] -fn whole_mixed_set_refuses_drag_delete_and_connect_without_singleton_bypass() { +fn whole_mixed_set_reviews_delete_and_refuses_drag_and_connect_without_singleton_bypass() { let f = fixture(Layer::Sa); let mut graph = graph(&f, &[local(f.nodes[0]), Target::ProcessLink(f.link)]); graph.selected = f.link; @@ -187,7 +187,7 @@ fn whole_mixed_set_refuses_drag_delete_and_connect_without_singleton_bypass() { }; assert!(matches!( send(&graph, &mut input, canvas::Event::Keyboard(key), start), - Some(Message::DiagramSelection(SelectionEvent::Refuse(_, _))) + Some(Message::ProcessRemoval(event)) if matches!(&*event, crate::app::process_removal::Event::Begin { snapshot, .. } if snapshot.targets().len() == 2) )); graph.interaction.as_mut().unwrap().1 = CanvasTool::Connect; assert!(matches!( @@ -401,3 +401,99 @@ fn mixed_group_at_endpoint_card_refuses_whole_drag_without_hidden_singleton_hand } } } + +#[test] +fn group_delete_modifiers_repeat_and_active_gesture_never_emit_singleton_or_confirmation() { + use iced::keyboard::{Event as KeyEvent, Modifiers, key::Named}; + let f = fixture(Layer::Oa); + let graph = graph(&f, &[local(f.nodes[0]), Target::ProcessLink(f.link)]); + let start = point(&graph, f.nodes[0], false); + for modifiers in [ + Modifiers::empty(), + Modifiers::SHIFT, + Modifiers::CTRL, + Modifiers::ALT, + ] { + for repeat in [false, true] { + let iced::Event::Keyboard(mut key) = + crate::behavior::canvas_workspace::tests::key(Named::Delete) + else { + panic!("key") + }; + if let KeyEvent::KeyPressed { + modifiers: value, + repeat: value_repeat, + .. + } = &mut key + { + *value = modifiers; + *value_repeat = repeat; + } + let message = send( + &graph, + &mut Input::default(), + canvas::Event::Keyboard(key), + start, + ); + if !repeat && (modifiers.is_empty() || modifiers == Modifiers::SHIFT) { + assert!( + matches!(message, Some(Message::ProcessRemoval(event)) if matches!(&*event, crate::app::process_removal::Event::Begin { snapshot, .. } if Some(&**snapshot) == graph.selection.as_ref())) + ); + } else { + assert!(message.is_none()); + } + } + } + let mut input = Input::default(); + press(&graph, &mut input, start); + let iced::Event::Keyboard(key) = crate::behavior::canvas_workspace::tests::key(Named::Delete) + else { + panic!("key") + }; + assert!(send(&graph, &mut input, canvas::Event::Keyboard(key), start).is_none()); + assert!(input.group.is_some()); +} + +#[test] +fn process_removal_gesture_lease_retires_on_release_cancel_and_widget_state_drop() { + let f = fixture(Layer::Oa); + for end in ["release", "escape", "drop", "unfocus"] { + let graph = graph(&f, &[local(f.nodes[0]), Target::ProcessLink(f.link)]); + let before = graph.removal_input.idle().unwrap(); + let at = point(&graph, f.nodes[0], false); + let mut input = Input::default(); + assert!(press(&graph, &mut input, at).is_none()); + assert!(input.group.is_some()); + assert!(graph.removal_input.idle().is_none()); + match end { + "release" => { + release(&graph, &mut input, at); + } + "escape" => { + let iced::Event::Keyboard(key) = crate::behavior::canvas_workspace::tests::key( + iced::keyboard::key::Named::Escape, + ) else { + panic!("key") + }; + send(&graph, &mut input, canvas::Event::Keyboard(key), at); + } + "unfocus" => { + send( + &graph, + &mut input, + canvas::Event::Window(iced::window::Event::Unfocused), + at, + ); + } + _ => { + drop(input); + } + } + assert!(graph.removal_input.idle().is_some()); + assert_ne!( + graph.removal_input.idle(), + Some(before), + "{end} cannot restore an old Begin generation" + ); + } +} diff --git a/crates/oa-desktop/src/behavior/topology/group_removal.rs b/crates/oa-desktop/src/behavior/topology/group_removal.rs new file mode 100644 index 00000000..36e212f0 --- /dev/null +++ b/crates/oa-desktop/src/behavior/topology/group_removal.rs @@ -0,0 +1,322 @@ +//! Exact local Process/FunctionalChain removal and definition-level interpretation +//! effects. Visibility, source drafts and captured UI lifetime belong to the App. +use super::*; +use crate::diagram_usage::Target; +use oa_core::topology_context::TopologyIndex; + +#[derive(Debug)] +pub(crate) struct Plan { + pub(crate) command: Command, + pub(crate) removed: BTreeSet, + pub(crate) changed: BTreeSet, + pub(crate) interpretation: Vec, + /// Definitions whose authored topology changed, including changes that do + /// not switch interpretation. Values are surviving transitive callers. + pub(crate) affected_uses: BTreeMap>, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct InterpretationChange { + pub(crate) owner: Id, + pub(crate) before: Interpretation, + pub(crate) after: Interpretation, + pub(crate) callers: BTreeSet, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Interpretation { + /// None means explicit; Some includes the empty legacy order. + pub(crate) local_order: Option>, + /// Definition-level implicit order used within contextual expansion. This + /// does not claim complete expanded connectivity or boundary analysis. + pub(crate) expanded_order: Option>, + pub(crate) contextual_analysis: bool, +} + +fn exchange(project: &Project, id: Id) -> bool { + matches!( + project.elements.get(&id).and_then(|e| e.shared()), + Some(SharedBody::Occurrence { + body: OccurrenceBody::Exchange { .. }, + .. + }) + ) +} + +fn interpretations(project: &Project) -> BTreeMap { + let index = TopologyIndex::new(project); + // This is exactly TopologyIndex::with_cache_capacity's relation ownership + // rule. Its link_definitions is private; do not expand repeated reuse trees + // just to answer this finite definition-level question. + let linked: BTreeSet<_> = project + .relations + .values() + .filter_map(|relation| { + relation.contextual_owner().or_else(|| { + (relation.kind() == RelationKind::SequenceLink) + .then(|| { + project + .elements + .get(&relation.source()) + .and_then(|e| e.parent) + }) + .flatten() + }) + }) + .collect(); + project + .elements + .values() + .filter(|e| matches!(e.kind(), Kind::Process | Kind::FunctionalChain)) + .map(|e| { + let steps = e.steps().expect("Sequence definitions have ordered steps"); + // legacy_order alone loses the explicit-versus-empty distinction. + let local_order = (!uses_explicit_topology(project, e.id) + && steps.iter().all(|id| exchange(project, *id))) + .then(|| steps.clone()); + let children = index.contributions(e.id); + // Mirrors topology_context/analysis.rs Builder::expand's legacy-order + // discriminator: all indexed contributions (including unlisted ones), + // and neither SequenceLink nor ContextualLink definitions. Tests retain + // literal discriminators where this intentionally differs from desktop. + let expanded_order = (!linked.contains(&e.id) + && children.iter().all(|id| exchange(project, *id))) + .then_some(children); + ( + e.id, + Interpretation { + local_order, + expanded_order, + contextual_analysis: index.has_contextual_links(e.id), + }, + ) + }) + .collect() +} + +fn caller_edges(project: &Project, edges: &mut BTreeMap>) { + for element in project.elements.values() { + if let ( + Some(parent), + Some(SharedBody::Occurrence { + body: + OccurrenceBody::SequenceReference { + sequence: Some(sequence), + }, + .. + }), + ) = (element.parent, element.shared()) + { + edges.entry(*sequence).or_default().insert(parent); + } + } +} + +fn callers( + owner: Id, + edges: &BTreeMap>, + surviving: &BTreeMap, +) -> BTreeSet { + let mut seen = BTreeSet::from([owner]); + let mut pending = vec![owner]; + while let Some(sequence) = pending.pop() { + for &caller in edges.get(&sequence).into_iter().flatten() { + if seen.insert(caller) { + pending.push(caller); + } + } + } + seen.remove(&owner); + seen.retain(|id| surviving.contains_key(id)); + seen +} + +fn changed_records( + before: &BTreeMap, + after: &BTreeMap, + changed: &mut BTreeSet, +) { + changed.extend( + before + .iter() + .filter_map(|(&id, record)| after.get(&id).filter(|next| *next != record).map(|_| id)), + ); +} + +fn definition_owner(project: &Project, id: Id) -> Option { + let owner = if let Some(element) = project.elements.get(&id) { + if matches!(element.kind(), Kind::Process | Kind::FunctionalChain) { + Some(id) + } else { + element + .parent + .filter(|parent| is_contribution(project, *parent, id)) + } + } else { + project.relations.get(&id).and_then(|relation| { + relation.contextual_owner().or_else(|| { + (relation.kind() == RelationKind::SequenceLink) + .then(|| { + project + .elements + .get(&relation.source()) + .and_then(|e| e.parent) + }) + .flatten() + }) + }) + }?; + project + .elements + .get(&owner) + .filter(|e| matches!(e.kind(), Kind::Process | Kind::FunctionalChain)) + .map(|_| owner) +} + +pub(crate) fn prepare(project: &Project, owner: Id, targets: &[Target]) -> Result { + let element = project + .elements + .get(&owner) + .filter(|e| matches!(e.kind(), Kind::Process | Kind::FunctionalChain)) + .ok_or("Choose an existing Process or Functional Chain.")?; + if targets.is_empty() { + return Err("Select at least one local contribution or maintained link to remove.".into()); + } + oa_core::session::validate_candidate(project).map_err(|error| error.to_string())?; + let mut listed = BTreeSet::new(); + for &id in element + .steps() + .ok_or("This owner has no contribution order.")? + { + if !is_contribution(project, owner, id) + || project.elements[&id].layer() != element.layer() + || !listed.insert(id) + { + return Err( + "Every process step must name a unique local contribution before removal.".into(), + ); + } + } + let mut selected = BTreeSet::new(); + let mut occurrences = BTreeSet::new(); + let mut links = BTreeSet::new(); + // Admit ALL original typed targets before any dependency normalization. + for target in targets { + let id = match target { + Target::ProcessOccurrence(endpoint) => { + if !endpoint.references.is_empty() { + return Err("Open the referenced definition to remove its contents.".into()); + } + if !listed.contains(&endpoint.occurrence) { + return Err("Every selected contribution must be listed directly in this process.".into()); + } + occurrences.insert(endpoint.occurrence); + endpoint.occurrence + } + Target::ProcessLink(id) => { + let relation = project.relations.get(id).ok_or("A selected maintained link no longer exists.")?; + let local = match &relation.body { + RelationBody::SequenceLink { source, target, .. } => listed.contains(source) && listed.contains(target), + RelationBody::ContextualLink { owner: actual, .. } => *actual == owner, + _ => false, + }; + if !local { + return Err("Every selected link must be maintained locally by this process, with listed local endpoints for sequence links.".into()); + } + links.insert(*id); + *id + } + _ => return Err("Remove only local process contributions and maintained links together; referenced preview links are read-only here.".into()), + }; + if !selected.insert(id) { + return Err("Every selected process subject must be unique.".into()); + } + } + // Contribution kinds cannot contain one another; only admitted links can be + // covered by an occurrence cascade. Ancestry alone misses incident links. + let covered: BTreeSet<_> = occurrences + .iter() + .flat_map(|id| oa_core::references::deletion_impact(project, *id)) + .collect(); + let command = Command::Batch( + occurrences + .into_iter() + .chain(links.into_iter().filter(|id| !covered.contains(id))) + .map(Command::Delete) + .collect(), + ); + let candidate = execute_batch(project, std::slice::from_ref(&command)) + .map_err(|error| error.to_string())?; + let original: BTreeSet<_> = project.identities().into_iter().map(|(id, _)| id).collect(); + let retained: BTreeSet<_> = candidate + .identities() + .into_iter() + .map(|(id, _)| id) + .collect(); + if !retained.is_subset(&original) { + return Err("Removal unexpectedly created model identities; no change was applied.".into()); + } + let removed: BTreeSet<_> = original.difference(&retained).copied().collect(); + if !selected.is_subset(&removed) { + return Err("The complete selected set could not be removed.".into()); + } + let mut changed = BTreeSet::new(); + changed_records(&project.elements, &candidate.elements, &mut changed); + changed_records(&project.relations, &candidate.relations, &mut changed); + changed_records(&project.diagrams, &candidate.diagrams, &mut changed); + changed_records( + &project.artifact_links, + &candidate.artifact_links, + &mut changed, + ); + changed_records(&project.traces, &candidate.traces, &mut changed); + for (&id, diagram) in &project.diagrams { + if let Some(after) = candidate.diagrams.get(&id) { + changed_records(&diagram.nodes, &after.nodes, &mut changed); + } + } + let before = interpretations(project); + let after = interpretations(&candidate); + let mut edges = BTreeMap::new(); + caller_edges(project, &mut edges); + caller_edges(&candidate, &mut edges); + let affected_owners: BTreeSet<_> = removed + .iter() + .chain(&changed) + .flat_map(|&id| { + [ + definition_owner(project, id), + definition_owner(&candidate, id), + ] + .into_iter() + .flatten() + }) + .filter(|owner| after.contains_key(owner)) + .collect(); + let affected_uses = affected_owners + .into_iter() + .map(|owner| (owner, callers(owner, &edges, &after))) + .collect(); + let interpretation = after + .iter() + .filter_map(|(&owner, next)| { + let previous = before.get(&owner)?; + (previous != next).then(|| InterpretationChange { + owner, + before: previous.clone(), + after: next.clone(), + callers: callers(owner, &edges, &after), + }) + }) + .collect(); + Ok(Plan { + command, + removed, + changed, + interpretation, + affected_uses, + }) +} + +#[cfg(test)] +mod tests; diff --git a/crates/oa-desktop/src/behavior/topology/group_removal/tests.rs b/crates/oa-desktop/src/behavior/topology/group_removal/tests.rs new file mode 100644 index 00000000..449bf5e7 --- /dev/null +++ b/crates/oa-desktop/src/behavior/topology/group_removal/tests.rs @@ -0,0 +1,909 @@ +use super::*; +use crate::document::Document; +use oa_core::{MessageKind, source_project::ProjectLimits}; + +fn endpoint(id: Id) -> OccurrenceEndpoint { + OccurrenceEndpoint { + references: vec![], + occurrence: id, + } +} + +fn local(id: Id) -> Target { + Target::ProcessOccurrence(endpoint(id)) +} + +fn add(project: &mut Project, owner: Id, name: &str, body: OccurrenceBody) -> Id { + let element = occurrence(project, owner, name, body); + let id = element.id; + project.elements.insert(id, element); + project + .elements + .get_mut(&owner) + .unwrap() + .steps_mut() + .unwrap() + .push(id); + id +} + +fn action(project: &mut Project, owner: Id, name: &str, work: Id) -> Id { + add( + project, + owner, + name, + OccurrenceBody::Action { + action: Some(work), + lifeline: None, + }, + ) +} + +fn exchange_step(project: &mut Project, owner: Id, name: &str, exchange: Id) -> Id { + add( + project, + owner, + name, + OccurrenceBody::Exchange { + exchange: Some(exchange), + sender: None, + receiver: None, + message_kind: MessageKind::Asynchronous, + reply_to: None, + values: vec![], + operation: None, + }, + ) +} + +fn sequence(project: &mut Project, layer: Layer, name: &str) -> Id { + project + .add_element( + if layer == Layer::Oa { + Kind::Process + } else { + Kind::FunctionalChain + }, + layer, + name, + None, + ) + .unwrap() +} + +fn reuse(project: &mut Project, owner: Id, name: &str, definition: Id) -> Id { + add( + project, + owner, + name, + OccurrenceBody::SequenceReference { + sequence: Some(definition), + }, + ) +} + +fn maintained(project: &mut Project, source: Id, target: Id) -> Id { + let Command::CreateRelation(relation) = link(project, source, target, "authored guard") else { + panic!() + }; + let id = relation.id; + project.relations.insert(id, relation); + id +} + +fn contextual( + project: &mut Project, + owner: Id, + source: OccurrenceEndpoint, + target: OccurrenceEndpoint, + purpose: ContextualLinkPurpose, +) -> Id { + let Command::CreateRelation(mut relation) = + link(project, source.occurrence, target.occurrence, "") + else { + panic!() + }; + let id = relation.id; + relation.body = RelationBody::ContextualLink { + owner, + source, + target, + purpose, + }; + project.relations.insert(id, relation); + id +} + +fn precedence() -> ContextualLinkPurpose { + ContextualLinkPurpose::Precedence { + guard: "caller guard".into(), + } +} + +fn qualified(references: &[Id], occurrence: Id) -> OccurrenceEndpoint { + OccurrenceEndpoint { + references: references.to_vec(), + occurrence, + } +} + +fn interpretation( + local: Option>, + expanded: Option>, + contextual: bool, +) -> Interpretation { + Interpretation { + local_order: local, + expanded_order: expanded, + contextual_analysis: contextual, + } +} + +fn expected_without(project: &Project, owner: Id, elements: &[Id], relations: &[Id]) -> Project { + let mut expected = project.clone(); + for id in elements { + expected.elements.remove(id); + } + for id in relations { + expected.relations.remove(id); + } + expected + .elements + .get_mut(&owner) + .unwrap() + .steps_mut() + .unwrap() + .retain(|id| !elements.contains(id)); + expected +} + +fn assert_history(project: &Project, expected: &Project, command: Command) { + let mut doc = Document::from_project(project.clone(), ProjectLimits::default()).unwrap(); + let original = doc.bundle().clone(); + doc.execute(command).unwrap(); + assert_eq!(doc.project(), expected); + let after = doc.bundle().clone(); + let reopened = Document::from_bundle(after.clone(), ProjectLimits::default()).unwrap(); + assert_eq!(reopened.project(), expected); + assert!(doc.undo()); + assert_eq!(doc.project(), project); + assert_eq!(doc.bundle(), &original); + assert!(!doc.can_undo()); + assert!(doc.redo()); + assert_eq!(doc.project(), expected); + assert_eq!(doc.bundle(), &after); + assert!(!doc.can_redo()); +} + +#[test] +fn process_group_removal_mixed_cards_preserve_sources_definitions_and_other_uses() { + for layer in [Layer::Oa, Layer::Sa] { + let (f, _) = super::super::direct_tests::empty_exchange_fixture(layer); + let mut p = f.p; + let definition = sequence(&mut p, layer, "Reusable"); + let leaf = action(&mut p, definition, "Leaf", f.work[0]); + let a = action(&mut p, f.owner, "Action", f.work[0]); + let e = exchange_step(&mut p, f.owner, "Exchange", f.exchange); + let c = add( + &mut p, + f.owner, + "Control", + OccurrenceBody::Control { + kind: ControlKind::Choice, + }, + ); + let r = reuse(&mut p, f.owner, "First use", definition); + let r2 = reuse(&mut p, f.owner, "Preserved use", definition); + let plan = prepare(&p, f.owner, &[local(r), local(c), local(e), local(a)]).unwrap(); + assert_eq!(plan.removed, BTreeSet::from([a, e, c, r])); + assert_eq!(plan.changed, BTreeSet::from([f.owner])); + assert!(plan.interpretation.is_empty()); + let expected = expected_without(&p, f.owner, &[a, e, c, r], &[]); + assert_eq!(expected.elements[&f.owner].steps().unwrap(), &[r2]); + assert!(expected.contains(leaf)); + assert_history(&p, &expected, plan.command); + } +} + +#[test] +fn process_group_removal_dependent_selected_links_are_normalized_after_whole_set_admission() { + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let [a, b, c] = f.nodes; + let parallel = maintained(&mut p, a, b); + let adjacent = maintained(&mut p, b, c); + let context = contextual( + &mut p, + f.owner, + endpoint(a), + endpoint(b), + ContextualLinkPurpose::Exchange { + exchange: f.exchange, + }, + ); + let targets = vec![ + Target::ProcessLink(f.link), + local(a), + Target::ProcessLink(parallel), + local(b), + Target::ProcessLink(context), + ]; + let first = prepare(&p, f.owner, &targets).unwrap(); + let mut reverse = targets.clone(); + reverse.reverse(); + let second = prepare(&p, f.owner, &reverse).unwrap(); + assert_eq!(first.command, second.command); + assert_eq!( + first.removed, + BTreeSet::from([a, b, f.link, parallel, adjacent, context]) + ); + assert_eq!(first.changed, BTreeSet::from([f.owner])); + assert_eq!( + first.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(None, None, true), + after: interpretation(None, None, false), + callers: BTreeSet::new(), + }] + ); + let Command::Batch(commands) = &first.command else { + panic!() + }; + assert_eq!( + commands.len(), + 2, + "All selected links are covered by admitted card roots" + ); + let expected = + expected_without(&p, f.owner, &[a, b], &[f.link, parallel, adjacent, context]); + assert_history(&p, &expected, first.command); + } +} + +#[test] +fn process_group_removal_refuses_every_invalid_original_typed_member() { + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let other = sequence(&mut p, layer, "Other"); + let foreign = action(&mut p, other, "Foreign", f.work[0]); + let foreign_other = action(&mut p, other, "Other foreign", f.work[0]); + let unlisted = action(&mut p, f.owner, "Owned but unlisted", f.work[0]); + p.elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .retain(|id| *id != unlisted); + let unlisted_link = maintained(&mut p, unlisted, f.nodes[0]); + let foreign_context = contextual( + &mut p, + other, + endpoint(foreign), + endpoint(foreign_other), + ContextualLinkPurpose::CommonWork { work: f.work[0] }, + ); + // Use two separate qualified paths with the same leaf: neither can be + // silently reduced to that leaf or to an outer reference occurrence. + let r1 = reuse(&mut p, f.owner, "Use one", other); + let r2 = reuse(&mut p, f.owner, "Use two", other); + let missing = Id::new_v4(); + let bad = vec![ + Target::Whole, + Target::Placement(f.nodes[0]), + Target::Port { + port: f.work[0], + placement: f.nodes[0], + }, + Target::Edge(f.link), + Target::StateRow { + placement: f.nodes[0], + relation: f.link, + }, + Target::AllocationGrouping { + relation: f.link, + work_placement: f.nodes[0], + performer_placement: f.nodes[1], + }, + Target::ScenarioObject(f.nodes[0]), + Target::ProcessPreviewLink { + relation: f.link, + source: endpoint(f.nodes[0]), + target: endpoint(f.nodes[1]), + }, + Target::ProcessOccurrence(qualified(&[r1], foreign)), + Target::ProcessOccurrence(qualified(&[r2], foreign)), + local(foreign), + local(unlisted), + local(missing), + local(f.work[0]), + Target::ProcessLink(unlisted_link), + Target::ProcessLink(foreign_context), + Target::ProcessLink(f.exchange), + Target::ProcessLink(missing), + Target::ProcessLink(f.nodes[0]), + ]; + oa_core::session::validate_candidate(&p).unwrap(); + let source = oa_core::language::format(&p); + for target in bad { + assert!( + prepare(&p, f.owner, &[local(f.nodes[0]), target.clone()]).is_err(), + "{target:?}" + ); + assert!(prepare(&p, f.owner, &[target, local(f.nodes[0])]).is_err()); + } + assert!(prepare(&p, f.owner, &[]).is_err()); + assert!(prepare(&p, f.owner, &[local(f.nodes[0]), local(f.nodes[0])]).is_err()); + assert!( + prepare( + &p, + f.owner, + &[Target::ProcessLink(f.link), Target::ProcessLink(f.link)] + ) + .is_err() + ); + assert!(prepare(&p, f.work[0], &[local(f.nodes[0])]).is_err()); + assert!(prepare(&p, missing, &[local(f.nodes[0])]).is_err()); + assert_eq!(oa_core::language::format(&p), source); + let mut duplicate_steps = p.clone(); + duplicate_steps + .elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .push(f.nodes[0]); + assert!(prepare(&duplicate_steps, f.owner, &[local(f.nodes[1])]).is_err()); + let mut wrong_parent = p.clone(); + wrong_parent.elements.get_mut(&f.nodes[0]).unwrap().parent = Some(other); + assert!(prepare(&wrong_parent, f.owner, &[local(f.nodes[1])]).is_err()); + let mut missing_step = p.clone(); + missing_step + .elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .push(missing); + assert!(prepare(&missing_step, f.owner, &[local(f.nodes[1])]).is_err()); + } +} + +#[test] +fn process_group_removal_last_markers_and_legacy_interior_report_literal_orders() { + for layer in [Layer::Oa, Layer::Sa] { + let (f, _) = super::super::direct_tests::empty_exchange_fixture(layer); + let mut p = f.p; + let a = action(&mut p, f.owner, "Marker", f.work[0]); + let e1 = exchange_step(&mut p, f.owner, "First", f.exchange); + let c = add( + &mut p, + f.owner, + "Control", + OccurrenceBody::Control { + kind: ControlKind::Choice, + }, + ); + let e2 = exchange_step(&mut p, f.owner, "Middle", f.exchange); + let e3 = exchange_step(&mut p, f.owner, "Last", f.exchange); + let caller = sequence(&mut p, layer, "Caller"); + reuse(&mut p, caller, "Reference", f.owner); + let outer = sequence(&mut p, layer, "Outer"); + reuse(&mut p, outer, "Transitive reference", caller); + let plan = prepare(&p, f.owner, &[local(c), local(a)]).unwrap(); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(None, None, false), + after: interpretation(Some(vec![e1, e2, e3]), Some(vec![e1, e2, e3]), false), + callers: BTreeSet::from([caller, outer]), + }] + ); + let expected = expected_without(&p, f.owner, &[a, c], &[]); + assert_history(&p, &expected, plan.command); + let bridge = prepare(&expected, f.owner, &[local(e2)]).unwrap(); + assert_eq!( + bridge.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(Some(vec![e1, e2, e3]), Some(vec![e1, e2, e3]), false), + after: interpretation(Some(vec![e1, e3]), Some(vec![e1, e3]), false), + callers: BTreeSet::from([caller, outer]), + }] + ); + assert_eq!(bridge.removed, BTreeSet::from([e2])); + assert_history( + &expected, + &expected_without(&expected, f.owner, &[e2], &[]), + bridge.command, + ); + } +} + +#[test] +fn process_group_removal_last_link_and_zero_or_one_exchange_have_no_invented_objects() { + for layer in [Layer::Oa, Layer::Sa] { + let (f, next) = super::super::direct_tests::empty_exchange_fixture(layer); + let mut p = f.p; + let e1 = exchange_step(&mut p, f.owner, "First", f.exchange); + let e2 = exchange_step(&mut p, f.owner, "Second", next); + let link = maintained(&mut p, e1, e2); + let plan = prepare(&p, f.owner, &[Target::ProcessLink(link)]).unwrap(); + assert_eq!(plan.removed, BTreeSet::from([link])); + assert!(plan.changed.is_empty()); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(None, None, false), + after: interpretation(Some(vec![e1, e2]), Some(vec![e1, e2]), false), + callers: BTreeSet::new(), + }] + ); + assert_history( + &p, + &expected_without(&p, f.owner, &[], &[link]), + plan.command, + ); + for remaining in [vec![], vec![e1]] { + let mut model = expected_without(&p, f.owner, &[e2], &[link]); + if remaining.is_empty() { + model = expected_without(&model, f.owner, &[e1], &[]); + } + let marker = action(&mut model, f.owner, "Marker", f.work[0]); + let plan = prepare(&model, f.owner, &[local(marker)]).unwrap(); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(None, None, false), + after: interpretation(Some(remaining.clone()), Some(remaining), false), + callers: BTreeSet::new(), + }] + ); + assert_history( + &model, + &expected_without(&model, f.owner, &[marker], &[]), + plan.command, + ); + } + } +} + +#[test] +fn process_group_removal_global_contextual_dependencies_and_caller_modes_are_separate_from_record_deltas() + { + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let caller = sequence(&mut p, layer, "Caller"); + let r1 = reuse(&mut p, caller, "Use one", f.owner); + let r2 = reuse(&mut p, caller, "Use two", f.owner); + let outer = sequence(&mut p, layer, "Outer"); + let outer_use = reuse(&mut p, outer, "Outer use", caller); + let caller_link = contextual( + &mut p, + caller, + qualified(&[r1], f.nodes[0]), + qualified(&[r2], f.nodes[1]), + precedence(), + ); + let outer_link = contextual( + &mut p, + outer, + qualified(&[outer_use, r1], f.nodes[0]), + qualified(&[outer_use, r2], f.nodes[1]), + precedence(), + ); + let top = sequence(&mut p, layer, "Top"); + reuse(&mut p, top, "Top use", outer); + let plan = prepare(&p, f.owner, &[local(f.nodes[0])]).unwrap(); + assert_eq!( + plan.removed, + BTreeSet::from([f.nodes[0], f.link, caller_link, outer_link]) + ); + assert_eq!(plan.changed, BTreeSet::from([f.owner])); + let by_owner: BTreeMap<_, _> = plan + .interpretation + .iter() + .map(|change| (change.owner, change)) + .collect(); + assert_eq!( + by_owner.keys().copied().collect::>(), + BTreeSet::from([caller, outer, top]) + ); + for id in [caller, outer, top] { + assert_eq!(by_owner[&id].before, interpretation(None, None, true)); + assert_eq!(by_owner[&id].after, interpretation(None, None, false)); + } + assert_eq!(by_owner[&caller].callers, BTreeSet::from([outer, top])); + assert_eq!(by_owner[&outer].callers, BTreeSet::from([top])); + assert!(by_owner[&top].callers.is_empty()); + assert_eq!( + plan.affected_uses, + BTreeMap::from([ + (f.owner, BTreeSet::from([caller, outer, top])), + (caller, BTreeSet::from([outer, top])), + (outer, BTreeSet::from([top])), + ]) + ); + let expected = expected_without( + &p, + f.owner, + &[f.nodes[0]], + &[f.link, caller_link, outer_link], + ); + assert_history(&p, &expected, plan.command); + // Removing an outer reuse removes only that use and paths through it. + let outer_plan = + prepare(&p, caller, &[local(r1), Target::ProcessLink(caller_link)]).unwrap(); + assert_eq!( + outer_plan.removed, + BTreeSet::from([r1, caller_link, outer_link]) + ); + let expected = expected_without(&p, caller, &[r1], &[caller_link, outer_link]); + assert_history(&p, &expected, outer_plan.command); + } +} + +#[test] +fn process_group_removal_qualified_local_relations_preserve_every_purpose_source() { + for layer in [Layer::Oa, Layer::Sa] { + for purpose_kind in 0..3 { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let owner = sequence(&mut p, layer, "Assembly"); + let r1 = reuse(&mut p, owner, "First", f.owner); + let r2 = reuse(&mut p, owner, "Second", f.owner); + let (target, purpose) = match purpose_kind { + 0 => (f.nodes[1], precedence()), + 1 => ( + f.nodes[1], + ContextualLinkPurpose::Exchange { + exchange: f.exchange, + }, + ), + _ => ( + f.nodes[0], + ContextualLinkPurpose::CommonWork { work: f.work[0] }, + ), + }; + let relation = contextual( + &mut p, + owner, + qualified(&[r1], f.nodes[0]), + qualified(&[r2], target), + purpose, + ); + let plan = prepare(&p, owner, &[Target::ProcessLink(relation)]).unwrap(); + assert_eq!(plan.removed, BTreeSet::from([relation])); + assert!(plan.changed.is_empty()); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner, + before: interpretation(None, None, true), + after: interpretation(None, None, false), + callers: BTreeSet::new(), + }] + ); + assert_history( + &p, + &expected_without(&p, owner, &[], &[relation]), + plan.command, + ); + } + } +} + +#[test] +fn process_group_removal_predicate_discriminators_include_contextual_only_and_unlisted_contributions() + { + for layer in [Layer::Oa, Layer::Sa] { + let (f, _) = super::super::direct_tests::empty_exchange_fixture(layer); + let mut p = f.p; + let e1 = exchange_step(&mut p, f.owner, "First", f.exchange); + let e2 = exchange_step(&mut p, f.owner, "Second", f.exchange); + // Core-valid incomplete source: desktop lists only exchanges, while the + // index includes two unlisted actions and their CommonWork relation. + let a1 = action(&mut p, f.owner, "Unlisted one", f.work[0]); + let a2 = action(&mut p, f.owner, "Unlisted two", f.work[0]); + p.elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .retain(|id| *id != a1 && *id != a2); + let relation = contextual( + &mut p, + f.owner, + endpoint(a1), + endpoint(a2), + ContextualLinkPurpose::CommonWork { work: f.work[0] }, + ); + let plan = prepare(&p, f.owner, &[Target::ProcessLink(relation)]).unwrap(); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(Some(vec![e1, e2]), None, true), + after: interpretation(Some(vec![e1, e2]), None, false), + callers: BTreeSet::new(), + }] + ); + let mut p = expected_without(&p, f.owner, &[a1, a2], &[relation]); + let unlisted = action(&mut p, f.owner, "Unlisted marker", f.work[0]); + p.elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .retain(|id| *id != unlisted); + let plan = prepare(&p, f.owner, &[local(e1)]).unwrap(); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(Some(vec![e1, e2]), None, false), + after: interpretation(Some(vec![e2]), None, false), + callers: BTreeSet::new(), + }] + ); + assert!(prepare(&p, f.owner, &[local(unlisted), local(e1)]).is_err()); + let p = expected_without(&p, f.owner, &[unlisted], &[]); + let mut p = p; + let extra = exchange_step(&mut p, f.owner, "Unlisted exchange", f.exchange); + p.elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .retain(|id| *id != extra); + let plan = prepare(&p, f.owner, &[local(e1)]).unwrap(); + assert_eq!( + plan.interpretation, + vec![InterpretationChange { + owner: f.owner, + before: interpretation(Some(vec![e1, e2]), Some(vec![e1, e2, extra]), false), + after: interpretation(Some(vec![e2]), Some(vec![e2, extra]), false), + callers: BTreeSet::new(), + }] + ); + assert_history(&p, &expected_without(&p, f.owner, &[e1], &[]), plan.command); + } +} + +#[test] +fn process_group_removal_explicit_topology_still_reports_transitive_reused_effects() { + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + maintained(&mut p, f.nodes[1], f.nodes[2]); + let caller = sequence(&mut p, layer, "Caller"); + reuse(&mut p, caller, "First use", f.owner); + reuse(&mut p, caller, "Repeated use", f.owner); + let outer = sequence(&mut p, layer, "Outer"); + reuse(&mut p, outer, "Transitive use", caller); + for target in [local(f.nodes[0]), Target::ProcessLink(f.link)] { + let plan = prepare(&p, f.owner, &[target]).unwrap(); + assert!( + plan.interpretation.is_empty(), + "Explicit modes remain unchanged" + ); + assert_eq!( + plan.affected_uses, + BTreeMap::from([(f.owner, BTreeSet::from([caller, outer]))]) + ); + assert!(!plan.changed.contains(&caller)); + assert!(!plan.changed.contains(&outer)); + } + } +} + +#[test] +fn process_group_removal_mixed_covered_and_independent_selected_links_delete_each_once() { + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let [a, b, c] = f.nodes; + let independent = maintained(&mut p, b, c); + let targets = [ + Target::ProcessLink(independent), + Target::ProcessLink(f.link), + local(a), + ]; + let plan = prepare(&p, f.owner, &targets).unwrap(); + let reverse = prepare(&p, f.owner, &targets.into_iter().rev().collect::>()).unwrap(); + assert_eq!(plan.command, reverse.command); + assert_eq!(plan.removed, BTreeSet::from([a, f.link, independent])); + assert_eq!(plan.changed, BTreeSet::from([f.owner])); + let Command::Batch(commands) = &plan.command else { + panic!() + }; + assert_eq!( + commands, + &[Command::Delete(a), Command::Delete(independent)] + ); + assert!(plan.interpretation.is_empty()); + let expected = expected_without(&p, f.owner, &[a], &[f.link, independent]); + assert_eq!(expected.elements[&f.owner].steps().unwrap(), &[b, c]); + assert_history(&p, &expected, plan.command); + } +} + +#[test] +fn process_group_removal_exact_attachments_traces_placements_and_diagram_impact() { + use oa_core::{ArtifactLink, ArtifactRelation, EdgeRoute, Position, Trace, TraceKind}; + + for layer in [Layer::Oa, Layer::Sa] { + let f = super::super::direct_tests::fixture(layer); + let mut p = f.p; + let [a, b, _] = f.nodes; + let overview = p.ensure_diagram(layer); + // Overview explicitly includes Occurrence in ProjectionKind::includes; + // saved placement validation admits these same-layer semantic elements. + assert!(p.diagrams[&overview].projection.includes(Kind::Occurrence)); + let node = p.place(overview, a, Position { x: 100., y: 100. }); + let kept_node = p.place(overview, b, Position { x: 400., y: 100. }); + p.diagrams + .get_mut(&overview) + .unwrap() + .routes + .push(EdgeRoute { + edge: f.link, + source_attachment: None, + target_attachment: None, + waypoints: vec![Position { x: 300., y: 80. }], + label: None, + }); + let other_view = Id::new_v4(); + let mut view = p.diagrams[&overview].clone(); + view.id = other_view; + view.alias = p.fresh_alias("second_overview", "view", other_view); + view.name = "Second overview".into(); + view.nodes.clear(); + view.routes.clear(); + p.diagrams.insert(other_view, view); + let other_node = p.place(other_view, a, Position { x: 220., y: 160. }); + let opposite = if layer == Layer::Oa { + Layer::Sa + } else { + Layer::Oa + }; + let counterpart = p + .add_element( + if opposite == Layer::Oa { + Kind::Activity + } else { + Kind::Function + }, + opposite, + "Preserved counterpart", + None, + ) + .unwrap(); + let unrelated_view = p.ensure_diagram(opposite); + p.place(unrelated_view, counterpart, Position { x: 50., y: 75. }); + let traces: [Id; 2] = [a, b].map(|subject| { + let id = Id::new_v4(); + p.traces.insert( + id, + Trace { + id, + alias: p.fresh_alias("manual_trace", "trace", id), + name: "Manual correspondence".into(), + description: "Preserve counterpart".into(), + oa: if layer == Layer::Oa { + subject + } else { + counterpart + }, + sa: if layer == Layer::Sa { + subject + } else { + counterpart + }, + kind: TraceKind::Manual, + baseline: None, + }, + ); + id + }); + // Include an attachment owned by the dependent placement: core impact + // must follow the fixed point rather than only direct semantic subjects. + let artifacts: [Id; 4] = [a, node, f.link, b].map(|owner| { + let id = Id::new_v4(); + p.artifact_links.insert( + id, + ArtifactLink { + id, + alias: p.fresh_alias("evidence", "artifact", id), + name: "Evidence".into(), + description: "Authored evidence".into(), + owner, + relation: ArtifactRelation::References, + uri: "evidence.txt".into(), + revision: Some("v1".into()), + fragment: Some("review".into()), + }, + ); + id + }); + let constraint = p + .add_element(Kind::Constraint, layer, "Surviving constraint", None) + .unwrap(); + *p.elements + .get_mut(&constraint) + .unwrap() + .shared_mut() + .unwrap() = SharedBody::Constraint { + expression: "review".into(), + constrained: vec![a, b], + }; + oa_core::session::validate_candidate(&p).unwrap(); + let plan = prepare(&p, f.owner, &[local(a)]).unwrap(); + assert_eq!( + plan.removed, + BTreeSet::from([ + a, + f.link, + node, + other_node, + traces[0], + artifacts[0], + artifacts[1], + artifacts[2], + ]) + ); + assert_eq!( + plan.changed, + BTreeSet::from([f.owner, overview, other_view, constraint]) + ); + assert_eq!( + plan.affected_uses, + BTreeMap::from([(f.owner, BTreeSet::new())]) + ); + let mut expected = expected_without(&p, f.owner, &[a], &[f.link]); + expected.traces.remove(&traces[0]); + for id in &artifacts[..3] { + expected.artifact_links.remove(id); + } + expected + .diagrams + .get_mut(&overview) + .unwrap() + .nodes + .remove(&node); + expected.diagrams.get_mut(&overview).unwrap().routes.clear(); + expected + .diagrams + .get_mut(&other_view) + .unwrap() + .nodes + .remove(&other_node); + let Some(SharedBody::Constraint { constrained, .. }) = + expected.elements.get_mut(&constraint).unwrap().shared_mut() + else { + panic!() + }; + *constrained = vec![b]; + assert_eq!(expected.elements[&counterpart], p.elements[&counterpart]); + assert_eq!( + expected.diagrams[&overview].nodes[&kept_node], + p.diagrams[&overview].nodes[&kept_node] + ); + assert_eq!( + expected.diagrams[&unrelated_view], + p.diagrams[&unrelated_view] + ); + assert_eq!(expected.traces[&traces[1]], p.traces[&traces[1]]); + assert_eq!( + expected.artifact_links[&artifacts[3]], + p.artifact_links[&artifacts[3]] + ); + assert_history(&p, &expected, plan.command); + } +} diff --git a/crates/oa-desktop/src/behavior/topology/removal_input.rs b/crates/oa-desktop/src/behavior/topology/removal_input.rs new file mode 100644 index 00000000..c7b599d5 --- /dev/null +++ b/crates/oa-desktop/src/behavior/topology/removal_input.rs @@ -0,0 +1,84 @@ +//! Transient pointer ownership shared by the canvas and its App adapter. +//! +//! Iced queues messages until the entire native event batch has reached widgets. +//! A later press must therefore revoke an earlier removal packet synchronously, +//! without relying on the ordering of two queued application messages. +use std::sync::{ + Arc, + atomic::{AtomicU64, Ordering}, +}; + +#[derive(Clone, Default)] +pub(super) struct Lifetime(Arc); + +impl Lifetime { + /// Even generations are idle; odd generations belong to one live gesture. + pub(super) fn idle(&self) -> Option { + let generation = self.0.load(Ordering::SeqCst); + (generation % 2 == 0).then_some(generation) + } + + /// Retire an idle packet at a native input boundary (including the + /// viewport's synthetic Unfocused event when it takes camera ownership). + pub(super) fn revoke_idle(&self) { + if let Some(generation) = self.idle() { + // Keep overflow terminal and unavailable rather than reusing zero. + let next = generation.checked_add(2).unwrap_or(u64::MAX); + let _ = self + .0 + .compare_exchange(generation, next, Ordering::SeqCst, Ordering::SeqCst); + } + } + + pub(super) fn acquire(&self) -> Option { + let generation = self.idle()?; + let active = generation.checked_add(1)?; + self.0 + .compare_exchange(generation, active, Ordering::SeqCst, Ordering::SeqCst) + .ok()?; + Some(Lease { + lifetime: self.clone(), + active, + }) + } +} + +pub(super) struct Lease { + lifetime: Lifetime, + active: u64, +} + +impl Drop for Lease { + fn drop(&mut self) { + // A terminal odd generation stays unavailable on overflow. No stale + // packet can become valid again through counter wraparound. + if let Some(next) = self.active.checked_add(1) { + let _ = self.lifetime.0.compare_exchange( + self.active, + next, + Ordering::SeqCst, + Ordering::SeqCst, + ); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn removal_input_lease_revokes_idle_capture_and_drop_never_reuses_it() { + let lifetime = Lifetime::default(); + let captured = lifetime.idle().unwrap(); + let clone = lifetime.clone(); + let lease = clone.acquire().unwrap(); + assert!(lifetime.idle().is_none()); + assert!(lifetime.acquire().is_none()); + drop(lease); + assert!(lifetime.idle().is_some()); + assert_ne!(lifetime.idle(), Some(captured)); + let after_drop = lifetime.idle(); + lifetime.revoke_idle(); + assert_ne!(lifetime.idle(), after_drop); + } +} diff --git a/crates/oa-desktop/src/behavior/topology/view.rs b/crates/oa-desktop/src/behavior/topology/view.rs index 5f05635a..9f992cb5 100644 --- a/crates/oa-desktop/src/behavior/topology/view.rs +++ b/crates/oa-desktop/src/behavior/topology/view.rs @@ -185,6 +185,7 @@ pub(super) fn view<'a>(app: &'a App, owner: Id, out: Column<'a, Message>) -> Ele &state.contextual_expanded, ); graph.selection = app.multiselect_snapshot(); + graph.removal_input = state.removal_input.clone(); graph.selected_preview = state .qualified_selection(p, owner) .filter(|address| { @@ -574,6 +575,15 @@ fn selection_controls(app: &App) -> Element<'_, Message> { .align_y(iced::Center) .height(32); if let Some(snapshot) = snapshot.filter(|s| !s.targets().is_empty()) { + if snapshot.targets().len() > 1 { + controls = controls.push( + button("Remove selected…").on_press_maybe( + app.process_removal_enabled() + .then(|| crate::app::process_removal::begin(app)) + .flatten(), + ), + ); + } controls = controls.push(button("Clear selection").on_press_maybe( enabled.then(|| Message::DiagramSelection(SelectionEvent::Clear(snapshot.context()))), )); diff --git a/crates/oa-desktop/src/model_menu.rs b/crates/oa-desktop/src/model_menu.rs index a401dcf9..bf129e93 100644 --- a/crates/oa-desktop/src/model_menu.rs +++ b/crates/oa-desktop/src/model_menu.rs @@ -77,7 +77,7 @@ impl Opened { { return None; } - let scenario_selection = if request.scenario.is_some() { + let scenario_selection = if request.scenario.is_some() || request.process.is_some() { Some(app.multiselect_snapshot()?) } else { None @@ -437,21 +437,23 @@ pub fn popup(app: &App) -> Option> { true, )); if let Some(snapshot) = app.multiselect_snapshot().filter(|s| s.targets().len() > 1) - && selected + && (selected || request.process.is_some()) { entries.push(Entry::title(format!( "{} selected", snapshot.targets().len() ))); - entries.push(item( - "Inspect this element", - Message::DiagramSelection(SelectionEvent::Choose( - context, - target.clone(), - Choice::Inspect, - )), - true, - )); + if selected { + entries.push(item( + "Inspect this element", + Message::DiagramSelection(SelectionEvent::Choose( + context, + target.clone(), + Choice::Inspect, + )), + true, + )); + } entries.push(item( "Select only this element", Message::DiagramSelection(SelectionEvent::Choose( @@ -465,9 +467,13 @@ pub fn popup(app: &App) -> Option> { entries.push(Entry::title( "Drag contributions within one column to reorder", )); - entries.push(Entry::disabled( - "Hide unavailable · cards are model contributions", - )); + if let Some(removal) = crate::app::process_removal::begin(app) { + entries.push(item( + "Remove selected contributions / links…", + removal, + true, + )); + } } else { entries.push(item( "Hide selected placements", @@ -475,9 +481,11 @@ pub fn popup(app: &App) -> Option> { crate::app::multiselect::placements(&snapshot).is_some(), )); } - entries.push(Entry::disabled( - "Delete selected from model (not yet available)", - )); + if request.process.is_none() { + entries.push(Entry::disabled( + "Delete selected from model (not yet available)", + )); + } return Some(Menu { token: opened.token, anchor: request.anchor, diff --git a/crates/oa-desktop/src/views.rs b/crates/oa-desktop/src/views.rs index a1caee4c..c03006d2 100644 --- a/crates/oa-desktop/src/views.rs +++ b/crates/oa-desktop/src/views.rs @@ -326,6 +326,13 @@ pub fn view(app: &App) -> View<'_, Message> { ); } if let Some(target) = app.delete_target.as_ref().filter(|_| app.delete_pending) { + if target.process.is_some() { + return modal_width( + root, + crate::app::process_removal::view::view(app, target), + 700., + ); + } if target.scenario.is_some() { return modal_width( root, diff --git a/docs/language.rst b/docs/language.rst index 028db93b..8636ba5c 100644 --- a/docs/language.rst +++ b/docs/language.rst @@ -2292,6 +2292,26 @@ A qualified selection inside an expanded reusable process MUST NOT silently delete the outer reference or shared definition item. The editor directs the user to open the definition or explicitly select the outer reference. +For a group of local Process or FunctionalChain contributions and maintained +links, removal MUST admit every original typed selection member before reducing +overlapping cascade commands. A selected link already removed by a selected +occurrence's cascade remains a selected effect in the review. A mixed local and +qualified selection MUST refuse as a whole, without a primary-only fallback. +A locally owned ContextualLink is distinct from a read-only relation displayed +inside a reused definition; its explicit owner governs removal even when both +endpoint paths are qualified. + +The aggregate review MUST identify actual deleted records, changed surviving +records, and affected caller definitions. Removing the last explicit topology +marker can restore an exchange-only sequence's implicit ordered meaning; +removing an interior exchange from a legacy order changes adjacent pairs. +These consequences MUST be explicit before confirmation. Reused topology and +entry/exit connections can also change without an interpretation-mode switch +or new stored relation. Caller-definition counts MUST NOT be presented as +counts of reference instances. Empty or incomplete surviving sequences retain +their ordinary diagnostics; deletion MUST NOT invent replacement controls or +links to conceal such consequences. + Confirmation identifies the captured object and removal effect. Cancel and Escape preserve model, source and history. A confirmation token belongs to one dialog; changes to its document, revision, owner or selected identity invalidate diff --git a/harness/fixtures/process-removal-baseline-20260925.arcadia b/harness/fixtures/process-removal-baseline-20260925.arcadia new file mode 100644 index 00000000..9570e150 --- /dev/null +++ b/harness/fixtures/process-removal-baseline-20260925.arcadia @@ -0,0 +1,22 @@ +arcadia 7; +project process_removal_trial "Process removal trial" id "93600000-0000-4000-8000-000000000001" { + oa_activity intake "Intake" id "93600000-0000-4000-8000-000000000002" { } + oa_activity assess "Assess" id "93600000-0000-4000-8000-000000000003" { } + oa_activity report "Report" id "93600000-0000-4000-8000-000000000004" { } + interaction received "Received" id "93600000-0000-4000-8000-000000000005" { source @intake; target @assess; } + interaction reported "Reported" id "93600000-0000-4000-8000-000000000006" { source @assess; target @report; } + oa_process assessment "01 Assessment" id "93600000-0000-4000-8000-000000000010" { steps [@a,@e1,@c,@e2]; } + oa_occurrence a "Review gate" id "93600000-0000-4000-8000-000000000011" { parent @assessment; body { Action { action @assess; lifeline none; }; }; } + oa_occurrence e1 "Receive report" id "93600000-0000-4000-8000-000000000012" { parent @assessment; body { Exchange { exchange @received; sender none; receiver none; message_kind Asynchronous; }; }; } + oa_occurrence c "Review gate" id "93600000-0000-4000-8000-000000000013" { parent @assessment; body { Control { kind Choice; }; }; } + oa_occurrence e2 "Forward report" id "93600000-0000-4000-8000-000000000014" { parent @assessment; body { Exchange { exchange @reported; sender none; receiver none; message_kind Asynchronous; }; }; } + sequence_link approved "Gate approval" id "93600000-0000-4000-8000-000000000015" { source @a; target @c; guard "approved"; } + oa_process caller "02 Response using Assessment" id "93600000-0000-4000-8000-000000000020" { steps [@r1,@r2]; } + oa_occurrence r1 "First assessment" id "93600000-0000-4000-8000-000000000021" { parent @caller; body { SequenceReference { sequence @assessment; }; }; } + oa_occurrence r2 "Second assessment" id "93600000-0000-4000-8000-000000000022" { parent @caller; body { SequenceReference { sequence @assessment; }; }; } + oa_process outer "03 Mission using Response" id "93600000-0000-4000-8000-000000000030" { steps [@r3]; } + oa_occurrence r3 "Response use" id "93600000-0000-4000-8000-000000000031" { parent @outer; body { SequenceReference { sequence @caller; }; }; } + sa_function sentinel "SA preserved function" id "93600000-0000-4000-8000-000000000040" { } + sa_functional_chain sentinel_chain "SA preserved chain" id "93600000-0000-4000-8000-000000000041" { steps [@sentinel_step]; } + sa_occurrence sentinel_step "SA preserved contribution" id "93600000-0000-4000-8000-000000000042" { parent @sentinel_chain; body { Action { action @sentinel; lifeline none; }; }; } +} diff --git a/harness/fixtures/process-removal-removed-20260925.arcadia b/harness/fixtures/process-removal-removed-20260925.arcadia new file mode 100644 index 00000000..585a44a9 --- /dev/null +++ b/harness/fixtures/process-removal-removed-20260925.arcadia @@ -0,0 +1,19 @@ +arcadia 7; +project process_removal_trial "Process removal trial" id "93600000-0000-4000-8000-000000000001" { + oa_activity intake "Intake" id "93600000-0000-4000-8000-000000000002" { } + oa_activity assess "Assess" id "93600000-0000-4000-8000-000000000003" { } + oa_activity report "Report" id "93600000-0000-4000-8000-000000000004" { } + interaction received "Received" id "93600000-0000-4000-8000-000000000005" { source @intake; target @assess; } + interaction reported "Reported" id "93600000-0000-4000-8000-000000000006" { source @assess; target @report; } + oa_process assessment "01 Assessment" id "93600000-0000-4000-8000-000000000010" { steps [@e1, @e2]; } + oa_occurrence e1 "Receive report" id "93600000-0000-4000-8000-000000000012" { parent @assessment; body { Exchange { exchange @received; sender none; receiver none; message_kind Asynchronous; }; }; } + oa_occurrence e2 "Forward report" id "93600000-0000-4000-8000-000000000014" { parent @assessment; body { Exchange { exchange @reported; sender none; receiver none; message_kind Asynchronous; }; }; } + oa_process caller "02 Response using Assessment" id "93600000-0000-4000-8000-000000000020" { steps [@r1,@r2]; } + oa_occurrence r1 "First assessment" id "93600000-0000-4000-8000-000000000021" { parent @caller; body { SequenceReference { sequence @assessment; }; }; } + oa_occurrence r2 "Second assessment" id "93600000-0000-4000-8000-000000000022" { parent @caller; body { SequenceReference { sequence @assessment; }; }; } + oa_process outer "03 Mission using Response" id "93600000-0000-4000-8000-000000000030" { steps [@r3]; } + oa_occurrence r3 "Response use" id "93600000-0000-4000-8000-000000000031" { parent @outer; body { SequenceReference { sequence @caller; }; }; } + sa_function sentinel "SA preserved function" id "93600000-0000-4000-8000-000000000040" { } + sa_functional_chain sentinel_chain "SA preserved chain" id "93600000-0000-4000-8000-000000000041" { steps [@sentinel_step]; } + sa_occurrence sentinel_step "SA preserved contribution" id "93600000-0000-4000-8000-000000000042" { parent @sentinel_chain; body { Action { action @sentinel; lifeline none; }; }; } +} diff --git a/harness/multiselect-contract-20260923.md b/harness/multiselect-contract-20260923.md index 2302b24d..6915d032 100644 --- a/harness/multiselect-contract-20260923.md +++ b/harness/multiselect-contract-20260923.md @@ -110,6 +110,11 @@ whole-set local admission, aggregate impact and one-step history. Its tested source, mixed participant/message workflow, exact saved-source recovery and explicit coverage limits; default-branch integration remains separate. +The [Process removal policy](process-group-removal-policy-20260925.md) defines +local occurrence/link admission, cascade normalization, implicit-order and +caller consequences, and captured one-step removal. It is a development policy; +its existence does not establish App, native or integration acceptance. + ## Acceptance and dependencies 1. Prove simultaneous geometry planning with unrelated roots/internal routes, diff --git a/harness/process-group-removal-policy-20260925.md b/harness/process-group-removal-policy-20260925.md new file mode 100644 index 00000000..8e840b1f --- /dev/null +++ b/harness/process-group-removal-policy-20260925.md @@ -0,0 +1,129 @@ +# Process and FunctionalChain group removal + +This policy refines the local Process part of the [multiple-selection +contract](multiselect-contract-20260923.md) and the focused-removal semantics in +[language section 8.1](../docs/language.rst). It is an implementation decision, +not runtime, native, integrated or release acceptance. + +## Exact admission and one transaction + +Remove the complete captured group of local contribution occurrences and +maintained links. An occurrence must be a unique listed direct child of the +active Process or FunctionalChain, with the matching layer and an Action, +Exchange, Control or SequenceReference body. A SequenceLink must connect listed +local contributions of that same owner. A ContextualLink is local when its +explicit owner matches, even if both endpoint addresses are qualified. Its +Precedence, Exchange and CommonWork purposes retain their distinct meanings. + +A qualified ProcessOccurrence or ProcessPreviewLink is not an editable local +object. Refuse the whole mixed selection; never strip its reference path, edit +the reused definition, select an outer reference implicitly, or fall back to +the primary. Open the defining sequence to edit its contents. Legacy ordered +arrows have no maintained relation identity and cannot be deleted as links. + +Admit every original typed target before normalizing execution. Selected local +occurrences are independent deletion roots. Compute their existing core +deletion-impact union, and omit a selected relation command only when that +relation is already covered by this union. It remains a selected removal in the +review. Blindly deleting both an occurrence and its incident selected link would +fail because the first cascade already removed the second identity. + +Evaluate one Batch with the core command engine and final structural validation. +Derive deleted identities and changed surviving records from the actual final +candidate, including embedded diagram placements. All selected identities must +be removed and no identity may be created. Preserve referenced work, exchanges, +sequence definitions, other uses and opposite-layer counterparts. Global +dependent ContextualLinks, attachments and traces follow existing core cascade +rules; never shorten their paths or silently retarget them. + +## Order and caller consequences + +Deletion can change meaning without creating any new stored record. Given +`[Action A, Exchange E1, Control C, Exchange E2]` and no maintained links, +removing A and C leaves implicit E1-to-E2 order. Removing the last authored +SequenceLink on an exchange-only list likewise restores implicit order, which +may retain a visible connection that the user expected to remove. Removing E2 +from legacy `[E1, E2, E3]` creates the implicit E1-to-E3 adjacency. A remainder +with zero or one exchange changes interpretation without adding an edge. + +The review therefore compares before/after interpretation for every surviving +Process and FunctionalChain, independently of whether its stored record changed: + +- Local implicit order follows the existing desktop listed-step/SequenceLink + predicate. Explicit mode is distinct from an empty implicit sequence. +- Reused-definition implicit order follows the core contextual expansion + predicate: indexed contributions (including owned but unlisted contributions) + must all be exchanges and the definition must have no maintained SequenceLink + or ContextualLink. This is a definition-level order fact, not a claim that the + entire expanded graph was analyzed or is connected. +- Contextual analysis follows the core index's transitive contextual-link + presence. Losing the last such link is reported separately from either order + predicate; it does not necessarily mean a local explicit-to-implicit switch. + +For each changed interpretation, identify surviving direct and transitive +callers through the union of before/after SequenceReference dependencies. Such +callers can be affected while their stored records remain unchanged. Keep these +effects separate from removed/updated record counts and make the full affected +set searchable and pageable, with full names and aliases. + +Also seed caller impact from definitions whose contributions or maintained links +are removed or changed, even when all three interpretation fields stay the same. +For example, removing A from explicit A-to-B-to-C changes every use of that +definition while the remaining B-to-C link keeps its mode explicit. Label the +caller effect as using a changed definition; do not imply that complete expanded +connectivity or boundary analysis was performed. Interpretation-only detection +would miss this ordinary case and is rejected. + +Explain that reused topology, entry/exit connections and diagnostics may change. +Deleting A-to-B while B-to-C remains can make B a new intrinsic entry and thus +add an effective reference-boundary connection in callers. No new stored +identity does not mean no new effective connection. + +Caller rows identify definitions, not individual SequenceReference instances; +two references to P in Q still yield one Q row. Never present that count as the +number of uses. An empty expanded-order classification supplies no edges and +does not establish a valid flow or boundary: core expansion of an empty sequence +is incomplete. Say that no contributions/ordered pairs remain, preserving the +ordinary completeness diagnostics. + +Incomplete models may have owned but unlisted contributions. Selected unlisted +cards are ineligible, but other such contributions must not be ignored in impact +computation. The local and expanded predicates can differ; report their actual +results separately rather than falsely treating them as one execution mode. +This operation does not resolve that broader semantic inconsistency or suppress +existing diagnostics. + +Alternatives considered: rejecting all interpretation changes prevents normal +editing of valid incomplete models; silently accepting them hides an engineering +consequence; inventing sentinel controls or replacement links manufactures model +objects; a new persisted topology-mode field requires an independent language, +migration and API decision. Explicitly reviewing the existing source-derived +consequences preserves graphical/textual parity without introducing new objects. + +## Capture, review and verification + +The App captures document, revision, owner, canvas epoch, exact selection +generation, membership and primary, and verifies actual rendered visibility. +Pending source/property/behavior edits, gestures, other modals and non-selection +tools refuse admission. Header, context menu, focused Delete and Shift+Delete +must all capture the complete group before ordinary singleton handlers can act. +Invoking a menu over an unselected object does not replace the existing group. + +Only explicit confirmation of the current token commits one Document +transaction. Recheck complete admission before applying it. Search, page, +Cancel and Confirm packets belong to their review token. Enter in search does +not implicitly confirm deletion. Cancel, stale/refused events and source-write +failure preserve the accepted model/source/history, selection and camera. +Success returns inspection to the owner and retires removed transient selection; +one model Undo/Redo restores the exact complete source and model. + +Required discriminators include OA and SA mixed occurrence/link groups; +cascade-covered selected links in different click/UUID orders; repeated reuse +and globally dependent links; all ContextualLink purposes; local plus qualified +whole-set refusal; explicit-to-implicit and legacy adjacency changes; zero/one +remainders; contextual-analysis-only changes; surviving transitive callers; +owned-unlisted contributions; stale secondary selection and tokens; pending +drafts; exact cancel/failure/Undo/Redo/save/reopen behavior. Tests must compare +literal expected identities and models, not only counts or the planner's own +output. Native acceptance must inspect meaningful review wording, full impact +reachability, focus, compact layout, cancellation and persistence.