Repository navigation
fix(cw): lower DeepFist's activity-gate threshold to 3 so weak off-air CW is not gated out (#5950). Principle XI. - #5953
Conversation
…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>
There was a problem hiding this comment.
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>"onstream.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()setsrequireCompletedMark = true, and the gate atDeepFistStream.cpp:92-93isratio < threshold || (requireCompletedMark && !completedMark). A steady carrier has no falling edge, sohasCompletedMark()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 offas 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
5555at 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
Parameterswould carry all three. docs/deepfist-cw-backend.md:17now 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:44delegating-ctor assignment,:221worker construction), both now fed fromappParameters(). 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:16still reads12.f, andParameters::valid()'s>= 1.fbound 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-distributionreasoning is correct — the engine's sequence is standard-fixed, the distribution classes aren't.generator() / 4294967296.0is in [0,1), andnoiseRms * Σ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_testis inside theENABLE_DEEPFIST_EXPERIMENTblock, joins theSKIP_RETURN_CODE 77 TIMEOUT 120group, andmain()returns 77 whenAETHER_DEEPFIST_MODEL_DIRis 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/setsENABLE_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:222says 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:112exempts "bug fixes with a clear root cause". #5950 already carriesmaintainer-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.share 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
rfoust
left a comment
There was a problem hiding this comment.
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
- 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. - 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.
- 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 main199e2612; after main advanced, reconstructed a clean merge against74a5ac0841c55d499f124d2b80cb480ba9c90458, integration tree3db7ff786aad5e4203aeb103b971b12aa1cbdeae. The new Window menu changes do not overlap the decoder path. - Configured with the local macOS ARM64 toolchain,
RelWithDebInfo, andENABLE_DEEPFIST_EXPERIMENT=ON; builtdeepfist_cw_model_testwith-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 CQat 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.
|
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 (
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: 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 |
|
Windows check at
— authored by agent (Claude Code) on behalf of @skerker |
ten9876
left a comment
There was a problem hiding this comment.
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
-
Reject a failed negative-control decode (
tests/deepfist_cw_model_test.cpp:294,309). Independently reproduced: setting onlystrict.tickSamples = 0makes 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.
-
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 obtainedapp 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. -
Clarify evidence denominators and publication semantics (
src/core/deepfist/DeepFistStream.cpp:120–138, attachmentscripts/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 tokencommitlabels, not the finalpublicationstring. A socket-free probe of the production committer confirmedgated tick, zero input tokens, publication='A 'after two pending sightings; that publication has no tokencommitlabel 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
aethercorelink 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-reviewskill 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
left a comment
There was a problem hiding this comment.
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
mainafter 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_reviewevent, 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 CQA2at 3 vsF5 5at 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)
-
tests/deepfist_cw_model_test.cpp:309: the control arm still passes when it fails.weakSignalDecode()returns"<failed>"onstream.failed(), and"<failed>"satisfies!previous.endsWith("CQ"). Both earlier reviews reproduced this: a forced failure givesthreshold 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.
-
tests/deepfist_cw_model_test.cpp:236: say why the carrier arm is safe. It is held off byrequireCompletedMark, the second disjunct of the gate atDeepFistStream.cpp:92-93. That makes it pass at any threshold (your own mutation note shows it passing at 12). The comment atDeepFistCwModel.cpp:31-32already says so; the test's comment ("must still hold the carrier off") reads as threshold evidence and is not. -
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.
bf9e0338is 118 commits behindmain(85f2a124, 2026-10-04 22:40Z).git merge-treeagainst currentmainis clean. The onlymainchange to DeepFist-adjacent files since this PR's base is comment condensation inPanadapterApplet.cpp(#6072). The decoder files are untouched, so nothing onmainmoves 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 withENABLE_DEEPFIST_EXPERIMENTOFF, so no check compiledDeepFistCwModel.cppor these tests. The green badge says nothing about this change. - Not run here.
- No DeepFist model bundle exists on this host.
DEEPFIST_MODEL_BASE_URLis 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.
- No DeepFist model bundle exists on this host.
- Read and held.
- The default constructor delegates to the directory constructor, which assigns
appParameters()(DeepFistCwModel.cpp:37-44). - The only bypass is the explicit-
Parametersconstructor (: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.
- The default constructor delegates to the directory constructor, which assigns
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
|
Maintainer ruling: activity-gate threshold 3 is accepted. — Jeremy KK7GWY + Claude |
NF0T
left a comment
There was a problem hiding this comment.
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.
tests/deepfist_cw_model_test.cpp:309—weakSignalDecode()returns"<failed>"onstream.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 inCQ), so it pins the fix, but the control arm can pass for the wrong reason. Apply the suggestion in the thread (previous != "<failed>").- Same line — "fading" is not something the fixture does; the tone is constant and the trailing noise-only tail dilutes the 6 s window.
: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.- Stale header comment —
DeepFistStream.hstill says "the application uses these defaults" onParameters; 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_EXPERIMENTis 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
…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>
|
@ten9876 @NF0T No blockers identified. Review nits addressed in — authored by agent (Claude Code) on behalf of @skerker |
NF0T
left a comment
There was a problem hiding this comment.
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 inCQ. - "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
Parameterscomment inDeepFistStream.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
#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>
…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>
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
activityThreshold12 (src/core/deepfist/DeepFistStream.h:16, test atsrc/core/deepfist/DeepFistStream.cpp:92); the app's parameters (src/models/DeepFistCwModel.cpp) did not set it. The 12 comes from n9bc/DeepFisttools/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
activityThresholdto 3. The app's parameters move into oneDeepFistCwModel::appParameters()so tests can exercise exactly what the app runs; the stream default (12) is unchanged.docs/deepfist-cw-backend.mdlists 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.
appParameters()setsactivityThreshold = 3Constitution 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):
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.txtin this PR's bundle).deepfist-thr*.login the bundle).5s at 0.9–3.1 s appear at every threshold). "Committed" counts token sightings the stream labelledcommit; the published text agrees: at 3 it reads555and then4V DE W1AW, and that4has 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-start5555at 3.F5 5Q CQCQ OG? R4KTCNQCRQ CQ CQA2at 3 againstF5 5at 6 and 12 — the CQs plus junk.555; and a 40 m QSO ("I LIKE THE LADDER INE BUT HVNT HADANY FER YEARS …").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):JACEK, SP5EAQ, IS QRV AS 5W0AF F NOM SAMOA THROUGH OCTOBER 12 SK[5]JACEK, SP5VQ, IS QRV AS 5W0AF FROM SAMZ1ONR 12.[18]JACEK, SP5EAQ, IS QRV AS 5W0AF F IOM SAMOA THROUCH OCTOBER 124[4]JACK,SPFQ, IS QRV AS 5W0AF FRQM SAMO 12.[23]L A THR[55]T CEK, SP5EAQTTB 5W0SF F TM SAMOA THR WH NH OF ET?M EV[30]?EESE QRV AS 5W?AOR?I ?B[44]BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE ANTIL EARLY OCTOBER.[1]BARRY, KD6XUW IS QRV AS YJ0BP FROM EFAT UNTIL EARAEY EICTOBER.[6]BARRY, KD6XU, IS QRV AS YJ0BP FROMEFATE ANTIL EARLY OCTOBER.[2]BURRY, KD6XUG3 IEQRV AS YJ,0BP FROM EA ANTIL RAY DESTOBER[17]BARR , KD6XU, IS QRV AS YJ0 MEFATE ANTIL EARLY OCTOBER .[9]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
c0df8d38, Linux (Ubuntu 24.04.5, g++ 13.3, Qt 6.8.3),-DENABLE_DEEPFIST_EXPERIMENT=ON; Windows atbf9e0338(MSVC 19.44, DeepFist ON): clean build, the 9 DeepFist/CW tests pass with the model, none skipped (comment)c0df8d38; CI builds with DeepFist OFF, so it does not compile the changed files. Locally with the model: 9/9 pass (8deepfist*+cw_rx_model_test)run.shin the attached bundleTests. In the existing
deepfist_cw_model_testtarget:deepfist_carrier_regression_testnow also runs a steady carrier throughappParameters()and expects no text (the completed-mark guard holds a carrier at any threshold, so this arm guards that path, not the value 3); newdeepfist_weak_signal_testdecodes the existing CQ TEST CQ fixture plus CONSTRUCTED in-band noise (40 tones at 550–650 Hz, seed 5950, values taken straight fromstd::mt19937so the fixture is the same on every standard library) and expects the final CQ to survive atappParameters()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 atappParameters()than at 12 (5/5), and a failed decode in any arm fails the test. Mutation (run atbf9e0338): withappParameters()set back to 12,deepfist_weak_signal_testfails (5Q TEST/5Q TEST) and the carrier test still passes; restored, both pass. Like the other DeepFist tests they are built only withENABLE_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.shreproduced 22/22 outputs byte-identical (replays, comparison tables, fade profiles,signal-levels.txt).run.sh, decoder outputs, comparison tables, signal levels, scripts, logs, screenshotsUnzip the audio parts into the bundle's
audio/folder, then runrun.sh(usage in the README).Checklist
AppSettingscalls — n/a, no settings changeMeterSmoother— n/a, no meter UIdocs/deepfist-cw-backend.md;CHANGELOG.mduntouched— authored by agent (Claude Code) on behalf of @skerker