fix(studio): version-scope Grade persist, flush pending edits via effect cleanup not render

Fixes three of the adversarial findings from the third #2416 tip re-review: - Grade rollback was identity-scoped but not attempt-scoped: two edits on the SAME element (e.g. drag Exposure, then Contrast, before Exposure's persist settles) could have the earlier edit's late completion stamp confirmedGradingRef with its now-superseded value, or revert `grading` out from under the newer optimistic edit. Added a monotonic per-commit version via the existing bumpDomEditCommitVersion primitive (the same one the DOM-attribute commit runner uses for the identical race) — persistColorGradingValue now checks both identity AND "is this still the latest attempt for this element" before applying any effect. - The render-phase identity-reset block consumed shared mutable state (clearing the pending-persist timer, reading and nulling pendingPersistValueRef) directly during render. Adjusting STATE during render this way is React's documented pattern and safe to repeat, but consuming a ref this way is not: if React discarded/interrupted that specific render before it committed, the timer would already be cancelled and the pending value already nulled, with no corresponding effect ever running to compensate, silently losing the edit. Replaced with the idiomatic pattern for "clean up a per-identity resource when it changes" — a useEffect keyed on identityKey whose CLEANUP performs the cancellation/flush. A cleanup only ever runs for the effect instance that actually committed, closing the gap entirely. The render-phase block now only performs pure, idempotent state resets. - FlatSelectRow's Preset row passes label="" (the visible "Preset" text is a sibling span, to avoid rendering it twice) which left the underlying <select> with no accessible name at all. Added a dedicated `ariaLabel` prop, distinct from the visible `label`, so a caller can supply a name without a duplicate visible label. Also hardened FlatSlider's lostpointercapture handling: it now resyncs the draft directly from a latestValueRef immediately, instead of only clearing the dragging flag and waiting for the separate [value]-keyed effect to notice — closing a narrow ordering gap where a value change arriving while still dragging, followed by capture loss with no further render, could otherwise leave the knob stuck. propertyPanelFlatPrimitives.tsx crossed the 600-line file-size gate after these changes; extracted FlatToggle (and its tests) into their own files, matching the FlatMaskInsetRows precedent from an earlier commit in this stack. New/updated regression tests: same-element version race, Preset select's aria-label. Full studio suite still at the known pre-existing 55-failure baseline, zero regressions.
2026-09-03 12:54:29 +00:00 · 2026-07-14 16:28:33 -07:00
parent 539e027b60
commit e07f1405f6
9 changed files with 318 additions and 146 deletions
@@ -238,6 +238,10 @@ describe("FlatColorGradingSection — Preset + LUT", () => {
    );
    if (!presetSelect) throw new Error("expected a preset select");
    expect(presetSelect.value).toBe("neutral");
+    // The visible "Preset" label is a sibling span outside FlatSelectRow
+    // (label="" there, to avoid rendering it twice) — the select still
+    // needs its own accessible name via the dedicated ariaLabel prop.
+    expect(presetSelect.getAttribute("aria-label")).toBe("Preset");
    act(() => {
      presetSelect.value = "fresh-pop";
      presetSelect.dispatchEvent(new Event("change", { bubbles: true }));
@@ -312,6 +312,7 @@ export function FlatColorGradingSection({
        <span className="text-[11px] text-panel-text-2">Preset</span>
        <FlatSelectRow
          label=""
+          ariaLabel="Preset"
          value={grading.preset ?? "neutral"}
          options={PRESET_OPTIONS}
          tier={resolveValueTier(
@@ -9,7 +9,8 @@ import {
  parseNumericValue,
  stripQueryAndHash,
 } from "./propertyPanelHelpers";
-import { FlatSelectRow, FlatSlider, FlatToggle } from "./propertyPanelFlatPrimitives";
+import { FlatSelectRow, FlatSlider } from "./propertyPanelFlatPrimitives";
+import { FlatToggle } from "./propertyPanelFlatToggle";

 // fallow-ignore-next-line complexity
 export function FlatMediaSection({
@@ -9,7 +9,6 @@ import {
  FlatSegmentedRow,
  FlatSelectRow,
  FlatSlider,
-  FlatToggle,
 } from "./propertyPanelFlatPrimitives";

 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
@@ -822,6 +821,55 @@ describe("FlatSlider — Grade extensions", () => {
    expect(track.getAttribute("aria-valuenow")).toBe("99");
    act(() => root.unmount());
  });
+
+  it("resyncs immediately from the latest value on lostpointercapture, even when the value changed WHILE still dragging", () => {
+    const { host, root } = renderInto(
+      <FlatSlider
+        label="Opacity"
+        value={10}
+        min={0}
+        max={100}
+        tier="explicitCustom"
+        displayValue="10%"
+        onCommit={vi.fn()}
+      />,
+    );
+    const track = host.querySelector<HTMLElement>('[data-flat-slider-track="true"]');
+    if (!track) throw new Error("expected a track element");
+    Object.defineProperty(track, "getBoundingClientRect", {
+      value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }),
+    });
+    act(() => {
+      track.dispatchEvent(
+        new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }),
+      );
+    });
+    expect(track.getAttribute("aria-valuenow")).toBe("30");
+    // Value changes to 99 WHILE still dragging — the [value] sync effect
+    // must skip it (draggingRef is still true), so draft stays at 30.
+    act(() => {
+      root.render(
+        <FlatSlider
+          label="Opacity"
+          value={99}
+          min={0}
+          max={100}
+          tier="explicitCustom"
+          displayValue="99%"
+          onCommit={vi.fn()}
+        />,
+      );
+    });
+    expect(track.getAttribute("aria-valuenow")).toBe("30");
+    act(() => {
+      // Capture lost with NO further render afterward — if the resync
+      // depended on a subsequent [value] effect run rather than reading
+      // latestValueRef directly, this would leave the knob stuck at 30.
+      track.dispatchEvent(new Event("lostpointercapture", { bubbles: true }));
+    });
+    expect(track.getAttribute("aria-valuenow")).toBe("99");
+    act(() => root.unmount());
+  });
 });

 describe("FlatSelectRow", () => {
@@ -967,44 +1015,3 @@ describe("FlatSelectRow — label/value options", () => {
    act(() => root.unmount());
  });
 });
-
-describe("FlatToggle", () => {
-  it("renders the off state with a dim label and dim knob, and fires onChange(true) on click", () => {
-    const onChange = vi.fn();
-    const { host, root } = renderInto(
-      <FlatToggle label="Loop" checked={false} onChange={onChange} />,
-    );
-    const label = host.querySelector('[data-flat-toggle-label="true"]');
-    expect(label?.className).toContain("text-panel-text-3");
-    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
-    expect(pill).not.toBeNull();
-    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
-    expect(onChange).toHaveBeenCalledWith(true);
-    act(() => root.unmount());
-  });
-
-  it("renders the on state with an emphasized label and mint knob, and fires onChange(false) on click", () => {
-    const onChange = vi.fn();
-    const { host, root } = renderInto(<FlatToggle label="Loop" checked onChange={onChange} />);
-    const label = host.querySelector('[data-flat-toggle-label="true"]');
-    expect(label?.className).toContain("text-panel-text-2");
-    const knob = host.querySelector('[data-flat-toggle-knob="true"]');
-    expect(knob?.className).toContain("bg-panel-accent");
-    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
-    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
-    expect(onChange).toHaveBeenCalledWith(false);
-    act(() => root.unmount());
-  });
-
-  it("does not fire onChange when disabled", () => {
-    const onChange = vi.fn();
-    const { host, root } = renderInto(
-      <FlatToggle label="Loop" checked={false} disabled onChange={onChange} />,
-    );
-    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
-    expect(pill?.disabled).toBe(true);
-    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
-    expect(onChange).not.toHaveBeenCalled();
-    act(() => root.unmount());
-  });
-});
@@ -300,6 +300,11 @@ export function FlatSlider({
  // different control in between.
  const onCommitRef = useRef(onCommit);
  onCommitRef.current = onCommit;
+  // Always this render's committed value — read directly (not via the
+  // effect below) by onLostPointerCapture, so the resync there doesn't
+  // depend on ordering between the native event and the [value] effect.
+  const latestValueRef = useRef(value);
+  latestValueRef.current = value;

  useEffect(() => {
    if (draggingRef.current) return;
@@ -407,10 +412,16 @@ export function FlatSlider({
        onLostPointerCapture={() => {
          // Capture can be lost without either pointerup or pointercancel
          // firing first (e.g. another element steals it, or the browser
-          // reclaims it for a scroll/touch gesture) — without this,
-          // draggingRef stays stuck true and the knob permanently stops
-          // syncing to the committed value prop.
+          // reclaims it for a scroll/touch gesture). Resync immediately and
+          // directly from latestValueRef, rather than only clearing
+          // draggingRef and waiting for the [value] effect to notice —
+          // that effect depends on `value` actually changing again to
+          // re-run, so if this event and any concurrent value update are
+          // ordered unfavorably, the knob could otherwise stay stuck at
+          // its mid-drag position indefinitely.
          draggingRef.current = false;
+          setDraft(latestValueRef.current);
+          lastCommittedRef.current = latestValueRef.current;
        }}
        onKeyDown={(e) => {
          if (disabled) return;
@@ -478,6 +489,7 @@ export function FlatSlider({

 export function FlatSelectRow({
  label,
+  ariaLabel,
  value,
  options,
  tier,
@@ -486,6 +498,11 @@ export function FlatSelectRow({
  onReset,
 }: {
  label: string;
+  /** Accessible name when a caller renders the visible label OUTSIDE this
+   *  row (label="" to avoid a duplicate) — e.g. Grade's "Preset" row, which
+   *  shows its own label span and would otherwise leave the <select>
+   *  unnamed. Falls back to `label` when omitted. */
+  ariaLabel?: string;
  value: string;
  options: Array<string | { value: string; label: string }>;
  tier: PropertyValueTier;
@@ -515,7 +532,7 @@ export function FlatSelectRow({
          <select
            value={value}
            disabled={disabled}
-            aria-label={label || undefined}
+            aria-label={ariaLabel || label || undefined}
            onChange={(e) => onChange(e.target.value)}
            className={`appearance-none bg-transparent text-right font-mono text-[11px] outline-none disabled:cursor-not-allowed ${VALUE_TIER_VALUE_CLASS[tier]}`}
          >
@@ -551,49 +568,3 @@ export function FlatSelectRow({
    </div>
  );
 }
-
-/* ------------------------------------------------------------------ */
-/*  FlatToggle — 24×14 pill switch                                     */
-/* ------------------------------------------------------------------ */
-
-export function FlatToggle({
-  label,
-  checked,
-  disabled,
-  onChange,
-}: {
-  label: string;
-  checked: boolean;
-  disabled?: boolean;
-  onChange: (next: boolean) => void;
-}) {
-  return (
-    <div className="flex min-h-[30px] items-center justify-between">
-      <span
-        data-flat-toggle-label="true"
-        className={`text-[11px] ${checked ? "text-panel-text-2" : "text-panel-text-3"}`}
-      >
-        {label}
-      </span>
-      <button
-        type="button"
-        data-flat-toggle="true"
-        role="switch"
-        aria-checked={checked}
-        aria-label={label}
-        disabled={disabled}
-        onClick={() => onChange(!checked)}
-        className={`relative h-[14px] w-6 flex-shrink-0 rounded-full transition-colors disabled:cursor-not-allowed disabled:opacity-50 ${
-          checked ? "bg-panel-accent/35" : "bg-panel-hover"
-        }`}
-      >
-        <span
-          data-flat-toggle-knob="true"
-          className={`absolute top-0.5 h-2.5 w-2.5 rounded-full transition-all ${
-            checked ? "right-0.5 bg-panel-accent" : "left-0.5 bg-panel-text-4"
-          }`}
-        />
-      </button>
-    </div>
-  );
-}
@@ -0,0 +1,63 @@
+// @vitest-environment happy-dom
+
+import React, { act } from "react";
+import { createRoot } from "react-dom/client";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { FlatToggle } from "./propertyPanelFlatToggle";
+
+(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+afterEach(() => {
+  document.body.innerHTML = "";
+});
+
+function renderInto(node: React.ReactElement) {
+  const host = document.createElement("div");
+  document.body.append(host);
+  const root = createRoot(host);
+  act(() => {
+    root.render(node);
+  });
+  return { host, root };
+}
+
+describe("FlatToggle", () => {
+  it("renders the off state with a dim label and dim knob, and fires onChange(true) on click", () => {
+    const onChange = vi.fn();
+    const { host, root } = renderInto(
+      <FlatToggle label="Loop" checked={false} onChange={onChange} />,
+    );
+    const label = host.querySelector('[data-flat-toggle-label="true"]');
+    expect(label?.className).toContain("text-panel-text-3");
+    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
+    expect(pill).not.toBeNull();
+    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
+    expect(onChange).toHaveBeenCalledWith(true);
+    act(() => root.unmount());
+  });
+
+  it("renders the on state with an emphasized label and mint knob, and fires onChange(false) on click", () => {
+    const onChange = vi.fn();
+    const { host, root } = renderInto(<FlatToggle label="Loop" checked onChange={onChange} />);
+    const label = host.querySelector('[data-flat-toggle-label="true"]');
+    expect(label?.className).toContain("text-panel-text-2");
+    const knob = host.querySelector('[data-flat-toggle-knob="true"]');
+    expect(knob?.className).toContain("bg-panel-accent");
+    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
+    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
+    expect(onChange).toHaveBeenCalledWith(false);
+    act(() => root.unmount());
+  });
+
+  it("does not fire onChange when disabled", () => {
+    const onChange = vi.fn();
+    const { host, root } = renderInto(
+      <FlatToggle label="Loop" checked={false} disabled onChange={onChange} />,
+    );
+    const pill = host.querySelector<HTMLButtonElement>('[data-flat-toggle="true"]');
+    expect(pill?.disabled).toBe(true);
+    act(() => pill?.dispatchEvent(new MouseEvent("click", { bubbles: true })));
+    expect(onChange).not.toHaveBeenCalled();
+    act(() => root.unmount());
+  });
+});
@@ -0,0 +1,47 @@
+/* ------------------------------------------------------------------ */
+/*  FlatToggle — 24×14 pill switch                                     */
+/*  (split out of propertyPanelFlatPrimitives.tsx to stay under the    */
+/*  600-line file-size gate)                                           */
+/* ------------------------------------------------------------------ */
+
+export function FlatToggle({
+  label,
+  checked,
+  disabled,
+  onChange,
+}: {
+  label: string;
+  checked: boolean;
+  disabled?: boolean;
+  onChange: (next: boolean) => void;
+}) {
+  return (
+    <div className="flex min-h-[30px] items-center justify-between">
+      <span
+        data-flat-toggle-label="true"
+        className={`text-[11px] ${checked ? "text-panel-text-2" : "text-panel-text-3"}`}
+      >
+        {label}
+      </span>
+      <button
+        type="button"
+        data-flat-toggle="true"
+        role="switch"
+        aria-checked={checked}
+        aria-label={label}
+        disabled={disabled}
+        onClick={() => onChange(!checked)}
+        className={`relative h-[14px] w-6 flex-shrink-0 rounded-full transition-colors disabled:cursor-not-allowed disabled:opacity-50 ${
+          checked ? "bg-panel-accent/35" : "bg-panel-hover"
+        }`}
+      >
+        <span
+          data-flat-toggle-knob="true"
+          className={`absolute top-0.5 h-2.5 w-2.5 rounded-full transition-all ${
+            checked ? "right-0.5 bg-panel-accent" : "left-0.5 bg-panel-text-4"
+          }`}
+        />
+      </button>
+    </div>
+  );
+}
@@ -13,6 +13,12 @@ function freshPopGrading() {
  return next;
 }

+function naturalLiftGrading() {
+  const next = normalizeHfColorGrading({ preset: "natural-lift", intensity: 1 });
+  if (!next) throw new Error("expected natural-lift preset to normalize");
+  return next;
+}
+
 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

 afterEach(() => {
@@ -231,6 +237,66 @@ describe("useColorGradingController", () => {
    vi.useRealTimers();
  });

+  it("a stale in-flight persist for edit A does not clobber edit B's state — SAME element, no selection change", async () => {
+    vi.useFakeTimers();
+    let resolveA: (() => void) | undefined;
+    let capturedOnSettledA: ((ok: boolean) => void) | undefined;
+    const onSetAttributeLive = vi
+      .fn()
+      // Edit A (fresh-pop): captures its onSettled and never resolves until
+      // resolveA() is called below — simulates a slow persist.
+      .mockImplementationOnce(
+        (_attr: string, _value: string | null, onSettled?: (ok: boolean) => void) => {
+          capturedOnSettledA = onSettled;
+          return new Promise<void>((resolve) => {
+            resolveA = resolve;
+          });
+        },
+      )
+      // Edit B (natural-lift): settles immediately and successfully.
+      .mockImplementationOnce(
+        (_attr: string, _value: string | null, onSettled?: (ok: boolean) => void) => {
+          onSettled?.(true);
+          return Promise.resolve();
+        },
+      );
+    const { root, getState } = renderHook(onSetAttributeLive);
+
+    act(() => {
+      getState().commitColorGrading(freshPopGrading());
+    });
+    act(() => {
+      vi.advanceTimersByTime(400);
+    });
+    expect(onSetAttributeLive).toHaveBeenCalledTimes(1); // A's persist now in flight
+
+    // B commits on the SAME element before A's persist has settled.
+    act(() => {
+      getState().commitColorGrading(naturalLiftGrading());
+    });
+    act(() => {
+      vi.advanceTimersByTime(400);
+    });
+    expect(onSetAttributeLive).toHaveBeenCalledTimes(2); // B's persist has already settled (mock resolves sync)
+    expect(getState().grading.preset).toBe("natural-lift");
+
+    // NOW A's stale persist finally settles as a FAILURE — must not revert
+    // `grading` (which now correctly shows B's newer edit) back to the
+    // pre-A baseline ("neutral"), and must not stamp confirmedGradingRef
+    // with A's now-superseded attempt on success either.
+    act(() => {
+      capturedOnSettledA?.(false);
+      resolveA?.();
+    });
+    await act(async () => {
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+    expect(getState().grading.preset).toBe("natural-lift");
+    act(() => root.unmount());
+    vi.useRealTimers();
+  });
+
  it("resetGrading returns to the neutral preset", () => {
    const { root, getState } = renderHook(vi.fn());
    act(() => {
@@ -13,6 +13,7 @@ import {
 } from "../../utils/studioPendingEdits";
 import type { DomEditSelection } from "./domEditing";
 import { selectionIdentityKey, stripQueryAndHash } from "./propertyPanelHelpers";
+import { bumpDomEditCommitVersion } from "../../hooks/domEditCommitRunner";
 import {
  acceptStudioRuntimeMessage,
  postRuntimeControlMessage,
@@ -194,65 +195,45 @@ export function useColorGradingController({
  const persistTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
  const pendingPersistValueRef = useRef<string | null | undefined>(undefined);
  const pendingPersistGradingRef = useRef<NormalizedHfColorGrading | null>(null);
-  // Populated (pure ref write only) during the render-phase identity-change
-  // reset below; the actual write happens in an effect, never during render.
-  const queuedOutgoingFlushRef = useRef<{
-    setAttributeLive: typeof onSetAttributeLive;
-    value: string | null;
-  } | null>(null);
  // The last grading value actually confirmed saved — distinct from `grading`
  // (the optimistic value shown immediately on commit). A rejected persist
  // reverts to this instead of leaving the UI permanently showing a value
  // that was never written.
  const confirmedGradingRef = useRef(grading);
+  // Monotonic per-commit version — guards against TWO edits on the SAME
+  // element racing (not just a selection change). If edit A's persist is
+  // still in flight when edit B commits, A's eventual settle must not stamp
+  // confirmedGradingRef with its now-superseded value or revert `grading`
+  // out from under B's newer optimistic state.
+  const gradingVersionRef = useRef(0);
  const statusTimersRef = useRef<number[]>([]);
  const onSetAttributeLiveRef = useRef(onSetAttributeLive);
  const latestGradingRef = useRef(grading);
  const compareEnabledRef = useRef(compareEnabled);
-  // Captured before reassignment below — still bound to whatever selection
-  // was current on the PREVIOUS render. `commitDataAttribute` (the eventual
-  // callee) closes over `domEditSelection` in its own useCallback deps, so a
-  // selection change mints an entirely new `onSetAttributeLive` closure; this
-  // stale reference is exactly what still targets the outgoing element.
-  const previousOnSetAttributeLive = onSetAttributeLiveRef.current;
  onSetAttributeLiveRef.current = onSetAttributeLive;
  latestGradingRef.current = grading;
  compareEnabledRef.current = compareEnabled;

-  // Reset all per-element state when the selection changes to a different
+  // Reset all per-element STATE when the selection changes to a different
  // element — unlike the legacy ColorGradingSection (remounted via a
  // `key={selectionIdentityKey(element)}` from its parent), this hook is
  // called unconditionally on every render, so nothing naturally remounts it.
  // Without this, switching selection reuses the previous element's grading/
-  // compare/mediaMetadata state and can commit stale pending work onto the
-  // new target. Adjusting state during render (comparing against a ref) is
-  // React's documented pattern for STATE updates specifically — resolving in
-  // the same render pass instead of flashing stale state for one frame. It
-  // does NOT license side effects: only pure ref/state writes happen in this
-  // block. The actual outgoing-element flush is enqueued here and performed
-  // in the effect below, after commit.
+  // compare/mediaMetadata state. Adjusting state during render (comparing
+  // against a ref) is React's documented pattern for STATE updates
+  // specifically — it resolves in the same render pass instead of flashing
+  // stale state for one frame, and is safe to repeat if React discards and
+  // re-runs this render, since every value here is a pure function of
+  // `element`. It must NOT be used for side effects or for consuming
+  // shared mutable state (like the pending-persist timer/value) — a
+  // discarded render would have already consumed them with no corresponding
+  // effect ever running to compensate. That part happens below, in an
+  // effect's cleanup, which is guaranteed to run only for a render that
+  // actually committed.
  const identityKey = selectionIdentityKey(element);
  const identityKeyRef = useRef(identityKey);
  if (identityKeyRef.current !== identityKey) {
    identityKeyRef.current = identityKey;
-    if (persistTimerRef.current) {
-      clearTimeout(persistTimerRef.current);
-      persistTimerRef.current = null;
-    }
-    // Flush — don't discard — a still-pending edit for the OUTGOING element.
-    // Cancelling the debounce without writing would silently drop whatever
-    // the user just changed; targeting it at the callback bound to the OLD
-    // selection (captured above) keeps it from landing on the new element.
-    if (pendingPersistValueRef.current !== undefined) {
-      queuedOutgoingFlushRef.current = {
-        setAttributeLive: previousOnSetAttributeLive,
-        value: pendingPersistValueRef.current,
-      };
-    }
-    pendingPersistValueRef.current = undefined;
-    pendingPersistGradingRef.current = null;
-    for (const timer of statusTimersRef.current) clearTimeout(timer);
-    statusTimersRef.current = [];
    const freshGrading = readColorGradingFromElement(element);
    latestGradingRef.current = freshGrading;
    confirmedGradingRef.current = freshGrading;
@@ -265,16 +246,29 @@ export function useColorGradingController({
    setMediaMetadata(null);
  }

-  // Performs the outgoing-element flush queued above — deliberately in an
-  // effect (post-commit), not inline in the render-phase block, since
-  // writing to disk is a real side effect and must not run during render
-  // (React may call render more than once per commit without this code ever
-  // becoming visible).
+  // Flushes — never discards — a still-pending edit when selection moves to
+  // a different element, and cancels the debounce timer. Implemented as an
+  // effect CLEANUP (not the render-phase block above, and not a queued ref
+  // consumed by a separate effect): a cleanup only ever runs for the
+  // specific effect instance that actually committed for `identityKey`, so
+  // there's no window where a discarded/interrupted render could have
+  // already consumed pendingPersistValueRef without this ever running to
+  // compensate. The cleanup's closure captures onSetAttributeLive/target
+  // bound to the OUTGOING identity, since it runs before the next effect
+  // instance (for the NEW identity) is established.
  useEffect(() => {
-    const queued = queuedOutgoingFlushRef.current;
-    if (!queued) return;
-    queuedOutgoingFlushRef.current = null;
-    trackStudioPendingEdit(queued.setAttributeLive(COLOR_GRADING_DATA_KEY, queued.value));
+    return () => {
+      if (persistTimerRef.current) {
+        clearTimeout(persistTimerRef.current);
+        persistTimerRef.current = null;
+      }
+      if (pendingPersistValueRef.current === undefined) return;
+      const value = pendingPersistValueRef.current;
+      pendingPersistValueRef.current = undefined;
+      pendingPersistGradingRef.current = null;
+      trackStudioPendingEdit(onSetAttributeLive(COLOR_GRADING_DATA_KEY, value));
+    };
+    // eslint-disable-next-line react-hooks/exhaustive-deps -- identityKey is the intended trigger; see comment above
  }, [identityKey]);

  const target = useMemo(
@@ -354,13 +348,18 @@ export function useColorGradingController({
      value: string | null,
      attemptedGrading: NormalizedHfColorGrading,
      attemptIdentityKey: string,
+      isLatestAttempt: () => boolean,
    ) => {
-      // Selection may move on to a different element while this is in
-      // flight — the identity-reset block already gave THAT element its own
-      // confirmedGradingRef baseline, so a result arriving for an element
-      // we've left must not touch its state.
+      // Two guards, not one: identity (selection moved to a DIFFERENT
+      // element — that element already got its own confirmedGradingRef
+      // baseline from the reset block) and version (a NEWER edit landed on
+      // the SAME element — e.g. the user dragged Exposure, then Contrast,
+      // before Exposure's persist settled; Exposure settling afterward must
+      // not stamp confirmedGradingRef with its now-superseded value or
+      // revert `grading` out from under Contrast's newer optimistic state).
      const applySettled = (ok: boolean) => {
        if (identityKeyRef.current !== attemptIdentityKey) return;
+        if (!isLatestAttempt()) return;
        if (ok) {
          confirmedGradingRef.current = attemptedGrading;
          return;
@@ -404,7 +403,11 @@ export function useColorGradingController({
    const attemptedGrading = pendingPersistGradingRef.current ?? latestGradingRef.current;
    pendingPersistValueRef.current = undefined;
    pendingPersistGradingRef.current = null;
-    return persistColorGradingValue(value, attemptedGrading, identityKeyRef.current);
+    // A direct flush (unmount / explicit "flush all pending edits") reads
+    // pendingPersistValueRef synchronously right now, not a stored version
+    // from an earlier commit — there's nothing else it could be racing
+    // against, so it's trivially "the latest attempt" by construction.
+    return persistColorGradingValue(value, attemptedGrading, identityKeyRef.current, () => true);
  }, [persistColorGradingValue]);

  useEffect(() => addStudioPendingEditFlushListener(flushPendingPersist), [flushPendingPersist]);
@@ -495,13 +498,22 @@ export function useColorGradingController({
      // moved on, at which point identityKeyRef.current would no longer
      // describe the element this edit was actually made for.
      const attemptIdentityKey = identityKeyRef.current;
+      // Bumps a monotonic version and hands back a checker bound to THIS
+      // specific commit — reused from the same primitive the DOM-attribute
+      // commit runner uses for the identical same-target-rapid-edits race.
+      const isLatestAttempt = bumpDomEditCommitVersion(gradingVersionRef);
      persistTimerRef.current = setTimeout(() => {
        const value = pendingPersistValueRef.current;
        const attemptedGrading = pendingPersistGradingRef.current ?? nextGrading;
        pendingPersistValueRef.current = undefined;
        pendingPersistGradingRef.current = null;
        persistTimerRef.current = null;
-        void persistColorGradingValue(value ?? null, attemptedGrading, attemptIdentityKey);
+        void persistColorGradingValue(
+          value ?? null,
+          attemptedGrading,
+          attemptIdentityKey,
+          isLatestAttempt,
+        );
      }, 350);
    },
    [persistColorGradingValue, postColorGrading, postCompare, scheduleRuntimeStatusRefresh],