Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/decisions.log.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
6 changes: 5 additions & 1 deletion docs/stages/S8-preview-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ front-ends and audio build on them:
`griff-preview --record=<chunk.json>` 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

Expand Down
60 changes: 60 additions & 0 deletions preview/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -48,6 +50,8 @@ pub struct Analysis {
pub metrics: Option<StructureMetrics>,
/// The focus track's per-axis complexity (S14); `None` for an empty score.
pub complexity: Option<ComplexityProfile>,
/// Start ticks of the focus track's S4 phrase boundaries, in order.
pub boundaries: Vec<u32>,
}

/// Derives the [`Analysis`] for a score: pick the busiest track, classify each
Expand All @@ -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<u32> {
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
Expand Down Expand Up @@ -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<u32> = 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.
Expand Down
32 changes: 16 additions & 16 deletions preview/src/golden/acted_80x20.txt
Original file line number Diff line number Diff line change
@@ -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
32 changes: 16 additions & 16 deletions preview/src/golden/initial_80x20.txt
Original file line number Diff line number Diff line change
@@ -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
52 changes: 52 additions & 0 deletions preview/src/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -439,6 +457,7 @@ mod tests {
],
metrics: None,
complexity: None,
boundaries: Vec::new(),
};
let vp = Viewport {
scroll_tick: 0,
Expand All @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions preview/src/tui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -598,6 +602,7 @@ mod tests {
},
],
metrics: None,
boundaries: vec![480],
complexity: Some(ComplexityProfile {
rhythmic: 0.25,
pitch: 0.5,
Expand Down