Skip to content

feat(review): the Review workspace — gate 1 / PR 3 (ADR-0064) - #373

Merged
jeffcrouse merged 1 commit into
mainfrom
feat-review-workspace
Sep 18, 2026
Merged

jeffcrouse merged 1 commit into
mainfrom
feat-review-workspace

Conversation

@jeffcrouse

Copy link
Copy Markdown
Member

Summary

Third PR of gate 1 — the Review workspace itself. Follows #371 (the player, merged). Supersedes #372, which GitHub closed when its stacked base branch was deleted.

An explicit Review workspace beside Create (ADR-0064 §1): a [Create | Review] switch in the transport bar with a pending-decision badge, persisted in the window prefs, switched only by the creator — switch_workspace() is the single entry point and the app never switches on its own. While Review is active the frame loop composes it as a sibling of the Create shell (the visual graph keeps rendering for the output / pop-out) and the controller owns every click and non-⌘ key below the bar.

The surface

ui/review_geom (renderer-free layout that draw and input both call, so hit-rects equal pixels) · ui/review_view (pure draw of a ReviewModel) · app/review_workspace (the controller).

  • Left: Current version (the document's preferred_version) · Since your last review (events after the last review_visited) · Direction & protections (hard protections marked) · Decisions (open first).
  • Main: honest work-status strip (idle — no runner connected) · the one question + passage · Baseline | A | B tiles playing in lockstep with exactly one audible (click / 1-4 / Tab — a mute swap, never a re-seek) · play, scrub, bar N · m:ss, Loop passage, Match levels (audition-only, labeled, reverts) · per-candidate Use this / Keep / Revise / Dismiss · Neither — dismiss all · a comment box attaching to the audible version at the current bar · this review's feedback.

Decisions write work/ records synchronously — no agent needed (§4)

  • Use this = promotion. EditGateway::apply_document() restores the candidate's snapshot with its preferred_version as one undo step. Refused with the reason and nothing mutated when: the project changed since the item was opened; the preferred version is no longer the review's baseline; the brief moved on; or the candidate changes files outside project.json (gate 2). Undo/redo of a promotion is watched: the candidate flips promoted↔kept and promotion_undone / promoted (via redo) events are appended — the reversal is recorded, nothing deleted (ADR-0062 §5).
  • Keep leaves the question open. Revise requires the typed feedback. Dismissing every alternative resolves the review as neither.

Also

  • Review.baseline_media record field (+ test); WindowPrefs.workspace (+ test).
  • Play/pause drawn as shapes (the UI font has no ⏸).
  • review_player.mm now compiles with ARC (this file only): its AVPlayer members were freed at autorelease-pool exit under the tree's manual-retain default — found by the smoke test as a SIGSEGV in current_time().

Smoke-tested live

On a copy of examples/song-sketch with three rendered excerpts and hand-written records: switch + badge, tiles with lockstep playback and audible switching, promote → undo → redo with records and the document pointer following, a bar-stamped comment, ⌘Q's dirty prompt, review_visited on exit.

Test plan

  • test_review_geom — 371 checks: draw == hit rects, no overlaps, responsive degradation to 640×480 (it caught a transport-row overflow)
  • test_window_prefs (+workspace), test_work_records (+baseline_media)
  • Full headless ctest green (91), MCP parity green, layering guard green, vivid builds
  • CI

Found along the way (not this PR)

The deterministic export_av runs at ~0.2 fps here (AVFExporter::wait_ready pumping a nested run loop; frames dropped → 49 of 116 in the file; the standalone AVAssetWriter is 120 frames in 0.27 s). Pre-existing; blocks real-media generation for PR 4 — reported separately.

🤖 Generated with Claude Code

…2 §3-§5)

An explicit Review workspace beside Create (ADR-0064 §1): a [Create | Review] switch in the
transport bar with a pending-decision badge, persisted in the window prefs, switched only by
the creator (switch_workspace() is the one entry point; the app never switches on its own).
While Review is active the frame loop composes it as a SIBLING of the Create shell (the visual
graph keeps rendering for the output/pop-out) and the controller owns every click and non-⌘ key
below the bar.

The surface (ui/review_geom + ui/review_view, pure; app/review_workspace, the controller):
- left: CURRENT VERSION (the document's preferred_version) · SINCE YOUR LAST REVIEW (events
  after the last review_visited) · DIRECTION & PROTECTIONS (the brief; hard protections marked)
  · DECISIONS (open reviews first)
- main: an honest work-status strip ("idle — no runner connected") · the ONE question + passage
  · Baseline | A | B tiles playing in lockstep with exactly one audible (click / 1-4 / Tab to
  switch — a mute swap, never a re-seek) · play, scrub, "bar N · m:ss", Loop passage, Match
  levels (audition-only, labeled, reverts) · per-candidate Use this / Keep / Revise / Dismiss ·
  Neither — dismiss all · a comment box that attaches to the AUDIBLE version at the current bar
  · this review's feedback

Decisions write work/ records synchronously; no agent needed (§4):
- Use this = PROMOTION: EditGateway::apply_document() restores the candidate's snapshot with its
  preferred_version as ONE undo step. Refused (with the reason, nothing mutated) when the project
  changed since the item was opened, the preferred version is no longer the review's baseline,
  the brief moved on, or the candidate changes files outside project.json (gate 2). Undo/redo of
  a promotion is watched: the candidate flips promoted↔kept and promotion_undone / promoted(via
  redo) events are appended — the reversal is recorded, nothing deleted (ADR-0062 §5).
- Keep leaves the question open; Revise requires the typed feedback; dismissing every
  alternative resolves the review as "neither".

Also: Review record gains baseline_media (+ test); WindowPrefs.workspace (+ test); the play/pause
glyphs are drawn as shapes (the UI font has no ⏸); the review player is compiled with ARC (its
AVPlayer members were freed at autorelease-pool exit under the tree's manual-retain default —
found by the smoke test as a SIGSEGV in current_time()).

Smoke-tested live on a copy of examples/song-sketch with three rendered excerpts: switch, badge,
tiles + lockstep playback, audible switching, promote → undo → redo with records + pointer
following, a bar-stamped comment, ⌘Q's dirty prompt, review_visited on exit.

Tests: test_review_geom (371 checks: draw==hit rects, no overlaps, responsive degradation to
640×480 — it caught a transport-row overflow), test_window_prefs (+workspace), test_work_records
(+baseline_media). Full headless ctest green (91); MCP parity green; layering guard green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Production gate (core): ✅ PASS

  • 91/91 tests passed (0 failed) in 19.806s

@jeffcrouse
jeffcrouse merged commit 6a94e59 into main Sep 18, 2026
8 checks passed
@jeffcrouse
jeffcrouse deleted the feat-review-workspace branch September 18, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant