(via jazzstack): Fix DPI editor race: picking a preset cancels the pending debounced slider commit

src/features/devices/cards/DpiCard.tsx

@@ -9,6 +9,7 @@ import * as control from "../../../device/controller";
 import type { ControlSnapshot, LiftOffLevel } from "../../../device/types";
 import { isNoirE1Status, isNoirKsnakeStatus, isNoirS1Status, NOIR_DPI_STAGE_COLORS } from "../../../device/noir.ts";
 import { closestDpiOption, dpiPresetValues } from "../../../device/dpi-presets";
+import { createDpiEditDebouncer } from "./dpi-edit-debounce";
 import { t, tp } from "../../../i18n/index";
 import { LiftOffDistance, hasLiftOff } from "./PerformanceCards";
 import { IconLinked, IconUnlinked } from "../../../shared/icons";
@@ -110,13 +111,13 @@ function DpiStageEditor({
   const rowsRef = useRef(rows);
   rowsRef.current = rows;
   const touchedRef = useRef(0);
-  const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const [edits] = useState(() => createDpiEditDebouncer());
 
   useEffect(() => {
     return () => {
-      if (timerRef.current) clearTimeout(timerRef.current);
+      edits.cancel();
     };
-  }, []);
+  }, [edits]);
 
   const fixedStageCount = mode === "stage" && stageEditor?.countEditable !== true;
   const fillsStageTable = singleFillsStageTable(snapshot);
@@ -160,7 +161,7 @@ function DpiStageEditor({
   useEffect(() => {
     if (sourceSig === lastSigRef.current) return;
     lastSigRef.current = sourceSig;
-    if (Date.now() - touchedRef.current > 1500 || (mode === "logitech" && !timerRef.current)) setRows(initRows(rowsRef.current));
+    if (Date.now() - touchedRef.current > 1500 || (mode === "logitech" && !edits.pending())) setRows(initRows(rowsRef.current));
     // initRows is recreated each render on purpose so it always reads fresh snapshot data.
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [sourceSig]);
@@ -242,17 +243,11 @@ function DpiStageEditor({
   const markEdited = (delay = 450, nextRows?: StageRow[]): void => {
     touchedRef.current = Date.now();
     const snapshotRows = nextRows ?? rowsRef.current;
-    if (timerRef.current) clearTimeout(timerRef.current);
-    timerRef.current = setTimeout(() => {
-      timerRef.current = null;
-      commit(snapshotRows);
-    }, delay);
+    edits.schedule(() => commit(snapshotRows), delay);
   };
 
   const flushEdit = (): void => {
-    if (!timerRef.current) return;
-    clearTimeout(timerRef.current);
-    timerRef.current = null;
+    if (!edits.cancel()) return;
     commit(rowsRef.current);
   };
 
@@ -262,6 +257,10 @@ function DpiStageEditor({
       if (!enabled || next[index].enabled === rows[index].enabled) return;
       const snap = closestDpiOption(options, parseRow(next[index].value) ?? status?.dpi ?? 0);
       if (snap === null) return;
+      // A slider or typed edit may still have a debounced commit pending with
+      // the rows it captured. The pick is newer, so drop that commit or it
+      // would re-apply the old row's DPI over this one when it fires.
+      edits.cancel();
       const selected = next.map((row, i) => (i === index ? { ...row, enabled: true } : { ...row, enabled: false }));
       setRows(selected);
       rowsRef.current = selected;

src/features/devices/cards/dpi-edit-debounce.test.ts

@@ -0,0 +1,91 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+
+import { createDpiEditDebouncer, type EditTimers } from "./dpi-edit-debounce.ts";
+
+function fakeTimers(): EditTimers & { advance(ms: number): void; active(): number } {
+  let now = 0;
+  let nextId = 1;
+  const queue = new Map<number, { at: number; callback: () => void }>();
+  return {
+    setTimeout(callback, delay) {
+      const id = nextId++;
+      queue.set(id, { at: now + delay, callback });
+      return id;
+    },
+    clearTimeout(handle) {
+      queue.delete(handle as number);
+    },
+    advance(ms) {
+      now += ms;
+      for (const [id, entry] of [...queue].sort((a, b) => a[1].at - b[1].at)) {
+        if (entry.at > now || !queue.has(id)) continue;
+        queue.delete(id);
+        entry.callback();
+      }
+    },
+    active: () => queue.size,
+  };
+}
+
+test("a scheduled commit runs once after its delay", () => {
+  const timers = fakeTimers();
+  const edits = createDpiEditDebouncer(timers);
+  const runs: string[] = [];
+  edits.schedule(() => runs.push("a"), 450);
+  assert.equal(edits.pending(), true);
+  timers.advance(449);
+  assert.deepEqual(runs, []);
+  timers.advance(1);
+  assert.deepEqual(runs, ["a"]);
+  assert.equal(edits.pending(), false);
+  timers.advance(1000);
+  assert.deepEqual(runs, ["a"]);
+});
+
+test("a newer schedule replaces the pending commit", () => {
+  const timers = fakeTimers();
+  const edits = createDpiEditDebouncer(timers);
+  const runs: string[] = [];
+  edits.schedule(() => runs.push("old"), 450);
+  timers.advance(200);
+  edits.schedule(() => runs.push("new"), 450);
+  assert.equal(timers.active(), 1);
+  timers.advance(1000);
+  assert.deepEqual(runs, ["new"]);
+});
+
+test("cancel drops the pending commit and reports whether one was pending", () => {
+  const timers = fakeTimers();
+  const edits = createDpiEditDebouncer(timers);
+  const runs: string[] = [];
+  assert.equal(edits.cancel(), false);
+  edits.schedule(() => runs.push("a"), 450);
+  assert.equal(edits.cancel(), true);
+  assert.equal(edits.pending(), false);
+  assert.equal(edits.cancel(), false);
+  timers.advance(1000);
+  assert.deepEqual(runs, []);
+});
+
+test("a preset picked after a slider edit is not overwritten by the slider's stale rows", () => {
+  const timers = fakeTimers();
+  const edits = createDpiEditDebouncer(timers);
+  const applied: number[] = [];
+
+  // Slider on the active row moves to 1600; the commit captures those rows.
+  const snapshotRows = [{ enabled: true, value: "1600" }, { enabled: false, value: "3200" }];
+  edits.schedule(() => {
+    const active = snapshotRows.find((row) => row.enabled);
+    if (active) applied.push(Number(active.value));
+  }, 450);
+
+  // The user picks the 3200 preset before the debounce fires, as the single
+  // DPI view does: drop the pending commit, then apply the pick right away.
+  timers.advance(100);
+  edits.cancel();
+  applied.push(3200);
+
+  timers.advance(1000);
+  assert.deepEqual(applied, [3200]);
+});

src/features/devices/cards/dpi-edit-debounce.ts

@@ -0,0 +1,49 @@
+export interface EditTimers {
+  setTimeout(callback: () => void, delay: number): unknown;
+  clearTimeout(handle: unknown): void;
+}
+
+const globalTimers: EditTimers = {
+  setTimeout: (callback, delay) => setTimeout(callback, delay),
+  clearTimeout: (handle) => clearTimeout(handle as ReturnType<typeof setTimeout>),
+};
+
+/**
+ * Holds at most one pending DPI editor commit. A newer schedule replaces the
+ * pending one, and anything applied immediately (a preset pick) must cancel it
+ * first so the older rows captured by the timer cannot land on top of it.
+ */
+export interface DpiEditDebouncer {
+  /** Run `run` after `delay` ms, dropping any commit still pending. */
+  schedule(run: () => void, delay: number): void;
+  /** Drop the pending commit without running it. Returns whether one was pending. */
+  cancel(): boolean;
+  pending(): boolean;
+}
+
+export function createDpiEditDebouncer(timers: EditTimers = globalTimers): DpiEditDebouncer {
+  let handle: unknown = null;
+  let scheduled = false;
+
+  const cancel = (): boolean => {
+    if (!scheduled) return false;
+    timers.clearTimeout(handle);
+    handle = null;
+    scheduled = false;
+    return true;
+  };
+
+  return {
+    schedule(run, delay) {
+      cancel();
+      scheduled = true;
+      handle = timers.setTimeout(() => {
+        handle = null;
+        scheduled = false;
+        run();
+      }, delay);
+    },
+    cancel,
+    pending: () => scheduled,
+  };
+}