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
107 changes: 107 additions & 0 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,29 @@ enum Command {
bars: usize,
},

/// Arrange a complementary part (S13) for a tab's primary track — a second
/// guitar/bass derived from it — and write both parts to a MIDI file.
Complement {
/// Source MIDI or Guitar Pro file whose primary track is part A.
#[arg(value_name = "INPUT")]
input: PathBuf,
/// Output `.mid` file for A plus the generated part B.
#[arg(value_name = "OUTPUT")]
output: PathBuf,
/// Relation mode: `rhythm_lock`, `register_contrast`, `call_response`,
/// `support_layer`, `octave_double`, or `counter_melody`.
#[arg(long, default_value = "rhythm_lock")]
mode: String,
/// Deterministic seed — the same seed always yields the same part.
#[arg(long, default_value_t = 0)]
seed: u64,
/// Semitone shift of B's register relative to A (e.g. -12 = octave
/// down). Defaults per mode: `octave_double` and `register_contrast`
/// reject a zero shift, so they default to -12; others to 0.
#[arg(long, allow_hyphen_values = true)]
offset: Option<i8>,
},

/// Interactively curate a MIDI or Guitar Pro file into a corpus `ChunkMeta` JSON record.
Curate {
/// Path to the MIDI or Guitar Pro file to curate.
Expand Down Expand Up @@ -133,6 +156,13 @@ fn run() -> Result<(), CliError> {
seed,
bars,
} => cmd_generate(&input, &output, seed, bars),
Command::Complement {
input,
output,
mode,
seed,
offset,
} => cmd_complement(&input, &output, &mode, seed, offset),
Command::Curate {
path,
output,
Expand Down Expand Up @@ -618,6 +648,73 @@ fn first_bar_rhythm(score: &Score) -> Vec<Ticks> {
vec![quarter; 4]
}

/// Arranges a complementary part B (S13) for the primary track of `input` and
/// writes A plus B to a MIDI file.
fn cmd_complement(
input: &Path,
output: &Path,
mode: &str,
seed: u64,
offset: Option<i8>,
) -> Result<(), CliError> {
let data = fs::read(input)?;
let score = import::import_score_auto(&data)?;
let relation = parse_relation_mode(mode)?;
// Part A is the first track that actually sounds: GP import keeps rest-only
// tracks, so track 0 may be empty while a later track carries the riff.
let track_index = score
.tracks
.iter()
.position(|t| primary_voice_note_count(t) > 0)
.ok_or(CliError::Complement(
complement::ComplementError::PartHasNoNotes,
))?;
let spec = complement::ComplementSpec {
mode: relation,
register_offset: offset.unwrap_or_else(|| default_offset(relation)),
};
let candidate =
complement::arrange_complement(&score, track_index, spec, generate::GenerationSeed(seed))?;
let out_bytes = midi::export_score(&candidate.score)?;
fs::write(output, &out_bytes)?;
println!(
"complement ({label}, seed {seed}) — part B appended as track {b} \
({n} bytes) -> {out}",
label = relation.label(),
b = candidate.part_b_index,
n = out_bytes.len(),
out = output.display(),
);
Ok(())
}

/// The default register shift when `--offset` is omitted: an octave down for
/// the modes that reject a zero shift, otherwise none.
const fn default_offset(mode: complement::RelationMode) -> i8 {
match mode {
complement::RelationMode::OctaveDouble | complement::RelationMode::RegisterContrast => -12,
_ => 0,
}
}

/// Parses a `--mode` string into a [`complement::RelationMode`].
fn parse_relation_mode(mode: &str) -> Result<complement::RelationMode, CliError> {
Ok(match mode {
"rhythm_lock" => complement::RelationMode::RhythmLock,
"register_contrast" => complement::RelationMode::RegisterContrast,
"call_response" => complement::RelationMode::CallResponse,
"support_layer" => complement::RelationMode::SupportLayer,
"octave_double" => complement::RelationMode::OctaveDouble,
"counter_melody" => complement::RelationMode::CounterMelody,
other => {
return Err(CliError::Argument(format!(
"unknown complement mode '{other}' (try rhythm_lock, register_contrast, \
call_response, support_layer, octave_double, counter_melody)"
)));
}
})
}

fn cmd_curate(path: &Path, output: Option<&Path>, ensemble: bool) -> Result<(), CliError> {
let data = fs::read(path)?;
let score = import::import_score_auto(&data)?;
Expand Down Expand Up @@ -964,8 +1061,10 @@ enum CliError {
Import(ImportError),
Midi(MidiError),
Json(serde_json::Error),
Argument(String),
Ensemble(String),
Generate(generate::GenerationError),
Complement(complement::ComplementError),
}

impl fmt::Display for CliError {
Expand All @@ -975,8 +1074,10 @@ impl fmt::Display for CliError {
Self::Import(e) => write!(f, "import error: {e}"),
Self::Midi(e) => write!(f, "MIDI error: {e}"),
Self::Json(e) => write!(f, "JSON error: {e}"),
Self::Argument(msg) => write!(f, "argument error: {msg}"),
Self::Ensemble(msg) => write!(f, "ensemble error: {msg}"),
Self::Generate(e) => write!(f, "generation error: {e:?}"),
Self::Complement(e) => write!(f, "complement error: {e:?}"),
}
}
}
Expand Down Expand Up @@ -1005,6 +1106,12 @@ impl From<generate::GenerationError> for CliError {
}
}

impl From<complement::ComplementError> for CliError {
fn from(e: complement::ComplementError) -> Self {
Self::Complement(e)
}
}

// ── tests ─────────────────────────────────────────────────────────────────────

/// Red → green for the Codex P2 finding on PR #36: ensemble part selection
Expand Down
85 changes: 81 additions & 4 deletions cli/tests/cli.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! S0 golden/characterization tests for the `griff` CLI.
//!
//! Every CLI subcommand (`import`, `inspect`, `export`, `classify`,
//! `structure`, `phrases`, `generate`) is run against every committed fixture
//! and its stdout/stderr pinned to a golden snapshot. These tests describe what
//! the CLI *does* today; they must not be "fixed" by changing expectations
//! without a deliberate re-bless.
//! `structure`, `phrases`, `generate`, `complement`) is run against every
//! committed fixture and its stdout/stderr pinned to a golden snapshot. These
//! tests describe what the CLI *does* today; they must not be "fixed" by
//! changing expectations without a deliberate re-bless.
//!
//! Regenerate fixtures: `cargo test -p griff-cli -- --ignored regenerate`
//! Re-bless snapshots: `GRIFF_BLESS=1 cargo test -p griff-cli`
Expand Down Expand Up @@ -147,6 +147,83 @@ fn generate_golden() {
}
}

#[test]
fn complement_golden() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Split complement CLI into red/green commits

AGENTS.md's mandatory TDD workflow for /workspace/griff requires non-trivial changes to commit failing tests before implementation and explicitly says reviewers must judge the commit sequence, not just the flattened diff. This commit introduces complement_golden and the CLI implementation/snapshots together, with no preceding red test commit for the complement subcommand on this branch, so the sequence is non-compliant; split the golden test commit from the implementation commit.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already satisfied on the branch — the complement subcommand was committed red→green:

  • 512c26d test(cli): pin a complement subcommand via golden test (red) — touches only cli/tests/cli.rs (+26/−4). The new complement_golden test fails at this commit: clap rejects the unknown complement subcommand and the snapshot files don't exist yet.
  • a0930e1 feat(cli): complement subcommand … (green) — adds cli/src/main.rs plus the snapshots that make the pinned test pass.

The flattened base→head diff collapses the two into one change, but the commit sequence has the failing-test-first split AGENTS.md asks reviewers to judge.


Generated by Claude Code

for (name, _) in fixtures() {
let src = fixture_path(name);
let dst = env::temp_dir().join(format!("griff_s0_complement_{name}.mid"));
fs::remove_file(&dst).ok();

let out = griff(
&["complement", src.to_str().unwrap(), dst.to_str().unwrap()],
dst.to_str(),
);
let out = out.replace(src.to_str().unwrap(), "<SRC>");
assert_golden(&format!("complement__{name}"), &out);

assert!(
dst.exists(),
"complement must have written the output file for `{name}`"
);
fs::remove_file(&dst).ok();
}
}

/// Pins the non-default argument path: a different `--mode`, an explicit
/// `--seed`, and a negative `--offset` (which clap must accept as a value).
#[test]
fn complement_options_golden() {
let src = fixture_path("simple_4_4");
let dst = env::temp_dir().join("griff_s0_complement_opts.mid");
fs::remove_file(&dst).ok();

let out = griff(
&[
"complement",
src.to_str().unwrap(),
dst.to_str().unwrap(),
"--mode",
"counter_melody",
"--seed",
"42",
"--offset",
"-12",
],
dst.to_str(),
);
let out = out.replace(src.to_str().unwrap(), "<SRC>");
assert_golden("complement__opts_counter_melody", &out);

assert!(dst.exists(), "complement must have written the output file");
fs::remove_file(&dst).ok();
}

/// Pins the invalid-mode failure path: a clear argument error, no output file.
#[test]
fn complement_invalid_mode_golden() {
let src = fixture_path("simple_4_4");
let dst = env::temp_dir().join("griff_s0_complement_invalid.mid");
fs::remove_file(&dst).ok();

let out = griff(
&[
"complement",
src.to_str().unwrap(),
dst.to_str().unwrap(),
"--mode",
"bad_mode",
],
dst.to_str(),
);
let out = out.replace(src.to_str().unwrap(), "<SRC>");
assert_golden("error__complement_invalid_mode", &out);

assert!(
!dst.exists(),
"complement must not write an output file when the mode is invalid"
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// A missing input file is observable CLI behavior worth pinning.
#[test]
fn missing_file_golden() {
Expand Down
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__multi_track.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT>
exit: 0
--- stdout ---
complement (rhythm_lock, seed 0) — part B appended as track 2 (276 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__opts_counter_melody.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT> --mode counter_melody --seed 42 --offset -12
exit: 0
--- stdout ---
complement (counter_melody, seed 42) — part B appended as track 1 (246 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__seven_eight.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT>
exit: 0
--- stdout ---
complement (rhythm_lock, seed 0) — part B appended as track 1 (192 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__simple_4_4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT>
exit: 0
--- stdout ---
complement (rhythm_lock, seed 0) — part B appended as track 1 (282 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__tempo_change.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT>
exit: 0
--- stdout ---
complement (rhythm_lock, seed 0) — part B appended as track 1 (290 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/complement__two_phrases.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT>
exit: 0
--- stdout ---
complement (rhythm_lock, seed 0) — part B appended as track 1 (500 bytes) -> <OUT>
--- stderr ---
5 changes: 5 additions & 0 deletions cli/tests/snapshots/error__complement_invalid_mode.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$ griff complement <SRC> <OUT> --mode bad_mode
exit: 1
--- stdout ---
--- stderr ---
error: argument error: unknown complement mode 'bad_mode' (try rhythm_lock, register_contrast, call_response, support_layer, octave_double, counter_melody)