diff --git a/docs/decisions.log.md b/docs/decisions.log.md index 6b199938..8d829d3a 100644 --- a/docs/decisions.log.md +++ b/docs/decisions.log.md @@ -962,3 +962,28 @@ Architectural decisions go to [`adr/`](adr/) instead. who genuinely wants transposed-with-the-band material has no knob for it (none of the six modes wants it — register is a placement axis, not a harmonic one). + +- 2026-06-11 — In the context of the S8 backlog item "boundary overlays + (S4)" (`preview/src/analysis.rs` / `scene.rs`), facing which boundary + config the preview should run, we decided for **the S4 defaults scaled + to the score's PPQN** (snap grid 1/16, minimum gap two quarters — the + exact closure.rs referee precedent), surfaced as plain start ticks on + `Analysis` and placed by the scene as `BoundaryMark` columns *after* + the section marks so a section keeps precedence on a shared column — + and against a preview-tunable config (knobs before the curation flow + needs them), and against drawing boundaries in the section band (the + band is the classification strip; boundaries are plane events like + gridlines). Accepted: boundary scores and reasons are dropped at the + view seam (ticks only) until an inspector surface wants them. + +- 2026-06-11 — In the context of Codex P2 on PR #39 (boundary overlays, + `preview/src/scene.rs`), facing a phrase boundary disappearing when + scrolled exactly to the viewport's left edge (the loop copied the + section-mark guard `tick <= scroll_tick`, dropping a tick that maps to + the leftmost plot column), we decided for **no scroll-origin guard on + boundaries**: `visible_col` already drops ticks before the scroll + origin, and a boundary at the edge is information the curator scrolled + to see — and against also changing the section-mark loop (skipping the + origin divider there is deliberate: the band already names the section + at the left edge). Accepted: a boundary at tick 0 of an unscrolled view + now renders a left-edge marker (harmless, and consistent). diff --git a/docs/stages/S8-preview-app.md b/docs/stages/S8-preview-app.md index b6e3e852..bd44bf23 100644 --- a/docs/stages/S8-preview-app.md +++ b/docs/stages/S8-preview-app.md @@ -57,7 +57,11 @@ front-ends and audio build on them: `griff-preview --record=` persisting the decision into the record's `reviewer` field on quit (`curation::decide_record`). Remaining: split/merge/rename/tag. -- [ ] Boundary overlays (S4) and candidate history. +- [ ] Boundary overlays (S4) and candidate history — **overlays landed + 2026-06-11**: `Analysis.boundaries` carries the S4 start ticks under a + PPQN-scaled default config, the scene places `BoundaryMark` columns + (sections keep precedence on shared columns), the TUI styles them. + Remaining: candidate history. ## Goal diff --git a/preview/src/analysis.rs b/preview/src/analysis.rs index 57ad1382..157913e0 100644 --- a/preview/src/analysis.rs +++ b/preview/src/analysis.rs @@ -3,7 +3,9 @@ //! Named sections (from [`griff_core::classify`]) and structure metrics (from //! [`griff_core::structure`]). Pure and headless-testable. +use griff_core::boundary::{detect_phrase_boundaries, BoundaryConfig}; use griff_core::classify::{bar_features_across_voices, classify_bar, BarClass}; +use griff_core::event::Ticks; use griff_core::score::{AtomEvent, Score, Voice}; use griff_core::structure::{ measure_complexity, measure_structure, ComplexityProfile, StructureMetrics, @@ -48,6 +50,8 @@ pub struct Analysis { pub metrics: Option, /// The focus track's per-axis complexity (S14); `None` for an empty score. pub complexity: Option, + /// Start ticks of the focus track's S4 phrase boundaries, in order. + pub boundaries: Vec, } /// Derives the [`Analysis`] for a score: pick the busiest track, classify each @@ -58,14 +62,35 @@ pub fn analyze(score: &Score) -> Analysis { let sections = sections_for(score, focus_track); let metrics = measure_structure(score, focus_track).ok(); let complexity = measure_complexity(score, focus_track).ok(); + let boundaries = phrase_boundary_ticks(score, focus_track); Analysis { focus_track, sections, metrics, complexity, + boundaries, } } +/// The focus track's phrase-boundary start ticks under the PPQN-scaled +/// default config (the S4 defaults assume PPQN 960; the `closure.rs` referee +/// precedent): snap grid 1/16, minimum boundary gap two quarter notes. +fn phrase_boundary_ticks(score: &Score, track_index: usize) -> Vec { + let ppqn = u32::from(score.ticks_per_quarter); + // Reason: ppqn / 4 and ppqn * 2 on a u32 PPQN cannot overflow or + // divide by zero. + #[allow(clippy::arithmetic_side_effects)] + let config = BoundaryConfig { + quantize_ticks: Ticks(ppqn / 4), + min_gap: Ticks(ppqn.saturating_mul(2)), + ..BoundaryConfig::default() + }; + detect_phrase_boundaries(score, track_index, &config) + .iter() + .map(|b| b.start_tick.0) + .collect() +} + /// Counts note atoms across all voices of a track. fn voice_note_count(voice: &Voice) -> usize { voice @@ -241,6 +266,41 @@ mod tests { ); } + // TDD red phase: the S8 backlog item "boundary overlays (S4)" — the + // analysis surfaces the focus track's phrase-boundary start ticks under + // the PPQN-scaled default config (the closure.rs referee precedent). + // References a field that does not exist yet, so the crate fails to + // compile until the green step. + + #[test] + fn analysis_surfaces_phrase_boundaries_of_the_focus_track() { + use griff_core::boundary::{detect_phrase_boundaries, BoundaryConfig}; + use griff_core::event::Ticks; + + let riff = vec![(40, 100), (43, 100), (45, 100), (47, 100), (40, 100)]; + let clean = vec![(60, 50), (62, 50), (64, 50)]; + let score = score_from(vec![riff.clone(), riff, clean]); + let a = analyze(&score); + + let ppq = u32::from(score.ticks_per_quarter); + let config = BoundaryConfig { + quantize_ticks: Ticks(ppq / 4), + min_gap: Ticks(ppq * 2), + ..BoundaryConfig::default() + }; + let expected: Vec = detect_phrase_boundaries(&score, a.focus_track, &config) + .iter() + .map(|b| b.start_tick.0) + .collect(); + assert_eq!(a.boundaries, expected, "the S4 start ticks, verbatim"); + } + + #[test] + fn empty_score_has_no_boundaries() { + let a = analyze(&score_from(vec![])); + assert!(a.boundaries.is_empty()); + } + #[test] fn merges_consecutive_equal_classes() { // Two loud riff bars (then one clean bar) → two sections. diff --git a/preview/src/golden/acted_80x20.txt b/preview/src/golden/acted_80x20.txt index c10ab0c8..298589eb 100644 --- a/preview/src/golden/acted_80x20.txt +++ b/preview/src/golden/acted_80x20.txt @@ -1,20 +1,20 @@ griff·preview demo.mid · ♩=120 · 2 bars · pos 2:1 SEC Riff Solo -F3 ││ ┃ │┌ Inspector ───────────────────┐ - ││ ┃ ││track Rhythm │ - ││ ┃ ││Solo │ - ││ ┃ ││bars 2–2 · 1 bar(s) │ - ││ ┃ ││curation — │ -C3 ││ ┃ ││ │ - ││ ┃██████████ ││transport │ - ││ ┃ ││♩=120 ▶ playing │ - ││ ┃ ││pos 2:1 │ - ││ ┃ ││ │ - ││ ┃ ││structure (S14) │ - ││ ┃ ││— │ - ││ ┃ ││ │ +F3 ││ ┊ ┃ │┌ Inspector ───────────────────┐ + ││ ┊ ┃ ││track Rhythm │ + ││ ┊ ┃ ││Solo │ + ││ ┊ ┃ ││bars 2–2 · 1 bar(s) │ + ││ ┊ ┃ ││curation — │ +C3 ││ ┊ ┃ ││ │ + ││ ┊ ┃██████████ ││transport │ + ││ ┊ ┃ ││♩=120 ▶ playing │ + ││ ┊ ┃ ││pos 2:1 │ + ││ ┊ ┃ ││ │ + ││ ┊ ┃ ││structure (S14) │ + ││ ┊ ┃ ││— │ + ││ ┊ ┃ ││ │ │███████████ ┃ ││complexity (S14) │ - ││ ┃ ││rhy 25% pit 50% │ - ││ ┃ ││tec 0% har 13% │ - ││ ┃ │└──────────────────────────────┘ + ││ ┊ ┃ ││rhy 25% pit 50% │ + ││ ┊ ┃ ││tec 0% har 13% │ + ││ ┊ ┃ │└──────────────────────────────┘ q quit · space play · ←/→ scroll · ↑/↓ pitch · +/- zoom · [/]/tab section · a/x \ No newline at end of file diff --git a/preview/src/golden/initial_80x20.txt b/preview/src/golden/initial_80x20.txt index d4eadf96..bc00f675 100644 --- a/preview/src/golden/initial_80x20.txt +++ b/preview/src/golden/initial_80x20.txt @@ -1,20 +1,20 @@ griff·preview demo.mid · ♩=120 · 2 bars · pos 1:1 SEC Riff Solo -F3 │┃ ╎ │┌ Inspector ───────────────────┐ - │┃ ╎ ││track Rhythm │ - │┃ ╎ ││Riff │ - │┃ ╎ ││bars 1–1 · 1 bar(s) │ - │┃ ╎ ││curation — │ -C3 │┃ ╎ ││ │ - │┃ ███████████ ││transport │ - │┃ ╎ ││♩=120 ⏸ paused │ - │┃ ╎ ││pos 1:1 │ - │┃ ╎ ││ │ - │┃ ╎ ││structure (S14) │ - │┃ ╎ ││— │ - │┃ ╎ ││ │ +F3 │┃ ┊ ╎ │┌ Inspector ───────────────────┐ + │┃ ┊ ╎ ││track Rhythm │ + │┃ ┊ ╎ ││Riff │ + │┃ ┊ ╎ ││bars 1–1 · 1 bar(s) │ + │┃ ┊ ╎ ││curation — │ +C3 │┃ ┊ ╎ ││ │ + │┃ ┊ ███████████ ││transport │ + │┃ ┊ ╎ ││♩=120 ⏸ paused │ + │┃ ┊ ╎ ││pos 1:1 │ + │┃ ┊ ╎ ││ │ + │┃ ┊ ╎ ││structure (S14) │ + │┃ ┊ ╎ ││— │ + │┃ ┊ ╎ ││ │ │┃██████████ ╎ ││complexity (S14) │ - │┃ ╎ ││rhy 25% pit 50% │ - │┃ ╎ ││tec 0% har 13% │ - │┃ ╎ │└──────────────────────────────┘ + │┃ ┊ ╎ ││rhy 25% pit 50% │ + │┃ ┊ ╎ ││tec 0% har 13% │ + │┃ ┊ ╎ │└──────────────────────────────┘ q quit · space play · ←/→ scroll · ↑/↓ pitch · +/- zoom · [/]/tab section · a/x \ No newline at end of file diff --git a/preview/src/scene.rs b/preview/src/scene.rs index a1b02fa7..4d52d829 100644 --- a/preview/src/scene.rs +++ b/preview/src/scene.rs @@ -57,6 +57,8 @@ pub enum CellRole { GridLine, /// A section boundary marker, carrying the section's class. SectionMark(BarClass), + /// An S4 phrase-boundary marker column. + BoundaryMark, /// A note block in the lane of the given index. Note(u16), /// The playhead column. @@ -231,6 +233,22 @@ fn resolve_plane( } } + // S4 phrase-boundary markers, over blank cells or gridlines. Placed + // after the section marks so a section keeps precedence when both land + // on one column. No scroll-origin guard: a boundary at the viewport's + // left edge is visible, and `visible_col` already drops earlier ticks + // (Codex P2, PR #39). + for &tick in &analysis.boundaries { + if let Some(col) = visible_col(vp, plot_w, tick) { + paint_column(plane, cols, rows, col, |c| { + if c.glyph == ' ' || c.glyph == '│' { + c.glyph = '┊'; + c.role = CellRole::BoundaryMark; + } + }); + } + } + place_notes(plane, view, vp, size); // Playhead, over everything. @@ -439,6 +457,7 @@ mod tests { ], metrics: None, complexity: None, + boundaries: Vec::new(), }; let vp = Viewport { scroll_tick: 0, @@ -458,6 +477,39 @@ mod tests { resolve(&view, &analysis, &vp, GridSize { cols: 40, rows: 14 }) } + // TDD red phase: phrase-boundary overlays (S8 × S4). References a field + // and a role that do not exist yet, so the crate fails to compile until + // the green step. + #[test] + fn a_boundary_at_the_scroll_origin_still_renders() { + // Codex P2 (PR #39): a boundary scrolled exactly to the viewport's + // left edge maps to the leftmost plot column and must render there; + // only ticks *before* the scroll origin are invisible. + let (view, mut analysis, mut vp) = fixture(); + analysis.boundaries = vec![480]; + vp.scroll_tick = 480; + let scene = resolve(&view, &analysis, &vp, GridSize { cols: 40, rows: 14 }); + let has_mark = (0..14) + .any(|r| scene.plane_cell(r, GUTTER).map(|c| c.role) == Some(CellRole::BoundaryMark)); + assert!(has_mark, "the left-edge boundary column carries the mark"); + } + + #[test] + fn boundary_marks_overlay_the_plane() { + let (view, mut analysis, vp) = fixture(); + analysis.boundaries = vec![480]; + let scene = resolve(&view, &analysis, &vp, GridSize { cols: 40, rows: 14 }); + // ticks_per_col 60 → tick 480 lands at plot column 8, scene column 13 + // (tick 960 would sit under the section mark, which takes precedence). + let col = GUTTER + 8; + let has_mark = (0..14) + .any(|r| scene.plane_cell(r, col).map(|c| c.role) == Some(CellRole::BoundaryMark)); + assert!(has_mark, "a visible boundary column carries the mark"); + let off_grid = (0..14) + .any(|r| scene.plane_cell(r, col + 1).map(|c| c.role) == Some(CellRole::BoundaryMark)); + assert!(!off_grid, "marks sit only on the boundary column"); + } + #[test] fn grid_has_exact_cell_counts() { let s = resolved(); diff --git a/preview/src/tui.rs b/preview/src/tui.rs index d8a767f8..67c94947 100644 --- a/preview/src/tui.rs +++ b/preview/src/tui.rs @@ -424,6 +424,10 @@ fn paint_cell(buf: &mut Buffer, x: u16, y: u16, cell: &SceneCell) { CellRole::Separator | CellRole::GridLine => { c.set_char(cell.glyph).set_style(dim); } + CellRole::BoundaryMark => { + c.set_char(cell.glyph) + .set_style(Style::new().fg(Color::Rgb(212, 177, 96))); + } CellRole::SectionMark(class) => { c.set_char(cell.glyph) .set_style(Style::new().fg(class_color(class))); @@ -598,6 +602,7 @@ mod tests { }, ], metrics: None, + boundaries: vec![480], complexity: Some(ComplexityProfile { rhythmic: 0.25, pitch: 0.5,