test(hl2): make the cadence test self-contained in its includes

[on8st]

A test that gets its types from the thing it tests

tests/hl2_telemetry_cadence_test.cpp uses std::array, std::optional and std::uint8_t, and includes none of them. All three arrive through core/backends/hl2/Hl2TelemetryCadence.h — the header under test.

That works today because acceptReply() takes a std::array<std::uint8_t, 6> and two std::optional parameters, so the header includes all three for its own declarations. It is a coincidence of the header's current shape rather than a contract. Change that signature and the header drops an include it no longer needs; the test then stops compiling, and reports it as a failure of the thing it is testing.

What this is NOT, because I got it wrong first

My initial reasoning was that the header might "tidy these away" and break the test. It cannot, and the experiment says so. Stripping <array>, <cstdint> and <optional> from Hl2TelemetryCadence.h fails the header, with or without this change — 15 errors either way, measured on this branch.

So the claim is narrower than I first wrote it: this buys independence from a future change to what the header needs, not protection against the header being tidied today. The commit message says the same. I would rather ship the smaller true reason than the larger one that sounded better.

Mutation check

The mutation here is on the claim rather than on a guard, because the change adds no guard — three include lines cannot be broken in an interesting way. What can be broken is the REASON, and that is what I broke:

mutation	result
strip <array>, <cstdint>, <optional> from Hl2TelemetryCadence.h, with this change	15 errors — in the header
the same, without this change	15 errors — in the header, identical

Same failure either way, which is what refuted my first justification and produced the narrower one above. The test itself passes before and after; there is no assertion whose inversion would fail, and I would rather say that than invent one.

Related in kind

#5642's Linux build failure was the same habit one file over: Hl2TelemetryCadence.h used std::uint8_t with no <cstdint>, libc++ supplied it transitively through <array>/<optional>, libstdc++ did not, and check-macos was green while build was red. That one is fixed on main at 0322c253. This is the test side of it, and it is the only remaining instance I found in that file's neighbourhood.

Validation

hl2_telemetry_cadence_test passes before and after. No behavioural change — three include lines and a comment.

Not measured: anything on hardware; nothing here touches a radio. I did not sweep the rest of tests/ for the same shape, so this fixes one file rather than a class. If a reviewer would rather have the sweep, say so and I will do it as its own change rather than growing this one.

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

No linked issue, and none is needed: this is test hygiene with a stated rationale in the diff itself, not an architectural change, so GOVERNANCE.md's RFC path doesn't apply. Reviewed against the PR's own claim — "make the cadence test self-contained in its includes" — and the claim holds. I verified the three added headers are each genuinely used by this translation unit rather than added speculatively: std::array and std::optional at tests/hl2_telemetry_cadence_test.cpp:156-158, std::uint8_t in those same declarations. Today they arrive transitively through Hl2TelemetryCadence.h:3-6; after this PR they arrive directly. That is exactly what the title says.

Scope

File / group	What it changes	Claimed by title/body?	Verdict
tests/hl2_telemetry_cadence_test.cpp (+3 #include, +13 comment lines)	Adds <array>, <cstdint>, <optional> directly; explains why	Yes	In scope

Everything in the diff is explained by the stated intent. No production code, no build config, no new target, no public surface, no settings key, no CHANGELOG.md entry, no deleted guard — the - side of the diff is empty. tests/tests.cmake:5102-5106 already registers hl2_telemetry_cadence_test and is untouched, so no registration question arises.

Socket-test surface: none. Grepped the test source at the PR head for QTcpServer/QTcpSocket/QUdpSocket/QLocalServer/bind(/listen(/connectToHost — zero hits. The file's own header comment ("Pure header, no Qt, no socket, no radio") is accurate, and this PR neither adds nor removes network behavior.

Blockers

None.

Nits (non-blocking)

• tests/hl2_telemetry_cadence_test.cpp:35 — header'''s is a shell-quoting artifact. Three literal apostrophes, the signature of ' passing through a single-quoted shell string during authoring. Inline comment has a one-click suggestion. Cosmetic, but this PR is almost entirely that comment, so it's the one thing worth fixing before merge.
• Lines 41-44, the parenthetical self-correction, is meta-commentary on the comment's own drafting history rather than on the code. "An earlier version of this comment said so wrongly" is information about the PR, not about the file — the sort of thing that reads as noise to someone opening this test in two years with no memory of the review. The substantive half (stripping these from the header breaks the header itself, so this isn't tidying the header could do) is worth keeping; the apology around it isn't. Author's call.

What I tried to break

• "All three arrive today only because the header needs them." Tested, holds. src/core/backends/hl2/Hl2TelemetryCadence.h:3-6 includes all three, and the header's own acceptReply() signature at :156-163 uses std::optional<std::array<std::uint8_t, 6>> — so the header genuinely needs them for its own declarations, and the comment's claim that this is a transitive coincidence is correct.
• Is the fix complete, or does one transitive dependency remain? The strongest possible finding here would be a fourth facility still reached through the header. I enumerated every std:: use and every braced-init/loop construct in the file: std::fprintf (<cstdio>, line 48, pre-existing), the {Hl2LinkState::…} / {false, true} range-for lists at lines 109 and 111 (<initializer_list>, line 49, pre-existing). Nothing else. The include set is now closed.
• Are the added includes dead weight — added to look thorough but unused? No; each maps to a concrete use at lines 156-158, so this isn't include-cargo-culting.
• Did the comment's factual claim survive? Partially adversarial: I checked whether the header could in fact drop <cstdint> while keeping std::uint8_t — it can't, and the header's own inline note (libstdc++ does not get it via <array>) says why. The parenthetical's correction is accurate, which is the reason I'm only asking to trim it for tone, not for error.
• CI. All 5 checks green at 70756ed (build, check-macos, check-windows, Static checks, Sanitizer option configures). That's meaningful here in a way it usually isn't: for an include-only change, "it still compiles on three platforms" is close to the whole correctness question, and the Linux job is the one that would catch a libstdc++-only transitive break.

Everything above is read from the diff and the PR head checkout at /tmp/aetherclaude/pr-5735; I have no build or test run in this environment, so "the test still compiles and passes" rests on CI, not on my own execution.

Recommendation

Approve with nits. The change is correct, minimal, complete, and does exactly what it says — I went looking for a missed transitive include and a speculative one, and found neither. The only thing I'd want touched before merge is the ''' typo at line 35; the comment-trimming nit is optional. Thanks for the IWYU cleanup and for writing down why rather than just adding the lines.

---

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

[jensenpat]

Issue fit

No linked issue. Reviewed against the PR's own claim: make hl2_telemetry_cadence_test self-contained for std::array, std::optional, and std::uint8_t. The claim holds. Those types are used at tests/hl2_telemetry_cadence_test.cpp:156-158; the header under test still includes them for acceptReply(), so today's compile is a coincidence of that signature, not a contract.

Scope

File / group	What it changes	Claimed?	Verdict
tests/hl2_telemetry_cadence_test.cpp	Adds <array>, <cstdint>, <optional> plus a comment	Yes	In scope

One file, additions only. No production code, CMake, public surface, settings, CHANGELOG, or deleted guard. tests/tests.cmake already registers hl2_telemetry_cadence_test and is untouched.

Socket-test surface: none. The file is a pure-header TU (no Qt sockets, bind/listen/connectToHost, or fake radio). This PR does not add or remove network behavior.

Follow-up commit c97353b0 addressed the prior nits (header'''s quoting artifact and the comment's drafting diary).

Blockers

None.

Verification

• Source: full diff of c97353b02729455308756e4e4f7c13fdfc9684cf vs 0322c253f90b40840b4c3207b7ef7cf55df5ad69 (1 file, +14/−0).
• Header at PR head still includes <array>, <cstdint>, <optional> for its own declarations.
• CI on this SHA: build, check-macos, check-windows, Static checks, Sanitizer option configures — all SUCCESS. For an include-only test change, those three-platform compiles are the correctness check.
• Local rebuild skipped: CI already compiled and ran this target at the reviewed revision.

Uncertain: no rest-of-tests/ IWYU sweep (author scoped that out).

Recommendation

Approve. Merge-eligible on source and CI.