Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/loopover-engine/src/loop-progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,21 @@ function activityChanged(prev: readonly LoopProgressActivity[], next: readonly L

/** True when `next` differs from `prev` in a way worth pushing to the customer — so the surface streams
* ON CHANGE instead of polling on a fixed interval (#4800's acceptance). A null `prev` (the first snapshot)
* always pushes. Compares the displayed axes: phase, status, iteration, and the activity tail's contents. */
* always pushes. Compares every displayed axis: phase, status, iteration, the iteration budget and the
* percent through it (maxIterations/percentComplete), and the activity tail's contents.
*
* maxIterations/percentComplete are displayed too but were originally left out of the diff (#9323): raising
* the iteration budget mid-run (#6773's percentComplete is `iteration / maxIterations`) moves the customer's
* percent while phase/status/iteration/activity all hold, so a budget-only change silently went unpushed and
* the progress bar went stale. Both are compared so any change to a shown number streams. */
export function progressChanged(prev: ProgressSnapshot | null, next: ProgressSnapshot): boolean {
if (prev === null) return true;
return (
prev.phase !== next.phase ||
prev.status !== next.status ||
prev.iteration !== next.iteration ||
prev.maxIterations !== next.maxIterations ||
prev.percentComplete !== next.percentComplete ||
activityChanged(prev.recentActivity, next.recentActivity)
);
}
30 changes: 30 additions & 0 deletions test/unit/loop-progress.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
MAX_PROGRESS_ACTIVITY,
type LoopProgressActivity,
type LoopProgressState,
type ProgressSnapshot,
} from "../../packages/loopover-engine/src/loop-progress";

function running(overrides: Partial<LoopProgressState> = {}): LoopProgressState {
Expand Down Expand Up @@ -67,6 +68,35 @@ describe("progressChanged — push on change, not on a fixed interval (#4800)",
expect(progressChanged(base, buildProgressSnapshot(running({ recentActivity: [{ step: "a" }] })))).toBe(false);
});

// #9323: maxIterations and percentComplete are displayed axes too, but were left out of the diff. Raising the
// iteration budget mid-run moves the customer's percent while phase/status/iteration/activity all hold — that
// must still push, or the progress bar shows a stale percent.
it("REGRESSION (#9323): pushes when only the iteration budget (maxIterations, and so percentComplete) changes", () => {
const widerBudget = buildProgressSnapshot(running({ maxIterations: 10, recentActivity: [{ step: "a" }] }));
// Everything `base` displays is held constant except the budget (5→10) and its derived percent (40→20).
expect(base).toMatchObject({ maxIterations: 5, percentComplete: 40 });
expect(widerBudget).toMatchObject({ phase: base.phase, status: base.status, iteration: base.iteration, maxIterations: 10, percentComplete: 20 });
expect(widerBudget.recentActivity).toEqual(base.recentActivity);
expect(progressChanged(base, widerBudget)).toBe(true);
});

// #9323 regression guard against over-triggering: identical on ALL six displayed axes (including the two new
// ones) must still not push.
it("does not push when maxIterations and percentComplete are identical alongside the other axes", () => {
const same = buildProgressSnapshot(running({ recentActivity: [{ step: "a" }] }));
expect(same).toMatchObject({ maxIterations: base.maxIterations, percentComplete: base.percentComplete });
expect(progressChanged(base, same)).toBe(false);
});

// #9323: percentComplete is compared as its own axis, so a change to just that shown number still pushes even
// if maxIterations is unchanged. It is normally derived from iteration/maxIterations, so isolate it by building
// the snapshot directly rather than through buildProgressSnapshot.
it("REGRESSION (#9323): pushes when only percentComplete differs, with maxIterations and every other axis held", () => {
const shiftedPercent: ProgressSnapshot = { ...base, percentComplete: (base.percentComplete ?? 0) + 1 };
expect(shiftedPercent.maxIterations).toBe(base.maxIterations);
expect(progressChanged(base, shiftedPercent)).toBe(true);
});

// #6171: the tail is capped, so past the cap every new event evicts the oldest and the LENGTH stops moving.
// A length-only check went permanently blind here — exactly on the long runs that stream the most.
describe("activity tail at its cap (#6171)", () => {
Expand Down