[rfoust]

The sanitizer run on #6173 head (37243767521) reports no TSan warnings but still times out in five test executables: audio rates, NR stereo independence, RTL averaging, weather radar loading, and peripheral authentication. The long suites accumulate independent DSP initialization, model teardown, decode, or FFT work under one deadline. The RTL capture fixture also previously compared a scheduling-dependent later spectrum frame against the analytic first DC-corrected window.

This patch:

• Registers the existing audio (15), NR (10), weather (37), and peripheral (9) scenarios as focused CTest cases, retaining the existing timeout ceilings per scenario and every assertion. A separate guard compares the weather executable's QtTest -functions inventory with the same list used to register its cases, failing on missing, unknown, or duplicate registrations. The executable targets and their full no-argument invocations remain available. Missing optional NR methods return CTest skip code 77 without masking assertion failures.
• Pools 64 separated noise bins after warmup in RTL averaging, retaining the variance and mean-power thresholds while reducing input batches from 500 to 96.
• Acknowledges one injected RTL callback at a time and services the owner-thread capture transaction, so the DC assertion measures the exact first complete corrected window.

No production code, product behavior, workflow, or frozen CI gate changes. Existing scenario timeout values remain unchanged.

Related to #6156; complements #6173. This does not close the consolidated issue: Linux TSan/LSan confirmation and the carried intermittent DecoderAudioModel report remain outstanding.

Validation

• NNR review correction: mandatory vendored NNR construction failure is now an assertion failure, never a skip. Forced constructor failure returned exit 1 for both focused rates and the full suite. After restoring production code, all 6 available NR cases passed; only 4 optional DFNR/BNR cases skipped.
• Registration-guard update: 38 weather CTest checks passed, including the guard (24.33 s). The guard also passed against the ASan/UBSan executable. Missing, unknown, and duplicate inventory mutations each failed with exit 1.
• Native ARM64 focused CTest: 69 passed, 4 optional NR cases skipped, 0 failed (133.11 s).
• ASan/UBSan focused CTest: 69 passed, 4 optional NR cases skipped, 0 failed (155.04 s), with immediate failure on sanitizer errors and no sanitizer reports.
• Full standalone suites passed; audio retains 14,220 checks / 0 failures, matching the baseline. Invalid selector checks return exit 2.
• Mutation proof: temporarily disabling production averaging makes variance ratios 1.0 and fails the smoothing assertion; temporarily preserving old DC spectrum history fails the exact first-window assertion. Both mutations were restored, both RTL tests passed again, and the app was rebuilt.
• Full Qt 6.12 ARM64 app build passed with RADE and RTL enabled. CMake host/system and executable are ARM64; no RNNoise x86 sources appear in the build graph.
• Strict test-registration check, frozen CI gate check, and git diff --check passed. Independent reviews checked scenario preservation, failure/skip handling, callback serialization, and statistical coverage.

Local limitations: DFNR and BNR are unavailable; Qt is the stock package. macOS LeakSanitizer is unavailable, so leak detection was disabled. The temporary ASan build compiled the unrelated generated EIBI code map at -O0 to avoid a Clang optimization stall, keeping sanitizer instrumentation enabled; other sources and the native app use normal RelWithDebInfo optimization. Linux sanitizer CI remains required.

The agent automation bridge cannot execute CTest fixtures or inject their test-owned RTL callbacks. Proof here comes from direct tests, sanitizers, and regression mutations; no live-radio or bridge-backed result is claimed.

Generated with OpenAI Codex.

[Copilot]

Copilot review overview

🟡 Changes recommended

NNR construction failures are incorrectly reported as skips even though NNR is an unconditional vendored feature.

Review effort: Balanced
Findings: 1

Open (1)

• Treat failed unconditional NNR construction as a test failure · New

What changed in this PR

Partitions sanitizer-heavy tests into focused CTest cases and stabilizes RTL DSP fixtures.

Changes:

• Splits audio, NR, weather, and peripheral suites.
• Adds QtTest registration verification.
• Reduces RTL averaging workload and serializes injected callbacks.

File	Description
tests/​verify_qttest_registration.cmake	Validates QtTest registration completeness.
tests/​tests.cmake	Registers focused scenarios.
tests/​RtlInjectedDevice.h	Tracks callback activity.
tests/​rtl_spectrum_averaging_test.cpp	Pools noise-bin measurements.
tests/​rtl_capture_worker_test.cpp	Makes DC-window testing deterministic.
tests/​peripheral_auth_dialog_test.cpp	Adds scenario selection.
tests/​nr_stereo_independence_test.cpp	Adds method/rate selection and skip handling.
tests/​audio_engine_rates_test.cpp	Adds scenario/effect selection.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[aethersdr-agent]

Issue fit

Partially, as the body itself says. #6156's readable content is 11 failures: 3 timeouts (audio_engine_rates, weather_radar_loading, peripheral_auth_dialog) and 8 real TSan reports (the WanConnection TLS lock-order-inversion, the WDSP TXASetupBPFilters/xbandpass races). This PR addresses the 3 timeouts and explicitly disclaims the rest ("does not close the consolidated issue"), which is the honest framing. Two caveats worth a maintainer's eye:

• rtl_spectrum_averaging_test and nr_stereo_independence_test are not among the 11 failures in #6156's body. Their justification rests entirely on run 37243767521 (on #6173's head), which is named only in the PR body and which I cannot read. The claim is plausible — both are long single-process suites — but it is unverified from here.
• Per [sanitizer fp churns on partial fix]: fp is the sorted failed-test name set. Renaming audio_engine_rates_test → 15 names, weather_radar_loading_test → 38, etc. means #6156's fingerprint 65304097123c can never recur verbatim. Any residual timeout opens a fresh sticky under the new names rather than reopening this one. Expected and arguably desirable, but the next triager should know, and it is not in the body.

Scope

File / group	What it changes	Claimed in body?	Verdict
tests/tests.cmake (4 blocks)	splits 4 monolithic registrations into 15 / 10 / 38 / 9 cases	yes	in scope
tests/audio_engine_rates_test.cpp	--case= selector, STAGE tracing, per-effect filter	yes	in scope
tests/nr_stereo_independence_test.cpp	--method=/--rate= selectors, 77 skips, NNR hard-fail	yes (+ Copilot fix)	in scope
tests/peripheral_auth_dialog_test.cpp	--case= selector; old main tail extracted to checkInteraction()	yes	in scope
tests/verify_qttest_registration.cmake (new)	QtTest inventory guard	yes	in scope; referenced from tests.cmake:2372, not a dead addition
tests/RtlInjectedDevice.h	callbackActive flag + notify_all	yes	in scope
tests/rtl_capture_worker_test.cpp	one-callback-at-a-time clocking, exact-first-window DC assertion	yes	in scope
tests/rtl_spectrum_averaging_test.cpp	500→96 frames, 64 pooled bins	yes	in scope

Everything in the diff is explained by the body. No production code, no workflow, no CHANGELOG.md, no public/protocol surface, no canon implicated (test-only). I confirmed none of the four renamed tests appears in .github/ci-test-gate.txt or any ctest -R pattern in ci.yml, so the frozen gate is genuinely untouched — consistent with Static checks being green.

Socket audit: this PR adds and modifies no socket-owning test. The weather test's ControlledRadarNetwork is a pre-existing in-process QNetworkAccessManager double (no bind/listen/connect) and is untouched; RtlInjectedDevice is an in-process device double. Nothing to record beyond that.

Blockers

None.

Nits (all non-blocking)

• tests/tests.cmake:2377-2378 — the new guard carries the tightest deadline in the diff, in the lane this PR exists to de-flake.

  set_tests_properties(weather_radar_loading_registration_test PROPERTIES
      ENVIRONMENT "QT_QPA_PLATFORM=offscreen" TIMEOUT 15)

  plus TIMEOUT 10 on the inner execute_process in verify_qttest_registration.cmake:10. Launching a TSan-instrumented binary that links Qt Widgets + OpenGL, constructing QApplication via QTEST_MAIN, and tearing it down just to print -functions is plausibly more than 10 s on the weekly runner. The retrofit default at tests.cmake:8125-8128 cannot absorb it — if(NOT _existing_timeout) only fills gaps, so an explicit TIMEOUT is an opt-out of the 300 s ceiling. On breach, execute_process sets _result to Process terminated due to timeout, which is not "0", so the guard raises FATAL_ERROR "QtTest method discovery failed" — a red test that looks like a registration defect but is a clock. The body validates the guard against ASan/UBSan but not TSan. Suggest matching the 30 s the real weather cases keep:

  set_tests_properties(weather_radar_loading_registration_test PROPERTIES
      ENVIRONMENT "QT_QPA_PLATFORM=offscreen" TIMEOUT 60)

  and TIMEOUT 30 on the execute_process.

• tests/audio_engine_rates_test.cpp:590-595 + tests/tests.cmake:168-170 — three audio cases will report PASS on vacuous coverage, while the sibling suite in this same PR returns 77.

  const bool optional = effect == Effect::Nr4 || effect == Effect::Dfnr
      || effect == Effect::Mnr;
  check(optional, "mandatory NR2/RN2 effect must be available");

  On a build without Specbleach / DeepFilterNet / MNR, audio_engine_rates_effects-NR4_test, -effects-DFNR_test and -effects-MNR_test each run exactly one trivially-true assertion and exit 0 — three green CTest names with zero effect coverage. Not a regression (the monolith did the same), but the split is what puts named green checkmarks on it, and the NR split in this very diff gets SKIP_RETURN_CODE 77 for exactly this situation (tests.cmake:986-987). Parity would mean exiting 77 from the optional-unavailable path when a single effect was selected, plus SKIP_RETURN_CODE 77 on the three cases.

• tests/rtl_capture_worker_test.cpp:260-265 — the 7 is correct but undocumented, and the assertion fails rather than retries if it is ever off by one.

  for (int i = 0; i < 7; ++i) {
      check(clockOne(*device), "DC correction clocks an acknowledged partial window");
  }
  check(waitFor([&] { return dcFrames.size() == 1; }),
        "DC correction emits the exact first complete observation");

  I derived it and it holds: callbackBytes{16384} (RtlInjectedDevice.h:23) is 8192 samples, 8 callbacks fill the 65536-sample window the analytic corrected gain below integrates over, and the adoption while loop above consumes exactly one callback — so 1 + 7 = 8. The exact-equality waitFor means that if callbackBytes, the window size, or the number of callbacks the DC transaction waits on ever changes, this does not go flaky — it goes permanently red with a label that blames DC correction. (The preceding check(waitForIdle(*device), "DC fixture drains prior callback permits") is the right guard against the zero-callback case, and I verified serviceCapture() is safe to call manually alongside its m_captureTimer driver since takeResult() is a pop.) Worth one comment stating 7 == kWindowCallbacks - 1.

• tests/rtl_spectrum_averaging_test.cpp:119-157 — the variance metric changed meaning, not just its sample count. Pooling 64 bins per frame mixes across-bin spread into what was a pure across-time variance. Temporal averaging cannot reduce the bin-to-bin component, so variance[2] now carries a floor set by any tilt or ripple in the noise floor over bins 1000–2008. Separately, kWarmupFrames dropped 100 → 64 while high is Probe(100,false), so the deepest averager is sampled before it has filled. Both effects push toward harder to satisfy, and your mutation proof (averaging off → ratios 1.0 → fails) shows the assertion kept its teeth, so this is an observation rather than a defect — but "retaining the variance and mean-power thresholds" understates it: the same thresholds are now applied to a differently-constructed statistic.

• The body's "retaining the existing timeout ceilings per scenario" is per-case accurate but worth one clarifying sentence. Audio went from one 120 s budget to 15 × 120 s, weather from 30 s to 38 cases, and so on. That extra aggregate headroom is the fix; just say so, so nobody reads "existing timeout values remain unchanged" as "same total budget."

What I tried to break

• The registration guard's inventory, which is the load-bearing new claim. weather_radar_loading_test.cpp has 41 methods matching ^ void name(); 4 are _data() helpers, and QtTest's isValidSlot excludes _data-suffixed slots along with init/cleanup/initTestCase/cleanupTestCase (the class declares none of the latter four). 41 − 4 = 37 = exactly the _weather_radar_loading_cases list at tests.cmake:2333-2371, with no duplicates after sort -u. -functions prints methodSignature(), i.e. name(), which the guard's regex matches. The list is complete and accurate today, and the guard is what keeps it so. This is the strongest part of the PR.
• The regex escaping in verify_qttest_registration.cmake:22. The diff as rendered to me showed \(\), which in a CMake quoted argument would collapse to regex capture-group parens and make the guard FATAL_ERROR on every well-formed line. The file in the checkout has \\(\\) — correct. Refuted against the checkout, not the diff.
• Shared mutable state across the newly-parallel cases. One registered test became 9–15 processes that ctest -j can run concurrently against the same profile name. TestSettingsProfile builds QDir::tempPath()/<name>-XXXXXX as a QTemporaryDir and additionally sets AETHER_SETTINGS_DIR under it, so each process gets a private store. No contention.
• Whether --case=interaction can stand alone. The extracted checkInteraction() (peripheral_auth_dialog_test.cpp:1877, 525 lines) opens by clearing the Peripherals document and seeding its own Vkamp/Lp100a values, and main still installs setRemovalConfirmationHookForTest before dispatching any stage. No file-scope mutable globals in the test. It does not depend on the eight groups that used to precede it.
• The selector dispatch arithmetic in audio_engine_rates_test.cpp. --case=effects-bypass carries Effect::None in an std::optional, so selectedCase->effect is truthy via has_value() and the bypass case is not silently skipped; selected("concurrentSourcesAndEffects") is false for every effects-* name while the || selectedCase->effect arm catches them; the full matrix still runs with no argument. The const char* name returned from the initializer_list loops in both parseCase implementations points at string literals, so nothing dangles. Invalid and multi-argument input returns 2.
• The NR 77/failure matrix, including Copilot's confirmed lead. f80ea826 is in the head: failed NnrFilter construction now takes the check(false, ...) branch in both focused and full-suite runs, and --method=nnr without --rate correctly evaluates both rates before main returns 1 — the hard-fail does not short-circuit the second rate. The #else arms compare selection->method == Method::Nr4 etc., which is false when method is nullopt, so a full-suite run on a build lacking an optional method still prints SKIP and continues instead of exiting 77. The if (selection->method) gate on the DFNR/BNR runtime-unavailable paths preserves that too.
• The new callbackActive handshake for a sampling hole. In InjectedDevice::read, --blocks and callbackActive = true happen under one uninterrupted lock hold, so no observer can ever see blocks == 0 && !callbackActive mid-callback; and ++callbacks completes before the relock that clears the flag, so clockOne's compound predicate cannot pass early. This is the right fix for the flake it targets.
• Gate and canon breakage from the renames. Grepped the whole tree: the four old names survive only in prose (docs/audio-engine-rate-domains.md:80, docs/weather-radar-regions.md:268, docs/peripherals-removal-evidence.md:10, docs/4o3a-remote-auth-hardware-evidence.md:208, two test comments, third_party/QGeoView/AETHERSDR-PATCHES.md:132). All of those name the executable, which still exists and still runs unfiltered, so none is now wrong. No -R pattern, frozen-list entry, or script keys on them.

What I could not check: I am headless — no build, no ctest, no cmake on this box, so every finding above is reasoned from the source in /tmp/aetherclaude/pr-6196, not reproduced at runtime. I could not run the registration guard, measure the guard's real startup cost under TSan, verify the 15/10 s timeouts empirically, or confirm the 500→96 frame reduction still clears the variance thresholds on the CI runner. I also could not read run 37243767521, which is the only evidence offered for the two RTL files being in scope.

Recommendation

Approve with nits. This is careful, well-evidenced test work: the split is faithful (every assertion retained, every selector round-tripped), the callbackActive handshake is a genuine fix rather than a sleep, and the registration guard is the right answer to the real hazard a 37-way manual split creates — I verified its inventory is exact. The four nits are all cosmetic or future-proofing except the guard's 10/15 s deadline, which I would fix before merge simply because it is the one new thing in this PR that can itself go red in the weekly lane for a non-defect. Concrete next step: raise the guard's two timeouts, add the one-line derivation comment for the 7, and decide whether the three optional audio effect cases should exit 77 for parity with the NR suite — then land it. Thanks for the mutation proofs; they are what let me take the statistical change on faith rather than guessing.

---

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

[jensenpat]

Verdict

Request changes. Reviewed f80ea82658f0b19d79a3701a5a967e2dfd3377b2. Not merged: GitHub reports mergeable: CONFLICTING against current main (deab0a763), and the two conflicted tests would drop coverage that has landed since this branch.

Issue fit at authorship was right. #6156's remaining sanitizer timeouts and the RTL capture flake were a fair slice, and this branch does not claim to close #6156 or the DecoderAudioModel race. That timeout slice was then fixed on main by #6208 (035a85835): sanitizer builds scale every TIMEOUT by AETHER_SANITIZER_TIMEOUT_SCALE (default 4), and rtl_capture_worker_test feeds one consumed block at a time. The sanitizer lane on that integration was ASan+UBSan 658/658 and TSan 658/658 with zero reports. #6202 stays open only for the intermittent DecoderAudioModel race, which this diff does not touch.

Required checks on this SHA are green (build, check-windows, check-macos, Static checks, Sanitizer option configures, run 37259499078). They ran on 2026-10-05 and do not execute the new split cases; the per-PR gate is the frozen list, and these names are not on it. That is expected and is not why this is blocked.

Scope

File	What it does	Against current main
tests/audio_engine_rates_test.cpp, tests/tests.cmake	15 focused cases, assertions kept	Conflict. Dispatcher and case list omit four stages main runs
tests/nr_stereo_independence_test.cpp	10 method/rate cases; NNR init failure is an assertion; optional DFNR/BNR return 77	Auto-merges. Copilot thread is fixed in this SHA and resolved
tests/peripheral_auth_dialog_test.cpp	9 cases; same checks, per-case settings profile	Auto-merges
tests/weather registration + verify_qttest_registration.cmake	37 QtTest slots plus an inventory guard	List still matches main's 37 test slots (*_data excluded). Auto-merges
tests/rtl_spectrum_averaging_test.cpp	96 frames, 64-bin pool, same inequality thresholds	Auto-merges. Bin indexes 1000..2008 are inside kRtlSpectrumBins (65536)
tests/rtl_capture_worker_test.cpp, tests/RtlInjectedDevice.h	callbackActive plus a fixed 7-callback DC window	Conflict with #6208's feedOne / adopt / nextFrame and rtl_capture_worker_slow_device_test

No production, workflow, or frozen-gate edits. Socket preflight: none of the eight files bind, listen, or construct a Qt socket.

Findings

1. tests/audio_engine_rates_test.cpp dispatcher drops four stages that main always runs. origin/main calls negotiatedSpeakerOutput, negotiationOpenFailure, negotiationLifetimeAndCoalescing, and auxiliaryAcrossNegotiation between bandwidthAndMono and transitionsAndRejection. This case table and the tests.cmake foreach have no names for them. The merge stops on that main(). Taking this side, or keeping the CMake split without those four names, removes them from the full run and from CTest. Rebase has to register each one as its own case and call it from the unfiltered path.

2. tests/rtl_capture_worker_test.cpp replaces the lockstep feeder #6208 added for this same flake. main already adopts on one consumed callback and adds rtl_capture_worker_slow_device_test (AETHER_TEST_RTL_CALLBACK_DELAY_MS=30) because a fixed block count is what went red under TSan. This DC section still clocks a fixed 7 callbacks (callbackBytes 16384 → 8192 samples, 8 callbacks fill the 65536-sample window, and the adopt loop is written to take the first). That count is exact only while the window size and the adopt loop stay put, and it conflicts with adopt() / nextFrame(). Keep #6208's feeder and the slow-device case. callbackActive can stay only if something still waits on it; the delay field #6208 added in RtlInjectedDevice.h already merges beside it.

Nits

• Pooling 64 bins per frame mixes across-bin spread into what had been one bin's across-time variance, and high is Probe(100) while warmup is 64 frames. The inequalities can still fail if averaging is removed; the statistic is no longer the one those thresholds were written for.
• verify_qttest_registration.cmake compares sorted lists, so a duplicate expected name fails, but REMOVE_ITEM then reports both sides empty. The 10 s execute_process timeout sits under a 15 s CTest ceiling.
• Focused optional audio effects (effects-NR4, effects-DFNR, effects-MNR) print SKIP and still exit 0. NR's optional methods use 77. Fine for the unfiltered suite; a focused case can pass without constructing that effect.
• Splitting multiplies the aggregate budget (15 × 120 s audio, 38 × 30 s weather, and so on). #6208 would scale those again by 4 in sanitizer builds. Say that in the body if the split is still worth landing.

Verification

Read the full diff, both conflicted main functions, the weather slot inventory, #6208, and the #6202 green-lane note. git merge-tree against origin/main deab0a763: conflicts only in audio_engine_rates_test.cpp (includes plus main()) and rtl_capture_worker_test.cpp (DC adopt). No local build or CTest. The author's ARM64 and ASan runs are on this parent, not on a tree that contains #6208 or the four negotiation stages.

Non-blocking follow-up

If the split is redone: the registration guard covers weather only. The same inventory check on the audio, NR, and peripheral selectors would catch the next stage added the way these four were. The pooled noise ratios are still only inequalities; a printed ratio from a known seed would show whether the 64-bin pool stays near the old single-bin result. Neither is a condition for a later rebase.

[jensenpat]

Rebased onto deab0a763 and approving 8c4a3fa01708e9ca827ad1d727fd8e1f2f538c12.

The two merge conflicts are resolved in the rebase:

• The audio dispatcher and tests.cmake case list now include negotiatedSpeakerOutput, negotiationOpenFailure, negotiationLifetimeAndCoalescing, and auxiliaryAcrossNegotiation, in the same order main runs them.
• rtl_capture_worker_test keeps #6208's feedOne / adopt / nextFrame path and rtl_capture_worker_slow_device_test. The fixed 7-callback clock and callbackActive are not in the branch.

Local Qt 6.12 ARM64 CTest, QT_QPA_PLATFORM=offscreen, 79 selected tests: 75 passed, 4 optional DFNR/BNR skips, 0 failed (31.65 s). That set is the split audio, NR, weather, and peripheral cases plus both RTL capture tests and rtl_spectrum_averaging_test. The app target was not built.