From 3923860ad2923ad2bcae93b2f0c3ccbcf02d7d75 Mon Sep 17 00:00:00 2001 From: Codex-ytr83r Date: Thu, 24 Sep 2026 18:56:16 -0700 Subject: [PATCH] Experiment: reset visible route pins and labels during Layout --- crates/oa-core/src/session.rs | 19 ++- crates/oa-core/tests/release_routing.rs | 196 +++++++++++++++++++++++- crates/oa-core/tests/state_diagrams.rs | 42 +++++ 3 files changed, 250 insertions(+), 7 deletions(-) diff --git a/crates/oa-core/src/session.rs b/crates/oa-core/src/session.rs index c9f65c7d..245230cd 100644 --- a/crates/oa-core/src/session.rs +++ b/crates/oa-core/src/session.rs @@ -952,6 +952,16 @@ fn layout(p: &mut Project, diagram: Id) -> Result<(), ModelError> { "Scenario layout requires the Scenario geometry operation.".into(), )); } + // Freeze reset eligibility before this operation changes geometry. In a + // Batch this is the staged input, not an independently validated prefix. + let view = crate::projection::project_view(p, diagram); + let visible_nodes: BTreeSet<_> = view.nodes.iter().map(|node| node.id).collect(); + let reset_routes: BTreeSet<_> = view.edges.iter().filter_map(|edge| { + let owners = crate::projection::edge_endpoint_placements(p, d, edge.id)?; + (owners == (edge.source, edge.target) + && visible_nodes.contains(&edge.source) + && visible_nodes.contains(&edge.target)).then_some(edge.id) + }).collect(); let parents = crate::projection::parents(p, d); let mut children: BTreeMap, Vec> = BTreeMap::new(); for (id, parent) in &parents { @@ -1044,8 +1054,7 @@ fn layout(p: &mut Project, diagram: Id) -> Result<(), ModelError> { } node }; - let edges: BTreeSet<_> = crate::projection::project_view(p, diagram) - .edges + let edges: BTreeSet<_> = view.edges .iter() .filter_map(|edge| { let source = root_of(edge.source); @@ -1138,5 +1147,11 @@ fn layout(p: &mut Project, diagram: Id) -> Result<(), ModelError> { *existing = route; } } + for route in &mut d.routes { + if reset_routes.contains(&route.edge) { + route.waypoints.clear(); + route.label = None; + } + } Ok(()) } diff --git a/crates/oa-core/tests/release_routing.rs b/crates/oa-core/tests/release_routing.rs index 1a16dfe0..6822c4b1 100644 --- a/crates/oa-core/tests/release_routing.rs +++ b/crates/oa-core/tests/release_routing.rs @@ -649,7 +649,7 @@ fn group_translation_beyond_coordinate_limit_rolls_back_nodes_and_routes() { } #[test] -fn layout_preserves_authored_route_count_order_labels_and_undo() { +fn layout_resets_visible_pins_labels_preserving_route_count_order_and_undo() { let f = routed(fixture()); let mut session = Session::new(f.project.clone()); session.execute(Command::Layout(f.diagram)).unwrap(); @@ -657,10 +657,10 @@ fn layout_preserves_authored_route_count_order_labels_and_undo() { assert_eq!(routes.len(), 2); assert_eq!(routes[0].edge, f.edge); assert_eq!(routes[1].edge, f.external); - assert_eq!(routes[0].waypoints.len(), 2); - assert_eq!(routes[1].waypoints.len(), 2); - assert_eq!(routes[0].label, route(f.edge).label); - assert_eq!(routes[1].label, route(f.external).label); + assert!(routes[0].waypoints.is_empty()); + assert!(routes[1].waypoints.is_empty()); + assert_eq!(routes[0].label, None); + assert_eq!(routes[1].label, None); assert_eq!( session.project().diagrams[&f.second], f.project.diagrams[&f.second] @@ -669,6 +669,192 @@ fn layout_preserves_authored_route_count_order_labels_and_undo() { assert_eq!(session.project(), &f.project); } +#[test] +fn layout_keeps_attachments_port_anchors_empty_entries_and_exact_history() { + let mut f = routed(fixture()); + let extra = f.project.relate(RelationKind::FunctionalExchange, "Attachment only", f.a, f.b); + let empty = f.project.relate(RelationKind::FunctionalExchange, "Explicit empty", f.a, f.b); + let attachment = EdgeAttachment { side: PortAnchorSide::Bottom, offset: 0.23 }; + let opposite = EdgeAttachment { side: PortAnchorSide::Top, offset: 0.81 }; + let mut fraction_only = route(f.edge); + fraction_only.waypoints.clear(); + fraction_only.label = Some(EdgeLabel { fraction: 0.73, offset: Position { x: 0., y: 0. } }); + fraction_only.source_attachment = Some(attachment); + fraction_only.target_attachment = Some(opposite); + let attachment_only = EdgeRoute { edge: extra, source_attachment: Some(attachment), + target_attachment: None, waypoints: vec![], label: None }; + let empty_route = EdgeRoute { edge: empty, source_attachment: None, + target_attachment: None, waypoints: vec![], label: None }; + f.project.diagrams.get_mut(&f.diagram).unwrap().routes = + vec![route(f.external), fraction_only.clone(), attachment_only.clone(), empty_route.clone()]; + let RelationBody::FunctionalExchange { source_port, .. } = f.project.relations[&f.edge].body + else { unreachable!() }; + let owner = node(&f.project, f.diagram, f.a); + let anchor = PortAnchor { port: source_port, side: PortAnchorSide::Left, offset: 0.37 }; + f.project.diagrams.get_mut(&f.diagram).unwrap().nodes.get_mut(&owner).unwrap() + .port_anchors.push(anchor.clone()); + validate_candidate(&f.project).unwrap(); + let original_source = language::format(&f.project); + let mut session = Session::new(f.project.clone()); + session.execute(Command::Layout(f.diagram)).unwrap(); + let after = session.project().clone(); + let mut expected_external = route(f.external); + expected_external.waypoints.clear(); + expected_external.label = None; + fraction_only.label = None; + assert_eq!(after.diagrams[&f.diagram].routes, + vec![expected_external, fraction_only, attachment_only, empty_route]); + assert_eq!(after.diagrams[&f.diagram].nodes[&owner].port_anchors, vec![anchor]); + assert_eq!(after.elements, f.project.elements); + assert_eq!(after.relations, f.project.relations); + assert_eq!(after.diagrams[&f.second], f.project.diagrams[&f.second]); + assert_eq!(language::parse(&language::format(&after)).unwrap(), after); + let command: Command = serde_json::from_value(serde_json::json!({"Layout": f.diagram})).unwrap(); + assert_eq!(execute_batch(&f.project, &[command]).unwrap(), after); + session.undo(); + assert_eq!(session.project(), &f.project); + assert_eq!(language::format(session.project()), original_source); + assert!(!session.can_undo()); + session.redo(); + assert_eq!(session.project(), &after); +} + +fn shift_all(p: &mut Project, d: Id, x: f32, y: f32) { + for placement in p.diagrams.get_mut(&d).unwrap().nodes.values_mut() { + placement.position.x += x; + placement.position.y += y; + } +} + +fn translated_expected(mut route: EdgeRoute, x: f32, y: f32) -> EdgeRoute { + for point in &mut route.waypoints { + point.x += x; + point.y += y; + } + route +} + +#[test] +fn layout_resets_only_visible_actual_owners_and_retains_filtered_and_missing_routes() { + let mut f = fixture(); + let outside = f.project.relations[&f.external].target(); + let visible = f.project.relate(RelationKind::FunctionalExchange, "Visible overview", f.group, outside); + let absent = f.project.add_element(Kind::Function, Layer::Sa, "Not placed", None).unwrap(); + let missing = f.project.relate(RelationKind::FunctionalExchange, "Dormant missing owner", f.b, absent); + f.project.diagrams.get_mut(&f.diagram).unwrap().view_options.hidden_edges = vec![f.edge, f.external]; + // Freeze a geometry-only starting point before adding sentinels. The oracle + // below names the eligible edge and exact authored deltas independently. + let base = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + f.project = base.clone(); + shift_all(&mut f.project, f.diagram, 40., 60.); + let outside_node = node(&f.project, f.diagram, outside); + f.project.diagrams.get_mut(&f.diagram).unwrap().nodes.get_mut(&outside_node).unwrap().position.x += 7.; + f.project.diagrams.get_mut(&f.diagram).unwrap().routes = + vec![route(visible), route(f.edge), route(f.external), route(missing)]; + let view = projection::project_view(&f.project, f.diagram); + assert_eq!(view.edges.iter().map(|edge| edge.id).collect::>(), vec![visible]); + let after = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + assert_eq!(after.diagrams[&f.diagram].nodes, base.diagrams[&f.diagram].nodes); + let mut reset = route(visible); + reset.waypoints.clear(); + reset.label = None; + assert_eq!(after.diagrams[&f.diagram].routes, vec![reset, + translated_expected(route(f.edge), -40., -60.), route(f.external), route(missing)]); + assert_eq!(after.diagrams[&f.second], f.project.diagrams[&f.second]); +} + +#[test] +fn layout_folded_routes_use_actual_owner_deltas_without_resetting_promoted_edges() { + let mut f = routed(fixture()); + let group = node(&f.project, f.diagram, f.group); + let outside = f.project.relations[&f.external].target(); + let outside_node = node(&f.project, f.diagram, outside); + f.project.diagrams.get_mut(&f.diagram).unwrap().nodes.get_mut(&group).unwrap().collapsed = true; + f.project.diagrams.get_mut(&f.diagram).unwrap().routes.clear(); + let base = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + f.project = base.clone(); + shift_all(&mut f.project, f.diagram, 40., 60.); + // The visible container's displacement differs from its hidden children. + f.project.diagrams.get_mut(&f.diagram).unwrap().nodes.get_mut(&group).unwrap().position.x += 13.; + f.project.diagrams.get_mut(&f.diagram).unwrap().nodes.get_mut(&outside_node).unwrap().position.x += 7.; + f.project.diagrams.get_mut(&f.diagram).unwrap().routes = vec![route(f.edge), route(f.external)]; + let view = projection::project_view(&f.project, f.diagram); + assert!(!view.edges.iter().any(|edge| edge.id == f.edge)); + let promoted = view.edges.iter().find(|edge| edge.id == f.external).unwrap(); + assert_eq!(promoted.source, group); + let after = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + assert_eq!(after.diagrams[&f.diagram].nodes, base.diagrams[&f.diagram].nodes); + assert_eq!(after.diagrams[&f.diagram].routes, + vec![translated_expected(route(f.edge), -40., -60.), route(f.external)]); + assert_eq!(after.diagrams[&f.second], f.project.diagrams[&f.second]); +} + +#[test] +fn layout_batch_uses_staged_visibility_and_one_final_validation_history_boundary() { + let f = routed(fixture()); + let mut hidden = f.project.diagrams[&f.diagram].clone(); + hidden.view_options.hidden_edges = vec![f.edge, f.external]; + let mut invalid = f.project.elements[&f.a].clone(); + invalid.alias = f.project.elements[&f.b].alias.clone(); + // Duplicate alias is repaired later in the same transaction. Layout must + // not add prefix validation, nor reset routes hidden by the preceding edit. + let mut session = Session::new(f.project.clone()); + session.execute(Command::Batch(vec![ + Command::EditElement(f.a, invalid), + Command::EditDiagram(f.diagram, hidden), + Command::Layout(f.diagram), + Command::EditElement(f.a, f.project.elements[&f.a].clone()), + ])).unwrap(); + let after = session.project().clone(); + assert_eq!(after.diagrams[&f.diagram].routes.iter().map(|r| (r.edge, r.waypoints.len(), r.label)).collect::>(), + vec![(f.edge, 2, route(f.edge).label), (f.external, 2, route(f.external).label)]); + session.undo(); + assert_eq!(session.project(), &f.project); + assert!(!session.can_undo()); + session.redo(); + assert_eq!(session.project(), &after); + let mut bad = after.elements[&f.a].clone(); + bad.alias = after.elements[&f.b].alias.clone(); + let revision = session.revision(); + assert!(session.execute(Command::Batch(vec![Command::Layout(f.diagram), Command::EditElement(f.a, bad)])).is_err()); + assert_eq!(session.project(), &after); + assert_eq!(session.revision(), revision); +} + +#[test] +fn layout_outside_projection_routes_keep_overrides_and_normal_translation() { + let mut f = fixture(); + f.project.diagrams.get_mut(&f.diagram).unwrap().projection = ProjectionKind::Decomposition; + let base = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + f.project = base.clone(); + shift_all(&mut f.project, f.diagram, 40., 60.); + f.project.diagrams.get_mut(&f.diagram).unwrap().routes = vec![route(f.edge), route(f.external)]; + assert!(projection::project_view(&f.project, f.diagram).edges.iter() + .all(|edge| edge.id != f.edge && edge.id != f.external)); + let after = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + assert_eq!(after.diagrams[&f.diagram].nodes, base.diagrams[&f.diagram].nodes); + assert_eq!(after.diagrams[&f.diagram].routes, vec![ + translated_expected(route(f.edge), -40., -60.), + translated_expected(route(f.external), -40., -60.)]); +} + +#[test] +fn identical_layout_transaction_keeps_redo() { + let mut p = Project::empty(); + let element = p.add_element(Kind::Function, Layer::Sa, "Unplaced", None).unwrap(); + let diagram = p.ensure_diagram(Layer::Sa); + let mut changed = p.elements[&element].clone(); + changed.name = "Renamed unplaced".into(); + let mut session = Session::new(p.clone()); + session.execute(Command::EditElement(element, changed)).unwrap(); + session.undo(); + let revision = session.revision(); + session.execute(Command::Batch(vec![Command::Layout(diagram), Command::Layout(diagram)])).unwrap(); + assert_eq!(session.project(), &p); + assert_eq!(session.revision(), revision); + assert!(session.can_redo()); +} + #[test] fn old_models_get_empty_routes_and_route_json_is_strict() { for bytes in [ diff --git a/crates/oa-core/tests/state_diagrams.rs b/crates/oa-core/tests/state_diagrams.rs index 7aef6d7a..c803f080 100644 --- a/crates/oa-core/tests/state_diagrams.rs +++ b/crates/oa-core/tests/state_diagrams.rs @@ -704,6 +704,48 @@ fn moving_a_region_does_not_translate_pins_attached_to_a_stationary_state_bounda assert_eq!(after.diagrams[&f.d].routes[0].waypoints, pins); } +#[test] +fn layout_retains_hidden_state_boundary_pins_and_internal_routes() { + let mut f = fixture(); + f.p.diagrams.get_mut(&f.d).unwrap().view_options.hidden_edges = vec![f.local]; + let baseline = execute_batch(&f.p, &[Command::Layout(f.d)]).unwrap(); + f.p = baseline.clone(); + let entry = node(&f.p, f.d, f.entry); + let leaf = node(&f.p, f.d, f.leaf); + let boundary = node(&f.p, f.d, f.a); + for id in [entry, leaf] { + let n = f.p.diagrams.get_mut(&f.d).unwrap().nodes.get_mut(&id).unwrap(); + n.position.x += 10.; + n.position.y += 10.; + } + let hidden = EdgeRoute { edge: f.local, source_attachment: None, target_attachment: None, + waypoints: vec![point(280., 390.), point(280., 470.)], + label: Some(EdgeLabel { fraction: 0.4, offset: point(3., -7.) }) }; + let internal = EdgeRoute { edge: f.internal, source_attachment: None, target_attachment: None, + waypoints: vec![point(320., 440.)], + label: Some(EdgeLabel { fraction: 0.7, offset: point(0., 0.) }) }; + let visible = EdgeRoute { edge: f.self_edge, + source_attachment: Some(EdgeAttachment { side: PortAnchorSide::Left, offset: 0.25 }), + target_attachment: Some(EdgeAttachment { side: PortAnchorSide::Right, offset: 0.75 }), + waypoints: vec![point(700., 100.)], + label: Some(EdgeLabel { fraction: 0.6, offset: point(0., 0.) }) }; + f.p.diagrams.get_mut(&f.d).unwrap().routes = vec![hidden.clone(), internal.clone(), visible.clone()]; + let projected = project_view(&f.p, f.d); + assert!(!projected.edges.iter().any(|e| e.id == f.local || e.id == f.internal)); + assert!(projected.edges.iter().any(|e| e.id == f.self_edge)); + let after = execute_batch(&f.p, &[Command::Layout(f.d)]).unwrap(); + for id in [entry, leaf, boundary] { + assert_eq!(after.diagrams[&f.d].nodes[&id].position, baseline.diagrams[&f.d].nodes[&id].position); + } + assert_eq!(after.diagrams[&f.d].nodes[&boundary].position, f.p.diagrams[&f.d].nodes[&boundary].position, + "The endpoint's owning State boundary stayed stationary"); + let mut reset = visible; + reset.waypoints.clear(); + reset.label = None; + assert_eq!(after.diagrams[&f.d].routes, vec![hidden, internal, reset], + "Equal entry/leaf deltas must not translate a pin on their stationary State boundary"); +} + #[test] fn initial_transition_attaches_to_the_compact_symbol_and_preview_keeps_settled_geometry() { let mut f = fixture();