reTraining/docs/audit-2026-08-07.md

# Audit — reTraining, 2026-08-07

Method: 4 mapping agents over the codebase, 5 bug-hunting agents by dimension, adversarial
verification of each finding. 22 findings survived verification; 2 CRITICALs come from a
dimension whose verifiers were cut short by a usage limit and are marked *unverified* — both
were confirmed by hand afterwards.

---

## 1. How the system actually works

**Archive → batch.** `backend/library.py:90` lists the video archive and, per file, calls
`ensure_video_preview`, which spawns a bare daemon thread running an ffmpeg H.264 transcode
(`library.py:132-142`) for anything not already previewed. Trim range → `backend/batches.py`
extracts frames into `data/projects/<slug>/batches/<id>/`, numbered per batch from 1
(`batches.py:174,200-204`).

**Auto-annotate.** `backend/autolabel.py:29` starts a job; `_run_autolabel` loads SAM3
(`sam3_engine.py`) or a YOLO model, runs per frame, and writes shapes with `source='auto'`
via `review.replace_auto` / `append_auto` (`review.py:223,237`). Unknown class names in the
request are *silently added to the project* (`autolabel.py:100-106`). The SAM3
`set_image`-once-per-image invariant holds — verified in the engine loop.

**Review.** `frontend/src/pages/ReviewPage.jsx` holds all shapes in one `annotations` array.
Canvas gestures write `{local:true}` updates during the drag, then commit a PATCH on
pointerup (`AnnotationCanvas.jsx:128-141`). Human edits flip `source` to `'manual'`
(`review.py:208`). Approve/reject → `frames.review_status`.

**Merge.** `dataset.approve()` (`dataset.py:30`) sets status `approved` and queues a `merge`
job. `_run_merge` copies each approved frame into `dataset/images/{split}/<batch>__<stem>.jpg`
plus a label `.txt` (`dataset.py:380-391`), guarded per-frame by a `dataset_items` row. Split
comes from `_next_split` (`dataset.py:69-75`), which is **positional** — every Nth row by
`COUNT(*) FROM dataset_items` goes to val.

**Train & compare.** `training.py:106` calls `write_data_yaml`, which *always* calls
`sync_labels` (`dataset.py:117`) — rewriting every merged frame's label file from the live
`annotations` table. Fine-tune runs from the base model; `evaluate.compare` (`evaluate.py:39`)
runs `YOLO.val(split='val')` for base and new against the same `data.yaml`.

---

## 2. Critical bugs

### C1 — The job queue was deleted; everything now runs concurrently *(uncommitted)*
`backend/jobs.py:152`. `create()` was changed to spawn one thread per job instead of enqueuing
on the single worker. `_queue`, `_worker`, `_worker_lock` and `import queue` are now dead code,
while the module docstring still claims "one worker". Two merges, or a merge and a train, now
touch `dataset/` simultaneously. This is the root cause of C2, H1 and H2.
**Fix:** revert to the single-worker queue (`git diff backend/jobs.py`), delete the dead
`_start_job`.

### C2 — `autolabel` no longer takes `gpu_lock` *(uncommitted)*
`backend/jobs.py:31`. `GPU_JOB_TYPES` was narrowed to `("train",)` on the theory that
per-frame inference can safely run in parallel. It cannot: N parallel SAM3 jobs each hold a
full backbone in VRAM. Worse, `release_engine()` (`sam3_engine.py:229-246`) only clears the
module global — it cannot free VRAM held by a *running* autolabel job, so a training run that
starts mid-autolabel OOMs. The synchronous routes `preview_autolabel`
(`api/batches.py:163`) and `/api/sam3/playground-test` (`api/batches.py:183`) take no lock
either, while `review.assist` (`review.py:302`) correctly does.
**Fix:** put `autolabel` back in `GPU_JOB_TYPES`; wrap both synchronous routes in
`gpu_lock.acquire(timeout=...)` the way `review.assist` does.

### C3 — `custom_model_path` is a raw client-supplied filesystem path
`backend/autolabel.py:127`, `api/batches.py:37,46`. `inspect-model` writes the upload to a
`NamedTemporaryFile(delete=False)` and returns **the server path to the browser**
(`api/batches.py:123`), which the client posts back. A stale or wrong path does not error —
it falls back to a different model, so the batch is labelled by weights the user did not pick.
**Fix:** return an opaque staging id, keep the id→path map server-side, and raise
`BatchError('staged model expired, re-upload')` instead of falling back.

---

## 3. Everything else, ranked

| Sev | Area | Location | Issue | Fix |
|---|---|---|---|---|
| H | dataset | `dataset.py:117` | `sync_labels` rewrites **merged** labels from live annotations with no `review_status` filter — re-running auto-annotate on a merged batch pushes unreviewed model output into the master dataset on the next training start | Filter to `review_status='approved'`, or snapshot labels at merge time |
| H | dataset | `dataset.py:91` | Class-filtered training rewrites the *shared* master labels with remapped 0..k-1 ids, contradicting `project_classes` | Write remapped labels to a separate `labels_selected/` tree |
| H | dataset | `batches.py:239` | Deleting a batch drops DB rows (FK cascade) but leaves `dataset/images/**` + `labels/**` orphans on disk, which `data.yaml` still trains on; also shifts `_next_split` positions → train/val leakage on re-import | Unlink the batch's `dataset_items` files before deleting; make split content-derived (hash) not positional |
| H | dataset | `dataset.py:30` | Double-approve queues two concurrent merge jobs for the same batch | Reject approve when a non-terminal merge job exists; `INSERT … ON CONFLICT DO NOTHING` |
| H | dataset | `dataset.py:117` | Label + `data.yaml` writes are truncate-in-place, not atomic — a training run reads a half-written dataset | `os.replace` from temp files; snapshot the file list before training |
| H | library | `library.py:92` | Listing the archive spawns one unbounded ffmpeg `-preset medium -crf 18` thread per non-H.264 video | Route through `jobs.create` or a 1–2 worker pool; transcode lazily on playback |
| H | frontend | `ReviewSidebar.jsx:86` | Sidebar trash button deletes the **previously** selected shape (stale closure: `setSelectedId` then `removeSelected` in one tick) | Pass the id explicitly: `removeAnnotation(item.id)` |
| H | frontend | `ReviewPage.jsx:239` | Capture-phase keydown ignores modifiers — Ctrl/Cmd+A/C/X/S/T/N all fire review shortcuts and `preventDefault()` | Early-return when `ctrlKey \|\| metaKey \|\| altKey` |
| H | frontend | `ReviewPage.jsx:93` | Index advances even when the status POST fails; optimistic status never reverted | Revert in `catch`, don't advance on rejection |
| H | api | `api/batches.py:117` | `inspect-model` leaks its staged `.pt` on every success and on modal cancel | Server-owned staging dir keyed by id, deleted on job completion + startup sweep |
| H | api | `dataset.py:275` | `datasetSummary` returns **every annotation in the project** as JSON on two page loads | Return histograms only; gate raw shapes behind `?detail=shapes` |
| H | frontend | `AutoAnnotateModal.jsx:82` | Preview errors (VRAM OOM, bad model, missing frame) are all swallowed to `console.error` and render as "no detections" | Add an error surface; distinguish 4xx config from 5xx inference |
| M | dataset | `dataset.py:402` | Cancelled merge still marks the batch `merged`; approved frames are then permanently unmergeable | Only set `merged` when the loop completed |
| M | dataset | `dataset.py:140,162` | Empty val set silently falls back to the **training** images — base-vs-new mAP is then measured on trained data | Refuse to train/compare with an empty val set |
| M | dataset | `dataset.py:154` | `selected_data.yaml` writes `nc` from the full class list while `names` holds the subset | `nc: len(target_classes)` in both branches |
| M | autolabel | `autolabel.py:100` | A labelling job silently creates project classes from client-supplied names (violates REQ-003's "no drift as a side effect") | Make class creation explicit; preview should report unknown names |
| M | jobs | `jobs.py:194` | `cancel()` flushes a whole-row snapshot, racing the handler thread's own flush → progress/log resurrection | Targeted `UPDATE … WHERE status='queued'` + append-only log |
| M | jobs | `jobs.py:66` | Cancellation is process-local; `recover()` marks such jobs `failed`, never `cancelled` | Persist a `cancel_requested` column |
| M | api | `api/batches.py:163` | Synchronous GPU inference in request handlers, no lock, no VRAM check | Same `gpu_lock` pattern as `review.assist` |
| M | api | `api/batches.py:140` | Staged uploaded `.pt` never deleted after a successful job | Stage under `data/projects/<slug>/uploads/`, delete in `finally` |
| M | api | `api/projects.py:129` | `DELETE` returns 200 for a nonexistent project (same at `api/review.py:65`) | 404 when `delete()` returns False |
| M | api | `api/batches.py:261` | Dataset download link is a plain `<a href download>`, so backend errors render as a raw JSON page | Disable when empty, or fetch via blob |
| M | frontend | `AnnotationCanvas.jsx:114` | Alt-click delete-vertex is a **no-op** — `updateShape` discards the passed geometry on commit and re-PATCHes the old one | Use the passed geometry on commit |
| M | frontend | `ReviewPage.jsx:71` | The 2 s job poll overwrites in-flight drag edits — shapes snap back mid-gesture | Skip the refetch while a gesture is active |
| M | frontend | `ReviewPage.jsx:93` | Rapid A/X approvals capture the same `frame` twice → one frame PATCHed twice, the next skipped unreviewed | Derive the frame inside the functional `setIndex` |
| M | frontend | `ReviewPage.jsx:129` | Failed geometry/class/delete requests are never rolled back — canvas and server diverge silently | Snapshot and restore in `catch` |
| M | frontend | `ReviewPage.jsx:198` | "Track 5 Frames" / `[T]` is dead code — it reads `geometry.coordinates`, which this app never produces | Use `geometry.points`, or delete (propagation is a stated non-goal) |
| M | frontend | `AutoAnnotateModal.jsx:294` | Custom-model chips list *project* classes, not the inspected model's | Render `customModelClasses` when `engine === 'custom'` |
| M | ops | `Dockerfile:1` | No root `.dockerignore` — 38 GB `data/`, 5.2 GB `.venv` and `.env` are all sent as build context | Add one |
| M | frontend | `AnnotationCanvas.jsx:163` | Escape clears the drag but leaves the shape visually moved and uncommitted | Restore `drag.start` before clearing |
| L | frontend | `ReviewPage.jsx:243` | Arrow-key nav during a drag silently drops the edit | Ignore nav keys mid-gesture, or key the canvas on `frame.id` |
| L | frontend | `ReviewPage.jsx:112` | SAM3 assist doesn't bump `annotation_count` | Add the `patchFrameLocally` call |
| L | frontend | `Filmstrip.jsx:7` | Every frame rendered, no virtualization, full reconcile every 2 s | Window it + `React.memo` |
| L | ops | `start.sh:7` | `rm -f docker-compose.override.yml` deletes a **tracked** file and silently drops your `./backend` bind mount | Untrack it; generate `docker-compose.gpu.yml` and pass via `-f` |
| L | ops | `README.md:18` | References a `.env.example` that doesn't exist; `HF_TOKEN` is required for gated SAM3 weights | Commit one |
| L | ops | `requirements.txt` | torch/torchvision/ultralytics/fastapi unpinned — CLAUDE.md §6's "cannot drift" claim is false | Pin or add a uv lockfile |
| L | api | `api.js:83` | `autolabelWithModel` is orphaned; its `iou_threshold` default (0.8) contradicts the endpoint's (0.0) | Delete both, or route the custom-model flow through it |

Also: **`docker-compose.override.yml` is committed and pins `nvidia.com/gpu=all`** — Compose
auto-merges it, so a CPU-only host cannot start, and README tells users to run
`docker compose up` directly, bypassing `start.sh`. This violates CLAUDE.md §9 head-on.

---

## 4. The dirty working tree

| File | Verdict | Reasoning |
|---|---|---|
| `backend/jobs.py` | **Revert** | Source of C1 and C2. The parallel-jobs change is the single most damaging uncommitted edit. |
| `docker-compose.override.yml` | **Untrack** | Should never have been committed; `start.sh` regenerates it and destroys your bind mount each run |
| `backend/test.py`, `backend/test_preview.py` | **Delete** | Scratch scripts in the package dir; not tests, not imported |
| `algoritma-batch/batch_video_cropper.py.bak` | **Delete** | Backup file in git's way |
| `scratch/` | **Gitignore** | |
| `frontend/src/pages/BatchesPage.jsx`, `DataPrepPage.jsx`, `AutoAnnotateModal.jsx` | **Finish** | Reachable from the app and carrying real bugs (rows above). Not abandoned. |
| `scripts/transcode_archive.py`, `algoritma-batch/src/h264_converter.py` | **Finish** | Directly relevant to the H `library.py:92` finding — the right home for that work |
| `algoritma-batch/predict.py`, `test_batch_logic.py` | **Review** | Unreferenced by the app; decide whether `algoritma-batch/` is still in scope |

---

## 5. Workflow gaps — where the design is wrong

1. **The positional val split is the design's weakest point.** `_next_split` counting rows
   means the split depends on *insertion order and history*. Any deletion, any re-merge, any
   reordering shifts it. The invariant in CLAUDE.md §8 is stated but not enforced by the
   mechanism. It should be content-derived: `hash(project_id + batch + stem) % 5 == 0 → val`.
   Then it is stable by construction and no code path can violate it.

2. **Master labels are regenerated from live annotations on every training start.** This is
   the reason three separate HIGH findings exist. A merged dataset should be *immutable* —
   merging means "snapshot these labels". Regeneration should be an explicit repair action,
   not a side effect of pressing Train.

3. **The base-vs-new comparison silently degrades instead of refusing.** Empty val → validate
   on train images. Deleted batch → phantom images in the val dir. Class filter → remapped
   ids. Every one of these produces a number that *looks* fine. For a system whose entire
   purpose is "did retraining help?", the comparison should be fail-loud.

4. **No visibility into a running job beyond a progress bar,** and cancellation doesn't
   survive restart. For jobs that run tens of minutes this is the main source of "rough".

5. **No resume.** A cancelled or crashed merge leaves the batch in a state that can never be
   merged again (M, `dataset.py:402`).

---

## 6. Recommended order of work

1. **Revert the `jobs.py` parallelism experiment.** → verify: start two autolabel jobs on
   different batches; expect the second to sit `queued` until the first finishes, and
   `nvidia-smi` to show one SAM3 backbone resident, not two.
2. **Restore `gpu_lock` on autolabel + the two synchronous GPU routes.** → verify: start an
   autolabel job, then hit Preview; expect "GPU is busy with a autolabel job", not an OOM.
3. **Stop `sync_labels` from touching merged labels** (filter to `approved`; move the
   class-filter remap into `labels_selected/`). → verify: merge a batch, edit an annotation in
   review, press Train; expect the merged `.txt` on disk to be unchanged.
4. **Make the val split content-derived** and backfill existing `dataset_items`. → verify:
   delete a batch, re-import, re-merge; expect every frame to land in the same split as before.
5. **Clean up batch deletion** — unlink the dataset files. → verify: delete a merged batch;
   expect `find dataset/images -name '<id>__*'` to return nothing.
6. **Fail loudly on empty val** and fix `nc`. → verify: train a project with no val items;
   expect a clear error, not a silent train-on-train run.
7. **Fix the two review-editor HIGHs** (sidebar delete, modifier keys) and the no-op
   delete-vertex. → verify: keyboard-only pass over 20 frames; Ctrl+S must not approve.
8. **Untrack `docker-compose.override.yml`, add `.dockerignore` and `.env.example`,
   pin requirements.** → verify: `docker compose build` on a CPU-only host succeeds.