diff --git a/Cargo.lock b/Cargo.lock index 311fa4c2..9347c21b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1828,6 +1828,7 @@ dependencies = [ "griff-core", "griff-pattern", "griff-swang", + "guitarpro", "midly", "serde_json", "thiserror 2.0.18", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 1e4a0afc..56a7262c 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -30,6 +30,12 @@ thiserror = { workspace = true } # Used only by the S0 characterization suite to synthesise `.mid` fixtures # directly, independent of griff's own export path. midly = { workspace = true } +# Same reasoning one format over: the SWG-4A-10 suite writes its Guitar Pro +# fixture with the parser crate's own serializer, so the bytes under test come +# from an encoder independent of griff's importer. Already in the tree via +# `griff-core`'s default `gp` feature — this adds no third-party code, only +# test-time access to it. +guitarpro = { version = "0.4", default-features = false } [lints] workspace = true diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 812be7d8..9d8aa3e2 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -13,5 +13,6 @@ pub mod curation_fs; pub mod generation_input; pub mod rhythm_pattern; +pub mod swang_dump; pub use griff_core::generation_input::primary_voice_note_count; diff --git a/cli/src/main.rs b/cli/src/main.rs index 8bd8fcd2..eb5a7ed6 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -11,6 +11,7 @@ use clap::{Parser, Subcommand}; use griff_cli::generation_input::{load_corpus_material, CorpusMaterial, GenerationInputError}; use griff_cli::primary_voice_note_count; use griff_cli::rhythm_pattern; +use griff_cli::swang_dump::dump_score; use griff_core::generation_input::{ranked_candidates, GenerationAsk, RankedSet}; use griff_core::{ boundary, @@ -33,6 +34,7 @@ use griff_core::{ split, structure, syncopation, technique, unfold, }; use griff_pattern::NodePath; +use griff_swang::exact::ExactWriteError; use griff_swang::{eval, syntax}; /// griff — guitar riff engine. @@ -299,6 +301,15 @@ enum SwangCommand { #[arg(value_name = "INPUT")] input: PathBuf, }, + /// Import a MIDI or Guitar Pro file and print its canonical Swang + /// level-2 exact text to stdout. Import warnings and diagnostics go to + /// stderr; the document goes to stdout and nowhere else. + Dump { + /// Path to the MIDI (`.mid`) or Guitar Pro + /// (`.gp3`/`.gp4`/`.gp5`/`.gpx`/`.gp`) file. + #[arg(value_name = "INPUT")] + input: PathBuf, + }, } fn run() -> Result<(), CliError> { @@ -379,6 +390,7 @@ fn run() -> Result<(), CliError> { SwangCommand::Fmt { input } => cmd_swang_fmt(&input), SwangCommand::Expand { input } => cmd_swang_expand(&input), SwangCommand::Build { input } => cmd_swang_build(&input), + SwangCommand::Dump { input } => cmd_swang_dump(&input), }, } } @@ -413,6 +425,31 @@ fn cmd_swang_fmt(path: &Path) -> Result<(), CliError> { Ok(()) } +/// `griff swang dump`: import through the existing adapters and print the +/// canonical level-2 exact text (SWG-4A-10). +/// +/// Two surfaces, and the split is the whole contract. The document goes to +/// stdout and only to stdout; import warnings and diagnostics go to stderr, +/// one rendered entry per warning and in the loss report's order. An entry +/// is not promised to occupy one physical line — an `Other` message may hold +/// a line break, and nothing here escapes it. +/// A warning stays in the document's `loss` block whether or not stderr also +/// mentioned it — `griff_cli::swang_dump` renders the two independently, and +/// this function only decides which stream each reaches. +/// +/// Both are computed before either is written, so a refusal prints nothing +/// at all rather than a truncated document. +fn cmd_swang_dump(path: &Path) -> Result<(), CliError> { + let data = fs::read(path)?; + let score = import::import_score_auto(&data)?; + let dumped = dump_score(&score)?; + for warning in &dumped.warnings { + eprintln!("warning: {warning}"); + } + print!("{}", dumped.document); + Ok(()) +} + /// `griff swang expand`: the pattern pipeline up to `map_rhythm`, printing /// the canonical expansion artifact to stdout (spec §3.5's CLI contract). /// The shared evaluator drives the same compiler the transport does, so law @@ -2238,6 +2275,9 @@ enum CliError { /// A Swang syntax diagnostic, already rendered against its script: /// `error[SWG____] (::): `. Swang(String), + /// The imported score is outside the exact writer's domain, so there is + /// no document to print (`docs/swang/exact-score-text.md` §3). + ExactWrite(ExactWriteError), } impl fmt::Display for CliError { @@ -2256,6 +2296,7 @@ impl fmt::Display for CliError { Self::Complement(e) => write!(f, "complement error: {e:?}"), Self::Pattern(d) => write!(f, "{d}"), Self::Swang(rendered) => write!(f, "{rendered}"), + Self::ExactWrite(e) => write!(f, "exact-text error: {e}"), } } } @@ -2266,6 +2307,12 @@ impl From for CliError { } } +impl From for CliError { + fn from(e: ExactWriteError) -> Self { + Self::ExactWrite(e) + } +} + impl From for CliError { fn from(e: MidiError) -> Self { Self::Midi(e) diff --git a/cli/src/swang_dump.rs b/cli/src/swang_dump.rs new file mode 100644 index 00000000..4106e476 --- /dev/null +++ b/cli/src/swang_dump.rs @@ -0,0 +1,83 @@ +//! SWG-4A-10: composing the two surfaces of `griff swang dump`. +//! +//! The command has exactly one interesting decision in it, and this module +//! is where it lives: a canonical `Score` produces **two** independent +//! renderings, and neither may consume the other. +//! +//! - The **document** is the canonical level-2 exact text, straight from +//! [`griff_swang::exact::write_score`]. It goes to stdout, whole or not at +//! all. Nothing here sorts, tidies, annotates, or re-spells it: the census +//! (`docs/swang/exact-score-text.md`) already decided every byte, and a +//! second opinion at the CLI would be a second formatter. +//! - The **warnings** are a human rendering of `score.loss` for stderr: one +//! rendered entry per `ImportWarning`, in the report's vector order. +//! +//! One *entry*, deliberately, and not one physical line. `ImportWarning` +//! includes `Other(String)`, whose message is unrestricted and may contain a +//! line break — SWG-4A-05 pinned that case in the exact text precisely +//! because the data can hold one. The exact writer escapes it, because its +//! grammar says a warning is one physical line. Nothing obliges the terminal +//! rendering to agree, and SWG-4A-10 introduces no escaping or sanitization +//! policy of its own: promising a line count here would be inventing a +//! contract to make a sentence true. +//! +//! The loss report appears on both surfaces, and that is deliberate rather +//! than redundant. `Score.loss` is a canonical fact the exact text is +//! obliged to carry (§2.8); the stderr rendering is a courtesy to whoever is +//! watching the terminal. Dropping the `loss` block because a human already +//! saw the warning would make the document depend on who was looking at it. + +use griff_core::score::{ImportWarning, Score}; +use griff_swang::exact::{write_score, ExactWriteError}; + +/// The two surfaces of one dump, both built before either is written. +/// +/// Returning them together is what makes "no partial document" structural +/// rather than a discipline: a refusal produces no `DumpOutput` at all, so +/// there is no half-formed value for a caller to print. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DumpOutput { + /// The canonical level-2 document, for stdout. + pub document: String, + /// One human-facing rendering per `ImportWarning`, in `LossReport` + /// vector order, for stderr. An entry is not promised to be a single + /// physical line: an `Other` message may carry its own line breaks, and + /// this surface adds no escaping policy to prevent that. + pub warnings: Vec, +} + +/// Renders `score` as the document and the warning entries that accompany +/// it, one entry per `ImportWarning`, in `LossReport` order. +/// +/// # Errors +/// [`ExactWriteError`] when the score is outside the exact writer's domain. +/// The writer produces no bytes in that case and neither does this. +pub fn dump_score(score: &Score) -> Result { + let document = write_score(score)?; + let warnings = score.loss.warnings.iter().map(describe_warning).collect(); + Ok(DumpOutput { document, warnings }) +} + +/// One import warning, in a sentence — verbatim for `Other`, whose message +/// is unrestricted and is neither escaped nor reflowed here. +/// +/// The exact text spells the same facts in its own frozen grammar; this is +/// the terminal rendering and is never the source of truth for any of them. +fn describe_warning(warning: &ImportWarning) -> String { + match warning { + ImportWarning::TrackNameInvalidUtf8 { track_index } => { + format!("track {track_index} has a name that is not valid UTF-8; it was dropped") + } + ImportWarning::SmpteTimingUnsupported => { + "the source used SMPTE timing, which griff does not support".to_owned() + } + ImportWarning::TempoApproximated { + bar_index, + nearest_micros, + } => format!( + "the tempo of bar {bar_index} has no exact microsecond form; \ + it was approximated to {nearest_micros}" + ), + ImportWarning::Other(message) => message.clone(), + } +} diff --git a/cli/tests/swang_dump.rs b/cli/tests/swang_dump.rs new file mode 100644 index 00000000..2fbd0252 --- /dev/null +++ b/cli/tests/swang_dump.rs @@ -0,0 +1,583 @@ +//! SWG-4A-10: `griff swang dump` — the exact writer's CLI edge. +//! +//! The command is a transport, not a second formatter. Its whole job is: +//! import through the existing adapters, hand the canonical `Score` to +//! `griff_swang::exact::write_score`, and put the result on stdout. Two +//! surfaces, kept apart on purpose: +//! +//! - **stdout** carries the canonical level-2 document and nothing else; +//! - **stderr** carries diagnostics and human-facing import warnings. +//! +//! A warning that reached `Score.loss` is *not* dropped from the exact text +//! because a human already saw it on stderr. The loss report is a canonical +//! fact; the stderr rendering is a courtesy. Conflating them would undo what +//! SWG-4A-05 spent twenty mutations establishing. + +// Reason: integration-test code. `unwrap`/`expect`/`panic` abort loudly with +// a clear message, which is exactly what a test harness wants. +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::missing_assert_message +)] + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::{env, fs}; + +use griff_cli::swang_dump::dump_score; +use griff_core::import::import_score_auto; +use griff_core::score::{LossReport, Score}; +use griff_swang::exact::write_score; + +// ── fixtures ──────────────────────────────────────────────────────────────── +// +// Both builders are deliberately *independent encoders*: `midly` and +// `guitarpro`'s own writer, never griff's export path. A fixture produced by +// the code under test would agree with it by construction. + +/// One sounding note in a MIDI fixture, in absolute ticks. +struct Note { + start: u32, + dur: u32, + key: u8, + vel: u8, +} + +/// A single-track SMF whose track name is written as raw bytes, so a caller +/// can hand it something that is not valid UTF-8. +fn midi_with_name(name: &'static [u8], notes: &[Note], ppqn: u16) -> Vec { + use midly::{ + num::{u15, u24, u28, u4, u7}, + Format, Header, MetaMessage, MidiMessage, Smf, Timing, TrackEvent, TrackEventKind, + }; + + let mut abs: Vec<(u32, TrackEventKind<'static>)> = vec![ + (0, TrackEventKind::Meta(MetaMessage::TrackName(name))), + ( + 0, + TrackEventKind::Meta(MetaMessage::TimeSignature(4, 2, 24, 8)), + ), + ( + 0, + TrackEventKind::Meta(MetaMessage::Tempo(u24::from_int_lossy(500_000))), + ), + ]; + let mut end = 0; + for n in notes { + abs.push(( + n.start, + TrackEventKind::Midi { + channel: u4::new(0), + message: MidiMessage::NoteOn { + key: u7::new(n.key), + vel: u7::new(n.vel), + }, + }, + )); + abs.push(( + n.start.saturating_add(n.dur), + TrackEventKind::Midi { + channel: u4::new(0), + message: MidiMessage::NoteOff { + key: u7::new(n.key), + vel: u7::new(0), + }, + }, + )); + end = end.max(n.start.saturating_add(n.dur)); + } + abs.push((end, TrackEventKind::Meta(MetaMessage::EndOfTrack))); + abs.sort_by_key(|&(tick, _)| tick); + + let mut track = Vec::new(); + let mut prev = 0; + for (tick, kind) in abs { + track.push(TrackEvent { + delta: u28::from_int_lossy(tick.saturating_sub(prev)), + kind, + }); + prev = tick; + } + + let mut smf = Smf::new(Header { + format: Format::SingleTrack, + timing: Timing::Metrical(u15::new(ppqn)), + }); + smf.tracks = vec![track]; + let mut bytes = Vec::new(); + smf.write_std(&mut bytes).expect("fixture must serialise"); + bytes +} + +/// A plain, lossless MIDI fixture: one named track, four quarter notes. +fn clean_midi() -> Vec { + midi_with_name( + b"Rhythm Gtr", + &[ + Note { + start: 0, + dur: 480, + key: 40, + vel: 96, + }, + Note { + start: 480, + dur: 480, + key: 43, + vel: 90, + }, + Note { + start: 960, + dur: 480, + key: 45, + vel: 88, + }, + Note { + start: 1440, + dur: 480, + key: 40, + vel: 84, + }, + ], + 480, + ) +} + +/// The same shape, but the track name is not valid UTF-8 — the importer +/// records `ImportWarning::TrackNameInvalidUtf8` and drops the name. +fn midi_with_a_lossy_name() -> Vec { + // 0xFF is never a valid UTF-8 leading byte. + midi_with_name( + b"Gtr \xff\xfe", + &[ + Note { + start: 0, + dur: 480, + key: 40, + vel: 96, + }, + Note { + start: 480, + dur: 480, + key: 43, + vel: 90, + }, + ], + 480, + ) +} + +/// Eleven note tracks, of which the ones at raw index 2 and 10 carry names +/// that are not valid UTF-8. Two warnings, in that order — and "track 10" +/// sorts *before* "track 2", so any tidying of the list is visible. +fn midi_with_two_lossy_names() -> Vec { + use midly::{ + num::{u15, u24, u28, u4, u7}, + Format, Header, MetaMessage, MidiMessage, Smf, Timing, TrackEvent, TrackEventKind, + }; + + let track = |name: &'static [u8], key: u8| -> Vec> { + vec![ + TrackEvent { + delta: u28::from_int_lossy(0), + kind: TrackEventKind::Meta(MetaMessage::TrackName(name)), + }, + TrackEvent { + delta: u28::from_int_lossy(0), + kind: TrackEventKind::Meta(MetaMessage::TimeSignature(4, 2, 24, 8)), + }, + TrackEvent { + delta: u28::from_int_lossy(0), + kind: TrackEventKind::Meta(MetaMessage::Tempo(u24::from_int_lossy(500_000))), + }, + TrackEvent { + delta: u28::from_int_lossy(0), + kind: TrackEventKind::Midi { + channel: u4::new(0), + message: MidiMessage::NoteOn { + key: u7::new(key), + vel: u7::new(90), + }, + }, + }, + TrackEvent { + delta: u28::from_int_lossy(480), + kind: TrackEventKind::Midi { + channel: u4::new(0), + message: MidiMessage::NoteOff { + key: u7::new(key), + vel: u7::new(0), + }, + }, + }, + TrackEvent { + delta: u28::from_int_lossy(0), + kind: TrackEventKind::Meta(MetaMessage::EndOfTrack), + }, + ] + }; + + let mut smf = Smf::new(Header { + format: Format::Parallel, + timing: Timing::Metrical(u15::new(480)), + }); + smf.tracks = (0..11_u8) + .map(|i| { + let name: &'static [u8] = if i == 2 || i == 10 { + b"Bad \xff" + } else { + b"Fine" + }; + track(name, 40_u8.saturating_add(i)) + }) + .collect(); + let mut bytes = Vec::new(); + smf.write_std(&mut bytes).expect("fixture must serialise"); + bytes +} + +/// A Guitar Pro (GP7 `.gp`) fixture, written by the `guitarpro` crate's own +/// serializer — an encoder independent of griff's importer. +fn guitar_pro_bytes() -> Vec { + use guitarpro::io::gpx::write_gp_bytes; + use guitarpro::model::legacy::beat::{Beat, Voice as GpVoice}; + use guitarpro::model::legacy::enums::NoteType; + use guitarpro::model::legacy::headers::MeasureHeader; + use guitarpro::model::legacy::key_signature::{Duration as GpDuration, TimeSignature as GpTs}; + use guitarpro::model::legacy::measure::Measure; + use guitarpro::model::legacy::note::Note as GpNote; + use guitarpro::model::legacy::track::Track as GpTrack; + + let header = MeasureHeader { + number: 1, + start: 0, + tempo: 120, + time_signature: GpTs::default(), + ..MeasureHeader::default() + }; + let note = GpNote { + value: 5, + string: 1, + velocity: 95, + kind: NoteType::Normal, + ..GpNote::default() + }; + let beat = Beat { + duration: GpDuration::default(), + notes: vec![note], + ..Beat::default() + }; + let voice = GpVoice { + measure_index: 0, + beats: vec![beat], + ..GpVoice::default() + }; + let measure = Measure { + number: 1, + start: 0, + track_index: 0, + header_index: 0, + voices: vec![voice], + ..Measure::default() + }; + let track = GpTrack { + name: String::from("Probe Gtr"), + measures: vec![measure], + ..GpTrack::default() + }; + let song = guitarpro::Song { + name: String::from("dump fixture"), + tempo: 120, + measure_headers: vec![header], + tracks: vec![track], + ..guitarpro::Song::default() + }; + + write_gp_bytes(&song).expect("the guitarpro writer must produce bytes") +} + +// ── harness ───────────────────────────────────────────────────────────────── + +/// Writes `bytes` to a uniquely named temp file and returns its path. +fn input(name: &str, bytes: &[u8]) -> PathBuf { + let path = env::temp_dir().join(format!("griff_swg_4a10_{name}")); + fs::write(&path, bytes).expect("temp input must write"); + path +} + +/// Runs the binary raw, for byte-exact stdout assertions. +fn dump(path: &Path) -> Output { + Command::new(env!("CARGO_BIN_EXE_griff")) + .args(["swang", "dump", path.to_str().unwrap()]) + .output() + .expect("griff binary must run") +} + +fn stdout_of(out: &Output) -> String { + String::from_utf8(out.stdout.clone()).expect("the document is UTF-8") +} + +fn stderr_of(out: &Output) -> String { + String::from_utf8_lossy(&out.stderr).into_owned() +} + +// ── 1, 2: both importers reach the writer ─────────────────────────────────── + +#[test] +fn a_guitar_pro_file_dumps_a_canonical_level_two_document() { + let path = input("gp_happy.gp", &guitar_pro_bytes()); + let out = dump(&path); + fs::remove_file(&path).ok(); + assert!( + out.status.success(), + "a valid Guitar Pro file must dump: {}", + stderr_of(&out) + ); + let text = stdout_of(&out); + assert!( + text.starts_with("swang 2\n"), + "the document opens with the frozen level-2 header: {text:?}" + ); + assert!( + text.contains("\"Probe Gtr\""), + "the imported track name is in the document: {text}" + ); +} + +#[test] +fn a_midi_file_dumps_a_canonical_level_two_document() { + let path = input("midi_happy.mid", &clean_midi()); + let out = dump(&path); + fs::remove_file(&path).ok(); + assert!( + out.status.success(), + "a valid MIDI file must dump: {}", + stderr_of(&out) + ); + let text = stdout_of(&out); + assert!( + text.starts_with("swang 2\n"), + "the document opens with the frozen level-2 header: {text:?}" + ); + assert!( + text.contains("\"Rhythm Gtr\""), + "the imported track name is in the document: {text}" + ); +} + +// ── 3: stdout is the document and only the document ───────────────────────── + +#[test] +fn stdout_carries_the_document_and_no_cli_chatter() { + let path = input("no_chatter.mid", &clean_midi()); + let out = dump(&path); + let text = stdout_of(&out); + let leaked = path.to_str().unwrap().to_owned(); + fs::remove_file(&path).ok(); + + for noise in ["Loaded", "warning:", "error:", "PPQN:", "Tracks:", "Bars:"] { + assert!( + !text.contains(noise), + "stdout must not carry CLI chatter ({noise}): {text}" + ); + } + assert!( + !text.contains(&leaked), + "the input path is not part of the canonical document: {text}" + ); + // The strongest form of the same claim: stdout is exactly the writer's + // output for the score the importer built, and holds nothing else. + let score = import_score_auto(&clean_midi()).expect("the fixture imports"); + assert_eq!( + text, + write_score(&score).expect("the fixture is inside the writer's domain"), + "stdout is the exact document, with nothing added or removed" + ); +} + +// ── 4: the two surfaces are independent ───────────────────────────────────── + +#[test] +fn an_import_warning_is_reported_to_stderr_and_kept_in_the_document() { + let bytes = midi_with_a_lossy_name(); + let score = import_score_auto(&bytes).expect("the fixture imports"); + assert!( + !score.loss.warnings.is_empty(), + "the fixture must actually be lossy, or this test proves nothing" + ); + + let path = input("lossy_name.mid", &bytes); + let out = dump(&path); + fs::remove_file(&path).ok(); + assert!(out.status.success(), "a lossy import still dumps"); + + let text = stdout_of(&out); + let errs = stderr_of(&out); + assert!( + text.contains("track_name_invalid_utf8"), + "the loss stays a canonical fact in the exact text: {text}" + ); + assert!( + errs.contains("track"), + "the human is told on stderr too: {errs:?}" + ); + assert!( + !errs.contains("swang 2"), + "stderr never carries the document: {errs:?}" + ); +} + +// ── 5: determinism ────────────────────────────────────────────────────────── + +#[test] +fn two_runs_of_one_file_produce_byte_identical_stdout() { + let path = input("determinism.mid", &clean_midi()); + let first = dump(&path); + let second = dump(&path); + fs::remove_file(&path).ok(); + assert!(first.status.success() && second.status.success()); + assert_eq!( + first.stdout, second.stdout, + "dump is a function of its input, byte for byte" + ); +} + +// ── 6: import failure ─────────────────────────────────────────────────────── + +#[test] +fn an_unimportable_file_fails_with_a_diagnostic_and_no_document() { + let path = input("garbage.mid", b"definitely not a music file"); + let out = dump(&path); + fs::remove_file(&path).ok(); + assert!( + !out.status.success(), + "an import failure is a non-zero exit" + ); + assert!( + out.stdout.is_empty(), + "nothing reaches stdout when there is no document: {:?}", + stdout_of(&out) + ); + assert!( + !stderr_of(&out).is_empty(), + "the failure is explained on stderr" + ); +} + +// ── 7: a refusal yields no partial document ───────────────────────────────── + +#[test] +fn a_score_outside_the_writer_domain_yields_no_document_at_all() { + // The composition returns the whole document or nothing: there is no + // half-written value for a caller to print. Proved at the seam the CLI + // calls, because the importer sanitises its own output — no file reaches + // `dump` that the writer would refuse (asserted below). + let refused = Score { + ticks_per_quarter: 0, + master_bars: Vec::new(), + tracks: Vec::new(), + source_meta: None, + loss: LossReport::new(), + }; + let verdict = dump_score(&refused); + assert!( + verdict.is_err(), + "ppqn 0 is outside the writer's domain, so there is no document" + ); +} + +#[test] +fn a_dumped_score_carries_its_document_and_its_warnings_together() { + // The other half of the same seam: on success both surfaces are built + // before either is written, so neither can be emitted without the other + // having been computed. + let score = import_score_auto(&midi_with_a_lossy_name()).expect("the fixture imports"); + let dumped = dump_score(&score).expect("the fixture is inside the writer's domain"); + assert_eq!( + dumped.document, + write_score(&score).expect("same writer, same bytes"), + "the document surface is the exact writer's output verbatim" + ); + assert_eq!( + dumped.warnings.len(), + score.loss.warnings.len(), + "one stderr entry per canonical warning — rendered, never consumed" + ); +} + +// ── 8: no hidden normalization ────────────────────────────────────────────── + +#[test] +fn the_cli_prints_exactly_what_the_writer_produces() { + // A fixture whose exact facts are easy to lose or reorder: a track name + // the importer had to drop (so `loss` is non-empty and ordered) alongside + // real notes. The comparison is against `write_score` of the imported + // score — not a hand-copied golden, which would let the CLI and the + // writer drift together into agreement about something wrong. + let bytes = midi_with_a_lossy_name(); + let path = input("no_normalization.mid", &bytes); + let out = dump(&path); + fs::remove_file(&path).ok(); + assert!(out.status.success(), "{}", stderr_of(&out)); + + let score = import_score_auto(&bytes).expect("the fixture imports"); + let expected = write_score(&score).expect("the fixture is inside the writer's domain"); + assert_eq!( + stdout_of(&out), + expected, + "the CLI is a transport: no sorting, no tidying, no second formatter" + ); +} + +#[test] +fn the_warning_lines_keep_the_loss_report_order() { + // Found by falsification: sorting the stderr entries survived the first + // pass, because nothing here looked at their order. `LossReport` appends + // and the exact walker compares positionally (§2.7); the human rendering + // is a rendering, so it inherits that order rather than choosing one. + let bytes = midi_with_two_lossy_names(); + let score = import_score_auto(&bytes).expect("the fixture imports"); + assert_eq!( + score.loss.warnings.len(), + 2, + "the fixture must produce exactly the two warnings this test reads" + ); + + let dumped = dump_score(&score).expect("the fixture is inside the writer's domain"); + let first = dumped.warnings.first().expect("two warnings were reported"); + let second = dumped.warnings.get(1).expect("two warnings were reported"); + assert!( + first.contains("track 2 "), + "vector order puts the lower raw index first: {first:?}" + ); + assert!( + second.contains("track 10 "), + "and the higher one second: {second:?}" + ); + + let mut tidied = dumped.warnings.clone(); + tidied.sort(); + assert_ne!( + tidied, dumped.warnings, + "the fixture must discriminate: if sorted order equalled vector \ + order, this test could not see the difference it exists to see" + ); + + // The same order survives the trip through the terminal. + let path = input("warning_order.mid", &bytes); + let out = dump(&path); + fs::remove_file(&path).ok(); + let errs = stderr_of(&out); + let at_two = errs + .find("track 2 ") + .expect("the first warning reached stderr"); + let at_ten = errs + .find("track 10 ") + .expect("the second warning reached stderr"); + assert!( + at_two < at_ten, + "stderr keeps the loss report's order too: {errs:?}" + ); +} diff --git a/docs/decisions.log.md b/docs/decisions.log.md index 2fbf69f0..9d55967b 100644 --- a/docs/decisions.log.md +++ b/docs/decisions.log.md @@ -2425,3 +2425,31 @@ Architectural decisions go to [`adr/`](adr/) instead. paths — `NotePosition` and `TechniqueEvidence` are composite canonical fields to the exact walker on purpose, and improving the comparator to make the test prettier would be 4A-05 editing something it was told not to touch. + +- 2026-08-21 — In the context of SWG-4A-10, facing a command that must show a + human what went wrong during import while also printing a document obliged + to carry the same facts, we decided to render `Score.loss` **twice** — as + the exact text's `loss` block on stdout and as one line per warning on + stderr — and against letting either surface consume the other, to achieve a + document that is a function of the score alone, accepting the apparent + redundancy of saying the same thing in two places. They are not the same + thing: stdout carries a canonical fact the grammar owns (§2.8), stderr a + courtesy to whoever is watching. Suppressing the block because a human had + already been told would make the canonical text depend on who was looking + at it, and would undo what SWG-4A-05 spent twenty mutations establishing. + The composition returns both surfaces together or neither, which is why "no + partial document" is structural here rather than a rule someone has to + remember. + +- 2026-08-21 — In the context of closing SWG-4A-10, facing an independent + review that found the entry above overclaiming its own surface, we decided + to correct the wording and against inventing the behaviour that would have + made it true, to achieve a documented contract the code actually keeps. + "One line per warning on stderr" is not something 4A-10 can promise: + `ImportWarning::Other(String)` is unrestricted and may contain an embedded + LF — SWG-4A-05 pinned exactly that case in the exact text, where the + grammar escapes it. The terminal rendering has no such grammar, and 4A-10 + introduces no escaping or sanitization policy of its own. The invariant is + one rendered entry per `LossReport` element, preserving vector order. + Production behavior is unchanged; so is the test suite, because freezing a + stderr line policy the contract never asked for is the defect, not the fix. diff --git a/docs/swang/foundation-backlog.md b/docs/swang/foundation-backlog.md index f67a9b21..ca607c3d 100644 --- a/docs/swang/foundation-backlog.md +++ b/docs/swang/foundation-backlog.md @@ -121,7 +121,7 @@ recorded in `decisions.log.md` if reversed. | SWG-4A-07 | Parser: exact scalar types | code | 4A-06 | | SWG-4A-08 | Parser: structural tree | code | 4A-07, 4A-02 | | SWG-4A-09 | Checked `ScoreBuilder` | code | 4A-08 | -| SWG-4A-10 | `griff swang dump` | CLI | 4A-05 | +| SWG-4A-10 | `griff swang dump` *(done)* | CLI | 4A-05 | | SWG-4A-11 | `griff swang verify` | CLI | 4A-09, 4A-10 | | SWG-4A-12 | The three round-trip laws + mutation matrix | code | 4A-09, 4A-05, CORE-01 | | SWG-4A-13 | Expressibility negative matrix | code | 4A-12 | @@ -683,7 +683,7 @@ syntactically valid text There is no third outcome, and no partially valid `Score` is ever returned. -### SWG-4A-10 — `griff swang dump` +### SWG-4A-10 — `griff swang dump` *(done)* **Kind:** CLI. **Depends on:** 4A-05 @@ -697,6 +697,38 @@ in the exact text; the score goes to stdout and only to stdout; diagnostics and import warnings go to stderr; no hidden normalization; two runs produce byte-identical output. +Acceptance, all met: + +- both importer branches reach the writer — a Guitar Pro fixture and a MIDI + fixture each produce a document opening `swang 2`, from encoders + independent of griff (`guitarpro`'s own serializer and `midly`); +- stdout carries the document and nothing else, checked as the absence of + CLI chatter **and**, more strongly, as equality with + `write_score(import_score_auto(bytes))` — a hand-copied golden would let + the CLI and the writer drift together into agreeing about something wrong; +- an import warning is reported on stderr **and** kept in the exact text. + Two surfaces, not one: stdout carries a canonical fact, stderr a courtesy; +- one rendered stderr entry per `ImportWarning`, keeping the loss report's + vector order (§2.7), on a fixture built so that sorted order and vector + order visibly disagree. An entry, not a physical line: `Other(String)` is + unrestricted and may contain a line break, and this task adds no escaping + policy for the terminal; +- two runs of one file produce byte-identical stdout; +- an unimportable file exits non-zero with an empty stdout; +- a score outside the writer's domain yields no document at all. Proved at + the `griff_cli::swang_dump` seam, because no file reaching `dump` is + refused — the MIDI reader rejects ppqn 0 and normalises a zero meter + numerator to 4/4 before the writer ever sees them. The composition returns + the whole document or nothing, so there is no partial value to print; +- eleven mutations of the dump path, ten caught on the first pass. The + eleventh — alphabetising the stderr entries — survived, and its guard is + the fourth commit. + +The exact writer is unchanged: the diff is one `griff-cli` module, one clap +subcommand, one dispatch arm, and one `CliError` variant. `griff swang +verify` (4A-11) now has its CLI-side dependency; its other one, 4A-09, is +still open. + ### SWG-4A-11 — `griff swang verify` **Kind:** CLI. **Depends on:** 4A-09, 4A-10 @@ -1216,14 +1248,16 @@ INF-01 status sync (done) └─→ 4A-01 exact grammar (done) │ ├─→ 4A-03 → 4A-04 → CORE-01 → 4A-05 → 4A-10 writer lane - │ (done) (done) (done) (done) ↑ - │ │ next + │ (done) (done) (done) (done) (done) + │ │ │ └─→ also gated 4A-12 and the level-2 freeze │ │ the exact writer is complete at 4A-05; 4A-10 is a CLI │ surface over it, not another slice of it │ └─→ 4A-02 → INF-04 → INF-06 → 4A-06 parser skeleton + ↑ + next -> 4A-02..4A-09 writer / parser / builder -> 4A-10..4A-14 dump / verify / laws / fuzz -> 4B corpus acceptance