target : outgoing) {
- newScores.merge(target, scorePerEdge, Double::sum);
- }
- }
- });
- }
-
/**
* Find strongly connected components using Kosaraju's algorithm
*/
diff --git a/graph-algorithms/src/test/java/org/hjug/feedback/arc/pageRank/PageRankFASDeterminismTest.java b/graph-algorithms/src/test/java/org/hjug/feedback/arc/pageRank/PageRankFASDeterminismTest.java
new file mode 100644
index 00000000..490021bc
--- /dev/null
+++ b/graph-algorithms/src/test/java/org/hjug/feedback/arc/pageRank/PageRankFASDeterminismTest.java
@@ -0,0 +1,189 @@
+package org.hjug.feedback.arc.pageRank;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.TreeSet;
+import org.hjug.feedback.SuperTypeToken;
+import org.jgrapht.Graph;
+import org.jgrapht.alg.cycle.CycleDetector;
+import org.jgrapht.graph.DefaultDirectedGraph;
+import org.jgrapht.graph.DefaultEdge;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression tests pinning the run-to-run determinism of {@link PageRankFAS#computeFeedbackArcSet()}.
+ *
+ * Both the htmlReport and the jsonReport invoke the same cycle-removal pipeline on the same
+ * package graph, so a non-deterministic feedback arc selection makes the two reports disagree on
+ * which package relationships (and, transitively, which class relationships) to remove. This
+ * suite reproduces the exact package-SCC shape that caused the htmlReport / jsonReport
+ * discrepancy between the {@code org.hjug.feedback} and
+ * {@code org.hjug.feedback.vertex.kernelized} packages.
+ */
+class PageRankFASDeterminismTest {
+
+ private static final String FEEDBACK = "org.hjug.feedback";
+ private static final String PAGE_RANK = "org.hjug.feedback.arc.pageRank";
+ private static final String KERNELIZED = "org.hjug.feedback.vertex.kernelized";
+
+ /** Builds the package-SCC shape observed in the RefactorFirst report output. */
+ private Graph createFeedbackPackageScc() {
+ Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class);
+ graph.addVertex(FEEDBACK);
+ graph.addVertex(PAGE_RANK);
+ graph.addVertex(KERNELIZED);
+ graph.addEdge(FEEDBACK, PAGE_RANK);
+ graph.addEdge(PAGE_RANK, FEEDBACK);
+ graph.addEdge(FEEDBACK, KERNELIZED);
+ graph.addEdge(KERNELIZED, FEEDBACK);
+ return graph;
+ }
+
+ /** Copies the graph topology with fresh edges so each run can remove edges independently. */
+ private Graph copyGraph(Graph original) {
+ Graph copy = new DefaultDirectedGraph<>(DefaultEdge.class);
+ original.vertexSet().forEach(copy::addVertex);
+ original.edgeSet()
+ .forEach(edge ->
+ copy.addEdge(original.getEdgeSource(edge), original.getEdgeTarget(edge), new DefaultEdge()));
+ return copy;
+ }
+
+ /** Converts edges to sorted source/target keys for comparison across graph copies. */
+ private Set edgeKeys(Graph graph, Set edges) {
+ Set keys = new TreeSet<>();
+ for (DefaultEdge edge : edges) {
+ keys.add(graph.getEdgeSource(edge) + " -> " + graph.getEdgeTarget(edge));
+ }
+ return keys;
+ }
+
+ /** Verifies repeated runs select the same edges and break both cycles in the package SCC. */
+ @DisplayName("computeFeedbackArcSet selects the same edges on every run for the feedback package SCC")
+ @Test
+ void computeFeedbackArcSet_isDeterministicAcrossRuns() {
+ // Given: the package SCC from the RefactorFirst report whose tied PageRank scores made
+ // htmlReport and jsonReport pick different package edges to remove
+ Graph template = createFeedbackPackageScc();
+
+ // When: the same algorithm runs many times on fresh copies of the same graph
+ Set firstRun = null;
+ for (int i = 0; i < 300; i++) {
+ Graph graph = copyGraph(template);
+ PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {});
+ Set run = edgeKeys(graph, pageRankFAS.computeFeedbackArcSet());
+ if (firstRun == null) {
+ firstRun = run;
+ } else {
+ assertEquals(
+ firstRun,
+ run,
+ "computeFeedbackArcSet must select identical edges across runs; run " + i + " differed");
+ }
+ }
+
+ // Then: the selection breaks both 2-cycles, so the graph is acyclic after removal
+ Graph graph = copyGraph(template);
+ PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {});
+ Set feedbackArcSet = pageRankFAS.computeFeedbackArcSet();
+ assertEquals(2, feedbackArcSet.size(), "Both 2-cycles require exactly one removed edge each");
+ feedbackArcSet.forEach(graph::removeEdge);
+ assertTrue(
+ !new CycleDetector<>(graph).detectCycles(),
+ "Removing the feedback arc set must leave the graph acyclic");
+ }
+
+ /** Verifies repeatable feedback arc selection and cycle removal in a larger symmetric graph. */
+ @DisplayName("computeFeedbackArcSet is deterministic for a graph with parallel tie candidates")
+ @Test
+ void computeFeedbackArcSet_isDeterministicForLargerTiedGraph() {
+ // Given: a larger symmetric SCC where many edges tie on PageRank score
+ Graph template = new DefaultDirectedGraph<>(DefaultEdge.class);
+ for (int i = 0; i < 6; i++) {
+ template.addVertex("P" + i);
+ }
+ for (int i = 0; i < 6; i++) {
+ template.addEdge("P" + i, "P" + (i + 1) % 6);
+ template.addEdge("P" + i, "P" + (i + 2) % 6);
+ }
+
+ // When: the algorithm runs repeatedly on fresh copies
+ Set firstRun = null;
+ for (int i = 0; i < 100; i++) {
+ Graph graph = copyGraph(template);
+ PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {});
+ Set run = edgeKeys(graph, pageRankFAS.computeFeedbackArcSet());
+ if (firstRun == null) {
+ firstRun = run;
+ } else {
+ assertEquals(
+ firstRun,
+ run,
+ "computeFeedbackArcSet must select identical edges across runs; run " + i + " differed");
+ }
+ }
+
+ // Then: the selection is a valid feedback arc set
+ Graph graph = copyGraph(template);
+ PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {});
+ Set feedbackArcSet = pageRankFAS.computeFeedbackArcSet();
+ Set removed = new HashSet<>(feedbackArcSet);
+ removed.forEach(graph::removeEdge);
+ assertTrue(
+ !new CycleDetector<>(graph).detectCycles(),
+ "Removing the feedback arc set must leave the graph acyclic");
+ }
+
+ /** Verifies edge insertion order does not change feedback arc selection for overlapping cycles. */
+ @DisplayName("computeFeedbackArcSet is independent of the order edges were added to the graph")
+ @Test
+ void computeFeedbackArcSet_isDeterministicAcrossEdgeInsertionOrders() {
+ // Given: one logical SCC expressing multiple overlapping cycles
+ String[][] logicalEdges = {
+ {"A", "B"},
+ {"B", "C"},
+ {"C", "A"}, // triangle 1
+ {"A", "D"},
+ {"D", "E"},
+ {"E", "A"}, // triangle 2
+ {"B", "E"},
+ {"D", "B"}, // cross edges
+ };
+
+ // And: several different edge insertion orders of the same logical edges
+ int[][] permutations = {
+ {0, 1, 2, 3, 4, 5, 6, 7}, // as declared
+ {7, 6, 5, 4, 3, 2, 1, 0}, // reversed
+ {3, 0, 6, 2, 5, 1, 7, 4}, // interleaved
+ {5, 2, 7, 1, 4, 6, 0, 3}, // another interleave
+ };
+
+ // When: the feedback arc set is computed for each insertion order
+ Set firstOrder = null;
+ for (int[] permutation : permutations) {
+ Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class);
+ for (int edgeIndex : permutation) {
+ String[] logicalEdge = logicalEdges[edgeIndex];
+ graph.addVertex(logicalEdge[0]);
+ graph.addVertex(logicalEdge[1]);
+ graph.addEdge(logicalEdge[0], logicalEdge[1]);
+ }
+ PageRankFAS pageRankFAS = new PageRankFAS<>(graph, new SuperTypeToken<>() {});
+ Set fas = edgeKeys(graph, pageRankFAS.computeFeedbackArcSet());
+
+ // Then: every insertion order selects the identical feedback arc set
+ if (firstOrder == null) {
+ firstOrder = fas;
+ } else {
+ assertEquals(
+ firstOrder,
+ fas,
+ "computeFeedbackArcSet must not depend on the order edges were added to the graph");
+ }
+ }
+ }
+}
diff --git a/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java b/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java
index e3abd549..f7102e97 100644
--- a/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java
+++ b/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java
@@ -23,6 +23,7 @@
import org.hjug.graphbuilder.metrics.DisharmonyMetric;
import org.hjug.metrics.DisharmonyInstance;
import org.hjug.refactorfirst.report.model.*;
+import org.jgrapht.graph.AsSubgraph;
import org.jgrapht.graph.DefaultWeightedEdge;
@Slf4j
@@ -384,8 +385,8 @@ public RefactorFirstReportDTO generateReportData(
targetUrl = repoUrl + endVertex.replace('.', '/');
}
- List breakClassRels =
- cells.length > 4 && !cells[4].isBlank() ? List.of(cells[4].split("
")) : List.of();
+ List breakClassRels = buildClassRelationshipsToBreakPackage(
+ classRelationshipsInPackageRelationship, repoUrl, codebaseGraphDTO);
// Build rendered label with links to package directories
String renderedLabel =
@@ -841,6 +842,71 @@ private String buildPackageLinkLabel(
return newLabel.toString();
}
+ /**
+ * Converts the class edges behind a package edge into structured DTOs.
+ * Cycle membership is calculated for every class edge from {@code classCycles} before the
+ * DTOs are constructed: {@code classEdgeCycleCounts} only covers edges of the class
+ * feedback arc set, so relying on it reported the nested edges — which are generally not
+ * feedback-arc-set members — as members of zero class cycles.
+ */
+ List buildClassRelationshipsToBreakPackage(
+ Set classEdges, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) {
+ if (classEdges == null || classEdges.isEmpty()) {
+ return List.of();
+ }
+
+ List orderedClassEdges = sortedClassEdgesInPackageRelationship(classEdges);
+ Map cycleCounts = new LinkedHashMap<>();
+ for (DefaultWeightedEdge classEdge : orderedClassEdges) {
+ cycleCounts.put(classEdge, countClassCyclesContaining(classEdge));
+ }
+
+ List relationships = new ArrayList<>();
+ for (DefaultWeightedEdge classEdge : orderedClassEdges) {
+ String[] vertexes = extractVertexes(classEdge);
+ String startVertex = vertexes[0].trim();
+ String endVertex = vertexes[1].trim();
+
+ String sourcePath =
+ codebaseGraphDTO.getClassToSourceFilePathMapping().get(startVertex);
+ String targetPath =
+ codebaseGraphDTO.getClassToSourceFilePathMapping().get(endVertex);
+ String sourceUrl = (sourcePath != null && !sourcePath.isBlank()) ? repoUrl + sourcePath : "";
+ String targetUrl = (targetPath != null && !targetPath.isBlank()) ? repoUrl + targetPath : "";
+ int weight = (int) classGraph.getEdgeWeight(classEdge);
+
+ relationships.add(ClassRelationshipDTO.builder()
+ .sourceClass(startVertex)
+ .targetClass(endVertex)
+ .sourceMarked(classesToRemove.contains(startVertex))
+ .targetMarked(classesToRemove.contains(endVertex))
+ .weight(weight)
+ .renderedLabel(
+ buildClassLinkLabel(startVertex, endVertex, sourceUrl, targetUrl, classesToRemove, weight))
+ .cycleCount(cycleCounts.get(classEdge))
+ .build());
+ }
+ return relationships;
+ }
+
+ /**
+ * Counts how many of the detected class cycles contain the given class edge — the same
+ * counting {@code CycleRemovalComputer} applies to the edges it marks for removal. Returns
+ * {@code 0} when no class cycles are available.
+ */
+ private int countClassCyclesContaining(DefaultWeightedEdge classEdge) {
+ if (classCycles == null || classCycles.isEmpty()) {
+ return 0;
+ }
+ int cycleCount = 0;
+ for (AsSubgraph cycle : classCycles.values()) {
+ if (cycle.containsEdge(classEdge)) {
+ cycleCount++;
+ }
+ }
+ return cycleCount;
+ }
+
/**
* Builds a rendered label with HTML links to class files.
* Transforms "GraphMetricsCollector → DependencyCollector : 1"
diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java
index ec4d5546..4f09587c 100644
--- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java
+++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java
@@ -649,11 +649,10 @@ String[] getPackageRelationshipDisharmony(
Set classRelationshipsInPackageRelationship =
codebaseGraphDTO.getClassRelationshipsInPackageRelationship().get(edgeInfo.getEdge());
- Set classEdges = new HashSet<>();
- if (classRelationshipsInPackageRelationship != null) {
- for (DefaultWeightedEdge defaultWeightedEdge : classRelationshipsInPackageRelationship) {
- classEdges.add(renderClassEdge(defaultWeightedEdge, repoUrl, codebaseGraphDTO));
- }
+ Set classEdges = new LinkedHashSet<>();
+ for (DefaultWeightedEdge defaultWeightedEdge :
+ sortedClassEdgesInPackageRelationship(classRelationshipsInPackageRelationship)) {
+ classEdges.add(renderClassEdge(defaultWeightedEdge, repoUrl, codebaseGraphDTO));
}
return new String[] {
@@ -665,6 +664,24 @@ String[] getPackageRelationshipDisharmony(
};
}
+ /**
+ * Deterministically orders the class edges behind a package relationship so that every
+ * report type (HTML, JSON) lists them identically on every run. The backing set's iteration
+ * order is identity-hash based and varies between executions, which made the htmlReport and
+ * the jsonReport disagree on the order of the class relationships to remove.
+ *
+ * @param classEdges the class edges mapped to a package relationship; may be {@code null}
+ * @return the edges ordered by source class FQN, then target class FQN
+ */
+ List sortedClassEdgesInPackageRelationship(Set classEdges) {
+ if (classEdges == null || classEdges.isEmpty()) {
+ return List.of();
+ }
+ return classEdges.stream()
+ .sorted(Comparator.comparing(classGraph::getEdgeSource).thenComparing(classGraph::getEdgeTarget))
+ .toList();
+ }
+
/** Renders summary rows for the ranked class cycles. */
private String renderClassCycleSummary(List rankedCycles) {
StringBuilder stringBuilder = new StringBuilder();
diff --git a/report/src/main/java/org/hjug/refactorfirst/report/model/PackageRelationshipDTO.java b/report/src/main/java/org/hjug/refactorfirst/report/model/PackageRelationshipDTO.java
index c38d4c10..5ec4692d 100644
--- a/report/src/main/java/org/hjug/refactorfirst/report/model/PackageRelationshipDTO.java
+++ b/report/src/main/java/org/hjug/refactorfirst/report/model/PackageRelationshipDTO.java
@@ -23,5 +23,5 @@ public class PackageRelationshipDTO {
private int effortRank;
@Builder.Default
- private List classRelationshipsToBreakPackage = new ArrayList<>();
+ private List classRelationshipsToBreakPackage = new ArrayList<>();
}
diff --git a/report/src/main/resources/templates/refactor-first-report.mustache b/report/src/main/resources/templates/refactor-first-report.mustache
index 428beaae..7f1faf27 100644
--- a/report/src/main/resources/templates/refactor-first-report.mustache
+++ b/report/src/main/resources/templates/refactor-first-report.mustache
@@ -298,7 +298,7 @@
{{cycleCount}} |
{{effortRank}} |
{{#classRelationshipsToBreakPackage}}
- {{{.}}}
{{/classRelationshipsToBreakPackage}} |
+ {{{renderedLabel}}}
{{/classRelationshipsToBreakPackage}}
{{/packageRelationshipsToRemove.relationships}}
diff --git a/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java b/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java
index 39238787..7f5ed731 100644
--- a/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java
+++ b/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java
@@ -9,6 +9,8 @@
import java.nio.file.Path;
import java.util.Comparator;
import org.hjug.refactorfirst.report.model.ChartJsBubbleDTO;
+import org.hjug.refactorfirst.report.model.ClassRelationshipDTO;
+import org.hjug.refactorfirst.report.model.PackageRelationshipDTO;
import org.hjug.refactorfirst.report.model.RefactorFirstReportDTO;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -426,4 +428,98 @@ public ClassB(ClassA classA) {
"renderedLabel should have target=\"_blank\" attribute");
}
}
+
+ /**
+ * Verifies that class relationships that break a package cycle are serialized as
+ * structured {@link ClassRelationshipDTO}s with class names, markers and a rendered label.
+ */
+ @Test
+ void given_packageCycle_when_reportGenerated_then_classRelationshipsToBreakPackageAreStructuredDTOs()
+ throws Exception {
+ // Given: two packages with a circular class dependency
+ File repoDir = tempDir.toFile();
+ new File(repoDir, ".git").mkdirs();
+
+ File pkgA = new File(repoDir, "src/main/java/com/example/pkga");
+ File pkgB = new File(repoDir, "src/main/java/com/example/pkgb");
+ pkgA.mkdirs();
+ pkgB.mkdirs();
+
+ Files.writeString(
+ new File(pkgA, "ClassA.java").toPath(),
+ """
+ package com.example.pkga;
+
+ public class ClassA {
+ private com.example.pkgb.ClassB classB;
+
+ public ClassA(com.example.pkgb.ClassB classB) {
+ this.classB = classB;
+ }
+ }
+ """);
+ Files.writeString(
+ new File(pkgB, "ClassB.java").toPath(),
+ """
+ package com.example.pkgb;
+
+ public class ClassB {
+ private com.example.pkga.ClassA classA;
+
+ public ClassB(com.example.pkga.ClassA classA) {
+ this.classA = classA;
+ }
+ }
+ """);
+
+ new ProcessBuilder("git", "init").directory(repoDir).start().waitFor();
+ new ProcessBuilder("git", "config", "user.email", "test@test.com")
+ .directory(repoDir)
+ .start()
+ .waitFor();
+ new ProcessBuilder("git", "config", "user.name", "Test")
+ .directory(repoDir)
+ .start()
+ .waitFor();
+ new ProcessBuilder("git", "add", ".").directory(repoDir).start().waitFor();
+ new ProcessBuilder("git", "commit", "-m", "initial")
+ .directory(repoDir)
+ .start()
+ .waitFor();
+
+ // When
+ new JsonGenerator().execute(0, true, false, true, "src/test", "PackageCycleProject", "1.0.0", repoDir, null);
+
+ // Then
+ Path jsonFile = tempDir.resolve(".refactorfirst").resolve("refactor-first.json");
+ RefactorFirstReportDTO report = objectMapper.readValue(jsonFile.toFile(), RefactorFirstReportDTO.class);
+ assertNotNull(report.getPackageRelationshipsToRemove());
+ assertNotNull(
+ report.getPackageRelationshipsToRemove().getRelationships(),
+ "Package relationships to remove should be present");
+ assertFalse(
+ report.getPackageRelationshipsToRemove().getRelationships().isEmpty(),
+ "Fixture produced no package relationships to remove");
+
+ int totalClassRelationships = 0;
+ for (PackageRelationshipDTO packageRel :
+ report.getPackageRelationshipsToRemove().getRelationships()) {
+ assertNotNull(packageRel.getClassRelationshipsToBreakPackage());
+ for (ClassRelationshipDTO classRel : packageRel.getClassRelationshipsToBreakPackage()) {
+ totalClassRelationships++;
+ assertTrue(
+ classRel.getSourceClass().startsWith("com.example.pkg"),
+ "sourceClass should be a fully qualified class name: " + classRel.getSourceClass());
+ assertTrue(
+ classRel.getTargetClass().startsWith("com.example.pkg"),
+ "targetClass should be a fully qualified class name: " + classRel.getTargetClass());
+ assertTrue(classRel.getWeight() >= 1, "weight should be at least 1");
+ assertNotNull(classRel.getRenderedLabel(), "ClassRelationshipDTO should have renderedLabel");
+ assertTrue(
+ classRel.getRenderedLabel().contains("→"),
+ "renderedLabel should contain the relationship arrow");
+ }
+ }
+ assertTrue(totalClassRelationships > 0, "Expected at least one class relationship to break the package cycle");
+ }
}
diff --git a/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeCycleCountTest.java b/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeCycleCountTest.java
new file mode 100644
index 00000000..f07401c4
--- /dev/null
+++ b/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeCycleCountTest.java
@@ -0,0 +1,115 @@
+package org.hjug.refactorfirst.report;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+import org.hjug.dsm.CircularReferenceChecker;
+import org.hjug.graphbuilder.CodebaseGraphDTO;
+import org.hjug.refactorfirst.report.model.ClassRelationshipDTO;
+import org.jgrapht.Graph;
+import org.jgrapht.graph.AsSubgraph;
+import org.jgrapht.graph.DefaultDirectedWeightedGraph;
+import org.jgrapht.graph.DefaultWeightedEdge;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+/**
+ * The nested class relationships inside a package relationship (the "class relationships to
+ * remove to break the package relationship") are generally not part of the class feedback arc
+ * set, so {@code classEdgeCycleCounts} — which {@code CycleRemovalComputer} populates only for
+ * feedback-arc-set edges — never contains them. Reporting {@code 0} for such edges falsely
+ * claims membership in zero class cycles. These tests pin that the nested DTOs carry the real
+ * class-cycle membership, calculated from the already-available {@code classCycles}.
+ */
+class PackageRelationshipClassEdgeCycleCountTest {
+
+ private static final String PKG_A = "org.example.pkgA";
+ private static final String PKG_B = "org.example.pkgB";
+ private static final String ALPHA = PKG_A + ".Alpha";
+ private static final String BETA = PKG_B + ".Beta";
+ private static final String DELTA = PKG_A + ".Delta";
+ private static final String REPO_URL = "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/example/repo/blob/";
+
+ /** Alpha and Beta form a 2-cycle; Delta -> Beta hangs off the cycle and is in no cycle. */
+ private Graph createClassGraph() {
+ Graph classGraph = new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class);
+ classGraph.addVertex(ALPHA);
+ classGraph.addVertex(BETA);
+ classGraph.addVertex(DELTA);
+
+ DefaultWeightedEdge alphaToBeta = classGraph.addEdge(ALPHA, BETA);
+ classGraph.setEdgeWeight(alphaToBeta, 5);
+ DefaultWeightedEdge betaToAlpha = classGraph.addEdge(BETA, ALPHA);
+ classGraph.setEdgeWeight(betaToAlpha, 2);
+ DefaultWeightedEdge deltaToBeta = classGraph.addEdge(DELTA, BETA);
+ classGraph.setEdgeWeight(deltaToBeta, 1);
+
+ return classGraph;
+ }
+
+ /** Creates a DTO mock with no source paths so relationships render without file links. */
+ private CodebaseGraphDTO mockDto() {
+ CodebaseGraphDTO dto = Mockito.mock(CodebaseGraphDTO.class);
+ Mockito.when(dto.getClassToSourceFilePathMapping()).thenReturn(new HashMap<>());
+ return dto;
+ }
+
+ /** Verifies nested relationships count detected class cycles even outside the feedback arc set. */
+ @DisplayName("nested class relationships report their real class-cycle membership, not a defaulted zero")
+ @Test
+ void buildClassRelationshipsToBreakPackage_reportsActualCycleMembership() throws Exception {
+ // Given: a class graph whose Alpha -> Beta and Beta -> Alpha edges participate in the
+ // Alpha/Beta class cycle, and a Delta -> Beta edge that participates in no cycle
+ Graph classGraph = createClassGraph();
+ DefaultWeightedEdge alphaToBeta = classGraph.getEdge(ALPHA, BETA);
+ DefaultWeightedEdge betaToAlpha = classGraph.getEdge(BETA, ALPHA);
+ DefaultWeightedEdge deltaToBeta = classGraph.getEdge(DELTA, BETA);
+
+ JsonGenerator generator = new JsonGenerator();
+ generator.classGraph = classGraph;
+ // The class cycles, built the same way CycleRemovalComputer builds them
+ Map> cycles =
+ new CircularReferenceChecker().getCycles(classGraph);
+ generator.classCycles = cycles;
+ assertEquals(1, cycles.size(), "Test graph must contain exactly one class cycle");
+ assertEquals(
+ 1,
+ cycles.values().stream()
+ .filter(c -> c.containsEdge(alphaToBeta))
+ .count(),
+ "The Alpha -> Beta edge must be a member of the class cycle");
+
+ // The nested edges are absent from classEdgeCycleCounts (which only covers
+ // feedback-arc-set edges), so their membership must be calculated from classCycles
+ Set classEdgesInPackageRelationship = new LinkedHashSet<>();
+ classEdgesInPackageRelationship.add(alphaToBeta);
+ classEdgesInPackageRelationship.add(betaToAlpha);
+ classEdgesInPackageRelationship.add(deltaToBeta);
+
+ // When: the nested class-relationship DTOs are built for the package edge
+ var relationships =
+ generator.buildClassRelationshipsToBreakPackage(classEdgesInPackageRelationship, REPO_URL, mockDto());
+
+ // Then: each nested DTO reports its real class-cycle membership
+ ClassRelationshipDTO alphaToBetaDto = relationships.stream()
+ .filter(r -> r.getSourceClass().equals(ALPHA))
+ .findFirst()
+ .orElseThrow();
+ ClassRelationshipDTO betaToAlphaDto = relationships.stream()
+ .filter(r -> r.getSourceClass().equals(BETA))
+ .findFirst()
+ .orElseThrow();
+ ClassRelationshipDTO deltaToBetaDto = relationships.stream()
+ .filter(r -> r.getSourceClass().equals(DELTA))
+ .findFirst()
+ .orElseThrow();
+
+ assertEquals(1, alphaToBetaDto.getCycleCount(), "Alpha -> Beta is in the class cycle, so its count must be 1");
+ assertEquals(1, betaToAlphaDto.getCycleCount(), "Beta -> Alpha is in the class cycle, so its count must be 1");
+ assertEquals(0, deltaToBetaDto.getCycleCount(), "Delta -> Beta is in no class cycle, so its count must be 0");
+ }
+}
diff --git a/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeOrderTest.java b/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeOrderTest.java
new file mode 100644
index 00000000..a3462b5b
--- /dev/null
+++ b/report/src/test/java/org/hjug/refactorfirst/report/PackageRelationshipClassEdgeOrderTest.java
@@ -0,0 +1,233 @@
+package org.hjug.refactorfirst.report;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.hjug.cbc.RankedDisharmony;
+import org.hjug.graphbuilder.CodebaseGraphDTO;
+import org.hjug.refactorfirst.report.model.ClassRelationshipDTO;
+import org.jgrapht.Graph;
+import org.jgrapht.graph.DefaultDirectedWeightedGraph;
+import org.jgrapht.graph.DefaultWeightedEdge;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+/**
+ * The htmlReport and the jsonReport both render the class relationships that must be removed to
+ * break a package relationship. Those class edges are stored in a
+ * {@code HashSet} whose iteration order is identity-hash based, so the two
+ * report types (and repeated runs of the same report) could list the same relationships in
+ * different orders. These tests pin a deterministic, source-ordered rendering shared by both
+ * reports.
+ */
+class PackageRelationshipClassEdgeOrderTest {
+
+ private static final String PKG_A = "org.example.pkgA";
+ private static final String PKG_B = "org.example.pkgB";
+ private static final String REPO_URL = "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/example/repo/blob/";
+
+ /** Number of class edges crossing the package boundary. */
+ private static final int EDGE_COUNT = 12;
+
+ /**
+ * Builds the class graph with {@link #EDGE_COUNT} edges from {@link #PKG_A} to
+ * {@link #PKG_B}, plus two edges sharing one source whose targets' natural order differs
+ * from their insertion order ({@code TargetZ} is inserted before {@code TargetB}).
+ */
+ private Graph createClassGraph() {
+ Graph classGraph = new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class);
+ for (int i = 0; i < EDGE_COUNT; i++) {
+ String source = PKG_A + ".Class" + (char) ('A' + i);
+ String target = PKG_B + ".Target" + (char) ('A' + i);
+ classGraph.addVertex(source);
+ classGraph.addVertex(target);
+ DefaultWeightedEdge edge = classGraph.addEdge(source, target);
+ classGraph.setEdgeWeight(edge, i + 1);
+ }
+
+ // Same-source edges whose target order differs from insertion order: TargetZ is added
+ // first, but TargetB must be rendered first because it sorts first
+ String multiSource = PKG_A + ".Multi";
+ String targetZ = PKG_B + ".TargetZ";
+ String targetB = PKG_B + ".TargetB";
+ classGraph.addVertex(multiSource);
+ classGraph.addVertex(targetZ);
+ classGraph.addVertex(targetB);
+ DefaultWeightedEdge multiToTargetZ = classGraph.addEdge(multiSource, targetZ);
+ classGraph.setEdgeWeight(multiToTargetZ, EDGE_COUNT + 1);
+ DefaultWeightedEdge multiToTargetB = classGraph.addEdge(multiSource, targetB);
+ classGraph.setEdgeWeight(multiToTargetB, EDGE_COUNT + 2);
+
+ return classGraph;
+ }
+
+ /** Builds a package graph with a single A -> B edge. */
+ private Graph createPackageGraph() {
+ Graph packageGraph = new DefaultDirectedWeightedGraph<>(DefaultWeightedEdge.class);
+ packageGraph.addVertex(PKG_A);
+ packageGraph.addVertex(PKG_B);
+ packageGraph.addEdge(PKG_A, PKG_B);
+ return packageGraph;
+ }
+
+ /** Inserts the given edges into a fresh HashSet in the given order. */
+ private Set newSetInOrder(Collection edges, int rotation) {
+ List rotated = new ArrayList<>(edges);
+ Collections.rotate(rotated, rotation);
+ return new HashSet<>(rotated);
+ }
+
+ /** Injects a graph fixture into a field declared by the shared HTML report base class. */
+ private void setField(Object target, String fieldName, Object value) throws Exception {
+ Field field = SimpleHtmlReport.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ field.set(target, value);
+ }
+
+ /** Maps the package edge to its class edges in a DTO mock with no source file paths. */
+ private CodebaseGraphDTO mockDto(DefaultWeightedEdge packageEdge, Set classEdges) {
+ CodebaseGraphDTO dto = Mockito.mock(CodebaseGraphDTO.class);
+ Map> relationships = new HashMap<>();
+ relationships.put(packageEdge, classEdges);
+ Mockito.when(dto.getClassRelationshipsInPackageRelationship()).thenReturn(relationships);
+ Mockito.when(dto.getClassToSourceFilePathMapping()).thenReturn(new HashMap<>());
+ return dto;
+ }
+
+ /** Creates a ranked package-edge fixture with priority, cycle count and effort rank set to one. */
+ private RankedDisharmony mockRankedDisharmony(DefaultWeightedEdge packageEdge) {
+ RankedDisharmony edgeInfo = Mockito.mock(RankedDisharmony.class);
+ Mockito.when(edgeInfo.getEdge()).thenReturn(packageEdge);
+ Mockito.when(edgeInfo.getPriority()).thenReturn(1);
+ Mockito.when(edgeInfo.getCycleCount()).thenReturn(1);
+ Mockito.when(edgeInfo.getEffortRank()).thenReturn(1);
+ return edgeInfo;
+ }
+
+ /** Renders the class-relationship cell of the package-relationship table for the given DTO. */
+ private String renderHtmlCell(SimpleHtmlReport report, RankedDisharmony edgeInfo, CodebaseGraphDTO dto)
+ throws Exception {
+ Method method = SimpleHtmlReport.class.getDeclaredMethod(
+ "getPackageRelationshipDisharmony", RankedDisharmony.class, String.class, CodebaseGraphDTO.class);
+ method.setAccessible(true);
+ String[] cells = (String[]) method.invoke(report, edgeInfo, REPO_URL, dto);
+ return cells[4];
+ }
+
+ /** Builds the JSON DTO list of class relationships behind the package edge for the given DTO. */
+ private List buildJsonList(JsonGenerator generator, CodebaseGraphDTO dto) {
+ DefaultWeightedEdge packageEdge = dto.getClassRelationshipsInPackageRelationship()
+ .keySet()
+ .iterator()
+ .next();
+ return generator.buildClassRelationshipsToBreakPackage(
+ dto.getClassRelationshipsInPackageRelationship().get(packageEdge), REPO_URL, dto);
+ }
+
+ /** Verifies HTML and JSON use the same source/target order across class-edge set insertions. */
+ @DisplayName(
+ "class relationships behind a package edge render in the same deterministic order regardless of set insertion order")
+ @Test
+ void classRelationshipsToBreakPackage_areOrderedDeterministically() throws Exception {
+ // Given: a class graph with edges crossing a package boundary, and DTOs whose
+ // class-relationship sets are populated in many different insertion orders
+ Graph classGraph = createClassGraph();
+ Graph packageGraph = createPackageGraph();
+ DefaultWeightedEdge packageEdge = packageGraph.edgeSet().iterator().next();
+ List edges = new ArrayList<>(classGraph.edgeSet());
+ RankedDisharmony edgeInfo = mockRankedDisharmony(packageEdge);
+
+ String firstHtmlCell = null;
+ List firstJsonList = null;
+ for (int rotation = 0; rotation < 24; rotation++) {
+ Set classEdges = newSetInOrder(edges, rotation);
+ CodebaseGraphDTO dto = mockDto(packageEdge, classEdges);
+
+ SimpleHtmlReport htmlReport = new SimpleHtmlReport();
+ setField(htmlReport, "classGraph", classGraph);
+ setField(htmlReport, "packageGraph", packageGraph);
+ String htmlCell = renderHtmlCell(htmlReport, edgeInfo, dto);
+
+ JsonGenerator jsonGenerator = new JsonGenerator();
+ setField(jsonGenerator, "classGraph", classGraph);
+ setField(jsonGenerator, "packageGraph", packageGraph);
+ List jsonList = buildJsonList(jsonGenerator, dto);
+ List jsonSources =
+ jsonList.stream().map(ClassRelationshipDTO::getSourceClass).toList();
+
+ // When: comparing each rotated insertion order against the first one
+ if (firstHtmlCell == null) {
+ firstHtmlCell = htmlCell;
+ firstJsonList = jsonList;
+ } else {
+ assertEquals(
+ firstHtmlCell,
+ htmlCell,
+ "HTML class-relationship cell must not depend on set insertion order (rotation " + rotation
+ + ")");
+ assertEquals(
+ firstJsonList.stream()
+ .map(ClassRelationshipDTO::getSourceClass)
+ .toList(),
+ jsonSources,
+ "JSON class-relationship order must not depend on set insertion order (rotation " + rotation
+ + ")");
+ }
+ }
+
+ // Then: the order is the deterministic source-class order, identical in both reports
+ List expectedOrder = edges.stream()
+ .sorted((e1, e2) -> {
+ int bySource = classGraph.getEdgeSource(e1).compareTo(classGraph.getEdgeSource(e2));
+ return bySource != 0
+ ? bySource
+ : classGraph.getEdgeTarget(e1).compareTo(classGraph.getEdgeTarget(e2));
+ })
+ .toList();
+ assertEquals(
+ expectedOrder.stream().map(classGraph::getEdgeSource).toList(),
+ firstJsonList.stream().map(ClassRelationshipDTO::getSourceClass).toList(),
+ "JSON class relationships must be ordered by source class FQN");
+ String expectedHtmlCell = String.join(
+ "
",
+ expectedOrder.stream()
+ .map(edge -> simpleName(classGraph.getEdgeSource(edge)) + " → "
+ + simpleName(classGraph.getEdgeTarget(edge)) + " : "
+ + (int) classGraph.getEdgeWeight(edge))
+ .toList());
+ assertEquals(
+ expectedHtmlCell,
+ firstHtmlCell,
+ "HTML class-relationship cell must list the same edges in the same order as the JSON report");
+
+ // And: edges sharing a source render their targets in natural order — not insertion
+ // order — in both reports (TargetZ was inserted first, but TargetB sorts first)
+ List multiSourceTargets = firstJsonList.stream()
+ .filter(r -> r.getSourceClass().equals(PKG_A + ".Multi"))
+ .map(ClassRelationshipDTO::getTargetClass)
+ .toList();
+ assertEquals(
+ List.of(PKG_B + ".TargetB", PKG_B + ".TargetZ"),
+ multiSourceTargets,
+ "JSON must order same-source class relationships by target class FQN, not insertion order");
+ assertTrue(
+ firstHtmlCell.indexOf("Multi → TargetB") < firstHtmlCell.indexOf("Multi → TargetZ"),
+ "HTML must render same-source targets in the same order as the JSON report");
+ }
+
+ /** Extracts the final name segment from a fully qualified class name for expected HTML labels. */
+ private static String simpleName(String fqn) {
+ return fqn.substring(fqn.lastIndexOf('.') + 1);
+ }
+}