Skip to content

fix(cw): lower DeepFist's activity-gate threshold to 3 so weak off-air CW is not gated out (#5950). Principle XI. - #5953

Merged
NF0T merged 2 commits into
aethersdr:mainfrom
skerker:fix/5950-deepfist-activity-threshold
Oct 6, 2026
Merged

NF0T merged 2 commits into
aethersdr:mainfrom
skerker:fix/5950-deepfist-activity-threshold

Conversation

@skerker

@skerker skerker commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Why. #5716 made the CW receive decoder selectable (ggmorse, DeepFist; DeepCW is proposed under #4817 in K5PTB#1). A choice of decoders serves the operator because each one suits different signal conditions — but only if each performs well across the signals actually met on the air. This PR comes out of testing the decoders side by side on off-air W1AW bulletins, whose published text gives a ground truth, received on different days at different signal levels, plus live receive on the FLEX-8400. DeepFist as shipped lost most of its text on the weaker recordings; the cause and the fix follow.

Fixes #5950. @rfoust @K5PTB

DeepFist's activity gate closes on weak and moderate off-air CW that a person can still copy, so much of the text never publishes; strong signals are unaffected. The gate compares a keying ratio against activityThreshold 12 (src/core/deepfist/DeepFistStream.h:16, test at src/core/deepfist/DeepFistStream.cpp:92); the app's parameters (src/models/DeepFistCwModel.cpp) did not set it. The 12 comes from n9bc/DeepFist tools/squelch.py, calibrated between dead air (keying ratio 3.4–3.9) and a tuned-in signal (41–73) — "~12 sits in the wide empty gap". Weak off-air CW scores in that gap: on the #5950 recordings (received 2026-09-24) the gate was closed on 66–72 % of ticks, and 1667 of 2960 gated ticks on ARLD038 scored 9–12.

This PR sets the app's activityThreshold to 3. The app's parameters move into one DeepFistCwModel::appParameters() so tests can exercise exactly what the app runs; the stream default (12) is unchanged. docs/deepfist-cw-backend.md lists the new setting.

The value and whether it should be user-adjustable are open for the maintainers and the model's author; the data below covers 12, 6 and 3.

Requirement (#5950) Change
DeepFist as shipped must not drop most of the text of weak/moderate off-air CW appParameters() sets activityThreshold = 3
No false text on noise or a steady carrier carrier regression extended to the app's parameters; noise measured (below)

Constitution principle honored

Principle XI — the attached bundle reproduces the defect and the fix from recorded audio (run.sh, outputs byte-identical), so the before/after is checkable independently of this PR's own test plan. Principle VIII — every figure below is measured and reproducible from the attachments.

Evidence

Character error rate against the published bulletin text (ground truth), each decoder on the same recording; W1AW off air on 17 m, received on different days at different signal levels (2026-09-24: #5950's bundle; 2026-09-25: this PR's bundle):

12 (before) 6 3 (this PR) DeepCW (unmerged, K5PTB#1) ggmorse
ARLD038, received 2026-09-24, +5.0 dB over noise 62.6 % 15.0 % 13.9 % 9.5 % 49.7 %
ARLP038, received 2026-09-24, +5.2 dB over noise 65.9 % 20.6 % 20.5 % 7.3 % 36.1 %
ARLD038, received 2026-09-25, +0.6 dB over noise, deep fading 99.8 % (3 chars) 78.3 % 50.9 % 37.2 % 79.6 %

DeepCW is not in AetherSDR; it runs from K5PTB's unmerged PR build (head f0306533) and is shown as the accuracy reference. Signal level = median key-down minus key-up level at the CW pitch (2 s windows, tools/fade_profile.cpp), minus the 13.5 dB the same measure reads on 17 m band noise with no signal; relative on post-AGC audio, not a calibrated SNR (signal-levels.txt in this PR's bundle).

  • ARLD038 received 2026-09-25, share of ticks gated by tone-to-noise quarter (weakest→strongest): 100 / 100 / 100 / 99.8 % at 12; 88.9 / 87.5 / 80.1 / 52.3 % at 6; 0 at 3. These shares are over the ticks on which the model emitted at least one token (the replay's token log has no row for the others). Over all 5179 ticks of the take the gate was closed on 99.7 % at 12, 79.3 % at 6 and 0 % at 3 (deepfist-thr*.log in the bundle).
  • Noise: the first 10.5 min of 17 m band noise (2026-09-25; W1AW started at 11.5 min): 0 characters committed at 12, 6 and 3 (the replay stream's first three 5s at 0.9–3.1 s appear at every threshold). "Committed" counts token sightings the stream labelled commit; the published text agrees: at 3 it reads 555 and then 4V DE W1AW, and that 4 has audio time 673.6 s. A separate 201 s empty-frequency take (DeepFist drops most of the text on off-air CW: activity gate closed on 66–72 % of ticks (two W1AW bulletins) #5950 follow-up): only the stream-start 5555 at 3.
  • The cost of 3, from the same follow-up bundle: a very weak CQ on which only "CQ" could be copied by ear reads F5 5Q CQCQ OG? R4KTCNQCRQ CQ CQA2 at 3 against F5 5 at 6 and 12 — the CQs plus junk.
  • 40 m QSO + practice sender (+10.2 dB over noise): 664 / 685 / 687 non-space characters at 12 / 6 / 3 — nearly the same text.
  • Live, a local build running this threshold on the FLEX-8400 (receive only) copied W1AW's announcements ("CODE PRACTICE STARTS IN 1 MINUE"), where the shipped threshold, replayed on the same recording, printed only the stream-start 555; and a 40 m QSO ("I LIKE THE LADDER INE BUT HVNT HADANY FER YEARS …").
  • Delay: DeepFist commits a character 1.3–1.7 s after it is sent (replay, median 1.49 s); unchanged by this PR.

Decoded text, the same two sentences at both signal strengths (sentence-aligned against the published bulletin by the bundle's scripts/side_by_side.py; character errors in brackets):

ARLD038 received 2026-09-24 (+5.0 dB) ARLD038 received 2026-09-25 (+0.6 dB)
published JACEK, SP5EAQ, IS QRV AS 5W0AF FROM SAMOA THROUGH OCTOBER 12.
DeepCW (unmerged, K5PTB#1) JACEK, SP5EAQ, IS QRV AS 5W0AF F NOM SAMOA THROUGH OCTOBER 12 SK [5] JACEK, SP5VQ, IS QRV AS 5W0AF FROM SAMZ1ONR 12. [18]
DeepFist 3 (this PR) JACEK, SP5EAQ, IS QRV AS 5W0AF F IOM SAMOA THROUCH OCTOBER 124 [4] JACK,SPFQ, IS QRV AS 5W0AF FRQM SAMO 12. [23]
DeepFist 12 (shipped) L A THR [55] (nothing) [62]
ggmorse T CEK, SP5EAQTTB 5W0SF F TM SAMOA THR WH NH OF ET?M EV [30] ?EESE QRV AS 5W?AOR?I ?B [44]
published BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE UNTIL EARLY OCTOBER.
DeepCW (unmerged, K5PTB#1) BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE ANTIL EARLY OCTOBER. [1] BARRY, KD6XUW IS QRV AS YJ0BP FROM EFAT UNTIL EARAEY EICTOBER. [6]
DeepFist 3 (this PR) BARRY, KD6XU, IS QRV AS YJ0BP FROMEFATE ANTIL EARLY OCTOBER. [2] BURRY, KD6XUG3 IEQRV AS YJ,0BP FROM EA ANTIL RAY DESTOBER [17]
DeepFist 12 (shipped) BARR , KD6XU, IS QRV AS YJ0 MEFATE ANTIL EARLY OCTOBER . [9] (nothing) [62]
ggmorse BARRKE ENC MEW EXU, IS QRV AS AS O06P SG FROM EFATE ANTIL EAE EA5L Y OCTOBEREEE EETIETETA [39] Y I? ?V IEI TE [51]

Test plan

  • Local build passes — clean build of c0df8d38, Linux (Ubuntu 24.04.5, g++ 13.3, Qt 6.8.3), -DENABLE_DEEPFIST_EXPERIMENT=ON; Windows at bf9e0338 (MSVC 19.44, DeepFist ON): clean build, the 9 DeepFist/CW tests pass with the model, none skipped (comment)
  • Behavior verified on a real radio — FLEX-8400 fw 4.2.20.41343, receive only, 17 m and 40 m (a local build of main with the threshold overridden to 3, not this PR's head; recordings attached)
  • Existing tests pass (CI) — all checks green on c0df8d38; CI builds with DeepFist OFF, so it does not compile the changed files. Locally with the model: 9/9 pass (8 deepfist* + cw_rx_model_test)
  • Reproduction steps documented — run.sh in the attached bundle

Tests. In the existing deepfist_cw_model_test target: deepfist_carrier_regression_test now also runs a steady carrier through appParameters() and expects no text (the completed-mark guard holds a carrier at any threshold, so this arm guards that path, not the value 3); new deepfist_weak_signal_test decodes the existing CQ TEST CQ fixture plus CONSTRUCTED in-band noise (40 tones at 550–650 Hz, seed 5950, values taken straight from std::mt19937 so the fixture is the same on every standard library) and expects the final CQ to survive at appParameters() but not at threshold 12 — true for noise RMS 0.06–0.13; the test uses 0.09. The same noise with the keying removed must not print more at appParameters() than at 12 (5 / 5), and a failed decode in any arm fails the test. Mutation (run at bf9e0338): with appParameters() set back to 12, deepfist_weak_signal_test fails (5Q TEST / 5Q TEST) and the carrier test still passes; restored, both pass. Like the other DeepFist tests they are built only with ENABLE_DEEPFIST_EXPERIMENT=ON, which no CI workflow sets, so CI neither builds nor runs them; without a model they skip (77).

Not tested: a macOS build with DeepFist ON; a Windows build of the review-round commit (Windows was run at bf9e0338); static crashes and digital signals near the pitch; a live display-delay measurement.

Proof

Bundle: three live takes received 2026-09-25 on the FLEX-8400 (receive only), replayed through DeepFist at 12 / 6 / 3 (main 2859fdef + the bundled replay-tool patch, threshold set per run), DeepCW (unmerged, K5PTB#1) and ggmorse; run.sh reproduced 22/22 outputs byte-identical (replays, comparison tables, fade profiles, signal-levels.txt).

File Contents
main bundle (1.0 MB) README, run.sh, decoder outputs, comparison tables, signal levels, scripts, logs, screenshots
audio A part 0 · part 1 take A — W1AW ARLD038, 17 m, +0.6 dB over noise, deep fading (34.5 min)
audio B part 0 · part 1 take B — 17 m, 21.7 min: band noise, then W1AW announcements from 11.5 min
audio C part 0 take C — 40 m QSO + practice sender, +10.2 dB over noise (12.0 min)
#5950 issue bundle ARLD038 / ARLP038 received 2026-09-24 (+5.0 / +5.2 dB over noise): DeepFist parameter sweeps incl. thresholds 3 / 6 / 9 / 12 / 15; audio from K5PTB#1
#5950 follow-up bundle (comment) 17 m, 2026-09-25: a very weak CQ (only "CQ" audible by ear) and 201 s of an empty frequency; DeepFist at 12 / 6 / 3, DeepCW, ggmorse

Unzip the audio parts into the bundle's audio/ folder, then run run.sh (usage in the README).

Checklist

  • Commits are signed
  • No new flat-key AppSettings calls — n/a, no settings change
  • Code is clean-room
  • All meter UI uses MeterSmoother — n/a, no meter UI
  • Documentation updated — docs/deepfist-cw-backend.md; CHANGELOG.md untouched
  • Security-sensitive changes reference a GHSA — n/a

— authored by agent (Claude Code) on behalf of @skerker

…r CW is not gated out (aethersdr#5950). Principle XI.

The stream default (12) comes from n9bc/DeepFist tools/squelch.py, calibrated between
dead air and a tuned-in signal; weak off-air CW scores in that gap and the gate
dropped most of it. The app's parameters now set 3; carriers stay held off by the
completed-mark guard (the carrier regression now also runs at the app's parameters).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@skerker
skerker marked this pull request as ready for review September 26, 2026 01:37
@skerker
skerker requested a review from a team as a code owner September 26, 2026 01:37

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at bf9e0338 against the head checkout. This is unusually well-evidenced work — the one-field-at-a-time sweeps, the three new takes at different signal levels, the disclosed mutation check, and the explicit "value is open for the maintainers" framing are all what makes the diff reviewable. What follows is what I tried to break, plus one decision I don't think a reviewer can make for you.

1. Issue fit

Yes, for the stated requirement. #5950's measured defect is that the activity gate closes on 66–72 % of ticks of copyable off-air CW, and the issue's own sweep shows CER tracking gated fraction near-linearly down to a ~14 % model floor. The diff sets the app's activityThreshold to 3 at the single place the app constructs Parameters, leaves the stream default at 12, and extends the carrier regression plus adds a new weak-signal test. The second requirement you set yourself — "no false text on noise or a steady carrier" — is where I'd push, below.

The issue thread's triage comment proposed four fixes. This PR implements item 2 only, and says so. Item 1 (the startup padding that opens the gate on dead air and produces the leading 5555) is untouched — correct to leave out of scope, and it is threshold-independent (your own table shows 5555 at 12, 6 and 3 alike), so it neither helps nor hurts this change. Worth naming in the body as a known-remaining defect so the 5555 in the new test's output isn't mistaken for a side effect of this diff.

2. Scope

File What it changes Claimed? Verdict
src/models/DeepFistCwModel.cpp Extracts the three existing app parameters into appParameters(); sets activityThreshold = 3 Yes In scope. Single call site — I grepped the whole tree for DeepFistStream( and activityThreshold: DeepFistCwModel.cpp:44 and :221 are the only construction paths, so no sibling left unfixed
src/models/DeepFistCwModel.h Declares static appParameters() Yes In scope. New public surface, but internal C++ only — not protocol, settings, or capability surface. Justified by the test needing to exercise exactly what ships
tests/deepfist_cw_model_test.cpp Third carrier arm + weakSignal() Yes In scope; see findings 2–5
tests/tests.cmake Registers deepfist_weak_signal_test with SKIP_RETURN_CODE 77 TIMEOUT 120 Yes In scope, registered correctly inside the ENABLE_DEEPFIST_EXPERIMENT block
docs/deepfist-cw-backend.md Documents the new value Yes In scope; one wording nit below

No CHANGELOG.md entry — correct. No settings keys, no capability records, no dialogs, no AppSettings calls, so the Persistence/Migration/credential/CMake-contract sections of AGENTS.md aren't engaged. No socket tests added or modified in this diff — I read the added test sources directly, not the base tree: no QTcpServer/QUdpSocket/bind/listen/connectToHost/peer process/Fake* anywhere, and weakSignal() drives DeepFistStream in-process. Everything in the diff is explained by the issue.

3. Needs maintainer decision (not a blocker, but don't merge on the code review alone)

1. The value 3 disables the gate rather than re-calibrating it, and diverges from the triage recommendation of 6. Inline at src/models/DeepFistCwModel.cpp:34. Your own justification cites upstream squelch.py as calibrated between dead air at 3.4–3.9 and a tuned-in signal at 41–73. Three sits below the bottom of that dead-air band, and every measurement you report at 3 has 0 % of ticks gated — on weak CW, on strong CW, on the 2026-09-25 take. So after this change the activity gate contributes nothing on any material measured, and dead-air protection rests entirely on the model/committer not emitting. That is not disproven by your evidence (0 characters over 10.5 min of band noise is a real result, and the 201 s empty take is clean), but it is contradicted once, in the same bundle you link: the #5950 follow-up's weak-CQ take, where 3 produced F5 5Q CQCQ OG? R4KTCNQCRQ CQ CQA2 on a signal where only "CQ" was copyable by ear, against F5 5 at both 6 and 12. That row is the one piece of evidence the triage comment cited against 3, and the PR body reports the empty-frequency row from that bundle but not this one. Please add it.

Against that, your new +0.6 dB take is the strongest argument for 3 and is new information the triage analysis did not have: 78.3 % CER at 6 versus 50.9 % at 3, with 80–89 % of ticks still gated at 6 in every tone-to-noise quarter. On that material 6 is still mostly broken and 3 is not. I don't think the 6-vs-3 call can be settled from the code.

One number would settle it and you can already produce it: the distribution of the in-app keying ratio (normalized, post-normalizeActivity, on the ~90 Hz envelope — not the upstream STFT scale) over the 10.5 min of band noise in take B. If dead air reads ≥6 on that scale, then 3 is below the noise floor and the gate is decoration; if it reads ≤2, then 3 has genuine margin and the dead-air numbers you quoted from squelch.py simply don't transfer. Everything else in the argument depends on which of those is true, and the same replay harness that produced the gated-fraction quartiles can emit it.

4. Nits (non-blocking)

  • The negative half of the new assertion fails open. tests/deepfist_cw_model_test.cpp:309 — weakSignalDecode() returns "<failed>" on stream.failed(), which satisfies !previous.endsWith("CQ"). So an inference or model failure in the threshold-12 control arm reads as "the gate correctly closed". The app arm fails closed correctly ("<failed>" doesn't end with " CQ"); only the control is exposed. Inline with a suggestion.
  • The added carrier arm can't distinguish 3 from any other value. appParameters() sets requireCompletedMark = true, and the gate at DeepFistStream.cpp:92-93 is ratio < threshold || (requireCompletedMark && !completedMark). A steady carrier has no falling edge, so hasCompletedMark() is false and the second term gates it regardless — the arm passes identically at 3, 12 or 100. That's a legitimate regression guard for the completed-mark path (and your mutation note says as much: the carrier test still passed at 12), but a reader can easily take // The application's parameters (activityThreshold 3, #5950) must still hold the carrier off as validating the new value. One clause would fix it.
  • "the fading final CQ" (:308, :272) — nothing in the fixture fades. The tone is constant 0.4 and the multitone noise is constant RMS; what closes the gate at 12 is the fixture's trailing 3 s of noise-only tail diluting the keying ratio over the 6 s window. That's a fair analogue for a ratio landing in the gap, but the comment describes a mechanism the fixture doesn't have.
  • The side of the change with no test is the side that can regress. There is no assertion that noise-only audio at 3 emits no more than at 12 — which is exactly the risk lowering the gate creates, and the only one your evidence covers by measurement alone. A socket-free CTest feeding the same seeded multitone with the tone removed through both parameter sets, asserting the app arm's output is no longer than the strict arm's, would pin it. It can't assert "empty", because #5950's root cause 2 (startup padding) emits 5555 at every threshold and isn't fixed here — which is fine, and makes the "no worse than" form the honest one.
  • The third carrier arm copy-pastes the 20-line resample loop verbatim from the two-arm loop above it. A small lambda over Parameters would carry all three.
  • docs/deepfist-cw-backend.md:17 now folds a shipped default into a sentence that ends "These are developer settings". The other three are; the threshold is now what every user of this backend runs. Splitting the sentence would keep the doc's own distinction intact.

5. What I tried to break

  • "the app's parameters did not set it" / single seam. Grepped the head checkout for activityThreshold, appParameters, DeepFistStream(: exactly two app construction paths (DeepFistCwModel.cpp:44 delegating-ctor assignment, :221 worker construction), both now fed from appParameters(). No second decoder path, no dev/settings UI writing the field, no sibling backend carrying a copy of the old three-line block. The refactor is behavior-preserving for the other three fields.
  • "the stream default (12) is unchanged." Confirmed — DeepFistStream.h:16 still reads 12.f, and Parameters::valid()'s >= 1.f bound already admits 3, so no validator change was needed or smuggled.
  • "Steady carriers are held off by the completed-mark guard above, not by this value." Read the gate and hasCompletedMark(): true as written, since the guard is a separate disjunct and a carrier has no key-up. I also checked whether it protects noise — it does not, as far as I can reason from the code: hasCompletedMark() peak-normalizes 5 ms energy bins and accepts any 2–180-bin run above 25 % of peak, which broadband noise will produce routinely. That's why finding 1 turns on the ratio gate alone. Reasoned from code; I could not run it.
  • Fixture determinism. The std::mt19937-direct / no-distribution reasoning is correct — the engine's sequence is standard-fixed, the distribution classes aren't. generator() / 4294967296.0 is in [0,1), and noiseRms * Σ40 sines / sqrt(20) does give RMS 0.09 for 40 unit sines. The fixture is reproducible across standard libraries. What it is not insulated from is ONNX Runtime / CPU numerics: both assertions are knife-edge (endsWith(" CQ") on one arm, !endsWith("CQ") on the other) over a 0.06–0.13 usable band, verified on one Linux/ORT combination. Since the test skips without a model and CI never builds it, the blast radius is a local developer, not the merge gate.
  • Test registration and timeout. deepfist_weak_signal_test is inside the ENABLE_DEEPFIST_EXPERIMENT block, joins the SKIP_RETURN_CODE 77 TIMEOUT 120 group, and main() returns 77 when AETHER_DEEPFIST_MODEL_DIR is unset. The fixture is ~10 s of 3.2 kHz audio × 2 arms ≈ 50 inferences, so 120 s is not tight. Not a retired or unregistered target.
  • CI green means nothing here, and the body says so. All five checks pass on bf9e0338, but no workflow in .github/workflows/ sets ENABLE_DEEPFIST_EXPERIMENT — I grepped. So CI compiles neither the changed model file nor any of the new tests. The PR discloses this accurately rather than leaning on the badge.
  • Governance. GOVERNANCE.md:222 says an agent "may not autonomously change … default values affecting all users", and the next sentence is the escape hatch: "the agent implements the fix and notes in the PR that a design decision needs maintainer review." The body does exactly that, so this is a routed decision rather than a violation, and it is a fix under §6 (the issue documents the old default as defective with a repro), not a smuggled preference. No RFC needed: GOVERNANCE.md:112 exempts "bug fixes with a clear root cause". #5950 already carries maintainer-review.
  • Could not check: anything at runtime. No build, no test run, no replay of your bundles, no radio. Every finding above is reasoned from the head checkout and from your and the triage comment's published numbers; none of it was reproduced. The CER figures, the gated-fraction quartiles and the 10.5-min noise result are taken at your word — the bundle and run.sh are what would verify them, and Principle XI puts that on the squash-merge and the maintainer, not on this review.

6. Recommendation

Needs maintainer decision. The code is clean, the seam is in the right place, the refactor has exactly one call site and doesn't change the other three parameters, and the diff carries no scope I can't explain from the issue. The open question is only the number: 3 puts the activity gate below the dead-air band your own justification quotes and gates 0 % of ticks on every recording measured, so it retires the gate rather than re-deriving it — while your new +0.6 dB take makes a genuinely strong case that 6 is still mostly broken. Concrete next step: post the in-app keying-ratio distribution over take B's 10.5 min of band noise, and add the weak-CQ row at 3 from the #5950 follow-up bundle to the body. With those two, a maintainer can pick 3 or 6 on evidence instead of on judgement. The test nits (fail-open control arm, the carrier arm's independence from the threshold) are worth fixing in this PR either way.


🤖 aethersdr-agent · cost: $7.1974 · model: claude-opus-5

Comment thread src/models/DeepFistCwModel.cpp
Comment thread tests/deepfist_cw_model_test.cpp Outdated
Comment thread tests/deepfist_cw_model_test.cpp Outdated
@rfoust rfoust self-assigned this Sep 26, 2026

@rfoust rfoust left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit

PR #5953 addresses #5950's activity-gate loss: the weak-signal fixture retains its final CQ at the app's threshold 3 and loses it at 12. Both construction and stream recreation receive the app parameters; the other three parameters and the stream default 12 remain unchanged. Arbitrary on-air accuracy and the attached CER figures were not independently reproduced.

Scope

File/group Change Claimed in body? Verdict
src/models/DeepFistCwModel.cpp/.h Shared app parameters; activity threshold 3 Yes In scope
tests/deepfist_cw_model_test.cpp Weak-signal and carrier coverage Yes In scope
tests/tests.cmake Registers optional regression test Yes In scope
docs/deepfist-cw-backend.md Documents threshold Yes In scope

Everything in the diff is explained by the issue. No unrelated settings, protocol, UI, or CHANGELOG changes; the body's scope claims hold up. The changed test adds no socket behavior or synthetic firmware peer.

Blockers

None confirmed in production code.

Maintainer decision

Needs maintainer decision on the value 3. Jeremy/KK7GWY should select the default based on the weak-signal versus false-text tradeoff. GOVERNANCE.md says agents may not autonomously change “default values affecting all users,” then directs them to implement the fix and identify the decision for maintainer review. The body correctly routes this decision. The newer +0.6 dB recording supports 3 over 6, but the issue's weak-CQ take also reports additional unverified text at 3; include that row in the PR body and characterize noise using the app's normalized ratio. See the inline comment at DeepFistCwModel.cpp:34.

Nits — non-blocking

  1. The threshold-12 control arm accepts "<failed>" as successful negative evidence. Independently reproduced with invalid parameters in only that arm; the test remained green. Inline suggestion at test line 309.
  2. Carrier coverage validates the completed-mark guard, not noise immunity at threshold 3. A socket-free noise-only comparison would strengthen coverage; account explicitly for the known startup output. Inline at test line 236.
  3. The constructed fixture does not actually fade. Its comment should describe the final CQ without claiming fading. Included in the suggestion at line 309.

Verification and attacks

  • Reviewed head bf9e03385d63bde95d95eef71f9984e07f5cea40. The initial scratch merge used main 199e2612; after main advanced, reconstructed a clean merge against 74a5ac0841c55d499f124d2b80cb480ba9c90458, integration tree 3db7ff786aad5e4203aeb103b971b12aa1cbdeae. The new Window menu changes do not overlap the decoder path.
  • Configured with the local macOS ARM64 toolchain, RelWithDebInfo, and ENABLE_DEEPFIST_EXPERIMENT=ON; built deepfist_cw_model_test with -j8. RADE remained enabled. Host/system processors and the test executable are ARM64; no RNNoise x86 sources appear in the build graph. This was a focused target build, not a full desktop app build.
  • Cached model and sidecar hashes match the pinned manifest; ONNX Runtime 1.29.1. Five focused tests passed: model contract, carrier, weak signal, worker inference, and injected-download/offline inference. Worker decoding produced CQ TEST CQ at 24/48 kHz in mono/stereo. Carrier and weak-signal tests passed again on latest main.
  • Original weak test: app parameters '5Q TEST CQ'; threshold 12 '5Q TEST' — PASS.
  • Mutated the production app threshold back to 12: app parameters '5Q TEST'; threshold 12 '5Q TEST' — FAIL. Restored the PR source and reran — PASS. The regression test notices removal of the fix.
  • Forced only the control stream into failed() using invalid parameters: app parameters '5Q TEST CQ'; threshold 12 '<failed>' — PASS. This confirms nit 1; it does not show a production inference failure.
  • Test registration, manifest, engine-boundary and frozen CI-gate checks passed. All five hosted checks pass, but their older merge context leaves DeepFist OFF and does not compile the changed decoder path.
  • Read the existing bot review and independently checked its findings. Its categorical claim that 3 disables the app gate is not established by upstream ratios from a different measurement path. No separate automated reviewer was available.
  • No GUI/demo or live radio was driven. Verification used the production stream and worker through socket-free tests; the simulator would not reproduce the attached off-air recordings. Windows with DeepFist ON, the recorded-audio CER/noise claims, static crashes, and nearby digital signals remain unverified.
  • All temporary scratch mutations were restored; the operator's active checkout was preserved.

Recommendation: Needs maintainer decision. The mechanism survives focused ARM64 tests and the threshold-reversion attack. Obtain Jeremy's ruling on 3 and fix the control-arm failure handling before merging.

Comment thread src/models/DeepFistCwModel.cpp
Comment thread tests/deepfist_cw_model_test.cpp Outdated
Comment thread tests/deepfist_cw_model_test.cpp Outdated
@skerker

skerker commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor Author

Premise: don't deny the operator weak-signal detection, at the expense of some noise. Threshold 6 takes that away (below: it gates about 92 % of weak-CW ticks); 3 keeps it. Per-character confidence may be the better long-term way: every character is shown and the doubtful ones are marked, so nobody has to make this trade-off in the future.

The value itself is the maintainers' decision (both reviews route it to Jeremy); the measurements below are offered for that ruling.

In answer to the review's two asks:

1. Where band noise sits on the in-app gate scale. One row per 0.4 s tick, the gate's own ratio (DeepFistStream.cpp:92, app Parameters), take B of this PR's bundle (17 m, 2026-09-25). Measured with a per-tick log added to the bundle's replay tool; the decoded text is byte-identical to the bundled threshold-3 output.

Stretch ticks median p95 max ≥ 3 ≥ 6
band noise 1559 4.00 4.26 4.74 100 % 0 %
weak W1AW announcements 1498 4.16 6.97 11.88 100 % 7.6 %

Band noise and weak CW overlap on this measure, so no threshold separates them. At 3 the gate is closed on 0 % of measured ticks, including pure band noise, and the model is the safety net: 0 characters in 10.5 min of that noise. At 6 the gate blocks all noise and about 92 % of weak-CW ticks, which is why 6 still reads 78.3 % CER on the +0.6 dB take against 50.9 % at 3.

2. The weak-CQ row at 3 (#5950 follow-up). The signal was extremely weak: by ear nothing but "CQ" could be copied, and no decoder got the callsign either. At 3: F5 5Q CQCQ OG? R4KTCNQCRQ CQ CQA2, the CQs plus junk. At 6 and 12: F5 5. On the same audio DeepCW (K5PTB#1) also printed CQs plus junk: RQ CQ C7 CQ DEUB KPT MU T TU 7M P I CQ CQ CQ C4D. This is the measured cost of 3, at the very edge of copy.

The test nits (fail-closed control arm, the carrier arm's independence from the threshold, the fixture wording, a noise no-worse-than-12 assertion) and the docs sentence split land in one amend.

— authored by agent (Claude Code) on behalf of @skerker

@skerker

skerker commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Windows check at bf9e0338, DeepFist ON:

  • Build: MSVC 2022 (19.44), Ninja, RelWithDebInfo, -DENABLE_DEEPFIST_EXPERIMENT=ON -DREQUIRE_ASR_ONNX=ON, ONNX Runtime 1.27. Builds clean, with no warnings in DeepFistCwModel.cpp or deepfist_cw_model_test.cpp.
  • Tests: the 9 DeepFist/CW tests pass with the exp27_bt model bundle, none skipped. That includes deepfist_weak_signal_test, so its seeded-RNG setup holds under MSVC's standard library.
  • App: About reads bf9e0338; DeepFist loads the model and decodes on the demo radio. On-air decode confirmed on a FLEX-8400 (40 m, receive only). A recording of a two-station QSO from that run is kept (5 min 47 s WAV, same build) for further analysis if needed.

— authored by agent (Claude Code) on behalf of @skerker

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit: yes for #5950's activity-gate loss, with a maintainer decision still needed on the value 3. Independent second review at bf9e03385d63bde95d95eef71f9984e07f5cea40, authorized by Jeremy alongside the existing reviewer. The app now constructs and recreates its stream with threshold 3; the three existing flags and stream default 12 stay unchanged. The weak-signal regression detects reverting the fix. This does not claim to solve startup junk, tone-lock/WPM estimation, or arbitrary on-air accuracy.

Scope

File/group Change Claimed in body? Verdict
src/models/DeepFistCwModel.cpp/.h Centralizes app parameters and sets threshold 3 Yes In scope; internal C++ accessor, no new wire/settings surface
tests/deepfist_cw_model_test.cpp App-parameter carrier arm and weak-signal fixture Yes In scope; control-arm weakness below
tests/tests.cmake Registers optional weak-signal test with skip/timeout Yes In scope
docs/deepfist-cw-backend.md Documents threshold Yes In scope

Everything in the diff is explained by the issue. The single commit has no unrelated authored-date outlier. No unrelated UI/default-decoder, settings, protocol, architecture, dependency or CHANGELOG changes. The scope checklist holds up. The modified test and injected HTTP-reply helper are socket-free; no synthetic firmware peer or socket-owning test was built or run.

Blockers

None confirmed in production code. The original bug has a clear root cause, so this is a fix, not a smuggled preference.

Maintainer decision

src/models/DeepFistCwModel.cpp:34: Jeremy/KK7GWY should explicitly choose the experimental app default. GOVERNANCE.md prohibits autonomous changes to “default values affecting all users” and then says, “When in doubt, the agent implements the fix and notes in the PR that a design decision needs maintainer review.” The body correctly routes that decision. The author's subsequent comment supplies the previously requested in-app noise distribution and weak-CQ tradeoff: measured noise and weak CW overlap, and 3 admits both. This is evidence for accepting more candidate text, not proof of universally correct text. The earlier request for those measurements has been answered; the ruling remains open.

Nits — explicitly non-blocking

  1. Reject a failed negative-control decode (tests/deepfist_cw_model_test.cpp:294,309). Independently reproduced: setting only strict.tickSamples = 0 makes the control return <failed>, yet the test exits 0:

    weak signal: app parameters '5Q TEST CQ'; threshold 12 '<failed>'
    

    This does not invalidate the demonstrated threshold-reversion check, but it lets a broken comparison masquerade as gate evidence. Replace the comment and return with:

     // The final CQ survives only with the app's parameters.
     return app != QStringLiteral("<failed>")
         && previous != QStringLiteral("<failed>")
         && app.endsWith(QStringLiteral(" CQ"))
         && !previous.endsWith(QStringLiteral("CQ"));

    “Fading” in the current comment is inaccurate: the constructed signal has constant-amplitude marks plus stationary noise. The promised test amend in the discussion is not present in this unchanged head.

  2. Retain automated noise-only coverage (tests/deepfist_cw_model_test.cpp:236–258). The carrier test exercises completed-mark rejection; it is not a noise-immunity assertion. The issue and PR do contain field-noise evidence, so this is a regression-coverage request, not a claim that noise was never measured. As a diagnostic, I removed the keyed signal from this exact constructed fixture and obtained app parameters '5'; threshold 12 '5': no additional text at 3 on that one fixture, with the known startup character still present. A retained noise-only comparison should explicitly account for startup output.

  3. Clarify evidence denominators and publication semantics (src/core/deepfist/DeepFistStream.cpp:120–138, attachment scripts/gate_by_quartile.py / scripts/noise_window.py). The bundled quartile calculation counts ticks represented by token sightings, not all ticks; the bundled noise script counts token commit labels, not the final publication string. A socket-free probe of the production committer confirmed gated tick, zero input tokens, publication='A ' after two pending sightings; that publication has no token commit label on the gated tick. Qualify those metrics, or record every tick and timestamped final publication for those claims. This is a limit of those diagnostic summaries, not an observed false-text regression or a refutation of the aggregate decode improvement.

Verification and attacks

  • Built the unchanged PR test source and its actual production model, stream, assets, resampler and native DeepFist dependencies in a focused standalone CMake target. Linux x86_64, GCC 16.2.1, Qt 6.11.2, official ONNX Runtime 1.27.0 with the setup script's verified archive hash. Model, sidecar and pinned vendored license all match the three manifest hashes. This was a focused production-source build, not a full desktop build or the repository's entire aethercore link graph.
  • Weak-signal test passed: app parameters '5Q TEST CQ'; threshold 12 '5Q TEST'. Mutating the production app threshold back to 12 failed (exit 1): app parameters '5Q TEST'; threshold 12 '5Q TEST'. Restored head passed again. Carrier test passed: carrier guard disabled: '9'; enabled: ''; app parameters: ''.
  • Production worker inference passed at 24/48 kHz, mono/stereo, each returning CQ TEST CQ; model-contract test passed. Confirmed the control-arm false positive separately as above. The noise-only diagnostic intentionally fails the original CQ assertion; only its emitted text is evidence. All scratch mutations restored; worktree diff clean.
  • Independently recomputed the supplied weak ARLD038 text scores against its 2,562-character reference: edit distances 2,558 / 2,005 / 1,305, yielding 99.8% / 78.3% / 50.9% at 12/6/3. Strong QSO non-space counts match 664/685/687. These verify the supplied output/scoring, not an independent replay from the original audio. No attached scripts were executed. The 22/22 byte-identical replay claim, live FLEX reception, delay distribution and all original audio were not independently reproduced.
  • Registration and frozen CI-gate checks pass. Both constructors, explicit developer-parameter override, reset/recreation and worker ownership were inspected; no new lifecycle/signal-routing change. Separate code-review skill unavailable; manual adversarial review plus independent parallel scope/evidence audit performed.

GUI/bridge boundary: no GUI or radio instance was launched for this DSP-only change. Runtime evidence comes from injected PCM through the production stream and worker; the demo GUI would not reproduce the supplied off-air recordings. No live radio or TX was touched. Native Windows/macOS builds were not rerun here; the later Windows qualification comment is author evidence, and the body's “Windows pending” note is now stale relative to it.

Hosted checks passed, but DeepFist is OFF in those workflows, so they do not compile these changed decoder files. Runs were created 2026-09-26 00:51:41 UTC, before current main 8df6db2f (2026-10-04 18:26:40 UTC); protection has strict: false. Thus green is not current-main or enabled-DeepFist validation. GitHub reports the commit signature verified/valid.

Recommendation: Needs maintainer decision. The fix survives direct inference and the threshold-reversion attack; no production blocker was established. Choose the default using the now-supplied weak-CW/noise tradeoff, fix the negative-control handling, and update the body with the current validation/evidence qualifications before landing.

@ten9876 ten9876 removed their assignment Oct 4, 2026

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit

Yes, for #5950's activity-gate loss. Whether 3 is the right value is still yours to rule on, @ten9876. Reviewed at bf9e03385d63bde95d95eef71f9984e07f5cea40, which is unchanged since 2026-09-26. The app now builds its DeepFistStream from one DeepFistCwModel::appParameters() with activityThreshold = 3. The stream default (12) and the three existing flags are unchanged. The mechanism is the one the issue names, and the previous reviews' executed evidence (the weak-signal test passes at 3 and fails at 12) is consistent with what I read.

How this PR relates to #6103

They are stacked, not duplicates. #6103's branch contains this PR's commit bf9e0338 byte-for-byte, then a merge of main, then #6103's own two commits (41a889ef, 4c9a73e5). Consequences:

  • This PR should be decided, and landed or closed, first. Merging #6103 first would land the threshold change through #6103's review instead of this one.
  • If 3 is accepted, #6103 needs no rebase. Its copy of the change is content-identical, so a merge of main after this squash-merges resolves cleanly.
  • If 3 is rejected or changed, #6103 has to drop or adjust bf9e0338. Every colour-band figure in #6103 was also measured at threshold 3, so its calibration would need re-measuring.
  • #6103's body says it "stays a draft until #5953 is decided". It was marked ready for review on 2026-10-02 (ready_for_review event, 23:15Z) while this PR was still undecided.

Scope

File Change Claimed? Verdict
src/models/DeepFistCwModel.{h,cpp} appParameters(), threshold 3 Yes In scope. Internal static accessor; no settings key or wire surface
tests/deepfist_cw_model_test.cpp Carrier arm at app parameters; new --weak fixture Yes In scope
tests/tests.cmake Registers deepfist_weak_signal_test (skip 77, 120 s timeout) inside if(ENABLE_DEEPFIST_EXPERIMENT) Yes In scope
docs/deepfist-cw-backend.md States the new threshold Yes In scope

There is one commit, with no outlier author date and no CHANGELOG entry. Everything in the diff is explained by #5950. Step-0 preflight: neither changed test opens a socket or uses a fake peer.

Blockers

None in production code.

Maintainer decision

src/models/DeepFistCwModel.cpp:34: the threshold value. This lowers a default for every DeepFist user, which GOVERNANCE.md reserves to the maintainer. The data for the ruling is on the PR:

  • For 3: 50.9 % CER vs 78.3 % at 6 on the +0.6 dB take. Zero characters over 10.5 min of band noise.
  • Against 3: the weak-CQ row (F5 5Q CQCQ OG? R4KTCNQCRQ CQ CQA2 at 3 vs F5 5 at 6/12).
  • Gate behaviour: the author's in-app gate-ratio table shows band noise at a median of 4.00, so at 3 the gate is open on every measured tick. Rejecting noise now rests entirely on the model and committer.

Nits (non-blocking)

  1. tests/deepfist_cw_model_test.cpp:309: the control arm still passes when it fails. weakSignalDecode() returns "<failed>" on stream.failed(), and "<failed>" satisfies !previous.endsWith("CQ"). Both earlier reviews reproduced this: a forced failure gives threshold 12 '<failed>' and the test exits 0. On 2026-09-26 you said this and the other test nits would "land in one amend". Nothing has been pushed since, so the head still carries it. Drop-in:

     // The final CQ survives only with the app's parameters.
     return app != QStringLiteral("<failed>")
         && previous != QStringLiteral("<failed>")
         && app.endsWith(QStringLiteral(" CQ"))
         && !previous.endsWith(QStringLiteral("CQ"));

    That also removes "fading": the fixture has constant-amplitude marks and stationary noise.

  2. tests/deepfist_cw_model_test.cpp:236: say why the carrier arm is safe. It is held off by requireCompletedMark, the second disjunct of the gate at DeepFistStream.cpp:92-93. That makes it pass at any threshold (your own mutation note shows it passing at 12). The comment at DeepFistCwModel.cpp:31-32 already says so; the test's comment ("must still hold the carrier off") reads as threshold evidence and is not.

  3. The body is stale.

    • It still says "Windows (MSVC) and macOS builds with DeepFist ON — pending", but your 2026-09-26 comment reports a clean MSVC build with 9/9 DeepFist tests passing.
    • It reports the empty-frequency result from the #5950 follow-up bundle but not that bundle's weak-CQ row.
    • Please fold both in so the body you hand the ruling over with is complete.

What I verified and what I could not

  • Merge state. bf9e0338 is 118 commits behind main (85f2a124, 2026-10-04 22:40Z). git merge-tree against current main is clean. The only main change to DeepFist-adjacent files since this PR's base is comment condensation in PanadapterApplet.cpp (#6072). The decoder files are untouched, so nothing on main moves the gate semantics this PR is calibrated against.
  • CI. The earliest runs on this head were created 2026-09-26 00:51Z, eight days before main's tip. CI also builds with ENABLE_DEEPFIST_EXPERIMENT OFF, so no check compiled DeepFistCwModel.cpp or these tests. The green badge says nothing about this change.
  • Not run here.
    • No DeepFist model bundle exists on this host. DEEPFIST_MODEL_BASE_URL is empty and the docs say no public download exists, so both model-dependent arms would skip (77), and I did not run them.
    • The executed evidence for the weak-signal test (passes at 3, fails when reverted to 12) is the author's, @rfoust's and the previous ten9876 review's, not mine.
    • I did not drive the GUI. The demo radio's clean CW tone cannot reproduce weak off-air copy, and the claim here is about recordings.
  • Read and held.
    • The default constructor delegates to the directory constructor, which assigns appParameters() (DeepFistCwModel.cpp:37-44).
    • The only bypass is the explicit-Parameters constructor (:67-70), the developer/test override.
    • The worker builds and recreates its stream from m_parameters (:205, :221), so no runtime path reverts to 12.
    • The test fixture's RNG avoids the standard-library distribution classes, so it is portable as claimed, and your MSVC run supports that.

Recommendation

Needs maintainer decision. Nothing in the production change is wrong. What remains is the value, which is Jeremy's call, plus the eight-day-old promised test amend and the stale body. Decide this PR before #6103, because #6103 carries this commit.

— Jeremy KK7GWY + Claude

@ten9876

ten9876 commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Maintainer ruling: activity-gate threshold 3 is accepted.

— Jeremy KK7GWY + Claude

@NF0T NF0T self-assigned this Oct 5, 2026

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: no blockers found — posted as a COMMENT, not an approval, so nothing can auto-merge on it before the promised amend lands

Reviewed at bf9e03385d63bde95d95eef71f9984e07f5cea40.

@rfoust — stale claim. You were assigned on 2026-09-26 04:16:41Z and posted your review six seconds later. Nothing from you since, and the maintainer ruling on the threshold (@ten9876, 2026-10-05 00:30Z: "activity-gate threshold 3 is accepted") arrived while the assignment was still held. I've added myself alongside rather than displacing you, and I'm continuing the review. Your agent may not be releasing the lock when it finishes — please check its prompt, and remove yourself with gh pr edit 5953 --remove-assignee '@me' if you're done here.

What is correct

The root cause is real and the fix sits at the one place the app constructs Parameters. DeepFistStream.cpp:92-93 gates on max(ratio, recentRatio) < activityThreshold || (requireCompletedMark && !completedMark); the stream default 12 is untouched and appParameters() is used by both the constructor and the stream-recreation path. This is a fix with an authority (the issue's measured gated-fraction-vs-CER table), not a smuggled preference, and the one default-affecting decision is now ruled by the maintainer.

Scope

File Claimed? Verdict
src/models/DeepFistCwModel.{h,cpp} Yes In scope; internal accessor, no settings key or wire surface
tests/deepfist_cw_model_test.cpp Yes In scope
tests/tests.cmake Yes In scope; skip 77 + timeout, inside ENABLE_DEEPFIST_EXPERIMENT
docs/deepfist-cw-backend.md Yes In scope

One commit, authored 2026-09-26, no outliers, no CHANGELOG edit. Test-layer boundary: no sockets, peers or Fake* classes anywhere in the diff.

Non-blocking, but please land the promised amend

The head has not moved since 2026-09-26, so the amend skerker promised on 09-26 ("land in one amend") has not landed. All six review threads are still unresolved, and branch protection will refuse the merge on that alone.

  1. tests/deepfist_cw_model_test.cpp:309 — weakSignalDecode() returns "<failed>" on stream.failed(), and "<failed>" satisfies !previous.endsWith("CQ"), so an inference failure in the threshold-12 control arm reads as "the gate closed". Reading the code confirms what @rfoust reproduced. The test still fails if the fix is reverted (the app arm must end in CQ), so it pins the fix, but the control arm can pass for the wrong reason. Apply the suggestion in the thread (previous != "<failed>").
  2. Same line — "fading" is not something the fixture does; the tone is constant and the trailing noise-only tail dilutes the 6 s window.
  3. :236 — the carrier arm is held off by the completed-mark disjunct and passes at any threshold; say so in the comment so it is not read as evidence that 3 is safe on noise.
  4. Stale header comment — DeepFistStream.h still says "the application uses these defaults" on Parameters; the app now overrides the threshold.

What I tried to break

  • Mutated in my head, then by reading: revert the threshold to 12 → app arm loses the final CQ → test fails. Control arm forced to failed() → test still passes (finding 1).
  • Looked for sibling construction sites left on the old parameters: the constructor and the stream-recreation path both use appParameters().
  • Looked for deleted behaviour: the diff removes only the three inline flag assignments, which move into appParameters() unchanged.
  • Noted what 3 means: per the author's own measurements the gate is closed on 0 % of ticks, including pure band noise; dead-air protection rests on the model and committer. That is the maintainer-ruled tradeoff, not a defect.
  • Not done: I did not build or run this. ENABLE_DEEPFIST_EXPERIMENT is OFF in my build tree and I have no model bundle, and the automation MCP server failed to connect, so nothing here was driven through the app. The CER/noise figures are the author's and were not replicated. Windows run at this head is per skerker's comment; CI is green on all five checks.

Merge

Tier 3 only (*, docs/, tests/ → @aethersdr/reviewers), so reviewers can merge. Not merging yet: the amend should land first (a push dismisses this approval anyway), and the six threads need resolving. Suggested squash subject once it does:
fix(cw): lower DeepFist activity-gate threshold to 3 so weak off-air CW is not gated out. Principle VIII. (#5953)

Thanks @skerker for the one-field-at-a-time sweeps and the disclosed mutation checks. #6103 is stacked on this commit and can follow once this lands.

—
👨🏼‍💻 Co-authored by Claude Sonnet 5.5

@NF0T NF0T removed their assignment Oct 5, 2026
…noise-only arm (aethersdr#5950). Principle VIII.

A failed decode in the threshold-12 control arm read as "the gate closed" and passed.
The same constructed noise with the keying removed must not print more with the app's
parameters than at 12. The carrier arm's comment names the completed-mark guard, which
holds a carrier at any threshold. The Parameters comment no longer says the app uses
the defaults unchanged.
docs: the threshold is the value every user runs, not a developer setting.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@skerker

skerker commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@ten9876 @NF0T No blockers identified. Review nits addressed in c0df8d38: the control arm fails closed, the carrier comment and "fading" wording are corrected, the stale Parameters comment is fixed, a noise-only check is added, and the docs sentence is split. The body is updated.

— authored by agent (Claude Code) on behalf of @skerker

@NF0T NF0T self-assigned this Oct 6, 2026

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — re-reviewed at c0df8d389e3317cbfb5a62ecb23cd06f052f53d2

My earlier review (at bf9e0338) found no blockers and listed four non-blocking nits. I diffed bf9e0338..c0df8d38 and each is fixed in code:

  • Control arm — weakSignal() now returns false if the threshold-12 control or either noise-only decode is "<failed>"; the app arm can't be "<failed>" because it must end in CQ.
  • "fading" wording — comment now says the trailing noise-only tail dilutes the 6 s window's keying ratio.
  • Carrier arm — comment now says the completed-mark guard holds the carrier at any threshold, so the arm says nothing about the value 3.
  • Stale Parameters comment in DeepFistStream.h — corrected.
  • Noise-only check (also asked by the bot and @rfoust) — added, through the same production stream: appNoise.size() <= previousNoise.size().
  • Docs sentence — split.

The diff since the first review touches only docs, a header comment and the test, so production behaviour is unchanged. The value 3 was accepted by @ten9876 on 2026-10-05. CI is green on all five checks and the PR is mergeable. I resolved all six review threads: the test threads are addressed in code, and both :34 threads are settled by the ruling.

Not verified by me: I did not build or run the new noise assertion (DeepFist experiment is OFF in my build, no model bundle). The Windows run in the thread was at bf9e0338; CI covers this head, but the --weak test is skipped (exit 77) without the model bundle there. The CER/noise figures are the author's.

Squash subject: fix(cw): lower DeepFist activity-gate threshold to 3 so weak off-air CW is not gated out. Principle VIII. (#5953)

Thanks @skerker — #6103 is stacked on this commit and can follow.

@rfoust — I added myself alongside you earlier because your assignment was stale; I'm releasing mine now, and yours is still on the PR.

—
👨🏼‍💻 Co-authored by Claude Sonnet 5.5

@NF0T
NF0T merged commit abc5b26 into aethersdr:main Oct 6, 2026
5 checks passed
@NF0T NF0T removed their assignment Oct 6, 2026
jensenpat pushed a commit that referenced this pull request Oct 7, 2026
#6099). Principle VIII.

Fixes #6099. Builds on #5953 (merged 2026-10-06 as `abc5b26e`). This PR's commits: `41a889ef`, `4c9a73e5`, and the review round `34bea416` (Blocker 1) and `e1eb9a9e` (Blocker 2). `ee5c816b` merges `main` (`25de97c3`): the two theme files keep `main`'s new token blocks beside this PR's `cw` block, and #5953's change has dropped out of the diff.

## Summary

Each DeepFist letter in the CW decode panel is now coloured by the model's own confidence, in the four colour bands the panel already uses for ggmorse, drawn from four theme tokens: Dark keeps the ggmorse colours, Light gets readable values. The score is the mean posterior over the letter's frames, averaged over the overlapping decode windows that saw it before it settled; cost = 1 − score; the existing bands (< 0.15 green, < 0.35 yellow, < 0.60 orange, else red) apply. Nothing is hidden: the sensitivity slider stays disabled for DeepFist, as before. ggmorse display, callsign spotting and MQTT output are unchanged. DeepFist remains off by default (`ENABLE_DEEPFIST_EXPERIMENT`), so a default build is unchanged.

| Issue criterion | Where |
|---|---|
| DeepFist letters in the four existing colours; ggmorse unchanged | `src/core/deepfist/DeepFistStream.{h,cpp}` (`spanConfidence()`), `DeepFistCommitter.h` (`Piece`, sighting average), `src/models/DeepFistCwModel.{h,cpp}` (`scoredTextDecoded`), `src/models/CwRxModel.{h,cpp}` (`coloredTextDecoded`), `src/gui/PanadapterApplet.{h,cpp}` (`cwCostColor()`, `appendColoredCwText()`), `src/gui/MainWindow_DigitalModes.cpp`, `resources/themes/default-{dark,light}.json` (`color.cw.confidence.*`) |
| Default build unchanged | the GUI and backend code is inside `HAVE_DEEPFIST` / the DeepFist sources; a default build also gets the unconnected `CwRxModel::coloredTextDecoded` signal (beside `unscoredTextDecoded`) and `cwCostColor()`, the ggmorse band table moved into a helper with no behaviour change |
| Socket-free test pins the per-letter score | `tests/deepfist_committer_test.cpp` (existing registered target, extended) |
| Colour table reproduced on a recording not used to choose the method | Held-out W1AW takes ARLP038 and ARLD038 (table in the PR) |

Kept on purpose: the unscored path (`CwRxModel::unscoredTextDecoded`, `PanadapterApplet::appendUnscoredCwText`) has no producer after this change — DeepFist was its only one — and stays as #5716's contract for a backend that reports no per-letter score; its comment now says so.

Left out: no sensitivity filtering for DeepFist (hiding helps mainly on weak signals; marking does not lose text); no claim for hand-sent code; no change to DeepFist's release status (#4817).

## Design choices

- **Score statistic:** mean posterior over the letter's frames while it stays the argmax (`DeepFistStream::spanConfidence()`), then the mean over its sightings in successive windows (`DeepFistCommitter`, same token within 0.12 s). Same letters chained within 0.12 s in one window pair with an earlier window's sightings by order when the counts match, otherwise each takes the closest sighting; closest alone gives a fast `EE` swapped scores once the windows' times shift by half the spacing (test case 25). Chosen by measurement on two W1AW takes (the sighting mean separated correct from wrong copy better than a single window's value).
- **Cost, not confidence, over the wire:** `cost = 1 − score` so the existing panel contract (lower is better, `PanadapterApplet.cpp` band table) applies unchanged — the convention #4817's review set for a neural backend.
- **Separate signal:** `coloredTextDecoded(text, cost)` beside `textDecoded` / `unscoredTextDecoded` on `CwRxModel`, because this cost is on the backend's own scale and must never meet the Sens threshold that `appendCwText()` applies to ggmorse's cost.
- **Colours:** four tokens, `color.cw.confidence.{high,medium,fair,low}`, added to both themes (`docs/style/theme-style-guide.md` §4), seed regenerated, recorded in `docs/theming/canonical-tokens.md`. Dark = the ggmorse colours (`#00ff88 / #e0e040 / #ff9020 / #ff4040`). Light = `#1a8040` (the theme's green) / `#737313` / `#ae5700` / `#c02020` (the theme's red), 4.59 / 4.61 / 4.64 / 5.56 : 1 on `color.background.0` `#f5f5f8`, the CW panel's background. In Light the yellow and orange are darker than their Dark counterparts (≥ 4.5 : 1 needs that); green and red read clearly. ggmorse keeps its literal table (hardcoded-colour ratchet +0); moving it to the same tokens is a separate follow-up.
- **Identifiers** spelled `Color`, matching the codebase.
- **Doc:** `docs/deepfist-cw-backend.md` now says letters are coloured by the model's posterior and never filtered by the sensitivity slider (it previously said output is displayed "without inventing confidence"). `CHANGELOG.md` untouched.

Governance note: colours are on the RFC list; this change reuses the existing bands on an off-by-default experiment and adds four theme tokens (Dark = the existing ggmorse colours, two new Light values) and no dependency, thread, setting or default. DeepFist itself lives under RFC #4817, where this is the decode-quality work #6099 cites. Stated here for the maintainers' call.

## Relation to the triage on #6099

The triage's sketch and this branch agree on: the score computed in `DeepFistStream.cpp`, not the vendored files; a separate per-piece signal carrying (text, cost) pairs rather than a widened `textDecoded`; a separate DeepFist append with no Sens filter and no `cwRxTextDisplayed`; the band table factored into a helper. Two differences: (1) the cross-window average does not go through `m_pending` — the committer keeps a short sighting history (same token id within 0.12 s, pruned after 8 s) that the commit-tick path reads, so the letter that commits on a tick does get its earlier windows; the extended test pins that case. (2) `spanConfidence()` normalises each frame row (max-subtracted softmax), so it yields the same value whether the model export hands back log-probabilities or raw logits; no row-sum test is needed for that.

## Constitution principle honored

Principle VIII — the colour is a measured quantity from the model's own output, scored against published bulletin text before and after the change, with the evidence attached.

Squashed-from: #6103

Co-authored-by: skerker <7691216+skerker@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
ten9876 added a commit that referenced this pull request Oct 11, 2026
…remote operation over CGNAT (#6389)

## Summary

Release prep for **v26.10.2**: the six release files plus the Stable
docs snapshot. Metadata and docs only; no source, test or CI changes.

Cutoff: `origin/main` at `46943eb3c` (#6083, merged 2026-10-11).
Anything that merges after it is folded in before the tag.

**106 merged changes** from **15 human contributors**, AetherClaude (2)
and Dependabot (0). First-time contributor: **@dacacioa**. This was
confirmed by checking each login for a merged PR before the previous
tag, not from `author_association`.

## Changelog

The range `v26.10.1..46943eb` was mapped through the commit-to-PR API
(`repos/…/commits/<sha>/pulls`), not from commit subjects. It holds 106
PRs and no direct commits, and every one is cited in the section.
`v26.10.1` is an ancestor of `main`, so nothing is excluded. #6083
merged before the branch was cut, so it is inside the range and needs no
unmerged-PR banner.

The notes lead with:
- the unified 52 px title bar (#6198);
- docs.aethersdr.com (#6286, #6298, #6294);
- remote operation over CGNAT through the in-radio tailnet shim (#6273,
#6318, #6325, #6319);
- the canon windows (#6227, #6334, #6339, #6340, #6341, #6364, #6372,
#6386).

Then come DVK import, FFT Smooth/De-comb, TCI tune and receiver events,
RTL-SDR multi-receiver delivery, HL2 and ANAN-G2 state memory, the
IC-7300MK2 fixes, the first clean sanitizer runs (stated beside #6350's
TSan red) and Authenticode signing.

Numbers come from the PR bodies. "On upgrading" callouts cover the
behavior changes the bodies flagged:
- HL2 receive audio is 12.04 dB quieter (#6254);
- HL2 host CW is +6.02 dB at a fixed drive (#5975);
- ANAN AGC-T is remapped (#6282);
- sleep prevention is on by default (#6195);
- the Tailscale MTU cap has no opt-out (#6319);
- IC-7300MK2 Auto SQL is withdrawn (#6184).

DeepFist (#5953, #6103) is described as applying only to
`ENABLE_DEEPFIST_EXPERIMENT` builds.

## Docs refresh, not just version strings

- **README**:
- Highlights: FFT Smooth/De-comb on the spectrum line; AetherMap naming;
remote over CGNAT and TCI tune/receiver events on the SmartLink/TCI
line; pre-NR Copy Assist; a new docs.aethersdr.com line.
- Supported Hardware: the KPA1500, marked as not yet validated on
hardware.
- Other radio families: HL2 state restore extended; ANAN-G2 gains AGC-T,
NB2, supply voltage/link and per-radio memory; RTL-SDR now "bundled in
every official package", with WFM stereo, PPM, per-receiver SQL/meters
and DAX/TCI feeds, and multi-receiver explicitly an evaluation.
- Roadmap section: the CTR2 line advanced and a visual-canon line added.
  - Maturity labels are unchanged.
- **ROADMAP**:
- In flight: a v26.10.2 sentence each for aetherd, Icom, ANAN-G2,
RTL-SDR (remaining is now production multi-receiver qualification and
digital FM, RFC #6079), Hermes-Lite 2 and the CTR2 relay (RFC #6332
extensions). A new entry for the visual canon (RFC #6226 / #6239).
  - Queued: RFC #4214 (approved 2026-10-09) added.
- Open RFCs, checked against `label:rfc`: #4214 leaves (approved); #4764
and #4861 leave (shipped by #6198 and #6216); #6346, #6232, #6212 and
#6209 join. #6332 is approved and folds into the CTR2 entry.
- Recently shipped: nine new `(v26.10.2)` entries, still within the
v26.10.x cycle, so no month-rollover trim.
- Backends (six: anan, flex, hl2, icom, rtl, sim) and NR engines (seven)
were checked against the tree; both are unchanged.

## Judgment calls flagged for the maintainer

All of these were decided through AskUserQuestion:
- Version **26.10.2**, date **2026-10-11**, opened ready for review.
- **#6350**, the TSan red in the weekly sanitizer lane: proceed, and
name it below.
- RFCs **#4764** and **#4861** come off Open RFCs as shipped. The issues
themselves are left open for you to close.
- RFC **#4214** goes under Queued (next cycle).
- A **visual canon** In-flight entry, plus a README Roadmap line.
- The **KPA1500** is listed in README Supported Hardware with "not yet
validated on hardware".
- Including **#6083**: you asked for it, and it merged before the
cutoff, so it ships as an ordinary part of the range.

## Known failing on `main`

- **`ctr2_hid_thread_port_test`** fails in the weekly TSan lane (runs of
2026-10-08 and 2026-10-10; #6350). `main`'s CI and Full Suite were green
at `414082197`. The CHANGELOG headline and intro say so: the lanes
achieved their first clean runs this cycle, and the test remains failing
under TSan. This prep does not fix the test, and the release ships with
it red in that lane.

## Scope

| file | change |
|---|---|
| `CMakeLists.txt` | `project(AetherSDR VERSION 26.10.2)` |
| `README.md` | Current version line, plus the prose refresh above |
| `AGENTS.md` | Current version line |
| `CHANGELOG.md` | new `[v26.10.2] — 2026-10-11` section; `[Unreleased]`
stays and stays empty; history below byte-identical |
| `packaging/linux/io.github.aethersdr.aethersdr.metainfo.xml` |
`<release version="26.10.2" date="2026-10-11"/>` first in `<releases>` |
| `ROADMAP.md` | cycle heading → post-v26.10.2, plus the prose refresh
above |
| `docs/user/versioned_docs/version-stable/**`,
`docs/user/versioned_sidebars/version-stable-sidebars.json`,
`docs/user/stable-version.json`,
`docs/user/src/components/LogAnalyzer/rules.json` | docs snapshot
(`tools/docs/snapshot_stable.py 26.10.2`), in its own commit. This is
the first snapshot since #6286. |

## Validation

Output of `check_release_prep.py --mapping mapping.json` at the branch
head: 30 checks, 1 FAIL (explained below), 1 WARN (the snapshot files,
listed in the scope table).

```
PASS v26.10.1 is an ancestor of origin/main
PASS all six version spots say 26.10.2
PASS CHANGELOG heading date equals metainfo release date
PASS appstreamcli validate
PASS metainfo <release> list is date-descending with unique versions
PASS `## [Unreleased]` stays and stays empty
PASS `## [v26.10.2]` sits directly under `[Unreleased]`
PASS the previous section is `[v26.10.1]`
PASS CHANGELOG diff is exactly the inserted section
PASS new section opens with a `### <headline>`
PASS intro sentence has the house shape
PASS new section has a `### Contributors` block
PASS release_contents.py --check (citations, intro and Contributors counts)
     PRs in range: 106  cited: 106; intro 106/15 matches; Contributors (16 entries) matches; first-time ['dacacioa']
     citations outside the range: #4764, #4861, #5468, #6192, #6226, #6271, #6332 (RFCs), #6238, #6350 (issues)
PASS code fences balance / no duplicated table headers or doubled `---` (README, ROADMAP, new section)
PASS every local link in README.md and ROADMAP.md resolves
PASS no heading removed from README/ROADMAP is still linked elsewhere
PASS Recently shipped `(vX)` markers run newest-first and lead with the new version
PASS Recently shipped intro names the current cycle (v26.10.x)
FAIL only the four current-version lines lose the `26.10.1` string outside CHANGELOG
     historical mentions must stay: "label": "26.10.1"
PASS diff touches no src/, tests/, third_party/ or .github/workflows/ path
PASS all six release files are in the diff
WARN files beyond the six (the docs snapshot)
PASS git diff --check
```

- **The one FAIL is the snapshot's version label, not a historical
mention.** `docs/user/stable-version.json`'s `"label"` is the docs
site's current Stable version, and `snapshot_stable.py` rewrites it by
design. The validator predates #6286 and knows only the four
current-version lines. Run on the six-file commit alone, the same check
passes (29 checks, 0 FAIL, 0 WARN).
- The docs build passes: `npm ci && npm run build` in `docs/user`. `node
tools/docs/test_log_rules.mjs` reports 707 checks passed for 45 rules.
The line moves it reports were already on `main`.
- Four merged PRs (#6340, #6341, #6364, #6372) named screenshots to
recapture after merge (`psk-reporter-sidebar.png`, `menu-tools.png`,
`connect-dialog.png`, `rx-agc-calibration.png`), and #6386 named the
panadapter shots (`panadapter.png` and the main-window captures, which
still show the old title strip). They were not recaptured, so the Stable
snapshot carries the older images.
- No configure, build or test was run for this prep; it is metadata
only, and CI builds it.
- `main` CI at the cutoff `46943eb3c`: Docs and CodeQL - Code Quality
are green; CI, Full Suite, CodeQL and the System Libraries Canary were
still running when this was opened. At `414082197`, every one of them
was green.

After merge: tag the resulting **main** commit, not this branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DeepFist drops most of the text on off-air CW: activity gate closed on 66–72 % of ticks (two W1AW bulletins)

4 participants