Make space for counter node when collapsed nodes exceed max columns (#1277)

Author: das7pad

src/main/frontend/pipeline-graph-view/pipeline-graph/main/PipelineGraph.tsx

@@ -64,9 +64,14 @@ export function PipelineGraph({
 
     const apply = (width: number) => {
       if (width <= 0) return;
+      const reservedSpace =
+        fullLayout.nodeSpacingH / 2 + // before start
+        fullLayout.nodeSpacingH * 0.7 + // start node with reduced spacing
+        -fullLayout.nodeSpacingH * 0.3 + // reduced spacing to end node
+        fullLayout.nodeSpacingH / 2; // after end;
       const next = Math.max(
         MIN_COLUMNS_WHEN_COLLAPSED,
-        Math.floor(width / fullLayout.nodeSpacingH) - 2,
+        Math.floor((width - reservedSpace) / fullLayout.nodeSpacingH),
       );
       setMaxColumnsWhenCollapsed((prev) => (prev === next ? prev : next));
     };

src/main/frontend/pipeline-graph-view/pipeline-graph/main/PipelineGraphLayout.spec.ts

@@ -442,22 +442,22 @@ describe("PipelineGraphLayout", () => {
       it("uses the default threshold of 13 when no override is provided", () => {
         const graph = callLayout(25, true);
 
-        expect(graph.nodes.length).toBe(16);
-        expect(counterFor(graph)?.stages.length).toBe(12);
+        expect(graph.nodes.length).toBe(15);
+        expect(counterFor(graph)?.stages.length).toBe(13);
       });
 
       it("respects an explicit lower maxColumns override", () => {
         const graph = callLayout(25, true, 5);
 
-        expect(graph.nodes.length).toBe(8);
-        expect(counterFor(graph)?.stages.length).toBe(20);
+        expect(graph.nodes.length).toBe(7);
+        expect(counterFor(graph)?.stages.length).toBe(21);
       });
 
       it("respects an explicit higher maxColumns override", () => {
         const graph = callLayout(25, true, 20);
 
-        expect(graph.nodes.length).toBe(23);
-        expect(counterFor(graph)?.stages.length).toBe(5);
+        expect(graph.nodes.length).toBe(22);
+        expect(counterFor(graph)?.stages.length).toBe(6);
       });
 
       it("omits the counter when maxColumns exceeds the stage count", () => {

src/main/frontend/pipeline-graph-view/pipeline-graph/main/PipelineGraphLayout.ts

@@ -108,22 +108,21 @@ export function layoutGraph(
 
     const newMiddleNodes = createNodeColumns(middleStages);
 
-    const visibleNodes = newMiddleNodes.slice(0, maxColumnsWhenCollapsed);
-    const hiddenNodes = newMiddleNodes.slice(maxColumnsWhenCollapsed);
+    if (newMiddleNodes.length <= maxColumnsWhenCollapsed) {
+      return [start, ...newMiddleNodes, end];
+    }
 
-    const result = [start, ...visibleNodes];
+    // Make space for the counter node.
+    const breakPoint = maxColumnsWhenCollapsed - 1;
 
-    if (hiddenNodes.length > 0) {
-      counterNode.stages = hiddenNodes.flatMap((node) =>
+    counterNode.stages = newMiddleNodes
+      .slice(breakPoint)
+      .flatMap((node) =>
         node.rows.flatMap((row) =>
           row.flatMap((e) => (e as StageNodeInfo).stage),
         ),
       );
-      result.push(counter);
-    }
-
-    result.push(end);
-    return result;
+    return [start, ...newMiddleNodes.slice(0, breakPoint), counter, end];
   }
 
   const allNodeColumns: Array<NodeColumn> = filterWhenCollapsed([

src/main/frontend/pipeline-graph-view/pipeline-graph/main/support/nodes.spec.tsx

@@ -68,21 +68,21 @@ describe("Counter node with 50+ parallel stages", () => {
       const graph = buildGraphWithManyStages(55, 5);
       const counter = counterFor(graph);
       expect(counter).toBeDefined();
-      expect(counter?.stages.length).toBe(50);
+      expect(counter?.stages.length).toBe(51);
     });
 
     it("collapses 100 stages into a counter node with correct count", () => {
       const graph = buildGraphWithManyStages(100, 5);
       const counter = counterFor(graph);
       expect(counter).toBeDefined();
-      expect(counter?.stages.length).toBe(95);
+      expect(counter?.stages.length).toBe(96);
     });
 
     it("counter node holds all stage references for 60 stages at default threshold", () => {
       const graph = buildGraphWithManyStages(60, 13);
       const counter = counterFor(graph);
       expect(counter).toBeDefined();
-      expect(counter?.stages.length).toBe(47);
+      expect(counter?.stages.length).toBe(48);
       // Each stage should retain its identity
       counter?.stages.forEach((stage) => {
         expect(stage.name).toMatch(/^Stage \d+$/);

src/test/java/io/jenkins/plugins/pipelinegraphview/PipelineGraphViewTest.java

@@ -41,7 +41,7 @@ void smokeTest(Page p, JenkinsConfiguredWithCodeRule j) throws Exception {
                 .goTo()
                 .hasBuilds(1)
                 .nthBuild(0)
-                .hasStages(5, "Checkout", "Test", "A1", "A2", "Build")
+                .hasStages(4, "Checkout", "Test", "A1", "A2" /* counter */)
                 .goToBuild()
                 .hasStagesInGraph(4, /*A*/ "Checkout", "Test", /*B*/ "Build", "Parallel")
                 .goToPipelineOverview()