[tropo1234]

Summary

The non-blocking follow-up @jensenpat asked for on #5920: "a socket-free routing
test could pin Flex wire delivery, ANAN typed dispatch, and Icom's no-command
path at the shared GUI/model seam." Tests only — no file under src/ changes.

It also closes the gap that review exposed twice. #5920's own description said
Icom declines the typed call and is then stopped by the hasCommandPlane()
guard; the approving review corrected it — shapesDisplayRatesLocally() covers
every non-Flex backend, so Icom accepts the typed width and eats it in the
default no-op. The behaviour was right and the story was wrong, and nothing in
the build could tell. Now one assertion does.

Two halves, because the decision cannot be called from a test. It lives in
MainWindow::sendPanDimensionsToRadio(), and no test target in this repo
constructs a MainWindow. Nor can the block move down into RadioModel, which
would have made it directly testable: the fallback is raw Flex wire text and
tools/check_command_plane.py is a per-file, shrink-only ratchet with
src/models/RadioModel.cpp frozen at its baseline count, so relocating one
sendCommand() there fails the ratchet. That is what
RadioModel::requestLocalPanWeightedAverage's own comment means by "moving it in
here would add a raw command above the seam". So the call site stays where it is
and the test pins it from both sides.

Part A — behaviour. Every family built through the production family switch
(rebuildBackendForTest() calls the same rebuildBackendForFamily() that
connectToRadio() calls, minus the dial). No socket, no device, no radio.

Family	Typed seam	Command plane	Channel used
Flex	declines	yes	the xpixels=/ypixels= text, exactly as before
ANAN	accepts	no	the typed call
HL2	accepts	no	the typed call, value ignored
Icom	accepts	no	neither — stopped at the seam, not by the guard
Sim	accepts	yes	the typed call; the only family with both open

Plus the dispatch itself: the point count reaches the backend unchanged, and
addressed with the backend's pan id rather than the model key — a width
addressed 0xe1000000 is a width no backend can resolve to a receiver.

Part B — the call site, read as text. A brace matcher that knows string
literals, char literals and both comment forms, so a { in either cannot end
the body early and the block's own comment is not read as code. It pins that the
condition is those two predicates, negated and &&-joined in that order, that
the seam gets panLocalSpectrumPointsFor(sw) while the wire keeps
panXpixelsFor(sw)/panYpixelsFor(sw), and that the fallback keeps the Flex
spelling and its .arg() order.

The composition is the point, not a workaround for the two halves. A
reviewer tidying the condition into
hasCommandPlane() && !requestLocalPanPixelWidth(...) reads identically and
satisfies every stated requirement of #4448 — and because ANAN has no command
plane, it short-circuits before the seam call and the ANAN width silently stops
reaching the analyzer. The panadapter goes soft again with nothing red. Part A
supplies "ANAN has no plane", Part B supplies "the seam is first", and neither
alone closes it.

Two things a reviewer may want to rule on differently, both deliberate:

1. A text check on a GUI call site rather than refactoring for testability.
   The alternative that would let a test call the decision is moving the block
   into RadioModel, which the command-plane ratchet refuses; extracting a
   predicate helper that MainWindow then calls would make the test pass
   through a function whose only caller is the thing being tested. The
   limitation is real and stated in the file: it is a brace matcher and a
   regular expression, not a compiler.
2. Five families in the table where the review named three. HL2 and Sim cost
   two lines each, and Sim is the one family where both channels are open —
   which is the case that makes the ordering requirement general rather than an
   ANAN quirk.

Constitution principle honored

Principle VIII — evidence over assertion. A claim made twice in #5920's
discussion, corrected once by a reviewer reading the source, is now a check that
fails if the claim stops being true.

Test plan

• Local build passes (cmake --build build)

• Full local ctest

• CI

• Every behavioural assertion has a mutation run behind it, each edit
  applied to real production source and rebuilt on its own:

  | Mutation | Assertion it fails |
  |---|---|
  | the call site's two halves swapped | the seam-first check |
  | `requestLocalPanPixelWidth` narrowed to `m_family == "anan"` | the HL2 and Icom rows |
  | `hasCommandPlane()` forced true | the three no-plane family rows plus the no-backend row (four assertions) |
  | `shapesDisplayRatesLocally()` widened to include Flex | the Flex row |
  | `backendPanIdFor()` dropped from the seam call | the pan-id row |
  | the `isEmpty()` guard dropped | the empty-pan-id row |
  | the seam accepting the width and forwarding nothing | the point-count and pan-id rows |
  
  The first needs no rebuild at all, which is worth stating: the text half
  reads `MainWindow_Wiring.cpp` at run time, so it catches a call-site
  regression in a file the test does not link.
  

• Part B's nine controls run on every invocation — the guard dropped,
  the halves swapped, the negation dropped, && weakened to ||, the seam
  handed the raw device width, xpix/ypix transposed, the wire fields
  renamed, the wire fed the crop-widened count, and a second unguarded send
  added. Each must be rejected, and a faithful copy of the real shape must
  be accepted, so the analyser cannot report health by never matching.

• Three checks are labelled controls, not claims, and deliberately have
  no mutation behind them: that every family built, that the fixture's two
  pan ids actually differ (that one exists so the pan-id assertion is not
  comparing a value to itself), and that the source file opened.

• Not exercised on a radio, and it should not be: the test opens no socket
  and addresses no device. feat(anan): give the panadapter one point per screen pixel #5920's own ANAN-G2 bench results stand.

Checklist

• Commits are signed
• No new flat-key AppSettings calls. None added.
• Code is clean-room (Principle IV). Nothing was read but this repo.
• All meter UI uses MeterSmoother. No meter UI touched.
• Documentation updated if user-visible behavior changed. No user-visible
  behaviour changes — one new test target, no src/ edit.
• Security-sensitive changes reference a GHSA. Not applicable.

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

No linked issue, and none is needed: this is the non-blocking follow-up @jensenpat asked for on #5920, which the author agreed to land separately (#5920 comment, 14:53 today). GOVERNANCE.md wants an RFC for architectural change and an issue for bug fixes; a test-only PR pinning an existing seam is neither. The stated goal — pin Flex wire delivery, ANAN typed dispatch and Icom's no-command path at the GUI/model seam — is what the diff does, and the header's central claim is one I could check and confirm: shapesDisplayRatesLocally() is m_backend != nullptr && m_flexBackend == nullptr (RadioModel.h:1563), only AnanBackend overrides setPanPixelWidth (IRadioBackend.h:451 is a no-op default), so Icom does accept the typed call and eat it in the default no-op. The account given twice in #5920's thread ("Icom returns false from the seam … and is then stopped by your guard") was wrong, and this test's assertion is right.

Scope

File	What it changes	Claimed by the title/body?	Verdict
tests/pan_dimension_routing_test.cpp	New socket-free test: Part A reads the two call-site predicates per family through the production family switch; Part B reads the call site as text	Yes	In scope
tests/tests.cmake (+16)	Registers pan_dimension_routing_test, AETHER_SOURCE_DIR define, comment	Yes	In scope

Everything in the diff is explained by the stated intent. No product code touched, no CHANGELOG.md entry, no ci.yml step (correct — AGENTS.md:253 forbids one; the unconditional add_executable/add_test means full-suite.yml and sanitizers.yml pick it up from declaration). No AETHER_SETTINGS_CONSUMERS entry, which matches the sibling backend_family_switch_test with the identical link line. No new public surface, no deletions, no UI or default changed — §6 preference check is trivially clean.

Socket-test disclosure. The PR adds no socket test. Part A does construct the real FlexBackend and SimBackend, which construct a RadioConnection and a PanadapterStream on worker threads (FlexBackend.cpp:45-57), and PanadapterStream::init() constructs a QUdpSocket (PanadapterStream.cpp:95-99). Nothing binds, listens or connects — the socket is an inert object until a bind path runs — and this is the same exposure the already-registered backend_family_switch_test has. Recording it rather than calling it a finding.

Blockers

None.

Nits (non-blocking)

• The && hasCommandPlane() conjunct is not falsifiable by any row in Part A, and Part B pins it in place anyway. Details and a suggested wording in the inline comment on line 385. Short version: for anan/hl2/icom the seam short-circuits first so the guard never runs, and for flex/sim FlexBackend's RadioConnection is created in the constructor and cleared only in the destructor (FlexBackend.cpp:54, :103), so hasCommandPlane() is always true when it is reached. The one state where the guard actually blocks a send is m_backend == nullptr, and routingFor() returns early on exactly that state.
• No profile.isValid() check (line 355). TestSettingsProfile's constructor early-returns having redirected nothing if the QTemporaryDir fails, and Part A reaches AppSettings (PanadapterStream.cpp:113), so the failure mode is "runs against the real settings store" rather than "fails". 48 of the 71 tests using the profile guard it, including backend_family_switch_test.
• The scanner does not know raw string literals. A future R"(...)" with an unbalanced brace in that function makes functionBody() return empty and the test FAIL rather than pass vacuously, so this is fail-loud and fine as-is — worth one line next to the state enum (line 206) so the next reader doesn't have to derive it. Relatedly, kSeamFirstGuard requires the sendCommand to be the guarded block's first statement, so adding a qCDebug inside that block fails a test that isn't about behaviour. That brittleness is inherent to the approach the file already discloses; just naming the specific edge.

What I tried to break

Findings below are reasoned from code and from re-implementing the test's own logic — I have no build and ran nothing from this PR.

• "Part B could report health because it never matches anything." Re-implemented functionBody() (comment/string/char states, brace depth, simplified()) and all four regexes in Python and ran them against the real src/gui/MainWindow_Wiring.cpp at this head: body scanned to the true closing brace at line 2882 with depth 0, and seamFirst, wireSpelling, xpixFromDeviceWidth, ypixFromDeviceHeight all true with seamCalls == planeChecks == wireSends == 1. Then ran the same analyser over kRealShape and all nine controls: reference healthy, all nine mutations rejected (each from string applies), and the no-function source reads unparsed. The two traps work — the } in the comment and the "} {" literal neither end the body early nor add a phantom send. Part B is neither vacuous nor a spelling checker.
• "The test won't compile / the mock is stale." IRadioBackend has exactly 11 pure virtuals (capabilities, connectRadio, disconnectRadio, isConnected, four slice setters, setPanCenter, setKeying, invokeExtension); WidthRecordingBackend overrides all 11 and nothing else.
• "Part A's five-family table is asserted, not measured." Walked makeBackend() and setupBackend() (RadioModel.cpp:775-960): flex → m_connection = flex->connection() and m_flexBackend = flex; sim → m_connection set, m_flexBackend = nullptr; anan/hl2/icom → pure seam backends, neither set, and teardownBackend() nulls m_connection at :2036 before the rebuild. Every row follows.
• "Calling a real backend's seam unconnected will crash or write to hardware." AnanBackend::setPanPixelWidth ignores panId, returns on pixels < 2, and null-checks m_dsp before the queued invoke — safe with no device. HL2 and Icom hit the interface's no-op.
• "The pan-id translation check compares a value to itself or asserts a one-way map." neutralPanIndexFor() fills m_backendPanIdByIndex alongside the forward map (RadioModel.cpp:10345-10371), and backendPanIdFor() reads the inverse, so recorder->panIds.value(0) == "test-2" is a real round trip, not an echo.
• "The PR fixed one of several xpixels= senders." Two siblings exist — RadioModel.cpp:6021 and MainWindow.cpp:11211, both xpixels=1024 ypixels=700. Both are inside display panafall create reply callbacks, and createPanadapter() returns early for any non-Flex backend at RadioModel.cpp:5992, so neither is reachable on a family that lacks a command plane. No unfixed sibling; Part B's "exactly one xpixels= send" is correctly scoped to the function.
• Green CI is real but narrow. All five checks pass on a659be4 (build, check-macos, check-windows, Static checks, sanitizer configure). None of them runs this test — the per-PR gate is the frozen ci-test-gate.txt allow-list — so "it compiles and links" is what green proves here. Part A's runtime behaviour (five real backend construct/teardown cycles per process, including two with worker threads) is unexercised until full-suite.yml runs post-merge; I could not check it, and the mutation runs behind Part A listed in the header are the author's, not reproduced by me.

Recommendation

Approve with nits. This is the rare source-text test that earns its keep: I reproduced its analyser independently and it both accepts the real call site and rejects every one of nine mutations, the Icom row corrects a factual error that shipped twice in #5920's discussion and that I confirmed was an error, and the scope is two test files with nothing smuggled. The three nits are all small and none needs a re-review — the one worth acting on before merge is the line-385 wording, because as written the test teaches a reader that the command-plane guard protects Icom when the seam is what actually does it.

---

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

[tropo1234]

All three nits are fixed.

1. The && hasCommandPlane() term. Correct, and the sharper half of the
point is the one I'd missed: Part B pins a term in place that no Part A row shows
doing any work. Took the sixth-row option rather than only rewording, because
rewording documents the gap instead of closing it.

Not via rebuildBackendForTest("rtl"), though: this build has
ENABLE_RTL:BOOL=ON, so that family builds a real backend and the row would skip
on the machine it matters most on. The new row reaches the state directly with
setBackendForTest(nullptr, ...) — seam declines, no plane, guard is the only
thing left. The family string is deliberately "flex": even named as the one
family that owns a command plane, a model with no backend has none, so the row
measures the backend's absence rather than the family name. Installing a null
backend prints two QObject::connect: invalid nullptr parameter warnings from
wireBackendPcm(), which has no null guard; they are expected output of that row
and the comment says so.

The Icom row is reworded as you suggested as well, so it no longer credits the
guard with stopping text the seam already stopped.

One correction to the PR body: its mutation table says forcing
hasCommandPlane() true fails "the three no-plane rows". With the new row it
fails four, which is what earns the term. Re-ran that mutation on this head to
confirm: four [FAIL] lines, then restored and green.

2. profile.isValid(). Guarded, same shape as backend_family_switch_test.
You're right about the failure mode — Part A does reach AppSettings through
PanadapterStream::init(), so unguarded it would have run against the real store
rather than failing.

3. Raw string literals. Recorded next to the state enum, including why the
failure direction is right: parsed == false is a loud FAIL, not a vacuous pass.
No code change. Same for kSeamFirstGuard requiring the send to be the guarded
block's first statement — that brittleness is inherent to reading a call site as
text, and the file already discloses it.

Checks on this head: 37/37 in the test itself, full local ctest 561/561,
tools/check_*.py --strict all nine pass, git diff --check clean. Still no file
under src/ touched.

[rfoust]

Issue fit

No linked issue. This is the requested test-only follow-up to #5920. The current head addresses all three previous review nits: it measures the no-backend state, checks settings-profile creation before accessing settings, and documents the text scanner's limitations. It pins the existing Flex fallback, ANAN typed route, Icom no-op acceptance, HL2 and Sim paths, backend pan-id translation, and empty-id refusal.

Scope

File	Change	Claimed?	Verdict
tests/pan_dimension_routing_test.cpp	Socket-free behavioral coverage and guarded source-shape checks with mutation controls	Yes	In scope
tests/tests.cmake	Register and link the test; supply source root	Yes	In scope

Everything in the diff follows the stated intent. No production, UI, default, protocol, or changelog changes. Corrected the PR body's stale mutation count to include the fourth no-plane assertion, as the author's follow-up already explains.

Blockers

None.

Nits (non-blocking)

No outstanding actionable nits. Source-shape coverage is deliberately brittle and is not a C++ parser; it complements the backend behavior checks rather than proving rendered spectrum quality or live firmware convergence.

Verification

Fresh native RelWithDebInfo build on the clean merge of main 0817cec57a0195fb9d1d24d129e0baec6dc0f134 and PR head 3ff7a7bd1daac5a79b22c4d950048e0804d53218 (tree 16796c5a59689b7145093d412d1234b39f8290cf). Revalidated after both #6030 and #5992 advanced main.

• Pinned Qt 6.12.0, package revision 202609280346. This Mac's Homebrew qtkeychain carries an absolute Qt 6.11 dependency; a process-local DYLD_FRAMEWORK_PATH pointed at the pinned kit resolves it. A dyld trace confirmed exactly one QtCore, from 6.12.0, and the final tests emitted no duplicate-class warnings. Shared libraries were untouched.
• Focused CTest selection: pan_dimension_routing_test, backend_family_switch_test, and spectrum_preview_logic_test: 3/3 passed, 4.42 seconds with single-Qt linkage.
• The new test passes 33 assertions, including all nine built-in mutation controls. Reversed the two predicates in the actual production GUI call site: the test failed at its seam-first assertion (CTest exit 8). Restored the exact source, reran green, and verified a clean diff against the integration tree.
• Strict test registration, engine-boundary, and command-plane checks passed; git diff --check clean. No boundary baseline increase.
• Required Static checks and Linux/macOS/Windows CI pass on the exact PR head. Their older base is covered by the fresh integration test above; post-merge CI remains independent verification.

Reviewed the issue-fit and lifecycle/failure paths independently. Inspected real backend constructors: they allocate inert socket objects but this test never binds, listens, connects, or addresses hardware. No synthetic firmware peer or live radio action. The test profile guards settings isolation. An automated code-review skill was unavailable; this was a manual adversarial pass.

Recommendation

Approve. The earlier concerns are addressed and the scoped coverage survives fresh integration testing. This approval applies to 3ff7a7bd1daac5a79b22c4d950048e0804d53218.