diff --git a/crates/epiphany-core/DECISIONS.md b/crates/epiphany-core/DECISIONS.md index 91bdc02..7dc5e0c 100644 --- a/crates/epiphany-core/DECISIONS.md +++ b/crates/epiphany-core/DECISIONS.md @@ -60,6 +60,12 @@ defined layout for the registry id. This crate encodes it as confirm the registry-id encoding (and whether `ObjectKindRegistryId` is a 128-bit value, as assumed here). +**Locked (M3 follow-up).** The discriminant table and `Registered` layout are now +pinned by a golden-bytes test (`typed_object_id_byte_form_is_locked`): any +reorder, discriminant reassignment, or layout change breaks it deliberately, +since these bytes are normative (ordering/hashing/equality). The values remain +*this crate's proposal* until the spec adopts or overrides them. + ### P11-2 — Graph-invariant count: spec body says 19, QUICKSTART says 18 `spec/QUICKSTART.md` (Agent B) refers to "the 18 graph invariants enumerated in @@ -87,6 +93,13 @@ The core spec listing now carries both operation ids. The exact hash-domain derivation remains provisional until the semantic-operations companion ratifies `derive_promoted_voice_id`. +**Locked (M3 follow-up).** The 64-byte `MUSCSVCE` preimage — `staff_instance || +original_voice || winning_op || losing_op`, each 16 big-endian bytes — and its +hash output are pinned by a golden-bytes test +(`promoted_voice_id_byte_form_is_locked`), so the layout cannot drift unnoticed; +the companion's ratification (or a different derivation) will update both the code +and that golden. + ### P11-4 — A prototype canonical encoding precedes the Binary Format companion Appendix D and Chapter 8 defer the canonical wire encoding of graph value types @@ -158,6 +171,13 @@ the pitch from a fixed canonical byte form of its intrinsic identity (scale position + acoustic realization; strings length-prefixed and NFC). The spec should pin the canonical input layout (or define a different derivation). +**Locked (M3 follow-up).** `canonical_pitch_bytes` now NFC-normalizes its string +fields *at the derivation boundary* (not merely relying on catalog ids being NFC +at construction), making the "NFC" guarantee explicit, and the `MUSCSPCH` input +layout + hash output are pinned by a golden-bytes test +(`system_pitch_id_byte_form_is_locked`). The exact field set ("intrinsic +identity") and layout are still this crate's proposal pending spec ratification. + ### P11-7 — Tempo "Linear" interpolation parameter Chapter 3 says a `Linear` segment is "linear interpolation from `start_tempo` to diff --git a/crates/epiphany-core/src/graph.rs b/crates/epiphany-core/src/graph.rs index ab6fae8..55ef997 100644 --- a/crates/epiphany-core/src/graph.rs +++ b/crates/epiphany-core/src/graph.rs @@ -1213,6 +1213,26 @@ mod tests { assert_ne!(a, c); } + #[test] + fn promoted_voice_id_byte_form_is_locked() { + // Golden: locks the MUSCSVCE 64-byte preimage layout (staff instance || + // original voice || winning op || losing op, each 16 big-endian bytes) + // and the hash output. A change to the input order, byte layout, or + // domain tag breaks this deliberately, forcing the derivation change to + // be acknowledged (DECISIONS P11-3). + let id = derive_promoted_voice_id( + StaffInstanceId::new(ReplicaId(3), 1), + VoiceId::new(ReplicaId(3), 2), + OperationId::new(ReplicaId(3), 10), + OperationId::new(ReplicaId(4), 11), + ); + assert_eq!(id.replica(), ReplicaId::SYSTEM_DERIVED); + const GOLDEN: [u8; 16] = [ + 255, 255, 255, 255, 255, 255, 255, 255, 76, 193, 12, 43, 57, 51, 131, 242, + ]; + assert_eq!(id.canonical_bytes(), GOLDEN); + } + #[test] fn wallclock_time_overlap_is_half_open() { let r = ReplicaId(1); diff --git a/crates/epiphany-core/src/ids.rs b/crates/epiphany-core/src/ids.rs index 7605261..3ea197b 100644 --- a/crates/epiphany-core/src/ids.rs +++ b/crates/epiphany-core/src/ids.rs @@ -887,6 +887,26 @@ mod tests { } } + #[test] + fn typed_object_id_byte_form_is_locked() { + // Golden: locks the 16-bit big-endian discriminant table (DECISIONS P11-1) + // and the payload layout. A reorder or discriminant reassignment breaks + // this deliberately — these bytes are normative (ordering/hashing/equality). + let r = ReplicaId(9); + // Event = discriminant 0, then the 16-byte big-endian id payload. + let event = TypedObjectId::Event(EventId::new(r, 1)).canonical_bytes(); + const GOLDEN_EVENT: &[u8] = &[0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 0, 0, 0, 1]; + assert_eq!(event, GOLDEN_EVENT); + // Registered = discriminant 27, then registry id (16) + raw extension (16). + let reg = TypedObjectId::Registered(ObjectKindRegistryId::new(r, 99), 0xdead_beef) + .canonical_bytes(); + const GOLDEN_REGISTERED: &[u8] = &[ + 0, 27, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 0, 0, 0, 99, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 222, 173, 190, 239, + ]; + assert_eq!(reg, GOLDEN_REGISTERED); + } + #[test] fn operation_id_orders_by_replica_then_counter() { let a = OperationId::new(ReplicaId(1), 9); diff --git a/crates/epiphany-core/src/invariants.rs b/crates/epiphany-core/src/invariants.rs index b932da4..776a99a 100644 --- a/crates/epiphany-core/src/invariants.rs +++ b/crates/epiphany-core/src/invariants.rs @@ -167,8 +167,57 @@ impl core::fmt::Display for InvariantViolation { } } +/// A Chapter 5 well-formedness check this crate could not *decide* for a given +/// score — e.g., a region-overlap test whose extents use symbolic +/// [`crate::TimeAnchor`]s that need the full tempo/measure machinery (out of this +/// crate's scope) to place on a common timeline. +/// +/// [`check_invariants`] stays *sound* — it never raises a false positive on an +/// undecidable check — but a sound-and-silent checker would treat "couldn't +/// decide" identically to "clean". [`deferred_checks`] makes the undecided cases +/// explicit and testable instead, so a stricter conformance profile can choose +/// to reject them rather than have them pass unseen. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct DeferredCheck { + /// The invariant whose decision was deferred. + pub invariant: GraphInvariant, + /// A human-readable witness explaining why it could not be decided. + pub reason: String, +} + +impl core::fmt::Display for DeferredCheck { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!( + f, + "invariant {} ({:?}) deferred: {}", + self.invariant.number(), + self.invariant, + self.reason + ) + } +} + +/// The well-formedness checks [`check_invariants`] could not decide for `score`. +/// +/// Currently this is region-overlap pairs that share a staff extent but whose +/// time overlap does not resolve to a common timeline (symbolic anchors needing +/// tempo/measure resolution). An empty result means every modelled invariant was +/// fully decided. The core checker reports these rather than silently accepting +/// them as valid; the caller decides how strict to be. +pub fn deferred_checks(score: &Score) -> Vec { + let idx = GraphIndex::build(score); + let mut out = Vec::new(); + idx.deferred_region_overlaps(&mut out); + out +} + /// Checks every Chapter 5 graph invariant over `score`, returning all /// violations found (empty iff the graph is well-formed). +/// +/// This is *sound but incomplete* for the few invariants that need absolute-time +/// resolution: it never raises a false positive, and the cases it could not +/// decide are reported separately by [`deferred_checks`] rather than silently +/// passed. pub fn check_invariants(score: &Score) -> Vec { let idx = GraphIndex::build(score); let mut v = Vec::new(); @@ -626,8 +675,11 @@ impl<'a> GraphIndex<'a> { } // No two regions overlap in both time and staff extent. Time overlap // is decided on a common resolved timeline (wall-clock, event/region/ - // measure-start anchors); pairs whose extents cannot be resolved to a - // common clock are skipped (sound but incomplete — see DECISIONS P11-4). + // measure-start anchors). Pairs whose extents cannot be resolved to a + // common clock are *not* silently passed: they are reported as undecided + // by `deferred_checks` (via `deferred_region_overlaps`) rather than + // treated as disjoint here. This check stays sound — it only flags a + // proven overlap. let regions = &self.score.canvas.regions; for i in 0..regions.len() { for j in (i + 1)..regions.len() { @@ -648,6 +700,33 @@ impl<'a> GraphIndex<'a> { } } + /// Region-overlap pairs that share a staff extent but whose time overlap is + /// undecidable here (symbolic anchors needing tempo/measure resolution). + /// Surfaced by [`deferred_checks`] so the undecided case is explicit, never + /// silently accepted as disjoint. + fn deferred_region_overlaps(&self, out: &mut Vec) { + let regions = &self.score.canvas.regions; + for i in 0..regions.len() { + for j in (i + 1)..regions.len() { + let (a, b) = (®ions[i], ®ions[j]); + if !a.staff_extent_intersects(b) { + continue; + } + if self.regions_overlap_in_time(a, b).is_none() { + out.push(DeferredCheck { + invariant: GraphInvariant::RegionExtents, + reason: format!( + "regions {:?} and {:?} share a staff extent but their time \ + overlap is undecidable (symbolic anchors need tempo/measure \ + resolution)", + a.id, b.id + ), + }); + } + } + } + } + /// `Some(true)`/`Some(false)` when both regions' extents resolve to absolute /// wall-clock coordinates; `None` when they cannot be compared (deferred /// tempo/measure machinery). Half-open: touching at a boundary is not an @@ -2358,7 +2437,12 @@ impl Endpoints { } EventPosition::WallClock(t) => { let end = match e.duration() { - EventDuration::WallClock(d) => t.0.saturating_add(d.0), + // Overflow is unresolvable, not a saturated (wrong) endpoint + // that could mask an ordering violation — report Unknown. + EventDuration::WallClock(d) => match t.0.checked_add(d.0) { + Some(end) => end, + None => return Endpoints::Unknown, + }, EventDuration::Indeterminate(_) => t.0, EventDuration::Musical(_) => return Endpoints::Unknown, }; @@ -2470,6 +2554,65 @@ mod review_fix_tests { assert!(!fires(&s, GraphInvariant::RegionExtents)); } + #[test] + fn inv7_unresolvable_overlap_is_deferred_not_silently_valid() { + let mut s = valid_score(1); + let staff = s.staves[0].id; + let r0 = s.canvas.regions[0].id; + let rid = s.identity.mint(); + let inst = StaffInstance::new(s.identity.mint(), staff); + // A second region on the same staff whose extent is anchored + // region-relative with a *musical* offset; with no tempo map it cannot + // resolve to wall-clock, so its overlap with region 0's wall-clock extent + // is undecidable. + let symbolic = TimeExtent { + start: TimeAnchor::Region { + id: r0, + edge: RegionEdge::Start, + offset: AnchorOffset::Musical(MusicalDuration::zero()), + }, + end: TimeAnchor::Region { + id: r0, + edge: RegionEdge::Start, + offset: AnchorOffset::Musical(MusicalDuration::whole()), + }, + }; + s.canvas.regions.push(Region { + id: rid, + time_model: RegionTimeModel::Metric(MetricTimeModel::default()), + content: RegionContent::StaffBased(StaffBasedContent { + staff_instances: vec![inst], + ..Default::default() + }), + time_extent: symbolic, + staff_extent: StaffExtent { + staves: vec![staff], + }, + local_tempo_map: None, + }); + + // Sound: the undecidable overlap is NOT raised as a (false-positive) + // violation — `check_invariants` stays clean... + assert!( + check_invariants(&s).is_empty(), + "unexpected violations: {:?}", + check_invariants(&s) + ); + // ...but it is surfaced as a deferred check naming both regions, rather + // than silently treated as disjoint/valid. + let deferred = deferred_checks(&s); + assert_eq!(deferred.len(), 1, "{deferred:?}"); + assert_eq!(deferred[0].invariant, GraphInvariant::RegionExtents); + assert!(deferred[0].reason.contains(&format!("{r0:?}"))); + assert!(deferred[0].reason.contains(&format!("{rid:?}"))); + + // A wall-clock (resolvable), disjoint second region is *decided*, so it is + // neither a violation nor deferred. + s.canvas.regions.last_mut().unwrap().time_extent = wc(2_000_000, 3_000_000); + assert!(deferred_checks(&s).is_empty()); + assert!(check_invariants(&s).is_empty()); + } + #[test] fn inv9_flags_offset_on_event_anchored_meter_change() { // A proportional region whose own meter list is empty; place a metric diff --git a/crates/epiphany-core/src/lib.rs b/crates/epiphany-core/src/lib.rs index f5e6e54..497d68b 100644 --- a/crates/epiphany-core/src/lib.rs +++ b/crates/epiphany-core/src/lib.rs @@ -111,4 +111,7 @@ pub use codec::ScoreDecodeError; pub use indexes::ScoreIndexes; -pub use invariants::{check_invariant, check_invariants, GraphInvariant, InvariantViolation}; +pub use invariants::{ + check_invariant, check_invariants, deferred_checks, DeferredCheck, GraphInvariant, + InvariantViolation, +}; diff --git a/crates/epiphany-core/src/pitch.rs b/crates/epiphany-core/src/pitch.rs index 730a0c8..633130d 100644 --- a/crates/epiphany-core/src/pitch.rs +++ b/crates/epiphany-core/src/pitch.rs @@ -425,9 +425,15 @@ impl Pitch { /// catalog ids normalize on construction); the layout is fixed-shape so equal /// pitches encode to equal bytes (Appendix D §"Canonical serialization"). fn canonical_pitch_bytes(p: &Pitch) -> Vec { + // Length-prefixed UTF-8, normalized to NFC at the derivation boundary so the + // canonical input is NFC regardless of how the string was obtained (Appendix + // D §"Text and Unicode"). Catalog ids are already NFC at construction, so this + // is a no-op for them; normalizing here makes the NFC guarantee explicit and + // robust rather than relying on every caller. fn push_str(out: &mut Vec, s: &str) { - out.extend_from_slice(&(s.len() as u32).to_le_bytes()); - out.extend_from_slice(s.as_bytes()); + let nfc: String = s.nfc().collect(); + out.extend_from_slice(&(nfc.len() as u32).to_le_bytes()); + out.extend_from_slice(nfc.as_bytes()); } let mut out = Vec::new(); push_str(&mut out, p.scale_position.space.as_str()); @@ -889,6 +895,21 @@ mod tests { assert_ne!(id, derive_system_pitch_id(&cmn(CmnNominal::D, 0, 4))); } + #[test] + fn system_pitch_id_byte_form_is_locked() { + // Golden: locks the MUSCSPCH canonical-input layout (space name, scale + // position discriminant + payload, tuning, acoustic realization; strings + // length-prefixed NFC) and the hash. A change to the byte form breaks + // this deliberately, forcing the derivation change to be acknowledged + // (DECISIONS P11-6). + let id = derive_system_pitch_id(&cmn(CmnNominal::C, 0, 4)); + assert_eq!(id.replica(), crate::ids::ReplicaId::SYSTEM_DERIVED); + const GOLDEN: [u8; 16] = [ + 255, 255, 255, 255, 255, 255, 255, 255, 164, 31, 138, 24, 68, 38, 241, 168, + ]; + assert_eq!(id.canonical_bytes(), GOLDEN); + } + #[test] fn reference_pitch_rejects_non_positive_frequency() { let pos = PitchSpacePosition::Cmn { diff --git a/crates/epiphany-core/src/tempo.rs b/crates/epiphany-core/src/tempo.rs index e205229..0f7d000 100644 --- a/crates/epiphany-core/src/tempo.rs +++ b/crates/epiphany-core/src/tempo.rs @@ -32,7 +32,7 @@ //! musical time is the exact rational and wall-clock is exact nanoseconds //! (Appendix D §"Exact and Quantized Representations"). -use epiphany_determinism::CanonicalF64; +use epiphany_determinism::{CanonicalF64, Tolerance, ToleranceClass, ToleranceGovernance}; use crate::time::{ MusicalDuration, MusicalPosition, RationalTime, RegionEdge, TimeAnchor, WallClockTime, @@ -158,6 +158,28 @@ pub const INVERSION_MAX_DENOMINATOR: u64 = 1_000_000; /// ordinary rhythm round-trips, yet far smaller than any musical distinction. pub const INVERSION_TOLERANCE_WHOLE_NOTES: f64 = 1e-6; +/// Relative tolerance below which two segment speeds count as *equal*, so the +/// integration takes the numerically-stable constant-speed limit instead of the +/// general logarithmic form (which loses precision via catastrophic cancellation +/// as the speeds converge). Expressed as a named [`Tolerance`] of class +/// [`ToleranceClass::TempoIntegration`] per Appendix D §"Tolerance Classes" (no +/// ad-hoc epsilons); the magnitude is far below any musical tempo distinction. +const TEMPO_SPEED_DEGENERACY_RELATIVE: f64 = 1e-12; + +/// The typed degeneracy tolerance used by [`SpeedModel`] to decide whether two +/// speeds are equal for integration purposes (see +/// [`TEMPO_SPEED_DEGENERACY_RELATIVE`]). `within(s1, s0)` is non-finite-safe and +/// relative to `s0`, replacing the former ad-hoc `f64::EPSILON` guards. +fn speed_degeneracy_tolerance() -> Tolerance { + Tolerance::absolute( + ToleranceClass::TempoIntegration, + 0.0, + ToleranceGovernance::Validation, + ) + .and_then(|t| t.with_relative(TEMPO_SPEED_DEGENERACY_RELATIVE)) + .expect("constant degeneracy tolerance is finite and non-negative") +} + /// The closed-form integration model of one stretch of musical time: a /// half-open interval `[start, end)` in whole notes (`end == None` is open) plus /// how the speed (whole notes per second) behaves across it. Built from the @@ -193,7 +215,7 @@ impl SpeedModel { SpeedModel::Const(s) => len * (u1 - u0) / s, SpeedModel::Linear { s0, s1 } => { let d = s1 - s0; - if d.abs() < f64::EPSILON { + if speed_degeneracy_tolerance().within(*s1, *s0) { len * (u1 - u0) / s0 } else { // ∫ du/(s0 + d·u) = (1/d) ln(s(u)); seconds scale by `len`. @@ -204,7 +226,7 @@ impl SpeedModel { } SpeedModel::Exponential { s0, s1 } => { let l = (s1 / s0).ln(); - if l.abs() < f64::EPSILON { + if speed_degeneracy_tolerance().within(*s1, *s0) { len * (u1 - u0) / s0 } else { // s(u) = s0·e^{l·u}; ∫ du/s(u) = (e^{-l·u0} - e^{-l·u1})/(s0·l). @@ -221,7 +243,7 @@ impl SpeedModel { SpeedModel::Const(s) => secs * s / len, SpeedModel::Linear { s0, s1 } => { let d = s1 - s0; - if d.abs() < f64::EPSILON { + if speed_degeneracy_tolerance().within(*s1, *s0) { secs * s0 / len } else { // secs = len/d · ln(s(u)/s0) ⇒ s(u) = s0·e^{secs·d/len}. @@ -231,7 +253,7 @@ impl SpeedModel { } SpeedModel::Exponential { s0, s1 } => { let l = (s1 / s0).ln(); - if l.abs() < f64::EPSILON { + if speed_degeneracy_tolerance().within(*s1, *s0) { secs * s0 / len } else { // secs = len/(s0·l)·(1 - e^{-l·u}) ⇒ u = -ln(1 - secs·s0·l/len)/l. @@ -577,8 +599,15 @@ fn rational_from_f64(x: f64, max_den: u64, tol: f64) -> Option { break; } let ai = a as i128; - let h = ai * h_prev1 + h_prev2; - let k = ai * k_prev1 + k_prev2; + // Checked convergent recurrence: a pathological `ai` must never wrap i128 + // silently (which would yield a wrong rational that still fits the final + // range check). On overflow, keep the last good convergent. + let Some(h) = ai.checked_mul(h_prev1).and_then(|p| p.checked_add(h_prev2)) else { + break; + }; + let Some(k) = ai.checked_mul(k_prev1).and_then(|p| p.checked_add(k_prev2)) else { + break; + }; if k <= 0 || (k as u128) > max_den as u128 { break; } @@ -591,7 +620,10 @@ fn rational_from_f64(x: f64, max_den: u64, tol: f64) -> Option { break; } let frac = value - a; - if frac.abs() < f64::EPSILON { + // Stop once the residual fraction is too small to introduce another + // convergent within `max_den` (a meaningful bound, not an ad-hoc epsilon); + // this also keeps the next `value = 1/frac`, hence `ai`, bounded. + if frac <= 1.0 / max_den as f64 { break; } value = 1.0 / frac; @@ -688,6 +720,39 @@ mod tests { assert_eq!(map.wallclock_to_musical(th).unwrap(), half); } + #[test] + fn equal_endpoint_linear_segment_uses_the_constant_limit() { + // start_tempo == end_tempo: the typed speed-degeneracy tolerance must + // select the constant-speed limit, integrating identically to a constant + // tempo (no catastrophic cancellation, no NaN from the general ln-form). + let map = TempoMap { + initial: None, + segments: vec![TempoSegment { + start: region_at(RationalTime::zero()), + end: Some(region_at(RationalTime::from_int(1))), + start_tempo: Tempo::quarter(120.0).unwrap(), + end_tempo: Some(Tempo::quarter(120.0).unwrap()), + shape: TempoShape::Linear, + }], + }; + let one = MusicalPosition(RationalTime::from_int(1)); + let t = map.musical_to_wallclock(&one).unwrap(); + // 120 q-bpm constant => one whole note = 2 s. + assert_eq!(t, WallClockTime(2_000_000_000)); + assert_eq!(map.wallclock_to_musical(t).unwrap(), one); + } + + #[test] + fn inversion_handles_extreme_inputs_without_overflow() { + // Pathological wall-clock times must not panic or wrap the continued- + // fraction convergent recurrence; each resolves to a finite musical + // position or a clean error, never UB. + let map = TempoMap::constant(Tempo::quarter(120.0).unwrap()); + for t in [i64::MAX, i64::MIN, i64::MAX - 1, 1_000_000_000_000_000] { + let _ = map.wallclock_to_musical(WallClockTime(t)); + } + } + #[test] fn exponential_segment_is_integrated() { let map = TempoMap {