fix: v0.12.1 — ISS-001 time-scale slider no longer snaps back to the stale value on speed change (optimistic scale kept until a snapshot confirms it); sim-control commit machinery hardened (synchronous busy ref, in-flight queue flush loop, sent/queued/failed outcomes so queued commits never drop the display)

2026-09-18 12:10:40 +00:00
parent ab2e9fa54a
commit 7c51cadef7
5 changed files with 132 additions and 25 deletions
@@ -34,44 +34,65 @@ function loadoutFor(state: StateSnapshot, p: PlaneState): string {
  return `${used}/${capacity} pkgs`;
 }

+// ISS-001 — an optimistic slider value is cleared exactly when a rendered snapshot CONFIRMS it
+// (server scale === pending), never earlier: clearing at commit time would snap the pointer back
+// to the stale scale for one poll cycle, then jump forward again on the confirming snapshot.
+export function pendingScaleConfirmed(pendingScale: number | null, serverTimeScale: number): boolean {
+  return pendingScale !== null && serverTimeScale === pendingScale;
+}
+
 export default function LeftPane({ state, loading, error, onRefresh }: LeftPaneProps) {
  const [busy, setBusy] = useState(false);

  // POST /api/sim, then refetch the full snapshot immediately so the pane reflects the new
  // controls without waiting for the next poll tick. On failure keep the stale UI — the
  // running poll retries on its own and surfaces the error state. A commit that lands while
-  // another POST is in flight is held in a single slot and flushed right after, so the last
-  // user intent always reaches the server (the server's [1,64] clamp stays as backstop).
+  // another POST is in flight is held in a single slot (last intent wins) and flushed by the
+  // in-flight call, so every user intent reaches the server (the [1,64] clamp stays as backstop).
+  // The gate is a REF, not React state: closures never see a stale "busy" value, which used to
+  // let a flush re-queue instead of send and silently drop a commit.
  const queuedRef = useRef<{ timeScale?: number; paused?: boolean } | null>(null);
+  const busyRef = useRef(false);

+  type SimControlsOutcome = 'sent' | 'queued' | 'failed';
+
+  // 'sent': this body reached the server (a confirming snapshot follows via onRefresh).
+  // 'queued': an in-flight call holds it — its flush will send it.
+  // 'failed': the POST for this body never succeeded; callers drop optimistic values only then.
  const applySimControls = useCallback(
-    async (body: { timeScale?: number; paused?: boolean }) => {
-      if (busy) {
-        queuedRef.current = body;
-        return;
+    async (body: { timeScale?: number; paused?: boolean }): Promise<SimControlsOutcome> => {
+      if (busyRef.current) {
+        queuedRef.current = body; // last intent wins — the in-flight call's flush sends it
+        return 'queued';
      }
+      busyRef.current = true;
      setBusy(true);
+      let sentOwn = false;
      try {
-        const res = await fetch('/api/sim', {
-          method: 'POST',
-          headers: { 'Content-Type': 'application/json' },
-          body: JSON.stringify(body),
-          cache: 'no-store',
-        });
-        if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`);
-        onRefresh();
+        let current: { timeScale?: number; paused?: boolean } | null = body;
+        while (current !== null) {
+          const res = await fetch('/api/sim', {
+            method: 'POST',
+            headers: { 'Content-Type': 'application/json' },
+            body: JSON.stringify(current),
+            cache: 'no-store',
+          });
+          if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`);
+          onRefresh();
+          if (current === body) sentOwn = true; // object identity — queued bodies are fresh literals
+          current = queuedRef.current; // pick up anything that arrived while we were in flight
+          queuedRef.current = null;
+        }
      } catch (err) {
        console.error('failed to apply sim controls', err);
+        queuedRef.current = null; // don't replay a stale intent after a failed round-trip
      } finally {
+        busyRef.current = false;
        setBusy(false);
-        const queued = queuedRef.current;
-        if (queued !== null) {
-          queuedRef.current = null;
-          void applySimControls(queued);
-        }
      }
+      return sentOwn ? 'sent' : 'failed';
    },
-    [busy, onRefresh],
+    [onRefresh],
  );

  const paused = state?.simState.paused ?? false;
@@ -97,10 +118,24 @@ export default function LeftPane({ state, loading, error, onRefresh }: LeftPaneP
    }
    const s = pendingRef.current;
    if (s === null) return;
-    setPending(null);
-    void applySimControls({ timeScale: s });
+    // ISS-001: keep the chosen value on screen until a snapshot CONFIRMS it — clearing at
+    // commit time would snap the slider back to the stale scale for one poll cycle, then jump
+    // forward again when the confirming snapshot lands. 'queued' also keeps it (the in-flight
+    // call flushes it); only a genuine failure drops it so the display falls back to truth.
+    void applySimControls({ timeScale: s }).then((outcome) => {
+      if (outcome === 'failed') setPending(null);
+    });
  }, [applySimControls, setPending]);

+  // Confirmation: once a rendered snapshot carries the pending scale AND no commit is in
+  // flight, the optimistic value has been superseded by server truth — clear it (display is
+  // unchanged either way). The busy guard matters: while a commit POST is still out, an OLD
+  // snapshot that happens to match the pending value is coincidence, not confirmation (e.g. a
+  // quick drag back to the previous scale must not be swallowed before its own commit fires).
+  useEffect(() => {
+    if (!busy && state && pendingScaleConfirmed(pendingScale, state.simState.timeScale)) setPending(null);
+  }, [state, pendingScale, busy, setPending]);
+
  const onSliderInput = useCallback(
    (e: ChangeEvent<HTMLInputElement>) => {
      setPending(sliderToScale(Number(e.target.value)));
@@ -0,0 +1,24 @@
+import { describe, expect, it } from 'vitest';
+import { pendingScaleConfirmed } from './LeftPane';
+
+// ISS-001 — the optimistic slider value is cleared exactly when a rendered snapshot confirms
+// it (server scale === pending), never earlier. Clearing at commit time would snap the pointer
+// back to the stale scale for one poll cycle, then jump forward again on the confirming snapshot.
+
+describe('pendingScaleConfirmed', () => {
+  it('is false while there is no pending value', () => {
+    expect(pendingScaleConfirmed(null, 1)).toBe(false);
+    expect(pendingScaleConfirmed(null, 64)).toBe(false);
+  });
+
+  it('stays false while the server still reports the stale scale (the ISS-001 window)', () => {
+    // User committed 64×; the last rendered snapshot is still at 1× → must NOT clear yet.
+    expect(pendingScaleConfirmed(64, 1)).toBe(false);
+    expect(pendingScaleConfirmed(32, 8)).toBe(false);
+  });
+
+  it('becomes true exactly when a snapshot carries the pending scale', () => {
+    expect(pendingScaleConfirmed(64, 64)).toBe(true);
+    expect(pendingScaleConfirmed(1, 1)).toBe(true); // committing the current value confirms immediately
+  });
+});