instrument: fix AHD DecayEnd overlay/grab defect, pin flaky curve test, close comment/doc findings

src/core/instrument/engine/voice.cpp

@@ -95,6 +95,9 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick
     // baseRatio_ source frames per output frame — needs the span converted, or a transposed
     // note's envelope outruns (or outlives) the note it shapes. Preserve reads at the source
     // rate, so its two domains already coincide.
+    // Divides by baseRatio_ alone, though the actual Varispeed read rate is baseRatio_ x
+    // envFactor — a deep pitch envelope makes this a first-order approximation, not exact.
+    // Strictly better than the un-converted source-frame span it replaced.
     const double pitchSpan =
         (pitchEngine_ == PitchEngine::Preserve || !(baseRatio_ > 0.0))
             ? static_cast<double>(postStart)
@@ -268,6 +271,9 @@ void Voice::retune(int note) {
     // legato phrase is one gesture, one strike (classic mono-synth behavior).
     if (!active_ || sample_ == nullptr) return;
     note_ = note;
+    // Changes baseRatio_ without re-converting pitchEnv_'s already-configured span (the
+    // baseRatio_ division in the note-on setup above), so a slide leaves that envelope on the
+    // first note's domain — consistent with "touch nothing else," but the drift lives here.
     baseRatio_ = keyTrackedRatio(note, sample_->rootNote, sample_->keyTrack);
     // Filter key-tracking follows the pitch: it is a function of the note, so a slide moves it
     // too. The velocity offset deliberately stays the first note's, matching velocityGain_.

src/core/instrument/engine/voice.h

@@ -6,11 +6,6 @@
 // configured an out-of-line render would put a call — and the envelope ticks behind it —
 // across a TU boundary on the hottest path in the program. The per-NOTE half (start /
 // retune / release / hardStop / presize) is cold enough to live in voice.cpp.
-//
-// DOCUMENTED ~600-line-ceiling EXCEPTION (root CLAUDE.md structural heuristic 1): this file
-// is over the ceiling because of the constraint above, not silent overshoot. A responsibility
-// seam here would move part of advanceFrame's inline body out of this header, reintroducing
-// the cross-TU call the header-inlining exists to avoid — worse than the overshoot.
 
 #include <cmath>
 #include <cstdint>

src/core/instrument/ui/CMakeLists.txt

@@ -51,7 +51,7 @@ reasampler_test(knob_deck LINK knob_deck)
 # MorphLaw) for the v9 filter tail -- plain value types, no filter symbol linked.
 reasampler_pure_library(deck_groups
     SOURCES deck_groups.cpp
-    LINK PUBLIC knob_deck velocity_curve peaks)
+    LINK PUBLIC knob_deck velocity_curve peaks curve_law)
 # sample_bands is linked directly for the test only: the deck-fits-the-floor-window assertion
 # needs the band allocator deck_groups itself has no reason to depend on.
 reasampler_test(deck_groups LINK deck_groups sample_bands)

src/core/instrument/ui/envelope_edit.cpp

@@ -108,11 +108,29 @@ double curveFromKnotDrag(const StageEnvelope& grabEnv, EnvNode knot, double grab
     return curveFromMidLevel((newLevel - seg.start) / span);
 }
 
+// An AHD's DecayEnd moves decaySeconds via X, scaled by 1/(1 - holdFraction) — see
+// resolveNodeDrag's DecayEnd case. At holdFraction == 1.0 that derivative is exactly 0, so a
+// drag there can never change anything; when it ALSO coincides with HoldEnd (decay ~ 0) it is a
+// dead handle sitting on top of a live one. Excluded from the grabbable set in that exact case
+// only — a functional DecayEnd (holdFraction < 1) stays grabbable even when it coincides.
+bool ahdDecayEndIsDead(const StageEnvelope& env, const std::vector<EnvVertex>& poly) {
+    if (env.kind != EnvKind::Ahd) return false;
+    if (1.0 - clamp01(env.holdFraction) > 1e-9) return false;
+    EnvVertex hold, decay;
+    bool haveHold = false, haveDecay = false;
+    for (const EnvVertex& v : poly) {
+        if (v.node == EnvNode::HoldEnd) { hold = v; haveHold = true; }
+        else if (v.node == EnvNode::DecayEnd) { decay = v; haveDecay = true; }
+    }
+    return haveHold && haveDecay && hold.x == decay.x;
+}
+
 }  // namespace
 
 NodeHit nodeAtPoint(const StageEnvelope& env, const OverlayArea& area, double totalSeconds,
                     int x, int y) {
     const std::vector<EnvVertex> poly = buildEnvelopePolyline(env, area, totalSeconds);
+    const bool dropDeadDecayEnd = ahdDecayEndIsDead(env, poly);
     // Nearest draggable, kind-matching node within the pick radius wins (Chebyshev distance);
     // ties go to the earlier draw-order node. Knots are appended last, so a knot coincident
     // with an endpoint handle loses — a drag there stays a time edit.
@@ -120,6 +138,7 @@ NodeHit nodeAtPoint(const StageEnvelope& env, const OverlayArea& area, double to
     int bestDist = kNodeGrabRadius + 1;
     for (const EnvVertex& v : poly) {
         if (!isDraggable(v.node) || !nodeInKind(v.node, env.kind)) continue;
+        if (dropDeadDecayEnd && v.node == EnvNode::DecayEnd) continue;
         const int dist = std::max(std::abs(x - v.x), std::abs(y - v.y));
         if (dist < bestDist) {  // strict-less-than keeps ties at the earlier draw order
             bestDist = dist;

src/core/instrument/ui/envelope_edit.h

@@ -37,7 +37,9 @@ struct EnvClampBounds {
 // inputs buildEnvelopePolyline drew from). `hit` is false for a point off every draggable node.
 // Nearest node within the radius wins (Chebyshev distance); an exact tie goes to the earlier
 // draw-order node, and since knots are appended last, a coincident endpoint handle wins over a
-// knot rather than the drag silently becoming a curve edit.
+// knot rather than the drag silently becoming a curve edit. An AHD's DecayEnd is excluded
+// entirely when it coincides with HoldEnd AND holdFraction == 1.0 — there it cannot move
+// (resolveNodeDrag's derivative is 0), so it is dropped rather than won by draw order.
 struct NodeHit {
     bool    hit  = false;
     EnvNode node = EnvNode::Origin;  // meaningful only when hit == true

src/core/instrument/ui/envelope_overlay.cpp

@@ -177,19 +177,14 @@ std::vector<EnvVertex> ahdPolyline(const StageEnvelope& env, const Rect& area,
     pts.reserve(6);
     pts.push_back(vtx(EnvNode::Origin,    area, totalSeconds, t0, 0.0));
     pts.push_back(vtx(EnvNode::AttackEnd, area, totalSeconds, t0 + s.attack, 1.0));
-    EnvVertex holdVtx = vtx(EnvNode::HoldEnd, area, totalSeconds, t0 + s.attack + s.hold, 1.0);
+    pts.push_back(vtx(EnvNode::HoldEnd,   area, totalSeconds, t0 + s.attack + s.hold, 1.0));
-    EnvVertex decayVtx = vtx(EnvNode::DecayEnd, area, totalSeconds, t0 + s.total, 0.0);
+    // DecayEnd is drawn at t0 + total, which coincides with HoldEnd exactly when decay ~ 0 —
-    // A hold that consumes the WHOLE post-attack/decay remainder (holdFraction == 1.0, the
+    // independent of holdFraction (total = attack + hold + decay always). Left at its true
-    // Trigger AHD default) puts HoldEnd and DecayEnd on the same wall-clock instant, and
+    // instant rather than nudged: the 1:1 axis this policy exists to keep honest must hold even
-    // nodeAtPoint's earlier-draw-order tie-break then hides DecayEnd behind HoldEnd forever.
+    // at a shared instant, including the Trigger default's abrupt (zero-decay) cutoff.
-    // Nudge apart, clamped to the canvas — the same minimum-separation rationale
+    // envelope_edit's nodeAtPoint handles the coincidence instead, by dropping DecayEnd from the
-    // kGateNodeSepPx exists for on the AHDSR schematic, applied to this coincidence instead.
+    // grabbable set when it also cannot move (holdFraction == 1.0).
-    if (decayVtx.x - holdVtx.x < kGateNodeSepPx) {
+    pts.push_back(vtx(EnvNode::DecayEnd,  area, totalSeconds, t0 + s.total, 0.0));
-        const int right = area.x + std::max(1, area.width) - 1;
-        decayVtx.x = std::min(holdVtx.x + kGateNodeSepPx, right);
-    }
-    pts.push_back(holdVtx);
-    pts.push_back(decayVtx);
     if (s.attack > 0.0) {
         pts.push_back(knotVtx(EnvNode::AttackCurve, area, pts[0].x, pts[1].x, 0.0, 1.0,
                               env.attackCurve));

tests/test_curve_law.cpp

@@ -156,8 +156,11 @@ static void testKnobLawRoundTripsOutsideTheDetent() {
         const double back = curveFromKnobNorm(knobNormFromCurve(e));
         CHECK(std::fabs(back - e) < 1e-9);
     }
-    CHECK(curveFromKnobNorm(0.0) == kCurveMin);
+    // exp(-log(10)) is not guaranteed bit-exact to kCurveMin's literal 0.1 (1-2 ulp either way);
-    CHECK(curveFromKnobNorm(-3.0) == kCurveMin);   // out-of-range norm saturates
+    // both norms below collapse to the same t == 0.0 computation, so both get the same
+    // tolerance rather than leaning on clampCurve's floor to land on it by luck.
+    CHECK(std::fabs(curveFromKnobNorm(0.0) - kCurveMin) < 1e-12);
+    CHECK(std::fabs(curveFromKnobNorm(-3.0) - kCurveMin) < 1e-12);  // out-of-range norm saturates
     CHECK(std::fabs(curveFromKnobNorm(1.0) - kCurveMax) < 1e-12);
     // The ENDS need only land on the domain, not on an exact norm — 0.1 is not exactly 1/10 in
     // binary, so log(kCurveMin) is a hair off -log(kCurveMax). Only the centre carries an

tests/test_envelope_edit.cpp

@@ -4,7 +4,8 @@
 // pixel delta produces exactly the param a knob would have.
 //
 // Covers: nodeAtPoint (every drawn handle grabbable, the anchored ReleaseEnd and the Origin
-// never grabbed, other-kind nodes rejected, misses outside the radius); resolveNodeDrag
+// never grabbed, other-kind nodes rejected, misses outside the radius, a dead coincident AHD
+// DecayEnd excluded while a functional one stays grabbable); resolveNodeDrag
 // (AHDSR stage times at the schematic scale, the sustain level on Y, the release dragged from
 // its START with the inverted sign, the caller's clamp domain, AHD stage times at the 1:1
 // scale, the hold FRACTION); curve-knot drags (the exponent domain, its endpoints, and the
@@ -47,6 +48,18 @@ static StageEnvelope ahdsrEnv() {
     return e;
 }
 
+// F1's coincidence cases need attack/decay/fraction/span combinations ahdEnv() doesn't cover.
+static StageEnvelope ahd(double a, double d, double frac, double origin, double span) {
+    StageEnvelope e;
+    e.kind = EnvKind::Ahd;
+    e.attackSeconds = a;
+    e.decaySeconds = d;
+    e.holdFraction = frac;
+    e.originSeconds = origin;
+    e.spanSeconds = span;
+    return e;
+}
+
 static StageEnvelope ahdEnv() {
     StageEnvelope e;
     e.kind = EnvKind::Ahd;
@@ -117,6 +130,26 @@ static void testAhdHasNoSustainNodes() {
     CHECK(out.attackSeconds == e.attackSeconds);
 }
 
+// F1: at the Trigger default (decay 0, holdFraction 1.0) DecayEnd sits on HoldEnd's own instant
+// AND cannot move there (resolveNodeDrag's decay branch has derivative 0 — see the denom guard).
+// A grab at its true (now un-nudged) position must miss rather than resolve to a dead handle;
+// HoldEnd, the live node underneath, stays fully grabbable.
+static void testDeadCoincidentDecayEndIsNotGrabbable() {
+    const StageEnvelope e = ahd(0.5, 0.0, 1.0, 0.0, 3.0);
+    CHECK(!grabAt(e, EnvNode::DecayEnd).hit);
+    CHECK(grabAt(e, EnvNode::HoldEnd).hit);
+    CHECK(grabAt(e, EnvNode::HoldEnd).node == EnvNode::HoldEnd);
+}
+
+// Coincidence alone does not drop DecayEnd — only holdFraction == 1.0 makes it truly dead. With
+// holdFraction < 1 the X-drag still moves decaySeconds (denom > 0), so it stays grabbable even
+// when decay is 0 and it starts out coincident with HoldEnd.
+static void testFunctionalCoincidentDecayEndStaysGrabbable() {
+    const StageEnvelope e = ahd(0.5, 0.0, 0.5, 0.0, 3.0);
+    CHECK(grabAt(e, EnvNode::DecayEnd).hit);
+    CHECK(grabAt(e, EnvNode::DecayEnd).node == EnvNode::DecayEnd);
+}
+
 static void testMissOutsideTheRadius() {
     const StageEnvelope e = ahdsrEnv();
     const Rect a = wideArea();
@@ -310,6 +343,8 @@ int main() {
     testEveryDrawnHandleIsGrabbable();
     testAnchoredEndAndOriginAreNotGrabbable();
     testAhdHasNoSustainNodes();
+    testDeadCoincidentDecayEndIsNotGrabbable();
+    testFunctionalCoincidentDecayEndStaysGrabbable();
     testMissOutsideTheRadius();
 
     testAhdsrStageTimesTrackTheSchematicScale();

tests/test_envelope_overlay.cpp

@@ -102,9 +102,10 @@ static void testDegenerateAreaAndDuration() {
 // This is what makes a dragged handle track the cursor 1:1 (envelope_edit's own inverse reads
 // this same function) — a scale regression here is exactly what a relational-only check misses.
 static void testGatePxPerSecond() {
-    const double expected =
+    // 967 / 8 px/s, pinned as a literal — restating the formula with the same named constants
-        (1000.0 - 1.0 - 4.0 * kGateNodeSepPx) / (4.0 * kGateStageMaxSeconds);  // 967/8 px/s
+    // would let a change to kGateNodeSepPx or kGateStageMaxSeconds move both sides and pass
-    CHECK(gatePxPerSecond(wideArea()) == expected);
+    // silently.
+    CHECK(gatePxPerSecond(wideArea()) == 120.875);
     CHECK(gatePxPerSecond(Rect::ltrb(5, 5, 5, 45)) == 0.0);   // zero-width area -> 0
     CHECK(gatePxPerSecond(Rect::ltrb(0, 0, 10, 10)) > 0.0);   // tiny area: usable floors at 1px, > 0
 }
@@ -304,6 +305,36 @@ static void testAhdIsOneToOneWithTheTimeAxis() {
     CHECK(!hasNode(poly, EnvNode::ReleaseCurve));
 }
 
+// F1 regression: DecayEnd stays at its true wall-clock instant even when that instant coincides
+// with HoldEnd's (decay ~ 0) — the 1:1 AHD axis promises N seconds -> N seconds, and a nudge
+// away from that instant lies about the shape, including the Trigger default's abrupt cutoff.
+// Fails against the prior nudge, which moved DecayEnd right whenever the gap was under
+// kGateNodeSepPx.
+static void testDecayEndStaysAtItsTrueInstantEvenWhenCoincidentWithHoldEnd() {
+    const Rect a = wideArea();
+    const double total = 4.0;
+
+    // Short but nonzero decay: the true gap to HoldEnd is a few px, under kGateNodeSepPx, so
+    // the retired nudge would have fired here too.
+    const StageEnvelope shortDecay = ahd(0.5, 0.02, 1.0, 0.0, 3.0);
+    const AhdSplit sShort = splitAhdSeconds(shortDecay);
+    EnvVertex decayShort;
+    CHECK(findNode(buildEnvelopePolyline(shortDecay, overlayOf(a), total), EnvNode::DecayEnd,
+                   decayShort));
+    CHECK(decayShort.x == timeToX(a, total, sShort.total));
+
+    // Zero decay (the Trigger AHD default's shape): DecayEnd and HoldEnd share the exact same
+    // instant — the abrupt cutoff — and DecayEnd must not be nudged off it.
+    const StageEnvelope zeroDecay = ahd(0.5, 0.0, 1.0, 0.0, 3.0);
+    const AhdSplit sZero = splitAhdSeconds(zeroDecay);
+    const std::vector<EnvVertex> polyZero = buildEnvelopePolyline(zeroDecay, overlayOf(a), total);
+    EnvVertex decayZero, holdZero;
+    CHECK(findNode(polyZero, EnvNode::DecayEnd, decayZero));
+    CHECK(findNode(polyZero, EnvNode::HoldEnd, holdZero));
+    CHECK(decayZero.x == timeToX(a, total, sZero.total));
+    CHECK(decayZero.x == holdZero.x);  // truly coincident, not nudged apart
+}
+
 // --- curve knots --------------------------------------------------------------
 
 // A knot rides every sloped stage that has a duration, and none that does not — a zero-length
@@ -392,6 +423,7 @@ int main() {
     testAhdSplitNeverExceedsTheSpan();
     testHoldFractionEndpoints();
     testAhdIsOneToOneWithTheTimeAxis();
+    testDecayEndStaysAtItsTrueInstantEvenWhenCoincidentWithHoldEnd();
 
     testKnotsRideOnlySlopedNonZeroSegments();
     testKnotHeightTracksTheExponent();

tests/test_sample_map.cpp

@@ -703,8 +703,10 @@ static void testMigratedFadeStretchesWhenTheDecodeRateDiffersFromTheProjectRate(
     CHECK(matched.trigAhd.attackFrames == 441);
     CHECK(matched.trigAhd.decayFrames == 882);
 
-    // A 44.1 kHz file opened in a 48 kHz project: 441 * 48000/44100 = 480 source frames, ~8.8%
+    // resolvePlay's second argument is the DECODE rate; the seconds above were lifted (divided)
-    // longer than the fade the saved instance actually had.
+    // at the PROJECT rate 44100 — so this is a 48 kHz file opened in a 44.1 kHz project (the
+    // mirror of component_state_io.h's worked example): 441 * 48000/44100 = 480 source frames,
+    // ~8.8% longer than the fade the saved instance actually had.
     const PlayParams stretched = resolvePlay(st, 48000);
     CHECK(stretched.trigAhd.attackFrames == 480);
     CHECK(stretched.trigAhd.decayFrames == 960);