Fix qualifying/finals/bump-up/judging/pause-timing bugs

A thorough audit of practice/qualifying/heats/finals, bump-up, judging and pause/timer code (requested by the user) found several real, concrete bugs, fixed together: - Qualifying reseed sorted heats as text ("H10" before "H2"), scrambling grids at 10+ heats; now sorts numerically and reports a clear warning instead of silently dropping/ghosting heats when the participant count changes. - A driver with zero laps in a finished heat/final (DNS) vanished entirely - not just from that round, but silently from every future reseeded round and the final grid. Now backfilled as a real, last-place "DNS" row. - Bump-up did nothing at all with "reserve bump slots" off (mains were already seeded full, so the old capacity check always blocked it), and a promoted driver stayed double-entered in their old main's upcoming leg. An oversized bump count could also blow the main-letter naming past "Z". All three fixed. - Judging undo restored an absolute snapshot instead of reversing its own delta - undoing two penalties out of order could leave a live penalty behind two "undone" log entries. Now always nets correctly regardless of undo order. - A competitor's elapsed time didn't exclude paused time (only the session clock did), making a paused heat's results incomparable to an unpaused one in qualifying ranking. Finishing while paused also dropped the final pause segment, so the displayed elapsed time kept growing after finish. Both fixed. Verified with new unit tests for every fix (including a new session_control.test.js - that file had zero prior coverage) and a real headless-browser run confirming the judging-undo and bump-up fixes end to end through the actual UI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-10 17:41:10 +02:00
parent 15b574a37d
commit f0838bc5db
14 changed files with 464 additions and 36 deletions
--- a/tests/judging_logic.test.js
+++ b/tests/judging_logic.test.js
@@ -99,6 +99,33 @@ test("manual corrections accumulate, reset, and undo to the prior state", () =>
  assert.equal(harness.getSaveCount(), 3);
 });

+test("undoing two time penalties in either order always removes exactly their own contribution", () => {
+  const forward = createHarness();
+  applyCompetitorCorrection(forward.session, forward.row, { timeMsDelta: 5000 }, forward.deps);
+  applyCompetitorCorrection(forward.session, forward.row, { timeMsDelta: 5000 }, forward.deps);
+  assert.equal(forward.row.manualTimeAdjustmentMs, 10000, "both +5s penalties applied");
+  const [first, second] = forward.result.adjustments;
+
+  // Undo the FIRST one first.
+  assert.equal(undoJudgingAdjustment(forward.session, first.id, forward.deps), true);
+  assert.equal(forward.row.manualTimeAdjustmentMs, 5000, "only the first penalty's +5s is removed, the second's stays");
+  assert.equal(undoJudgingAdjustment(forward.session, second.id, forward.deps), true);
+  assert.equal(forward.row.manualTimeAdjustmentMs, 0, "both undone, in first-applied order, nets to zero");
+
+  const reverse = createHarness();
+  applyCompetitorCorrection(reverse.session, reverse.row, { timeMsDelta: 5000 }, reverse.deps);
+  applyCompetitorCorrection(reverse.session, reverse.row, { timeMsDelta: 5000 }, reverse.deps);
+  const [firstR, secondR] = reverse.result.adjustments;
+
+  // Undo the SECOND one first (out of order) - this is exactly the sequence the user
+  // reported: undoing an earlier penalty used to wipe out both, then undoing the other
+  // silently re-added one back, leaving a live +5s penalty behind two "undone" entries.
+  assert.equal(undoJudgingAdjustment(reverse.session, secondR.id, reverse.deps), true);
+  assert.equal(reverse.row.manualTimeAdjustmentMs, 5000, "only the second penalty's +5s is removed, the first's stays");
+  assert.equal(undoJudgingAdjustment(reverse.session, firstR.id, reverse.deps), true);
+  assert.equal(reverse.row.manualTimeAdjustmentMs, 0, "both undone, in reverse order, still nets to zero - order must not matter");
+});
+
 test("invalidating and restoring the latest lap recalculates the competitor", () => {
  const harness = createHarness();

--- a/tests/race_timing_logic.test.js
+++ b/tests/race_timing_logic.test.js
@@ -15,19 +15,28 @@ import {
 } from "../src/event_race_logic.js";
 import {
  buildFinalStandings,
+  buildLeaderboard,
+  buildQualifyingStandings,
+  buildQualifyingTieBreakNote as buildQualifyingTieBreakNoteLogic,
  buildSessionChannelStats,
  compareNumberSet,
  formatLeaderboardGap,
+  getCompetitorElapsedMs,
  getCompetitorSeedMetric,
  getQualifyingPointsValue,
  getSessionLapWindow,
  getSessionTargetMs,
  getSessionTiming,
+  hasQualifyingPrimaryTie,
  isHighPointsTable,
  isUntimedSession,
  ensureSessionResult,
 } from "../src/timing_logic.js";

+const testFormatLap = (ms) => (Number.isFinite(ms) ? `${ms}ms` : "-");
+const testFormatRaceClock = (ms) => `${ms}ms`;
+const testTranslate = (key, vars) => (vars ? `${key}:${JSON.stringify(vars)}` : key);
+
 function createUid() {
  let index = 0;
  return (prefix) => `${prefix}_${++index}`;
@@ -283,12 +292,44 @@ test("qualifying reseed updates ready auto grids but leaves manual grids locked"
    }
  );

-  assert.deepEqual(result, { updated: 1, locked: 1 });
+  assert.deepEqual(result, { updated: 1, locked: 1, mismatched: 0 });
  assert.deepEqual(sessions[0].driverIds, ["driver_3", "driver_1"]);
  assert.deepEqual(sessions[1].driverIds, ["locked"]);
  assert.deepEqual(sessions[2].driverIds, ["finished"]);
 });

+test("qualifying reseed sorts heats numerically, not as text (H10 must not sort before H2)", () => {
+  const sessions = ["H1", "H2", "H3", "H4", "H5", "H6", "H7", "H8", "H9", "H10", "H11"].map((heat) => ({
+    name: `Q1 ${heat}`, type: "qualification", generated: true, status: "ready", driverIds: [], manualGridIds: [], gridCustomized: false,
+  }));
+  const standings = Array.from({ length: 22 }, (_, index) => ({ driverId: `driver_${index + 1}`, rank: index + 1 }));
+  const result = reseedUpcomingQualifying(
+    { id: "event_1", raceConfig: { carsPerHeat: 2 } },
+    { buildQualifyingStandings: () => standings, buildPracticeStandings: () => [], getEventDrivers: () => [], getSessionsForEvent: () => sessions, chunkArray }
+  );
+
+  assert.deepEqual(result, { updated: 11, locked: 0, mismatched: 0 });
+  const heat2 = sessions.find((session) => session.name === "Q1 H2");
+  const heat10 = sessions.find((session) => session.name === "Q1 H10");
+  assert.deepEqual(heat2.driverIds, ["driver_3", "driver_4"], "the 2nd-fastest pair lands in heat 2, not wherever text-sort would put it");
+  assert.deepEqual(heat10.driverIds, ["driver_19", "driver_20"], "heat 10 gets the pair its numeric position actually calls for");
+});
+
+test("qualifying reseed reports a field-size mismatch instead of silently dropping or ghosting heats", () => {
+  const sessions = [
+    { name: "Q1 H1", type: "qualification", generated: true, status: "ready", driverIds: [], manualGridIds: [], gridCustomized: false },
+    { name: "Q1 H2", type: "qualification", generated: true, status: "ready", driverIds: [], manualGridIds: [], gridCustomized: false },
+  ];
+  // 4 heats' worth of drivers now exist, but only 2 ready heat sessions do.
+  const standings = Array.from({ length: 8 }, (_, index) => ({ driverId: `driver_${index + 1}`, rank: index + 1 }));
+  const result = reseedUpcomingQualifying(
+    { id: "event_1", raceConfig: { carsPerHeat: 2 } },
+    { buildQualifyingStandings: () => standings, buildPracticeStandings: () => [], getEventDrivers: () => [], getSessionsForEvent: () => sessions, chunkArray }
+  );
+
+  assert.equal(result.mismatched, 1, "the round's ready-heat count no longer matches the participant list");
+});
+
 test("final generation reserves bump slots and keeps seeded grid order", () => {
  const state = { sessions: [] };
  const sourceRows = Array.from({ length: 7 }, (_, index) => ({ driverId: `driver_${index + 1}` }));
@@ -332,11 +373,34 @@ test("final generation reserves bump slots and keeps seeded grid order", () => {
  assert.deepEqual(state.sessions[0].manualGridIds, state.sessions[0].driverIds);
 });

-test("bump-up promotes lower-main winners into reserved upper-main slots", () => {
+test("final generation never produces more than 26 mains, even with a large field and an aggressive bump count", () => {
+  const state = { sessions: [] };
+  const sourceRows = Array.from({ length: 60 }, (_, index) => ({ driverId: `driver_${index + 1}` }));
+  const event = {
+    id: "event_1",
+    raceConfig: {
+      finalsSource: "qualifying", finalLegs: 1, carsPerFinal: 4, finalDurationMin: 5,
+      finalStartMode: "position", followUpSec: 0, bumpCount: 50, reserveBumpSlots: true,
+    },
+  };
+  generateFinalsForRace(event, {
+    buildPracticeStandings: () => [], buildQualifyingStandings: () => sourceRows, clearGeneratedFinals: () => {},
+    state, normalizeStartMode, normalizeSession: (session) => session, uid: createUid(), chunkArray, t: (key) => key,
+  });
+
+  assert.ok(state.sessions.length <= 26, `expected at most 26 mains, got ${state.sessions.length}`);
+  assert.ok(state.sessions.every((session) => /^[A-Z] /.test(session.name)), "every main name is a single real letter, never running past Z");
+});
+
+test("bump-up promotes lower-main winners into reserved upper-main slots and removes them from the lower main's remaining legs", () => {
  const sessions = [
    { name: "A final 1", type: "final", status: "ready", maxCars: 4, reservedBumpSlots: 1, driverIds: ["a1", "a2", "a3"], manualGridIds: ["a1", "a2", "a3"] },
-    { name: "B final 1", type: "final", status: "ready", maxCars: 4, reservedBumpSlots: 1, driverIds: ["b1", "b2", "b3"], manualGridIds: ["b1", "b2", "b3"] },
-    { name: "C final 1", type: "final", status: "ready", maxCars: 4, reservedBumpSlots: 0, driverIds: ["c1"] },
+    // B final 1 already ran (produced the standings below) and keeps its real result. B
+    // final 2 is the next leg still to come - that's the one that should lose the
+    // promoted driver, not the one that already happened.
+    { name: "B final 1", type: "final", status: "finished", maxCars: 4, driverIds: ["b1", "b2", "b3"] },
+    { name: "B final 2", type: "final", status: "ready", maxCars: 4, reservedBumpSlots: 1, driverIds: ["b1", "b2", "b3"], manualGridIds: ["b1", "b2", "b3"] },
+    { name: "C final 1", type: "final", status: "finished", maxCars: 4, driverIds: ["c1"] },
  ];
  const standings = [
    { rank: "A1", driverId: "a1" },
@@ -345,15 +409,33 @@ test("bump-up promotes lower-main winners into reserved upper-main slots", () =>
    { rank: "C1", driverId: "c1" },
  ];
  const applied = applyBumpsForRace(
-    { id: "event_1", raceConfig: { bumpCount: 1, carsPerFinal: 4 } },
+    { id: "event_1", raceConfig: { bumpCount: 1, carsPerFinal: 4, reserveBumpSlots: true } },
    { buildFinalStandings: () => standings, getSessionsForEvent: () => sessions }
  );

  assert.equal(applied, 2);
-  assert.deepEqual(sessions[0].driverIds, ["a1", "a2", "a3", "b1"]);
-  assert.deepEqual(sessions[1].driverIds, ["b1", "b2", "b3", "c1"]);
+  assert.deepEqual(sessions[0].driverIds, ["a1", "a2", "a3", "b1"], "b1 promoted into A");
+  assert.deepEqual(sessions[1].driverIds, ["b1", "b2", "b3"], "B's already-finished leg keeps its real historical result");
+  assert.deepEqual(sessions[2].driverIds, ["b2", "b3", "c1"], "b1 removed from B's upcoming leg (promoted out), c1 added (promoted in) - no more double-entry");
  assert.equal(sessions[0].reservedBumpSlots, 0);
-  assert.equal(sessions[1].reservedBumpSlots, 0);
+});
+
+test("bump-up still grows a main when reserveBumpSlots is off, instead of silently doing nothing because the main looks full", () => {
+  const sessions = [
+    { name: "A final 1", type: "final", status: "ready", maxCars: 4, reservedBumpSlots: 0, driverIds: ["a1", "a2", "a3", "a4"], manualGridIds: ["a1", "a2", "a3", "a4"] },
+    { name: "B final 1", type: "final", status: "finished", maxCars: 4, driverIds: ["b1", "b2", "b3", "b4"] },
+  ];
+  const standings = [
+    { rank: "A1", driverId: "a1" },
+    { rank: "B1", driverId: "b1" },
+  ];
+  const applied = applyBumpsForRace(
+    { id: "event_1", raceConfig: { bumpCount: 1, carsPerFinal: 4, reserveBumpSlots: false } },
+    { buildFinalStandings: () => standings, getSessionsForEvent: () => sessions }
+  );
+
+  assert.equal(applied, 1, "b1 is actually added, not blocked because A already has 4/4 seeded drivers");
+  assert.deepEqual(sessions[0].driverIds, ["a1", "a2", "a3", "a4", "b1"]);
 });

 test("session timing handles untimed practice, countdown, and follow-up", () => {
@@ -378,6 +460,58 @@ test("session timing handles untimed practice, countdown, and follow-up", () =>
  assert.equal(untimed.elapsedMs, 60000);
 });

+test("a finished session's elapsed time is frozen at endedAt, not still counting up against the real clock", () => {
+  const getTarget = (session) => getSessionTargetMs(session, { isUntimedSession });
+  const finished = { type: "final", durationMin: 5, status: "finished", startedAt: 1000, endedAt: 301000 };
+
+  const rightAfter = getSessionTiming(finished, 301000, { getSessionTargetMs: getTarget });
+  const muchLater = getSessionTiming(finished, 301000 + 3600000, { getSessionTargetMs: getTarget });
+  assert.equal(rightAfter.elapsedMs, 300000);
+  assert.equal(muchLater.elapsedMs, 300000, "checking an hour later must report the exact same, fixed elapsed time");
+});
+
+test("getCompetitorElapsedMs excludes the session's accumulated paused time", () => {
+  const session = { startedAt: 1000, totalPausedMs: 5000 };
+  assert.equal(getCompetitorElapsedMs(session, { lastTimestamp: 21000 }), 15000);
+  assert.equal(getCompetitorElapsedMs({ startedAt: 1000, totalPausedMs: 0 }, { lastTimestamp: 21000 }), 20000, "no pause, no change");
+  assert.equal(getCompetitorElapsedMs(session, { lastTimestamp: 0 }), 0, "no passing recorded yet");
+});
+
+test("buildLeaderboard excludes paused wall-clock time from a competitor's elapsed/comparison time, so a paused heat stays comparable to an unpaused one", () => {
+  const result = {
+    competitors: {
+      driver_1: { key: "driver_1", driverName: "One", startTimestamp: 1000, laps: 1, lastLapMs: 20000, lastTimestamp: 21000, bestLapMs: 20000, manualLapAdjustment: 0, manualTimeAdjustmentMs: 0 },
+    },
+  };
+  const passings = [{ competitorKey: "driver_1", timestamp: 21000, lapMs: 20000, validLap: true }];
+  // A 5000ms track call happened mid-heat - nobody actually raced during it, so it must
+  // not count against this driver's comparison time.
+  const session = { id: "s1", type: "final", startedAt: 1000, totalPausedMs: 5000, durationMin: 5 };
+
+  const rows = buildLeaderboard(session, {
+    ensureSessionResult: () => result,
+    getSessionTargetMs: (s) => getSessionTargetMs(s, { isUntimedSession }),
+    getCompetitorPassings: (_s, row, options = {}) => passings
+      .filter((passing) => passing.competitorKey === row.key)
+      .filter((passing) => options.includeInvalid || passing.validLap !== false),
+    isCountedPassing: (passing) => passing.validLap !== false,
+    getCompetitorElapsedMs,
+    getCompetitorSeedMetric: () => null,
+    getSessionLapWindow: () => ({ minLapMs: 0, maxLapMs: Infinity }),
+    getPassingValidationLabel: () => "",
+    formatSeedMetric: () => "",
+    formatRaceClock: (ms) => `${ms}`,
+    formatLeaderboardGap: () => "",
+    formatLapDelta: () => "",
+    formatLap: (ms) => `${ms}`,
+    t: (key) => key,
+  });
+
+  assert.equal(rows.length, 1);
+  assert.equal(rows[0].totalElapsedMs, 15000, "the 5000ms pause is excluded from the driver's own elapsed/comparison time");
+  assert.equal(rows[0].comparisonMs, 15000);
+});
+
 test("race lap window and seed metrics enforce timing rules", () => {
  const state = {
    events: [
@@ -465,6 +599,64 @@ test("final standings count the configured best legs per main", () => {
  assert.equal(standings[1].score, "3 (1+2+2)");
 });

+test("final standings keep a driver who never recorded a passing (DNS) instead of dropping them, ranked last in their main", () => {
+  const sessions = [{ id: "a1", type: "final", name: "A final 1", status: "finished" }];
+  const rowsBySession = {
+    a1: [{ driverId: "driver_1", driverName: "One", totalElapsedMs: 301000, resultDisplay: "12/5:01.000" }],
+  };
+  const entrants = [{ id: "driver_1", name: "One" }, { id: "driver_2", name: "Two" }];
+  const standings = buildFinalStandings(
+    { id: "event_1", raceConfig: {} },
+    {
+      getSessionsForEvent: () => sessions,
+      buildLeaderboard: (session) => rowsBySession[session.id],
+      getSessionEntrants: () => entrants,
+      t: testTranslate,
+    }
+  );
+
+  assert.deepEqual(standings.map((row) => row.driverId), ["driver_1", "driver_2"]);
+  assert.equal(standings[1].driverId, "driver_2");
+  assert.equal(standings[1].rank, "A2", "the DNS driver still gets a real, last-place rank in their main");
+
+  // A not-yet-run final must not invent DNS rows for drivers who simply haven't raced yet.
+  const notYetRun = buildFinalStandings(
+    { id: "event_1", raceConfig: {} },
+    {
+      getSessionsForEvent: () => [{ id: "a1", type: "final", name: "A final 1", status: "ready" }],
+      buildLeaderboard: () => rowsBySession.a1,
+      getSessionEntrants: () => entrants,
+      t: testTranslate,
+    }
+  );
+  assert.deepEqual(notYetRun.map((row) => row.driverId), ["driver_1"], "driver_2 hasn't raced yet, not DNS - correctly absent");
+});
+
+test("qualifying standings keep a driver who never recorded a passing (DNS) so they aren't silently dropped from future rounds or finals", () => {
+  const sessions = [{ id: "q1h1", type: "qualification", name: "Q1 H1", status: "finished" }];
+  const rows = [{ driverId: "driver_1", driverName: "One", comparisonMs: 16000, bestLapMs: 16000, resultDisplay: "16.000" }];
+  const entrants = [{ id: "driver_1", name: "One" }, { id: "driver_2", name: "Two" }];
+  const standings = buildQualifyingStandings(
+    { id: "event_1", raceConfig: {} },
+    {
+      getSessionsForEvent: () => sessions,
+      buildLeaderboard: () => rows,
+      getSessionEntrants: () => entrants,
+      isHighPointsTable,
+      getQualifyingPointsValue,
+      compareNumberSet,
+      buildQualifyingTieBreakNote: (row, tieBreak) => buildQualifyingTieBreakNoteLogic(row, tieBreak, { t: testTranslate, formatLap: testFormatLap }),
+      formatLap: testFormatLap,
+      formatRaceClock: testFormatRaceClock,
+      t: testTranslate,
+      hasQualifyingPrimaryTie,
+    }
+  );
+
+  assert.deepEqual(standings.map((row) => row.driverId), ["driver_1", "driver_2"], "driver_2 (DNS) still appears, ranked last");
+  assert.equal(standings[1].bestSingleLapMs, Number.MAX_SAFE_INTEGER, "a DNS driver's missing lap is never confused for a 0ms fastest lap");
+});
+
 test("session channel statistics summarize splits and pit stops per competitor", () => {
  const result = {
    competitors: {
--- a/tests/run.js
+++ b/tests/run.js
@@ -5,6 +5,7 @@ import "./track_layouts_logic.test.js";
 import "./race_day_logic.test.js";
 import "./backup_service.test.js";
 import "./judging_logic.test.js";
+import "./session_control.test.js";
 import "./runtime_persistence.test.js";
 import "./audio_runtime.test.js";
 import "./race_audio.test.js";
--- a/tests/session_control.test.js
+++ b/tests/session_control.test.js
@@ -0,0 +1,61 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { beginSession, finishSession, pauseSession, resumeSession } from "../src/session_control.js";
+
+test("beginSession resets pause/timing bookkeeping for a fresh start", () => {
+  const session = { status: "ready", pausedAt: 999, totalPausedMs: 5000, followUpPausedMs: 2000 };
+  beginSession(session, 1000);
+  assert.equal(session.status, "running");
+  assert.equal(session.startedAt, 1000);
+  assert.equal(session.pausedAt, null);
+  assert.equal(session.totalPausedMs, 0);
+  assert.equal(session.followUpPausedMs, 0);
+});
+
+test("pauseSession only takes effect on a running session", () => {
+  const session = { status: "running" };
+  assert.equal(pauseSession(session, 1000), true);
+  assert.equal(session.status, "paused");
+  assert.equal(session.pausedAt, 1000);
+
+  assert.equal(pauseSession({ status: "paused" }, 1000), false, "already paused");
+  assert.equal(pauseSession({ status: "ready" }, 1000), false, "not started yet");
+});
+
+test("resumeSession accumulates paused duration across multiple pause/resume cycles", () => {
+  const session = { status: "running", startedAt: 0, totalPausedMs: 0 };
+  pauseSession(session, 1000);
+  resumeSession(session, 3000); // paused 2000ms
+  assert.equal(session.totalPausedMs, 2000);
+  assert.equal(session.pausedAt, null);
+  assert.equal(session.status, "running");
+
+  pauseSession(session, 5000);
+  resumeSession(session, 5750); // paused another 750ms
+  assert.equal(session.totalPausedMs, 2750, "second pause segment adds to the running total, not replaces it");
+});
+
+test("resumeSession also folds the pause into an active follow-up period", () => {
+  const session = { status: "running", followUpStartedAt: 4000, followUpPausedMs: 0, totalPausedMs: 0 };
+  pauseSession(session, 4500);
+  resumeSession(session, 5200); // paused 700ms during follow-up
+  assert.equal(session.totalPausedMs, 700);
+  assert.equal(session.followUpPausedMs, 700);
+});
+
+test("finishSession while paused folds the still-open pause segment into totalPausedMs instead of dropping it", () => {
+  const session = { status: "paused", startedAt: 0, pausedAt: 10000, totalPausedMs: 2000 };
+  finishSession(session, 13000); // was paused for 3000ms when finished
+  assert.equal(session.status, "finished");
+  assert.equal(session.endedAt, 13000);
+  assert.equal(session.pausedAt, null);
+  assert.equal(session.totalPausedMs, 5000, "the 3000ms final pause segment is added to the 2000ms already accumulated");
+});
+
+test("finishSession on a running (not paused) session leaves totalPausedMs untouched", () => {
+  const session = { status: "running", startedAt: 0, totalPausedMs: 1500 };
+  finishSession(session, 20000);
+  assert.equal(session.totalPausedMs, 1500);
+  assert.equal(session.endedAt, 20000);
+});