Detect mask hierarchy cycles before reducing direct edges

src/morphobricks_preprocessing/mask_scan.py

@@ -571,15 +571,19 @@ def _relations(
     limits: Mapping[str, float],
 ) -> tuple[PairRelation, ...]:
     """Return every pair's relation, marking the direct ones (module docstring)."""
-    found = {
-        (child, parent): _relation(types[child], counts, limits)
-        for (child, parent), counts in totals.items()
-    }
+    found = {pair: _relation(types[pair[0]], counts, limits) for pair, counts in totals.items()}
     nested = {pair for pair, relation in found.items() if relation in ("inside", "grouped_by")}
+    # Cycles first (a reduction can remove every edge of one): edges inside a strongly
+    # connected set stay direct, and the reduction runs only between the sets.
+    up = {n: [p for c, p in nested if c == n] for n in types}
+    reach = {n: _ancestors(up, n) for n in types}
+    group = {n: frozenset({n, *(o for o in reach[n] if n in reach[o])}) for n in types}
+    links = {(group[c], group[p]) for c, p in nested if group[c] != group[p]}
     pairs = []
     for (child, parent), relation in sorted(found.items()):
-        between = any((child, other) in nested and (other, parent) in nested for other in types)
-        direct = (child, parent) in nested and not between
+        low, high = group[child], group[parent]
+        between = any((low, g) in links and (g, high) in links for g in group.values())
+        direct = (child, parent) in nested and (low == high or not between)
         evidence = MappingProxyType(dict(totals[(child, parent)]))
         pairs.append(PairRelation(child, parent, relation, direct, evidence))
     return tuple(pairs)

tests/test_mask_hierarchy_cycles.py

@@ -5,7 +5,14 @@
 import numpy as np
 
 from morphobricks_preprocessing import preflight, scan_masks
-from morphobricks_preprocessing.mask_scan import MaskReport, MaskTypeSummary, PairRelation
+from morphobricks_preprocessing.mask_scan import (
+    MaskReport,
+    MaskTypeSummary,
+    PairRelation,
+    _relations,
+    hierarchy_cycles,
+    mask_thresholds,
+)
 from morphobricks_preprocessing.mask_text import mask_hierarchy_lines, mask_report_summary
 from tiff_tree import write_data
 
@@ -110,3 +117,47 @@ def box(size):
     assert scanned.hierarchy_cycles == (("A", "B", "C"),)
     lines = mask_hierarchy_lines(scanned)
     assert len(warnings(lines)) == 1 and lines[1:] == ["    [cycle: A, B, C]"]
+
+
+def relations(*links, names=None):
+    """The pairs ``_relations`` marks when every ``(child, parent)`` link is ``inside``."""
+    names = names or sorted({name for link in links for name in link})
+    types = {name: MaskTypeSummary(name, "instances", 1, 1, 1.0) for name in names}
+    none = {"children": 1, "parents": 1, "children_matched_one_to_one": 0}
+    none |= {"parents_matched_one_to_one": 0, "inside": 0, "inside_parent_larger": 0}
+    inside = none | {"inside": 1, "inside_parent_larger": 1}
+    totals = {(c, p): inside if (c, p) in links else none for c in names for p in names if c != p}
+    return _relations(types, totals, mask_thresholds())
+
+
+def test_a_complete_three_cycle_keeps_its_edges_and_warns():
+    links = [(c, p) for c in "ABC" for p in "ABC" if c != p]
+    pairs = relations(*links)
+    assert all(pair.direct for pair in pairs)
+    r = MaskReport(
+        images=1,
+        types=MappingProxyType({n: MaskTypeSummary(n, "instances", 1, 1, 1.0) for n in "ABC"}),
+        pairs=pairs,
+    )
+    assert r.hierarchy_cycles == (("A", "B", "C"),)
+    assert len(warnings(mask_hierarchy_lines(r))) == 1
+
+
+def test_a_cycle_with_a_chord_keeps_its_edges_and_warns():
+    pairs = relations(("A", "B"), ("B", "C"), ("C", "A"), ("A", "C"))
+    assert all(pair.direct for pair in pairs if pair.relation == "inside")
+    hierarchy = {p.child: [] for p in pairs}
+    for p in pairs:
+        if p.direct:
+            hierarchy[p.child].append(p.parent)
+    assert hierarchy_cycles(hierarchy) == (("A", "B", "C"),)
+
+
+def test_the_reduction_runs_between_cycles_and_acyclic_edges_are_unchanged():
+    # Dots in X in Y in Z (chain): Dots-Y and X-Z and Dots-Z are implied, not direct.
+    chain = relations(("D", "X"), ("X", "Y"), ("Y", "Z"), ("D", "Y"), ("X", "Z"), ("D", "Z"))
+    assert {(p.child, p.parent) for p in chain if p.direct} == {("D", "X"), ("X", "Y"), ("Y", "Z")}
+    # A and B nested in each other, both inside Top; Dots inside A (and so inside Top).
+    mixed = relations(("A", "B"), ("B", "A"), ("A", "T"), ("B", "T"), ("D", "A"), ("D", "T"))
+    direct = {(p.child, p.parent) for p in mixed if p.direct}
+    assert direct == {("A", "B"), ("B", "A"), ("A", "T"), ("B", "T"), ("D", "A")}