Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Thanks for this, @rfoust — this is a substantial, carefully-built feature. The clean-room GCC-PHAT + envelope-fallback estimator is well-structured, the bounded automation PCM capture is nicely guarded (frame-aligned, byte/duration-capped, double-checked under lock), settings follow Principle V (one nested ReceivePresentationSync JSON key with flat-key legacy migration), the diagnostics caches are properly std::atomic, and the focused tests + green CI (incl. the a11y rerun) are all there. Nice work.

One concern worth a look before merge:

Possible data race in AudioEngine::setReceivePresentationDelays (missing m_dspMutex)

src/core/AudioEngine.cpp:800 — this method mutates the shared RX buffers directly: it calls trimReceivePresentationBuffers(...) on m_rxBuffer/m_rxPackets/m_rxOutputBuffer, the legacy Kiwi buffers, and each m_externalKiwiSources entry's rxBuffer/rxPackets/outputBuffer, and also iterates the m_externalKiwiSources vector — all without acquiring m_dspMutex.

Those same buffers are appended to under m_dspMutex from the audio/network path (feedAudioData, feedKiwiSdrAudioData, processMixedRxAudioData), and setReceivePresentationDelays is driven from the GUI thread via MainWindow::syncReceivePresentationDelaysToAudioEngine. A GUI-thread QByteArray::remove/std::deque mutation racing with a network-thread append is UB (heap corruption / crash).

The asymmetry is the tell: the two sibling methods — resetReceivePresentationAudioBuffers() (:887) and resetReceivePresentationAudioBuffersForKiwiSource() — both take std::lock_guard<std::recursive_mutex> dspLock(m_dspMutex); before touching the very same buffers. setReceivePresentationDelays looks like it just missed it.

Suggested fix: wrap the buffer-trimming/source-iteration body in the same std::lock_guard<std::recursive_mutex> dspLock(m_dspMutex); (the atomic exchanges for the delay values can stay outside if you prefer, but the m_externalKiwiSources iteration and all trimReceivePresentationBuffers calls need to be inside it). It's recursive_mutex, so the nested updateRxBufferStats() call is fine.

Minor / non-blocking

• The Auto-Assist estimate (m_receiveAudioDelayEstimator.estimate) runs synchronously on the post-DSP-audio signal handler (GUI thread). It's gated by kReceiveSyncEstimateIntervalMs and a min-sample check, so this is fine — just flagging that the FFT cost lands on the GUI thread if the window/interval are ever tuned down.
• tests/antenna_alias_test.cpp rx_ant_list coverage is slightly tangential to "Kiwi receive sync," but it's test-only and pairs with the slice rxant/rxsource automation additions, so no objection.

Everything else looks clean. Happy to re-look once the locking question is addressed.

_{🤖 aethersdr-agent · cost: $6.0013 · model: claude-opus-4-8}

Thanks @rfoust — this is an impressively thorough, well-structured contribution. I reviewed the DSP core, settings persistence, automation surface, and the GUI integration. CI is fully green (build/macos/windows/CodeQL/analyze all pass). Summary: looks good to merge after considering a couple of minor points below.

What's solid

• Conventions honored. Receive-sync prefs are stored as one nested-JSON object under AppSettings["ReceivePresentationSync"] with clean migration from the legacy flat keys (receivePresentationSettingsToJson / …FromJson / removeLegacyReceivePresentationSettings) — exactly Principle V. No new flat-key calls, no QSettings.
• DSP core is clean and self-contained. Standard iterative radix-2 FFT, GCC-PHAT with a normalized-correlation refine plus an envelope-feature fallback, bounded offsets, and kEpsilon-guarded denominators throughout. ReceivePresentationSync.cpp/.h link only against Qt6::Core, and receive_presentation_sync_test.cpp gives good focused coverage of the pure logic (delay breakdown, manual/auto offsets, surface gating).
• Defensive integration. resolveReceiveSyncTarget() and friends null-check m_kiwiSdrManager, every SliceModel*, and sliceMayBelongToUs() before use. Ambiguity (multiple audible Flex/Kiwi) is explicitly modeled rather than guessed.
• Audio-capture path is careful. captureAutomationAudio() does an atomic relaxed fast-path check before taking the mutex (keeps the audio callback cheap), then bounds by both duration and a 64 MB byte cap. doAudioCapture null-checks m_audioEngine and handles file-open/mkpath/short-write failures at the FS boundary. Nicely done.

Minor points (non-blocking)

1. Estimator cost on the GUI thread. runReceivePresentationAutoAssist() runs on the GUI thread (fed via the queued receivePresentationOutputAudioReady signal). With a 5 s window at 12 kHz, nextPowerOfTwo(60000+60000) = 131072, so each pass does three 131072-point double FFTs plus a ~72k-sample offset scan. It's throttled to 1 Hz, so likely only a few ms, but on slower hardware it could show up as periodic UI jitter. Worth confirming it stays smooth in practice; if it ever bites, moving the estimate() call to a worker (QtConcurrent) would be the natural fix. Not a merge blocker.

2. audioCapture "AETHER_AUTOMATION-only" claim. The docs state capture is active only in an AETHER_AUTOMATION=1 process. That holds today because AutomationServer is the sole caller and is itself gated — but startAutomationAudioCapture() has no internal guard, so the invariant is enforced one layer up. Fine as-is; just flagging in case the engine API ever gets another caller.

Constitution principles IV (clean-room) and V (settings) both look respected. Nice work — no changes required from my side.

_{🤖 aethersdr-agent · cost: $4.2390 · model: claude-opus-4-8}

Approved. Reviewed the DSP core, threading, settings, automation surface, and GUI integration on the current head (8bf96fc):

• Both prior review concerns are resolved in 8bf96fc:
  • The GUI/audio-thread data race in setReceivePresentationDelays is fixed — m_dspMutex now guards the buffer trims + m_externalKiwiSources iteration, matching the sibling reset methods.
  • The GCC-PHAT estimate() now runs on QtConcurrent's thread pool (not the GUI thread), guarded by an in-flight flag, a generation counter for stale-result discard, a 1 Hz throttle, and input-vector copies — no contention with the real-time audio path. QtConcurrent is the right home (the estimate spans both Flex+Kiwi audio; the Kiwi/AudioEngine workers must stay real-time).
• Clean-room (Principle IV) — standard radix-2 FFT + GCC-PHAT + normalized-correlation refine + envelope fallback, kEpsilon-guarded; ReceivePresentationSync.{h,cpp} link only Qt6::Core, with a 425-line focused unit test.
• Principle V — one nested AppSettings["ReceivePresentationSync"] JSON key with flat-key legacy migration.
• No TX/keying path (RX audio+visual sync; Principle VI N/A). Defensive null-checks throughout; ambiguity (multiple audible Flex/Kiwi) explicitly modeled with held/coasting locks.
• audioCapture bounded (duration + 64 MB cap), atomic fast-path before the mutex, FS-boundary error handling, reachable only via the AETHER_AUTOMATION-gated bridge.

CI green on all 6. Excellent, carefully-built feature — thanks @rfoust.