Skip to content

Commit f79e5d7

Browse files
committed
feat: add Mermaid acyclic parity profile
Split the layered acyclic policy so the Mermaid profile uses Dagre's default DFS feedback-arc behavior while native Flux layouts keep semantic compound feedback selection. Wire the policy through both layered config propagation paths and add parity coverage for the compound backward-edge fixture, including MMDS replay evidence. This preserves the acyclic/reversal contract targeted by Plan 0157. Full Mermaid compound-positioning parity remains a follow-up because it is an ordering/positioning gap, not a feedback-arc selection issue.
1 parent f1ae9e1 commit f79e5d7

15 files changed

Lines changed: 630 additions & 49 deletions

File tree

src/engines/graph/algorithms/layered/float_layout.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub(crate) fn build_float_layout_with_flags(
9999
let direction = diagram.direction;
100100
let mut layered_config = layered_config_for_layout(diagram, config);
101101
if let Some(flags) = engine_flags {
102+
layered_config.acyclic_policy = flags.acyclic_policy;
102103
layered_config.greedy_switch = flags.greedy_switch;
103104
layered_config.model_order_tiebreak = flags.model_order_tiebreak;
104105
layered_config.variable_rank_spacing = flags.variable_rank_spacing;

src/engines/graph/algorithms/layered/kernel/acyclic.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use std::collections::BTreeSet;
88

99
use super::graph::LayoutGraph;
10+
use super::types::AcyclicPolicy;
1011

1112
#[derive(Clone, Copy)]
1213
struct CompoundEdge {
@@ -23,7 +24,12 @@ struct CompoundEdge {
2324
/// Uses DFS to find back-edges (edges pointing to ancestors in the DFS tree).
2425
/// This preserves the natural forward flow of the graph better than
2526
/// greedy_feedback_arc_set which may reverse arbitrary edges.
27+
#[cfg(test)]
2628
pub fn run(graph: &mut LayoutGraph) {
29+
run_with_policy(graph, AcyclicPolicy::default());
30+
}
31+
32+
pub(super) fn run_with_policy(graph: &mut LayoutGraph, policy: AcyclicPolicy) {
2733
let n = graph.node_ids.len();
2834
if n == 0 {
2935
return;
@@ -51,7 +57,8 @@ pub fn run(graph: &mut LayoutGraph) {
5157
// missed. An edge from compound X to compound Y participates in a
5258
// compound-level cycle if there is already a path from Y back to X through
5359
// other compounds. See issue #155.
54-
if !graph.compound_nodes.is_empty() {
60+
if matches!(policy, AcyclicPolicy::SemanticCompoundFeedback) && !graph.compound_nodes.is_empty()
61+
{
5562
detect_compound_back_edges(graph, &mut back_edges);
5663
}
5764

@@ -340,6 +347,30 @@ mod tests {
340347
}
341348
}
342349

350+
#[test]
351+
fn test_acyclic_policy_dfs_only_skips_compound_feedback_edge() {
352+
let mut lg = compound_cycle_graph_with_order(&["sg_c", "sg_b", "sg_a"]);
353+
run_with_policy(&mut lg, AcyclicPolicy::DfsOnly);
354+
355+
assert!(
356+
!lg.reversed_edges.contains(&2),
357+
"DfsOnly should skip semantic compound feedback detection; got {:?}",
358+
lg.reversed_edges
359+
);
360+
}
361+
362+
#[test]
363+
fn test_acyclic_policy_semantic_compound_feedback_reverses_c_to_a() {
364+
let mut lg = compound_cycle_graph_with_order(&["sg_c", "sg_b", "sg_a"]);
365+
run_with_policy(&mut lg, AcyclicPolicy::SemanticCompoundFeedback);
366+
367+
assert!(
368+
lg.reversed_edges.contains(&2),
369+
"SemanticCompoundFeedback should preserve compound feedback detection; got {:?}",
370+
lg.reversed_edges
371+
);
372+
}
373+
343374
#[test]
344375
fn test_acyclic_compound_level_back_edges_converge_across_disjoint_cycles() {
345376
let mut graph: DiGraph<()> = DiGraph::new();

src/engines/graph/algorithms/layered/kernel/dagre_parity_tests.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::path::Path;
88

99
use serde::Deserialize;
1010

11+
use super::types::AcyclicPolicy;
1112
use super::{DiGraph, LayoutConfig, NodeId, layout};
1213

1314
#[test]
@@ -115,6 +116,14 @@ fn build_digraph_from_input(input: &InputGraph) -> DiGraph<(f64, f64)> {
115116
graph
116117
}
117118

119+
fn dagre_node<'a>(layout: &'a DagreLayout, id: &str) -> &'a DagreNode {
120+
layout
121+
.nodes
122+
.iter()
123+
.find(|node| node.id == id)
124+
.unwrap_or_else(|| panic!("missing dagre node {id}"))
125+
}
126+
118127
/// Border node info parsed from debug dump files.
119128
#[derive(Debug, Clone)]
120129
struct BorderNodeInfo {
@@ -229,6 +238,80 @@ fn assert_points_close(actual: &[(f64, f64)], expected: &[[f64; 2]], tolerance:
229238
// Parity Tests
230239
// =============================================================================
231240

241+
mod compound_backward_disconnected {
242+
use super::*;
243+
244+
const INPUT_PATH: &str =
245+
"tests/parity-fixtures/compound_backward_disconnected/mmdflux-dagre-input.json";
246+
const EXPECTED_PATH: &str =
247+
"tests/parity-fixtures/compound_backward_disconnected/dagre-layout.json";
248+
249+
#[test]
250+
fn compound_backward_disconnected_input_records_mermaid_order() {
251+
let input: InputGraph = load_json(INPUT_PATH);
252+
let ids: Vec<&str> = input.nodes.iter().map(|node| node.id.as_str()).collect();
253+
254+
assert_eq!(
255+
ids,
256+
vec!["C", "B", "A", "a1", "a2", "b1", "b2", "c1", "c2"],
257+
"raw Dagre fixture should preserve Mermaid FlowDB node order"
258+
);
259+
}
260+
261+
#[test]
262+
fn compound_backward_disconnected_raw_dagre_fixture_is_tall_top_right() {
263+
let expected: DagreLayout = load_json(EXPECTED_PATH);
264+
let top = dagre_node(&expected, "A");
265+
let middle = dagre_node(&expected, "B");
266+
let bottom = dagre_node(&expected, "C");
267+
let sibling_max = middle.height.max(bottom.height);
268+
269+
assert!(top.is_compound && middle.is_compound && bottom.is_compound);
270+
assert!(
271+
top.x > middle.x && top.x > bottom.x,
272+
"raw Dagre should place A/Top to the right; A={top:?} B={middle:?} C={bottom:?}"
273+
);
274+
assert!(
275+
top.height > sibling_max * 1.8,
276+
"raw Dagre should stretch A/Top vertically; A={top:?} B={middle:?} C={bottom:?}"
277+
);
278+
assert!(
279+
middle.y + middle.height <= bottom.y,
280+
"raw Dagre should stack B/Middle above C/Bottom; B={middle:?} C={bottom:?}"
281+
);
282+
283+
let edge2 = expected
284+
.edges
285+
.iter()
286+
.find(|edge| edge.index == 2)
287+
.expect("raw Dagre fixture should include edge 2");
288+
assert_eq!(edge2._from, "c2");
289+
assert_eq!(edge2._to, "a2");
290+
}
291+
292+
#[test]
293+
fn compound_backward_disconnected_dfs_only_keeps_edge_2_forward() {
294+
let input: InputGraph = load_json(INPUT_PATH);
295+
let graph = build_digraph_from_input(&input);
296+
let config = LayoutConfig {
297+
node_sep: 50.0,
298+
edge_sep: 20.0,
299+
rank_sep: 75.0,
300+
margin: 8.0,
301+
acyclic_policy: AcyclicPolicy::DfsOnly,
302+
..Default::default()
303+
};
304+
305+
let result = layout(&graph, &config, |_, dims| *dims);
306+
307+
assert!(
308+
!result.reversed_edges.contains(&2),
309+
"DFS-only strict parity should not reverse c2 -> a2; got {:?}",
310+
result.reversed_edges
311+
);
312+
}
313+
}
314+
232315
mod subgraph_bounds {
233316
use super::*;
234317

src/engines/graph/algorithms/layered/kernel/pipeline.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ where
6464

6565
// Phase 1: Make graph acyclic
6666
if config.acyclic {
67-
acyclic::run(&mut lg);
67+
acyclic::run_with_policy(&mut lg, config.acyclic_policy);
6868
}
6969

7070
// Phase 1.5: Create space for edge label dummies.

src/engines/graph/algorithms/layered/kernel/types.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ pub enum Direction {
4444
RightLeft, // RL
4545
}
4646

47+
/// Strategy for selecting feedback edges during acyclic preprocessing.
48+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
49+
pub enum AcyclicPolicy {
50+
/// Mermaid dagre-wrapper default: Dagre DFS feedback-arc detection,
51+
/// without the greedy `acyclicer` mode.
52+
DfsOnly,
53+
/// Preserve semantic compound feedback selection for native Flux layouts.
54+
#[default]
55+
SemanticCompoundFeedback,
56+
}
57+
4758
impl Direction {
4859
/// Is this a vertical (TB/BT) layout?
4960
pub fn is_vertical(self) -> bool {
@@ -328,6 +339,9 @@ pub struct LayoutConfig {
328339
/// Whether to apply layout optimization for acyclic graphs.
329340
pub acyclic: bool,
330341

342+
/// Strategy to use when selecting feedback edges.
343+
pub acyclic_policy: AcyclicPolicy,
344+
331345
/// Ranking algorithm to use.
332346
pub ranker: Ranker,
333347

@@ -406,6 +420,7 @@ impl Default for LayoutConfig {
406420
rank_sep_overrides: HashMap::new(),
407421
margin: 8.0,
408422
acyclic: true,
423+
acyclic_policy: AcyclicPolicy::default(),
409424
ranker: Ranker::default(),
410425
greedy_switch: false,
411426
model_order_tiebreak: false,

src/engines/graph/algorithms/layered/measurement.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ pub fn run_layered_layout(
130130
let override_subgraphs = override_subgraph_projections(diagram, layered_cfg);
131131
let grid_config = layout_config_from_layered(layered_cfg, diagram);
132132
let mut lc = layered_config_for_layout(diagram, &grid_config);
133+
lc.acyclic_policy = layered_cfg.acyclic_policy;
133134
lc.greedy_switch = layered_cfg.greedy_switch;
134135
lc.model_order_tiebreak = layered_cfg.model_order_tiebreak;
135136
lc.variable_rank_spacing = layered_cfg.variable_rank_spacing;

src/engines/graph/algorithms/layered/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ pub use kernel::graph::DiGraph;
1818
#[cfg(test)]
1919
pub(crate) use kernel::pipeline::layout;
2020
pub use kernel::types::{
21-
Direction, LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy, LayoutConfig, Ranker,
21+
AcyclicPolicy, Direction, LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy,
22+
LayoutConfig, Ranker,
2223
};
2324
#[cfg(test)]
2425
pub use kernel::types::{EdgeLayout, LayoutResult, NodeId, Point, Rect, SelfEdgeLayout};

src/engines/graph/flux.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//! routing behavior.
66
77
use crate::engines::graph::algorithms::layered::{
8-
LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy, LayoutConfig,
8+
AcyclicPolicy, LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy, LayoutConfig,
99
build_float_layout_with_flags, layout_config_from_layered, run_layered_layout,
1010
};
1111
use crate::engines::graph::contracts::MeasurementMode;
@@ -32,6 +32,7 @@ pub(crate) fn flux_layout_profile(
3232
_edge_routing: EdgeRouting,
3333
) -> LayoutConfig {
3434
LayoutConfig {
35+
acyclic_policy: AcyclicPolicy::SemanticCompoundFeedback,
3536
greedy_switch: true,
3637
model_order_tiebreak: input_cfg.model_order_tiebreak,
3738
variable_rank_spacing: true,

src/engines/graph/layout.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ impl From<LayoutConfig> for crate::engines::graph::algorithms::layered::LayoutCo
252252
rank_sep_overrides: value.rank_sep_overrides,
253253
margin: value.margin,
254254
acyclic: value.acyclic,
255+
acyclic_policy: Default::default(),
255256
ranker: value.ranker.into(),
256257
greedy_switch: value.greedy_switch,
257258
model_order_tiebreak: value.model_order_tiebreak,

src/engines/graph/mermaid.rs

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
use std::collections::HashMap;
88

99
use crate::engines::graph::algorithms::layered::{
10-
LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy, LayoutConfig,
10+
AcyclicPolicy, LabelDummyPlacement, LabelDummyRouting, LabelSideStrategy, LayoutConfig,
1111
build_float_layout_with_flags, layout_config_from_layered,
1212
};
1313
use crate::engines::graph::contracts::MeasurementMode;
@@ -102,6 +102,30 @@ fn apply_mermaid_subgraph_direction_policy(diagram: &Graph) -> Option<Graph> {
102102
/// Mermaid-layered engine: shared layered layout with Mermaid-compatible policy.
103103
pub struct MermaidLayeredEngine;
104104

105+
fn mermaid_layout_flags() -> LayoutConfig {
106+
LayoutConfig {
107+
acyclic_policy: AcyclicPolicy::DfsOnly,
108+
always_compound_ordering: true,
109+
label_side_selection: true,
110+
label_side_strategy: LabelSideStrategy::DirectionDown,
111+
// Plan 0147 Task 2.3 / 2.6: mermaid profile pins placement +
112+
// routing explicitly at the dagre-parity defaults so later
113+
// `..Default::default()` changes never drift the profile off parity.
114+
label_dummy_placement: LabelDummyPlacement::Midpoint,
115+
label_dummy_routing: LabelDummyRouting::Center,
116+
// Plan 0147 Task 1.7: mermaid profile enables wrap at 200 px;
117+
// dagre parity is preserved because `LabelDummyRouting` stays on
118+
// `Center` here.
119+
edge_label_max_width: Some(200.0),
120+
..Default::default()
121+
}
122+
}
123+
124+
#[cfg(test)]
125+
pub(crate) fn mermaid_layout_flags_for_test() -> LayoutConfig {
126+
mermaid_layout_flags()
127+
}
128+
105129
impl Default for MermaidLayeredEngine {
106130
fn default() -> Self {
107131
Self::new()
@@ -153,22 +177,7 @@ impl GraphEngine for MermaidLayeredEngine {
153177
let EngineConfig::Layered(ref layered_cfg) = *config;
154178
let mut layout_config = layout_config_from_layered(layered_cfg, diagram);
155179
layout_config.cluster_rank_sep = 0.0;
156-
let mermaid_flags = LayoutConfig {
157-
always_compound_ordering: true,
158-
label_side_selection: true,
159-
label_side_strategy: LabelSideStrategy::DirectionDown,
160-
// Plan 0147 Task 2.3 / 2.6: mermaid profile pins placement +
161-
// routing explicitly at the dagre-parity defaults so later
162-
// `..Default::default()` changes never drift the profile off
163-
// parity.
164-
label_dummy_placement: LabelDummyPlacement::Midpoint,
165-
label_dummy_routing: LabelDummyRouting::Center,
166-
// Plan 0147 Task 1.7: mermaid profile enables wrap at 200 px;
167-
// dagre parity is preserved because `LabelDummyRouting` stays on
168-
// `Center` here.
169-
edge_label_max_width: Some(200.0),
170-
..Default::default()
171-
};
180+
let mermaid_flags = mermaid_layout_flags();
172181
let geometry = build_float_layout_with_flags(
173182
diagram,
174183
&layout_config,

0 commit comments

Comments
 (0)