fix(audio): guard every DSP-fed int16 cast against a non-finite sample. Principle VIII.

[tropo1234]

Summary

std::clamp returns v unless v < lo or hi < v, and every comparison
against NaN is false, so a NaN passes straight through the clamp and the
following static_cast to a 16-bit integer is undefined behaviour. jensenpat
raised it on his #5992 approval for the ANAN radio-speaker quantiser; the same
clamp-then-cast shape sits at every float→int16 conversion that consumes DSP,
resampler or capture audio. This guards all of them, each at its own site and
at its own existing scale:

Site	Path	Direction	Test
AnanSliceAudio.h sliceAudioToInt16() (call site AnanBackend.cpp)	ANAN radio-speaker wire	RX	anan_p2_protocol_test
MetisProtocol.cpp ep2WriteTxIq	HL2 TX IQ on the wire	TX	hl2_metis_protocol_test
TciIoWorker.cpp encodeRxAudio (int16 format)	TCI audio to external clients	RX	tci_rx_audio_test
Hl2Backend.cpp forwardSpeakerAudioToCodec toI16	HL2 radio-speaker codec	RX	—
ClientPuduMonitor.cpp playback	monitor after the resampler	RX	—
AudioEngine.cpp RADE capture float→int16	RADE mic	TX	—
AudioEngine.cpp RADE resampler output	RADE mic	TX	—
RADEEngine.cpp LPCNet input	RADE TX	TX	—

The guard is separate from the clamp because it has to be: a clamp cannot test a
value that fails every comparison. It covers infinity too. An infinity clamps
correctly, but to a full-scale rail — a click on a speaker feed, a full-scale
sample on a TX feed — so one std::isfinite predicate makes both silence, which
is also what each path already sends when muted or idle.

Finite input is unchanged. Every edit is a guard in front of the existing
expression; no scale, rail or rounding changes, and the symmetric ×32767 sites
(ANAN, HL2, Metis) and the ×32768 sites (AudioEngine, RADE, TCI, monitor) keep
their own convention. A 1,000,000-bit-pattern probe of the ANAN conversion found
zero mismatches between old and new for every finite input. A non-finite
sample becomes silence for that sample only; finite samples in the same block
are untouched.

The ANAN speaker interleave moves into sliceAudioToInt16() so it can be
unit-tested; the five comment lines the AnanBackend.cpp call site loses are
#6072's clamp rationale, which moves into that function with tests for it — the
shortening AGENTS.md sanctions ("as long as the guard or test it describes
stays").

Not in scope, and why:

• Already guarded: AudioEngine.cpp TX float→int16, CwDecoder.cpp,
  TxMicChannelNormalizer.cpp, QsoPcmConverter.cpp (whole block validated
  first).
• Not undefined: IcomAudio.cpp, TxVoiceProcessor.cpp,
  RemoteAsrBackend.cpp round with lround() and then narrow an integer, which
  C++20 defines.
• Own tone generator as input: CwSidetoneQAudioSink.cpp, the CW-sidetone
  record path in AudioEngine.cpp, ClientQuindarTone.cpp, ClientTxTestTone.cpp.
• Not a cast: asr/Fbank.cpp, RNNoiseFilter.cpp scale float into float.

This guards the conversion, not the source. A NaN that reaches a resampler or
IIR filter can latch in its state; that would now produce steady silence rather
than undefined behaviour, and nothing here detects or resets it.

jensenpat's other #5992 follow-up — capping catch-up at the estimated FIFO
room — is deliberately not here. It was implemented and then failed on an
ANAN-G2 (speaker FIFO median 995 → 117 locations, underflow reports 1 per 14 min
→ 782 per 11); the measurement is written up on #5992 for a maintainer's
direction.

Constitution principle honored

Principle VIII — Evidence Over Assertion. The existing comments asserted
that clamping before the cast made the conversion safe. It does for overshoot
and not for NaN. Each tested guard is shown red without its fix below.

Test plan

• Local build passes (cmake --build build)
• Existing tests pass — 651/653 locally at 44d7cb29 (Arch, Qt 6.12, ctest -j16).
  One deliberate skip (app_settings_safety_explicit-profile-path-isolation); one
  failure, connection_panel_size_test ("horizontal overflow is reachable,
  not clipped"), deterministic on this machine with ThemeManager reporting
  its resources failed to load. This PR touches no file under src/gui/.
• Behaviour on a real radio — the ANAN quantiser was in the audio path for
  two ANAN-G2 sessions (14 min and 11 min, Saturn gateware 27) with clean
  audio. A NaN cannot be induced from a radio, which is why the guards are
  covered by arithmetic tests instead.

Mutation proofs — each check below goes red with its guard removed:

mutation	caught by
ANAN guard narrowed to std::isnan	an infinity quantises to silence…
ANAN clamp removed	an overshoot clamps, not wraps
ANAN full scale 32767 → 32768	full scale is the positive peak
ANAN guard removed, built with -fsanitize=float-cast-overflow	runtime error: nan is outside the range of representable values of type 'short int'
Metis EP2 guard removed	an infinite Q sample is silence, not a full-scale rail, a negative infinity is silence too; NaN under the sanitizer as above
TCI int16 guard removed	non-finite samples encode as int16 silence

Without the sanitizer, a NaN check alone cannot go red: the undefined cast lands
on 0 on both x86-64 (cvttss2si → 0x80000000, low 16 bits zero) and arm64
(fcvtzs → 0). The sanitizer build is the proof for NaN; the infinity checks
are the deterministic ones.

The HL2 speaker, monitor and three RADE sites have no test seam short of a
connected backend or a running audio engine; each is the same one-line
std::isfinite guard as the tested sites, verified by build and by reading.

The four aetherd gates (check_engine_boundary, gen_touchpoint_manifest --check, check_capability_records, check_command_plane, all --strict)
produce identical output on the merge base and this head. No new tests target;
the new checks land in existing, socket-free targets.

Checklist

• Commits are signed (docs/COMMIT-SIGNING.md)
• No new flat-key AppSettings calls — no settings touched
• Code is clean-room (Principle IV)
• All meter UI uses MeterSmoother — n/a, no UI
• Documentation updated if user-visible behavior changed — n/a: nothing
  changes for finite audio
• Security-sensitive changes reference a GHSA if applicable — n/a, local DSP
  values, not untrusted input

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

No linked issue, and the body says so: this is the second of two review follow-ups from jensenpat's approval on #5992. GOVERNANCE.md doesn't want an RFC here — it's a bug fix with a stated root cause and no new public surface. The quoted finding is correct on the language: std::clamp(v, lo, hi) returns v unless v < lo or hi < v, both false for NaN, so NaN reaches static_cast<qint16>(NaN * 32767.0f), which is UB. The guard genuinely cannot be folded into the clamp, and the fix does exactly what the follow-up asked. I could not read jensenpat's review text itself (PR reviews don't appear in list_issue_comments), so the attribution is taken on trust; tropo1234's own #5992 comment of today is consistent with it.

The one thing the title doesn't name is the move: two lines leave AnanBackend.cpp and become a tested function. That's disclosed in the body, and I agree it's the right seam — this is the one half of the law that cannot be reached by listening.

Scope

Everything in the diff is explained by the stated fix.

File	What it changes	Claimed by the body?	Verdict
src/core/backends/anan/AnanSliceAudio.h	adds sliceAudioToInt16(); adds <algorithm> and <cstdint> for the new std::clamp/std::int16_t uses	yes	In scope
src/core/backends/anan/AnanBackend.cpp	call site uses the helper; 5 comment lines removed	yes, flagged explicitly	In scope
tests/anan_p2_protocol_test.cpp	8 checks, <limits>	yes	In scope

On the deleted comment, which is the one - block worth reading carefully: it named a symptom ("a loud transient becomes a full-scale click of the opposite polarity"), so I checked what now prevents it. The clamp is still there, the symptom survives in compressed form in the new header ("the resampler overshoots on transients and an unclamped cast wraps sign"), and it is now covered by an overshoot clamps, not wraps. AGENTS.md:182–183 — "Deleting or shortening a historical comment is welcome cleanup, as long as the guard or test it describes stays" — sanctions this exactly, and the test is new here rather than assumed. Not a regression.

No CHANGELOG.md entry — correct. No settings, no capability, no protocol surface, no UI, so Constitution I/II/III/V/VI and the Settings sections aren't implicated.

Socket tests: none added, modified, or removed. The added checks land in anan_p2_protocol_test, which is already registered (tests/tests.cmake:962) and is socket-free — no QTcpSocket/QUdpSocket/bind/listen anywhere in it, and the new checks are pure arithmetic. (#5992's anan_speaker_audio_test does bind, but this PR doesn't touch it.)

Blockers

None.

Nits

• The new block orphans an existing heading comment. // Degenerate input must not walk off anything. was the heading for the nullptr/zero-frame checks; the quantiser block was inserted between it and the lines it describes. Inline on tests/anan_p2_protocol_test.cpp:597.
• One row of the mutation table looks non-discriminating. With the isfinite guard deleted, the common lowerings of a NaN→int16_t conversion both land on 0 (x86-64 cvttss2si → 0x80000000, low 16 bits zero; arm64 fcvtzs → 0), so sliceAudioToInt16(NaN) == 0 plausibly still passes. Doesn't touch the fix — UB is UB whatever it happens to yield — but it bears on the Principle VIII claim. Inline on AnanSliceAudio.h:54.
• The siblings carry the same hole. Hl2Backend.cpp:4267 toI16 is the structural twin (radio-speaker feed, clamp-then-cast, no isfinite), with MetisProtocol.cpp:297 and CwSidetoneQAudioSink.cpp:146 close behind. Out of scope for a fix(anan) PR, but worth naming so this doesn't read as "the quantiser hole is fixed". Inline on AnanSliceAudio.h:57.
• The guard sits at one of three consumers of the same buffer. Inline on AnanBackend.cpp:1505.
• CI's green doesn't cover the new checks. All 5 checks pass on 3e0a8ab5 (build, check-macos, check-windows, Static checks, Sanitizer configures), but every ctest step in ci.yml is -R-filtered and anan_p2_protocol_test is in none of them. The body's unticked "Existing tests pass (CI)" box is therefore the honest state and will stay that way; the 595/595 local run is the only evidence these checks run at all.

What I tried to break

• The mute path, which is where a non-finite value gets most interesting. lineout is exactly 0.0f when muted, and inf * 0.0f is NaN — so a muted block containing an infinity goes NaN inside the multiply, after any guard placed on outL[i] would have run. Guarding the product rather than the input is the correct placement and it holds: NaN → isfinite false → 0, so mute still sends silence. Survived.
• Symmetry and off-by-one at the rails. -1.0f → -32767, not -32768; matches the "symmetric clamp: 32767, not 32768" convention the HL2 quantisers document, and no asymmetric rail is introduced. qint16 (the m_speakerInterleaved element type) and std::int16_t are the same type here, so the return type change narrows nothing.
• The header's own closure. It stays Qt-, socket- and DSP-free, and both new includes are the ones the new code needs — <algorithm> for std::clamp is now owned by the header rather than inherited from whatever the .cpp happened to pull in, which is the transitive-include loss that usually shows up as a Linux-only break.
• Sibling call sites inside the fix's own scope. Grepped the whole ANAN backend for other float→int16 conversions: this was the only one. P2Protocol::buildSpeakerPacket takes already-quantised std::int16_t, so the wire encoder needs nothing.
• Whether the tests would pass against the unfixed code. Three of the four would fail: ±inf → ±32767 and overshoot-wraps (2.0f → 65534, wrapping to -2) are deterministic. The NaN one is the exception, which is the nit above.

Findings are reasoned from the head checkout at /tmp/aetherclaude/pr-6089; I have no build or test run, so nothing here was reproduced at runtime. I could not check the mutation table, the 595/595 local run, or the two G2 sessions.

Recommendation

Approve with nits. The fix is correct, minimal, placed where the law can be tested, and the one deletion that looks like a revert is explained and sanctioned by canon. Both substantive nits are evidence-and-comment matters, not code: move the orphaned heading comment below the new block, and either say which build the NaN mutation turned red on or drop that row from the table. The sibling quantisers and the pacing follow-up are maintainer-direction items, not conditions on this merge. Nice disclosure on the withdrawn second follow-up — "it passed locally and then the hardware said otherwise" is the write-up that saves the next person the bench run.

---

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

[tropo1234]

All three nits taken, and the one that was a code change is pushed.

The orphaned heading — fixed. // Degenerate input must not walk off anything. now sits directly above the applySliceAudioInPlace(nullptr, 4, false, 50, 50); line it describes; the quantiser checks stay where they are under their own two-line heading. The heading moved rather than the block, so it is comment-only and the three-platform CI build is the only compile behind it.

The NaN mutation row — you are right, and it comes out rather than gets defended. The lowering argument holds on both targets that matter here: unguarded, std::clamp(NaN, -1.0f, 1.0f) returns NaN, NaN * 32767.0f is NaN, and the cast is undefined behaviour that in practice lands on zero either way — x86-64 cvttss2si gives the integer-indefinite 0x80000000, whose low 16 bits are 0, and arm64 fcvtzs gives 0 directly. == 0 therefore stays green with the if deleted, on this machine and on the macOS arm64 runner both.

I cannot produce the run that row claims, so the table is now 3/3 — the isnan narrowing, the clamp removal and the full-scale off-by-one, all three deterministic without their fix. The NaN check itself stays, relabelled for what it is: an assertion of the contract (a non-finite input yields silence, never an undefined value), not evidence that the guard is load-bearing. The body claimed "none of them is a check that cannot fail" and for that row it was not true; thank you for not letting it stand.

If a maintainer would rather see it proven than dropped, say so — it is two ~45-minute full rebuilds, because this branch and the one in flight differ in IRadioBackend.h, SliceModel.h and RadioModel.h, for a row whose expected result is "still green".

The sibling quantisers — staying out, and the framing is corrected. The sites you name do have the same shape, and this PR fixes the quantiser hole at the ANAN wire cast, not the quantiser hole. I will open a follow-up issue listing the four you found — the two HL2 ones, the sidetone sink, and AudioEngine.cpp downstream of the two earlier taps — so the inventory lives somewhere other than a review thread, and leave the shared-versus-per-family question in it for a maintainer. AnanSliceAudio.h is deliberately ANAN-local, so moving the law somewhere shared is a decision about the seam rather than a cleanup.

On the CI observation: confirmed independently — ci.yml has four ctest -R steps and anan_p2_protocol_test is in none of them, so the unticked box is the honest state and will stay unticked. I am not adding it, because tools/check_ci_test_gate.py polices that list and widening it is a separate decision about the gate.

[ten9876]

Issue fit

Yes — the stated NaN-cast fix is addressed. No closing issue is linked; I read jensenpat's review on #5992 and confirmed the requested follow-up. At head dd7d669bc, AnanSliceAudio.h:52–57 rejects non-finite values before the cast, retains finite clipping and symmetric ±32767 scaling, and AnanBackend.cpp:1505–1506 applies it to both channels after lineout gain/mute. The separate FIFO-pacing follow-up remains explicitly excluded. This is a clear-root-cause bug fix under GOVERNANCE.md's RFC exemption.

Scope

Everything in the diff is explained by the stated fix.

File/group	Change	Claimed in body?	Verdict
src/core/backends/anan/AnanSliceAudio.h	Pure guarded float-to-int16 helper and explicit standard includes	Yes	In scope
src/core/backends/anan/AnanBackend.cpp	Replace both casts with helper; retain clamp rationale at helper	Yes	In scope
tests/anan_p2_protocol_test.cpp	Eight arithmetic checks and <limits>	Yes	In scope

Both commits' author dates and subjects match this work; GitHub verifies both signatures. No settings, defaults, UI, TX, new protocol/capability surface, or architecture change. The internal inline helper does not expand the backend seam. No baseline/manifest edits or CHANGELOG entry. The scope checklist holds; one behavioral description needs the small correction below.

Blockers

None.

Nits — non-blocking

1. Document the available NaN mutation proof (tests/anan_p2_protocol_test.cpp:603, src/core/backends/anan/AnanSliceAudio.h:54). The body is right that an ordinary zero comparison cannot establish absence of UB, but its statement that this case cannot be mutation-proven is too broad. The standalone target needs no full app rebuild. I compiled it with the command below: head passes; deleting the guard exits 1 with runtime error: nan is outside the range of representable values of type 'short int'. Please include this verification in the body. This does not require widening the frozen PR test gate.

   g++ -std=c++20 -O2 -fsanitize=float-cast-overflow -fno-sanitize-recover=all \
     -Isrc tests/anan_p2_protocol_test.cpp src/core/backends/anan/P2Protocol.cpp \
     -o /tmp/anan_p2_protocol_test
   /tmp/anan_p2_protocol_test

2. Describe sample-level replacement accurately (src/core/backends/anan/AnanBackend.cpp:1505–1506). The body's checklist says “silence for the affected block.” Each non-finite sample becomes zero independently; finite samples in the same block remain audible. Suggested wording: “silence for each affected sample.”

Verification and attacks

• Test-boundary preflight: inspected the complete modified test and its source dependencies. No sockets or synthetic peer; no socket test added, modified, or removed. Built the standalone target's exact two sources with GCC/C++20 at -O2; both merge base f10aa757a and head pass. Head also passes with float-cast-overflow sanitization.
• Mutation checks: narrowing the guard to isnan fails both infinity assertions; removing the clamp fails both overshoot assertions; using 32768 fails both full-scale and overshoot pairs. Removing the entire guard under the sanitizer detects the NaN cast. All mutations were confined to scratch and restored.
• Additional socket-free probe: 1,000,000 deterministic float bit patterns, including 996,062 finite values; zero mismatches between old/new finite conversion, all non-finite outputs zero. Explicit infinity-times-zero mute product also returns zero. The original clamp-and-cast expression independently triggers the same sanitizer diagnostic for NaN.
• All four backend conformance gates pass on base and head with identical output: engine boundary, touchpoint manifest, capability records, command plane. Read the lifetime contract and #5554 notice against the diff: no new coupling, family branch, signal/thread routing, or keying path. Existing sibling quantisers remain outside this ANAN-local fix as the revised body discloses.
• No automated code-review skill was available; this was a manual review with executable arithmetic checks. No full GUI build, broad suite, or bridge session: SimBackend cannot reach this ANAN-specific speaker conversion. No radio or operator app instance was driven. The author's historic 595-test run, eleven-ratchet run, and G2 audio/FIFO measurements were not independently reproduced and are not evidence for my verdict.

CI and recommendation

Approve with nits. The focused behavior and all documented mutations are independently verified, with an additional sanitizer proof for the original defect. The earlier orphaned-heading nit is fixed.

All four required checks and sanitizer configuration passed on this head, but their pull_request runs were created 2026-10-02 22:03:10 UTC; current main f4e463c2a is dated 2026-10-04 09:02:58 UTC. These green checks cover an older merge result. Branch protection currently has strict: false, so it does not force a refresh. Also, anan_p2_protocol_test is outside the filtered per-PR CTest gate; the local runs above supply the focused evidence. Update the two descriptions and refresh integration validation against current main before landing; approval remains the maintainer's decision.

[ten9876]

Approving at db1187f8.

Fixed

1. ANAN speaker quantiser: NaN/±inf → silence via sliceAudioToInt16() (3e0a8ab55, tropo1234). Proven by anan_p2_protocol_test. A sanitizer build with the guard deleted reports nan is outside the range of representable values of type 'short int'.
2. Orphaned test heading moved (dd7d669bc).
3. The same guard at the six other DSP-, resampler- and capture-fed int16 casts (44d7cb290, maintainer's call to fold them in): HL2 speaker toI16, Metis EP2 TX IQ, TCI int16 RX, Pudu monitor, and the RADE capture, resampler and LPCNet casts. Each is a guard placed in front of the existing expression at the site's own scale.
   • hl2_metis_protocol_test with the guard removed: FAIL: an infinite Q sample is silence, not a full-scale rail.
   • tci_rx_audio_test with the guard removed: FAIL: non-finite samples encode as int16 silence. With it: 2682 checks, 0 failures.
4. PR body: per-sample (not per-block) silence; the sanitizer NaN proof.

Decided by the maintainer: fold in all seven sites; inline guard at each site rather than a shared header.

Verified: full local ctest 651/653 at 44d7cb29 (1 deliberate skip; connection_panel_size_test fails in a GUI file this PR doesn't touch). All 11 tools/check_*.py pass, and the four aetherd gates match main.

Not verified: the HL2 speaker, Pudu and RADE sites have no test seam. They're covered only by the build and by reading. NaN can't be induced on hardware.