From b71f1d4bf29193bdf95a01ddb0213cfc05901280 Mon Sep 17 00:00:00 2001 From: asus Date: Mon, 5 Oct 2026 12:01:59 +0700 Subject: [PATCH] fix: track 5 frames works for bbox and polygon shapes (REQ-189) trackForward read geometry.coordinates, a key the backend never emits, and had no bbox branch - the action silently no-op'd for every shape (flagged dead in docs/audit-2026-08-07.md:103). - bbox points used as-is, polygon reduced to its bounding box - guards speak instead of returning silently - a frame SAM3 refuses no longer aborts the remaining frames - a press during an in-flight run is ignored (would double-write) - REQ-189 added; non-goal requirements.md:22 amended to keep exemplar propagation out while admitting this one-shot hand-off --- docs/requirements.md | 14 ++++++- docs/tasks.md | 17 +++++++++ docs/ui-spec.md | 11 ++++-- frontend/src/pages/ReviewPage.jsx | 61 +++++++++++++++++++++---------- 4 files changed, 79 insertions(+), 24 deletions(-) diff --git a/docs/requirements.md b/docs/requirements.md index f5ce41c..0e4d6c1 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -19,7 +19,10 @@ changes. - Login, multi-user, tenants, quotas. The architecture leaves room for them; the features are not built. -- Tracking or annotation propagation between frames. +- Propagation of **drawn exemplars** across a batch or between frames — an example pools + features from its own image (REQ-172), so it cannot travel. One-shot propagation of an + already-drawn shape into the next few frames is a different thing and is REQ-189. + (Amended by REQ-189: was "Tracking or annotation propagation between frames".) - Collaborative annotation by several people at once. - Public internet deployment. @@ -165,6 +168,15 @@ changes. class. - **REQ-043** — The user can ask SAM3 for help inside the editor: click or drag a box around one object and the model produces its shape. +- **REQ-189** — In the review editor, the user can push the **selected shape's** box forward + with *Track 5 Frames* (`[T]`): the next five frames each get their own REQ-043 box-assist + run, seeded with that box, and the shape is written as `source=manual`. Both geometry types + work — a bbox shape supplies its own box, a polygon is reduced to its bounding box. Each + frame is an **independent** run: a frame SAM3 refuses (nothing inside the box) is reported + and the rest still run; nothing selected, an unusable shape, or the last frame says so + instead of doing nothing. This is one-shot hand-off across the next frames, **not** + propagation of drawn exemplars (which stays a non-goal): nothing is carried frame to frame, + and the runs do not depend on each other. - **REQ-044** — All annotations and review statuses are **persistent** — they survive a server restart, unlike today's in-memory sessions. - **REQ-045** — Review progress is visible (e.g. "120/300 reviewed"), and a batch can only diff --git a/docs/tasks.md b/docs/tasks.md index 264d158..7033bb6 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -1405,6 +1405,23 @@ read it. (accent border + glow) — no CSS added; **browser eyeball still owed by the user** (375 px 2-col wrap, long class names). +## Task — Track 5 Frames works at all (REQ-189) `[DONE]` + +1. `ReviewPage.jsx:trackForward` read `geometry.coordinates` — a key the backend never emits + (`backend/review.py:35-43` writes `points`) and had no bbox branch, so the action no-op'd + silently for **every** shape; `docs/audit-2026-08-07.md:103` had flagged it as dead. + Now: bbox `points` used as-is, polygon reduced to its bounding box, each guard speaks + (no selection / shape gone / last frame / unusable geometry), one refusing frame no longer + aborts the rest (`Tracked N of M` + per-frame failures), a press during a run is ignored + (double-press used to double-write the same five frames once the path went live). + REQ-189 added; non-goal `requirements.md:22` amended to keep exemplar propagation out while + admitting this one-shot hand-off → verify: **[DONE]** reviewer SPEC ✅ (geometry order + + normalization traced against `validate`/`bbox`, loop bounds `slice(index+1, index+6)`, + frame numbering matches the frame bar, dep array sufficient); `npm run build` green; + doc truth fixed (`ui-spec:991` count wording, stale known-limitation line removed, audit + snapshot left as history); **SAM3 actually finding the object in the next 5 frames needs + the user's GPU run** — select a shape, press `T`, expect a shape on each of the next 5. + ## Known open points - *Not closed by any task, by choice:* **any rebuild kills the running job.** Task 14's resume diff --git a/docs/ui-spec.md b/docs/ui-spec.md index 61ca4b2..c0338af 100644 --- a/docs/ui-spec.md +++ b/docs/ui-spec.md @@ -989,9 +989,13 @@ previous geometry** and shows the error. Same optimistic-with-rollback pattern f - *Copy Prev* copies every annotation from frame `n-1` onto this frame (one POST per shape); disabled at index 0 or when the previous frame is empty. - *Track 5 Frames* takes the selected shape's bounding box and runs SAM3 assist on the next 5 - frames with it. **Only implemented for polygon geometry** in the current code — a bbox - selection yields no box and the action no-ops. A rebuild should either keep that limitation - or extend it deliberately, not accidentally. + frames with it. Works for both geometry types: a bbox shape supplies its own box, a polygon + is reduced to its bounding box. Nothing selected, an unusable shape, or the last frame + says so in the error slot instead of silently doing nothing; a frame SAM3 refuses (nothing + inside the box) does not stop the rest — the banner then **starts** with `Tracked N of M` + (M = frames actually ahead, ≤ 5) followed by the per-frame failures. A second press while + a run is in flight is ignored. Shapes are written `source=manual`, exactly as a hand-drawn + one (REQ-189). **Quick reclass bar** appears whenever a shape is selected: one button per class (`[n] name`, class-coloured) plus *Delete [Del]*. @@ -1285,7 +1289,6 @@ Removing any of these breaks the pipeline, corrupts data, or produces numbers th - `AutoAnnotateModal` and `MassAutoAnnotateModal` duplicate ~70 % of their logic. - `BatchList` and `ActiveJobsBanner` are exported from `LibraryPage` and imported by `BatchesPage`; they belong in `components/`. -- Track-5-frames only works for polygon geometry. --- diff --git a/frontend/src/pages/ReviewPage.jsx b/frontend/src/pages/ReviewPage.jsx index dd52d05..c7db8e9 100644 --- a/frontend/src/pages/ReviewPage.jsx +++ b/frontend/src/pages/ReviewPage.jsx @@ -355,15 +355,27 @@ export default function ReviewPage({ batchId: rawBatchId, projectId, onProject } }, [frames, index, frame, hiddenClasses]) const trackForward = useCallback(async () => { - if (selectedId == null || !frames?.length) return + if (busy) return + if (!frames?.length) return + if (selectedId == null) { + setError('Select a shape first — Track 5 Frames follows one shape forward.') + return + } const current = annotations.find((a) => a.id === selectedId) - if (!current) return - + if (!current) { + setError('That shape is gone — select another one.') + return + } + + // SAM3 assist wants one normalized [x0, y0, x1, y1] box (REQ-043). A bbox + // shape already is that; a polygon is reduced to its bounding box. + const geometry = current.geometry let box = null - if (current.geometry?.type === 'polygon' && current.geometry.coordinates?.[0]) { - const pts = current.geometry.coordinates[0] + if (geometry?.type === 'bbox' && geometry.points?.length === 4) { + box = geometry.points + } else if (geometry?.type === 'polygon' && geometry.points?.length) { let minX = 1, minY = 1, maxX = 0, maxY = 0 - for (const [x, y] of pts) { + for (const [x, y] of geometry.points) { if (x < minX) minX = x if (x > maxX) maxX = x if (y < minY) minY = y @@ -371,27 +383,38 @@ export default function ReviewPage({ batchId: rawBatchId, projectId, onProject } } box = [minX, minY, maxX, maxY] } - if (!box) return + if (!box) { + setError('That shape has no usable box — Track 5 Frames needs one.') + return + } + const ahead = frames.slice(index + 1, index + 6) + if (!ahead.length) { + setError('Last frame — Track 5 Frames has no frames ahead to follow into.') + return + } setBusy(true) setError('') try { - let updatedCount = 0 - for (let i = 1; i <= 5; i++) { - const nextIdx = index + i - if (nextIdx >= frames.length) break - const targetFrame = frames[nextIdx] - - await api.assist(targetFrame.id, { box, class_id: current.class_id }) - updatedCount++ - patchFrameLocally(targetFrame.id, { annotation_count: (targetFrame.annotation_count ?? 0) + 1 }) + let tracked = 0 + const failures = [] + for (let i = 0; i < ahead.length; i++) { + const targetFrame = ahead[i] + try { + await api.assist(targetFrame.id, { box, class_id: current.class_id }) + tracked++ + patchFrameLocally(targetFrame.id, { annotation_count: (targetFrame.annotation_count ?? 0) + 1 }) + } catch (exc) { + failures.push(`frame ${index + i + 2}: ${exc.message}`) + } + } + if (failures.length) { + setError(`Tracked ${tracked} of ${ahead.length} — ${failures.join(' · ')}`) } - } catch (exc) { - setError(exc.message) } finally { setBusy(false) } - }, [annotations, selectedId, index, frames]) + }, [annotations, selectedId, index, frames, busy]) const stateRef = useRef({}) stateRef.current = { frames, index, project, selectedId, setStatus, removeSelected, reclass, jumpToPending, jumpToNextAnnotated, setAssistMode, copyPrevious, trackForward, mode, setMode, markedIds, removeMarked, reclassMarked, setMarkedIds, toggleHideSelected }