note: close every value type's domain at construction, so resolveNote is finite for every constructible input

Division and OffsetAmount get single normalizing doors and private constructors; fromBpm validates by running the conversions rather than their reciprocal. Readers drop their re-clamps and default labels.

src/core/instrument/note/CLAUDE.md

@@ -31,6 +31,19 @@ diverge: the capture-signal popup that edits it and the bake that renders it.
   the ladder ever gained a rung or a modifier.
 - **An offset stores the denomination it was entered in** — see `OffsetAmount` in
   `note_program.h` for why.
+- **Every value type establishes its domain at construction, and nothing downstream can
+  fail.** `Tempo::fromBpm` rejects, alone, because an unusable BPM has no nearest usable one
+  to fall to. `Division`, `OffsetAmount`, and `Velocity` clamp, because an off-ladder rung,
+  an unrepresentable magnitude, and an out-of-range velocity each do. Each has exactly one
+  door (`makeDivision`, `offsetOf`, `Velocity::of`) and a private constructor behind it, so
+  an out-of-domain value cannot be held, only passed in. That is what lets every reader
+  branch without a fallback, equality compare fields raw, and `resolveNote` return finite
+  times for every constructible input with no failure path and no validity flag.
+- **The module will not tell a caller a record is junk, because a junk record cannot exist
+  here.** Corruption is only visible where raw bytes are: a codec sees both the bytes it
+  read and the value construction produced, and reporting the difference is the codec's job.
+  Do not add a validity flag to `NoteProgram` or `ResolvedNote` to carry that signal upward
+  — `windowCollapsed` describes a legal program, and is not the seed of an error channel.
 - **Does not carry a MIDI note number.** `NoteProgram` describes timing and velocity only;
   render pitch is deferred to a later additive field (Ξ-W2) rather than assumed to live
   here.
@@ -41,9 +54,11 @@ diverge: the capture-signal popup that edits it and the bake that renders it.
   exponent of its length in quarter notes, -4..8), each straight, dotted (x3/2), or triplet
   (x2/3); the 39-entry picker order; and the `"1/8."` / `"1/4t"` label notation. Beats only
   — see `musical_division.h` for why it links no tempo.
-- `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion.
-  Construction (`Tempo::fromBpm`) is the only place a bad BPM is rejected, which is what
-  lets each conversion be total and every downstream resolver be failure-free.
+- `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion, and
+  `kMaxConvertibleMagnitude`, the beats-or-ms ceiling the whole directory caps its domains
+  to. `fromBpm` validates by running the extreme conversions rather than by testing the
+  `60/bpm` reciprocal they start from — that reciprocal stays finite well past the point the
+  multiply after it overflows.
 - `note_program` — `Velocity` (clamped 1..127), the denominated `OffsetAmount` and its unit
   toggle, the anchored `StartOffset` / `EndOffset`, the `NoteProgram` record, and
   `resolveNote`.
@@ -64,3 +79,8 @@ diverge: the capture-signal popup that edits it and the bake that renders it.
   Both are legal; `resolveNote` only refuses to invert the window.
 - **ms <-> beats round-trips are lossless to double precision, not bit-identical.** The
   conversion is a multiply/divide pair; compare with an epsilon.
+- **Editing the ms field of a beats-stored offset stores beats, and the ms readout will then
+  move with the tempo.** `withMsView` keeps the stored denomination on purpose, so typing 250
+  into the ms field of a beats offset stores 0.5 beats at 120 BPM. That is the intended
+  semantic, but it is a UI-visible surprise worth a word in the popup: `redenominate` — the
+  unit toggle — is the only thing that changes which denomination is stored.

src/core/instrument/note/CMakeLists.txt

@@ -6,8 +6,7 @@ reasampler_test(musical_division LINK musical_division)
 reasampler_pure_library(tempo SOURCES tempo.cpp)
 reasampler_test(tempo LINK tempo)
 
-# note_program links exactly these two: it composes the ladder and the tempo and nothing
-# else (see note_program.h).
+# note_program links exactly these two: it composes the ladder and the tempo and nothing else.
 reasampler_pure_library(note_program
     SOURCES note_program.cpp
     LINK PUBLIC musical_division tempo)

src/core/instrument/note/musical_division.cpp

@@ -21,28 +21,26 @@ int clampExponent(int quarterExponent) {
     return (std::max)(kMinQuarterExponent, (std::min)(kMaxQuarterExponent, quarterExponent));
 }
 
+// The underlying type is unsigned, so an out-of-enum byte can only be too large.
+DivisionModifier clampModifier(DivisionModifier m) {
+    return static_cast<int>(m) < kModifierCount ? m : DivisionModifier::Straight;
+}
+
 }  // namespace
 
 bool operator==(Division a, Division b) {
-    // Normalize both sides through makeDivision first: a persisted off-ladder exponent must
-    // compare equal to its clamped form, the same as every other reader in this file.
-    const Division la = makeDivision(a.quarterExponent, a.modifier);
-    const Division lb = makeDivision(b.quarterExponent, b.modifier);
-    return la.quarterExponent == lb.quarterExponent && la.modifier == lb.modifier;
+    return a.quarterExponent() == b.quarterExponent() && a.modifier() == b.modifier();
 }
 
 bool operator!=(Division a, Division b) { return !(a == b); }
 
 Division makeDivision(int quarterExponent, DivisionModifier modifier) {
-    Division d;
-    d.quarterExponent = static_cast<std::int8_t>(clampExponent(quarterExponent));
-    d.modifier = modifier;
-    return d;
+    return Division(static_cast<std::int8_t>(clampExponent(quarterExponent)),
+                    clampModifier(modifier));
 }
 
 double divisionBeats(Division d) {
-    const Division legal = makeDivision(d.quarterExponent, d.modifier);
-    return std::ldexp(1.0, legal.quarterExponent) * modifierFactor(legal.modifier);
+    return std::ldexp(1.0, d.quarterExponent()) * modifierFactor(d.modifier());
 }
 
 Division divisionAt(int index) {
@@ -52,20 +50,18 @@ Division divisionAt(int index) {
 }
 
 int divisionIndex(Division d) {
-    const Division legal = makeDivision(d.quarterExponent, d.modifier);
-    return (legal.quarterExponent - kMinQuarterExponent) * kModifierCount
-           + static_cast<int>(legal.modifier);
+    return (d.quarterExponent() - kMinQuarterExponent) * kModifierCount
+           + static_cast<int>(d.modifier());
 }
 
 std::string divisionLabel(Division d) {
-    const Division legal = makeDivision(d.quarterExponent, d.modifier);
-    const int e = legal.quarterExponent;
+    const int e = d.quarterExponent();
     // Both branches meet at e == 2 ("1/1"): a division's written form is its length in
     // whole notes, which is 2^(e-2).
     std::string label = e <= 2 ? "1/" + std::to_string(1 << (2 - e))
                                : std::to_string(1 << (e - 2)) + "/1";
-    if (legal.modifier == DivisionModifier::Dotted) label += '.';
-    else if (legal.modifier == DivisionModifier::Triplet) label += 't';
+    if (d.modifier() == DivisionModifier::Dotted) label += '.';
+    else if (d.modifier() == DivisionModifier::Triplet) label += 't';
     return label;
 }
 

src/core/instrument/note/musical_division.h

@@ -6,6 +6,7 @@
 
 #include <cstdint>
 #include <string>
+#include <type_traits>
 
 namespace reasampler::instrument::note {
 
@@ -24,19 +25,43 @@ inline constexpr int kRungCount = kMaxQuarterExponent - kMinQuarterExponent + 1;
 inline constexpr int kModifierCount = 3;
 inline constexpr int kDivisionCount = kRungCount * kModifierCount;
 
-struct Division {
-    std::int8_t quarterExponent = 0;  // 1/4
-    DivisionModifier modifier = DivisionModifier::Straight;
+// The longest programmable note — the dotted top rung — so a caller composing this ladder
+// with the tempo conversions can check the two domains against each other at compile time.
+inline constexpr double kMaxDivisionBeats = (1 << kMaxQuarterExponent) * 1.5;
+
+class Division;
+
+// Off-ladder inputs clamp rather than reject: the only ways to reach one are a corrupt
+// persisted record or a picker bug, and the nearest legal length beats a nonsense duration.
+// An unnamed modifier byte has no nearest rung to fall to, so it takes the field's default.
+Division makeDivision(int quarterExponent, DivisionModifier modifier);
+
+// In-domain by construction — `makeDivision` is the only door and it clamps BOTH fields, so
+// every reader below trusts the stored pair instead of re-clamping it, and equality compares
+// the two fields raw without disagreeing with any of them.
+class Division {
+public:
+    Division() = default;  // 1/4 straight
+
+    constexpr std::int8_t quarterExponent() const { return quarterExponent_; }
+    constexpr DivisionModifier modifier() const { return modifier_; }
+
+private:
+    Division(std::int8_t quarterExponent, DivisionModifier modifier)
+        : quarterExponent_(quarterExponent), modifier_(modifier) {}
+    friend Division makeDivision(int quarterExponent, DivisionModifier modifier);
+
+    std::int8_t quarterExponent_ = 0;
+    DivisionModifier modifier_ = DivisionModifier::Straight;
 };
 
+static_assert(!std::is_constructible_v<Division, int, DivisionModifier>,
+              "makeDivision must be the only way to give a Division a rung");
+
 bool operator==(Division a, Division b);
 bool operator!=(Division a, Division b);
 
-// Off-ladder exponents clamp rather than reject: the only ways to reach one are a corrupt
-// persisted record or a picker bug, and the nearest legal length beats a nonsense duration.
-Division makeDivision(int quarterExponent, DivisionModifier modifier);
-
-// Length in beats (quarter notes). Always > 0.
+// Length in beats (quarter notes). Always > 0, and never above kMaxDivisionBeats.
 double divisionBeats(Division d);
 
 // Picker order: shortest rung first, straight/dotted/triplet within each rung. Index is

src/core/instrument/note/note_program.cpp

@@ -3,6 +3,7 @@
 #include "core/instrument/note/note_program.h"
 
 #include <algorithm>
+#include <cmath>
 
 namespace reasampler::instrument::note {
 
@@ -15,56 +16,69 @@ Velocity Velocity::of(int value) {
 bool operator==(Velocity a, Velocity b) { return a.value() == b.value(); }
 
 bool operator==(OffsetAmount a, OffsetAmount b) {
-    return a.magnitude == b.magnitude && a.denomination == b.denomination;
+    return a.magnitude() == b.magnitude() && a.denomination() == b.denomination();
 }
 
 bool operator!=(OffsetAmount a, OffsetAmount b) { return !(a == b); }
 
-OffsetAmount offsetFromMs(double ms) { return {ms, Denomination::Milliseconds}; }
+OffsetAmount offsetOf(double magnitude, Denomination denomination) {
+    const double bounded =
+        std::isnan(magnitude) ? 0.0
+                              : (std::max)(-kMaxConvertibleMagnitude,
+                                           (std::min)(kMaxConvertibleMagnitude, magnitude));
+    const bool named = denomination == Denomination::Milliseconds
+                       || denomination == Denomination::Beats;
+    return OffsetAmount(bounded, named ? denomination : Denomination::Milliseconds);
+}
 
-OffsetAmount offsetFromBeats(double beats) { return {beats, Denomination::Beats}; }
+OffsetAmount offsetFromMs(double ms) { return offsetOf(ms, Denomination::Milliseconds); }
 
-// All three readers switch on Denomination with the same default (Milliseconds, the
-// struct's own default value) so a corrupt persisted record reads identically everywhere —
-// a popup and a bake must never disagree on an out-of-range denomination byte.
+OffsetAmount offsetFromBeats(double beats) { return offsetOf(beats, Denomination::Beats); }
+
+// Milliseconds is pinned AFTER the switch rather than by a `default:` inside it, so the
+// switch stays exhaustive over the enum and a third denomination trips switch-exhaustiveness
+// diagnostics here instead of silently resolving as ms in all three. Those diagnostics are
+// off at this project's warning level, so read it as a signpost — the tests are the gate.
 double offsetMs(OffsetAmount amount, Tempo tempo) {
-    switch (amount.denomination) {
-        case Denomination::Beats: return tempo.beatsToMs(amount.magnitude);
-        case Denomination::Milliseconds:
-        default: return amount.magnitude;
+    switch (amount.denomination()) {
+        case Denomination::Beats: return tempo.beatsToMs(amount.magnitude());
+        case Denomination::Milliseconds: break;
     }
+    return amount.magnitude();
 }
 
 double offsetBeats(OffsetAmount amount, Tempo tempo) {
-    switch (amount.denomination) {
-        case Denomination::Beats: return amount.magnitude;
-        case Denomination::Milliseconds:
-        default: return tempo.msToBeats(amount.magnitude);
+    switch (amount.denomination()) {
+        case Denomination::Beats: return amount.magnitude();
+        case Denomination::Milliseconds: break;
     }
+    return tempo.msToBeats(amount.magnitude());
 }
 
 double offsetSeconds(OffsetAmount amount, Tempo tempo) {
-    switch (amount.denomination) {
-        case Denomination::Beats: return tempo.beatsToSeconds(amount.magnitude);
-        case Denomination::Milliseconds:
-        default: return msToSeconds(amount.magnitude);
+    switch (amount.denomination()) {
+        case Denomination::Beats: return tempo.beatsToSeconds(amount.magnitude());
+        case Denomination::Milliseconds: break;
     }
+    return msToSeconds(amount.magnitude());
 }
 
 OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) {
-    if (amount.denomination == to) return amount;
-    return to == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo))
-                                     : offsetFromMs(offsetMs(amount, tempo));
+    // Route the requested target through the same door a stored denomination goes through,
+    // so an out-of-enum target lands where a corrupt stored one does.
+    const Denomination target = offsetOf(0.0, to).denomination();
+    if (amount.denomination() == target) return amount;
+    return target == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo))
+                                         : offsetFromMs(offsetMs(amount, tempo));
 }
 
 OffsetAmount withMsView(OffsetAmount amount, double ms, Tempo tempo) {
-    return amount.denomination == Denomination::Milliseconds
-               ? offsetFromMs(ms)
-               : offsetFromBeats(tempo.msToBeats(ms));
+    return amount.denomination() == Denomination::Beats ? offsetFromBeats(tempo.msToBeats(ms))
+                                                        : offsetFromMs(ms);
 }
 
 OffsetAmount withBeatsView(OffsetAmount amount, double beats, Tempo tempo) {
-    return amount.denomination == Denomination::Beats
+    return amount.denomination() == Denomination::Beats
                ? offsetFromBeats(beats)
                : offsetFromMs(tempo.beatsToMs(beats));
 }
@@ -83,7 +97,6 @@ ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo) {
     const double rawEndSeconds = out.noteOffSeconds + offsetSeconds(program.end.amount(), tempo);
     // An inverted window has no meaning to a renderer, so a far-negative end offset yields a
     // zero-length capture the caller can reject rather than a negative one it cannot.
-    // windowCollapsed distinguishes that from a genuinely zero-length program.
     out.windowCollapsed = rawEndSeconds < out.captureStartSeconds;
     out.captureEndSeconds = (std::max)(rawEndSeconds, out.captureStartSeconds);
     out.velocity = program.velocity.value();

src/core/instrument/note/note_program.h

@@ -13,6 +13,11 @@
 
 namespace reasampler::instrument::note {
 
+// The ladder and the offsets both feed the tempo conversions, so both must sit inside the
+// domain fromBpm validates — checked here because this is the one file that composes them.
+static_assert(kMaxDivisionBeats <= kMaxConvertibleMagnitude,
+              "the note-length ladder must stay inside the tempo conversions' domain");
+
 class Velocity {
 public:
     static constexpr int kMin = 1;  // 0 is note-off in MIDI; a programmed note must sound
@@ -21,7 +26,7 @@ public:
     Velocity() = default;
     static Velocity of(int value);  // clamped into [kMin, kMax]
 
-    std::uint8_t value() const { return value_; }
+    constexpr std::uint8_t value() const { return value_; }
 
 private:
     std::uint8_t value_ = 100;
@@ -31,20 +36,42 @@ bool operator==(Velocity a, Velocity b);
 
 enum class Denomination : std::uint8_t { Milliseconds, Beats };
 
+class OffsetAmount;
+
+// The one door. Normalizes both fields so nothing downstream has to: a magnitude past
+// +/-kMaxConvertibleMagnitude clamps to it, a NaN magnitude — which names no value to clamp
+// toward — becomes zero, and a denomination outside the enum becomes Milliseconds, the
+// field's own default. A corrupt persisted record therefore resolves to a plausible offset
+// rather than an unrepresentable one, and no two readers can disagree about which.
+OffsetAmount offsetOf(double magnitude, Denomination denomination);
+OffsetAmount offsetFromMs(double ms);
+OffsetAmount offsetFromBeats(double beats);
+
 // One magnitude, in the denomination it was entered in; the other view is derived on demand
 // and never stored. Which one was entered is itself the intent: a beats offset must follow a
 // tempo change and a ms offset must hold still, and only a stored denomination says which.
-struct OffsetAmount {
-    double magnitude = 0.0;
-    Denomination denomination = Denomination::Milliseconds;
+class OffsetAmount {
+public:
+    OffsetAmount() = default;
+
+    constexpr double magnitude() const { return magnitude_; }
+    constexpr Denomination denomination() const { return denomination_; }
+
+private:
+    OffsetAmount(double magnitude, Denomination denomination)
+        : magnitude_(magnitude), denomination_(denomination) {}
+    friend OffsetAmount offsetOf(double magnitude, Denomination denomination);
+
+    double magnitude_ = 0.0;
+    Denomination denomination_ = Denomination::Milliseconds;
 };
 
+static_assert(!std::is_constructible_v<OffsetAmount, double, Denomination>,
+              "offsetOf must be the only way to give an OffsetAmount a value");
+
 bool operator==(OffsetAmount a, OffsetAmount b);
 bool operator!=(OffsetAmount a, OffsetAmount b);
 
-OffsetAmount offsetFromMs(double ms);
-OffsetAmount offsetFromBeats(double beats);
-
 double offsetMs(OffsetAmount amount, Tempo tempo);
 double offsetBeats(OffsetAmount amount, Tempo tempo);
 double offsetSeconds(OffsetAmount amount, Tempo tempo);
@@ -100,7 +127,7 @@ struct ResolvedNote {
     double noteOffSeconds = 0.0;       // == the note's sounding length, note-on being 0
     double captureStartSeconds = 0.0;  // negative when the capture opens before the note
     double captureEndSeconds = 0.0;
-    std::uint8_t velocity = 100;  // resolveNote always overwrites this; matches Velocity's own default
+    std::uint8_t velocity = Velocity{}.value();  // resolveNote always overwrites this
     // True when the programmed end offset inverted the window and resolveNote collapsed it
     // to zero length instead — lets a popup explain an empty window rather than just show one.
     bool windowCollapsed = false;
@@ -108,6 +135,8 @@ struct ResolvedNote {
     double captureLengthSeconds() const { return captureEndSeconds - captureStartSeconds; }
 };
 
+// Total: every field of the result is finite for every constructible program and tempo,
+// which is why there is no failure path here. See this directory's CLAUDE.md.
 ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo);
 
 }  // namespace reasampler::instrument::note

src/core/instrument/note/tempo.cpp

@@ -11,11 +11,15 @@ constexpr double kSecondsPerMinute = 60.0;
 
 std::optional<Tempo> Tempo::fromBpm(double beatsPerMinute) {
     if (!std::isfinite(beatsPerMinute) || beatsPerMinute <= 0.0) return std::nullopt;
-    // A subnormal BPM is finite and positive but overflows 60/bpm to +inf, which then turns
-    // any beatsToSeconds(0) into NaN downstream — reject it here so every conversion below
-    // stays total.
-    if (!std::isfinite(kSecondsPerMinute / beatsPerMinute)) return std::nullopt;
-    return Tempo(beatsPerMinute);
+    // Guard by running the conversions, not by testing the 60/bpm reciprocal they start
+    // from: that reciprocal stays finite for BPMs whose beatsToMs has already overflowed,
+    // because the conversions scale it by up to kMaxConvertibleMagnitude. Both directions
+    // are checked — one overflows at an absurdly slow tempo, the other at an absurdly fast
+    // one. Calling them here is what keeps the guard from drifting away from what they do.
+    const Tempo candidate(beatsPerMinute);
+    if (!std::isfinite(candidate.beatsToMs(kMaxConvertibleMagnitude))) return std::nullopt;
+    if (!std::isfinite(candidate.msToBeats(kMaxConvertibleMagnitude))) return std::nullopt;
+    return candidate;
 }
 
 double Tempo::secondsPerBeat() const { return kSecondsPerMinute / bpm_; }

src/core/instrument/note/tempo.h

@@ -16,10 +16,16 @@ inline constexpr double kMsPerSecond = 1000.0;
 constexpr double msToSeconds(double ms) { return ms / kMsPerSecond; }
 constexpr double secondsToMs(double seconds) { return seconds * kMsPerSecond; }
 
+// The largest magnitude, in beats or in milliseconds, the conversions below are required to
+// keep finite. `fromBpm` validates against it and every caller caps its own domain to it, so
+// the two halves of the totality claim meet at one number. Astronomically above anything
+// musical — a billion milliseconds is eleven days — so nothing real is excluded.
+inline constexpr double kMaxConvertibleMagnitude = 1e9;
+
 class Tempo {
 public:
-    // The only place a bad BPM is rejected, which is what lets every conversion below be
-    // total — no resolver downstream needs a failure path.
+    // Rejects rather than clamps, alone among this module's doors: an unusable BPM has no
+    // nearest usable one to fall to. See this directory's CLAUDE.md for the rule.
     static std::optional<Tempo> fromBpm(double beatsPerMinute);
 
     double bpm() const { return bpm_; }