Merge ps-w1-t2-sample-intrinsics: S2 Sample seam fields (rootNote + loop points)

src/bank_model.cpp

@@ -35,6 +35,10 @@ bool Levels::operator==(const Levels& o) const {
     return peakDb == o.peakDb && rmsDb == o.rmsDb && lufs == o.lufs;
 }
 
+bool LoopPoints::operator==(const LoopPoints& o) const {
+    return start == o.start && end == o.end;
+}
+
 bool Sample::operator==(const Sample& o) const {
     return id == o.id && displayName == o.displayName && relativePath == o.relativePath &&
            sourceMode == o.sourceMode && sourceRange == o.sourceRange &&
@@ -43,7 +47,8 @@ bool Sample::operator==(const Sample& o) const {
            lengthSeconds == o.lengthSeconds && lengthBeats == o.lengthBeats &&
            captureTempo == o.captureTempo &&
            captureTimeSigNum == o.captureTimeSigNum &&
-           captureTimeSigDenom == o.captureTimeSigDenom && key == o.key && levels == o.levels &&
+           captureTimeSigDenom == o.captureTimeSigDenom && key == o.key &&
+           rootNote == o.rootNote && loop == o.loop && levels == o.levels &&
            clipped == o.clipped && tier == o.tier && contentHash == o.contentHash &&
            provenance == o.provenance && createdTimestamp == o.createdTimestamp;
 }
@@ -253,6 +258,21 @@ void writeSample(std::string& out, const Sample& s) {
     w.keyBegin("key");
     if (s.key) writeEscaped(out, *s.key); else out += "null";
 
+    // Phase S seam fields (D-B). Emitted as null when absent (same shape as `key`
+    // and `provenance`) so pre-Phase-S JSON — which lacks these keys entirely —
+    // parses to empty optionals and re-serializes without invention.
+    w.keyBegin("rootNote");
+    if (s.rootNote) out += numToStr(*s.rootNote); else out += "null";
+
+    w.keyBegin("loop");
+    if (s.loop) {
+        ObjWriter lp(out);
+        lp.keyRaw("start", numToStr(s.loop->start));
+        lp.keyRaw("end", numToStr(s.loop->end));
+    } else {
+        out += "null";
+    }
+
     w.keyBegin("levels");
     {
         ObjWriter l(out);
@@ -608,6 +628,41 @@ bool Parser::parseSample(Sample& s) {
                 if (!parseString(k)) return false;
                 s.key = k;
             }
+        } else if (key == "rootNote") {
+            bool wasNull = false;
+            if (!expectNullOr(wasNull)) return false;
+            if (wasNull) {
+                s.rootNote.reset();
+            } else {
+                int v = 0;
+                if (!parseInt(v)) return false;
+                // Valid MIDI note range: 0..127 inclusive (boundaries valid).
+                if (v < 0 || v > 127) return false;
+                s.rootNote = v;
+            }
+        } else if (key == "loop") {
+            bool wasNull = false;
+            if (!expectNullOr(wasNull)) return false;
+            if (wasNull) {
+                s.loop.reset();
+            } else {
+                if (!consume('{')) return false;
+                LoopPoints lp;
+                do {
+                    std::string lk;
+                    if (!parseKey(lk)) return false;
+                    std::int64_t lv = 0;
+                    if (!parseInt64(lv)) return false;
+                    if (lk == "start") lp.start = lv;
+                    else if (lk == "end") lp.end = lv;
+                } while (consume(','));
+                if (!consume('}')) return false;
+                // Invariant: 0 <= start <= end. start == end is a valid zero-length
+                // marker; a negative index or start > end is malformed, not silently
+                // clamped (mirrors the enum-range rejection above).
+                if (lp.start < 0 || lp.end < lp.start) return false;
+                s.loop = lp;
+            }
         } else if (key == "levels") {
             if (!consume('{')) return false;
             do {

src/bank_model.h

@@ -63,6 +63,22 @@ struct Levels {
     bool operator==(const Levels& o) const;
 };
 
+// Sample-accurate sustain-loop bounds, as frame indices into the captured file
+// (Phase S seam field, D-B). A bank intrinsic — a fact about the file, like
+// sampleRate or length — consumed by the future MIDI-playback instrument to hold
+// notes past the recorded length. Modeled as one optional struct (not two loose
+// optionals) so "both points or neither" is a structural invariant, not a rule to
+// re-check at every boundary. Frame indices, not seconds, because the loop is a
+// per-sample-frame contract; the instrument reads the file's sample rate to relate
+// them to time. Invariant (enforced at the deserialize boundary): 0 <= start <= end.
+// start == end is a valid zero-length loop marker.
+struct LoopPoints {
+    std::int64_t start = 0;
+    std::int64_t end   = 0;
+
+    bool operator==(const LoopPoints& o) const;
+};
+
 // The metadata record for one captured sample. The audio itself lives in a
 // project-relative file; `relativePath` is ALWAYS relative (enforced at the
 // BankIndex::add boundary — see AddResult).
@@ -95,6 +111,18 @@ struct Sample {
 
     std::optional<std::string> key;  // musical key, when known
 
+    // Phase S seam fields (D-B) — bank intrinsics for the MIDI-playback instrument,
+    // additive like `provenance` (M1). Both default cleanly empty: pre-Phase-S
+    // samples deserialize without them and re-serialize without inventing values.
+    //  - rootNote: MIDI note (0..127) the sample was recorded at, so the instrument
+    //    can repitch it across the keyboard. DISTINCT from the musical `key` above:
+    //    `key` is a human label ("F#m"); `rootNote` is the exact pitch for repitch.
+    //    Populated at/after capture only where derivable — left empty (never guessed)
+    //    when the source is not a single played note.
+    //  - loop: sustain-loop bounds, populated only where explicitly set.
+    std::optional<int>        rootNote;
+    std::optional<LoopPoints> loop;
+
     Levels levels;
     bool   clipped = false;
 

src/capture.cpp

@@ -530,6 +530,11 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) {
         }
     }
     s.createdTimestamp = static_cast<std::int64_t>(std::time(nullptr));
+    // Phase S seam fields (rootNote / loop) left empty (D-B). An offline render of a
+    // master mix / track / time-selection is not a single played note, so no root
+    // note is derivable here — we do NOT guess one. Loop points are set later by an
+    // explicit user action, not at capture. Leaving them empty is the honest default;
+    // the instrument (Phase S) treats an absent root note as "not a pitched sample".
 
     result.status  = CaptureStatus::Ok;
     result.sample  = s;

src/realtime_record.cpp

@@ -58,6 +58,9 @@ Sample sampleFromRecordedCapture(const RecordedCapture& cap) {
     // finalized and on disk — the hash is over the finished file bytes. Left empty
     // here because sampleFromRecordedCapture runs before the file exists (the
     // mapping is pure / DAW-free); the shell patches it in after the move+trim.
+    // Phase S seam fields (rootNote / loop) left empty (D-B) — same reasoning as the
+    // offline path: a realtime record of wet output is not a single played note, so
+    // no root note is derivable; loop points are set by a later explicit action.
     s.createdTimestamp = cap.createdTimestamp;
     return s;
 }

tests/test_bank_model.cpp

@@ -36,6 +36,8 @@ static Sample fullSample(const std::string& seed) {
     s.captureTimeSigNum = 6;    // L7 F1 meter stamp (non-4/4 to prove it round-trips)
     s.captureTimeSigDenom = 8;
     s.key = "F#m";
+    s.rootNote = 60;                 // Phase S seam field (present)
+    s.loop = LoopPoints{4096, 65536}; // Phase S seam field (present)
     s.levels = {-0.3, -12.7, -14.2};
     s.clipped = true;
     s.tier = Tier::Archive;
@@ -86,11 +88,18 @@ static void testFullFieldRoundTrip() {
         CHECK(minMeter && minMeter->captureTimeSigNum == 0 && minMeter->captureTimeSigDenom == 0);
         CHECK(full && full->provenance.has_value());
         CHECK(full && full->provenance->fxChainSnapshot == "<FXCHAIN\n  BYPASS 0 0 0\n>");
+        // Phase S seam fields survive round-trip exactly.
+        CHECK(full && full->rootNote.has_value() && *full->rootNote == 60);
+        CHECK(full && full->loop.has_value());
+        CHECK(full && full->loop && full->loop->start == 4096 && full->loop->end == 65536);
 
         const Sample* min = back->query("min-b");
         CHECK(min && !min->key.has_value());
         CHECK(min && !min->provenance.has_value());
         CHECK(min && min->trackGuids.empty());
+        // Seam fields absent on the minimal sample and stay absent.
+        CHECK(min && !min->rootNote.has_value());
+        CHECK(min && !min->loop.has_value());
     }
 }
 
@@ -417,6 +426,128 @@ static void testEnumRangeValidation() {
     CHECK(!r2.has_value());
 }
 
+// S2 test case 2: a legacy Sample JSON — written before the Phase S seam fields
+// existed, so it has NO "rootNote" or "loop" keys at all — parses to clean empty
+// optionals (no loss, no migration) and re-serializes without inventing values.
+// (The parser's forward-compat unknown-key skipping is what makes the reverse case
+// — new keys ignored by an old parser — safe too; here we test old-JSON→new-parser.)
+static void testLegacyJsonDefaults() {
+    const char* legacy =
+        "{\"samples\":[{\"id\":\"leg1\",\"relativePath\":\"bank/leg.wav\","
+        "\"displayName\":\"legacy\","
+        "\"sourceMode\":0,\"sourceRange\":{\"startSeconds\":1.5,\"endSeconds\":2.5,"
+        "\"startPpq\":0.0,\"endPpq\":0.0},\"trackGuids\":[],"
+        "\"wetDry\":1.0,\"channelCount\":2,\"sampleRate\":48000,"
+        "\"lengthSeconds\":1.0,\"lengthBeats\":0.0,\"captureTempo\":120.0,"
+        "\"key\":null,\"levels\":{\"peakDb\":0.0,\"rmsDb\":0.0,\"lufs\":0.0},"
+        "\"clipped\":false,\"tier\":0,\"contentHash\":\"h-leg1\","
+        "\"provenance\":null,\"createdTimestamp\":0}]}";
+    auto r = BankIndex::deserialize(legacy);
+    CHECK(r.has_value());
+    if (r) {
+        const Sample* s = r->query("leg1");
+        CHECK(s != nullptr);
+        CHECK(s && !s->rootNote.has_value());  // clean default, not a guessed value
+        CHECK(s && !s->loop.has_value());
+
+        // Re-serialize is lossless: parsing it again yields an equal index. This
+        // proves the absent fields did not silently gain values on the way out.
+        std::string out = r->serialize();
+        auto again = BankIndex::deserialize(out);
+        CHECK(again.has_value());
+        CHECK(again && *again == *r);
+        if (again) {
+            const Sample* s2 = again->query("leg1");
+            CHECK(s2 && !s2->rootNote.has_value());
+            CHECK(s2 && !s2->loop.has_value());
+        }
+    }
+}
+
+// S2 test case 4: boundary values for the seam fields are representable and
+// round-trip. rootNote 0 and 127 (the MIDI edges); loopStart == loopEnd (a valid
+// zero-length marker); a loop whose end sits at the file's last frame. Also asserts
+// the deserialize-boundary validation rules reject out-of-range input rather than
+// storing a bogus value.
+static void testSeamFieldBoundaries() {
+    // rootNote at both MIDI edges + equal-and-end-anchored loop points round-trip.
+    BankIndex idx;
+    Sample lo = minimalSample("lo"); lo.contentHash = "h-lo";
+    lo.rootNote = 0;
+    lo.loop = LoopPoints{0, 0};            // zero-length marker at frame 0
+    Sample hi = minimalSample("hi"); hi.contentHash = "h-hi";
+    hi.rootNote = 127;
+    hi.loop = LoopPoints{100, 100};        // start == end elsewhere
+    Sample end = minimalSample("end"); end.contentHash = "h-end";
+    end.loop = LoopPoints{0, 9223372036854775807LL};  // end at max int64 frame
+    CHECK(idx.add(lo) == AddResult::Added);
+    CHECK(idx.add(hi) == AddResult::Added);
+    CHECK(idx.add(end) == AddResult::Added);
+
+    auto back = BankIndex::deserialize(idx.serialize());
+    CHECK(back.has_value());
+    CHECK(back && *back == idx);
+    if (back) {
+        CHECK(back->query("min-lo")->rootNote == 0);
+        CHECK(back->query("min-hi")->rootNote == 127);
+        // Named local: a brace-init with a comma inside CHECK(...) would be parsed
+        // as two macro arguments by the preprocessor.
+        const LoopPoints zeroLen{0, 0};
+        CHECK(back->query("min-lo")->loop == zeroLen);
+        CHECK(back->query("min-end")->loop->end == 9223372036854775807LL);
+    }
+
+    // Validation rule (chosen for this design, surfaced in the handoff):
+    //   rootNote must be 0..127; loop must satisfy 0 <= start <= end.
+    // Out-of-range input is rejected at the deserialize boundary (nullopt), mirroring
+    // the existing enum-range and integer-overflow rejections — never clamped.
+    const char* head =
+        "{\"samples\":[{\"id\":\"bad\",\"relativePath\":\"bank/b.wav\","
+        "\"displayName\":\"\",\"sourceMode\":0,"
+        "\"sourceRange\":{\"startSeconds\":0.0,\"endSeconds\":0.0,"
+        "\"startPpq\":0.0,\"endPpq\":0.0},\"trackGuids\":[],"
+        "\"wetDry\":1.0,\"channelCount\":1,\"sampleRate\":44100,"
+        "\"lengthSeconds\":0.0,\"lengthBeats\":0.0,\"captureTempo\":0.0,"
+        "\"key\":null,";
+    const char* tail =
+        "\"levels\":{\"peakDb\":0.0,\"rmsDb\":0.0,\"lufs\":0.0},"
+        "\"clipped\":false,\"tier\":0,\"contentHash\":\"h-bad\","
+        "\"provenance\":null,\"createdTimestamp\":0}]}";
+
+    CHECK(!BankIndex::deserialize(std::string(head) + "\"rootNote\":128," + tail).has_value());
+    CHECK(!BankIndex::deserialize(std::string(head) + "\"rootNote\":-1," + tail).has_value());
+    CHECK(!BankIndex::deserialize(
+        std::string(head) + "\"loop\":{\"start\":10,\"end\":5}," + tail).has_value());  // start > end
+    CHECK(!BankIndex::deserialize(
+        std::string(head) + "\"loop\":{\"start\":-1,\"end\":5}," + tail).has_value());  // negative start
+}
+
+// S2 test case 3: the seam-field addition is purely additive — dedup-by-hash, tier
+// moves/filtering, and BankIndex ordering are byte-for-byte unchanged by the
+// presence (or absence) of rootNote/loop. Two samples differing ONLY in seam fields
+// but sharing a content hash still collapse; a seam-populated sample tiers exactly
+// like any other.
+static void testSeamFieldsAdditiveInvariant() {
+    BankIndex idx;
+    Sample a = fullSample("z");   // has rootNote + loop populated
+    CHECK(idx.add(a) == AddResult::Added);
+
+    // Same hash, seam fields cleared — dedup keys off contentHash only, so this
+    // still collapses. Seam fields do NOT enter the dedup identity.
+    Sample dup = fullSample("z2");
+    dup.contentHash = a.contentHash;
+    dup.rootNote.reset();
+    dup.loop.reset();
+    CHECK(idx.add(dup) == AddResult::Collapsed);
+    CHECK(idx.size() == 1);
+
+    // Tier move on a seam-populated sample behaves exactly as before.
+    CHECK(idx.query("id-z")->tier == Tier::Archive);
+    CHECK(idx.moveTier("id-z", Tier::Scratch));
+    CHECK(idx.query("id-z")->tier == Tier::Scratch);
+    CHECK(idx.query("id-z")->rootNote == 60);  // move did not disturb seam fields
+}
+
 int main() {
     testFullFieldRoundTrip();
     testDedupByHash();
@@ -430,6 +561,9 @@ int main() {
     testUnicodeEscapeDecoding();
     testIntegerOverflow();
     testEnumRangeValidation();
+    testLegacyJsonDefaults();
+    testSeamFieldBoundaries();
+    testSeamFieldsAdditiveInvariant();
 
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;