Address code review: single source of truth for presets, useEffect guard, tests

- Extract CHESS_CLOCK_PRESETS (pure data) into draft-timer.ts as the
  canonical source of truth; DraftSpeedPicker.tsx now spreads those
  values into CHESS_PRESETS rather than duplicating the numbers
- getInitialDraftSpeed moved to draft-timer.ts and rewritten to use
  CHESS_CLOCK_PRESETS.find() — no more hardcoded bankSec/incrSec values
  in three separate places
- parseDraftSpeed switch replaced with CHESS_CLOCK_PRESETS.find() and
  an explicit fallback comment; eliminates the silent default-handles-
  "standard" fragility called out in review
- useEffect that re-marks settings dirty now guards against non-settings
  errors (draft-order, commissioner actions) which carry a `section`
  field — previously any error actionData would spuriously set
  hasUnsavedSettingsChanges=true
- Add parseDraftSpeed and getInitialDraftSpeed test suites to
  draft-timer.test.ts, including parametrised preset coverage via
  it.each(CHESS_CLOCK_PRESETS)

https://claude.ai/code/session_01DgHNkvJXGizx41CE2tkVS1

app/components/league/DraftSpeedPicker.tsx

@@ -10,6 +10,7 @@ import {
   SelectValue,
 } from "~/components/ui/select";
 import { cn } from "~/lib/utils";
+import { CHESS_CLOCK_PRESETS } from "~/lib/draft-timer";
 
 // ─── Types & constants ────────────────────────────────────────────────────────
 
@@ -37,10 +38,10 @@ export const CHESS_PRESETS: {
   bankSec: number;
   incrSec: number;
 }[] = [
-  { value: "fast",      label: "Fast",      icon: Zap,    desc: "~1 hour",     bankSec: 60,    incrSec: 10   },
-  { value: "standard",  label: "Standard",  icon: Timer,  desc: "~90 minutes", bankSec: 120,   incrSec: 15   },
-  { value: "slow",      label: "Slow",      icon: Coffee, desc: "~1 week",     bankSec: 28800, incrSec: 3600 },
-  { value: "very-slow", label: "Very Slow", icon: Snail,  desc: "~2 weeks",    bankSec: 43200, incrSec: 3600 },
+  { ...CHESS_CLOCK_PRESETS[0], label: "Fast",      icon: Zap,    desc: "~1 hour"     },
+  { ...CHESS_CLOCK_PRESETS[1], label: "Standard",  icon: Timer,  desc: "~90 minutes" },
+  { ...CHESS_CLOCK_PRESETS[2], label: "Slow",      icon: Coffee, desc: "~1 week"     },
+  { ...CHESS_CLOCK_PRESETS[3], label: "Very Slow", icon: Snail,  desc: "~2 weeks"    },
 ];
 
 export function isSlowPreset(s: string): boolean {

app/lib/__tests__/draft-timer.test.ts

@@ -3,8 +3,80 @@ import {
   formatClockTime,
   calculateTimeAfterPick,
   getTimerColorClass,
+  parseDraftSpeed,
+  getInitialDraftSpeed,
+  CHESS_CLOCK_PRESETS,
 } from "../draft-timer";
 
+// ─── parseDraftSpeed ─────────────────────────────────────────────────────────
+
+describe("parseDraftSpeed", () => {
+  describe("standard mode", () => {
+    it("returns the raw seconds for both fields", () => {
+      expect(parseDraftSpeed("120", "standard")).toEqual({ draftInitialTime: 120, draftIncrementTime: 120 });
+      expect(parseDraftSpeed("28800", "standard")).toEqual({ draftInitialTime: 28800, draftIncrementTime: 28800 });
+    });
+
+    it("falls back to 90s for null or unparseable input", () => {
+      expect(parseDraftSpeed(null, "standard")).toEqual({ draftInitialTime: 90, draftIncrementTime: 90 });
+      expect(parseDraftSpeed("abc", "standard")).toEqual({ draftInitialTime: 90, draftIncrementTime: 90 });
+    });
+  });
+
+  describe("chess_clock mode — named presets", () => {
+    it.each(CHESS_CLOCK_PRESETS)("maps $value to bankSec=$bankSec / incrSec=$incrSec", ({ value, bankSec, incrSec }) => {
+      expect(parseDraftSpeed(value, "chess_clock")).toEqual({
+        draftInitialTime: bankSec,
+        draftIncrementTime: incrSec,
+      });
+    });
+  });
+
+  describe("chess_clock mode — custom string", () => {
+    it("parses custom:{bank}:{incr} correctly", () => {
+      expect(parseDraftSpeed("custom:28800:1800", "chess_clock")).toEqual({
+        draftInitialTime: 28800,
+        draftIncrementTime: 1800,
+      });
+    });
+  });
+
+  describe("chess_clock mode — fallback", () => {
+    it("returns standard preset values for null or unknown input", () => {
+      expect(parseDraftSpeed(null, "chess_clock")).toEqual({ draftInitialTime: 120, draftIncrementTime: 15 });
+      expect(parseDraftSpeed("unknown", "chess_clock")).toEqual({ draftInitialTime: 120, draftIncrementTime: 15 });
+    });
+  });
+});
+
+// ─── getInitialDraftSpeed ─────────────────────────────────────────────────────
+
+describe("getInitialDraftSpeed", () => {
+  it("returns increment seconds as string for standard mode", () => {
+    expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: 120, draftIncrementTime: 120 })).toBe("120");
+    expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: 28800, draftIncrementTime: 28800 })).toBe("28800");
+  });
+
+  it("defaults to '90' for standard mode with missing increment", () => {
+    expect(getInitialDraftSpeed({ draftTimerMode: "standard", draftInitialTime: null, draftIncrementTime: null })).toBe("90");
+  });
+
+  it.each(CHESS_CLOCK_PRESETS)("recognises the $value preset from its DB values", ({ value, bankSec, incrSec }) => {
+    expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: bankSec, draftIncrementTime: incrSec })).toBe(value);
+  });
+
+  it("returns custom:{bank}:{incr} for non-preset chess clock values", () => {
+    expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: 28800, draftIncrementTime: 1800 }))
+      .toBe("custom:28800:1800");
+  });
+
+  it("defaults to 'standard' preset key when chess clock has no saved values", () => {
+    expect(getInitialDraftSpeed({ draftTimerMode: "chess_clock", draftInitialTime: null, draftIncrementTime: null })).toBe("standard");
+    expect(getInitialDraftSpeed(null)).toBe("standard");
+    expect(getInitialDraftSpeed(undefined)).toBe("standard");
+  });
+});
+
 // ─── formatClockTime ─────────────────────────────────────────────────────────
 
 describe("formatClockTime", () => {

app/lib/draft-timer.ts

@@ -51,6 +51,14 @@ export function calculateTimeAfterPick(
   return bankTime + incrementTime;
 }
 
+/** Pure preset data for chess-clock mode — the single source of truth shared with DraftSpeedPicker. */
+export const CHESS_CLOCK_PRESETS: { value: string; bankSec: number; incrSec: number }[] = [
+  { value: "fast",      bankSec: 60,    incrSec: 10   },
+  { value: "standard",  bankSec: 120,   incrSec: 15   },
+  { value: "slow",      bankSec: 28800, incrSec: 3600 },
+  { value: "very-slow", bankSec: 43200, incrSec: 3600 },
+];
+
 /**
  * Converts a draftSpeed form value + timerMode into DB time fields.
  *
@@ -73,16 +81,35 @@ export function parseDraftSpeed(
     const incr = parseInt(incrStr ?? "", 10);
     if (!isNaN(bank) && !isNaN(incr)) return { draftInitialTime: bank, draftIncrementTime: incr };
   }
-  switch (draftSpeed) {
-    case "fast":
-      return { draftInitialTime: 60, draftIncrementTime: 10 };
-    case "slow":
-      return { draftInitialTime: 28800, draftIncrementTime: 3600 };
-    case "very-slow":
-      return { draftInitialTime: 43200, draftIncrementTime: 3600 };
-    default:
-      return { draftInitialTime: 120, draftIncrementTime: 15 };
+
+  const preset = CHESS_CLOCK_PRESETS.find((p) => p.value === draftSpeed);
+  if (preset) return { draftInitialTime: preset.bankSec, draftIncrementTime: preset.incrSec };
+
+  // Unknown/null speed — fall back to "standard" preset
+  return { draftInitialTime: 120, draftIncrementTime: 15 };
+}
+
+type DraftSpeedSource = {
+  draftTimerMode?: "chess_clock" | "standard" | null;
+  draftInitialTime?: number | null;
+  draftIncrementTime?: number | null;
+} | null | undefined;
+
+/**
+ * Derives the draftSpeed UI value (preset key or "custom:{bank}:{incr}") from
+ * saved DB fields. Used to initialise and reset the DraftSpeedPicker.
+ */
+export function getInitialDraftSpeed(season: DraftSpeedSource): string {
+  const mode = season?.draftTimerMode ?? "chess_clock";
+  if (mode === "standard") return season?.draftIncrementTime?.toString() ?? "90";
+  const preset = CHESS_CLOCK_PRESETS.find(
+    (p) => p.bankSec === season?.draftInitialTime && p.incrSec === season?.draftIncrementTime
+  );
+  if (preset) return preset.value;
+  if (season?.draftInitialTime != null && season?.draftIncrementTime != null) {
+    return `custom:${season.draftInitialTime}:${season.draftIncrementTime}`;
   }
+  return "standard";
 }
 
 /**

app/routes/leagues/$leagueId.settings.tsx

@@ -12,6 +12,7 @@ import {
   Users,
 } from "lucide-react";
 import { type ScoringRules, DEFAULT_SCORING_RULES } from "~/lib/scoring-types";
+import { getInitialDraftSpeed } from "~/lib/draft-timer";
 import type { Route } from "./+types/$leagueId.settings";
 export { action, loader } from "./$leagueId.settings.server";
 import { OMNIFANTASY_SCORING } from "~/components/league/ScoringPresetPicker";
@@ -48,19 +49,6 @@ type LoaderSportsSeason = Route.ComponentProps["loaderData"]["allSportsSeasons"]
   fantasySeasonId?: string | null;
 };
 
-function getInitialDraftSpeed(season: LoaderSeason): string {
-  const mode = season?.draftTimerMode ?? "chess_clock";
-  if (mode === "standard") return season?.draftIncrementTime?.toString() ?? "90";
-  if (season?.draftInitialTime === 60 && season?.draftIncrementTime === 10) return "fast";
-  if (season?.draftInitialTime === 120 && season?.draftIncrementTime === 15) return "standard";
-  if (season?.draftInitialTime === 28800 && season?.draftIncrementTime === 3600) return "slow";
-  if (season?.draftInitialTime === 43200 && season?.draftIncrementTime === 3600) return "very-slow";
-  if (season?.draftInitialTime != null && season?.draftIncrementTime != null) {
-    return `custom:${season.draftInitialTime}:${season.draftIncrementTime}`;
-  }
-  return "standard";
-}
-
 function getInitialScoringPreset(season: LoaderSeason) {
   const rules: ScoringRules = {
     pointsFor1st: season?.pointsFor1st ?? DEFAULT_SCORING_RULES.pointsFor1st,
@@ -189,10 +177,11 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone
     setHasDraftOrderChanges(false);
   }, [draftSlots, teams]);
 
-  // Re-mark dirty if the save returned an error (success path redirects away, so no cleanup needed there)
+  // Re-mark dirty if the settings save returned an error (success path redirects away, so no cleanup
+  // needed there). Guard against non-settings errors (e.g. draft-order) which carry a `section` field.
   useEffect(() => {
     if (navigation.state !== "idle") return;
-    if (actionData && "error" in actionData) {
+    if (actionData && "error" in actionData && !("section" in actionData)) {
       setHasUnsavedSettingsChanges(true);
     }
   }, [actionData, navigation.state]);