Skip to content

Commit 94de58c

Browse files
jhfclaude
andcommitted
fix: Correct seenSteps restart detection and remove dead code
Fix false restart detection in mergeSeenSteps: steps with total <= 1 (like derive_reports) were absent from the filtered `progress` array but present in `seenSteps`, causing false iteration resets on every SSE update. Added `lastProgressSteps` (unfiltered) to PhaseStatus and compare against that instead. Remove unused `computePhaseProgress` function, replaced by `computeWeightedPhaseProgress`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0105006 commit 94de58c

File tree

2 files changed

+17
-21
lines changed

2 files changed

+17
-21
lines changed

app/src/atoms/worker_status.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export interface PhaseStatus {
4545
active: boolean;
4646
progress: PipelineStep[];
4747
seenSteps: string[];
48+
lastProgressSteps: string[];
4849
}
4950

5051
export interface WorkerStatus {
@@ -168,20 +169,24 @@ export const setWorkerStatusAtom = atom(
168169

169170
// Track seenSteps: remember which top-level steps we've observed.
170171
// If a step reappears that was already seen (and is not currently in
171-
// pipeline_progress), it means a new iteration started — reset seenSteps.
172+
// the previous unfiltered pipeline_progress), it means a new iteration
173+
// started — reset seenSteps.
174+
//
175+
// We compare against lastProgressSteps (unfiltered) rather than progress
176+
// (filtered to total > 1) to avoid false restarts for steps with total <= 1
177+
// like derive_reports.
172178
const mergeSeenSteps = (
173179
currentSteps: PipelineStep[],
174180
prevPhase: PhaseStatus | null,
175181
): string[] => {
176182
const incomingNames = currentSteps.map(s => s.step);
177183
const prevSeen = prevPhase?.seenSteps ?? [];
184+
const prevInProgress = new Set(prevPhase?.lastProgressSteps ?? []);
178185
// Detect new iteration: a step that was previously seen and completed
179186
// (no longer in pipeline_progress) is now back in pipeline_progress.
180-
// Since pipeline_progress entries are deleted when parent completes,
181-
// a step being in both prevSeen and incomingNames means it restarted.
182187
const restarted = incomingNames.some(name =>
183188
prevSeen.includes(name) &&
184-
!prevPhase?.progress.some(s => s.step === name)
189+
!prevInProgress.has(name)
185190
);
186191
if (restarted) {
187192
// New iteration — start fresh with only current steps
@@ -200,13 +205,15 @@ export const setWorkerStatusAtom = atom(
200205
active: true,
201206
progress: phase1Steps.filter(s => s.total > 1),
202207
seenSteps: mergeSeenSteps(phase1Steps, prevStatus.derivingUnits),
208+
lastProgressSteps: phase1Steps.map(s => s.step),
203209
}
204210
: prevStatus.derivingUnits,
205211
derivingReports: phase2Steps.length > 0
206212
? {
207213
active: true,
208214
progress: phase2Steps.filter(s => s.total > 1),
209215
seenSteps: mergeSeenSteps(phase2Steps, prevStatus.derivingReports),
216+
lastProgressSteps: phase2Steps.map(s => s.step),
210217
}
211218
: prevStatus.derivingReports,
212219
// Keep boolean fields in sync
@@ -233,12 +240,12 @@ export const setWorkerStatusAtom = atom(
233240
} else if (type === 'is_deriving_statistical_units') {
234241
updatedStatus.isDerivingUnits = status;
235242
if (!status) {
236-
updatedStatus.derivingUnits = { active: false, progress: [], seenSteps: [] };
243+
updatedStatus.derivingUnits = { active: false, progress: [], seenSteps: [], lastProgressSteps: [] };
237244
}
238245
} else if (type === 'is_deriving_reports') {
239246
updatedStatus.isDerivingReports = status;
240247
if (!status) {
241-
updatedStatus.derivingReports = { active: false, progress: [], seenSteps: [] };
248+
updatedStatus.derivingReports = { active: false, progress: [], seenSteps: [], lastProgressSteps: [] };
242249
}
243250
}
244251

@@ -318,16 +325,16 @@ export const refreshWorkerStatusAtom = atom(
318325

319326
// Parse JSONB responses - PostgREST types still say boolean but DB returns jsonb objects
320327
const importData = importingRes.data as unknown as ImportStatus | null;
321-
const unitsData = derivingUnitsRes.data as unknown as Omit<PhaseStatus, 'seenSteps'> | null;
322-
const reportsData = derivingReportsRes.data as unknown as Omit<PhaseStatus, 'seenSteps'> | null;
328+
const unitsData = derivingUnitsRes.data as unknown as Omit<PhaseStatus, 'seenSteps' | 'lastProgressSteps'> | null;
329+
const reportsData = derivingReportsRes.data as unknown as Omit<PhaseStatus, 'seenSteps' | 'lastProgressSteps'> | null;
323330

324331
set(workerStatusAtom, {
325332
isImporting: importData?.active ?? null,
326333
isDerivingUnits: unitsData?.active ?? null,
327334
isDerivingReports: reportsData?.active ?? null,
328335
importing: importData,
329-
derivingUnits: unitsData ? { ...unitsData, seenSteps: [] } : null,
330-
derivingReports: reportsData ? { ...reportsData, seenSteps: [] } : null,
336+
derivingUnits: unitsData ? { ...unitsData, seenSteps: [], lastProgressSteps: [] } : null,
337+
derivingReports: reportsData ? { ...reportsData, seenSteps: [], lastProgressSteps: [] } : null,
331338
loading: false,
332339
error: null,
333340
});

app/src/components/navbar.tsx

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,6 @@ export function NavbarSkeleton() {
2828
);
2929
}
3030

31-
/**
32-
* Compute an overall progress percentage from pipeline steps.
33-
*/
34-
function computePhaseProgress(progress: PipelineStep[]): number | null {
35-
if (progress.length === 0) return null;
36-
const totalSum = progress.reduce((acc, s) => acc + s.total, 0);
37-
const completedSum = progress.reduce((acc, s) => acc + s.completed, 0);
38-
if (totalSum === 0) return null;
39-
return Math.round((completedSum / totalSum) * 100);
40-
}
41-
4231
// Analysis is ~25% of total import time, processing ~75% (measured on real jobs).
4332
const ANALYSIS_WEIGHT = 0.25;
4433
const PROCESSING_WEIGHT = 1 - ANALYSIS_WEIGHT;

0 commit comments

Comments
 (0)