Fix progress bar for replicates.

Author: sampottinger

editor/index.html

@@ -180,7 +180,7 @@ <h2>Config</h2>
                 <div class="form-group">
                   <div><label for="config-textarea">Configuration (editor.jshc):</label></div>
                   <div>
-                    <textarea id="config-textarea" name="config-textarea" class="config-textarea long" rows="10" cols="85" placeholder="testVariable = 5 m"></textarea>
+                    <textarea id="config-textarea" name="config-textarea" class="config-textarea long" rows="10" cols="50" placeholder="testVariable = 5 m"></textarea>
                   </div>
                 </div>
                 <div class="dialog-buttons buttons-panel bold-buttons">

editor/style/style.css

@@ -58,7 +58,7 @@ dialog .form-group input.long {
 }
 
 dialog .form-group textarea.config-textarea {
-  width: 650px;
+  width: 90%;
   font-family: monospace;
 }
 

src/main/java/org/joshsim/command/RemoteResponseHandler.java

@@ -169,10 +169,14 @@ private void handleProgressResponse(WireResponse response, AtomicInteger cumulat
     long stepCountToReport = response.getStepCount();
 
     if (useCumulativeProgress && cumulativeStepCount != null) {
-      // Use cumulative progress for coordinated reporting
-      WireResponse cumulativeProgress = WireRewriteUtil.rewriteProgressToCumulative(
+      // Use proper cumulative progress calculation
+      long stepsPerReplicate = context.getMetadata().getTotalSteps();
+      int completedReplicatesCount = completedReplicates.get();
+
+      WireResponse cumulativeProgress = WireRewriteUtil.rewriteProgressToProperCumulative(
           response,
-          cumulativeStepCount
+          completedReplicatesCount,
+          stepsPerReplicate
       );
       stepCountToReport = cumulativeProgress.getStepCount();
     }

src/main/java/org/joshsim/pipeline/remote/LeaderResponseHandler.java

@@ -62,8 +62,8 @@ private String generateOutputLine(WireResponse parsedResponse, int replicateNumb
                                    AtomicInteger cumulativeStepCount) {
     return switch (parsedResponse.getType()) {
       case PROGRESS -> {
-        // Convert per-replicate progress to cumulative
-        WireResponse cumulativeResponse = WireRewriteUtil.rewriteProgressToCumulative(
+        // Convert per-replicate progress to cumulative for worker coordination
+        WireResponse cumulativeResponse = WireRewriteUtil.rewriteProgressForWorkerCoordination(
             parsedResponse,
             cumulativeStepCount
         );

src/main/java/org/joshsim/wire/WireRewriteUtil.java

@@ -89,20 +89,51 @@ public static WireResponse rewriteReplicateNumber(WireResponse response, int new
     };
   }
 
+
+  /**
+   * Creates a new WireResponse with proper cumulative progress across replicates.
+   *
+   * <p>This method correctly calculates cumulative progress by treating the step count
+   * as the current position within the active replicate, not as an increment to add.
+   * The cumulative progress is calculated as:
+   * (completedReplicates * stepsPerReplicate) + currentStep</p>
+   *
+   * @param response The original progress response containing current step within replicate
+   * @param completedReplicates Number of fully completed replicates
+   * @param stepsPerReplicate Total steps per individual replicate
+   * @return A new WireResponse with proper cumulative progress count
+   * @throws IllegalArgumentException if the response is not a PROGRESS type
+   */
+  public static WireResponse rewriteProgressToProperCumulative(WireResponse response,
+                                                              int completedReplicates,
+                                                              long stepsPerReplicate) {
+    if (response == null) {
+      throw new IllegalArgumentException("Response cannot be null");
+    }
+
+    if (response.getType() != WireResponse.ResponseType.PROGRESS) {
+      throw new IllegalArgumentException("Can only rewrite PROGRESS responses to cumulative");
+    }
+
+    long currentStepInReplicate = response.getStepCount();
+    long cumulativeSteps = (completedReplicates * stepsPerReplicate) + currentStepInReplicate;
+    return new WireResponse(cumulativeSteps);
+  }
+
   /**
-   * Creates a new WireResponse with cumulative progress count.
+   * Creates a new WireResponse with cumulative progress count for worker coordination.
    *
    * <p>This method creates a PROGRESS response with a cumulative step count calculated
-   * by adding the original step count to the cumulative counter. This is useful for
-   * combining progress from multiple workers into a single cumulative progress stream.</p>
+   * by adding the original step count to the cumulative counter. This is specifically
+   * designed for coordinating progress from multiple parallel workers.</p>
    *
    * @param response The original progress response
    * @param cumulativeCounter The cumulative counter to update and use
    * @return A new WireResponse with cumulative progress count
    * @throws IllegalArgumentException if the response is not a PROGRESS type
    */
-  public static WireResponse rewriteProgressToCumulative(WireResponse response,
-                                                        AtomicInteger cumulativeCounter) {
+  public static WireResponse rewriteProgressForWorkerCoordination(WireResponse response,
+                                                                AtomicInteger cumulativeCounter) {
     if (response == null) {
       throw new IllegalArgumentException("Response cannot be null");
     }

src/test/java/org/joshsim/command/RemoteResponseHandlerTest.java

@@ -22,6 +22,7 @@
 import org.joshsim.util.OutputOptions;
 import org.joshsim.util.ProgressCalculator;
 import org.joshsim.util.ProgressUpdate;
+import org.joshsim.util.SimulationMetadata;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -37,6 +38,7 @@ public class RemoteResponseHandlerTest {
   private ExportFacade exportFacade;
   private ProgressCalculator progressCalculator;
   private OutputOptions outputOptions;
+  private SimulationMetadata metadata;
   private RemoteResponseHandler handler;
   private RemoteResponseHandler cumulativeHandler;
 
@@ -50,9 +52,12 @@ public void setUp() {
     exportFacade = mock(ExportFacade.class);
     progressCalculator = mock(ProgressCalculator.class);
     outputOptions = mock(OutputOptions.class);
+    metadata = mock(SimulationMetadata.class);
 
     when(context.getProgressCalculator()).thenReturn(progressCalculator);
     when(context.getOutputOptions()).thenReturn(outputOptions);
+    when(context.getMetadata()).thenReturn(metadata);
+    when(metadata.getTotalSteps()).thenReturn(100L); // 100 steps per replicate
     when(exportFactory.build(any(ExportTarget.class))).thenReturn(exportFacade);
 
     handler = new RemoteResponseHandler(context, exportFactory, false);
@@ -80,20 +85,51 @@ public void testProcessProgressResponse() {
   @Test
   public void testProcessProgressResponseWithCumulative() {
     // Arrange
-    String progressLine = "[progress 10]";
-    AtomicInteger cumulativeCounter = new AtomicInteger(30);
-    ProgressUpdate progressUpdate = new ProgressUpdate(true, 40.0, "Progress: 40%");
-    when(progressCalculator.updateStep(40)).thenReturn(progressUpdate);
+    String progressLine = "[progress 25]"; // Step 25 within current replicate
+    AtomicInteger cumulativeCounter = new AtomicInteger(0); // Not used in new implementation
+    ProgressUpdate progressUpdate = new ProgressUpdate(true, 25.0, "Progress: 25%");
+    // 0 completed replicates * 100 + 25 = 25
+    when(progressCalculator.updateStep(25)).thenReturn(progressUpdate);
 
     // Act
     Optional<org.joshsim.wire.WireResponse> result =
         cumulativeHandler.processResponseLine(progressLine, 0, cumulativeCounter);
 
     // Assert
     assertTrue(result.isPresent());
-    assertEquals(40, cumulativeCounter.get()); // 30 + 10
-    verify(progressCalculator).updateStep(40);
-    verify(outputOptions).printInfo("Progress: 40%");
+    verify(progressCalculator).updateStep(25); // Should be called with cumulative step count
+    verify(outputOptions).printInfo("Progress: 25%");
+  }
+
+  @Test
+  public void testProcessProgressResponseWithCumulativeAfterCompletedReplicate() {
+    // Arrange - Simulate having 2 completed replicates
+    final String endLine1 = "[end 0]";
+    final String endLine2 = "[end 1]";
+    ProgressUpdate endUpdate1 = new ProgressUpdate(true, 100.0, "Replicate 1 completed");
+    ProgressUpdate endUpdate2 = new ProgressUpdate(true, 100.0, "Replicate 2 completed");
+    when(progressCalculator.updateReplicateCompleted(1)).thenReturn(endUpdate1);
+    when(progressCalculator.updateReplicateCompleted(2)).thenReturn(endUpdate2);
+
+    // Complete 2 replicates first
+    cumulativeHandler.processResponseLine(endLine1, 0, null);
+    cumulativeHandler.processResponseLine(endLine2, 1, null);
+
+    // Now test progress in the 3rd replicate
+    String progressLine = "[progress 30]"; // Step 30 within current replicate
+    AtomicInteger cumulativeCounter = new AtomicInteger(0); // Not used in new implementation
+    ProgressUpdate progressUpdate = new ProgressUpdate(true, 230.0, "Progress: 230 steps");
+    // 2 completed * 100 + 30 = 230
+    when(progressCalculator.updateStep(230)).thenReturn(progressUpdate);
+
+    // Act
+    Optional<org.joshsim.wire.WireResponse> result =
+        cumulativeHandler.processResponseLine(progressLine, 2, cumulativeCounter);
+
+    // Assert
+    assertTrue(result.isPresent());
+    verify(progressCalculator).updateStep(230); // Should be called with proper cumulative count
+    verify(outputOptions).printInfo("Progress: 230 steps");
   }
 
   @Test

src/test/java/org/joshsim/wire/WireRewriteUtilTest.java

@@ -105,53 +105,87 @@ public void testRewriteReplicateNumberNullResponse() {
   }
 
   @Test
-  public void testRewriteProgressToCumulative() {
+  public void testRewriteProgressForWorkerCoordination() {
     AtomicInteger cumulativeCounter = new AtomicInteger(10);
     WireResponse original = new WireResponse(5);
 
     WireResponse rewritten =
-        WireRewriteUtil.rewriteProgressToCumulative(original, cumulativeCounter);
+        WireRewriteUtil.rewriteProgressForWorkerCoordination(original, cumulativeCounter);
 
     assertEquals(WireResponse.ResponseType.PROGRESS, rewritten.getType());
     assertEquals(15, rewritten.getStepCount()); // 10 + 5
     assertEquals(15, cumulativeCounter.get()); // Counter should be updated
   }
 
   @Test
-  public void testRewriteProgressToCumulativeMultipleTimes() {
+  public void testRewriteProgressForWorkerCoordinationMultipleTimes() {
     AtomicInteger cumulativeCounter = new AtomicInteger(0);
 
     WireResponse first = new WireResponse(10);
     WireResponse rewritten1 =
-        WireRewriteUtil.rewriteProgressToCumulative(first, cumulativeCounter);
+        WireRewriteUtil.rewriteProgressForWorkerCoordination(first, cumulativeCounter);
     assertEquals(10, rewritten1.getStepCount());
     assertEquals(10, cumulativeCounter.get());
 
     WireResponse second = new WireResponse(5);
     WireResponse rewritten2 =
-        WireRewriteUtil.rewriteProgressToCumulative(second, cumulativeCounter);
+        WireRewriteUtil.rewriteProgressForWorkerCoordination(second, cumulativeCounter);
     assertEquals(15, rewritten2.getStepCount());
     assertEquals(15, cumulativeCounter.get());
   }
 
   @Test
-  public void testRewriteProgressToCumulativeNullResponse() {
+  public void testRewriteProgressForWorkerCoordinationNullResponse() {
     AtomicInteger cumulativeCounter = new AtomicInteger(0);
     assertThrows(IllegalArgumentException.class, () -> {
-      WireRewriteUtil.rewriteProgressToCumulative(null, cumulativeCounter);
+      WireRewriteUtil.rewriteProgressForWorkerCoordination(null, cumulativeCounter);
     });
   }
 
   @Test
-  public void testRewriteProgressToCumulativeNonProgressResponse() {
+  public void testRewriteProgressForWorkerCoordinationNonProgressResponse() {
     AtomicInteger cumulativeCounter = new AtomicInteger(0);
     WireResponse datumResponse = new WireResponse(5, "data");
 
     assertThrows(IllegalArgumentException.class, () -> {
-      WireRewriteUtil.rewriteProgressToCumulative(datumResponse, cumulativeCounter);
+      WireRewriteUtil.rewriteProgressForWorkerCoordination(datumResponse, cumulativeCounter);
     });
   }
 
+  @Test
+  public void testRewriteProgressToProperCumulative() {
+    // Test with 0 completed replicates, step 50 of 100
+    WireResponse response = new WireResponse(50);
+    WireResponse rewritten = WireRewriteUtil.rewriteProgressToProperCumulative(response, 0, 100);
+
+    assertEquals(WireResponse.ResponseType.PROGRESS, rewritten.getType());
+    assertEquals(50, rewritten.getStepCount()); // 0 * 100 + 50 = 50
+  }
+
+  @Test
+  public void testRewriteProgressToProperCumulativeMultipleReplicates() {
+    // Test with 2 completed replicates, step 30 of 100 in current replicate
+    WireResponse response = new WireResponse(30);
+    WireResponse rewritten = WireRewriteUtil.rewriteProgressToProperCumulative(response, 2, 100);
+
+    assertEquals(WireResponse.ResponseType.PROGRESS, rewritten.getType());
+    assertEquals(230, rewritten.getStepCount()); // 2 * 100 + 30 = 230
+  }
+
+  @Test
+  public void testRewriteProgressToProperCumulativeWithNullResponse() {
+    assertThrows(IllegalArgumentException.class, () ->
+        WireRewriteUtil.rewriteProgressToProperCumulative(null, 1, 100));
+  }
+
+  @Test
+  public void testRewriteProgressToProperCumulativeWithNonProgressResponse() {
+    WireResponse datumResponse = new WireResponse(1, "test data");
+
+    assertThrows(IllegalArgumentException.class, () ->
+        WireRewriteUtil.rewriteProgressToProperCumulative(datumResponse, 1, 100));
+  }
+
   @Test
   public void testFormatWireResponseProgress() {
     WireResponse response = new WireResponse(42);