diff --git a/backend/api/review.py b/backend/api/review.py index 4df848b..12b0b63 100644 --- a/backend/api/review.py +++ b/backend/api/review.py @@ -38,6 +38,8 @@ class AssistRequest(BaseModel): box: List[float] class_id: int = 0 threshold: float = 0.5 + # Track Forward only: drop the result when it lands on an existing shape. + dedupe: bool = False class PoolExemplar(BaseModel): @@ -116,7 +118,7 @@ def bulk_reclass(request: BulkReclassRequest) -> dict: def assist(frame_id: int, request: AssistRequest) -> dict: try: return review_store.assist(frame_id, request.box, request.class_id, - request.threshold) + request.threshold, dedupe=request.dedupe) except review_store.ReviewError as exc: raise HTTPException(400, str(exc)) diff --git a/backend/batches.py b/backend/batches.py index d5d11e8..a919541 100644 --- a/backend/batches.py +++ b/backend/batches.py @@ -137,15 +137,11 @@ def frames(batch_id: int) -> List[dict]: with db.cursor() as cur: cur.execute( """SELECT f.*, (SELECT COUNT(*) FROM annotations a WHERE a.frame_id = f.id) - AS annotation_count, - (SELECT GROUP_CONCAT(DISTINCT a.class_id) FROM annotations a - WHERE a.frame_id = f.id) AS class_ids + AS annotation_count FROM frames f WHERE f.batch_id = ? ORDER BY f.idx""", (batch_id,), ) rows = [dict(row) for row in cur.fetchall()] - for row in rows: - row["class_ids"] = [int(v) for v in row["class_ids"].split(",")] if row["class_ids"] else [] return rows diff --git a/backend/review.py b/backend/review.py index 36a1652..cad0901 100644 --- a/backend/review.py +++ b/backend/review.py @@ -320,7 +320,7 @@ def frames_with_auto(batch_id: int) -> set: def assist(frame_id: int, box: List[float], class_id: int = 0, - threshold: float = 0.5) -> dict: + threshold: float = 0.5, dedupe: bool = False) -> dict: """Drag a rough box, get SAM3's shape for the object inside it (REQ-043). The box is a visual exemplar rather than a crop: SAM3 may return several @@ -378,6 +378,16 @@ def assist(frame_id: int, box: List[float], class_id: int = 0, finally: jobs.gpu_lock.release() + # Track Forward passes dedupe=True (REQ-189): the run still happens, but a + # result that lands on a shape already there is the same object twice, so + # nothing is written and the caller reports the frame as skipped. Runs after + # the lock is released because this part needs no GPU. + if dedupe: + landed = to_box(geometry) + for existing in listing(frame_id): + if existing["class_id"] == class_id and intersects(landed, to_box(existing["geometry"])): + return {"skipped": True} + return add(frame_id, class_id, geometry, source="manual", score=detection.score) @@ -397,6 +407,16 @@ def _overlap(detection_box: List[float], drawn: List[float], return inter / (area_box + area_drawn - inter) +def intersects(a: List[float], b: List[float]) -> bool: + """Any pixel of intersection between two normalized xyxy boxes (REQ-189). + + Deliberately not IoU: two trucks that merely touch are two trucks. Raising + this to a ratio is the upgrade path if adjacent same-class boxes ever start + getting dropped as duplicates. + """ + return min(a[2], b[2]) > max(a[0], b[0]) and min(a[3], b[3]) > max(a[1], b[1]) + + def clear_batch_class_annotations(batch_id: int, class_id: int) -> int: """Delete all annotations matching class_id across all frames in a batch (REQ-046).""" with db.cursor() as cur: diff --git a/docs/requirements.md b/docs/requirements.md index 624dd04..895d8b8 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -200,9 +200,12 @@ changes. 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. A frame that **already holds a shape of the tracked class is skipped** — hidden or - dimmed shapes count as present — and the skip is reported in the result banner and marked - with a dot on that frame's filmstrip thumbnail, never silently passed over. A long run can be + nothing. Every frame ahead **runs** — a frame that already holds a shape of the tracked class + is still annotated when SAM3 finds a *different* object there. What is dropped is a + **duplicate**: a result that intersects an existing shape of the tracked class (any shared + area counts, hidden or dimmed shapes count) is not written, and that skip is reported in the + result banner and marked with a dot on that frame's filmstrip thumbnail, never silently passed + over. A long run can be **cancelled** mid-flight: cancelling stops the remaining frames, but a request already in flight is not aborted server-side and may still save its shape. The box is never grown and never reseeded from a result, so there is no size or position readjustment between frames: a fast-moving object can still leave the diff --git a/docs/tasks.md b/docs/tasks.md index 26f8f85..7f7395b 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -1511,7 +1511,8 @@ read it. slice bound is computed from `Number(trackFrames)` so a string can never concatenate into the slice end; `slice(index+1, index+1+N)` and `slice(index+1)` both bounded; cancel lands as `cancelled — frame n may still be saved by the server` and keeps the shapes already written; - skip check reads the `class_ids` array, not `annotation_count`; marks cleared at the start of + skip check reads the `class_ids` array, not `annotation_count` — *that rule was later replaced + by the post-run overlap test in the next task*; marks cleared at the start of every run; `Shift+T` deliberately **not** bound (a fat-fingered 500-frame GPU run is not worth a keystroke). Reviewer pass done — 4 findings fixed. **Still owed, user browser + GPU**: set N=3 → 3 shapes, reload → N remembered; to-end over the already-reviewed part of batch 19 → @@ -1534,6 +1535,36 @@ read it. 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. +## Task — Track Forward skips on overlap, not on class presence (REQ-189 amended) `[DONE]` + +- The old rule skipped a frame *before* SAM3 ran when it already held a shape of the tracked + class, which contradicted REQ-189's own "each frame is an independent run": a second truck in + the same frame could never be annotated. Amended to a duplicate test **after** the run. +- `backend/review.py:intersects()` — any intersection between two normalized boxes (shared area + > 0; edge touch is *not* an overlap), deliberately not IoU. `assist()` gained + `dedupe: bool = False`: once the GPU lock is released and right before `add()`, an existing + shape of the same class on that frame whose box intersects the result makes it answer + `{"skipped": True}` and write nothing — no create-then-delete, no transient row. + `backend/api/review.py:AssistRequest.dedupe` carries the flag; `False` keeps the single-shot + box-assist path byte-identical. +- `frontend/src/pages/ReviewPage.jsx:trackForward` — pre-skip on `class_ids` and the local + `class_ids` union are gone, the call sends `dedupe: true`, and `{skipped: true}` lands in the + banner as `skipped K overlapping ` plus the filmstrip dot `overlaps an existing `. +- `backend/batches.py:frames()` no longer ships `class_ids` (it was added for the pre-skip and + died with it); `annotation_count` stays. +- Docs: `requirements.md` REQ-189 skip sentence rewritten, `ui-spec.md` banner + thumb tooltip. +- Cost accepted: a long run now pays a GPU call on **every** frame ahead, including frames that + end up skipped, where the old pre-skip made those free. Only worth layering a cheap seed-box + pre-skip back if that hurts. +→ verify: **[DONE]** `uv run python` harness (mocked engine + temp PNG) — 6 `intersects` + cases (edge-touch false, partial/contained/corner true, disjoint false) and 5 `assist()` paths: + dedupe+overlap → `{"skipped": True}` with `add()` never called; other-class box → annotated; + empty frame → annotated; `dedupe=False` → annotated; polygon annotation → skipped. + `batches.frames()` run against the live DB (3737 rows, `class_ids` gone, `annotation_count` + kept); `npm run build` green; `git diff --check` clean. **Still worth a GPU run:** track into a + frame already holding an overlapping truck → banner `skipped K overlapping truck`, dot, count + unchanged, while a frame holding a truck elsewhere gets its own second box. + ## 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 bb1bbab..79206ec 100644 --- a/docs/ui-spec.md +++ b/docs/ui-spec.md @@ -937,7 +937,7 @@ worked". The jump happens once per load, and again after an auto-label job finis ### 5.2 Data model in play ``` -frame { id, idx, filename, width, height, review_status, annotation_count, class_ids } +frame { id, idx, filename, width, height, review_status, annotation_count } annotation { id, class_id, geometry, score, source: "auto"|"manual" } batch { …, review: { pending, approved, rejected }, annotation_count, status } ``` @@ -1036,9 +1036,12 @@ an "asking SAM3…" indicator · 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) followed by `skipped K already had ` and the per-frame - failures. Skipped frames are those that already hold a shape of the tracked class (hidden or - dimmed counts); they get a dot on their filmstrip thumb and the thumb's tooltip says why. The + (M = frames actually ahead) followed by `skipped K overlapping ` and the per-frame + failures. Every frame ahead runs SAM3; a frame is skipped only when its **result** lands on a + shape of the tracked class already there (any intersection — hidden or dimmed counts), which + means a frame holding the tracked class elsewhere still gets its own second box. Skipped frames + get a dot on their filmstrip thumb and the thumb's tooltip says `overlaps an existing `. + The "asking SAM3…" indicator becomes a **Cancel** button while a run is in flight (Cancel stops the remaining frames; the frame already in flight can still be saved by the server, and the banner says so). A second diff --git a/frontend/src/components/Filmstrip.jsx b/frontend/src/components/Filmstrip.jsx index 0f09d1a..ff289c5 100644 --- a/frontend/src/components/Filmstrip.jsx +++ b/frontend/src/components/Filmstrip.jsx @@ -1,7 +1,7 @@ import React from 'react' import { api } from '../api' -export default function Filmstrip({ frames, index, onSelectIndex, stripRef, markedIds, markLabel = 'already has it' }) { +export default function Filmstrip({ frames, index, onSelectIndex, stripRef, markedIds, markLabel = 'overlaps an existing shape' }) { return (
{frames.map((item, position) => { diff --git a/frontend/src/pages/ReviewPage.jsx b/frontend/src/pages/ReviewPage.jsx index 1fd1b81..8c19835 100644 --- a/frontend/src/pages/ReviewPage.jsx +++ b/frontend/src/pages/ReviewPage.jsx @@ -418,22 +418,23 @@ export default function ReviewPage({ batchId: rawBatchId, projectId, onProject } const marked = new Set() for (let i = 0; i < ahead.length; i++) { const targetFrame = ahead[i] - // A frame that already holds this class is left alone — hidden or - // dimmed shapes count. It is reported, never silently passed over. - if ((targetFrame.class_ids ?? []).includes(current.class_id)) { - skipped++ - marked.add(targetFrame.id) - continue - } try { - await api.assist(targetFrame.id, { box, class_id: current.class_id }, { signal: controller.signal }) + // dedupe makes the backend drop a result that lands on a shape of + // this class already on the frame — that is the same object twice — + // and answer { skipped: true } without writing it. + const created = await api.assist( + targetFrame.id, + { box, class_id: current.class_id, dedupe: true }, + { signal: controller.signal }, + ) + if (created?.skipped) { + skipped++ + marked.add(targetFrame.id) + continue + } tracked++ patchFrameLocally(targetFrame.id, { annotation_count: (targetFrame.annotation_count ?? 0) + 1, - // Keeps the skip check honest for a second run in this session. - class_ids: (targetFrame.class_ids ?? []).includes(current.class_id) - ? targetFrame.class_ids - : [...(targetFrame.class_ids ?? []), current.class_id], }) } catch (exc) { if (controller.signal.aborted) { @@ -448,7 +449,7 @@ export default function ReviewPage({ batchId: rawBatchId, projectId, onProject } setSkippedLabel(className) } const parts = [`Tracked ${tracked} of ${ahead.length}`] - if (skipped) parts.push(`skipped ${skipped} already had ${className}`) + if (skipped) parts.push(`skipped ${skipped} overlapping ${className}`) if (failures.length) parts.push(failures.join(' · ')) if (tracked === 0 || skipped || failures.length) setError(parts.join(' — ')) } finally { @@ -780,7 +781,7 @@ export default function ReviewPage({ batchId: rawBatchId, projectId, onProject }