Clear the second review's findings, though it approved

**A derive that throws was retried for ever, silently.** `sweepMissingAnalyses`
discarded `analyseMissing`'s `skipped` map, so a video whose derive throws — an
imported map with `holeRadius_px: 0` reaches this, since the map file accepts
any finite radius and `geometry.ts` demands a positive one — was retried on
every notification, with nothing said and the figures reporting only "1 of 3
videos not analysed yet" beside a button to the Track step that cannot help,
because the video is tracked. The failures are now remembered until the maze map
or the parameters change, announced once, and named on the figures' own line
with the Track button hidden: the Track step fixes "not tracked", not this.

**The try/catch added last commit was dead under test.** happy-dom returns null
from `getContext`, so no test in the suite ever entered the draw path, and the
assertion I wrote about the cards after a failure ("still current, not stale")
was checking captions written before the try block from `describe()`, which
cannot throw — it passed whether or not `draw()` threw. That is the same shape
of error as the tier test the first pass caught. The path is now driven with a
stubbed context and a figure whose `draw` throws, asserting the failed card's
note, its disabled Save button and the next card drawing normally.

Nits: the comment claiming 3.4 million cells understated it by three orders of
magnitude (the platform is 92 cm across, not 46, so 0.001 cm asks for 8.46e9
cells); `CELL_CM_RANGE` no longer claims any positive number works;
`trimNumber`'s comment no longer promises behaviour the clamp makes
unreachable; and `app-smoke.spec.ts` names the button this chunk actually
shipped rather than one that never existed.

One limitation recorded: a maze nudge re-derives the whole cohort from a step
the user is not on — 10.5 ms for three videos, but roughly 0.4 s for sixty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLagxs47Fmp7TGEidN8wqk

Author: alexbastuba

docs/known-limitations.md

@@ -23,6 +23,14 @@ session it is found (D39).
   but a 59 cm "entry" is a number a user can export without noticing. Whether a user assertion
   should be believed unconditionally, warned about, or refused beyond some distance from a hole is
   an operational definition, so it is recorded rather than changed here.
+- **Every maze nudge re-derives the whole cohort, from a step the user is not looking at.** The app
+  shell refreshes every step on every store notification, and the Review step re-derives whatever
+  the store invalidated so its figures and its export line are not left claiming that analysed
+  videos are unanalysed. So one arrow key on the Maze step costs a derive per video. Measured on the
+  example cohort (5,539 / 741 / 905 frames): 10.5 ms per nudge steady-state, 7.3 ms of it the long
+  video. Comfortable at three videos; a sixty-video cohort would put roughly 0.4 s of main-thread
+  work behind each keypress. The fix is to re-derive lazily when the Review step is shown rather
+  than on every notification, or to move the sweep off the main thread.
 - **The cohort figures always plot primary latency.** `PlottableMetric` offers seven measures and
   `src/viz/cohort.ts` names them all in `METRIC_LABELS`, but the Review step's figures section
   exposes no control for it, so the learning curve and the group comparison are fixed to

src/ui/review-figures.ts

@@ -83,6 +83,13 @@ export interface ReviewFiguresProps {
   session: SessionFile;
   /** The video on screen; null when none can be shown. */
   videoId: string | null;
+  /**
+   * Videos that are tracked but whose analysis could not be computed at all, and
+   * why. Different from "not analysed yet": the Track step cannot help, so the
+   * reason is shown and the button to it is not (D16 — say what went wrong
+   * rather than a true-but-misleading count).
+   */
+  problems?: readonly string[];
 }
 
 export interface ReviewFiguresCallbacks {
@@ -365,10 +372,20 @@ export function createReviewFigures(
   }
 
   function renderMissing(): void {
+    const problems = current.problems ?? [];
     const line = notAnalysedInFigures(current.session);
-    missing.hidden = line === null;
-    missingLine.textContent = line ?? '';
+    missing.hidden = line === null && problems.length === 0;
     const { notAnalysed, total } = notAnalysedCount(current.session);
+
+    if (problems.length > 0) {
+      // The Track step cannot fix a derive that throws, so it is not offered.
+      missingLine.textContent = `${problems.length} of ${total} video${total === 1 ? '' : 's'} could not be analysed, so ${problems.length === 1 ? 'it is' : 'they are'} not in these figures: ${problems.join('; ')}.`;
+      trackButton.hidden = true;
+      return;
+    }
+
+    missingLine.textContent = line ?? '';
+    trackButton.hidden = line === null;
     trackButton.setAttribute(
       'aria-label',
       `Go to the Track step to analyse the ${notAnalysed} of ${total} videos that are not in these figures`,

src/ui/review-step.ts

@@ -178,6 +178,10 @@ export function createReviewStep(context: AppContext): Step {
   let adopted: { videoId: VideoId; run: AnalysisRun } | null = null;
   /** True while a cohort sweep is writing derived caches; one render follows, not one per video. */
   let sweeping = false;
+  /** Videos whose derive threw, and why, so the sweep stops retrying them silently. */
+  const failedAnalyses = new Map<VideoId, string>();
+  /** The maze map and parameters those failures were recorded against. */
+  let failedInputs: { mazeMap: unknown; parameters: unknown } | null = null;
 
   const body = el('div', { class: 'review-step' });
 
@@ -995,16 +999,44 @@ export function createReviewStep(context: AppContext): Step {
    */
   function sweepMissingAnalyses(): void {
     if (sweeping) return;
+    // A derive that throws would otherwise be retried on every notification for
+    // ever, silently, while the figures said only "not analysed yet" and offered
+    // the Track step — which cannot help, because the video *is* tracked. The
+    // failures are remembered until an input changes, and the reason is shown.
+    const inputs = { mazeMap: store.current.mazeMap, parameters: store.current.parameters };
+    if (failedInputs?.mazeMap !== inputs.mazeMap || failedInputs?.parameters !== inputs.parameters) {
+      failedAnalyses.clear();
+      failedInputs = inputs;
+    }
     const stale = store.videos.some(
-      (video) => analysisBlockedReason(store, video.id) === null && !store.analysisFor(video.id)?.derived,
+      (video) =>
+        analysisBlockedReason(store, video.id) === null &&
+        !store.analysisFor(video.id)?.derived &&
+        !failedAnalyses.has(video.id),
     );
     if (!stale) return;
     sweeping = true;
+    let run;
     try {
-      analyseMissing(store);
+      run = analyseMissing(store);
     } finally {
       sweeping = false;
     }
+    for (const [videoId, reason] of run.skipped) {
+      // Only a throw, not "not tracked yet" — that one the Track step does fix.
+      if (analysisBlockedReason(store, videoId) !== null) continue;
+      if (failedAnalyses.has(videoId)) continue;
+      failedAnalyses.set(videoId, reason);
+      const name = store.videoById(videoId)?.filename ?? videoId;
+      context.announce(`${name} could not be analysed: ${reason}`);
+    }
+  }
+
+  /** The reason each failed video failed, in the words the figures show. */
+  function analysisProblems(): string[] {
+    return [...failedAnalyses].map(
+      ([videoId, reason]) => `${store.videoById(videoId)?.filename ?? videoId} — ${reason}`,
+    );
   }
 
   function render(): void {
@@ -1367,6 +1399,7 @@ export function createReviewStep(context: AppContext): Step {
     const figuresProps: ReviewFiguresProps = {
       session: store.current,
       videoId: currentVideo()?.id ?? null,
+      problems: analysisProblems(),
     };
     if (figures) {
       figures.update(figuresProps);

src/viz/figure-export.ts

@@ -107,7 +107,11 @@ function defaultColormapOf(figure: FigureSpec): ColormapName {
   return figure.id === 'heatmap' ? 'cividis' : 'viridis';
 }
 
-/** `4`, not `4.0`; `4.5` stays `4.5`. A filename should not carry a stray zero. */
+/**
+ * `4`, not `4.0`. `cellSizeCm` rounds to the option's step, so today every value
+ * reaching this is an integer; the `.` substitution keeps a fractional step from
+ * putting a second dot in the filename if one is ever offered.
+ */
 function trimNumber(value: number): string {
   return String(Number(value.toFixed(2))).replace('.', '-');
 }

src/viz/heatmap.ts

@@ -32,7 +32,7 @@ const TITLE = 'Occupancy heatmap';
  */
 export const DEFAULT_CELL_CM = 4;
 
-/** The bin sizes the UI offers; any positive number works. */
+/** The bin sizes on offer. `cellSizeCm` clamps to this range and rounds to the step. */
 export const CELL_CM_RANGE = { min: 1, max: 12, step: 1 };
 
 /**
@@ -51,10 +51,10 @@ function occupancyMap(name: ColormapName | undefined): Colormap {
  *
  * Clamped here rather than at the control, so every consumer agrees on one
  * value — the drawing, the `describe()` table and the PNG filename. The grid
- * allocates cells as the square of the span, so an unclamped 0.001 cm asks for
- * 3.4 million cells on a 46 cm platform and throws `Invalid array length` from
- * inside `draw()`; and an unrounded 4.001 and 4.002 would produce two different
- * figures under one filename.
+ * allocates cells as the square of the span, so on a platform 92 cm across an
+ * unclamped 0.001 cm asks for a span of 92,000 and 8.46e9 cells, which throws
+ * `Invalid array length` from inside `draw()`; and an unrounded 4.001 and 4.002
+ * would produce two different figures under one filename.
  */
 export function cellSizeCm(options: FigureOptions | undefined): number {
   const chosen = options?.heatmapCellSize_cm;

tests/browser/app-smoke.spec.ts

@@ -442,7 +442,8 @@ test.describe('the whole demo flow through the shipped UI', () => {
     // The export bundle carries the six files of D11.
     const [download] = await Promise.all([
       page.waitForEvent('download'),
-      page.getByRole('button', { name: /Download export bundle/i }).click(),
+      // Chunk 7b's button, which did not exist when this spec was written.
+      page.getByRole('button', { name: /Export bundle \(\.zip\)/i }).click(),
     ]);
     const zipPath = await download.path();
     const { readFileSync } = await import('node:fs');

tests/ui/review-figures.test.ts

@@ -21,6 +21,7 @@ import {
   previewRatio,
 } from '../../src/ui/review-figures.js';
 import { syntheticSession } from '../fixtures/synthetic-analysis.js';
+import { fakeContext } from '../viz/fake-context.js';
 
 function withOneUnanalysed(session: SessionFile): SessionFile {
   const last = session.videos[session.videos.length - 1]!;
@@ -201,7 +202,6 @@ describe('the mounted figures section', () => {
     expect(bin.value).toBe('1');
     expect(caption()).toContain('1 cm cell');
     expect(announce).toHaveBeenCalledWith(expect.stringContaining('outside the 1–12 cm range'));
-    // The cards after the heatmap are still current, not stale from before.
     const captions = [...container.querySelectorAll('.figure-caption')].filter(
       (node) => (node.textContent ?? '').trim().length > 0,
     );
@@ -260,6 +260,64 @@ describe('the mounted figures section', () => {
     expect(label()).not.toBe(before);
   });
 
+  it('reports a figure that throws on its own card, and still draws the rest', () => {
+    // happy-dom returns null from `getContext`, so nothing in this suite entered
+    // the draw path at all until this stub — the catch was dead code under test.
+    const session = syntheticSession();
+    const context = fakeContext();
+    const getContext = vi
+      .spyOn(HTMLCanvasElement.prototype, 'getContext')
+      .mockReturnValue(context as unknown as CanvasRenderingContext2D);
+    const broken = TRIAL_FIGURES[1]!;
+    const draw = vi.spyOn(broken, 'draw').mockImplementation(() => {
+      throw new Error('deliberate failure');
+    });
+    try {
+      createReviewFigures(container, { session, videoId: session.videos[0]!.id }, {
+        onAnnounce: announce,
+        onGoToTrack: goToTrack,
+      });
+
+      const cards = [...container.querySelectorAll('.figure-card')];
+      const failed = cards[1]!;
+      expect(failed.querySelector('h5')!.textContent).toBe(broken.title);
+      expect(failed.querySelector<HTMLElement>('.figure-note')!.hidden).toBe(false);
+      expect(failed.querySelector('.figure-note')!.textContent).toContain('deliberate failure');
+      expect(failed.querySelector('button')!.disabled).toBe(true);
+      expect(announce).toHaveBeenCalledWith(expect.stringContaining('deliberate failure'));
+
+      // The cards after it drew normally: one bad figure is not eight.
+      const after = cards[2]!;
+      expect(after.querySelector<HTMLElement>('.figure-note')!.hidden).toBe(true);
+      expect(after.querySelector('button')!.disabled).toBe(false);
+      expect(after.querySelector('.figure-caption')!.textContent).toBeTruthy();
+    } finally {
+      draw.mockRestore();
+      getContext.mockRestore();
+    }
+  });
+
+  it('names the videos it could not analyse instead of calling them untracked', () => {
+    const session = syntheticSession();
+    const figures = createReviewFigures(container, { session, videoId: session.videos[0]!.id }, {
+      onAnnounce: announce,
+      onGoToTrack: goToTrack,
+    });
+    const block = container.querySelector<HTMLElement>('.figure-missing-block')!;
+    const button = block.querySelector<HTMLButtonElement>('button')!;
+
+    figures.update({
+      session,
+      videoId: session.videos[0]!.id,
+      problems: ['test51.mp4 — the maze has no usable hole radius'],
+    });
+
+    expect(block.hidden).toBe(false);
+    expect(block.textContent).toContain('the maze has no usable hole radius');
+    // The Track step cannot fix a derive that throws, so it is not offered.
+    expect(button.hidden).toBe(true);
+  });
+
   it('takes its listeners with it when destroyed', () => {
     const session = syntheticSession();
     const figures = createReviewFigures(container, { session, videoId: session.videos[0]!.id }, {

tests/viz/figures.test.ts

@@ -138,8 +138,8 @@ describe('figure options in the PNG name (D53)', () => {
 
   it('refuses a bin size that would allocate millions of cells', () => {
     const heatmap = FIGURES.find((f) => f.id === 'heatmap')!;
-    // The grid is the square of the span, so 0.001 cm on a 46 cm platform asked
-    // for 3.4 million cells and threw `Invalid array length` from inside `draw`.
+    // The grid is the square of the span, so 0.001 cm on a platform 92 cm across
+    // asked for 8.46e9 cells and threw `Invalid array length` from inside `draw`.
     expect(cellSizeCm({ heatmapCellSize_cm: 0.001 })).toBe(CELL_CM_RANGE.min);
     expect(cellSizeCm({ heatmapCellSize_cm: 1e6 })).toBe(CELL_CM_RANGE.max);
     expect(() => heatmap.describe(data, { heatmapCellSize_cm: 0.001 })).not.toThrow();