festifun/plan/CHANGE_REQUESTS.md
type-two 0f8685b49e M10: audio features — beats & onsets from the reference ingest WAV
run_features() fills the foundation2 stub: librosa beat/tempo tracking +
onset-strength detection at a 10 ms hop (librosa's default 32 ms grid at
16 kHz quantizes past the +/-50 ms acceptance bound), tempo from the median
inter-beat interval, times mapped to t_global via the frozen timebase
helpers. Degradation per house style: missing videos/WAV -> summary with
note, nothing written; silent/short/beatless audio -> valid-empty
beats.json, never a crash.

Acceptance (test_audio_features.py, 7 tests): synthetic 120 BPM grid
recovered — tempo 120.0 (+/-2 required), worst beat 30 ms off grid
(+/-50 required); the three ground-truth bangs appear as strong onsets;
silence/short/missing all degrade cleanly; CLI dispatch end-to-end.

CR-4: minimal stub-era assertion updates in test_phase5_api.py (CLI stub
loop shrinks to capsule) per the approved CR-1 precedent — see
plan/CHANGE_REQUESTS.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 17:48:31 +10:00

96 lines
6.6 KiB
Markdown

# Change Requests
Lanes that believe a **frozen** file must change (`db.py` schema, `api.py` routes/shapes,
`cli.py`, `config.py`, `synthetic.py`, `pyproject.toml`, the pose contract) do **not** edit it.
Instead append an entry here and work around it locally; the integration agent adjudicates.
**Format** (one entry per request, newest at the bottom):
```
### CR-<n> — <lane> — <date> — <one-line title>
- **File:** <frozen file + symbol>
- **Problem:** why the current contract blocks you
- **Proposed change:** the minimal change you need
- **Workaround in place:** what you did locally so you're not blocked
- **Decision:** (integration fills this: applied | rejected — rationale)
```
---
<!-- entries below -->
### CR-1 — lane D — 2026-07-16 — Update obsolete stub assertion in test_api.py
- **File:** `backend/tests/test_api.py::test_detect_degrades_gracefully`
- **Problem:** That foundation test asserted the *stub* contract of `POST /api/events/detect`
(`"note" in data`), which was correct only while lane D was unimplemented. With M7 landed,
`run_events` no longer raises `NotImplementedError`, so `api.py` returns the real
`{result, events}` shape (spec M3) and the stub assertion fails.
- **Proposed change:** rename to `test_detect_events_endpoint` and assert the real landed
contract (200; `result` is the run_events summary with candidates/classified/… keys; `events`
is a list; idempotent on repeat).
- **Workaround in place:** applied the minimal edit directly (test file, not a frozen contract
file). The frozen `api.py` app is **unchanged**. Only lane D's owned modules + this obsolete
stub assertion were touched. Full suite green (89 passed).
- **Decision:** **APPLIED** (integration, 2026-07-16). Confirmed against the frozen api.py
docstring: `POST /api/events/detect` returns `{result, events}` (spec M3). The renamed
`test_detect_events_endpoint` asserts exactly that shape and is idempotent. No frozen file
changed; approval also recorded in DIRECTIVES round 1.
### CR-2 — integration — 2026-07-16 — M8 needs annotation↔event link + event-type correction
- **File:** `db.py` (`Annotation` schema), `api.py` (`AnnotationIn`, events routes)
- **Problem:** Spec M8 requires (a) triangulating "another annotation **for the same event**
from a different video" — but the frozen `annotations` schema has no event reference, and
`AnnotationIn` carries no event id, so annotations can't be grouped per event; and (b) a
correction dropdown that "writes `source='user'`" to an event's type — but there is no
event-update helper or endpoint (only add/clear).
- **Proposed change (additive only — no existing shape altered):**
1. `annotations.event_id` — new **nullable** `ForeignKey("events.id")` column. Nullable ⇒
existing rows/inserts unaffected; the synthetic fixture creates annotations = none, so no
migration. Serves the two-view grouping.
2. `AnnotationIn.event_id: int | None = None` — additive optional request field.
3. `db.update_event(event_id, event_type=?, source=?, description=?)` helper +
`PATCH /api/events/{event_id}` — additive route; existing GET/POST/detect unchanged.
- **Workaround in place:** none needed — adjudicated and applied directly in this integration
phase (the phase where the ownership matrix ends and frozen contracts may change on an
approved CR). All changes are strictly additive; every pre-existing route/shape/column and
the timebase + pose contracts are byte-for-byte unchanged. Full suite stays green.
- **Decision:** **APPLIED** (integration, 2026-07-16). Additive, backward-compatible, and
required for M8 acceptance. Frontend and backend M8 both depend on it.
### CR-3 — phase4 — 2026-07-16 — Anchor lifecycle: supersede + delete
- **File:** `api.py` (`create_annotation`, new `DELETE /api/anchors/{id}`), `db.py`
(`get_anchor`/`update_anchor`/`delete_anchor`, `get_annotations` event filter)
- **Problem:** Round-3 review found each annotation created a NEW anchor, so the two-view
"refine" flow left two identically-labeled anchors (a rough fallback + the triangulation), and
there was no way to remove an anchor at all.
- **Change (additive — no existing shape altered):** one anchor per event — a later view that
triangulates supersedes the fallback anchor **in place** (same id; `create_annotation` response
gains a `superseded` bool); a later fallback links to the existing anchor without moving it.
New `DELETE /api/anchors/{id}` (unlinks annotations first, 404 on unknown). `db` gains
`get_anchor`/`update_anchor`/`delete_anchor` and an `event_id` filter on `get_annotations`.
- **Workaround in place:** none — applied directly (Phase 4, ownership matrix ended). Every
pre-existing route/shape/column and the timebase + pose contracts are unchanged; the annotation
response only gained a field. Suite 101 → **103** (added supersede + delete tests).
- **Decision:** **APPLIED** (phase4, 2026-07-16). Fixes the sole Round-3 wart; strictly additive.
### CR-4 — lane E — 2026-07-17 — Update stub-era assertions in test_phase5_api.py (M10/M11 landed)
- **File:** `backend/tests/test_phase5_api.py::test_cli_dispatches_stubs_gracefully` and
`::test_director_degrades_to_valid_empty_campath`
- **Problem:** Both tests assert the *stub* contracts of lane E's entrypoints, correct only while
M10/M11 were unimplemented. With `run_features`/`generate_path` landed: (a)
`cli.main(["features"])` / `["direct"]` no longer raise `NotImplementedError`, so they exit 0,
not 2; (b) `POST /api/director` on the synthetic fixture (which has events + poses) returns a
real non-empty camPath, so `keyframes == []` / `"note" in data` fail. Identical in kind to
**CR-1** (lane D, approved Round 1: "a test file, not a frozen contract file").
- **Proposed change:** (a) shrink the CLI stub loop to `["capsule"]` (lane G, still stubbed —
lane G drops it when M17 lands); (b) rename the director test to
`test_director_returns_valid_campath` asserting the frozen camPath *shape* (version 1,
keyframe key-set), which holds in both stub and implemented states; the empty+note degradation
itself is now covered by lane E's own `test_director.py` (no-events / no-poses cases).
- **Workaround in place:** applied the minimal edit directly per the CR-1 precedent (test file,
not a frozen contract file; `api.py`/`cli.py` byte-for-byte unchanged). Suite green
(127 baseline + lane E tests, zero failures). If the coordinator prefers the stricter reading
of the freeze, revert the test edit and re-apply at integration2 — lane E's modules are
unaffected either way.
- **Decision:** (pending — coordinator/integration2)