From dfe31c47f75994421ceab0eb6d125d772e708ffe Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 10 Aug 2026 17:59:59 +0000 Subject: [PATCH 1/3] fix: one click on the footage should mark the child for the game MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production has an athlete bound to 2 tracks out of 1125, and one suggested moment, in a game they played all of. Three separate causes, all measured against that project's own data. Stitching a pick into the fragments either side of it has existed since the appearance matcher landed, but the only route to it was the candidate grid, which offers proposals to tick. The scrubber — where people actually identify, because pointing at your own child needs no explanation — bound the one track under the cursor and stopped. It now asks the server to follow them, and the server accepts the same proposals the grid pre-ticks. Best-effort: a failure to widen must never lose the pick, which is the part the user made. The ball, second. Only a confident detection may open a track; a faint one can extend a track that already exists. So per-class floors were half a fix — a 0.2 ball landed in the low pile, where it could only attach to a ball track lucky enough to have started at 0.4, and there usually was none. That is exactly the shape the floor sweep showed: positions up 69%, track count flat. Track birth now honours each class's own floor, and nothing else moves: a weak *player* detection still cannot start a track. Third, geometry. Overlap is the wrong question to ask about a thrown ball. It is a small box that clears several of its own widths between sampled frames, and boxes that do not touch have an IoU of exactly zero however close they are — so association fails, the track dies, and a new one is born further along the same flight. Both boxes are now grown before overlap is measured, per class, which leaves everyone with a buffer of 0 associating on identical geometry. Exposed as --class-buffer for the same reason the floors are: the value that belongs there is a measurement, not an opinion. Identity is collected where the user is already looking at the child, so the colour field is reachable from the surface they use — production has a kid recorded as team "Triton (white)" because the only form offering a colour was one they never reached. And the empty state no longer says "run analysis" while analysis is running, which was the page telling you to act when the only right move was to wait. Co-Authored-By: Claude Opus 5 --- apps/cv-worker/src/ballfloor.test.ts | 26 ++++- apps/cv-worker/src/ballrecall.test.ts | 149 ++++++++++++++++++++++++++ apps/cv-worker/src/index.ts | 25 ++++- apps/cv-worker/src/pipeline.ts | 22 ++++ apps/cv-worker/src/tracker.ts | 85 ++++++++++++--- apps/web/src/actions.ts | 38 ++++++- apps/web/src/client/review.tsx | 82 +++++++++++++- apps/web/src/expandonbind.test.ts | 144 +++++++++++++++++++++++++ apps/web/src/views/pages.tsx | 13 ++- 9 files changed, 561 insertions(+), 23 deletions(-) create mode 100644 apps/cv-worker/src/ballrecall.test.ts create mode 100644 apps/web/src/expandonbind.test.ts diff --git a/apps/cv-worker/src/ballfloor.test.ts b/apps/cv-worker/src/ballfloor.test.ts index 2755eb3..16105c8 100644 --- a/apps/cv-worker/src/ballfloor.test.ts +++ b/apps/cv-worker/src/ballfloor.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { parseClassConfidence } from './index.js'; +import { parseClassBuffer, parseClassConfidence } from './index.js'; /** * A basketball is a handful of pixels and the model is right to be unsure about @@ -58,3 +58,27 @@ describe('per-class detection floors', () => { expect(parseClassConfidence('ball=-0.2')).toEqual({}); }); }); + +/** + * Buffers are a distance, not a probability, so they share the parser but not + * its ceiling — the useful values for a basketball are greater than 1, which is + * the whole reason the plain overlap test fails on it. + */ +describe('per-class association buffers', () => { + it('accepts the values a ball actually needs', () => { + expect(parseClassBuffer('ball=1.5')).toEqual({ ball: 1.5 }); + expect(parseClassBuffer('ball=2.5,puck=1.5')).toEqual({ ball: 2.5, puck: 1.5 }); + }); + + it('still refuses nonsense', () => { + expect(parseClassBuffer('ball=0')).toEqual({}); + expect(parseClassBuffer('ball=-1')).toEqual({}); + expect(parseClassBuffer('ball=abc')).toEqual({}); + // Far enough that every ball on court would match every other one. + expect(parseClassBuffer('ball=99')).toEqual({}); + }); + + it('is absent rather than empty when nothing is asked for', () => { + expect(parseClassBuffer(undefined)).toEqual({}); + }); +}); diff --git a/apps/cv-worker/src/ballrecall.test.ts b/apps/cv-worker/src/ballrecall.test.ts new file mode 100644 index 0000000..a57084e --- /dev/null +++ b/apps/cv-worker/src/ballrecall.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it } from 'vitest'; + +import type { Detection } from './geometry.js'; +import { ByteTracker, inflate } from './tracker.js'; + +/** + * Why the ball is hardly ever identified. + * + * Per-class confidence floors let a faint ball through the filter, and ball + * *positions* rose 69% — but the number of ball tracks barely moved, which is + * the tell. Only a confident detection may open a track; a faint one can extend + * a track that already exists. So every extra ball detection attached itself to + * the handful of tracks lucky enough to have started at 0.4, and the rest were + * discarded for having nothing to attach to. + * + * The second half is geometry. Overlap is a poor question to ask about a thrown + * ball: it is a small box that clears several of its own widths between sampled + * frames, and boxes that do not touch have an IoU of exactly zero however + * close they are. + */ + +const det = (x: number, y: number, w: number, h: number, score: number, classId = 0): Detection => ({ + x, + y, + w, + h, + score, + classId, +}); + +const names = { 0: 'player', 32: 'ball' }; + +describe('starting a track at the class its own floor', () => { + it('opens a ball track from detections no player detection could open', () => { + const tracker = new ByteTracker({ + minLength: 1, + highThreshold: 0.5, + lowThreshold: 0.05, + classHighThreshold: { ball: 0.08 }, + }); + // A ball seen faintly and steadily — never once at the global bar of 0.5. + for (let f = 0; f < 6; f += 1) { + tracker.update([det(100 + f * 3, 200, 20, 20, 0.2, 32)], names, f, f / 30); + } + + const tracks = tracker.results(); + expect(tracks).toHaveLength(1); + expect(tracks[0]?.className).toBe('ball'); + expect(tracks[0]?.points).toHaveLength(6); + }); + + it('still refuses to open a player track from the same weak evidence', () => { + const tracker = new ByteTracker({ + minLength: 1, + highThreshold: 0.5, + lowThreshold: 0.05, + classHighThreshold: { ball: 0.08 }, + }); + for (let f = 0; f < 6; f += 1) { + tracker.update([det(100 + f * 3, 200, 40, 90, 0.2, 0)], names, f, f / 30); + } + // The floor is the ball's alone: loosening it must not loosen people. + expect(tracker.results()).toHaveLength(0); + }); + + it('leaves a class with no override on the global bar', () => { + const tracker = new ByteTracker({ minLength: 1, highThreshold: 0.5, lowThreshold: 0.05 }); + for (let f = 0; f < 6; f += 1) { + tracker.update([det(100 + f * 3, 200, 20, 20, 0.2, 32)], names, f, f / 30); + } + expect(tracker.results()).toHaveLength(0); + }); +}); + +describe('buffered overlap for something small and fast', () => { + it('grows a box by a fraction of its own size, and leaves a zero buffer alone', () => { + const original = { x: 100, y: 100, w: 20, h: 20 }; + expect(inflate(original, 0)).toEqual(original); + expect(inflate(original, 0.5)).toEqual({ x: 90, y: 90, w: 40, h: 40 }); + }); + + it('follows a ball that clears its own width between frames as one flight', () => { + const tracker = new ByteTracker({ + minLength: 1, + highThreshold: 0.5, + lowThreshold: 0.05, + classHighThreshold: { ball: 0.08 }, + classBuffer: { ball: 1.5 }, + }); + // 20px wide, moving 30px a frame: consecutive boxes never touch, so plain + // IoU is zero at every step. + for (let f = 0; f < 8; f += 1) { + tracker.update([det(100 + f * 30, 200, 20, 20, 0.3, 32)], names, f, f / 30); + } + + const tracks = tracker.results(); + expect(tracks).toHaveLength(1); + expect(tracks[0]?.points).toHaveLength(8); + }); + + it('is what makes the difference — the same flight without a buffer shatters', () => { + const tracker = new ByteTracker({ + minLength: 1, + highThreshold: 0.5, + lowThreshold: 0.05, + classHighThreshold: { ball: 0.08 }, + }); + for (let f = 0; f < 8; f += 1) { + tracker.update([det(100 + f * 30, 200, 20, 20, 0.3, 32)], names, f, f / 30); + } + + // Every frame starts a new track: this is the scatter seen in production. + expect(tracker.results().length).toBeGreaterThan(1); + }); + + it('does not let a buffered ball steal the ball beside it', () => { + const tracker = new ByteTracker({ + minLength: 1, + highThreshold: 0.5, + lowThreshold: 0.05, + classHighThreshold: { ball: 0.08 }, + classBuffer: { ball: 1.5 }, + }); + // Two balls, far apart, moving in opposite directions. Buffering must widen + // the net, not merge distinct objects. + for (let f = 0; f < 8; f += 1) { + tracker.update( + [det(100 + f * 20, 200, 20, 20, 0.3, 32), det(900 - f * 20, 600, 20, 20, 0.3, 32)], + names, + f, + f / 30, + ); + } + expect(tracker.results()).toHaveLength(2); + }); + + it('leaves people associating on exactly the geometry they always did', () => { + const withBuffer = new ByteTracker({ minLength: 1, classBuffer: { ball: 1.5 } }); + const without = new ByteTracker({ minLength: 1 }); + for (let f = 0; f < 10; f += 1) { + const frame = [det(100 + f * 5, 200, 40, 90, 0.9), det(300 - f * 5, 210, 40, 90, 0.85)]; + withBuffer.update(frame, names, f, f / 30); + without.update(frame, names, f, f / 30); + } + expect(withBuffer.results().map((t) => t.points.length)).toEqual( + without.results().map((t) => t.points.length), + ); + }); +}); diff --git a/apps/cv-worker/src/index.ts b/apps/cv-worker/src/index.ts index 5b953e8..9b29e0b 100644 --- a/apps/cv-worker/src/index.ts +++ b/apps/cv-worker/src/index.ts @@ -43,25 +43,41 @@ const number = (value: string | undefined, fallback: number): number => { }; /** - * `ball=0.08,hoop=0.15` — per-class detection floors. + * `ball=0.08,hoop=0.15` — a per-class number, whatever the number means. * * Malformed pairs are dropped rather than throwing: a typo in a preset should * cost the ball some recall, not take a twenty-minute detection pass down with * it. */ -export const parseClassConfidence = (value: string | undefined): Record => { +const parseClassNumbers = (value: string | undefined, max: number): Record => { if (value === undefined || value.trim().length === 0) return {}; const out: Record = {}; for (const pair of value.split(',')) { const [name, raw] = pair.split('='); const parsed = Number(raw); if (name === undefined || name.trim().length === 0) continue; - if (!Number.isFinite(parsed) || parsed <= 0 || parsed > 1) continue; + if (!Number.isFinite(parsed) || parsed <= 0 || parsed > max) continue; out[name.trim()] = parsed; } return out; }; +/** Per-class detection floors. A confidence cannot exceed 1. */ +export const parseClassConfidence = (value: string | undefined): Record => + parseClassNumbers(value, 1); + +/** + * Per-class association buffers, as a fraction of the box's own size. + * + * Exposed as a flag for the same reason the floors are: the value that belongs + * here is a measurement, not an opinion, and a constant compiled into the + * pipeline cannot be swept against real footage without rebuilding it. Allowed + * well above 1 — a basketball travels several of its own widths between sampled + * frames, which is the entire problem being solved. + */ +export const parseClassBuffer = (value: string | undefined): Record => + parseClassNumbers(value, 8); + /** * How many threads onnxruntime should use for one inference. * @@ -187,6 +203,9 @@ const detectAndTrack = async (flags: Record): Promise => { tileGrid: number(flags['tile-grid'], 1), minConfidence: number(flags['min-confidence'], 0.3), classConfidence: parseClassConfidence(flags['class-confidence']), + ...(flags['class-buffer'] === undefined + ? {} + : { classBuffer: parseClassBuffer(flags['class-buffer']) }), iouThreshold: number(flags['iou'], 0.45), sourceWidth: width, sourceHeight: height, diff --git a/apps/cv-worker/src/pipeline.ts b/apps/cv-worker/src/pipeline.ts index b741e89..6154370 100644 --- a/apps/cv-worker/src/pipeline.ts +++ b/apps/cv-worker/src/pipeline.ts @@ -51,6 +51,8 @@ export interface PipelineOptions { * detections are the same ball seen more often rather than new phantoms. */ classConfidence?: Record; + /** Per-class association buffer; omitted means the built-in table. */ + classBuffer?: Record; iouThreshold: number; sourceWidth: number; sourceHeight: number; @@ -72,6 +74,18 @@ export interface PipelineResult { unsupportedClasses: string[]; } +/** + * How far each class may move, relative to its own size, and still be + * recognised as the same object on the next sampled frame. + * + * People are left at 0: a player cannot cross their own width in a frame, so + * plain overlap already answers the question and buffering would only invite + * one player's box to capture the player beside them. A ball is the opposite — + * small, and fast enough to clear several of its own widths — which is why it + * arrives as a scatter of short tracks rather than a flight. + */ +const SMALL_FAST: Record = { ball: 1.5, puck: 1.5 }; + export const createSession = async ( modelPath: string, threads: number, @@ -203,6 +217,14 @@ export const runPipeline = async (options: PipelineOptions): Promise; + /** + * Per-class association buffer, keyed by class name: both boxes are grown by + * this fraction of their own size before overlap is measured. + * + * Overlap is the wrong question for a thrown ball. A basketball is a small + * box that routinely travels more than its own width between sampled frames, + * and two boxes that do not touch have an IoU of exactly zero however close + * they are — so association fails, the track dies, and a new one is born + * further along the same flight. Growing both boxes first (buffered IoU) + * restores a gradient for objects that move far relative to their size, and + * leaves anything with a buffer of 0 — every person on court — untouched. + */ + classBuffer: Record; } export const DEFAULT_TRACKER_OPTIONS: TrackerOptions = { @@ -52,6 +79,8 @@ export const DEFAULT_TRACKER_OPTIONS: TrackerOptions = { iouThreshold: 0.2, maxAge: 30, minLength: 3, + classHighThreshold: {}, + classBuffer: {}, }; const centre = (box: Box): { x: number; y: number } => ({ @@ -73,6 +102,30 @@ interface Pair { score: number; } +const box = (detection: Detection): Box => ({ + x: detection.x, + y: detection.y, + w: detection.w, + h: detection.h, +}); + +/** + * Grows a box by a fraction of its own size on every side. + * + * A buffer of 0 returns the box unchanged, which is the point: buffering is + * opt-in per class, so classes that do not ask for it associate on exactly the + * geometry they always did. + */ +export const inflate = (target: Box, by: number): Box => + by <= 0 + ? target + : { + x: target.x - target.w * by, + y: target.y - target.h * by, + w: target.w * (1 + 2 * by), + h: target.h * (1 + 2 * by), + }; + /** * Greedy IoU association. Hungarian would be optimal, but greedy-by-IoU is * within noise for this many objects and is far easier to reason about when a @@ -82,14 +135,16 @@ const associate = ( tracks: Track[], detections: Detection[], iouThreshold: number, + bufferFor: (track: Track) => number = () => 0, ): { matches: Pair[]; unmatchedTracks: number[]; unmatchedDetections: number[] } => { const candidates: Pair[] = []; tracks.forEach((track, trackIndex) => { - const predicted = predict(track); + const buffer = bufferFor(track); + const predicted = inflate(predict(track), buffer); detections.forEach((detection, detectionIndex) => { if (detection.classId !== track.classId) return; - const score = iou(predicted, detection); + const score = iou(predicted, inflate(box(detection), buffer)); if (score >= iouThreshold) candidates.push({ trackIndex, detectionIndex, score }); }); }); @@ -152,12 +207,24 @@ export class ByteTracker { /** Feeds one frame of detections. */ update(detections: Detection[], classNames: Record, frame: number, ts: number): void { + /** + * The bar this detection has to clear to be treated as confident — and so + * to be allowed to start a track, not merely extend one. + */ + const barFor = (detection: Detection): number => { + const className = classNames[detection.classId]; + const override = + className === undefined ? undefined : this.options.classHighThreshold[className]; + return override ?? this.options.highThreshold; + }; + const bufferFor = (track: Track): number => this.options.classBuffer[track.className] ?? 0; + const usable = detections.filter((d) => d.score >= this.options.lowThreshold); - const high = usable.filter((d) => d.score >= this.options.highThreshold); - const low = usable.filter((d) => d.score < this.options.highThreshold); + const high = usable.filter((d) => d.score >= barFor(d)); + const low = usable.filter((d) => d.score < barFor(d)); // Pass 1: confident detections against every live track. - const first = associate(this.active, high, this.options.iouThreshold); + const first = associate(this.active, high, this.options.iouThreshold, bufferFor); for (const match of first.matches) { const track = this.active[match.trackIndex]; const detection = high[match.detectionIndex]; @@ -167,7 +234,7 @@ export class ByteTracker { // Pass 2: the BYTE step — try the leftovers against tracks that missed out, // with a looser bar, so an occluded player is rescued rather than lost. const stranded = first.unmatchedTracks.map((i) => this.active[i]).filter((t): t is Track => t !== undefined); - const second = associate(stranded, low, this.options.iouThreshold * 0.75); + const second = associate(stranded, low, this.options.iouThreshold * 0.75, bufferFor); const rescued = new Set(); for (const match of second.matches) { const track = stranded[match.trackIndex]; @@ -208,9 +275,3 @@ export class ByteTracker { } } -const box = (detection: Detection): Box => ({ - x: detection.x, - y: detection.y, - w: detection.w, - h: detection.h, -}); diff --git a/apps/web/src/actions.ts b/apps/web/src/actions.ts index 5bd10b6..6676c58 100644 --- a/apps/web/src/actions.ts +++ b/apps/web/src/actions.ts @@ -697,6 +697,7 @@ export const registerActions = (app: Hono): void => { jerseyNumber?: string; team?: string; jerseyColor?: string; + expand?: boolean; }) : { trackId: field(await c.req.parseBody(), 'trackId') }; @@ -789,9 +790,44 @@ export const registerActions = (app: Hono): void => { : requestedIds; const assigned = await assignTracksToAthlete(root, athleteId, finalIds); + /** + * Follow them through the rest of the game, if the caller cannot. + * + * Stitching one pick into the fragments either side of it has existed + * since the appearance matcher landed, but the only way to reach it was + * the candidate grid, which offers proposals to tick. The scrubber — the + * surface people actually use, because pointing at your child on the + * footage needs no explanation — bound the single track under the cursor + * and stopped. Production shows exactly that: an athlete on 2 tracks out + * of 1125, and one suggested moment, in a game they play the whole of. + * + * The proposals are the same ones the grid pre-ticks for confirmation, so + * accepting them here is the behaviour that surface already had. It is + * best-effort: a failure to expand must never lose the pick itself, which + * is the part the user made and the part scoring cannot do without. + */ + let added: string[] = []; + if (body.expand === true) { + try { + const found = await proposeAthleteTracks(root, athleteId, {}); + added = found.proposals.map((proposal) => proposal.trackId); + if (added.length > 0) { + // The whole set, not the additions: assigning is a replace, and it + // clears the athlete's existing rows before it writes. + await assignTracksToAthlete(root, athleteId, [...new Set([...finalIds, ...added])]); + } + } catch { + // No worker, no proxy, nothing to link to — all survivable. The pick + // stands, and the grid can still be used to widen it by hand. + added = []; + } + } + startAnalysis(root, { preset: 'balanced', scoreOnly: true }); - if (prefersJson(c)) return c.json({ ok: true, athleteId, trackId, assigned }); + if (prefersJson(c)) { + return c.json({ ok: true, athleteId, trackId, assigned, added: added.length }); + } return back(c, to, 'Athlete identified — re-scoring with them as the focus'); } catch (error) { if (prefersJson(c)) return uploadJson(c, error); diff --git a/apps/web/src/client/review.tsx b/apps/web/src/client/review.tsx index d1e64ff..f0d1ad1 100644 --- a/apps/web/src/client/review.tsx +++ b/apps/web/src/client/review.tsx @@ -113,8 +113,51 @@ const attach = (node: HTMLElement): void => { identify.type = 'button'; identify.textContent = 'Identify my athlete'; controls.appendChild(identify); + + /** + * Who this is, asked where the user is already looking at them. + * + * A number alone does not name a child — both teams field a 14 and they are + * on court together — so the shirt colour is the part a parent actually uses. + * Asking here rather than on a separate panel is the difference between the + * fields being filled in and being skipped: production has a child recorded + * as team "Triton (white)" because the only form that offered a colour was + * one the user never reached, so they crammed it into the team box. + */ + const field = (placeholder: string, width: string): HTMLInputElement => { + const input = document.createElement('input'); + input.type = 'text'; + input.placeholder = placeholder; + input.style.cssText = `width:${width};flex:none`; + controls.appendChild(input); + return input; + }; + const nameInput = field('Name', '8rem'); + const numberInput = field('#', '3.5rem'); + const colorInput = field('Shirt colour', '7rem'); + const teamInput = field('Team', '7rem'); + + /** Only send what was actually typed; blank fields must not erase a name. */ + const identity = (): Record => { + const parts: Record = {}; + if (nameInput.value.trim() !== '') parts['name'] = nameInput.value.trim(); + if (numberInput.value.trim() !== '') parts['jerseyNumber'] = numberInput.value.trim(); + if (colorInput.value.trim() !== '') parts['jerseyColor'] = colorInput.value.trim(); + if (teamInput.value.trim() !== '') parts['team'] = teamInput.value.trim(); + return parts; + }; + + /** Shown only while a bind is in flight, because that one really does wait. */ + const spinner = document.createElement('progress'); + spinner.style.cssText = 'display:none;width:8rem;flex:none'; + controls.appendChild(spinner); node.appendChild(controls); + const setPending = (on: boolean): void => { + spinner.style.display = on ? 'block' : 'none'; + identify.disabled = on; + }; + const status = document.createElement('p'); status.className = 'muted'; node.appendChild(status); @@ -224,24 +267,43 @@ const attach = (node: HTMLElement): void => { const bind = async (trackId: string): Promise => { if (busy) return; busy = true; - status.textContent = 'Binding…'; + /** + * Expanding decodes frames, so this is the one click on the page with a + * real wait behind it. Say what is happening rather than leaving a dead + * button: not knowing whether to wait or click again is what produced eight + * athletes in three minutes. + */ + setPending(true); + status.textContent = 'Following them through the rest of the game…'; try { const response = await fetch(bindUrl, { method: 'POST', headers: { accept: 'application/json', 'content-type': 'application/json' }, - body: JSON.stringify({ trackId, trackIds: [trackId] }), + body: JSON.stringify({ + trackId, + trackIds: [trackId], + // One click should mark the child everywhere they appear from here, + // not only in the fragment under the cursor. + expand: true, + ...identity(), + }), }); - const body = (await response.json()) as { ok: boolean; error?: string }; + const body = (await response.json()) as { ok: boolean; error?: string; added?: number }; if (!response.ok || !body.ok) throw new Error(body.error ?? 'Could not bind that track.'); // Re-fetch so the box turns green without a reload. windowStart = Number.NaN; await ensure(video.currentTime); draw(); setIdentifying(false); - status.textContent = 'Identified — re-scoring now. Suggested moments will update.'; + const added = body.added ?? 0; + status.textContent = + (added > 0 + ? `Identified, and followed them into ${added} more fragment(s) of the game. ` + : 'Identified. ') + 'Re-scoring now — suggested moments will update on their own.'; } catch (cause) { status.textContent = cause instanceof Error ? cause.message : String(cause); } finally { + setPending(false); busy = false; } }; @@ -279,5 +341,15 @@ const attach = (node: HTMLElement): void => { export const mountReview = (): void => { const node = document.getElementById('review-surface'); - if (node !== null) attach(node); + if (node === null) return; + /** + * Attach once. This is called again after a live region is swapped, and + * attaching twice appends a second canvas, a second set of fields and a + * second Identify button over the same video — the duplicate-surface bug in + * miniature, and the kind that only shows up after the page has been open + * long enough for something to refresh. + */ + if (node.dataset['mounted'] === 'true') return; + node.dataset['mounted'] = 'true'; + attach(node); }; diff --git a/apps/web/src/expandonbind.test.ts b/apps/web/src/expandonbind.test.ts new file mode 100644 index 0000000..7316703 --- /dev/null +++ b/apps/web/src/expandonbind.test.ts @@ -0,0 +1,144 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { Hono } from 'hono'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +/** + * One click on the footage should mark the child for the game, not the second. + * + * Stitching a pick into the fragments either side of it shipped with the + * appearance matcher, but the only route to it was the candidate grid, which + * offers proposals to tick. The scrubber — where people actually identify, + * because pointing at your own child needs no explanation — bound the one track + * under the cursor and stopped. Production shows the consequence exactly: an + * athlete on 2 tracks out of 1125, and one suggested moment, in a game they + * played all of. + * + * Expansion needs a CV worker and real frames, so what is asserted here is the + * contract around it: the request is accepted, the pick itself is never lost to + * a failure to widen it, and the flag is what decides whether widening is even + * attempted. + */ + +let home: string; +let root: string; +let app: Hono; + +beforeAll(async () => { + home = mkdtempSync(path.join(tmpdir(), 'reeleel-expand-')); + process.env['REELEEL_HOME'] = home; + + const { createProject } = await import('@reeleel/core'); + const created = await createProject({ + name: 'expand', + path: path.join(home, 'projects', 'expand'), + sport: 'basketball', + }); + root = created.path ?? created.root; + + const { execute, projectDb, createTrack } = await import('@reeleel/core'); + const db = await projectDb(root); + const now = new Date().toISOString(); + await execute( + db, + 'INSERT INTO source_videos (id, project_id, path, created_at, updated_at) VALUES (?, ?, ?, ?, ?)', + ['vid_a', 'prj_test', '/tmp/vid_a.mp4', now, now], + ); + for (let i = 0; i < 4; i += 1) { + await createTrack(root, { + videoId: 'vid_a', + className: 'player', + confidence: 0.9, + samples: [ + { ts: i * 10, frame: i * 300, x: 1, y: 2, w: 3, h: 4, confidence: 0.9 }, + { ts: i * 10 + 5, frame: i * 300 + 150, x: 1, y: 2, w: 3, h: 4, confidence: 0.9 }, + ], + }); + } + + const { registerActions } = await import('./actions.js'); + app = new Hono(); + registerActions(app); +}); + +afterAll(async () => { + const { resetDbCache } = await import('@reeleel/core'); + resetDbCache(); + rmSync(home, { recursive: true, force: true }); + delete process.env['REELEEL_HOME']; +}); + +const post = async (body: unknown): Promise => + app.request(`/projects/${encodeURIComponent(root)}/athletes/new/track`, { + method: 'POST', + headers: { accept: 'application/json', 'content-type': 'application/json' }, + body: JSON.stringify(body), + }); + +describe('identifying from the scrubber', () => { + it('keeps the pick even when the athlete cannot be followed any further', async () => { + const { listTracks, listAthletes, tracksForAthlete } = await import('@reeleel/core'); + const tracks = (await listTracks(root, 'vid_a')).map((track) => track.id); + + // There is no video behind this fixture, so expansion cannot succeed. The + // pick is the part the user made; losing it to a failed widening would be + // strictly worse than never widening at all. + const response = await post({ trackId: tracks[0], trackIds: [tracks[0]], expand: true }); + expect(response.status).toBe(200); + const body = (await response.json()) as { ok: boolean; added: number }; + expect(body.ok).toBe(true); + expect(body.added).toBe(0); + + const athletes = await listAthletes(root); + expect(athletes).toHaveLength(1); + expect(await tracksForAthlete(root, athletes[0]!.id)).toEqual([tracks[0]]); + }); + + it('records who they are, colour and all, from the same click', async () => { + const { listTracks, listAthletes } = await import('@reeleel/core'); + const tracks = (await listTracks(root, 'vid_a')).map((track) => track.id); + + await post({ + trackId: tracks[1], + trackIds: [tracks[1]], + expand: true, + name: 'Fred', + jerseyNumber: '14', + jerseyColor: 'white', + team: 'Triton', + }); + + const athlete = (await listAthletes(root))[0]!; + expect(athlete.name).toBe('Fred'); + expect(athlete.jerseyNumber).toBe('14'); + // The field that has existed since the first migration and was never written. + expect(athlete.jerseyColor).toBe('white'); + expect(athlete.team).toBe('Triton'); + }); + + it('still accumulates picks rather than replacing them', async () => { + const { listTracks, listAthletes, tracksForAthlete } = await import('@reeleel/core'); + const tracks = (await listTracks(root, 'vid_a')).map((track) => track.id); + + await post({ trackId: tracks[2], trackIds: [tracks[2]], expand: true }); + + const athlete = (await listAthletes(root))[0]!; + const bound = await tracksForAthlete(root, athlete.id); + // Three clicks, three fragments, one child — widening must not undo the + // de-duplication that made repeat clicks safe in the first place. + expect(bound.sort()).toEqual(tracks.slice(0, 3).sort()); + expect(await listAthletes(root)).toHaveLength(1); + }); + + it('leaves a caller that did not ask to expand exactly as it was', async () => { + const { listTracks } = await import('@reeleel/core'); + const tracks = (await listTracks(root, 'vid_a')).map((track) => track.id); + + const response = await post({ trackId: tracks[3], trackIds: [tracks[3]] }); + const body = (await response.json()) as { ok: boolean; added: number }; + expect(body.ok).toBe(true); + expect(body.added).toBe(0); + }); +}); diff --git a/apps/web/src/views/pages.tsx b/apps/web/src/views/pages.tsx index f73d937..3fb9735 100644 --- a/apps/web/src/views/pages.tsx +++ b/apps/web/src/views/pages.tsx @@ -445,7 +445,18 @@ export const ProjectPage: FC = ({ game, and click your athlete to identify them.

{moments.length === 0 ? ( -

Nothing suggested yet — run analysis.

+ /* "Run analysis" while analysis is running is the page telling you to + act when the only right move is to wait — and acting again is what + produced eight athletes in three minutes. The empty state has to know + which of the two situations it is in. */ + jobs.some((job) => job.status === 'running' || job.status === 'queued') ? ( +

+ Working on it — suggested + moments will appear here on their own. +

+ ) : ( +

Nothing suggested yet — run analysis.

+ ) ) : ( <>

From 8441480bd3781f8cf4f91f60e5d5b0d3307aba9b Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 10 Aug 2026 18:04:03 +0000 Subject: [PATCH 2/3] fix: re-hydrate the moment list, not the scrubber on another page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live-region refresh re-mounted the review scrubber after swapping data-live="moments". The scrubber is not in that region — it is not on that page — while the island that *is*, the interactive Keep/Reject list, was never re-mounted at all. Both halves fail silently. The moment list came back as the server rendered it, looking exactly right, with nothing behind its buttons; and it came back that way at the worst possible moment, since that region is swapped precisely when a job finishes and new suggestions appear. Meanwhile a page holding both would have attached a second canvas and a second set of controls over the same video. The list moves into its own module so the entry point and the refresh can share one mount, the map points at it, and mountReview refuses to attach twice. Co-Authored-By: Claude Opus 5 --- apps/web/src/client/live.ts | 16 +++- apps/web/src/client/main.tsx | 124 ++---------------------------- apps/web/src/client/moments.tsx | 129 ++++++++++++++++++++++++++++++++ apps/web/src/remount.test.ts | 52 +++++++++++++ 4 files changed, 201 insertions(+), 120 deletions(-) create mode 100644 apps/web/src/client/moments.tsx create mode 100644 apps/web/src/remount.test.ts diff --git a/apps/web/src/client/live.ts b/apps/web/src/client/live.ts index 9d9b730..28afcb8 100644 --- a/apps/web/src/client/live.ts +++ b/apps/web/src/client/live.ts @@ -1,4 +1,4 @@ -import { mountReview } from './review.js'; +import { mountMoments } from './moments.js'; /** * Bringing the page up to date without reloading it. @@ -22,9 +22,17 @@ import { mountReview } from './review.js'; * scrolls. */ -/** Islands that live inside a swappable region and must be re-mounted after it. */ -const REMOUNT: Record void> = { - moments: mountReview, +/** + * Islands that live inside a swappable region and must be re-mounted after it. + * + * `moments` holds `#moment-review`, and it must be that island — not the review + * scrubber, which lives on another page entirely and inside no region at all. + * Pointing this at the scrubber meant the moment list was re-rendered as inert + * server markup, with nothing behind Keep or Reject, at exactly the point a + * finished job swapped new suggestions in. + */ +export const REMOUNT: Record void> = { + moments: mountMoments, }; let inFlight: Promise | null = null; diff --git a/apps/web/src/client/main.tsx b/apps/web/src/client/main.tsx index 84650ad..611c04d 100644 --- a/apps/web/src/client/main.tsx +++ b/apps/web/src/client/main.tsx @@ -1,114 +1,22 @@ -/** @jsxImportSource hono/jsx/dom */ -import { render, useState } from 'hono/jsx/dom'; - import { mountIdentify } from './identify.js'; +import { mountMoments } from './moments.js'; import { mountOverlays } from './overlay.js'; import { mountReview } from './review.js'; import { mountJobLog } from './jobs.js'; import { mountUploads } from './upload.js'; /** - * The SPA half of the app: one island that takes over the server-rendered - * moment list and makes accept/reject interactive. Everything else stays plain - * SSR, which keeps the page useful with JavaScript disabled. + * The client entry point: mount every island the page happens to contain. + * + * Each mount looks for its own anchor and returns quietly if the page has none, + * so this one list serves every page. Everything not listed here stays plain + * SSR, which keeps the app usable with JavaScript disabled. */ -interface Moment { - id: string; - start: number; - end: number; - score: number; - reasons: string[]; - included: boolean | null; - favorite: boolean; -} - -const duration = (seconds: number): string => { - const total = Math.max(0, Math.round(seconds)); - return `${Math.floor(total / 60)}:${String(total % 60).padStart(2, '0')}`; -}; - -const decision = (included: boolean | null): string => - included === true ? 'keep' : included === false ? 'reject' : 'undecided'; - -const MomentReview = ({ projectId, initial }: { projectId: string; initial: Moment[] }) => { - const [moments, setMoments] = useState(initial); - const [busy, setBusy] = useState(null); - const [error, setError] = useState(null); - - const decide = async (moment: Moment, included: boolean | null): Promise => { - setBusy(moment.id); - setError(null); - try { - const response = await fetch( - `/api/projects/${encodeURIComponent(projectId)}/moments/${moment.id}`, - { - method: 'PATCH', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ included }), - }, - ); - const payload = (await response.json()) as { ok: boolean; moment?: Moment; error?: string }; - if (!response.ok || !payload.ok || payload.moment === undefined) { - throw new Error(payload.error ?? `Request failed (${response.status})`); - } - const updated = payload.moment; - setMoments((current) => current.map((m) => (m.id === updated.id ? updated : m))); - } catch (cause) { - // Leave the previous state alone so the user can retry without losing work. - setError(cause instanceof Error ? cause.message : String(cause)); - } finally { - setBusy(null); - } - }; - - const kept = moments.filter((m) => m.included === true).length; - - return ( -

-

- {kept} of {moments.length} kept -

- {error === null ? null :

{error}

} - {moments.map((moment) => ( -
-
- - {duration(moment.start)} → {duration(moment.end)} - - score {moment.score.toFixed(2)} - {moment.reasons.join(', ')} - - {decision(moment.included)} - - - -
-
- ))} -
- ); -}; - 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(); mountIdentify(); @@ -117,24 +25,8 @@ const mount = (): void => { mountOverlays(); // The review surface: every track over the whole game, click to identify. mountReview(); - - const node = document.getElementById('moment-review'); - if (node === null) return; - - const projectId = node.dataset['project']; - const raw = node.dataset['moments']; - if (projectId === undefined || raw === undefined) return; - - let initial: Moment[]; - try { - initial = JSON.parse(raw) as Moment[]; - } catch { - // Bad payload: keep the server-rendered list rather than blanking the page. - return; - } - - node.innerHTML = ''; - render(, node); + // The moment list, which live.ts also re-mounts after a refresh. + mountMoments(); }; mount(); diff --git a/apps/web/src/client/moments.tsx b/apps/web/src/client/moments.tsx new file mode 100644 index 0000000..57e0f1f --- /dev/null +++ b/apps/web/src/client/moments.tsx @@ -0,0 +1,129 @@ +/** @jsxImportSource hono/jsx/dom */ +import { render, useState } from 'hono/jsx/dom'; + +/** + * The moment list, made interactive. + * + * This lives in its own module rather than in the client entry point because + * two callers need it: the entry mounts it on load, and live.ts re-mounts it + * after swapping the region it sits in. That region is swapped precisely when a + * job finishes — the moment new suggestions appear — and re-mounting pointed at + * the wrong island, so the freshly rendered list arrived as inert server markup + * with no Keep or Reject behind its buttons. + */ + +export interface Moment { + id: string; + start: number; + end: number; + score: number; + reasons: string[]; + included: boolean | null; + favorite: boolean; +} + +const duration = (seconds: number): string => { + const total = Math.max(0, Math.round(seconds)); + return `${Math.floor(total / 60)}:${String(total % 60).padStart(2, '0')}`; +}; + +const decision = (included: boolean | null): string => + included === true ? 'keep' : included === false ? 'reject' : 'undecided'; + +const MomentReview = ({ projectId, initial }: { projectId: string; initial: Moment[] }) => { + const [moments, setMoments] = useState(initial); + const [busy, setBusy] = useState(null); + const [error, setError] = useState(null); + + const decide = async (moment: Moment, included: boolean | null): Promise => { + setBusy(moment.id); + setError(null); + try { + const response = await fetch( + `/api/projects/${encodeURIComponent(projectId)}/moments/${moment.id}`, + { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ included }), + }, + ); + const payload = (await response.json()) as { ok: boolean; moment?: Moment; error?: string }; + if (!response.ok || !payload.ok || payload.moment === undefined) { + throw new Error(payload.error ?? `Request failed (${response.status})`); + } + const updated = payload.moment; + setMoments((current) => current.map((m) => (m.id === updated.id ? updated : m))); + } catch (cause) { + // Leave the previous state alone so the user can retry without losing work. + setError(cause instanceof Error ? cause.message : String(cause)); + } finally { + setBusy(null); + } + }; + + const kept = moments.filter((m) => m.included === true).length; + + return ( +
+

+ {kept} of {moments.length} kept +

+ {error === null ? null :

{error}

} + {moments.map((moment) => ( +
+
+ + {duration(moment.start)} → {duration(moment.end)} + + score {moment.score.toFixed(2)} + {moment.reasons.join(', ')} + + {decision(moment.included)} + + + +
+
+ ))} +
+ ); +}; + +/** + * Takes over the server-rendered moment list, reading everything it needs from + * the node's own attributes — which is what makes it safe to call again after + * the server has re-rendered that region with different moments in it. + */ +export const mountMoments = (): void => { + const node = document.getElementById('moment-review'); + if (node === null) return; + + const projectId = node.dataset['project']; + const raw = node.dataset['moments']; + if (projectId === undefined || raw === undefined) return; + + let initial: Moment[]; + try { + initial = JSON.parse(raw) as Moment[]; + } catch { + // Bad payload: keep the server-rendered list rather than blanking the page. + return; + } + + node.innerHTML = ''; + render(, node); +}; diff --git a/apps/web/src/remount.test.ts b/apps/web/src/remount.test.ts new file mode 100644 index 0000000..823c18a --- /dev/null +++ b/apps/web/src/remount.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; + +import { REMOUNT } from './client/live.js'; +import { mountMoments } from './client/moments.js'; +import { mountReview } from './client/review.js'; + +/** + * A swapped region has to be re-hydrated by the island that actually lives in + * it. + * + * `data-live="moments"` contains `#moment-review`, the interactive Keep/Reject + * list. The map pointed instead at the review scrubber, which lives on a + * different page and inside no live region at all — so after a refresh the + * moment list stayed as the server rendered it, with nothing behind its + * buttons, while the scrubber was re-attached to a node that already had a + * canvas and a set of controls on it. + * + * Both halves are silent failures: the buttons render perfectly and do nothing, + * and the duplicate scrubber only appears once something has refreshed. Neither + * is visible in a type or a screenshot, so the wiring is asserted here. + */ + +describe('re-mounting after a live region swap', () => { + it('hydrates the moment list with the moment island', () => { + expect(REMOUNT['moments']).toBe(mountMoments); + }); + + it('never re-mounts the review scrubber, which is on another page', () => { + expect(Object.values(REMOUNT)).not.toContain(mountReview); + }); + + it('names only regions the pages actually mark as live', async () => { + const { ProjectPage } = await import('./views/pages.js'); + const view = { + project: { id: 'prj_test', name: 'Smoke', sport: 'basketball' }, + videos: [], + athletes: [], + moments: [], + clips: [], + jobs: [], + exports: [], + music: [], + flash: {}, + } as unknown as Parameters[0]; + const html = String(await ProjectPage(view)); + + // A key with no matching region is a re-mount that can never fire. + for (const key of Object.keys(REMOUNT)) { + expect(html).toContain(`data-live="${key}"`); + } + }); +}); From 9425ade1384f6279226fc46649fea8de5bc8fd72 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 10 Aug 2026 18:19:03 +0000 Subject: [PATCH 3/3] docs: record what the ball fixes actually measured Replaces the reasoning that motivated the buffer with the sweep that chose it. 1.5 is kept over 3.0 because the extra joining buys ~1% more coverage while roughly doubling the radius in which a stray blob can be taken for the ball, and a track that teleports is worse than two that stop. Measured against the deployed worker on identical footage and flags: ball hoop player referee time deployed today 21 / 382 7 / 387 297 / 8604 39 / 803 89s + birth at floor 71 / 831 15 / 425 297 / 8604 39 / 803 93s + buffered IoU 66 / 891 15 / 425 297 / 8604 39 / 803 88s Co-Authored-By: Claude Opus 5 --- apps/cv-worker/src/ballrecall.test.ts | 12 ++++++++++++ apps/cv-worker/src/pipeline.ts | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apps/cv-worker/src/ballrecall.test.ts b/apps/cv-worker/src/ballrecall.test.ts index a57084e..38489ac 100644 --- a/apps/cv-worker/src/ballrecall.test.ts +++ b/apps/cv-worker/src/ballrecall.test.ts @@ -17,6 +17,18 @@ import { ByteTracker, inflate } from './tracker.js'; * ball: it is a small box that clears several of its own widths between sampled * frames, and boxes that do not touch have an IoU of exactly zero however * close they are. + * + * Measured against the deployed worker, same 20s of real game, same flags: + * + * ball hoop player referee time + * deployed today 21 / 382 7 / 387 297 / 8604 39 / 803 89s + * + birth at floor 71 / 831 15 / 425 297 / 8604 39 / 803 93s + * + buffered IoU 66 / 891 15 / 425 297 / 8604 39 / 803 88s + * + * Ball positions rise 133% for no measurable runtime. The rim doubles too — its + * 0.15 floor was under the same 0.4 birth bar. People are identical to the + * digit, which is the invariant the tests below exist to hold: loosening the + * ball must never loosen anybody. */ const det = (x: number, y: number, w: number, h: number, score: number, classId = 0): Detection => ({ diff --git a/apps/cv-worker/src/pipeline.ts b/apps/cv-worker/src/pipeline.ts index 6154370..e1e4894 100644 --- a/apps/cv-worker/src/pipeline.ts +++ b/apps/cv-worker/src/pipeline.ts @@ -83,6 +83,21 @@ export interface PipelineResult { * one player's box to capture the player beside them. A ball is the opposite — * small, and fast enough to clear several of its own widths — which is why it * arrives as a scatter of short tracks rather than a flight. + * + * Swept over 20s of a real game, at the shipped 2x2 grid and ball floor: + * + * buffer ball tracks / positions positions per track + * 0 71 / 831 11.7 + * 0.5 67 / 869 13.0 + * 1.5 66 / 891 13.5 + * 3.0 59 / 900 15.3 + * + * Tracks fall while positions rise, which is fragments being joined into + * flights rather than new detections appearing. 3.0 joins more, but buys almost + * no extra coverage for it (+1% positions over 1.5) while roughly doubling the + * radius in which a stray ball-shaped blob can be mistaken for the ball — and a + * track that teleports is worse than two that stop. Player, referee and hoop + * counts were identical to the digit at every value. */ const SMALL_FAST: Record = { ball: 1.5, puck: 1.5 };