fix(nr2): retain the noise estimate across the TX→RX edge — Principle XI.

[crypticpy]

Summary

Fixes #3821.

AudioEngine::setRadioTransmitting called the full SpectralNR::reset() on
every TX→RX edge, which threw away the accumulated noise profile along with the
transient state. The estimator then had to re-converge from scratch on each
unkey: measured against a 6 s settled profile, output noise sits ~13 dB above
settled depth at 1.0–1.3 s post-reset and does not reach depth until ~1.6–1.9 s
(2–3 s on v26.6.4, before #4400's fast-adoption seeding).

reset() is split into resetTransientState() (overlap-add rings, gain masks,
AGC common-mode references, dry→wet ramp) and resetNoiseEstimate()
(OSMS/MMSE/NSTAT state). The TX→RX edge now calls the new public
resetTransient(), which flushes only the transients. With the profile
retained, suppression is back at settled depth the moment the ~1 s ramp
completes, while the re-queued zero-latency padding still keeps stale ring audio
from leaking (#3340).

Enable and source-switch paths deliberately keep the full reset() — a retained
profile is only valid when the stream resumes on the same band.

Constitution principle honored

Principle XI — Fixes Are Demonstrated. The regression test drives both
variants across a simulated TX gap and asserts the retained profile beats a full
reset by >6 dB in the 1.1–1.6 s post-ramp window (measured gap: 12.6 dB). The
test was mutation-checked two ways, each failing a different assertion:
re-adding resetNoiseEstimate() to resetTransient() fails the depth
assertions; skipping the ring flush fails the stale-leak assertion.

Principle VIII — Evidence Over Assertion. Every number above is measured,
not asserted.

Test plan

• Local build passes (cmake --build build) — clean, exit 0
• Behavior verified on a real radio if applicable — N/A: the change is
  entirely client-side DSP on the RX audio path, so it is exercised
  identically by the test harness; no radio-side state is involved
• Existing tests pass (CI) — local suite: 314 passed, 1 skipped.
  spectral_nr_test passes. Three tests failed on the first run;
  vkamp_connection_test passes on re-run (flaky, untouched by this
  change), and bridge_docs_check + hl2_state_restore_test reproduce
  unchanged on clean main — neither touches SpectralNR or the
  AudioEngine NR path
• Reproduction steps documented if user-reported bug — in NRS (Spectral Subtraction) noise reduction takes 2–3 s to re-engage after TX→RX transition #3821; the
  timing measurements above were taken against that repro

Checklist

• Commits are signed (docs/COMMIT-SIGNING.md) — GPG, GitHub reports
  verified: true
• No new flat-key AppSettings calls — this change adds no settings at all
• Code is clean-room — the split is of AetherSDR's own SpectralNR; nothing
  is derived from a proprietary binary (Principle IV)
• All meter UI uses MeterSmoother — N/A, no UI is touched (the diff is
  src/core/ + tests/ only)
• Documentation updated if user-visible behavior changed — no doc change
  needed: docs/architecture/audio-pipeline.md does not describe the TX→RX
  reset behaviour, so nothing on record goes stale. The user-visible effect
  (NR re-engages at settled depth after unkey instead of taking 2–3 s) is
  described in the Summary above. CHANGELOG.md deliberately untouched
• Security-sensitive changes reference a GHSA if applicable — N/A

---

Note on the claim protocol (AGENTS.md §Issue / PR Claim Protocol): I attempted
gh issue edit 3821 --add-assignee @me first, but GitHub rejects assignee
changes from an account without write access to this repo
(ReplaceActorsForAssignable permission error). This PR's Fixes #3821 link is
the visible claim on the issue timeline instead.

[ten9876]

Issue fit

#3821: NR2 threw away its whole noise profile on every TX→RX unkey (reset()), so suppression re-converged from scratch (~1.6–1.9 s to depth, ~13 dB above settled at ~1 s). The fix splits reset() into resetTransientState() + resetNoiseEstimate() and has the TX→RX edge flush only transients, keeping the OSMS/MMSE/NSTAT profile. That is the right shape, and the reasoning that a retained profile is only valid when the stream resumes on the same band (so enable/source-switch keep the full reset()) is correct and disclosed. Principle XI is honored — a regression test drives both edges.

The split's completeness is the thing most likely to be silently wrong in a refactor like this, and it checks out: every member of the old reset() lands in exactly one of the two new functions, none lost or duplicated, and the retained estimator genuinely survives resetTransient() because OSMS is gated on m_subwc/m_ambIdx (retained), not m_frameCount (reset).

Scope

Four files, all explained by the issue; new spectral_nr_test rows. No CI has run yet — that's a merge gate. Preflight: no sockets, pure DSP test.

Blockers

None. One risk to confirm below rather than a demonstrated defect.

Nits / one risk to confirm (non-blocking)

1. Retained m_noisePsd vs. a re-leveled signal during the ramp (inline). The AGC common-mode references correctly flush with the transients, and the comment at line 556 explains why (their scale corrector is dormant while m_frameCount < m_rampFrames, and the receiver AGC state they describe may have changed). But m_noisePsd — the retained estimate that is the point of the PR — is an absolute level, and if the receiver's AGC gain stepped during the over, gamma = lambdaY/noisePsd is computed against a stale absolute floor for the ~1 s ramp window, with the scale corrector returning early. The dry→wet ramp masks it (the dry blend dominates early), and minimum-statistics tracking re-levels within the window, so this may be fully benign — but it's the one path that could reintroduce a post-unkey over-suppression transient in the opposite direction from the old reset() (which re-seeded noisePsd high = gentle). Worth a bench row with an AGC step across the over, or a sentence in the PR confirming NR2's input is pre-AGC (in which case there's no mismatch and this is a non-issue).
2. The fullPostRampDb - transientPostRampDb > 6.0 assertion hard-codes a gap that depends on exact OSMS convergence timing under one PRNG seed — a future ramp/window retune that speeds full-reset convergence fails it with no real regression. Prefer asserting the behavioral invariant (transient path is at settled depth by ramp end) over the inter-path magnitude gap.
3. immediateDb averages samples 0..3600, but the first m_fftSize=1024 (~28%) are the guaranteed-zero OLA padding resetTransientState() re-queues — the "returns immediately" assertion is measuring a dry-plus-silence blend, and the same samples are asserted ~0 by the stale-peak check. Measure the immediate-return window after the padding, so a real leading-gap regression can't hide if padding length grows.

What was verified vs read

• Verified by me in the PR head: that resetTransientState flushes the AGC common-mode/scale state and m_frameCount while resetNoiseEstimate owns the profile; the author's own comment anticipating the ramp-window AGC dormancy (which reframes the automated pass's "blocker" here as the confirm-only risk above).
• From the automated pass, verified: the reset-split completeness (every member lands once) and the OSMS-survives-resetTransient gating — the load-bearing correctness claims.
• Not run: no hardware, no bridge session, tests read not executed (CI hasn't run). The nits are static/test-structure; the risk needs the author's AGC-topology answer or a bench row to settle.

[ten9876]

Risk to confirm (non-blocking) — retained m_noisePsd vs. an AGC level change. This comment correctly justifies flushing the common-mode references (their corrector is dormant during the ramp). But the retained m_noisePsd is an absolute level: if receiver AGC gain stepped during the over, gamma = lambdaY/noisePsd runs against a stale floor for the ~1 s ramp with the scale corrector returning early — a possible post-unkey over-suppression transient in the opposite direction from the old reset(). The dry→wet ramp and min-statistics tracking likely mask it. Confirm with a bench row that steps AGC across the over, or note that NR2's input is pre-AGC (then this is a non-issue).

[crypticpy]

Confirmed rather than a non-issue: NR2's input is post-AGC on every path (the radio's AGC for a Flex, WDSP's inside Hl2RxDsp for an HL2). The comment now records why the corrector cannot observe a step that straddles the gap and what bounds the fallout (min-statistics re-levels within one m_U * m_V window, ~1.5 s — no slower than the full reset() this replaced). Pinned by the new ±6 dB rows in spectral_nr_test; numbers in the PR thread. b2d90ce.

[aethersdr-agent]

1. Issue fit

Yes, for the reported scenario. #3821 asks that after unkey NR2 "re-engage within approximately the same 1-second window as the initial cold start" and explicitly offers the accepted remedy: "The noise profile should either be retained across the TX→RX transition or be rebuilt within the same ~1 s timeframe." This PR takes the first branch. The seam is placed correctly: the #3340 hazard (stale overlap-add ring read out as a whistle) and the #1863 hazard (maxed-out ramp counter slamming to full-wet) both live in the transient half and are still flushed; only the estimator state survives. The comment at AudioEngine.cpp:8999 is honest about the remaining scope — RN2/NR4/DFNR/MNR share the same bypass-and-stale-state path and are deliberately left for a follow-up, which the issue does not require.

Test layer is right per AGENTS.md ("Test-layer boundary" → DSP goes to a socket-free CTest), and spectral_nr_test is not merely registered — it is inside the ci.yml:305 -R "^(spectral_nr|nr2_settings_model)_test$" gate on the build job, which is green on 0e45e0e. So the new test genuinely ran.

Socket tests: none. tests/spectral_nr_test.cpp is pure in-process DSP — no QTcpServer/QUdpSocket/bind/listen/connectToHost, no peer process, no Fake* device class. Nothing to record.

2. Scope

File	What it changes	Claimed by title/body?	Verdict
src/core/SpectralNR.cpp	Splits reset() into resetTransientState() + resetNoiseEstimate(); adds public resetTransient()	Yes	In scope
src/core/SpectralNR.h	Declares resetTransient() + the two private halves	Yes	In scope — new public method on an internal DSP class, not protocol/settings/wire surface, so no maintainer gate
src/core/AudioEngine.cpp	TX→RX edge switches reset() → resetTransient(); comment rewritten	Yes	In scope
tests/spectral_nr_test.cpp	One new differential regression test + registration in main()	Yes	In scope

Everything in the diff is explained by the issue. No CHANGELOG.md entry (correct — release-prep file). No settings keys, no capability changes, no UI or default changes, so §6 personal-preference does not engage: this is a defect fix against a filed repro with a maintainer-labelled issue (bug, audio, maintainer-review), not a taste change. Per GOVERNANCE.md a bug fix with a clear root cause does not need an RFC.

Deleted behavior: the - lines remove the reset() call and the old comment. I checked mechanically that the split loses nothing: I extracted the member set written by the pre-PR reset() body (HEAD~1) and the union of the two new functions — the sets are identical (m_rampFrames is the only difference, and it appears only inside a new comment). Every write in both halves is an unconditional constant fill/assign to a disjoint member, so the reordering leaves reset() behaviorally identical to before. That is the load-bearing claim of the refactor and it holds.

CodeGuard: the five CG-PATH-001 findings are all in src/gui/MainWindow.cpp, which is not in this diff. They are pre-existing and unrelated to this PR; I am not carrying them forward.

3. Blockers

None.

4. Nits (non-blocking)

• The retained noise floor and an AGC step across the over — inline on SpectralNR.cpp:556. @ten9876 flagged this as a risk to confirm; I can now say something more specific than "the corrector returns early." resetTransient() sets m_commonReferenceInitialized = false, so the first post-TX frame re-seeds m_commonReferencePsd from the post-TX m_lambdaY (SpectralNR.cpp:1350-1354) — i.e. the reference is re-anchored to the new level, permanently erasing the evidence that a step occurred. scalePowerHistory() can therefore never fire to rescale the retained m_noisePsd for that step. Not a blocker (see the inline for why the fallout is bounded), but it is the one place where "retain the estimate, flush the references" is not free.
• Retune-during-TX is the weak direction — the AudioEngine.cpp comment says the estimator "keeps adapting from there if the band moved during TX." True, but asymmetric: a move to a noisier band leaves the retained floor too low, and min-statistics adapts upward slowly. The old reset() seeded high (initNoise = 1.0 → gentle suppression, converging downward, the fast direction). So for the retune-across-the-over case this is a small step back. Worth either softening the comment or, as a follow-up, calling the full reset() from a frequency/band-change hook.
• Test ordering fragility — inline on tests/spectral_nr_test.cpp:2428. settledDb is captured by reference and written only on the transientReset == true path, so both post-ramp assertions silently depend on runResumed(true) being called before runResumed(false).
• A body claim with thin support — "Enable/source-switch paths keep the full reset()." The enable path does. But the source-switch function that carries the surviving m_nr2->reset() at AudioEngine.cpp:4743, AudioEngine::resetRxChainStateForSourceSwitch(), has no callers anywhere in the tree (I grepped all .cpp/.h/.mm). That is pre-existing dead code, not introduced here — just flagging that the claim reads stronger than the code supports.

5. What I tried to break

• Is the split lossy? Mechanically compared the member set of the pre-PR reset() against the union of resetTransientState() + resetNoiseEstimate(). Identical. No member silently dropped out of reset(), and no member is written by both halves, so the reordering is a no-op.
• Does m_frameCount = 0 in the transient half secretly re-arm an estimator warm-up that would overwrite the retained profile? This was my best candidate for a real bug — a fast-adoption seed gated on frame count would defeat the whole PR. It does not exist: m_frameCount's only readers are the wet ramp (SpectralNR.cpp:1094-1097) and the common-mode reference calibration (:1418, :1745). No noise-estimator path reads it. The author's justification comment is accurate.
• Sibling reset() call sites. The other two (AudioEngine.cpp:1563 in resetExternalKiwiDspState, :4743 in the source-switch path) correctly keep the full reset — those are stream/source changes where the old profile does not describe the new audio. m_kiwiSdrNr2 and the per-source nr2 instances are not touched at the TX edge, but they were not before this PR either, so nothing regressed.
• Does the test pass against the unfixed code? No. fullPostRampDb - transientPostRampDb > 6.0 is a genuine differential assertion — if resetTransient() were an alias for reset() the gap is 0 dB and it fails. The stale-ring guard is also meaningful: skipping the m_outAccum flush while still setting m_outWritePos = m_fftSize would leak pre-TX content into the first 1024 samples and break stalePeak < 1e-9.
• Test index bounds. rmsGainDb reads output[i + latencySamples]; the furthest window (2.5–3.0 s, latency 1024) tops out at index 73023 against a 144000-sample buffer. No OOB. The latency offset also means immediateDb starts past the 1024-sample zero pad, so it measures real dry audio rather than being dragged down by the pad — the > -3.0 threshold is measuring what it claims to.
• Threading / lifetime. Unchanged: same previous && !tx edge, same m_dspMutex guard, same m_nr2Enabled && m_nr2 null check. resetTransient() adds no allocation and no I/O, so it is no worse on the calling thread than the reset() it replaces.

What I could not check: everything here is reasoned from source in /tmp/aetherclaude/pr-5364. I am headless — no build, no test execution, no radio. The PR body's measured numbers (~13 dB at 1.0–1.3 s, 12.6 dB measured gap) I did not reproduce; CI green on the spectral_nr_test gate is the strongest independent evidence available to me, and it only proves the assertions as written hold on the CI runner. The AGC-step interaction in the first nit is a code-path argument, not an observed transient.

6. Recommendation

Approve with nits. This is a well-targeted fix: the seam is in the right place, the split is provably behavior-preserving for reset(), the strongest failure hypothesis I had (a frame-count-gated estimator warm-up defeating the retention) is refuted by the code, and the new test is a real differential guard that CI actually runs. Nothing here rises to a blocker.

Concrete next step: answer the SpectralNR.cpp:556 inline — either confirm the bounded-fallout reasoning is acceptable, or note it in the comment so the next reader does not have to re-derive that the common-mode corrector is disarmed for a step that straddles the gap. Softening the "keeps adapting" clause to acknowledge the slow-upward direction would be worth doing in the same pass. Nice work, and thanks for the mutation check in the commit message — that is exactly the evidence that makes a DSP test believable.

---

_{🤖 aethersdr-agent · cost: $3.2889 · model: claude-opus-5}

[aethersdr-agent]

Confirming @ten9876's flagged risk with a specific mechanism, and then arguing why it is still non-blocking.

This comment says the references' "calibration re-runs inside the re-armed ramp window," which is true but understates it. resetTransientState() sets m_commonReferenceInitialized = false, so the first post-TX frame hits the early return at SpectralNR.cpp:1350-1354 and copies the current m_lambdaY straight into m_commonReferencePsd. The reference is re-anchored to the post-TX level. For the rest of the ramp (m_frameCount < m_rampFrames, :1418) it only EMA-smooths that reference and returns without any scale detection.

Net effect: if the receiver AGC stepped during the over, the step is invisible to detectCommonModeScale() — not merely deferred past the ramp, but erased, because the reference now describes the new level. scalePowerHistory() will therefore never fire to rescale the retained m_noisePsd, which is an absolute level and is now inconsistent with the input by exactly that step.

Why I still don't think this blocks:

• Gain stepped down → retained floor too high → gamma = lambdaY/noisePsd too low → over-suppression. Min-statistics tracks downward fast, so this self-corrects quickly, and m_currentWet is ramping from 0 through the first ~1 s anyway.
• Gain stepped up → retained floor too low → under-suppression. Min-statistics is slow upward here, so this is the direction that could linger — but it lands the user in roughly the pre-PR behavior (audible band noise for a beat), not somewhere worse.

So the failure mode is bounded on both sides and the ramp covers the loud half. Two things would settle it:

1. If NR2's input is pre-AGC, this is a non-issue outright — worth stating in the comment either way.
2. Otherwise, a bench row that steps AGC-T across an over would close it. Not something I can run headless.

Either way, please leave a sentence here recording that the corrector cannot observe a step straddling the gap, so the next reader doesn't have to re-derive it from :1350.

[aethersdr-agent]

settledDb is captured by reference and written only on the transientReset == true branch inside runResumed, so both post-ramp assertions silently depend on runResumed(true) running before runResumed(false). As written that ordering holds, but if someone ever swaps the two calls (or evaluates them in one expression), settledDb stays 0.0 and std::abs(transientPostRampDb - settledDb) < 2.5 starts asserting against a meaningless reference rather than failing loudly.

Cheap hardening: have runResumed return the settled value alongside the resumed buffer, or assert settledDb != 0.0 before the comparisons. Non-blocking.

[crypticpy]

Taken — each run now measures and returns its own settled depth alongside the buffer, and the two settle phases are asserted identical, so no assertion depends on run order any more. b2d90ce.

[aethersdr-agent]

Cisco CodeGuard — static analysis of this PR (5 finding(s))

• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-5364/src/gui/MainWindow.cpp:8805
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-5364/src/gui/MainWindow.cpp:8917
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-5364/src/gui/MainWindow.cpp:9036
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-5364/src/gui/MainWindow.cpp:9037
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-5364/src/gui/MainWindow.cpp:9038

_{Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.}

---

_{🤖 aethersdr-agent · cost: $3.7590 · model: claude-opus-5}

[crypticpy]

@ten9876 @aethersdr-agent Thanks, both — all of it taken, pushed as b2d90ce.

The AGC question. NR2's input is post-AGC on every path: the radio's own AGC for a Flex (audio arrives already levelled), and WDSP's AGC inside Hl2RxDsp for an HL2. So the risk is real, and the bot's mechanism is exactly right — the first post-TX frame re-seeds m_commonReferencePsd from the post-TX spectrum, so the corrector never sees a step that straddles the gap. That sentence is now in resetTransient()'s comment, together with the bound: minimum statistics re-levels a floor that is too high within a few frames, and one that is too low within one window (m_U * m_V frames, ~1.5 s at the default geometry), which is no slower than the full reset() convergence this path replaced.

Bench row, synthetic. Rather than leave that as a code-path argument, spectral_nr_test now resumes at ±6 dB after the edge. Measured on this branch:

resume level	1.1–1.6 s, retained	1.1–1.6 s, full reset	2.5–3.0 s, both
unchanged	−27.7 dB	−15.1 dB	−28.2 / −27.5 dB
+6 dB (floor too low)	−15.5 dB	−14.6 dB	−27.4 / −27.5 dB
−6 dB (floor too high)	−27.6 dB	−21.5 dB	−28.1 / −27.6 dB

Settled depth is −27.4 dB. So the upward step is the one that costs anything, and it lands within a dB of where the old full reset already put the operator; the downward step is at depth immediately. Asserted as "no worse than a full reset post-ramp" and "settled depth within one window".

Test nits.

• Order dependence: each run now measures its own settled depth and returns it with the buffer; the two settle phases are asserted identical, so nothing depends on which run goes first.
• The 6 dB inter-path gap assertion is gone. The behavioural invariant ("settled depth restored right at ramp end") is what remains, and it is still a real differential: a mutation that makes resetTransient() also call resetNoiseEstimate() fails that row (post-ramp −15.1 dB against the settled −27.4 dB) and no other. The gap is printed for information.
• immediateDb and the zero padding: the bot has this right — rmsGainDb() reads output[i + latencySamples] with latency = 1024, so the 0–0.15 s window starts just past the pad that the stale-peak check covers. I have added a sentence in the test saying so.

Comment softening. The AudioEngine.cpp comment no longer says the estimator simply "keeps adapting" if the band moved; it names the slow upward direction and the ~1.5 s bound.

Two things of my own. The test's comments had double-encoded UTF-8 arrows (TXâ†'RX), now fixed. And the resetTransientState() private wrapper is collapsed: reset() calls resetTransient() and resetNoiseEstimate() directly.

Not touched. resetRxChainStateForSourceSwitch() is indeed uncalled on main — pre-existing dead code, so I have left it alone rather than fold an unrelated deletion into this PR. The five CodeGuard path findings are in MainWindow.cpp, which this PR does not touch.