From f0d64b531addcea584083e35296d1e5a4cb1fa8d Mon Sep 17 00:00:00 2001 From: Codex-ytr83r Date: Thu, 24 Sep 2026 18:59:42 -0700 Subject: [PATCH] Verify route-only Layout reset history and format candidate --- crates/oa-core/src/session.rs | 20 +- crates/oa-core/tests/release_routing.rs | 347 ++++++++++++++++++++---- crates/oa-core/tests/state_diagrams.rs | 84 ++++-- 3 files changed, 371 insertions(+), 80 deletions(-) diff --git a/crates/oa-core/src/session.rs b/crates/oa-core/src/session.rs index 245230cd..1a7723d3 100644 --- a/crates/oa-core/src/session.rs +++ b/crates/oa-core/src/session.rs @@ -956,12 +956,17 @@ fn layout(p: &mut Project, diagram: Id) -> Result<(), ModelError> { // 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 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 { @@ -1054,7 +1059,8 @@ fn layout(p: &mut Project, diagram: Id) -> Result<(), ModelError> { } node }; - let edges: BTreeSet<_> = view.edges + let edges: BTreeSet<_> = view + .edges .iter() .filter_map(|edge| { let source = root_of(edge.source); diff --git a/crates/oa-core/tests/release_routing.rs b/crates/oa-core/tests/release_routing.rs index 6822c4b1..46b792d4 100644 --- a/crates/oa-core/tests/release_routing.rs +++ b/crates/oa-core/tests/release_routing.rs @@ -672,27 +672,70 @@ fn layout_resets_visible_pins_labels_preserving_route_count_order_and_undo() { #[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 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.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 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!() }; + 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()); + 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()); @@ -702,14 +745,25 @@ fn layout_keeps_attachments_port_anchors_empty_entries_and_exact_history() { 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.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(); + 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); @@ -738,28 +792,71 @@ fn translated_expected(mut route: EdgeRoute, x: f32, y: f32) -> EdgeRoute { 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]; + 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)]; + 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]); + 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); + 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.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]); } @@ -769,23 +866,63 @@ fn layout_folded_routes_use_actual_owner_deltas_without_resetting_promoted_edges 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(); + 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() + .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(); + 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.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]); } @@ -799,15 +936,26 @@ fn layout_batch_uses_staged_visibility_and_one_final_validation_history_boundary // 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(); + 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)]); + 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()); @@ -816,7 +964,14 @@ fn layout_batch_uses_staged_visibility_and_one_final_validation_history_boundary 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!( + 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); } @@ -829,32 +984,112 @@ fn layout_outside_projection_routes_keep_overrides_and_normal_translation() { 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)); + 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.)]); + 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 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 + .execute(Command::EditElement(element, changed)) + .unwrap(); session.undo(); let revision = session.revision(); - session.execute(Command::Batch(vec![Command::Layout(diagram), Command::Layout(diagram)])).unwrap(); + 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 layout_route_reset_alone_is_one_edit_and_repeated_layout_keeps_redo() { + let f = fixture(); + let mut before = execute_batch(&f.project, &[Command::Layout(f.diagram)]).unwrap(); + assert!(!before.diagrams[&f.diagram].nodes.is_empty()); + before.diagrams.get_mut(&f.diagram).unwrap().routes = vec![route(f.edge), route(f.external)]; + // Node geometry is already arranged. The complete expected Project differs + // only in the two independently named eligible route overrides. + let mut expected = before.clone(); + for route in &mut expected.diagrams.get_mut(&f.diagram).unwrap().routes { + route.waypoints.clear(); + route.label = None; + } + let mut session = Session::new(before.clone()); + session.execute(Command::Layout(f.diagram)).unwrap(); + assert_eq!( + session.project().diagrams[&f.diagram].nodes, + before.diagrams[&f.diagram].nodes + ); + assert_eq!(session.project(), &expected); + assert_eq!( + session.revision(), + 1, + "Clearing overrides is a real edit without any node movement" + ); + session.execute(Command::Layout(f.diagram)).unwrap(); + assert_eq!(session.project(), &expected); + assert_eq!( + session.revision(), + 1, + "A repeated identical Layout adds no history" + ); + session.undo(); + assert_eq!(session.project(), &before); + assert!( + !session.can_undo(), + "The route-only reset was exactly one history entry" + ); + session.redo(); + assert_eq!(session.project(), &expected); + + let mut renamed = expected.elements[&f.a].clone(); + renamed.name = "Pending redo of rename".into(); + session.execute(Command::EditElement(f.a, renamed)).unwrap(); + let after_rename = session.project().clone(); + session.undo(); + assert_eq!(session.project(), &expected); + assert!(session.can_redo()); + let revision = session.revision(); + session.execute(Command::Layout(f.diagram)).unwrap(); + assert_eq!(session.project(), &expected); + assert_eq!(session.revision(), revision); + assert!( + session.can_redo(), + "No-op Layout retains an unrelated undone edit" + ); + session.redo(); + assert_eq!(session.project(), &after_rename); +} + #[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 c803f080..0b6b5892 100644 --- a/crates/oa-core/tests/state_diagrams.rs +++ b/crates/oa-core/tests/state_diagrams.rs @@ -707,43 +707,93 @@ fn moving_a_region_does_not_translate_pins_attached_to_a_stationary_state_bounda #[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]; + 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(); + 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, + 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, + 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 }), + 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()]; + 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.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[&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"); + 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"); + 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]