Code review — high-effort recall pass (8 finder angles → verify)

Verdict: clean, well-tested migration — no confirmed functional regression that reaches a live consumer. This finishes moving all 13 display pan keys + 3 waterfall keys out of the now-deleted applyPanStatus/applyWaterfallStatus into FlexBackend decoders behind 5 typed signals + a panState extension bundle. A key-by-key audit confirms every deleted key is re-homed with identical guards (loop mutual-exclusion, client_handle !=0 && changed, fps/line_duration ok-guards), identical side-effects (setDbmRange change-gated, PerfTelemetry, antListChanged), and it correctly single-sources the waterfall center/bandwidth onto decodePanCenterBandwidth — closing the #4063 dual-parse gap. Nice to see real decode tests (NaN sentinel + client_handle fail-closed). The #4063 dangling-m_flexBackend finding is fixed here (~RadioModel now nulls the alias, citing the review).

Four findings inline. Only the two parse-guard items are worth acting on — cheap, and they bring the new boundary decoders in line with their own siblings and FlexLib's fail-closed semantics. #3/#4 are latent (no live consumer / not synchronously reachable).

Cleanup, non-blocking: the m_panadapters.value(panId); if(!pan) pan=activePanadapter(); idiom is now duplicated ~7× — a one-line private resolvePan(panId) helper would single-source the addressing policy (commented inline). The per-status shape (6 decode passes + up to 5 typed-signal dispatches, each re-resolving the pan; three coexisting "unchanged" conventions -1.0/NaN/present-only) is largely inherent to the RFC's approved per-field-signal design — noted as a future consolidation, not a blocker.

Thanks @ten9876 — this is a clean, faithful completion of the PanadapterModel conversion. I traced every removed field through the new backend decode → normalized signal → setter chain and the semantics hold up: the >= 0.0 / NaN "unchanged" sentinels, the setDbmRange gating on setRange()'s changed-return, the fps "reported-always / changed-on-change" quirk, loop A/B mutual exclusion, and the #3977 client_handle re-stamp are all preserved verbatim. All decode references to applyPanStatus/applyWaterfallStatus are gone (only comments remain), and CI is green across build/macOS/Windows/CodeQL/engine-boundary. Nice work.

A few notes:

1. The two guard gaps you flagged are real — worth closing (low severity).
Confirmed both against the diff:

• decodePanRange: kvs.value("min_dbm").toDouble() / max_dbm are unguarded. A present-but-malformed field parses to 0.0 (not NaN), so setRange() — which only skips NaN — applies 0 as a real level and drives setDbmRange(…, 0, …), collapsing the vertical scale.
• decodePanRfGain: kvs.value("rfgain").toInt() unguarded → setRfGain(0) overwrites the displayed gain.

You're right that neither is a regression (old applyPanStatus used the same unguarded toFloat()/toInt()), but the asymmetry is now stark within a single file: decodeWaterfallLineDuration and decodePanExtensions' wnb_level both guard with toInt(&ok) and the WNB one explicitly cites Principle VII as the reason. Since this decode is now the formal validation boundary, I'd apply your two suggested &ok guards for consistency. min_dbm is the higher-value fix (scale collapse vs. a plausible 0 gain).

2. Latent-regression fix on the GUI center nudges — verified, good catch.
The six applyPanStatus({{"center",…}}) → setCenterBandwidth(…, -1.0) swaps are correct. After #4063 moved center/bandwidth decode out of applyPanStatus, those calls were genuinely no-ops, and setCenterBandwidth's >= 0.0 gate means the -1.0 bandwidth sentinel leaves bandwidth untouched — exactly the center-only nudge those call sites intend.

3. Minor: stale comment. RadioModel.cpp:4261 (in noteForeignPanWriteIfAny) still reads "Note applyPanStatus re-stamps the pan when the radio reassigns it…". The mechanism is preserved, but the re-stamp now lives in applyStateExtension (the panState extension), so the comment names a method that no longer exists. Worth a one-line touch-up while you're here since it documents the #3977 ordering invariant.

None of these block; (1) and (3) would be nice to fold in before merge.

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

Reviewed the full diff, traced the removed applyPanStatus/applyWaterfallStatus decode against the new backend decoders field-by-field, and read the tests. This is a clean, well-structured completion of the PanadapterModel conversion — the model now genuinely holds zero wire decode, the per-field semantics (fps ok-guard + always-report quirk, loop A/B mutual exclusion, preamp no-emit, -1/NaN sentinels, #3977 client_handle fail-closed re-stamp) are preserved verbatim, and aetherd_pan_decode_test pins each one including the zero-handle fail-closed case. All 7 checks green. Nice work, @ten9876.

A few notes, none blocking:

1. The two self-flagged unguarded parses are worth fixing (Copilot findings — both valid).
decodePanRange's min_dbm/max_dbm .toDouble() and decodePanRfGain's rfgain .toInt() have no ok-guard, while the sibling decodeWaterfallLineDuration in this same PR guards with toInt(&ok). You're right that neither is a regression (old applyPanStatus was equally unguarded), but now that decode sits squarely on the Principle VII validation boundary and FlexLib itself fail-closes (TryParseDouble/int.TryParse + continue), the asymmetry is the thing to close. The min_dbm case has the higher practical impact: a malformed present field parses to 0.0 (not NaN), setRange() only skips NaN, so 0 is applied as a real level and setDbmRange(…, 0, …) collapses the vertical scale — a more visible failure than a bad rfgain clamping to 0. The dbm lambda suggestion you posted is the right shape; I'd take both.

2. Minor unmentioned deviation in the pan-null path for panadapterLevelChanged.
The old inline block emitted panadapterLevelChanged even when no pan resolved (falling back to the wire value or -130/-20 defaults). The new ctor-wired panRangeChanged handler does if (!pan) return; before the emit, so with zero panadapters present the level signal no longer fires. This is a very low-severity edge case (only reachable when both the addressed and active pan are null), but it's a behavioral difference the "pan-null path preserved" claim in the description doesn't cover — worth a one-line acknowledgement or restoring the fallback emit if any global scale listener depends on it.

3. The #4063 regression fix is a legitimate, well-reasoned catch. I confirmed on main that applyPanStatus no longer decodes center/bandwidth, so the six GUI applyPanStatus({{"center",…}}) nudges were indeed silent no-ops since #4063 — swapping to setCenterBandwidth (with -1.0 correctly skipped by the >= 0.0 guard for the center-only callers) restores the optimistic repaint. Worth calling out that routing the waterfall center/bandwidth path through decodePanCenterBandwidth now also emits panadapterInfoChanged, which the old model-only infoChanged path did not — harmless (arguably more correct), just an extra emit to be aware of.

Overall this is in good shape; only #1 is worth acting on before merge, and it's the fix you already drafted.

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

Reviewed against the current upstream/main merge result.

No blocking findings from my pass. The earlier review items appear addressed in the current code: dBm/rfgain parse guards are in place, WAN teardown hardening is present, resolvePan() is added, and the dropped no-pan panadapterLevelChanged behavior is documented as intentional.

One non-blocking cleanup nit: a couple of comments still reference the removed applyPanStatus() path near the ownership/decode code. Runtime behavior looks correct, but updating those comments would make the new 2.3 seam easier to trace later.

Local validation passed:

• git diff --check upstream/main...upstream/pr/4065
• cmake --build build --target aetherd_pan_decode_test panadapter_model_rx_antenna_test -j8
• ctest --test-dir build -R '^(aetherd_pan_decode_test|panadapter_model_rx_antenna_test)$' --output-on-failure
• python3 tools/check_engine_boundary.py --strict
• python3 tools/check_a11y.py on the touched GUI files
• cmake --build build --target AetherSDR -j8

Independent red-team pass (second review round, 30b3e65c)

Traced this against actual source (not just the diff) rather than taking the prior passes' claims at face value — three things held up under independent verification, one small gap confirmed, and two process items nobody's raised yet.

What I independently verified (not just re-asserted)

• #3977 ownership-ordering. Grepped noteForeignPanWriteIfAny's call site (RadioModel.cpp:3662) — it fires in the raw per-line status dispatch, before any object-specific routing reaches handlePanadapterStatus. Relocating the client_handle re-stamp to the end of the decode chain (inside decodePanState/applyStateExtension) doesn't cross that boundary. Ordering invariant holds.
• "Synchronous DirectConnection" claim. FlexBackend's ctor does moveToThread() on m_panStream/m_connection, which initially read as a contradiction — but those are FlexBackend's owned wire objects; the FlexBackend QObject itself stays on the main thread and is what emits panRangeChanged/panRfGainChanged/etc. Chain is synchronous as claimed.
• Commit signing. All three commits verify true/valid via the API.

Confirmed gap (low severity, already self-disclosed — independently reproduced)

panRangeChanged's ctor handler does if (!pan) return; before emitting panadapterLevelChanged, where the old inline code emitted it unconditionally (wire value or -130/-20 defaults) even with zero panadapters resolved. I grepped src/ for panadapterLevelChanged — one emit site, one declaration; the two other hits (MainWindow_Session.cpp, MainWindow_Wiring.cpp) are stale comments, not connect()s. So this is confirmed dead-signal territory today, not a live regression — but "pan-null path preserved" in the description should read "preserved except the now-provably-dead no-pan case."

CI: green claims are stale, not false

Both prior review passes state "all 7 checks green" / "CI is green across build/macOS/Windows/CodeQL/engine-boundary" — posted 00:22–00:23. The fix commit 30b3e65c wasn't pushed until 00:27:16. As of this pass, on the actual current HEAD:

Not a red flag on the code — just: nobody has actually watched CI go green on 30b3e65c yet. Please confirm both before merge.

Process items — need resolution before this can be approved, not just noted for later

1. No issue reference. Body has no Fixes #/Closes #; closingIssuesReferences is empty. If this is tracked informally against #3849 + the touchpoints doc rather than a dedicated issue, that's fine, but CONTRIBUTING.md's one-issue-per-PR rule expects an explicit reference — worth adding even if retroactive.
2. Self-approval deadlock. This diff touches src/gui/MainWindow.cpp, which CODEOWNERS gates Tier 1 to @aethersdr/maintainers — currently just @ten9876 per the file's own header comment. The author is @ten9876. GitHub hard-blocks self-approval on every tier, so as filed there is currently no one who can satisfy this PR's Tier-1 gate. This holds regardless of the hunk being a 3-line mechanical applyPanStatus→setCenterBandwidth swap — CODEOWNERS matches on path, not diff size. Options: add a second maintainer to the team, use an admin merge override, or split the MainWindow.cpp line out of this PR so it lands Tier-3-only.

Separate from this PR, worth a maintainer look

CONTRIBUTING.md's "Reviews and merging" table lists src/core/RadioModel.{h,cpp}, src/core/AudioEngine.{h,cpp}, and src/core/PanadapterStream.{h,cpp} as Tier-1 maintainer-only. The actual .github/CODEOWNERS doesn't gate any of them (only MainWindow.{h,cpp} is Tier 1), and the documented path is stale besides — RadioModel lives under src/models/, not src/core/. This PR happens to touch RadioModel.cpp's destructor/teardown ordering (exactly the class of change burned before in #502/#4063) but only requires Tier-3 sign-off under the enforced gate. Not blocking this PR, but the docs/CODEOWNERS mismatch should get reconciled.

Net: the PanadapterModel conversion itself is sound and the two guard fixes from the last round are exactly right. Nice catch on the dtor UAF window, too — that's a real one, and the fix (null m_flexBackend before reset, disconnect m_wanConn) is the correct shape. Once check-windows/CodeQL finish clean on 30b3e65c and the Tier-1 gate has a path to clear, this is ready.

Reviewed the full diff against main (checked out the branch to trace the removed decoders). This is a clean, careful conversion — the field-by-field behavior preservation is real, not just claimed. All 7 checks green, 65/65 tests. Nice work, @ten9876.

Verification notes:

Field coverage is complete. Every field the old applyPanStatus/applyWaterfallStatus decoded (client_handle, min/max_dbm, rfgain, pre, wide, loopa/b, fps, ant_list, rxant, waterfall, daxiq_channel, center/bandwidth) maps to exactly one new decoder, with the per-field semantics preserved verbatim — including the fps reported-vs-changed split, the loop A/B mutual exclusion, the pre no-emit, and the line_duration ok-guard. No decode responsibility was dropped.

The two Copilot fail-closed findings are already fixed in this diff. decodePanRange's dbm lambda and decodePanRfGain both carry the toDouble/toInt(&ok) guard now, so a malformed present field carries NaN / is skipped rather than collapsing the scale or overwriting gain with 0. Confirmed valid concerns, correctly resolved — good catch closing the asymmetry with decodeWaterfallLineDuration.

setCenterBandwidth(freq, -1.0) GUI swaps are behavior-preserving — setCenterBandwidth guards each arg on >= 0.0, so the -1.0 bandwidth sentinel is a clean "center only", matching the old single-key applyPanStatus({{"center",…}}). And these genuinely restore the optimistic pre-echo nudge that #4063 silently turned into no-ops — well spotted.

Destructor hardening is a good defensive catch. Moving m_flexBackend = nullptr ahead of m_backend.reset() and severing m_wanConn closes a real UAF window now that WAN-delivered status flows through decodePan* → m_flexBackend.

Two small, non-blocking items:

1. Intentional-but-new legacy emit on the waterfall path. The old applyWaterfallStatus synced center/bandwidth via the per-pan infoChanged only. Routing the waterfall path through the shared decodePanCenterBandwidth now also fires RadioModel's legacy panadapterInfoChanged (the ctor handler emits it). This looks like the deliberate single-sourcing of the #4063 gap and is benign (same authoritative values MainWindow already consumes), but it is a new signal emission on that path — worth a one-line confirmation it's intended rather than incidental.

2. Stale comment at src/gui/MainWindow_DigitalModes.cpp:1167 still says "applyPanStatus updates the local model immediately" though the code below now calls setCenterBandwidth. Trivial, fix if you touch the file.

Nothing here blocks. The extension-channel split and the resolvePan single-sourcing read well and set up the remaining 2.3 mixed-model conversions cleanly.

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