[on8st]

Refs #6156

Limits, first

• No test covers the backend's call site. With Hl2Backend::healthSnapshot() changed to pass true instead of the helper's verdict, the full suite stays as green as the head: 643 tests, the same single known failure. The helper is pinned; the line that calls it is not. A test for it would build an Hl2Backend with an injected transport, let the slice peak go stale and read the adcPairing row of healthSnapshot(). That needs either a clock the test can set (the snapshot reads steady_clock through Hl2RxDsp) or a real wait past 150 ms, which is the wall-clock dependence this PR removes. It is not in this PR.
• This touches product code (three files) to fix a test. The change is a move of two expressions into named functions.
• TSan was not run. The claim that the weekly lane goes green rests on the test no longer reading a clock in that section, and on a 400 ms sleep standing in for the slowdown.

What was wrong

hl2_adc_sampling_seam_test checked "the held peak is still FRESH BY AGE at key-up" as steadyNowNs() minus the held stamp against kSliceStaleMs (150 ms). That bounds how long the test's own muted feed loop takes, so a slow host fails it with nothing wrong in the product. It is listed in #6156 as gone; it failed there on 2026-09-19 and 2026-09-26. In both of those TSan runs (2026-09-19 at cc09af0c, 2026-09-26 at 33c0032c) the test's own block in the lane's sanitizer.log shows exactly one failure, FAIL: the held peak is still FRESH BY AGE at key-up, so the age gate is open, and no ThreadSanitizer report inside it.

The change

The age verdict was formed inline in two places. Both expressions move, unchanged, into Hl2AdcPairing.h:

helper	expression	caller
slicePeakAgeMs(peakAtNs, nowNs)	stamp 0 gives nullopt; (nowNs - peakAtNs) / 1'000'000; clamp at 0	Hl2RxDsp::adcPeakObservedAgoMs()
slicePeakIsCurrent(agoMs)	agoMs && *agoMs <= kSliceStaleMs	Hl2Backend::healthSnapshot()
slicePeakIsCurrent(peakAtNs, nowNs)	the two composed	the tests

No behaviour change:

• kSliceStaleMs is compared with an age in exactly one place in src/, the backend site above. It now calls the helper with the same ago it puts in the adcSliceObservedAgoMs row, so the row and the gate still come from one clock read.
• adcPeakObservedAgoMs() loads the stamp, then reads the clock, as before. One difference: with stamp 0 the clock is now read and discarded; the result is nullopt either way.

The seam test asks the helper about the real held stamp at two instants on that stamp's own timeline: the last one the gate admits (held + 150 ms + 999 999 ns) and the first one it refuses (held + 151 ms). Those verdicts feed the existing adcPairing() checks, plus one new negative: a stale held peak is Unknown even on the predicted input. The section reads no clock and holds no copy of the comparison. hl2_adc_pairing_test gains the helper's edges: never sampled, age 0, an instant before the stamp, truncation, exactly 150 ms, 151 ms.

Proof

macOS arm64, Qt 6.12.0, RelWithDebInfo, full ctest -j8, failures rerun at -j1.

build	result
this head (e95f7273, the review nits; same result at 15a59620)	642 of 643 pass. peripheral_auth_dialog_test fails (#6008, fails on main on this host). Both ADC tests pass.
helper <= changed to <	does not compile: a static_assert in hl2_adc_pairing_test
same, static_assert removed	hl2_adc_pairing_test fails 3 checks, hl2_adc_sampling_seam_test fails 2
healthSnapshot() passes true	642 of 643, same as the head: nothing catches it (see Limits)
main + 400 ms sleep before key-up	hl2_adc_sampling_seam_test fails "the held peak is still FRESH BY AGE at key-up", at -j8 and at -j1
this head + the same sleep	hl2_adc_sampling_seam_test passes

Every step of static-checks.yml's static-checks job passes locally. The touchpoint manifest is unchanged.

Not verified

• TSan and ASan/UBSan lanes: not run.
• Linux and Windows: not built locally; CI builds both.
• The sanitizer-option job of static-checks.yml: not run (it needs the CI image).
• No radio and no app launch: nothing here is observable on the air.

73, ON8ST

🤖 Generated with Claude Code

[aethersdr-agent]

Reviewed at head 15a59620 against the PR head checkout. Findings below are reasoned from code — I have no build and ran nothing.

1. Issue fit

Yes, for what it claims. Refs #6156, not Fixes, and correctly so: this addresses exactly one of the eleven tests in that run's list and closes nothing. I verified the motivation rather than taking it: #5959 (TSan weekly, 2026-09-26) names hl2_adc_sampling_seam_test explicitly in its triage tail — "The remaining failures in this run (… hl2_adc_sampling_seam_test) are not covered by this triage" — so the test really did go red on that lane, and #6156's own failure list does not contain it, which is the "listed as gone" the body describes.

What that also means, and the body does not quite say: nobody ever read the failing block for this test. #5959's triage explicitly left it un-pulled from the artifact. So "a slow host fails the 150 ms freshness assertion" is a hypothesis fitted to the only wall-clock budget in the file, not a diagnosis read off the log. The 400 ms-sleep experiment is good evidence that the budget can fail and that the head survives it; it is not evidence that the budget is what failed on 09-19/09-26. The body's Limits section is honest about the TSan lane not being run; it could add this one sentence.

Separately: I walked the rest of the file for other duration-bounded assertions and found none. The one remaining steadyNowNs() (seam test line 156) feeds SliceSamplingGate::m_resumedAtNs, and applied() is peakAtNs > m_resumedAtNs — a monotonic ordering comparison with no duration bound, so it is not the same flake class. After this change the file has no wall-clock budget left anywhere.

2. Scope

Everything in the diff is explained by the stated change. No file, hunk or removal needs a requirement the body does not give.

File	What it changes	Claimed?	Verdict
src/core/backends/hl2/Hl2AdcPairing.h	+slicePeakAgeMs(), two slicePeakIsCurrent() overloads, +<optional>; one comment rewritten to name the helper	yes	In scope
src/core/backends/hl2/Hl2Backend.cpp:5918	inline ago && *ago <= kSliceStaleMs → hl2::slicePeakIsCurrent(ago)	yes	In scope, behaviour-identical
src/core/backends/hl2/Hl2RxDsp.h:351-355	body of adcPeakObservedAgoMs() → slicePeakAgeMs(at, steadyNowNs())	yes	In scope, behaviour-identical
tests/hl2_adc_pairing_test.cpp	new §9b, the helper's edges	yes	In scope
tests/hl2_adc_sampling_seam_test.cpp	the clock read removed; two helper probes + one new negative	yes (this is the fix)	In scope

Checked and clean: no new public/protocol surface, no settings key, no capability, no CHANGELOG.md entry, no default changed, no UI touched, no guard or early return deleted (the only - lines are the five-line inline body and the superseded comment text), no new gui→engine touchpoint (Hl2AdcPairing.h has no src/gui includer, and the body's "touchpoint manifest unchanged" holds). Both tests are registered in tests/tests.cmake (6812/6824) with real add_test names.

No socket test added, modified or removed. Neither file contains QTcpServer/QUdpSocket/QLocalServer/bind()/listen()/connectToHost() or a Fake* peer; the seam test is QCoreApplication plus a queued QMetaObject::invokeMethod, which binds nothing.

Completeness: kSliceStaleMs is compared against an age in exactly one place in src/ and the helper now owns it — I grepped for siblings and there are none. The new call site consumes the same ago that fills the adcSliceObservedAgoMs display row, so the body's "the row and the gate still come from one clock read" is literally true at Hl2Backend.cpp:5905-5918.

3. Blockers

None.

4. Nits (non-blocking)

• tests/hl2_adc_sampling_seam_test.cpp:227-230 — the two new freshness probes are constants, so the comment above them overstates what they test.

      const bool freshAtLimit =
          slicePeakIsCurrent(held, held + kSliceStaleMs * kNsPerMs + (kNsPerMs - 1));
      const bool staleLater = slicePeakIsCurrent(held, held + (kSliceStaleMs + 1) * kNsPerMs);

  held cancels: for any non-zero stamp these are true and false respectively, whatever kSliceStaleMs is. So freshAtLimit ≡ true, staleLater ≡ false, and the three adcPairing() calls below are byte-for-byte the old /*current=*/true plus one false row. The body's "asks the helper about the real held stamp at two instants" reads as though held were under test; nothing about it is. That is the right outcome — the subject of this section is the sampling gate, and pinning the age input as a deliberate constant is exactly how you get the flake out — but say that plainly rather than implying a measurement, and note that the edge semantics themselves are now covered by §9b of hl2_adc_pairing_test. check(held == frozen, …) is the one new assertion that touches the fixture, and it is a good one.
• tests/hl2_adc_pairing_test.cpp:277 — the message and the expression use different instants.

      check(slicePeakAgeMs(at, at - 3 * kNsPerMs) == 0 && slicePeakIsCurrent(at, at - 1),
            "an instant before the stamp reads as age 0, current");

  Both halves are correct, but one probes 3 ms before the stamp and the other 1 ns before. Using at - 3 * kNsPerMs in both would make the check read as the single claim its message states.
• tests/hl2_adc_pairing_test.cpp:210 still quotes the old inline predicate (so `ago <= kSliceStaleMs` is TRUE). Harmless as prose, but it is now the only place that spells the expression out by hand, which is the thing this PR set out to stop.
• The untested backend call site is pre-existing, not opened here. The body's first Limit is accurate and worth the maintainer's attention, but the inline ago && *ago <= kSliceStaleMs at that same line was equally uncovered on main — swapping it for a named call neither adds nor removes coverage there. Reading it as a regression this PR introduces would be wrong.
• The "Linux and Windows: not built" limit is conservative. build, check-macos, check-windows, Static checks and Sanitizer option configures are all green on 15a59620. That matters beyond politeness: the static_assert in §9b exercises constexpr std::optional in a target the full-tree build compiles, so that path is proven on the Linux Qt image and under MSVC, not only on the author's Mac.

5. What I tried to break

• Behavioural equivalence of both extractions, expression by expression. Hl2RxDsp: old was if (at == 0) return nullopt; ago = (steadyNowNs() - at)/1e6; return ago < 0 ? 0 : ago; — the helper is the same three steps in the same order, and at is still loaded before the clock is read, so the ordering the comment relies on survives. Hl2Backend: ago && *ago <= kSliceStaleMs → slicePeakIsCurrent(ago), identical by inspection. The one real difference is the author's own: with stamp 0 the clock is now read and thrown away. Disclosed, and nullopt either way.
• Overload resolution, which is where a two-function set named alike usually bites. slicePeakIsCurrent(kSliceStaleMs) (one int64_t) takes the age overload through the converting optional constructor; (0, limit) and (0, 0) take the stamp/instant overload exactly; slicePeakIsCurrent(std::nullopt) is unambiguous; the two-arg body calling the one-arg version is an exact optional match, so no recursion and no ambiguity. The residual hazard is semantic, not a compile error: slicePeakIsCurrent(ageMs, nowNs) — an age mistaken for a stamp — compiles silently. Low risk with one call site, and not worth a signature change.
• Every row of §9b against the arithmetic. Truncation toward zero at limit - 1 gives 149; limit gives exactly 150 and passes; limit + 1 ms - 1 ns still truncates to 150 and passes; limit + 1 ms gives 151 and fails. An instant before the stamp clamps to 0 and reads current. Stamp 0 is nullopt at every instant including 0. No row is wrong and none is vacuous. I also confirmed the body's "<= → < does not compile" claim holds: that mutation makes slicePeakIsCurrent(at, limit) false, which is the left half of the static_assert at line 290.
• Whether the seam test still fails if the fix it guards is reverted. The assertions that carry the original defect — requested(), !gate.applied(held), and the adcPairing(..., gate.applied(held), ...) == Unknown row — are untouched by this diff, so a simplification back to !(keyed && !monitor) still dies here. The diff weakens the age probes into constants (nit above) but does not weaken the sampling-gate assertions, which are the test's actual subject.
• Sibling call sites and dead additions. Grepped src, tests and docs for every new and old symbol: three helpers, three callers, no stale hand-rolled copy left behind, nothing added that nothing references.
• Could not check: the TSan lane itself (not run here or by the author), and whether the 09-19/09-26 failures were in fact the freshness assertion rather than something else in the file — the sanitizer artifact would settle it and #5959's triage never pulled that block. One pre-existing residual I noticed but am not asking anyone to fix: applied() needs peakAtNs > m_resumedAtNs strictly, so a clock coarse enough to tie the resume request and the first post-unmute stamp would fail "the first post-unmute peak reopens the gate" — unreachable on the platforms in CI, and older than this PR.

6. Recommendation

Approve with nits. Both extractions are behaviour-preserving by inspection, the single remaining kSliceStaleMs comparison is the one the display row already uses, the flake's only wall-clock budget is gone, and the Limits section discloses the two things I would otherwise have had to find — the untested backend line and the un-run TSan lane. The nits are comment and message wording plus one message/expression mismatch; none of them needs a new round trip if the maintainer would rather land it. Next step: rewrite the comment at hl2_adc_sampling_seam_test.cpp:224-231 to say the age input is deliberately a constant there and that its edges live in hl2_adc_pairing_test §9b, and add one line to Limits noting the 09-19/09-26 failing blocks were never read.

Thank you for the mutation table and for stating outright that passing true at the backend site catches nothing — that is the kind of Limits section that makes a review faster rather than slower.

---

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

[on8st]

One point of the review I can answer with the logs: the failing blocks were read. In the TSan runs of 19 September (cc09af0c) and 26 September (33c0032c), the test's own block in sanitizer.log shows exactly one failure, FAIL: the held peak is still FRESH BY AGE at key-up, so the age gate is open, and no ThreadSanitizer report inside it. The body now says so.

The three wording nits are in e95f7273, tests only:

• the seam test's comment says the age input is a deliberate constant there, and that the gate's edges are pinned in hl2_adc_pairing_test;
• the before-the-stamp check uses one instant in both halves;
• the remaining comment names the helper instead of the old expression.

Full suite at e95f7273 (macOS, Qt 6.12.0): 642 of 643; the one failure is peripheral_auth_dialog_test, as before.

73, ON8ST

[ten9876]

Reviewed head e95f72730f3a07ce8964fb817e1c15e9078ef5fa.

Issue fit: Yes for the explicitly limited Refs #6156 scope. The seam test no longer imposes a 150 ms wall-clock budget on its own feed loop. The two production expressions were extracted unchanged; this does not claim to repair every failure in the weekly sanitizer issue.

Scope

File/group	Change	Claimed?	Verdict
Hl2AdcPairing.h	Pure timestamp-to-age/current helpers	Yes	In scope
Hl2Backend.cpp, Hl2RxDsp.h	Use extracted expressions	Yes	In scope
hl2_adc_pairing_test.cpp	Never-sampled, truncation, freshness boundary cases	Yes	In scope
hl2_adc_sampling_seam_test.cpp	Deterministic age input, preserve sampling gate checks	Yes	In scope

No unrelated changes, new capability flags, new vendor coupling, persisted settings, or UI behavior. Commit subjects and authored dates show the extraction and follow-up review edits rather than an unrelated bundled change.

Blockers

None found.

Nits / limits (non-blocking)

1. src/core/backends/hl2/Hl2Backend.cpp:5918: the production call-site coverage gap remains exactly as disclosed: a helper test cannot establish that healthSnapshot() consumes its result. Preserve the Limits section; a future injectable clock/session fixture should cover that row without sleeping or adding synthetic firmware.
2. tests/hl2_adc_sampling_seam_test.cpp:224: I did not reproduce the TSan weekly lane or the historical artifact failures. The deterministic helper check establishes the replacement arithmetic, not that every sanitizer failure is resolved. No claim here that the full suite or sanitizer lanes pass.

Verification and attacks

• Rechecked both modified test sources and registered targets: socket-free, no listener, socket bind, peer process, or synthetic firmware. No application/radio session launched; this change has no simulator-reachable GUI behavior.
• Compiled and ran the PR's actual hl2_adc_pairing_test.cpp using GCC 16.2.1/C++20: PASSED: 0 failure(s). Tested no stamp, a future stamp, integer-ms truncation, exactly 150 ms and first stale millisecond.
• Mutation: changed only the helper's <= to < in an isolated header copy. Compilation failed at test line 291 with static assertion failed: usable in a constant expression. The boundary check is sensitive to the defect; it is not just self-reporting success.
• Read the unchanged queued-mute/requested-versus-applied gate checks in the seam test and the production load-stamp-before-clock ordering. Zero stamp now reads an unused clock but still returns nullopt; the health row and gate still share ago. The seam executable itself and full application were not built in this pass.
• Ran engine boundary, touchpoint manifest, capability-record and command-plane gates on merge base and head: all pass, and each tool's output is byte-identical between them. No new #5554 patterns found. Separate automated code-review skill was unavailable; this was an independent manual audit plus focused compile/mutation.

CI and recommendation

Approve with nits; posted as COMMENT so approval remains the maintainer's decision. The five displayed checks are successful. Earliest workflow creation on this head is 2026-10-04T21:19:29Z, after current main's 2026-10-04T18:26:40Z; this timestamp check finds no stale-main gap. PR workflows test their frozen merge result, not an unfiltered full-suite guarantee. The useful next step is maintainer approval with the backend call-site and sanitizer limitations retained.

— Jeremy KK7GWY + Codex

[ten9876]

Issue fit: yes, for the scope it claims (Refs #6156, one of that sticky's tests). At e95f7273 the seam test no longer puts a 150 ms wall-clock budget on its own muted feed loop. Both production expressions move unchanged into Hl2AdcPairing.h helpers.

Scope

File	Change	Claimed?	Verdict
src/core/backends/hl2/Hl2AdcPairing.h:45-70	slicePeakAgeMs() plus two slicePeakIsCurrent() overloads	Yes	In scope
src/core/backends/hl2/Hl2Backend.cpp:5918	inline ago && *ago <= kSliceStaleMs → slicePeakIsCurrent(ago)	Yes	In scope, identical
src/core/backends/hl2/Hl2RxDsp.h:351-355	body → slicePeakAgeMs(at, steadyNowNs()); still loads the stamp before reading the clock	Yes	In scope, identical
tests/hl2_adc_pairing_test.cpp §9b	the helper's edges	Yes	In scope
tests/hl2_adc_sampling_seam_test.cpp	clock read replaced by constant age inputs; one new stale-negative row	Yes (this is the fix)	In scope

Everything in the diff is explained by the stated change. There are two authored commits, both on 10-04, and no CHANGELOG.md, settings, capability or UI change. Socket preflight: neither test opens a socket. Merge is clean against current main (85f2a124).

Blockers

None.

Nits (non-blocking)

• src/core/backends/hl2/Hl2Backend.cpp:5918: no test covers this call site, as the body's first Limit says; swapping it for true goes unnoticed. This gap predates the PR (the inline expression was equally uncovered), so it is not a regression. It is worth a follow-up issue so the "Limits" sentence doesn't become the only record.

What I tried to break / verified

• Built and ran both targets at this head (Linux, Qt 6.12.0): hl2_adc_pairing_test and hl2_adc_sampling_seam_test both pass.

• Reproduced the flake mechanism, both directions, using the body's own experiment: a 400 ms sleep inserted immediately before the key-up keyed = false;.

  • main's seam test plus the sleep, built against this head's headers: exit 1, FAIL: the held peak is still FRESH BY AGE at key-up, so the age gate is open. That is exactly the line the weekly TSan lane logged on 09-19 and 09-26.
  • This head's seam test plus the same sleep: exit 0, hl2_adc_sampling_seam_test: OK.

  Both files were restored from git afterwards.

• Behavioural equivalence of the extraction:

  • stamp 0 → nullopt, before and after;
  • truncation toward zero, with an instant before the stamp clamping to 0;
  • load-before-clock ordering preserved in Hl2RxDsp;
  • the gate and the adcSliceObservedAgoMs row still share one ago;
  • kSliceStaleMs itself is still "current".

  The one difference is that a stamp of 0 now reads (and discards) the clock. The body discloses it, and it is harmless.

• Overload resolution: slicePeakIsCurrent(int64) takes the age overload through optional's converting constructor, and (stamp, now) takes the two-argument one. Neither is ambiguous, and nothing recurses.

• Whether the seam test still guards its real subject (the sampling gate): it does. requested(), !gate.applied(held) and the gated == Unknown row are untouched. The age probes are now constants by design, and the new comment says so.

• Not run: the TSan lane, and Windows/macOS locally.

Recommendation: no blocker; ready for a maintainer merge.

— Jeremy KK7GWY + Claude

[Ozy311]

Reviewed head e95f7273. It merges cleanly onto current main (0156894f, 2026-10-10).

Issue fit: yes, for what it claims. Refs #6156, not Fixes, which is right: this PR takes one test out of that tracker. #6156 is now closed, so nothing is red today. What the PR removes is the test's only wall-clock budget: the 150 ms bound on its own muted feed loop that failed the 09-19 and 09-26 TSan runs. It does that without changing product behaviour.

Scope

File	Change	Claimed?	Verdict
src/core/backends/hl2/Hl2AdcPairing.h:45-70	slicePeakAgeMs() and two slicePeakIsCurrent() overloads; <optional>; one comment names the helper	Yes	In scope
src/core/backends/hl2/Hl2RxDsp.h:351-355	adcPeakObservedAgoMs() body → slicePeakAgeMs(at, steadyNowNs()); stamp still loaded before the clock	Yes	In scope, identical
src/core/backends/hl2/Hl2Backend.cpp:5918	ago && *ago <= kSliceStaleMs → hl2::slicePeakIsCurrent(ago)	Yes	In scope, identical
tests/hl2_adc_pairing_test.cpp §9b	Edge cases for the helpers	Yes	In scope
tests/hl2_adc_sampling_seam_test.cpp:218-253	Clock read replaced by constant age inputs, plus one stale-negative row	Yes (this is the fix)	In scope

Everything in the diff is explained by the stated change. The PR adds no CHANGELOG.md entry, settings key, capability, protocol surface or UI change, and removes no guard. Neither test opens a socket.

Blockers

None.

Nits (non-blocking)

1. src/core/backends/hl2/Hl2Backend.cpp:5918: the call site is still uncovered. The body's first Limit says so: replacing it with true passes every test. That gap was already there on main with the inline expression, so this is not a regression. It deserves a tracking issue, so the Limits paragraph is not its only record, if one has not been filed yet.
2. The CI is from 2026-10-04. main has taken many commits since, including some that touch Hl2Backend.cpp and Hl2RxDsp.h. The merge is textually clean, but the green checks describe the old merge, and with strict: false nothing forces a rerun before merge.

Verified

• hl2_adc_pairing_test (header-only) builds and passes on the PR head. I applied three mutations to the new helpers, and each one fails the test: dropping the clamp at 0, dropping the stamp-0 nullopt, and treating nullopt as current. The body's <=→< mutation trips the static_assert.
• hl2_adc_sampling_seam_test was not run here. It needs the full core build. I checked it by reading only.

What I tried to break, and what held

• Equivalence of the extraction. The order is unchanged: load the stamp, then read the clock. Stamp 0 still gives nullopt. Truncation and the clamp at 0 are unchanged. The display row and the gate still share one ago.
• Overload resolution. A bare int64_t age converts to the optional overload, (stamp, now) takes the two-argument overload, and std::nullopt is unambiguous. Nothing recurses.
• Whether the seam test still guards its real subject, the sampling gate. requested(), !gate.applied(held) and the gated == Unknown row are unchanged. The new held == frozen check ties the probe to the fixture's frozen stamp (:182). The age inputs are constants by design, and the comment says so.
• Not driven in the app. This is an HL2 health-snapshot path that the demo simulator cannot reach, and the diff has no UI effect. TSan was not run.

73, K6OZY

[jensenpat]

No blockers. Approving for merge.

This is not already on main. #6173 and #6208 never touch the ADC age gate, and the ago && *ago <= kSliceStaleMs check plus the seam test's live-clock freshness assertion are still on main. Those lines are also unchanged since this PR's merge base, so the 2026-10-04 green checks still describe this diff.

The extraction is behaviour-identical. The seam test no longer spends a 150 ms budget on its own feed loop. The uncovered healthSnapshot() call site is pre-existing and tracked in #6186.