festifun/plan/CHANGE_REQUESTS.md
type-two 52674bbaf1 CR-6 ratified at merge (coordinator); launch.json: alt-port web config
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 21:38:16 +10:00

9.9 KiB

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)

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.
  • 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 G — 2026-07-17 — Drop obsolete capsule stub assertion in test_phase5_api.py

  • File: backend/tests/test_phase5_api.py::test_cli_dispatches_stubs_gracefully
  • Problem: The foundation2 test asserts cli.main(["capsule"]) == 2, i.e. the stub contract (build_capsule raises NotImplementedError). With M17 landed the entrypoint is real, so the CLI no longer exits 2 — the assertion fails by design, exactly like CR-1 when lane D landed M7.
  • Proposed change: remove only ["capsule"] from the loop (features/direct stay — lane E still stubbed) with a comment pointing at the landed coverage. The capsule CLI dispatch is now covered for real in backend/tests/test_capsule.py (builds a bundle via cli.main(["capsule", "--out", ...]), and asserts the clear no-dist error path).
  • Workaround in place: none possible without perverting build_capsule semantics (the spec requires a clear "run npm run build" error, not NotImplementedError); applied the minimal edit directly per the CR-1 precedent, flagged here for integration2 to ratify.
  • Decision: APPLIED — RATIFIED by coordinator at merge (2026-07-17). Exact CR-1 situation: a stub-contract assertion superseded by the milestone landing; the real CLI dispatch is covered in test_capsule.py. Suite 127 → 169 on the merged tree.

CR-5 — lane E — 2026-07-17 — Retire remaining 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 assert lane E's stub contracts, 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 (events + poses present) returns a real non-empty camPath, so keyframes == [] / "note" in data fail. Identical in kind to CR-1 and to CR-4 (lane G, coordinator-ratified the same day for capsule).
  • Proposed change: (a) with CR-4 already applied, lane E's landing empties the CLI stub loop entirely — retire test_cli_dispatches_stubs_gracefully (a comment points at the real dispatch coverage: test_capsule.py / test_audio_features.py / test_director.py); (b) rename the director test to test_director_returns_valid_campath, asserting the frozen camPath shape (version 1, exact keyframe key-set — holds stubbed or implemented); the empty+note degradation 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/CR-4 precedent (test file, not a frozen contract file; api.py/cli.py byte-for-byte unchanged), including the merge reconciliation with CR-4's side of the same loop. Suite green on the merged tree.
  • Decision: APPLIED — RATIFIED by coordinator at merge (2026-07-17). Same supersession pattern as CR-1/CR-4; the shape-locked director test is stronger than the stub assertion it replaces. Suite 169 → 189 on the merged tree.

CR-6 — foundation3 — 2026-07-17 — Conscious test-lock updates for the two additive M18 contract fields

  • File: backend/tests/test_api.py::test_manifest_shape (manifest exact-set lock) and backend/tests/test_capsule.py::test_bundle_structure (baked-paths list).
  • Problem: Not a supersession — these are the two deliberate lock edits M18 mandates (spec plan/30-phase6.md steps 3 & 7). foundation3 adds an additive manifest field has_tracks (drives the frontend loading friend tracks) and an additive capsule bake api/tracks (so ribbons + follow work zero-backend). The manifest key-set lock is intentionally exact ("has already caught two silent drifts"), and the capsule bundle-structure test enumerates baked paths, so both must be updated consciously in the same commit as the source change — the spec calls for exactly this + a CR note.
  • Proposed change: (a) add "has_tracks" to the test_manifest_shape exact-set and assert it is False on the live synthetic build (no solver has run — solving is lane H); (b) add "api/tracks" to the test_bundle_structure baked-paths tuple so the test now asserts the new read-only bake exists. No behavior removed; both are stronger post-edit.
  • Workaround in place: N/A — these are foundation-sanctioned edits to test files (not to a frozen contract file: db.py/api.py/capsule.py were extended additively per M18, and synthetic.py's marker code is a coordinator-sanctioned M18 edit). Applied directly in the foundation3 commit per the house pattern; flagged here for integration3 to ratify.
  • Decision: APPLIED — RATIFIED by coordinator at merge (2026-07-17). Both edits are the spec-mandated conscious lock updates (30-phase6.md steps 3 & 7), additive and stronger post-edit. Suite 189 → 209 on the merged tree.