feat: stop reloading the page out from under people

apps/web/src/client/identify.tsx

@@ -1,6 +1,8 @@
 /** @jsxImportSource hono/jsx/dom */
 import { render, useEffect, useState } from 'hono/jsx/dom';
 
+import { emitChanged } from './live.js';
+
 /**
  * "Which one is yours?"
  *
@@ -92,6 +94,8 @@ const Identify = ({ base }: { base: string }) => {
   const [scores, setScores] = useState<Record<string, Match>>({});
   const [finding, setFinding] = useState(false);
   const [found, setFound] = useState<string | null>(null);
+  /** What just happened, said here rather than via a redirect and a flash. */
+  const [saved, setSaved] = useState<string | null>(null);
 
   const load = async (): Promise<void> => {
     try {
@@ -190,20 +194,33 @@ const Identify = ({ base }: { base: string }) => {
     const target = athleteId === '' ? 'new' : athleteId;
     setBusy(picked[0] ?? 'saving');
     setError(null);
+    setSaved(null);
     try {
       const response = await fetch(`${base}/athletes/${target}/track`, {
         method: 'POST',
         headers: { accept: 'application/json', 'content-type': 'application/json' },
         body: JSON.stringify({ trackId: picked[0], trackIds: picked }),
       });
-      const body = (await response.json()) as { ok: boolean; error?: string };
+      const body = (await response.json()) as { ok: boolean; athleteId?: string; error?: string };
       if (!response.ok || !body.ok) throw new Error(body.error ?? 'Could not save that choice.');
-      // Re-scoring runs in the background; the job log shows it finishing.
-      window.location.assign(
-        `${base}?ok=${encodeURIComponent(`Athlete identified across ${picked.length} track(s) — re-scoring`)}`,
-      );
+
+      /**
+       * Stay on the page.
+       *
+       * This navigated, which meant every save cost a full reload — losing the
+       * grid, the scroll position and any sense of what had just happened. It
+       * also raced the user: a click made before the reload landed arrived with
+       * no athlete loaded, and the server minted another one. Seven duplicates
+       * came from exactly this.
+       */
+      if (body.athleteId !== undefined) setAthleteId(body.athleteId);
+      setSaved(`Identified across ${picked.length} track(s) — re-scoring now`);
+      // Reload this island's own data, then let the rest of the page catch up.
+      await load();
+      emitChanged();
     } catch (cause) {
       setError(cause instanceof Error ? cause.message : String(cause));
+    } finally {
       setBusy(null);
     }
   };
@@ -252,6 +269,7 @@ const Identify = ({ base }: { base: string }) => {
       )}
 
       {error === null ? null : <p class="pill reject upload-error">{error}</p>}
+      {saved === null ? null : <p class="notice">{saved}</p>}
 
       {candidates.length === 0 ? (
         <p class="muted">

apps/web/src/client/jobs.tsx

@@ -1,6 +1,8 @@
 /** @jsxImportSource hono/jsx/dom */
 import { render, useEffect, useRef, useState } from 'hono/jsx/dom';
 
+import { refreshLive } from './live.js';
+
 /**
  * The live analysis log.
  *
@@ -104,9 +106,16 @@ const JobLog = ({ base }: { base: string }) => {
         if (next.some((job) => job.status === 'running' || job.status === 'queued')) {
           reloadWhenDone.current = true;
         } else if (reloadWhenDone.current && next.some((job) => job.status === 'completed')) {
-          // Analysis writes moments; the page around this island is stale now.
+          /**
+           * Analysis writes moments, so the page around this island is stale —
+           * but reloading it was a blunt instrument. It threw away scroll
+           * position, collapsed whatever the user had open, and interrupted a
+           * selection in progress; worse, it raced their clicks, which is how
+           * one athlete became seven. Swapping the server-rendered regions
+           * leaves everything else exactly where it was.
+           */
           reloadWhenDone.current = false;
-          window.setTimeout(() => window.location.reload(), 1500);
+          void refreshLive();
         }
       });
 

apps/web/src/client/live.ts

@@ -0,0 +1,86 @@
+import { mountReview } from './review.js';
+
+/**
+ * Bringing the page up to date without reloading it.
+ *
+ * The pages are server-rendered and must keep working with no JavaScript, so
+ * the server stays the single source of truth — there is no client router and
+ * no client-side view model to keep in sync. What there was instead was
+ * `window.location.reload()`, fired 1.5 seconds after any job finished, plus a
+ * "Analysis is running. Refresh for progress." link for the rest of the time.
+ *
+ * That is what made the app feel broken to a new user: the page moved under
+ * them without being asked, they lost their place, and between refreshes there
+ * was no way to tell whether to wait or to click again. Clicking again is
+ * exactly what produced seven duplicate athletes — the reloads were racing the
+ * clicks.
+ *
+ * So: fetch the same URL the user is already on, and swap only the regions
+ * marked `data-live`. The server renders them exactly as it always did, the
+ * islands that own state (the job log holding an EventSource, the identify
+ * grid holding a half-finished selection) are never inside one, and nothing
+ * scrolls.
+ */
+
+/** Islands that live inside a swappable region and must be re-mounted after it. */
+const REMOUNT: Record<string, () => void> = {
+  moments: mountReview,
+};
+
+let inFlight: Promise<void> | null = null;
+
+/**
+ * Replaces every `data-live` region with the server's current version.
+ *
+ * Coalesced: several finishing jobs, or a save and a job completing together,
+ * should cost one fetch and one repaint rather than three.
+ */
+export const refreshLive = async (): Promise<void> => {
+  if (inFlight !== null) return inFlight;
+
+  inFlight = (async () => {
+    try {
+      const response = await fetch(window.location.href, {
+        headers: { accept: 'text/html' },
+        credentials: 'same-origin',
+      });
+      if (!response.ok) return;
+      const parsed = new DOMParser().parseFromString(await response.text(), 'text/html');
+
+      // Array.from rather than iterating the NodeList: the DOM lib this project
+      // compiles against does not give NodeListOf an iterator.
+      for (const current of Array.from(document.querySelectorAll<HTMLElement>('[data-live]'))) {
+        const key = current.dataset['live'];
+        if (key === undefined) continue;
+        const fresh = parsed.querySelector<HTMLElement>(`[data-live="${key}"]`);
+        // A region that has gone away entirely is left alone rather than
+        // blanked: an empty page is a worse lie than a stale one.
+        if (fresh === null) continue;
+        if (fresh.innerHTML === current.innerHTML) continue;
+        current.innerHTML = fresh.innerHTML;
+        REMOUNT[key]?.();
+      }
+    } catch {
+      // Offline, or the session expired. The page is stale, which is survivable;
+      // throwing here would take an island down with it.
+    } finally {
+      inFlight = null;
+    }
+  })();
+
+  return inFlight;
+};
+
+type Listener = () => void;
+const listeners = new Set<Listener>();
+
+/** Something changed the project; islands that show its data should catch up. */
+export const emitChanged = (): void => {
+  for (const listener of listeners) listener();
+  void refreshLive();
+};
+
+export const onChanged = (listener: Listener): (() => void) => {
+  listeners.add(listener);
+  return () => listeners.delete(listener);
+};

apps/web/src/client/main.tsx

@@ -105,6 +105,9 @@ const MomentReview = ({ projectId, initial }: { projectId: string; initial: Mome
 };
 
 const mount = (): void => {
+  // Proof the bundle ran, so the stylesheet can hide instructions that only
+  // make sense without it.
+  document.documentElement.classList.add('js');
   // Independent of the review island, and present on pages that have no moments.
   mountUploads();
   mountJobLog();

apps/web/src/client/upload.tsx

@@ -1,6 +1,8 @@
 /** @jsxImportSource hono/jsx/dom */
 import { render, useEffect, useRef, useState } from 'hono/jsx/dom';
 
+import { refreshLive } from './live.js';
+
 /**
  * The realtime uploader.
  *
@@ -301,9 +303,10 @@ const Uploader = ({ base }: { base: string }) => {
         name: finished.upload?.fileName ?? item.name,
       });
 
-      // Everything settled: bring the page's own video list up to date.
+      // Everything settled: bring the page's own video list up to date. Swapped
+      // in place, so a second upload queued behind this one is not interrupted.
       if (items.current.every((entry) => entry.phase === 'done' || entry.phase === 'canceled')) {
-        window.setTimeout(() => window.location.reload(), 1200);
+        void refreshLive();
       }
     } catch (error) {
       // A canceled request is a user action, not a failure to report.
@@ -402,7 +405,7 @@ const Uploader = ({ base }: { base: string }) => {
     try {
       await api(`${base}/uploads/${dto.id}/finish`, { method: 'POST' });
       await refresh();
-      window.setTimeout(() => window.location.reload(), 800);
+      void refreshLive();
     } catch (error) {
       setListError(error instanceof Error ? error.message : String(error));
     }

apps/web/src/liveregions.test.ts

@@ -0,0 +1,94 @@
+import { describe, expect, it } from 'vitest';
+
+import { ProjectPage } from './views/pages.js';
+import type { ProjectView } from './views/pages.js';
+
+/**
+ * The page updates itself by fetching its own URL and swapping the regions
+ * marked `data-live`. That only works while the islands holding state stay
+ * outside them.
+ *
+ * The job log owns an EventSource; swapping it would drop the analysis feed and
+ * silently reconnect. The identify grid holds a selection the user is part-way
+ * through making; swapping it would discard their clicks — which is close to
+ * the original bug, where reloads raced clicks into seven duplicate athletes.
+ *
+ * `#moment-review` is the exception: it is inside `data-live="moments"` on
+ * purpose, because it re-reads everything from a `data-moments` attribute, and
+ * live.ts re-mounts it after a swap.
+ */
+
+const view: ProjectView = {
+  project: { id: 'prj_test', name: 'Smoke', sport: 'basketball' } as ProjectView['project'],
+  videos: [],
+  athletes: [],
+  moments: [],
+  clips: [],
+  jobs: [],
+  exports: [],
+  music: [],
+  flash: {},
+};
+
+const render = async (extra: Partial<ProjectView> = {}): Promise<string> =>
+  String(await ProjectPage({ ...view, ...extra }));
+
+/** Crude but sufficient: is `needle` inside a `data-live` block in `html`? */
+const insideLiveRegion = (html: string, needle: string): boolean => {
+  const at = html.indexOf(needle);
+  if (at < 0) return false;
+  const before = html.slice(0, at);
+  const opens = (before.match(/<div data-live=/g) ?? []).length;
+  // Count only the closing tags that belong to a region we have opened.
+  let depth = 0;
+  let closed = 0;
+  for (const match of before.matchAll(/<div data-live=|<div|<\/div>/g)) {
+    if (match[0] === '<div data-live=') depth += 1;
+    else if (match[0] === '<div' && depth > 0) depth += 1;
+    else if (match[0] === '</div>' && depth > 0) {
+      depth -= 1;
+      if (depth === 0) closed += 1;
+    }
+  }
+  return opens > closed;
+};
+
+describe('the regions the page swaps in place', () => {
+  it('marks the ones that go stale after a job', async () => {
+    const html = await render();
+    for (const key of ['videos', 'athletes', 'moments']) {
+      expect(html).toContain(`data-live="${key}"`);
+    }
+  });
+
+  it('keeps the job log outside them, so the analysis feed survives', async () => {
+    // An EventSource inside a swapped region is dropped and reconnected on
+    // every update, losing the log the user is reading.
+    const html = await render({ jobs: [] });
+    expect(html).toContain('id="job-log"');
+    expect(insideLiveRegion(html, 'id="job-log"')).toBe(false);
+  });
+
+  it('keeps the identify grid outside them, so a half-made selection survives', async () => {
+    const html = await render();
+    expect(html).toContain('id="identify-athlete"');
+    expect(insideLiveRegion(html, 'id="identify-athlete"')).toBe(false);
+  });
+
+  it('keeps the uploader outside them, so an upload in flight is not interrupted', async () => {
+    const html = await render();
+    const at = html.indexOf('id="uploads"');
+    if (at >= 0) expect(insideLiveRegion(html, 'id="uploads"')).toBe(false);
+  });
+
+  it('stops telling a user to refresh once the page can update itself', async () => {
+    const running = [
+      { id: 'job_x', kind: 'detection', status: 'running', progress: 0.5 },
+    ] as unknown as ProjectView['jobs'];
+    const html = await render({ jobs: running });
+    // Still present for the no-JavaScript case, but hidden once the bundle runs.
+    expect(html).toContain('no-js-only');
+    const at = html.indexOf('Analysis is running');
+    expect(html.slice(Math.max(0, at - 200), at)).toContain('no-js-only');
+  });
+});

apps/web/src/views/Layout.tsx

@@ -37,6 +37,9 @@ const STYLES = `
   h1 { font-size: 1.4rem; margin: 0 0 .25rem; }
   h2 { font-size: 1.05rem; margin: 2rem 0 .75rem; }
   .muted { color: var(--muted); }
+  /* Advice that only applies without JavaScript — telling someone to refresh
+     is actively wrong once the page updates itself. */
+  html.js .no-js-only { display: none; }
   .card {
     border: 1px solid var(--line); border-radius: .6rem;
     padding: .9rem 1rem; margin-bottom: .6rem; background: var(--card);

apps/web/src/views/pages.tsx

@@ -156,6 +156,8 @@ export const ProjectPage: FC<ProjectView> = ({
       <Notice flash={flash} />
 
       <h2>Footage</h2>
+      {/* The uploader below posts into this list; swapped rather than reloaded. */}
+      <div data-live="videos">
       {videos.length === 0 ? (
         <p class="empty">Nothing imported yet.</p>
       ) : (
@@ -191,6 +193,8 @@ export const ProjectPage: FC<ProjectView> = ({
         </table>
       )}
 
+      </div>
+
       <details class="card" open={videos.length === 0}>
         <summary>Import footage</summary>
 
@@ -226,6 +230,10 @@ export const ProjectPage: FC<ProjectView> = ({
       </details>
 
       <h2>Athlete to follow</h2>
+      {/* Swapped in place when anything changes it — see client/live.ts. The
+          identify grid below is deliberately outside: it holds a selection the
+          user is part-way through making. */}
+      <div data-live="athletes">
       {athletes.length === 0 ? (
         <p class="empty">Nobody yet. Analysis needs someone to follow.</p>
       ) : (
@@ -288,6 +296,7 @@ export const ProjectPage: FC<ProjectView> = ({
           <button type="submit">Add athlete</button>
         </form>
       </details>
+      </div>
 
       {/* The one irreducibly manual step: a detector cannot know which player
           is yours. Until an athlete is bound to a track, scoring has no focal
@@ -416,14 +425,19 @@ export const ProjectPage: FC<ProjectView> = ({
             </tbody>
           </table>
         )}
+        {/* The no-JavaScript fallback only. With the bundle loaded the job log
+            below streams progress and the page updates itself, so telling a
+            user to refresh — the thing that loses their place and races their
+            clicks — would be advice to make it worse. */}
         {jobs.some((job) => job.status === 'running' || job.status === 'queued') ? (
-          <p class="muted">
+          <p class="muted no-js-only">
             Analysis is running. <a href={base}>Refresh</a> for progress.
           </p>
         ) : null}
       </div>
 
       <h2>Suggested moments</h2>
+      <div data-live="moments">
       {/* The honest answer to "is it even following the right kid" — and the
           place to fix it when it is not. */}
       <p class="muted">
@@ -513,6 +527,7 @@ export const ProjectPage: FC<ProjectView> = ({
           </div>
         </>
       )}
+      </div>
 
       <h2>Reel</h2>
       <div class="card">