diff --git a/cli/src/generation_input.rs b/cli/src/generation_input.rs index 9ba55211..3212074b 100644 --- a/cli/src/generation_input.rs +++ b/cli/src/generation_input.rs @@ -114,8 +114,8 @@ mod tests { use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::midi; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Track, Voice, }; use griff_core::slice::TickRange; use std::cell::Cell; @@ -137,7 +137,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).unwrap().saturating_mul(1920); MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start.saturating_add(1920))) .unwrap(), time_signature: TimeSignature { diff --git a/cli/src/main.rs b/cli/src/main.rs index 4ff840a9..8bd8fcd2 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -27,7 +27,7 @@ use griff_core::{ ingest, midi::{self, MidiError}, novelty, rerank, - score::{AtomEvent, LossReport, Score, Track, Voice}, + score::{index_from_ordinal, AtomEvent, LossReport, Score, Track, Voice}, scoring, slice::{self, TickRange}, split, structure, syncopation, technique, unfold, @@ -860,12 +860,20 @@ fn cmd_phrases(path: &Path) -> Result<(), CliError> { /// The index of the master bar containing `tick`, or the last bar when the tick /// falls at or past the end of the timeline. -fn bar_at_tick(score: &Score, tick: u32) -> usize { +/// +/// Display only. The two branches are not quite the same kind of number — the +/// hit returns a stored canonical index, the fallback the last *ordinal* — and +/// that pre-existing conflation is left alone here; SWG-CORE-01 only makes +/// both sides the same width so neither has to be narrowed to meet the other. +fn bar_at_tick(score: &Score, tick: u32) -> u64 { score .master_bars .iter() .find(|mb| tick >= mb.tick_range.start.0 && tick < mb.tick_range.end.0) - .map_or_else(|| score.master_bars.len().saturating_sub(1), |mb| mb.index) + .map_or_else( + || index_from_ordinal(score.master_bars.len().saturating_sub(1)), + |mb| mb.index, + ) } /// Renders the heuristic signals that fired at a phrase boundary as a comma @@ -2328,8 +2336,8 @@ mod tests { use griff_core::generate::RhythmTemplate; use griff_core::gesture; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, SourceMeta, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, SourceMeta, Voice, }; use griff_core::slice::TickRange; @@ -2567,7 +2575,7 @@ mod tests { } /// One 4/4 bar with explicit bounds (no arithmetic in the fixture). - fn mbar(index: usize, start: u32, end: u32) -> MasterBar { + fn mbar(index: u64, start: u32, end: u32) -> MasterBar { MasterBar { index, tick_range: TickRange::new(Ticks(start), Ticks(end)).expect("ordered"), @@ -2732,7 +2740,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).unwrap_or(0).saturating_mul(1920); MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start.saturating_add(1920))) .expect("ordered"), time_signature: TimeSignature { @@ -3039,7 +3047,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).expect("small index").saturating_mul(1920); MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start.saturating_add(1920))) .expect("ordered"), time_signature: TimeSignature { diff --git a/cockpit/src/lib.rs b/cockpit/src/lib.rs index 4b91e43d..90707c25 100644 --- a/cockpit/src/lib.rs +++ b/cockpit/src/lib.rs @@ -4044,7 +4044,7 @@ mod tests { const BAR: u32 = 3840; let master_bars = (0..bars) .map(|i| MasterBar { - index: i as usize, + index: u64::from(i), tick_range: TickRange::new(Ticks(i * BAR), Ticks((i + 1) * BAR)).expect("range"), time_signature: TimeSignature::new(4, 4).expect("4/4"), tempo: Tempo::from_bpm_integer(120).expect("120"), diff --git a/cockpit/src/swang.rs b/cockpit/src/swang.rs index 2380e78f..ca1c1918 100644 --- a/cockpit/src/swang.rs +++ b/cockpit/src/swang.rs @@ -307,8 +307,8 @@ fn line_col(source: &str, offset: u32) -> (u32, u32) { mod tests { use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -321,7 +321,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/src/candidate_chain.rs b/core/src/candidate_chain.rs index 913c8130..bf275efd 100644 --- a/core/src/candidate_chain.rs +++ b/core/src/candidate_chain.rs @@ -1028,8 +1028,9 @@ mod tests { use crate::layered_path::{PathError, StateId}; use crate::rerank::SetCandidate; use crate::score::{ - AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, ImportWarning, LossReport, - MasterBar, RepeatMarker, Score, SourceMeta, TechniqueSpan, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, + ImportWarning, LossReport, MasterBar, RepeatMarker, Score, SourceMeta, TechniqueSpan, + Track, Voice, }; use crate::scoring::{Axes, Axis, Scored, WeightPolicy}; use crate::slice::TickRange; @@ -1049,7 +1050,7 @@ mod tests { for (index, notes) in bars.iter().enumerate() { let start = u32::try_from(index).unwrap() * BAR; master_bars.push(MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).unwrap(), time_signature: TimeSignature::new(4, 4).unwrap(), tempo: Tempo::from_bpm_integer(120).unwrap(), @@ -1512,7 +1513,7 @@ mod tests { /// Every fact of a master bar, in a comparable form (`MasterBar` is not /// `PartialEq`, and `Tempo` holds an `f64`). - fn bar_facts(bar: &MasterBar) -> (usize, TickRange, TimeSignature, Tempo, RepeatMarker) { + fn bar_facts(bar: &MasterBar) -> (u64, TickRange, TimeSignature, Tempo, RepeatMarker) { ( bar.index, bar.tick_range, diff --git a/core/src/dump.rs b/core/src/dump.rs index 7148a456..e3dee9bf 100644 --- a/core/src/dump.rs +++ b/core/src/dump.rs @@ -49,8 +49,12 @@ pub struct NormTrack { /// A normalized bar: shared transport plus the track's voices in it. #[derive(Debug, Clone, PartialEq, Serialize)] pub struct NormBar { - /// Zero-based bar index. - pub index: usize, + /// Zero-based bar index, copied from the stored `MasterBar::index`. + /// + /// Carries a canonical value, so it carries the canonical width: reducing + /// it to `usize` here would be a truncating conversion on a 32-bit target, + /// inside a type that is serialized (spec §1.2, SWG-CORE-01). + pub index: u64, /// Meter as `[numerator, denominator]`. pub time_sig: [u8; 2], /// Tempo in BPM at the bar start. diff --git a/core/src/generate.rs b/core/src/generate.rs index 3e38d83c..d82f077a 100644 --- a/core/src/generate.rs +++ b/core/src/generate.rs @@ -5,8 +5,8 @@ use crate::event::{ }; use crate::pitch::{PitchClassSet, PitchRange, PitchSelectionError, ScaleLadder}; use crate::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use crate::slice::TickRange; @@ -447,7 +447,7 @@ fn bars_to_score( let tick_range = TickRange::new(bar_start, bar_end).map_err(|_| GenerationError::InvalidConstraints)?; master_bars.push(MasterBar { - index, + index: index_from_ordinal(index), tick_range, time_signature: c.time_signature, tempo: c.tempo, diff --git a/core/src/generation_input.rs b/core/src/generation_input.rs index 6afcfe70..e4b41789 100644 --- a/core/src/generation_input.rs +++ b/core/src/generation_input.rs @@ -521,8 +521,8 @@ mod tests { use crate::corpus::{ChunkId, ChunkMeta, SourceFormat, SourceRef}; use crate::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use crate::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use crate::slice::TickRange; @@ -562,7 +562,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).expect("small").saturating_mul(1920); MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start.saturating_add(1920))) .expect("ordered"), time_signature: TimeSignature { diff --git a/core/src/gp.rs b/core/src/gp.rs index a4bf5b70..19bdaf9d 100644 --- a/core/src/gp.rs +++ b/core/src/gp.rs @@ -31,8 +31,9 @@ use crate::{ TechniqueEvidence, Tempo, Ticks, TimeSignature, Tuning, Velocity, }, score::{ - AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, ImportWarning, LossReport, - MasterBar, RepeatMarker, Score, SourceMeta, TechniqueSpan, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, + ImportWarning, LossReport, MasterBar, RepeatMarker, Score, SourceMeta, TechniqueSpan, + Track, Voice, }, slice::TickRange, }; @@ -279,7 +280,7 @@ fn build_gp_master_bars( .unwrap_or(Tempo::FALLBACK_120); MasterBar { - index: idx, + index: index_from_ordinal(idx), tick_range: TickRange { start: Ticks(start), end: Ticks(end), diff --git a/core/src/midi.rs b/core/src/midi.rs index 2ff490f5..24b286cb 100644 --- a/core/src/midi.rs +++ b/core/src/midi.rs @@ -15,8 +15,8 @@ use crate::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, ValidationError, Velocity}, fretboard::{self, assign_inferred_positions, FingeringWeights}, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, ImportWarning, LossReport, MasterBar, - RepeatMarker, Score, SourceMeta, Track as ScoreTrack, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, ImportWarning, + LossReport, MasterBar, RepeatMarker, Score, SourceMeta, Track as ScoreTrack, Voice, }, slice::TickRange, }; @@ -345,7 +345,7 @@ fn abs_to_delta( /// other tempo converts to its nearest in-range value and records a /// [`ImportWarning::TempoApproximated`] on `loss` (S16 Phase 4-pre A: an /// approximation is a reported fact, never a silent rounding). -fn tempo_to_micros(tempo: Tempo, bar_index: usize, loss: &mut LossReport) -> u32 { +fn tempo_to_micros(tempo: Tempo, bar_index: u64, loss: &mut LossReport) -> u32 { let max_u24 = u32::from(u24::max_value()); if let Some(exact) = tempo.to_micros_per_quarter_exact() { if (1..=max_u24).contains(&exact) { @@ -513,7 +513,7 @@ fn build_master_bars( .map_err(|_| MidiError::TickOverflow)?; master_bars.push(MasterBar { - index, + index: index_from_ordinal(index), tick_range, time_signature: sig, tempo, @@ -545,7 +545,7 @@ fn build_score_track( let mut loss = LossReport::new(); let name = name_result.unwrap_or_else(|()| { loss.add(ImportWarning::TrackNameInvalidUtf8 { - track_index: raw_idx, + track_index: index_from_ordinal(raw_idx), }); None }); @@ -829,6 +829,7 @@ mod tests { bar_ticks, build_master_bars, build_score_meta_track, export_score, import_score, tempo_to_micros, MidiError, Ppqn, }; + use crate::score::index_from_ordinal; use crate::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, score::{ @@ -893,7 +894,7 @@ mod tests { // different exact tempo — projected again (clean), no second event. let tempo_same_mpq = Tempo::from_micros_per_quarter(495_868).expect("495 868 µs is valid"); let bar = |index: usize, tempo: Tempo| MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new( Ticks(u32::try_from(index).expect("small") * 1920), Ticks((u32::try_from(index).expect("small") + 1) * 1920), @@ -953,7 +954,7 @@ mod tests { ); let bar = |index: usize, tempo: Tempo| MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new( Ticks(u32::try_from(index).expect("small") * 1920), Ticks((u32::try_from(index).expect("small") + 1) * 1920), diff --git a/core/src/score.rs b/core/src/score.rs index 1d20f793..a42d4d48 100644 --- a/core/src/score.rs +++ b/core/src/score.rs @@ -25,6 +25,24 @@ use crate::{ slice::TickRange, }; +// ── ordinal ↔ canonical index ────────────────────────────────────────────── + +/// Widens an operational vector position into the canonical index width. +/// +/// The two are different things and the census says so (H4): an ordinal is +/// where an element sits in a `Vec`, a canonical index is a stored exact fact +/// that may disagree with it. Importers legitimately derive the second from +/// the first, and this is the only sanctioned way to do it. +/// +/// Total and lossless: `usize` is at most 64 bits on every target Rust +/// supports. It is a named function rather than a bare cast so that each +/// ordinal → canonical crossing is greppable, and so that the reverse +/// direction — which is *not* total — cannot be written by accident. +#[must_use] +pub const fn index_from_ordinal(ordinal: usize) -> u64 { + ordinal as u64 +} + // ── loss reporting ───────────────────────────────────────────────────────────── /// A loss or approximation incurred during format import or export. @@ -32,8 +50,9 @@ use crate::{ pub enum ImportWarning { /// A track name was present but could not be decoded as UTF-8. TrackNameInvalidUtf8 { - /// Zero-based index of the affected track. - track_index: usize, + /// Zero-based index of the affected track. Fixed-width so the value + /// means the same thing on every target (SWG-CORE-01). + track_index: u64, }, /// The MIDI file used SMPTE/timecode timing; `griff` does not yet support /// it. @@ -43,7 +62,8 @@ pub enum ImportWarning { /// approximation is a reported fact, never a silent rounding). TempoApproximated { /// Zero-based master-bar index whose tempo was approximated. - bar_index: usize, + /// Fixed-width (SWG-CORE-01). + bar_index: u64, /// The microseconds-per-quarter value actually written. nearest_micros: u32, }, @@ -258,8 +278,13 @@ impl RepeatMarker { /// `MasterBar` is the single source of truth for transport (ADR-0003). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct MasterBar { - /// Zero-based bar index. - pub index: usize, + /// Zero-based bar index, as stored. + /// + /// Not the vector position: an importer may disagree with the ordinal and + /// the disagreement is itself an exact fact. Fixed-width rather than + /// platform-sized, so a score written on a 64-bit host means the same + /// thing on a 32-bit one (spec §1.2, SWG-CORE-01). + pub index: u64, /// Absolute half-open tick range `[start, end)` of this bar. pub tick_range: TickRange, /// Meter of this bar. diff --git a/core/src/semantic_diff.rs b/core/src/semantic_diff.rs index ef46a95a..73f84dad 100644 --- a/core/src/semantic_diff.rs +++ b/core/src/semantic_diff.rs @@ -176,7 +176,10 @@ pub enum SemanticPathSegment { ordinal: usize, /// The agreed stored bar index; `None` when the index itself /// differs, so both diff directions render the same path. - index: Option, + /// + /// A canonical value, so the canonical width (SWG-CORE-01) — the + /// `ordinal` beside it is an operational position and stays `usize`. + index: Option, }, /// A track, by position. Track { @@ -218,7 +221,9 @@ pub enum SemanticPathSegment { /// Zero-based position in the projection's `bars`. ordinal: usize, /// The agreed stored bar index; `None` when it differs. - index: Option, + /// + /// Canonical width, for the same reason as the exact segment above. + index: Option, }, /// A normalized-projection note, by position. Note { diff --git a/core/src/slice.rs b/core/src/slice.rs index 16c1a0f4..75182a17 100644 --- a/core/src/slice.rs +++ b/core/src/slice.rs @@ -5,8 +5,8 @@ use std::ops::Range; use crate::event::{Ticks, ValidationError}; use crate::score::{ - AtomEvent, AtomNote, AtomRest, EventGroup, LossReport, MasterBar, Score, TechniqueSpan, Track, - Voice, + index_from_ordinal, AtomEvent, AtomNote, AtomRest, EventGroup, LossReport, MasterBar, Score, + TechniqueSpan, Track, Voice, }; /// Half-open tick range: `start <= tick < end`. @@ -71,7 +71,7 @@ pub fn extract_bars(score: &Score, bars: Range) -> Score { .iter() .enumerate() .map(|(i, b)| MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: rebased_range(b.tick_range, offset), time_signature: b.time_signature, tempo: b.tempo, diff --git a/core/src/split.rs b/core/src/split.rs index 39e1ebd1..7213bc19 100644 --- a/core/src/split.rs +++ b/core/src/split.rs @@ -114,7 +114,7 @@ mod tests { use super::{bar_segments, cap_segment_bars, is_trivial_phrase}; use crate::event::{Tempo, Ticks, TimeSignature}; - use crate::score::{MasterBar, RepeatMarker}; + use crate::score::{index_from_ordinal, MasterBar, RepeatMarker}; use crate::slice::TickRange; /// `n` consecutive 4/4 bars of 1920 ticks each. @@ -123,7 +123,7 @@ mod tests { let mut start = 0_u32; for index in 0..n { out.push(MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new(Ticks(start), Ticks(start + 1920)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/src/structure.rs b/core/src/structure.rs index 7364c020..399cb9ff 100644 --- a/core/src/structure.rs +++ b/core/src/structure.rs @@ -56,8 +56,8 @@ use crate::generate::{ GenerationStrategy, PitchMaterial, RhythmTemplate, RuleGenerationRequest, }; use crate::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - TechniqueSpan, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, TechniqueSpan, Track, Voice, }; use crate::scoring::{rank_indices, Axes, Axis, Scored, WeightPolicy}; use crate::slice::TickRange; @@ -779,7 +779,7 @@ fn tile_and_vary( let start = i_u32.checked_mul(bar_dur.0).ok_or_else(invalid)?; let end = start.checked_add(bar_dur.0).ok_or_else(invalid)?; master_bars.push(MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(end)).map_err(|_| invalid())?, time_signature: c.time_signature, tempo: c.tempo, diff --git a/core/src/unfold.rs b/core/src/unfold.rs index 1c83c19e..b9983068 100644 --- a/core/src/unfold.rs +++ b/core/src/unfold.rs @@ -77,7 +77,7 @@ mod tests { use crate::slice::TickRange; /// A 4/4 bar at `index`, laid out back-to-back, carrying `repeat`. - fn bar(index: usize, repeat: RepeatMarker) -> MasterBar { + fn bar(index: u64, repeat: RepeatMarker) -> MasterBar { let start = u32::try_from(index).unwrap_or(0).saturating_mul(1920); MasterBar { index, diff --git a/core/tests/canonical_index_contract.rs b/core/tests/canonical_index_contract.rs new file mode 100644 index 00000000..baafd558 --- /dev/null +++ b/core/tests/canonical_index_contract.rs @@ -0,0 +1,201 @@ +//! SWG-CORE-01, step B: the fixed-width contract. +//! +//! Red before the migration, and red by **failing to compile** — which is the +//! right failure mode for a type contract. `usize` and `u64` are distinct +//! types in Rust even where they have the same size, so these assertions +//! cannot be satisfied on any target while the fields are `usize`, and cannot +//! be accidentally satisfied on a 64-bit host once they are `u64`. +//! +//! Kept in its own file so that step A's characterization keeps compiling and +//! keeps passing through this commit. A red file that also holds the green +//! evidence would make "the characterization was green before and after" +//! unverifiable at exactly the moment it matters. +//! +//! Two claims, and they are different: +//! +//! 1. the three canonical index fields are `u64`; +//! 2. a value above `u32::MAX` is representable **on every target**, not just +//! on a 64-bit host — the property step A could only assert behind a +//! `cfg`, and the one H3 said the format did not have. + +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::missing_assert_message, + clippy::indexing_slicing +)] + +use griff_core::event::{Tempo, Ticks, TimeSignature}; +use griff_core::score::{ImportWarning, MasterBar, RepeatMarker}; +use griff_core::slice::TickRange; + +/// Accepts a `u64` and nothing that merely happens to be the same size. +const fn require_u64(_: u64) {} + +fn master_bar(index: u64) -> MasterBar { + MasterBar { + index, + tick_range: TickRange::new(Ticks(0), Ticks(1920)).expect("ordered range"), + time_signature: TimeSignature::new(4, 4).expect("4/4"), + tempo: Tempo::from_bpm_integer(120).expect("120 BPM"), + repeat: RepeatMarker::default(), + } +} + +// ── claim 1: the three fields are `u64` ──────────────────────────────────── + +#[test] +fn the_stored_bar_index_is_u64() { + require_u64(master_bar(0).index); +} + +#[test] +fn both_warning_payload_indices_are_u64() { + let warnings = [ + ImportWarning::TrackNameInvalidUtf8 { track_index: 0 }, + ImportWarning::TempoApproximated { + bar_index: 0, + nearest_micros: 500_000, + }, + ]; + for warning in warnings { + match warning { + ImportWarning::TrackNameInvalidUtf8 { track_index } => require_u64(track_index), + ImportWarning::TempoApproximated { bar_index, .. } => require_u64(bar_index), + ImportWarning::SmpteTimingUnsupported | ImportWarning::Other(_) => { + panic!("no other variant is constructed here") + } + } + } +} + +/// `nearest_micros` is **not** an index and stays `u32`. +/// +/// Stated because the migration passes right by it, and "widen the integers +/// near the ones we are widening" is the cheapest way for a mechanical change +/// to grow a scope it was never given. +#[test] +fn nearest_micros_is_left_alone() { + const fn require_u32(_: u32) {} + let ImportWarning::TempoApproximated { nearest_micros, .. } = + (ImportWarning::TempoApproximated { + bar_index: 0, + nearest_micros: 500_000, + }) + else { + panic!("constructed variant") + }; + require_u32(nearest_micros); +} + +// ── claim 2: above `u32::MAX`, on every target ───────────────────────────── + +/// No `cfg`. That absence is the deliverable. +#[test] +fn a_stored_bar_index_above_u32_max_is_representable_on_every_target() { + let index: u64 = u64::from(u32::MAX) + 1; + assert_eq!(master_bar(index).index, 4_294_967_296_u64); +} + +/// No `cfg` here either, for the same reason. +#[test] +fn warning_payload_indices_above_u32_max_are_representable_on_every_target() { + let beyond: u64 = u64::from(u32::MAX) + 1; + + let ImportWarning::TempoApproximated { bar_index, .. } = (ImportWarning::TempoApproximated { + bar_index: beyond, + nearest_micros: 500_000, + }) else { + panic!("constructed variant") + }; + assert_eq!(bar_index, 4_294_967_296_u64); + + let ImportWarning::TrackNameInvalidUtf8 { track_index } = + (ImportWarning::TrackNameInvalidUtf8 { + track_index: beyond, + }) + else { + panic!("constructed variant") + }; + assert_eq!(track_index, 4_294_967_296_u64); +} + +// ── the ordinal → canonical conversion ───────────────────────────────────── + +/// `index_from_ordinal` must widen, never narrow. +/// +/// Added after the fact, because the migration's own falsification pass found +/// this hole: replacing the function's body with `(ordinal as u32) as u64` +/// left the entire suite green. Nothing else could catch it — reaching the +/// truncation through a real `Vec` needs four billion elements, which no test +/// can build. +/// +/// Calling the conversion directly does reach it, because the input is a +/// `usize` rather than a collection. That is also why the test is gated: on a +/// 32-bit target the offending input does not exist, so there is nothing to +/// check and nothing to truncate. +#[cfg(target_pointer_width = "64")] +#[test] +fn the_ordinal_conversion_widens_rather_than_truncating() { + use griff_core::score::index_from_ordinal; + + let beyond = usize::try_from(u64::from(u32::MAX) + 1).expect("64-bit host"); + assert_eq!( + index_from_ordinal(beyond), + 4_294_967_296_u64, + "an ordinal above u32::MAX must survive the widening whole" + ); + assert_eq!(index_from_ordinal(0), 0_u64); + assert_eq!( + index_from_ordinal(usize::try_from(u32::MAX).expect("fits")), + 4_294_967_295_u64 + ); +} + +/// A payload index above `u32::MAX` survives the path an importer actually +/// takes. +/// +/// The second hole the falsification pass found. Every other test here builds +/// an `ImportWarning` and inspects it immediately; none of them goes through +/// `LossReport::add`, so a narrowing inserted *there* — clamping `bar_index` +/// to `u32::MAX` on the way into the report — left the whole suite green. +/// +/// Importers never construct the variant and read it back; they hand it to +/// `add`. This checks the path they use. +#[test] +fn a_payload_index_above_u32_max_survives_being_added_to_a_report() { + use griff_core::score::LossReport; + + let beyond: u64 = u64::from(u32::MAX) + 1; + let mut report = LossReport::new(); + report.add(ImportWarning::TempoApproximated { + bar_index: beyond, + nearest_micros: 500_000, + }); + report.add(ImportWarning::TrackNameInvalidUtf8 { + track_index: beyond, + }); + + assert_eq!( + report.warnings, + vec![ + ImportWarning::TempoApproximated { + bar_index: 4_294_967_296, + nearest_micros: 500_000, + }, + ImportWarning::TrackNameInvalidUtf8 { + track_index: 4_294_967_296, + }, + ], + "`add` records the payload as given and narrows nothing" + ); + + // `absorb` concatenates through the same door. + let mut outer = LossReport::new(); + outer.absorb(report); + let ImportWarning::TempoApproximated { bar_index, .. } = outer.warnings[0] else { + panic!("first warning") + }; + assert_eq!(bar_index, 4_294_967_296_u64); +} diff --git a/core/tests/canonical_index_width.rs b/core/tests/canonical_index_width.rs new file mode 100644 index 00000000..0451abbf --- /dev/null +++ b/core/tests/canonical_index_width.rs @@ -0,0 +1,248 @@ +//! SWG-CORE-01, step A: what the three canonical index fields do **today**. +//! +//! `MasterBar.index`, `ImportWarning::TrackNameInvalidUtf8.track_index`, and +//! `ImportWarning::TempoApproximated.bar_index` are about to change from +//! `usize` to `u64`. These tests are written before that change and must pass +//! before it, so that "the migration altered no behaviour" is a checked claim +//! rather than a hopeful one. They are characterization tests: the backlog +//! exempts them from the red phase, and they must not be edited to make the +//! migration pass. +//! +//! The point they exist to pin is the one the width decision turns on. A +//! stored index is **not** a vector position (H4): it is an exact fact of its +//! own, an importer may disagree with the ordinal, and on a 64-bit host it may +//! already exceed `u32::MAX`. Narrowing to `u32` would therefore shrink an +//! inhabited canonical model; `u64` removes the platform dependence without +//! shrinking anything. The `>u32::MAX` witnesses below are what makes that +//! sentence checkable. + +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::missing_assert_message, + clippy::indexing_slicing +)] + +use griff_core::dump::normalize; +use griff_core::event::{Tempo, Ticks, TimeSignature, Tuning}; +use griff_core::score::{ImportWarning, LossReport, MasterBar, RepeatMarker, Score, Track, Voice}; +use griff_core::semantic_diff::{exact_semantic_diff, SemanticDiffReport}; +use griff_core::slice::TickRange; + +// ── fixtures ─────────────────────────────────────────────────────────────── + +fn range(start: u32, end: u32) -> TickRange { + TickRange::new(Ticks(start), Ticks(end)).expect("ordered range") +} + +/// A bar whose stored index is given explicitly, never derived. +fn bar(index: u64, start: u32, end: u32) -> MasterBar { + MasterBar { + index, + tick_range: range(start, end), + time_signature: TimeSignature::new(4, 4).expect("4/4"), + tempo: Tempo::from_bpm_integer(120).expect("120 BPM"), + repeat: RepeatMarker::default(), + } +} + +fn score(master_bars: Vec) -> Score { + Score { + ticks_per_quarter: 480, + master_bars, + tracks: Vec::new(), + source_meta: None, + loss: LossReport::new(), + } +} + +/// A score carrying one track, so the normalized projection has bars to walk. +fn score_with_track(master_bars: Vec) -> Score { + let mut out = score(master_bars); + out.tracks.push(Track { + name: None, + channel: 0, + voices: vec![Voice { + id: 0, + event_groups: Vec::new(), + }], + tuning: Tuning::new(Vec::new()), + }); + out +} + +fn differing_fields(report: &SemanticDiffReport) -> Vec { + report + .differences + .iter() + .map(|d| d.path.to_string()) + .collect() +} + +// ── the stored index is its own fact (H4) ────────────────────────────────── + +#[test] +fn a_stored_bar_index_is_not_derived_from_its_vector_position() { + let s = score(vec![bar(5, 0, 1920), bar(2, 1920, 3840)]); + assert_eq!(s.master_bars[0].index, 5); + assert_eq!(s.master_bars[1].index, 2); +} + +#[test] +fn the_exact_diff_reports_a_changed_stored_index() { + let expected = score(vec![bar(0, 0, 1920)]); + let mut actual = expected.clone(); + actual.master_bars[0].index = 9; + + let report = exact_semantic_diff(&expected, &actual); + assert_eq!( + differing_fields(&report), + vec!["score.master_bars[ordinal=0].index"], + "a changed stored index is exactly one exact difference" + ); + assert_eq!(report.differences[0].expected.as_deref(), Some("0")); + assert_eq!(report.differences[0].actual.as_deref(), Some("9")); +} + +#[test] +fn the_normalized_projection_carries_the_stored_index_not_the_ordinal() { + let projected = normalize(&score_with_track(vec![bar(5, 0, 1920), bar(2, 1920, 3840)])); + let indices: Vec = projected.tracks[0].bars.iter().map(|b| b.index).collect(); + assert_eq!( + indices, + vec![5, 2], + "the projection copies the stored index and does not renumber" + ); +} + +// ── warning payload indices are payload, not lookups ─────────────────────── + +#[test] +fn warning_payload_indices_survive_unchanged() { + let mut s = score(Vec::new()); + s.loss + .add(ImportWarning::TrackNameInvalidUtf8 { track_index: 7 }); + s.loss.add(ImportWarning::TempoApproximated { + bar_index: 11, + nearest_micros: 4_200_000, + }); + + // No track 7 and no bar 11 exist in this score. Nothing normalizes the + // payload against the tree, and nothing may start to. + assert!(s.tracks.is_empty()); + assert!(s.master_bars.is_empty()); + assert_eq!( + s.loss.warnings, + vec![ + ImportWarning::TrackNameInvalidUtf8 { track_index: 7 }, + ImportWarning::TempoApproximated { + bar_index: 11, + nearest_micros: 4_200_000, + }, + ] + ); +} + +#[test] +fn the_exact_diff_reports_changed_warning_payload_indices() { + let mut expected = score(Vec::new()); + expected + .loss + .add(ImportWarning::TrackNameInvalidUtf8 { track_index: 1 }); + expected.loss.add(ImportWarning::TempoApproximated { + bar_index: 3, + nearest_micros: 500_000, + }); + + let mut actual = expected.clone(); + actual.loss.warnings[0] = ImportWarning::TrackNameInvalidUtf8 { track_index: 2 }; + actual.loss.warnings[1] = ImportWarning::TempoApproximated { + bar_index: 4, + nearest_micros: 500_000, + }; + + let report = exact_semantic_diff(&expected, &actual); + assert_eq!( + differing_fields(&report), + vec![ + "score.loss.warnings[0].track_index", + "score.loss.warnings[1].bar_index", + ] + ); +} + +// ── the inhabited range today, and why `u32` would shrink it ─────────────── + +/// A stored index above `u32::MAX`, checked rather than asserted in prose. +/// +/// This is the whole argument for `u64` over `u32`: the value was already +/// inside the model before the migration, so `u32` would have shrunk an +/// inhabited canonical domain rather than merely fixing a portability defect. +/// +/// Written under `#[cfg(target_pointer_width = "64")]`, because while the +/// fields were `usize` that is the only place these scores could be built — +/// exactly the false portability claim H3 recorded. SWG-CORE-01 removed the +/// `cfg`'s reason to exist and the `cfg` with it; every assertion below is +/// unchanged from the pre-migration commit and now runs on every target. +mod above_u32_max { + use super::{bar, differing_fields, exact_semantic_diff, normalize, score, score_with_track}; + use griff_core::score::ImportWarning; + + /// `u32::MAX + 1`, spelled through arithmetic so the constant is + /// unambiguous. No narrowing step any more — that was the `usize` era. + fn beyond_u32() -> u64 { + u64::from(u32::MAX) + 1 + } + + #[test] + fn a_stored_bar_index_above_u32_max_is_inhabited() { + let s = score(vec![bar(beyond_u32(), 0, 1920)]); + assert_eq!(s.master_bars[0].index, 4_294_967_296); + } + + #[test] + fn a_stored_bar_index_above_u32_max_survives_the_exact_diff() { + let expected = score(vec![bar(beyond_u32(), 0, 1920)]); + let mut actual = expected.clone(); + actual.master_bars[0].index = beyond_u32() + 1; + + let report = exact_semantic_diff(&expected, &actual); + assert_eq!( + differing_fields(&report), + vec!["score.master_bars[ordinal=0].index"] + ); + assert_eq!( + report.differences[0].expected.as_deref(), + Some("4294967296"), + "the value is compared whole, not truncated to 32 bits" + ); + assert_eq!(report.differences[0].actual.as_deref(), Some("4294967297")); + } + + #[test] + fn a_stored_bar_index_above_u32_max_survives_the_projection() { + let projected = normalize(&score_with_track(vec![bar(beyond_u32(), 0, 1920)])); + assert_eq!(projected.tracks[0].bars[0].index, 4_294_967_296); + } + + #[test] + fn warning_payload_indices_above_u32_max_are_inhabited() { + let warning = ImportWarning::TempoApproximated { + bar_index: beyond_u32(), + nearest_micros: 500_000, + }; + let ImportWarning::TempoApproximated { bar_index, .. } = warning else { + panic!("constructed variant"); + }; + assert_eq!(bar_index, 4_294_967_296); + + let name_warning = ImportWarning::TrackNameInvalidUtf8 { + track_index: beyond_u32(), + }; + let ImportWarning::TrackNameInvalidUtf8 { track_index } = name_warning else { + panic!("constructed variant"); + }; + assert_eq!(track_index, 4_294_967_296); + } +} diff --git a/core/tests/closure.rs b/core/tests/closure.rs index 154a1943..071d4afe 100644 --- a/core/tests/closure.rs +++ b/core/tests/closure.rs @@ -34,8 +34,8 @@ use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, generate::PitchMaterial, score::{ - AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, LossReport, MasterBar, - RepeatMarker, Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, LossReport, + MasterBar, RepeatMarker, Score, Track, Voice, }, scoring::{rank_indices, Scored}, slice::TickRange, @@ -76,7 +76,7 @@ fn build_score(bar_count: usize, atoms: Vec) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/complement.rs b/core/tests/complement.rs index 92babadc..98117e08 100644 --- a/core/tests/complement.rs +++ b/core/tests/complement.rs @@ -30,8 +30,8 @@ use griff_core::{ }, generate::GenerationSeed, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, TechniqueSpan, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, TechniqueSpan, Track, Voice, }, slice::TickRange, }; @@ -66,7 +66,7 @@ fn score_with_part_a(bar_count: usize, pitches: &[u8]) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/explicit_rhythm.rs b/core/tests/explicit_rhythm.rs index 923896d2..4f685f9d 100644 --- a/core/tests/explicit_rhythm.rs +++ b/core/tests/explicit_rhythm.rs @@ -22,8 +22,8 @@ use griff_core::generate::{ }; use griff_core::generation_input::{ranked_candidates, CorpusMaterial, GenerationAsk}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -115,7 +115,7 @@ fn seed_score(bar_count: usize) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/gesture.rs b/core/tests/gesture.rs index 3ff3253c..053ecd4a 100644 --- a/core/tests/gesture.rs +++ b/core/tests/gesture.rs @@ -32,8 +32,8 @@ use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, gesture::{measure_gesture, GestureError}, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, }; @@ -50,7 +50,7 @@ fn score_with_notes(bar_count: usize, notes: &[(u32, u32, u8)]) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/novelty.rs b/core/tests/novelty.rs index cd18c87c..f027aa11 100644 --- a/core/tests/novelty.rs +++ b/core/tests/novelty.rs @@ -36,8 +36,8 @@ use griff_core::{ NOVELTY_AXIS_LABELS, }, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, scoring::{rank_indices, Scored}, slice::TickRange, @@ -92,7 +92,7 @@ fn build_score(ppqn: u16, tracks: Vec) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * bar; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + bar)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/s6_chain_baseline.rs b/core/tests/s6_chain_baseline.rs index 05d584f5..00a4622b 100644 --- a/core/tests/s6_chain_baseline.rs +++ b/core/tests/s6_chain_baseline.rs @@ -38,8 +38,8 @@ use griff_core::{ generation_input::{ranked_candidates, GenerationAsk}, rerank::{rerank_weights_v1, RERANK_AXIS_LABELS}, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, scoring::Scored, slice::TickRange, @@ -78,7 +78,7 @@ fn source() -> Score { for (index, notes) in bars.iter().enumerate() { let start = u32::try_from(index).unwrap() * BAR; master_bars.push(MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).unwrap(), time_signature: TimeSignature::new(4, 4).unwrap(), tempo: Tempo::from_bpm_integer(120).unwrap(), diff --git a/core/tests/score_props.rs b/core/tests/score_props.rs index fcf777c1..6c9ceb30 100644 --- a/core/tests/score_props.rs +++ b/core/tests/score_props.rs @@ -28,8 +28,8 @@ use griff_core::{ Velocity, }, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, }; @@ -216,7 +216,7 @@ fn build_score(bars: Vec) -> Score { .iter() .enumerate() .map(|(i, &(start, len, num, den))| MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start.saturating_add(len))).unwrap(), time_signature: TimeSignature::new(num, den).unwrap(), tempo: Tempo::from_bpm_integer(120).unwrap(), diff --git a/core/tests/semantic_diff_props.rs b/core/tests/semantic_diff_props.rs index bf28dc68..48f7df3c 100644 --- a/core/tests/semantic_diff_props.rs +++ b/core/tests/semantic_diff_props.rs @@ -24,8 +24,8 @@ use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, score::{ - AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, ImportWarning, LossReport, - MasterBar, RepeatMarker, Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, AtomRest, EventGroup, EventGroupKind, + ImportWarning, LossReport, MasterBar, RepeatMarker, Score, Track, Voice, }, semantic_diff::{exact_semantic_diff, SemanticDifferenceKind}, slice::TickRange, @@ -62,7 +62,7 @@ fn build_score(bpms: &[u32], tracks: &[Vec], warnings: usize) -> Score .map(|(index, &bpm)| { let start = u32::try_from(index).expect("few bars") * BAR; MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature::new(4, 4).expect("4/4"), tempo: Tempo::from_bpm_integer(bpm.max(1)).expect("positive BPM"), diff --git a/core/tests/slice_extract.rs b/core/tests/slice_extract.rs index d0a2ed1b..53ab646d 100644 --- a/core/tests/slice_extract.rs +++ b/core/tests/slice_extract.rs @@ -36,7 +36,7 @@ fn note(start: u32, pitch: u8) -> AtomEvent { }) } -fn bar(index: usize, start: u32) -> MasterBar { +fn bar(index: u64, start: u32) -> MasterBar { MasterBar { index, tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), diff --git a/core/tests/strategy_selection.rs b/core/tests/strategy_selection.rs index 0e98c811..8632ad27 100644 --- a/core/tests/strategy_selection.rs +++ b/core/tests/strategy_selection.rs @@ -19,8 +19,8 @@ use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, V use griff_core::generate::GenerationStrategy; use griff_core::generation_input::{ranked_candidates, select_ranked, GenerationAsk, RankedSet}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -34,7 +34,7 @@ fn seed_score(bar_count: usize) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/structure.rs b/core/tests/structure.rs index 8d185279..4a34f52c 100644 --- a/core/tests/structure.rs +++ b/core/tests/structure.rs @@ -26,8 +26,8 @@ use griff_core::{ Tuning, Velocity, }, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, TechniqueSpan, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, TechniqueSpan, Track, Voice, }, slice::TickRange, structure::{measure_complexity, measure_structure, StructureError}, @@ -55,7 +55,7 @@ fn build_score(bars: &[Vec]) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, @@ -268,7 +268,7 @@ fn measures_all_voices_of_a_track() { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, @@ -348,7 +348,7 @@ fn loopability_penalizes_leading_silence() { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/syncopation.rs b/core/tests/syncopation.rs index 29d0ff12..f741f5e7 100644 --- a/core/tests/syncopation.rs +++ b/core/tests/syncopation.rs @@ -17,8 +17,8 @@ use griff_core::corpus::SwancoreTag; use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; use griff_core::syncopation::derive_syncopated; @@ -48,7 +48,7 @@ fn score_with_onsets(bars: usize, onsets: &[u32]) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/tonal.rs b/core/tests/tonal.rs index 129cfd25..b8cf6f37 100644 --- a/core/tests/tonal.rs +++ b/core/tests/tonal.rs @@ -34,8 +34,8 @@ use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, feature::PitchRange, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, tonal::{estimate_key, EvidenceScope, KeyMode, PitchEvidence}, @@ -69,7 +69,7 @@ fn master_bars(bar_count: usize) -> Vec { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/core/tests/tonal_context.rs b/core/tests/tonal_context.rs index 8897f8ab..c30dcfe0 100644 --- a/core/tests/tonal_context.rs +++ b/core/tests/tonal_context.rs @@ -49,8 +49,8 @@ use griff_core::{ generate::GenerationStrategy, generation_input::{ranked_candidates, GenerationAsk, RankedSet}, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, tonal::{ @@ -109,7 +109,7 @@ fn score_of(tracks: Vec, bars: usize) -> Score { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/docs/decisions.log.md b/docs/decisions.log.md index 44b19904..c33b81de 100644 --- a/docs/decisions.log.md +++ b/docs/decisions.log.md @@ -2325,3 +2325,41 @@ Architectural decisions go to [`adr/`](adr/) instead. stronger than the number that was claimed for it, not weaker, and the stage doc had sensibly stated the ranges rather than a product; the correction is noted there beside them so the wrong count cannot propagate. + +- 2026-08-18 — In the context of SWG-CORE-01 closing H3, facing the choice + between `u32` and `u64` for `MasterBar.index`, + `ImportWarning::TrackNameInvalidUtf8.track_index`, and + `ImportWarning::TempoApproximated.bar_index`, we decided for **`u64`** and + against `u32`, to achieve a fixed width that removes the platform dependence + without shrinking a canonical domain that already has values in it — the + fields are public, so a stored index above `u32::MAX` was already + constructible on a 64-bit host, and `MasterBar.index` is an exact fact of its + own rather than a bounded ordinal (H4). Accepting eight bytes where four + would usually do, and accepting that the ordinal/canonical boundary now needs + a named crossing: `core::score::index_from_ordinal` widens, and there is + deliberately no inverse, because the inverse is not total. Operational + `usize` — `Vec` positions, lengths, slice indices, and MIDI's + `MAX_MASTER_BARS` resource bound — is untouched and was never part of the + argument. Verified by 1341 tests plus a falsification pass over **twelve** + mutations — the entry first said nine, which was a miscount of my own + standalone runs and is corrected here rather than left to propagate. Two of + the twelve, a truncating widening inside `index_from_ordinal` and a clamp + inside `LossReport::add`, survived the original suite and are now covered by + witnesses written for exactly those seams. + +- 2026-08-18 — In the context of the same closure, facing an independent review + that found the acceptance witness for "no `usize` remains in any type + reachable from `Score`" weaker than the criterion it was recorded against, we + decided for repairing the witness in three further commits — red, green, + this correction — and against amending the six already made, to achieve a + history in which the gap and its repair are both legible, accepting three + more commits on the branch. The witness scanned `core/src/score.rs` only, + while the tree also reaches `event.rs` and `slice.rs`; and its scanner could + not see tuple forms at all, so `Other(String)` and every `pub struct + Pitch(pub u8);` were invisible — including a hypothetical `usize` inside + either. The migration itself is not implicated: the tree had no `usize` left, + the witness simply could not have shown it. Five further mutations confirm + the repair. Accepting that the module list is written out rather than + discovered, because discovering it means resolving `use` paths and type + aliases — a half-compiler to check three files — and that the list can rot, + which is why the coverage test asserts each listed module contributes. diff --git a/docs/swang/exact-score-text.md b/docs/swang/exact-score-text.md index ea719763..26858264 100644 --- a/docs/swang/exact-score-text.md +++ b/docs/swang/exact-score-text.md @@ -97,7 +97,7 @@ loss the diff can see. | Field | Type | Req | Order | Empty | Malformed | | --- | --- | --- | --- | --- | --- | -| `index` | `usize` | yes | — | — | may disagree with position (H4); platform-sized (H3) | +| `index` | `u64` | yes | — | — | may disagree with position (H4) | | `tick_range` | `TickRange { start, end }` | yes | — | `start == end` legal | `start > end` violates `InvalidTickRange` (H5) | | `time_signature` | `{ numerator: u8, denominator: u8 }` | yes | — | — | numerator `0`, non-power-of-two denominator (H5) | | `tempo` | `Tempo` (private rational) | yes | — | — | see H1 | @@ -210,9 +210,9 @@ combination. `ImportWarning` has four variants, three with payloads: ```text -TrackNameInvalidUtf8 { track_index: usize } ← H3 +TrackNameInvalidUtf8 { track_index: u64 } SmpteTimingUnsupported -TempoApproximated { bar_index: usize, nearest_micros: u32 } ← H3 +TempoApproximated { bar_index: u64, nearest_micros: u32 } Other(String) ← H2 ``` @@ -313,9 +313,9 @@ A payload field added to either enum is canonical state that adds no `SemanticField` variant and no struct field, so only an entry here — and the witness that checks it — stands between such a field and silent loss. -`` is the `usize` of H3, written in decimal. Its width becomes a -fixed one when SWG-CORE-01 closes; until then the text is as portable as the -model is, which is the honest amount. +`` is a `u64`, written in decimal — fixed-width since SWG-CORE-01 +closed H3, so the same document means the same thing on a 32-bit target as on +a 64-bit one. It is still not an ordinal: see H4. ### 2.9 Opaque exact leaves and their decomposition @@ -491,22 +491,45 @@ managed to make even string escaping a reproducibility question. The policy itself is fixed in the grammar section of this document. -### H3 — three `usize` fields in a hashed tree — **prerequisite blocker** +### H3 — three `usize` fields in a hashed tree — **CLOSED by SWG-CORE-01** `MasterBar.index`, `ImportWarning::TrackNameInvalidUtf8.track_index`, and -`ImportWarning::TempoApproximated.bar_index` are `usize`, inside a graph +`ImportWarning::TempoApproximated.bar_index` were `usize`, inside a graph that derives `Hash`. Spec §1.2 forbids platform-sized integers in hashed or -serialized state. A document written on a 64-bit host with an index above -`u32::MAX` cannot exact-round-trip on a 32-bit target, so the portable -exact format is, today, not portable. +serialized state, so a document written on a 64-bit host with an index above +`u32::MAX` could not exact-round-trip on a 32-bit target: the portable exact +format was not portable. + +All three are **`u64`** since SWG-CORE-01. The hole is recorded rather than +deleted, because the width it settled is a normative fact and the reasoning +behind it constrains what a later change may do. ```text discovered pre-existing model violation (spec §1.2) → out of scope to repair in SWG-4A-01 -→ BLOCKS the round-trip gate and the level-2 freeze -→ separate fixed-width migration, its own scope and commit chain +→ blocked the round-trip gate and the level-2 freeze +→ closed by SWG-CORE-01, its own scope and commit chain ``` +**Why `u64` and not `u32`.** The narrower type would not merely have removed +a platform dependence; it would have shrunk an inhabited canonical domain. +The fields are public, so values above `u32::MAX` were already constructible +on a 64-bit host, and `MasterBar.index` is a stored exact fact that need not +agree with its vector position (H4) — it is not a bounded ordinal that could +be argued into 32 bits. `u64` keeps every value the model already admitted +and makes the set of admitted values the same on every target. + +Two consequences worth naming, because they are easy to undo: + +- an ordinal and a canonical index are different things and now have + different types. `core::score::index_from_ordinal` is the one sanctioned + widening; there is deliberately no inverse, because the inverse is not + total; +- `usize` remains correct for `Vec` positions, lengths, and slice indices. + Nothing in this hole was ever an argument about those, and MIDI's + `MAX_MASTER_BARS` is an operational resource bound rather than a claim + about canonical width. + Dependency shape: ```text @@ -535,10 +558,10 @@ Scope of the block, precisely, in three parts: (`ExactScoreDocument`, a transient syntax form) is the named case, and the same licence covers the writer slices that spell `` through ordinary decimal conversion. -2. **The migration must close before the completed round trip.** Before the - production writer/parser-builder round trip, and certainly before - SWG-4A-12 and Phase 4A acceptance — until then the portability claim is - false. +2. **The migration had to close before the completed round trip.** Before + the production writer/parser-builder round trip, and certainly before + SWG-4A-12 and Phase 4A acceptance. It did; the portability claim is no + longer false. 3. **Where exactly it lands inside that window is not this document's call.** Sequencing among unblocked tasks is an execution-plan decision and lives in the non-normative backlog. That the migration currently @@ -551,11 +574,13 @@ and "4A builder" as a whole, which said something stronger than the prose beneath it and stronger than any task index has ever claimed. An ASCII edge is a normative statement when it sits in a normative document. -Whether the replacement is `u32` or `u64` is that task's decision on -evidence, never the grammar picking a width because it needed one. +The width was left to that task on evidence rather than picked here, so that +the grammar could not choose a number because it happened to need one. It +chose `u64`, for the reason recorded above. -Level 2 remains *allocated* while this stands. It cannot honestly be -*frozen*, which is precisely the distinction spec §5.3 exists to keep +Level 2 remains *allocated*. Closing this hole removed the portability +objection to freezing it, not the phase-acceptance requirement: the freeze +still waits on Phase 4A, which is the distinction spec §5.3 exists to keep available. ### H4 — `MasterBar.index` is not the vector position @@ -588,8 +613,8 @@ discovered pre-existing encapsulation gap → filed as SWG-CORE-02; a later decision on sealing these newtypes ``` -Unlike H3 this is not a freeze blocker: exact text is well defined over the -invariant-valid domain either way. It is recorded because a reader who sees +Unlike H3 this was never a freeze blocker: exact text is well defined over +the invariant-valid domain either way. It is recorded because a reader who sees `Pitch::new` and concludes pitches are always in range will be wrong, and because a future builder that trusts the type instead of checking will be wrong in a more expensive way. @@ -598,11 +623,12 @@ wrong in a more expensive way. | ID | What | Blocks | | --- | --- | --- | -| SWG-CORE-01 | Replace the three `usize` fields with a fixed width (H3) | round-trip gate, level-2 freeze | +| SWG-CORE-01 | Replace the three `usize` fields with a fixed width (H3) — **closed**: `u64` | round-trip gate, level-2 freeze | | SWG-CORE-02 | Decide whether the canonical newtypes seal their fields (H5) | nothing; recorded for a later decision | -Both are `griff-core` scopes with their own commit chains. Neither is -touched by SWG-4A-01. +Both are `griff-core` scopes with their own commit chains. Neither was +touched by SWG-4A-01, which is the point of listing them here rather than +performing them. ## 6. The grammar diff --git a/docs/swang/foundation-backlog.md b/docs/swang/foundation-backlog.md index 8a79c2fd..f5d052d5 100644 --- a/docs/swang/foundation-backlog.md +++ b/docs/swang/foundation-backlog.md @@ -111,7 +111,7 @@ recorded in `decisions.log.md` if reversed. | SWG-INF-05 | Deterministic multi-error recovery | code | INF-04 | | SWG-INF-06 | Parser resource gate and differential harness | code | INF-02, INF-03 | | SWG-4A-01 | Normative exact-score-text grammar *(done)* | docs | INF-02 | -| SWG-CORE-01 | Fixed-width migration for the three `usize` fields | code | 4A-01 | +| SWG-CORE-01 | Fixed-width migration for the three `usize` fields *(done)* | code | 4A-01 | | SWG-CORE-02 | Decide whether the canonical newtypes seal their fields | docs | 4A-01 | | SWG-4A-02 | `ExactScoreDocument` as a transient syntax form | code | 4A-01 | | SWG-4A-03 | Writer: transport and master timeline | code | 4A-01 | @@ -416,21 +416,22 @@ Acceptance: Landing in [`exact-score-text.md`](exact-score-text.md). Census and representability decisions first; grammar second, in the same document. -### SWG-CORE-01 — Fixed-width migration for the three `usize` fields +### SWG-CORE-01 — Fixed-width migration for the three `usize` fields *(done)* **Kind:** code (`griff-core`). **Depends on:** 4A-01 (which discovered it). -Scheduled after 4A-04 — a sequencing choice, not a dependency; nothing in -this task needs the writer to exist. +Ran after 4A-04 — a sequencing choice, not a dependency; nothing in this task +needed the writer to exist. -`MasterBar.index`, `ImportWarning::TrackNameInvalidUtf8.track_index`, and -`ImportWarning::TempoApproximated.bar_index` are `usize` inside a graph that -derives `Hash`, which spec §1.2 forbids. Until this closes, a document -written on a 64-bit host is not guaranteed to round-trip on a 32-bit one, -so Phase 4A's portability claim is false. +**Outcome: `u64`.** `MasterBar.index`, +`ImportWarning::TrackNameInvalidUtf8.track_index`, and +`ImportWarning::TempoApproximated.bar_index` were `usize` inside a graph that +derives `Hash`, which spec §1.2 forbids: a document written on a 64-bit host +was not guaranteed to round-trip on a 32-bit one. All three are `u64` now and +H3 is closed; the normative record of the width and its reasoning lives in +[`exact-score-text.md`](exact-score-text.md) H3, not here. -**Blocks** the round-trip gate (4A-12), the level-2 freeze, and — since the -scheduling refinement below — 4A-05. Does **not** block 4A-02, provided that -task models these fields abstractly rather than baking a width in. +**Unblocked** the round-trip gate (4A-12), the level-2 freeze, and 4A-05. +Never blocked 4A-02, which models these fields abstractly. **Why it now sits between 4A-04 and 4A-05.** Two of the three fields live in `ImportWarning`, and 4A-05 is the first task that serializes `LossReport` at @@ -441,17 +442,32 @@ the second pass would land as a mechanical rewrite of freshly reviewed code. 4A-03 and 4A-04 are unaffected either way: `MasterBar.index` is written through `to_string()`, which does not care about the width. -Its own scope and commit chain — 4A-01 pinned the requirement and does not -perform the migration. `u32` versus `u64` is this task's decision on -evidence, never a width the grammar picked because it needed one. +Its own scope and commit chain — 4A-01 pinned the requirement and did not +perform the migration. `u32` versus `u64` was this task's decision on +evidence, never a width the grammar picked because it needed one; `u64` won +because values above `u32::MAX` were already inhabited and `u32` would have +shrunk the canonical model rather than merely making it portable. -Acceptance: +Acceptance, all met: - no `usize` remains in any type reachable from `Score`; -- characterization tests land before the migration (existing behaviour, so - no red phase required, but they must pass first); -- adapters and the projection follow without a musical behaviour change; -- a witness test fails if a `usize` reappears in the tree. +- characterization tests landed before the migration (existing behaviour, so + no red phase required, but they had to pass first); +- adapters and the projection followed without a musical behaviour change; +- a witness test fails if a `usize` reappears in the tree — + `exact_text_census::the_canonical_tree_declares_no_platform_sized_integer`. + It scans `score.rs`, `event.rs`, and `slice.rs`, which is where every type + reachable from `Score` is declared, and reads named fields, tuple enum + payloads, and tuple structs alike. The module list is written out; + `…scans_every_module_the_canonical_tree_reaches` checks that each entry + contributes, so a rotted list fails rather than silently narrowing the + witness. + +The last of those was weaker when first recorded — one module, no tuple forms +— and an independent review caught it after closure. The gap was in the +acceptance witness, not in the migration: the tree had no `usize` left either +way. Repaired in three follow-up commits on the same branch, with five +mutations confirming the repair. ### SWG-CORE-02 — Decide whether the canonical newtypes seal their fields @@ -534,7 +550,9 @@ about right". ### SWG-4A-05 — Writer: techniques, positions, evidence, metadata, losses -**Kind:** code. **Depends on:** 4A-04 +**Kind:** code. **Depends on:** 4A-04, CORE-01 — both **done**, so this task +is unblocked. CORE-01 ran first so the metadata writer is built on the final +`u64` payload indices rather than rewritten onto them. The remaining facts: note marks, technique spans and their ranges, evidence, string/fret position, position evidence and confidence (`ConfidenceBps`), @@ -1184,8 +1202,9 @@ INF-01 status sync (done) └─→ 4A-01 exact grammar (done) │ ├─→ 4A-03 → 4A-04 → CORE-01 → 4A-05 → 4A-10 writer lane - │ │ - │ └─→ also gates 4A-12 and the level-2 freeze + │ (done) (done) (done) ↑ + │ │ next + │ └─→ also gated 4A-12 and the level-2 freeze │ └─→ 4A-02 → INF-04 → INF-06 → 4A-06 parser skeleton -> 4A-02..4A-09 writer / parser / builder diff --git a/fuzz/fuzz_targets/complement_request.rs b/fuzz/fuzz_targets/complement_request.rs index 916dad41..95110586 100644 --- a/fuzz/fuzz_targets/complement_request.rs +++ b/fuzz/fuzz_targets/complement_request.rs @@ -23,8 +23,8 @@ use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, generate::GenerationSeed, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, }; @@ -64,7 +64,7 @@ fn build_part_a(bar_count: usize, ppqn: u16, pitches: &[u8]) -> Option { let end = start.checked_add(bar)?; let range = TickRange::new(Ticks(start), Ticks(end)).ok()?; master_bars.push(MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: range, time_signature: TimeSignature::new(4, 4).ok()?, tempo: Tempo::from_bpm_integer(120).ok()?, diff --git a/fuzz/fuzz_targets/structure_metrics.rs b/fuzz/fuzz_targets/structure_metrics.rs index bf1b8a7a..24d48044 100644 --- a/fuzz/fuzz_targets/structure_metrics.rs +++ b/fuzz/fuzz_targets/structure_metrics.rs @@ -20,8 +20,8 @@ use libfuzzer_sys::fuzz_target; use griff_core::{ event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}, score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Score, - Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }, slice::TickRange, structure::measure_structure, @@ -46,7 +46,7 @@ fn build_score(bar_count: usize, ppqn: u16, pitches: &[u8]) -> Option { let start = u32::try_from(i).ok()?.checked_mul(bar)?; let end = start.checked_add(bar)?; master_bars.push(MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(end)).ok()?, time_signature: TimeSignature::new(4, 4).ok()?, tempo: Tempo::from_bpm_integer(120).ok()?, diff --git a/swang/src/eval.rs b/swang/src/eval.rs index 564bff5c..2f86ecd8 100644 --- a/swang/src/eval.rs +++ b/swang/src/eval.rs @@ -461,8 +461,8 @@ mod tests { use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::generation_input::{ranked_candidates, select_ranked, GenerationAsk}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -477,7 +477,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).unwrap() * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature { numerator: 4, diff --git a/swang/src/pattern_compile.rs b/swang/src/pattern_compile.rs index 790b770e..a57d7d3b 100644 --- a/swang/src/pattern_compile.rs +++ b/swang/src/pattern_compile.rs @@ -626,8 +626,8 @@ mod tests { use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::generate::explicit_rhythm_diagnostics; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -651,7 +651,7 @@ mod tests { .map(|(i, &(numerator, denominator))| { let len = 480 * 4 * u32::from(numerator) / u32::from(denominator); let mb = MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + len)) .expect("ordered range"), time_signature: TimeSignature { diff --git a/swang/tests/exact_index_width.rs b/swang/tests/exact_index_width.rs new file mode 100644 index 00000000..53f0a04d --- /dev/null +++ b/swang/tests/exact_index_width.rs @@ -0,0 +1,102 @@ +//! SWG-CORE-01, step A: how the exact writer spells a stored bar index today. +//! +//! `swang/tests/exact_writer_transport.rs` already pins that the writer emits +//! the **stored** `MasterBar.index` rather than the vector position. What it +//! does not pin is the range: every index it uses is small, so the whole suite +//! would stay green if the field's width changed underneath it. +//! +//! These characterization tests close that gap before the `usize` → `u64` +//! migration, so "the writer's bytes did not move" is checked rather than +//! assumed. They must not be edited to make the migration pass. + +// The allowances the repository's other test files take. +#![allow( + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::indexing_slicing, + clippy::missing_assert_message +)] + +use griff_core::event::{Tempo, Ticks, TimeSignature}; +use griff_core::score::{LossReport, MasterBar, RepeatMarker, Score}; +use griff_core::slice::TickRange; +use griff_swang::exact::write_score; + +fn score_with_index(index: u64) -> Score { + Score { + ticks_per_quarter: 480, + master_bars: vec![MasterBar { + index, + tick_range: TickRange::new(Ticks(0), Ticks(1920)).expect("ordered range"), + time_signature: TimeSignature::new(4, 4).expect("4/4"), + tempo: Tempo::from_bpm_integer(120).expect("120 BPM"), + repeat: RepeatMarker::default(), + }], + tracks: Vec::new(), + source_meta: None, + loss: LossReport::new(), + } +} + +fn write(index: u64) -> String { + write_score(&score_with_index(index)).expect("inside the writer domain") +} + +#[test] +fn a_small_index_is_written_as_plain_decimal() { + assert!(write(0).contains("index 0\n")); + assert!(write(42).contains("index 42\n")); +} + +/// The upper end of a `u32`, written whole. +/// +/// Nothing in the writer narrows here today, and nothing may start to. +#[test] +fn an_index_at_u32_max_is_written_whole() { + let text = write(u64::from(u32::MAX)); + assert!( + text.contains("index 4294967295\n"), + "the stored index is written in full: {text:?}" + ); +} + +/// Above `u32::MAX` — inhabited before the migration on a 64-bit host, and +/// the reason it targets `u64` rather than `u32`. +/// +/// Written under `#[cfg(target_pointer_width = "64")]` while the field was +/// `usize`, because that is the only place this score could be built — the +/// portability claim H3 called false. SWG-CORE-01 removed the `cfg`'s reason +/// to exist and the `cfg` with it; the assertion is unchanged and now runs on +/// every target. +#[test] +fn an_index_above_u32_max_is_written_whole() { + let index = u64::from(u32::MAX) + 1; + let text = write(index); + assert!( + text.contains("index 4294967296\n"), + "no truncation to 32 bits anywhere on the way out: {text:?}" + ); +} + +/// The byte-golden the migration must not move. +#[test] +fn a_large_index_document_matches_its_canonical_bytes() { + assert_eq!( + write(u64::from(u32::MAX)), + "\ +swang 2 + +score { + ppqn 480 + + master_bar { + index 4294967295 + ticks 0..1920 + meter 4/4 + tempo 120/1 + } +} +" + ); +} diff --git a/swang/tests/exact_text_census.rs b/swang/tests/exact_text_census.rs index c04e3e26..1459c152 100644 --- a/swang/tests/exact_text_census.rs +++ b/swang/tests/exact_text_census.rs @@ -175,20 +175,55 @@ fn field_name(declaration: &str) -> Option { /// Commas nested inside `<…>` or `(…)` belong to a type argument, not to /// the payload: `Vec<(u8, u8)>` is one element, `String, u32` is two. fn tuple_arity(inner: &str) -> usize { + tuple_elements(inner).len() +} + +/// The top-level elements of a tuple payload, in order. +/// +/// The splitter `tuple_arity` counts, kept as one function so the count and +/// the types can never disagree about where an element ends. Commas nested +/// inside `<…>`, `(…)`, or `[…]` belong to a type argument, not to the +/// payload: `Vec<(u8, u8)>` is one element, `String, u32` is two. +fn tuple_elements(inner: &str) -> Vec { if inner.trim().is_empty() { - return 0; + return Vec::new(); } + let mut out = Vec::new(); let mut depth = 0_i32; - let mut count = 1_usize; + let mut current = String::new(); for c in inner.chars() { match c { '<' | '(' | '[' => depth += 1, '>' | ')' | ']' => depth -= 1, - ',' if depth == 0 => count += 1, + ',' if depth == 0 => { + out.push(current.trim().to_owned()); + current = String::new(); + continue; + } _ => {} } + current.push(c); } - count + out.push(current.trim().to_owned()); + out +} + +/// A declaration with any visibility prefix removed, leaving the type. +/// +/// `pub u8` and `pub(crate) u8` both reduce to `u8`; a bare `u8` is already +/// there. Visibility is not part of the type and would otherwise make the +/// same width look like two different declarations. +fn without_visibility(declaration: &str) -> String { + let trimmed = declaration.trim(); + if let Some(rest) = trimmed.strip_prefix("pub ") { + return rest.trim().to_owned(); + } + if trimmed.starts_with("pub(") { + if let Some((_, tail)) = trimmed.split_once(") ") { + return tail.trim().to_owned(); + } + } + trimmed.to_owned() } /// Payload fields of every variant of an enum, keyed `Enum::Variant.field`. @@ -751,3 +786,308 @@ pub enum Probe { "a widened tuple payload must yield one key per element" ); } + +// ── SWG-CORE-01: no platform-sized integer in the canonical tree ─────────── + +/// Every field declaration inside every `pub struct` / `pub enum` body in +/// `source`, as `Type.field: type` strings. +/// +/// Declarations are discovered rather than listed, so a canonical type added +/// to the file later is covered without anyone remembering to add it here. +/// The `#[cfg(test)]` tail is cut first: its fixtures are not canonical state. +fn declared_field_types(source: &str) -> Vec { + let body = source.split("#[cfg(test)]").next().unwrap_or(source); + let mut out = Vec::new(); + for (offset, _) in body.match_indices("pub ") { + let rest = &body[offset..]; + let Some(line_end) = rest.find('\n') else { + continue; + }; + let header_line = &rest[..line_end]; + + // A tuple struct declares its whole payload on one line and has no + // braced body at all, so the braced scan below never sees it. These + // are the transparent newtypes of §2.9 — canonical leaves. + if let Some(declaration) = header_line.strip_prefix("pub struct ") { + if declaration.trim_end().ends_with(");") { + if let (Some(open), Some(close)) = (declaration.find('('), declaration.rfind(')')) { + let name = declaration.get(..open).unwrap_or_default().trim(); + let inner = declaration + .get(open.saturating_add(1)..close) + .unwrap_or_default(); + for (position, element) in tuple_elements(inner).into_iter().enumerate() { + out.push(format!( + "{name}.{position}: {}", + without_visibility(&element) + )); + } + continue; + } + } + } + + let Some(header_end) = rest.find(" {\n") else { + continue; + }; + let header = &rest[..header_end]; + let Some(name) = header + .strip_prefix("pub struct ") + .or_else(|| header.strip_prefix("pub enum ")) + else { + continue; + }; + if name.contains(char::is_whitespace) { + continue; + } + // `\n}` closes a top-level item; nested variant braces are indented. + let inner_start = offset.saturating_add(header_end).saturating_add(3); + let inner = &body[inner_start..]; + let Some(inner_end) = inner.find("\n}") else { + continue; + }; + for line in inner[..inner_end].lines().map(str::trim) { + if line.is_empty() || line.starts_with("//") || line.starts_with('#') { + continue; + } + if let Some((field, declared)) = line.split_once(':') { + let Some(field) = field_name(field) else { + continue; + }; + let declared = declared.trim().trim_end_matches(',').trim(); + out.push(format!("{name}.{field}: {declared}")); + continue; + } + // A tuple variant: `Other(String)`, `Two(String, u32)`. It carries + // no `:`, so the branch above skips it and every element of it + // used to vanish — including a `usize`. + if let (Some(open), Some(close)) = (line.find('('), line.rfind(')')) { + if open < close { + let payload = line.get(open.saturating_add(1)..close).unwrap_or_default(); + for (position, element) in tuple_elements(payload).into_iter().enumerate() { + out.push(format!( + "{name}.{position}: {}", + without_visibility(&element) + )); + } + } + } + } + } + out +} + +/// Every module that declares a type reachable from `Score`. +/// +/// `score.rs` holds the tree's own structs and enums; `event.rs` holds the +/// leaves it is built from — `TimeSignature`, `Tuning`, `NoteMarks`, +/// `NotePosition`, `FretboardPosition`, `TechniqueEvidence`, `ConfidenceBps`, +/// `Pitch`, `Velocity`, `Ticks`, `Tempo` — and `slice.rs` holds `TickRange`, +/// which sits directly in `MasterBar` and `TechniqueSpan`. +/// +/// Listed rather than discovered: a module list is short, checkable by +/// reading `score.rs`'s imports, and cheaper than the alias-resolving +/// half-compiler that discovering it would need. The list is load-bearing, so +/// `the_no_usize_witness_scans_every_module_the_canonical_tree_reaches` +/// checks that each entry actually contributes. +const CANONICAL_SOURCES: [&str; 3] = [ + "core/src/score.rs", + "core/src/event.rs", + "core/src/slice.rs", +]; + +/// Every field declaration of the canonical tree, across all its modules. +fn canonical_field_types() -> Vec { + CANONICAL_SOURCES + .iter() + .flat_map(|path| declared_field_types(&read(path))) + .collect() +} + +/// Whether `haystack` uses `needle` as a whole type token. +/// +/// A substring match would let `MyUsizeAlias` count as `usize`, and — more +/// to the point — would let a genuine `usize` hide inside a longer name that +/// happens to contain it. +fn mentions_type(haystack: &str, needle: &str) -> bool { + haystack + .split(|c: char| !(c.is_alphanumeric() || c == '_')) + .any(|token| token == needle) +} + +#[test] +fn the_canonical_tree_declares_no_platform_sized_integer() { + // Spec §1.2 forbids platform-sized integers in hashed or serialized + // state, and `Score` derives `Hash`. H3 recorded the three fields that + // violated it; SWG-CORE-01 closed them. This witness is what keeps a + // fourth from arriving quietly — including inside an enum payload, which + // no struct-field scan reaches. + let offenders: Vec = canonical_field_types() + .into_iter() + .filter(|declaration| mentions_type(declaration, "usize")) + .collect(); + assert!( + offenders.is_empty(), + "`usize` is not a canonical width — spec §1.2, H3: {offenders:?}" + ); +} + +#[test] +fn the_three_migrated_index_fields_are_u64() { + // The general witness above would stay green if a field were widened to + // `u128` or narrowed to `u32`. These three carry the decision SWG-CORE-01 + // actually made, so they are named. + let declarations = canonical_field_types(); + for expected in [ + "MasterBar.index: u64", + "ImportWarning.track_index: u64", + "ImportWarning.bar_index: u64", + ] { + assert!( + declarations.iter().any(|d| d == expected), + "expected `{expected}` among {declarations:?}" + ); + } +} + +#[test] +fn the_declaration_scanner_reads_the_type_side() { + // Falsified against a synthetic source, because a scanner that silently + // found nothing would make both witnesses above vacuously green — the + // exact failure this suite has already hit twice. + let synthetic = "\ +pub struct Probe { + pub ordinal: usize, + pub index: u64, +} + +pub enum Warn { + Named { + track_index: usize, + }, + Bare, +} +"; + let declared = declared_field_types(synthetic); + assert_eq!( + declared, + vec![ + "Probe.ordinal: usize".to_owned(), + "Probe.index: u64".to_owned(), + "Warn.track_index: usize".to_owned(), + ] + ); + assert!(mentions_type("Probe.ordinal: usize", "usize")); + assert!(!mentions_type("Probe.index: u64", "usize")); + assert!( + !mentions_type("Probe.x: MyUsizeAlias", "usize"), + "a substring is not a type token" + ); +} + +// ── review round: the no-`usize` witness does not cover what it claims ───── + +/// The scanner must read **tuple** enum payload types, not only named ones. +/// +/// `declared_field_types` requires a `:` on the line, so `Other(String)` is +/// skipped entirely and `Tuple(usize)` would be too. The doc comment on +/// `the_canonical_tree_declares_no_platform_sized_integer` claims coverage +/// "including inside an enum payload"; for tuple payloads that claim is +/// false. The synthetic falsification did not notice, because it only used a +/// named payload — a witness against vacuous witnesses that was itself a +/// little vacuous. +#[test] +fn the_declaration_scanner_reads_tuple_payload_types() { + let synthetic = "\ +pub enum Probe { + Tuple(usize), + Two(String, u32), + Named { + alpha: u64, + }, + Bare, +} +"; + let declared = declared_field_types(synthetic); + assert_eq!( + declared, + vec![ + "Probe.0: usize".to_owned(), + "Probe.0: String".to_owned(), + "Probe.1: u32".to_owned(), + "Probe.alpha: u64".to_owned(), + ], + "every tuple element is its own declared type" + ); + assert!( + declared.iter().any(|d| mentions_type(d, "usize")), + "a `usize` in a tuple payload must be visible to the offender filter" + ); +} + +/// The scanner must read **tuple struct** types. +/// +/// The same hole one level up: `pub struct Pitch(pub u8);` has no braced body, +/// so the header match never fires. Those newtypes are canonical leaves — +/// §2.9 lists them — and a `pub struct Ordinal(pub usize);` added tomorrow +/// would be exact state the witness cannot see. +#[test] +fn the_declaration_scanner_reads_tuple_struct_types() { + let synthetic = "\ +pub struct Newtype(pub usize); + +pub struct Pair(pub u8, u32); + +pub struct Braced { + pub field: u64, +} +"; + assert_eq!( + declared_field_types(synthetic), + vec![ + "Newtype.0: usize".to_owned(), + "Pair.0: u8".to_owned(), + "Pair.1: u32".to_owned(), + "Braced.field: u64".to_owned(), + ] + ); +} + +/// The witness must scan every module the canonical tree reaches. +/// +/// It reads `core/src/score.rs` and nothing else, but `Score` reaches +/// `TimeSignature`, `Tuning`, `NoteMarks`, `NotePosition`, +/// `FretboardPosition`, `TechniqueEvidence`, `ConfidenceBps`, `Pitch`, +/// `Velocity`, and `Ticks` in `core/src/event.rs`, and `TickRange` in +/// `core/src/slice.rs`. The backlog records the acceptance criterion as "no +/// `usize` remains in any type reachable from `Score`" — which may well be +/// true, but this witness does not show it. +/// +/// The input expression below is the defect. It is the one the witness uses +/// today; the repair replaces it with the aggregate and leaves these +/// assertions where they are. +#[test] +fn the_no_usize_witness_scans_every_module_the_canonical_tree_reaches() { + let scanned = canonical_field_types(); + for expected in [ + "MasterBar.index: u64", + "TimeSignature.numerator: u8", + "TickRange.start: Ticks", + ] { + assert!( + scanned.iter().any(|d| d == expected), + "the scanned set must reach `{expected}`, found {} declarations", + scanned.len() + ); + } + + // Every listed module must actually contribute. Without this, dropping an + // entry from `CANONICAL_SOURCES` would narrow the witness back to where + // it started and only the three probes above would notice — and they + // would stop noticing the moment someone moved a type between modules. + for path in CANONICAL_SOURCES { + assert!( + !declared_field_types(&read(path)).is_empty(), + "{path} is listed as a canonical source but declares nothing" + ); + } +} diff --git a/swang/tests/exact_writer_structure.rs b/swang/tests/exact_writer_structure.rs index 72222d0a..5919b48c 100644 --- a/swang/tests/exact_writer_structure.rs +++ b/swang/tests/exact_writer_structure.rs @@ -51,7 +51,7 @@ fn bpm(n: u32) -> Tempo { Tempo::from_bpm_integer(n).expect("a positive integer BPM") } -fn bar(index: usize, start: u32, end: u32) -> MasterBar { +fn bar(index: u64, start: u32, end: u32) -> MasterBar { MasterBar { index, tick_range: range(start, end), diff --git a/swang/tests/exact_writer_transport.rs b/swang/tests/exact_writer_transport.rs index c8a0c8c5..473ae95b 100644 --- a/swang/tests/exact_writer_transport.rs +++ b/swang/tests/exact_writer_transport.rs @@ -32,7 +32,7 @@ fn range(start: u32, end: u32) -> TickRange { TickRange::new(Ticks(start), Ticks(end)).expect("ordered range") } -fn bar(index: usize, start: u32, end: u32, tempo: Tempo) -> MasterBar { +fn bar(index: u64, start: u32, end: u32, tempo: Tempo) -> MasterBar { MasterBar { index, tick_range: range(start, end), diff --git a/ui-core/src/analysis.rs b/ui-core/src/analysis.rs index a3e13491..e69b1eeb 100644 --- a/ui-core/src/analysis.rs +++ b/ui-core/src/analysis.rs @@ -21,9 +21,12 @@ pub struct Section { /// The classification shared by every bar in the run. pub class: BarClass, /// First bar index (inclusive). - pub bar_start: usize, - /// One past the last bar index (exclusive). - pub bar_end: usize, + /// + /// Copied from the stored `MasterBar::index`, so it carries the canonical + /// width rather than a platform-sized one (SWG-CORE-01). + pub bar_start: u64, + /// One past the last bar index (exclusive), in the same width. + pub bar_end: u64, /// Absolute onset tick of the section. pub tick_start: u32, /// Absolute end tick of the section (exclusive). @@ -33,7 +36,7 @@ pub struct Section { impl Section { /// Number of bars covered by the section. #[must_use] - pub const fn bar_count(&self) -> usize { + pub const fn bar_count(&self) -> u64 { self.bar_end.saturating_sub(self.bar_start) } } @@ -154,6 +157,7 @@ mod tests { use super::*; use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; + use griff_core::score::index_from_ordinal; use griff_core::score::{ AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Track, Voice, }; @@ -165,7 +169,7 @@ mod tests { fn bar_of_notes(index: usize, pitches: &[(u8, u8)]) -> (MasterBar, Vec) { let start = u32::try_from(index).expect("small") * BAR; let mb = MasterBar { - index, + index: index_from_ordinal(index), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature::new(4, 4).expect("4/4"), tempo: Tempo::from_bpm_integer(120).expect("120 BPM"), diff --git a/ui-core/src/generate.rs b/ui-core/src/generate.rs index 10018412..41dc54d7 100644 --- a/ui-core/src/generate.rs +++ b/ui-core/src/generate.rs @@ -424,6 +424,7 @@ mod tests { use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::generate::{GenerationSeed, GenerationStrategy}; use griff_core::rerank::SetCandidate; + use griff_core::score::index_from_ordinal; use griff_core::score::{ AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, Track, Voice, }; @@ -528,7 +529,7 @@ mod tests { for (i, &pitch) in pitches.iter().enumerate() { let start = u32::try_from(i).expect("small") * BAR; master_bars.push(MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature::new(4, 4).expect("4/4"), tempo: Tempo::from_bpm_integer(120).expect("120"), diff --git a/ui-core/src/view.rs b/ui-core/src/view.rs index 22f8def5..b4c2a424 100644 --- a/ui-core/src/view.rs +++ b/ui-core/src/view.rs @@ -158,8 +158,8 @@ mod tests { use super::*; use griff_core::event::{NoteMarks, Pitch, Tempo, Ticks, TimeSignature, Tuning, Velocity}; use griff_core::score::{ - AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, RepeatMarker, - Score, Track, Voice, + index_from_ordinal, AtomEvent, AtomNote, EventGroup, EventGroupKind, LossReport, MasterBar, + RepeatMarker, Score, Track, Voice, }; use griff_core::slice::TickRange; @@ -189,7 +189,7 @@ mod tests { .map(|i| { let start = u32::try_from(i).expect("small") * BAR; MasterBar { - index: i, + index: index_from_ordinal(i), tick_range: TickRange::new(Ticks(start), Ticks(start + BAR)).expect("ordered"), time_signature: TimeSignature::new(4, 4).expect("4/4"), tempo: Tempo::from_bpm_integer(120).expect("120 BPM"),