fix(web): settle every review save, not just the latest

Per-call mutate() callbacks fire only for an observer's latest call, so
with overlapping saves the in-flight count could stay above zero and the
review would ignore every later server value, including a Restore.

Saves now settle through useEntryPatch's hook-level onSettled (fires for
every mutation), in FIFO order (the entry-patch scope is serial). The
draft counts pending saves, remembers the latest incoming value, skips our
own optimistic/rollback echoes, and applies an outside change once the
saves settle.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

apps/web/src/components/DetailBoxes.tsx

@@ -6,16 +6,21 @@ import { orderEngagements, setCustomValue } from "../edits.ts";
 import {
   actorConnectionId,
   describeRating,
+  draftIncoming,
+  draftSettled,
+  draftStartSave,
   engagementNouns,
   formatDateRange,
   formatPartialDate,
   formatRating,
   friendlyError,
+  newDraft,
   ordinal,
   parsePartialDate,
   rollupLabel,
   statusLabel,
   timeAgo,
+  type Draft,
   type PartialDate,
 } from "../format.ts";
 import { useEntryPatch, useSources } from "../hooks.ts";
@@ -357,6 +362,7 @@ export function SourcesBox({ entry, engagements }: { entry: EntryView; engagemen
 export function CustomFieldsBox({ entry, type }: { entry: EntryView; type: string }) {
   const defs = useQuery({ queryKey: ["custom-fields", type], queryFn: () => api.customFields(type), staleTime: 60_000 });
   const patch = useEntryPatch(entry.id);
+  const scale = useRatingScale();
   if (!defs.data?.length) return null;
   const save = (def: CustomFieldDef, value: unknown) =>
     patch.edit((cur) => {
@@ -370,7 +376,7 @@ export function CustomFieldsBox({ entry, type }: { entry: EntryView; type: strin
           <CustomField key={d.id} def={d} value={entry.customFields[d.id]} onSave={(v) => save(d, v)} />
         ))}
       </div>
-      {patch.error && <p className="error-text" role="alert">{friendlyError(patch.error)}</p>}
+      {patch.error && <p className="error-text" role="alert">{friendlyError(patch.error, { ratingScale: scale })}</p>}
     </Box>
   );
 }
@@ -492,23 +498,29 @@ export function ChildrenBox({ entry, type }: { entry: EntryView; type: string })
 // ---------------------------------------------------------------------------------------------
 
 export function ReviewBox({ entry }: { entry: EntryView }) {
-  const patch = useEntryPatch(entry.id);
   const [saved, setSaved] = useState(false);
-  const [text, setText] = useState(entry.review ?? "");
-  // The server value this text was last in sync with. When the review changes elsewhere (Restore,
-  // another tab) and the user has not typed since, show the new value, so a later blur can't undo it.
-  const synced = useRef(entry.review ?? "");
-  useEffect(() => {
-    const incoming = entry.review ?? "";
-    if (incoming === synced.current) return;
-    setText((cur) => (cur === synced.current ? incoming : cur));
-    synced.current = incoming;
-  }, [entry.review]);
+  // Local text plus the server value it last agreed with (see `Draft` in format.ts). A Restore shows
+  // up here unless the user has typed since; a failed save keeps what they typed.
+  const [draft, setDraft] = useState<Draft>(() => newDraft(entry.review ?? ""));
+  // Saves in flight, oldest first. The entry-patch scope runs them serially, so they settle in order.
+  const saves = useRef<{ was: string; stored: string }[]>([]);
+  const patch = useEntryPatch(entry.id, {
+    onSettled: (_p, error) => {
+      const save = saves.current.shift();
+      if (!save) return;
+      setDraft((d) => draftSettled(d, save, error === null));
+      setSaved(error === null && saves.current.length === 0);
+    },
+  });
+  useEffect(() => setDraft((d) => draftIncoming(d, entry.review ?? "")), [entry.review]);
   const save = () => {
-    const next = text.trim() === "" ? null : text;
-    if ((next ?? "") === (entry.review ?? "")) return;
-    synced.current = next ?? "";
-    patch.mutate({ review: next }, { onSuccess: () => setSaved(true) });
+    const next = draft.text.trim() === "" ? null : draft.text;
+    const stored = next ?? "";
+    if (stored === draft.synced && stored === (entry.review ?? "")) return;
+    const s = { was: draft.synced, stored };
+    saves.current.push(s);
+    setDraft((d) => draftStartSave(d, s));
+    patch.mutate({ review: next });
   };
   return (
     <Box
@@ -521,9 +533,13 @@ export function ReviewBox({ entry }: { entry: EntryView }) {
       <label htmlFor="review-text" className="sr-only">Your review (Markdown)</label>
       <textarea
         id="review-text"
-        value={text}
+        value={draft.text}
         placeholder="What stayed with you? Markdown works here."
-        onChange={(e) => (setText(e.target.value), setSaved(false))}
+        onChange={(e) => {
+          const text = e.target.value;
+          setDraft((d) => ({ ...d, text }));
+          setSaved(false);
+        }}
         onBlur={save}
         onKeyDown={(e) => {
           if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) save();

apps/web/src/components/EntryDetail.tsx

@@ -18,7 +18,9 @@ const TIMES = ["", "once", "twice", "three times"];
 export function EntryDetail({ id }: { id: string }) {
   const entry = useQuery({ queryKey: ["entry", id], queryFn: () => api.entry(id) });
   if (entry.isPending) return <div className="detail" aria-busy="true" />;
-  if (entry.error) {
+  // A failed background refetch (server briefly down) keeps showing the cached entry, so nothing the
+  // user is typing gets unmounted; only a first load that fails shows the error screen.
+  if (!entry.data) {
     const gone = entry.error instanceof ApiError && entry.error.status === 404;
     return (
       <div className="detail-empty" role="alert">
@@ -96,7 +98,7 @@ function Detail({ entry }: { entry: EntryView }) {
             onAdd={(t) => patch.edit((cur) => ({ tags: addTag(cur.tags, t) }))}
             onRemove={(t) => patch.edit((cur) => ({ tags: removeTag(cur.tags, t) }))}
           />
-          {patch.error && <p className="error-text" role="alert">{friendlyError(patch.error)}</p>}
+          {patch.error && <p className="error-text" role="alert">{friendlyError(patch.error, { ratingScale: scale })}</p>}
         </div>
       </header>
 

apps/web/src/components/HistoryDrawer.tsx

@@ -30,6 +30,8 @@ export function HistoryDrawer({ entry, onClose }: { entry: EntryView; onClose: (
   const closeBtn = useRef<HTMLButtonElement>(null);
   const history = useQuery({ queryKey: ["history", entry.id], queryFn: () => api.history(entry.id) });
   const restore = useMutation({
+    // Same scope as the entry's PATCHes (useEntryPatch), so a restore never races a pending blur-save.
+    scope: { id: `entry-patch:${entry.id}` },
     mutationFn: api.restore,
     onSuccess: () => {
       for (const key of [["entry", entry.id], ["history", entry.id], ["engagements", entry.id], ["entries"]]) void qc.invalidateQueries({ queryKey: key });

apps/web/src/format.ts

@@ -231,8 +231,9 @@ const TOO_LARGE = "That file is too large. The limit is 25 MB.";
 /**
  * An API error in the product's voice: what happened and how to fix it. Validation messages arrive as
  * `field: reason` (core) or `field: Zod message`, several joined by "; "; the first one is described.
+ * Pass the user's rating scale so out-of-range ratings are described in the scale they see.
  */
-export function friendlyError(err: unknown): string {
+export function friendlyError(err: unknown, opts: { ratingScale?: RatingScale } = {}): string {
   if (!err || typeof err !== "object" || !("code" in err) || !("message" in err)) return "Something went wrong. Try again.";
   const { status, code, message } = err as { status?: number; code: string; message: string };
   if (status === 413) return TOO_LARGE;
@@ -249,7 +250,11 @@ export function friendlyError(err: unknown): string {
   if (field === "currency" || reason === "not_cents") return "Price needs a number like 14 or 14.99, and a three-letter currency like GBP.";
   if (reason === "not_url") return `${label} needs a full web address, starting with https://.`;
   if (reason === "not_date" || reason === "bad_precision") return `${label} needs a date like 2025, 2025-03 or 2025-03-14.`;
-  if (reason === "out_of_range") return `${label} is out of range. Ratings go from 0 to 100.`;
+  if (reason === "out_of_range") {
+    const scale = opts.ratingScale ?? "5half";
+    const range = scale === "10" ? "0 and 10" : scale === "100" ? "0 and 100" : "0 and 5 stars";
+    return label === "Rating" ? `Pick a rating between ${range}.` : `Pick a rating for ${label} between ${range}.`;
+  }
   if (reason === "required" || /^Too small: .*>=1 (character|item)/.test(reason) || /received undefined$/.test(reason)) return `${label} can't be empty.`;
   if (reason === "too_long" || reason.startsWith("Too big")) return `${label} is too long. Shorten it and try again.`;
   if (reason.startsWith("Too small")) return `${label} is too short.`;
@@ -258,3 +263,52 @@ export function friendlyError(err: unknown): string {
   if (reason === "duplicate") return `${label} is already in use. Pick another.`;
   return `${label} wasn't accepted. Check it and try again.`;
 }
+
+// ---------------------------------------------------------------------------------------------
+// Review draft sync
+// ---------------------------------------------------------------------------------------------
+
+/**
+ * A text field edited locally and saved on blur. `synced` is the server value the text last agreed
+ * with; text that differs from it is unsaved and is never replaced. `pending` counts saves in flight
+ * (one entry's saves run serially, so they settle in order). While any is pending, incoming server
+ * values are only remembered in `incoming`. Values that are our own saves' echoes (optimistic updates
+ * show a save's `stored`, rollbacks show its `was`) are listed in `echoes` and ignored; any other
+ * value (an outside change, a Restore) is applied once the saves settle. So the outcome does not
+ * depend on the order in which rollbacks, refetches and settle callbacks land.
+ */
+export interface Draft {
+  text: string;
+  synced: string;
+  pending: number;
+  incoming: string;
+  echoes: string[];
+}
+
+export function newDraft(value: string): Draft {
+  return { text: value, synced: value, pending: 0, incoming: value, echoes: [] };
+}
+
+/** A server value arrived. Untouched text follows it; edited text is kept. Deferred while saving. */
+export function draftIncoming(d: Draft, incoming: string): Draft {
+  if (d.pending > 0 || incoming === d.synced) return { ...d, incoming };
+  return { ...d, text: d.text === d.synced ? incoming : d.text, synced: incoming, incoming };
+}
+
+/** A save of `stored` started (`was` is the synced value before it): it counts as synced unless it fails. */
+export function draftStartSave(d: Draft, save: { was: string; stored: string }): Draft {
+  return { ...d, synced: save.stored, pending: d.pending + 1, echoes: [...d.echoes, save.was, save.stored] };
+}
+
+/**
+ * The oldest pending save settled. Success: `stored` is on the server. Failure: back to `was`, so the
+ * local text still counts as unsaved. When the last pending save settles, a remembered incoming value
+ * that is not one of our own echoes is applied.
+ */
+export function draftSettled(d: Draft, save: { was: string; stored: string }, ok: boolean): Draft {
+  const pending = Math.max(0, d.pending - 1);
+  const next: Draft = { ...d, synced: ok ? save.stored : save.was, pending };
+  if (pending > 0) return next;
+  const settled: Draft = { ...next, echoes: [] };
+  return d.echoes.includes(d.incoming) ? settled : draftIncoming(settled, d.incoming);
+}

apps/web/src/hooks.ts

@@ -1,5 +1,5 @@
 import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
-import { useEffect, useState } from "react";
+import { useEffect, useRef, useState } from "react";
 import { api, type ChangeEvent, type EntryPatch, type EntryView } from "./api.ts";
 import { applyEntryPatch } from "./edits.ts";
 
@@ -37,9 +37,15 @@ export function useDebounced<T>(value: T, ms = 220): T {
  * error) and PATCHes for one entry run one at a time (mutation scope), in the order they were made.
  * Use `edit(cur => patch)` when the next value depends on the current one (tags, custom fields): it
  * builds from the latest cache, so quick successive edits compose.
+ *
+ * `onSettled` fires once for EVERY mutation of this hook (it is a hook-level option). Per-call
+ * `mutate(patch, { onSuccess })` callbacks only fire for the observer's latest call, so never use
+ * them for bookkeeping.
  */
-export function useEntryPatch(id: string) {
+export function useEntryPatch(id: string, opts: { onSettled?: (patch: EntryPatch, error: Error | null) => void } = {}) {
   const qc = useQueryClient();
+  const onSettledRef = useRef(opts.onSettled);
+  onSettledRef.current = opts.onSettled;
   const key = ["entry", id];
   const mutationKey = ["entry-patch", id];
   const m = useMutation({
@@ -60,9 +66,10 @@ export function useEntryPatch(id: string) {
       // Only the last queued edit writes the server's answer, so earlier answers don't flash old values.
       if (qc.isMutating({ mutationKey }) <= 1) qc.setQueryData(key, entry);
     },
-    onSettled: () => {
+    onSettled: (_entry, error, patch) => {
       void qc.invalidateQueries({ queryKey: ["entries"] });
       void qc.invalidateQueries({ queryKey: ["history", id] });
+      onSettledRef.current?.(patch, error);
     },
   });
   const edit = (build: (cur: EntryView) => EntryPatch | null) => {

apps/web/src/routes/register.tsx

@@ -29,6 +29,7 @@ export function Register() {
     setError(null);
     try {
       await api.register({ username, password, displayName: displayName || undefined, inviteCode: inviteCode || undefined });
+      void qc.invalidateQueries({ queryKey: ["auth-status"] });
       qc.setQueryData(["me"], await api.login(username, password));
       navigate("/", { replace: true });
     } catch (err) {

apps/web/test/format.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { formatDateRange, formatPartialDate, formatRating, friendlyError, parsePartialDate, rollupLabel, shortDate } from "../src/format.ts";
+import { draftIncoming, draftSettled, draftStartSave, newDraft, type Draft, formatDateRange, formatPartialDate, formatRating, friendlyError, parsePartialDate, rollupLabel, shortDate } from "../src/format.ts";
 
 describe("formatRating", () => {
   test("5 stars with halves", () => {
@@ -72,7 +72,9 @@ describe("friendlyError", () => {
   test("maps validation reasons to plain copy", () => {
     expect(friendlyError(v("customFields.Website: not_url"))).toBe("Website needs a full web address, starting with https://.");
     expect(friendlyError(v("finishedAt: not_date"))).toBe("Finished needs a date like 2025, 2025-03 or 2025-03-14.");
-    expect(friendlyError(v("rating: out_of_range"))).toBe("Rating is out of range. Ratings go from 0 to 100.");
+    expect(friendlyError(v("rating: out_of_range"))).toBe("Pick a rating between 0 and 5 stars.");
+    expect(friendlyError(v("rating: out_of_range"), { ratingScale: "10" })).toBe("Pick a rating between 0 and 10.");
+    expect(friendlyError(v("customFields.Mood: out_of_range"), { ratingScale: "5" })).toBe("Pick a rating for Mood between 0 and 5 stars.");
     expect(friendlyError(v("title: required"))).toBe("Title can't be empty.");
     expect(friendlyError(v("review: too_long"))).toBe("Review is too long. Shorten it and try again.");
     expect(friendlyError(v("password: Too small: expected string to have >=8 characters"))).toBe("Passwords need at least 8 characters.");
@@ -94,3 +96,53 @@ describe("friendlyError", () => {
 test("shortDate", () => {
   expect(shortDate("2026-10-12T10:00:00")).toBe("12 Oct");
 });
+
+describe("review draft sync", () => {
+  const draft = (text: string, synced: string): Draft => ({ ...newDraft(synced), text });
+  const pick = (d: Draft) => [d.text, d.synced, d.pending];
+
+  test("a rollback after a failed save keeps the local text", () => {
+    const save = { was: "First thoughts.", stored: "Second thoughts." };
+    let d = draftStartSave(draft("Second thoughts.", save.was), save);
+    d = draftIncoming(d, save.stored); // optimistic update
+    d = draftIncoming(d, save.was); // rollback
+    d = draftSettled(d, save, false);
+    expect(pick(d)).toEqual(["Second thoughts.", "First thoughts.", 0]);
+  });
+
+  // Two overlapping saves: "a" (was "old"), then "b" (was "a"). Incoming values mirror what the cache does.
+  const A = { was: "old", stored: "a" };
+  const B = { was: "a", stored: "b" };
+  const twoSaves = () => draftIncoming(draftIncoming(draftStartSave({ ...draftStartSave(draft("a", "old"), A), text: "b" }, B), "a"), "b");
+
+  test("two overlapping saves both settle and the flag clears (success, success)", () => {
+    let d = twoSaves();
+    expect(d.pending).toBe(2);
+    d = draftSettled(draftSettled(d, A, true), B, true);
+    expect(pick(d)).toEqual(["b", "b", 0]);
+    expect(d.echoes).toEqual([]);
+  });
+
+  test("two overlapping saves: fail then success ends synced; success then fail keeps text unsaved", () => {
+    let d = draftIncoming(twoSaves(), "old"); // A's rollback
+    d = draftSettled(draftSettled(d, A, false), B, true);
+    expect(pick(d)).toEqual(["b", "b", 0]);
+    let e = draftIncoming(twoSaves(), "a"); // B's rollback
+    e = draftSettled(draftSettled(e, A, true), B, false);
+    expect(pick(e)).toEqual(["b", "a", 0]);
+  });
+
+  test("an incoming value that arrives while saving is applied once saving ends", () => {
+    const save = { was: "x", stored: "y" };
+    let d = draftStartSave(draft("y", "x"), save);
+    d = draftIncoming(d, "Restored.");
+    expect(d.text).toBe("y");
+    d = draftSettled(d, save, true);
+    expect(pick(d)).toEqual(["Restored.", "Restored.", 0]);
+  });
+
+  test("a change from elsewhere replaces untouched text but not edited text", () => {
+    expect(draftIncoming(draft("Second.", "Second."), "First.").text).toBe("First.");
+    expect(draftIncoming(draft("Typed.", "Second."), "Other.").text).toBe("Typed.");
+  });
+});

packages/core/src/preferences.ts

@@ -1,4 +1,5 @@
 import type { Db } from "@sttracker/db/db";
+import type { Executor } from "./history.ts";
 import { fromJson, toJson } from "@sttracker/db/json";
 import { nowIso } from "@sttracker/db/ids";
 import type { Visibility } from "@sttracker/db/schema";
@@ -16,15 +17,20 @@ export interface Preferences {
 
 export const DEFAULT_PREFERENCES: Preferences = { ratingScale: "5half", defaultVisibility: "private" };
 
-async function readRaw(db: Db, userId: string): Promise<Record<string, unknown>> {
-  const u = await db.k.selectFrom("users").select("preferences").where("id", "=", userId).executeTakeFirst();
+async function readRaw(k: Executor, userId: string, lock = false): Promise<Record<string, unknown>> {
+  let q = k.selectFrom("users").select("preferences").where("id", "=", userId);
+  if (lock) q = q.forUpdate();
+  const u = await q.executeTakeFirst();
   if (!u) throw new NotFoundError("user");
   return fromJson<Record<string, unknown>>(u.preferences, {});
 }
 
 /** The user's preferences; missing or invalid stored values fall back to the defaults. */
 export async function getPreferences(db: Db, userId: string): Promise<Preferences> {
-  const raw = await readRaw(db, userId);
+  return toPreferences(await readRaw(db.k, userId));
+}
+
+function toPreferences(raw: Record<string, unknown>): Preferences {
   return {
     ratingScale: (RATING_SCALES as readonly unknown[]).includes(raw.ratingScale) ? (raw.ratingScale as RatingScale) : DEFAULT_PREFERENCES.ratingScale,
     defaultVisibility: (VISIBILITIES as readonly unknown[]).includes(raw.defaultVisibility)
@@ -37,8 +43,12 @@ export async function getPreferences(db: Db, userId: string): Promise<Preference
 export async function updatePreferences(db: Db, userId: string, patch: Partial<Preferences>): Promise<Preferences> {
   if (patch.ratingScale !== undefined && !RATING_SCALES.includes(patch.ratingScale)) throw new ValidationError("ratingScale", "invalid");
   if (patch.defaultVisibility !== undefined && !VISIBILITIES.includes(patch.defaultVisibility)) throw new ValidationError("defaultVisibility", "invalid");
-  const raw = await readRaw(db, userId);
-  const next = { ...raw, ...Object.fromEntries(Object.entries(patch).filter(([, v]) => v !== undefined)) };
-  await db.k.updateTable("users").set({ preferences: toJson(next), updated_at: nowIso() }).where("id", "=", userId).execute();
-  return getPreferences(db, userId);
+  // Read-modify-write in one transaction, with the user row locked on Postgres (SQLite runs one
+  // connection), so concurrent updates of different keys don't drop each other.
+  return db.k.transaction().execute(async (tx) => {
+    const raw = await readRaw(tx, userId, db.dialect === "postgres");
+    const next = { ...raw, ...Object.fromEntries(Object.entries(patch).filter(([, v]) => v !== undefined)) };
+    await tx.updateTable("users").set({ preferences: toJson(next), updated_at: nowIso() }).where("id", "=", userId).execute();
+    return toPreferences(next);
+  });
 }