Skip to content

feat(cw): DeepCW and DeepFist join ggmorse as CW receive decoders, models downloaded on first use (#4817). Principle VIII. - #6300

Open
skerker wants to merge 8 commits into
aethersdr:mainfrom
skerker:feat/4817-deepcw-backend
Open

skerker wants to merge 8 commits into
aethersdr:mainfrom
skerker:feat/4817-deepcw-backend

Conversation

@skerker

@skerker skerker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part of #4817. This makes the two neural CW receive decoders standard features beside ggmorse, through the backend interface #5716 added: the CW panel's decoder selector offers ggmorse · DeepFist · DeepCW in every build that finds ONNX Runtime (Apple Silicon macOS, Linux, Windows). ggmorse stays the default and still decodes your own sending. Neither model ships with the app: each downloads on first use from its upstream source, at a pinned size and SHA-256, with no AetherSDR copy. ENABLE_DEEPFIST_EXPERIMENT is retired. Merging this is the decision that the two decoders are no longer experimental.

What it delivers against the 2026-09-14 approval on #4817:

  • DeepCW lands first, toggle only. DeepCwRxBackend joins CwRxModel's catalog as "deepcw". The CW Neural compare applet and the second decoder instance stay on the prototype branch. Only the selected decoder runs; the TX decoder stays ggmorse.
  • Download decoupled from the ASR build option (the blocker in the approval). Both models go through the existing DeepFistModelAssets downloader, which has no ASR dependency.
  • Upstream sources only. DeepCW: e04/deepcw-engine at commit 9185d5da (its only model revision). DeepFist: N9BC's exp27_bt-champion release, whose deepfist.onnx and deepfist.onnx.json match main's pins. That release has no LICENSE file, so an asset can now name its own HTTPS source, and the pinned LICENSE bytes come from the DeepFist repository's first commit, 061fc1d7.
  • Backend key stays a string in AppSettings["CwDecoder"] ("ggmorse" / "deepfist" / "deepcw").
  • Licensing: DeepCW follows the approval's attribution route: THIRD_PARTY_LICENSES entry 29 covers the ported code and the downloaded weights, and DeepCwEngine.{h,cpp} now also say the AGPL-3.0 port was modified (per-character emissions), per AGPL §5(a).

DeepCW uses the sliding-window committer from K5PTB#1, which this supersedes. The decisions this needs are listed on #4817 (2026-10-08).

Before merge

Design choices

  • A selector whenever a second decoder exists. The selector, status and Cancel/Retry from Add optional DeepFist CW receive backend #5716 now compile under HAVE_CW_RX_BACKENDS, defined with ONNX Runtime, instead of HAVE_DEEPFIST.
  • DeepCW output is coloured, never hidden by Sens, like DeepFist: one colour per committed chunk (1 − mean CTC confidence). With either neural decoder, Sens, the locks and the search ranges are unavailable, contact cards and received MQTT text come from ggmorse only, and AetherSDR's Zero Beat is disabled (it needs ggmorse's pitch). A FlexRadio's own Autotune is unaffected.
  • DeepCW emission. The committer shows a character only once it is 5 s behind the live edge, so the model's corrections inside that window are used: there is no hard 15 s reset, words are not cut at window seams, and text already shown never changes. This replaces the prototype loop the 2026-09-14 approval described and stands in for the follow-up issue it asked for; I'll file one if a reviewer wants it. Against the prototype loop: ARLD038 51.4 → 9.5 %, ARLP038 45.1 → 7.3 % CER (DeepCW: keep word spaces in the panels; sliding-window decode with time-anchored commit K5PTB/AetherSDR#1). The model loads on the backend's worker, never on the GUI thread.
  • DEEPFIST_MODEL_BASE_URL is now an override: empty selects the published release, so a build directory that cached the old empty value still downloads.
  • Intel Macs get ggmorse only: the pinned ONNX Runtime has no Intel-macOS build, and the code builds without the neural decoders there.
  • AETHER_DEEPCW_MODEL_DIR points at a local model for development, like AETHER_DEEPFIST_MODEL_DIR.
  • The commits separate cleanly (DeepCW engine · DeepCW backend · retiring the option · test fix · docs · review fixes) if you would rather split DeepFist's enablement into its own PR.

Constitution principle honored

Principle VIII. Principle V: the backend choice stays one key in the existing CwDecoder document.

Test plan

Test seams, all socket-free:

  • deepcw_rx_backend_test (new): missing model → unavailable → Retry → stop clears; cancel → Retry → stop clears.
  • deepfist_model_assets_test: new per-asset source case (fetched from its own source; non-HTTPS refused).
  • deepfist_cw_model_test: its unavailable case now passes an empty source, so it never reaches the network now that a real release URL is the default.

Mutation checks: with stop()'s clearing removed, 2 failures (first "stop clears Retry, detail and status"); with the per-asset URL ignored, 2 failures ("asset with its own source is fetched from that source", "non-HTTPS asset source is refused"). Both restored and passing.

CI: no per-PR job installs ONNX Runtime, so neither the neural decoders nor these tests compile in PR CI or in full-suite.yml. They compile in the release workflows (tag / workflow_dispatch) and in system-libs-canary.yml after merge. A maintainer dispatch of the release workflows on this branch would be the strongest pre-merge compile check.

Local (Intel Mac, Qt 6.12, ONNX Runtime 1.23.2): full registered suite at 2d7ce9ef 681/682; the one failure, unified_title_bar_test ("Alt+F still opens the File menu with the menu bar hidden"), compiles only sources identical to main and fails the same way every run here.

  • Local build passes (cmake --build build): clean builds at d065dd4d (Intel Mac), a1b32197 (Linux, Windows) and f58a19ab (Windows)
  • Behavior verified on a real radio if applicable — demo radio; FLEX-8400 RX on Linux and Windows
  • Existing tests pass (CI) — green at f58a19ab; the neural decoder tests don't compile in PR CI (no ONNX Runtime) and ran locally on Linux, Windows and Intel Mac
  • Reproduction steps documented if user-reported bug — n/a, feature

Proof

Built and tested at d065dd4d (About read): clean build, the CW and DeepFist tests above pass, and on the demo radio each decoder produces text on the CW tone, with Zero Beat enabled only for ggmorse. a1b32197 is that commit plus a merge of main at 65b72a65 to resolve one docs conflict with #6298; main's changes since the base touch help links, docs and a CTR2 test, no CW code. The Linux and Windows runs below are at a1b32197; the head, f58a19ab, adds only the Windows model-path fix from @K5PTB's review (rebuilt and re-run on Windows).

Intel Mac, ONNX Runtime 1.23.2 staged by hand, About 2d7ce9ef (2026-10-08): demo radio (DEMO-0001), isolated settings, TX blocked:

  • Selector lists ggmorse, DeepFist and DeepCW; each produced text on the demo's CW tone.
  • Both downloads from upstream: DeepCW model.onnx 15139839 bytes ef120799…; DeepFist deepfist.onnx 13051998 6d2d4e3d…, deepfist.onnx.json 1257 840ceb8d…, LICENSE 1068 9ad70a9e… (via the per-asset source). All match the pins.
  • Persistence: DeepCW selected → app closed and relaunched → DeepCW restored, ready from the cache with no download.
  • Display delay, demo CW tone (Morse L, 18 WPM, noise off), five on/off cycles per decoder, tone off → last character shown:
Decoder Median Range
ggmorse 0.52 s 0.26–0.77 s
DeepFist 1.82 s 1.55–2.83 s
DeepCW 7.57 s 5.95–7.81 s

This is timing on clean synthetic CW, not accuracy. The user manual uses these figures.

Linux, head a1b32197 (2026-10-09/10): build + tests + demo smoke · FLEX-8400 receive run · CW decoder regression set —
aethersdr-pr6300-linux-2026-10-10.zip

Recording (UTC) Signal ggmorse DeepFist DeepCW
ARLD038, 2026-09-24, 17 m strong 49.7 % 13.9 % 9.3 %
ARLD038, 2026-09-25, 17 m very weak 79.9 % 50.9 % 37.8 %
ARLP038, 2026-09-24, 15 m strong 35.7 % 20.5 % 7.3 %
ARLP039, 2026-09-26, 40 m moderate 61.2 % 18.4 % 11.2 %

CER from score_cer.py: normalized text, semi-global alignment, edit distance ÷ bulletin length. One sentence from each decoder; the number in parentheses is that sentence's character errors (wrong + missing + extra characters):

Recording (UTC) Bulletin text ggmorse DeepFist DeepCW
ARLD038, 2026-09-24, 17 m, strong BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE UNTIL EARLY OCTOBER. BARRKE ENC MEW EXU, IS QRV AS AS O06P SG FROM EFATE ANTIL EAE EA5L Y OCTOBEREEE EETIETETA (39) BARRY, KD6XU, IS QRV AS YJ0BP FROMEFATE ANTIL EARLY OCTOBER. (2) BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE ANTIL EARLY OCTOBER. (1)
ARLD038, 2026-09-25, 17 m, very weak BARRY, KD6XU, IS QRV AS YJ0BP FROM EFATE UNTIL EARLY OCTOBER. Y I? ?V IEI TE (50) BURRY, KD6XUG3 IEQRV AS YJ,0BP FROM EA ANTIL RAY DESTOBER (17) BARRY, KD6XUW IS QRV AS YJ0BP FROM EFATE UNTIL EARAEY EIXOBER. (6)
ARLP039, 2026-09-26, 40 m, moderate A CALM PERIOD IS EXPECTED TO FOLLOW DURING THE LAST FEW DAYS OF THE MONTH. A CALM INER EOEI IS E IS EBEPENSEEED TX FGLLOW D?I PINNE EHE LAST FE D EIA3S NTF THE AE?NTH. (36) A CALG ANERIOD IS EXPECTED TO FOLLOW DURING THE LAST FEW DAYS OF THE AONTH. (4) A CALM PERIOD IS EXPECTED TO FOLLOW DURING THE LAST FEW DAYS OF THE MONTH. (1)

Windows, head a1b32197 (2026-10-09/10): build + tests + demo smoke · FLEX-8400 receive run · CW decoder regression set · non-ASCII model path (before) —
aethersdr-pr6300-windows-2026-10-10.zip

Windows, head f58a19ab (2026-10-10): the model-path fix — build, the non-ASCII path after the fix, regression set again —
aethersdr-pr6300-windows-f58a19ab-2026-10-10.zip

  • Build and tests at a1b32197: clean MSVC build with ONNX Runtime 1.27.0 (the copy sherpa-onnx bundles), 0 errors; targeted tests 11/11 offline and 5/5 with the model; full suite 676: 655 pass, 4 skipped, 17 fail. 10 of the 17 also fail on main on this machine; the other 7 are line-ending or offscreen-layout tests whose sources this PR does not touch. Demo smoke as on Linux; display delay medians ggmorse 0.51 s, DeepFist 2.28 s, DeepCW 6.32 s.
  • FLEX-8400, receive only, live 40 m CW, About a1b32197: each decoder selected in turn, reported ready and decoded live signals.
  • Regression set: all 15 decoded texts are byte-identical to Linux, so every number in the tables above holds on Windows. DeepFist's per-letter confidence values differ from Linux in 10 of 9,105, by 0.0001 (same letters). At f58a19ab all 30 output files are byte-identical to the a1b32197 Windows run.
  • DeepCW model in a folder named Modèle (AETHER_DEEPCW_MODEL_DIR): at a1b32197 "Model load failed" (ONNX Runtime received the path as Modᅢᄄle); at f58a19ab "DeepCW ready". The same file in an ASCII folder loads at both heads.

Intel Mac, no ONNX Runtime, a1b32197 (2026-10-09): configure prints ONNX Runtime not found and builds without the neural decoders. On the demo radio, with a saved deepcw choice from a runtime build: no decoder selector, ggmorse decodes the tone, Zero Beat is enabled, and the stale choice causes no error or crash.

Checklist

  • Commits are signed (docs/COMMIT-SIGNING.md)
  • No new flat-key AppSettings calls — the decoder choice is a value in the existing CwDecoder document
  • Code is clean-room — not decompiled, disassembled, or reverse-engineered from a proprietary binary (Principle IV); DeepCW's engine is an attributed port of AGPL-3.0 source, as approved on RFC: DeepCW neural CW decoder as an optional second CW backend #4817
  • All meter UI uses MeterSmoother — n/a, no meters
  • Documentation updated: docs/user/docs/cw-decoder.md (choosing a decoder, delays, downloads; docs(user): real-radio screenshots on every page, with automatic PII redaction. Principle VIII. #6298's decoder-pane screenshot predates the selector), BUILD-OPTIONS.md, docs/deepfist-cw-backend.md, THIRD_PARTY_LICENSES. CHANGELOG.md untouched.
  • Security-sensitive changes reference a GHSA if applicable — n/a

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

skerker and others added 7 commits October 8, 2026 11:34
…the RFC aethersdr#4817 prototype. Principle VIII.

DeepCwEngine is K5PTB's C++ port of e04/deepcw-engine's decode_morse.py
(spectrogram + ONNX session + greedy CTC; inert without HAVE_ONNX).
DeepCwCommitter is the sliding-window, time-anchored commit from
K5PTB#1, and tools/deepcw_replay the offline harness that
re-runs the recorded takes (not built by default). The AGPL-3.0
attribution for the ported code and the downloaded weights is
THIRD_PARTY_LICENSES entry 29.

Nothing is compiled into the app yet; the backend follows.

Refs aethersdr#4817

Co-authored-by: K5PTB <274291579+K5PTB@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Fist. Principle VIII.

RFC aethersdr#4817 (approved 2026-09-14 as a pluggable backend framework, DeepCW
first): DeepCwRxBackend enters CwRxModel's catalog as "deepcw" whenever
ONNX Runtime is found, through the same interface DeepFist uses. Only the
selected backend runs; the TX decoder stays ggmorse.

- Model: e04/deepcw-engine model.onnx at its pinned commit (upstream
  source only, no AetherSDR mirror), size + SHA-256 checked, cached under
  the app data dir; prepared by the existing DeepFistModelAssets
  downloader, so the download no longer depends on AETHER_ASR_ENABLED.
  Loaded on the backend's worker, never the GUI thread.
- Output: coloredTextDecoded, one colour per committed chunk (1 - mean
  CTC confidence), like DeepFist's; the dominant-tone pitch feeds Zero
  Beat and is cleared when the decoder stops or resets.
- UI: the CW panel's decoder selector, status label and Cancel/Retry now
  appear whenever a second backend exists (HAVE_CW_RX_BACKENDS), not only
  in DeepFist experiment builds. The CW Neural compare applet and the
  second decoder instance stay on the prototype branch.

Refs aethersdr#4817

Co-authored-by: K5PTB <274291579+K5PTB@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…odels downloaded from upstream. Principle VIII.

Both neural CW receive decoders now build in every configuration that finds
ONNX Runtime (Apple Silicon, Linux, Windows; the pinned runtime has no
Intel-macOS build), so ENABLE_DEEPFIST_EXPERIMENT is retired. Neither model
ships with the app: each downloads on first use at a pinned size and SHA-256.

- DEEPFIST_MODEL_BASE_URL defaults to N9BC's exp27_bt-champion release, whose
  deepfist.onnx and deepfist.onnx.json match the pinned sizes and hashes.
- That release publishes no LICENSE. An asset may now carry its own HTTPS
  source, and the manifest's LICENSE (the same 1068 pinned bytes) is fetched
  from the DeepFist repository's first commit, 061fc1d7.
- The DeepFist model tests register whenever ONNX Runtime is found.

Refs aethersdr#4817

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e VIII.

With DEEPFIST_MODEL_BASE_URL now defaulting to the published release, the
default-constructed model in deepfist_cw_model_test started a real download
and timed out waiting for "unavailable". The case now passes an empty source,
as the file's decode() helper already does, so it never reaches the network.

Refs aethersdr#4817

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es; DeepCW modification notice. Principle VIII.

- User manual (cw-decoder.md): choosing ggmorse, DeepFist or DeepCW, when
  text appears (measured on the demo radio at 2d7ce9e, tone-off to last
  character, n=5 each: ggmorse 0.52 s, DeepFist 1.82 s, DeepCW 7.57 s
  median), first-use model downloads with Cancel/Retry, what is unavailable
  with a neural decoder, Intel Macs keep ggmorse only.
- BUILD-OPTIONS.md and docs/deepfist-cw-backend.md: no experiment option;
  DeepFist's release URL and LICENSE source; both model-directory overrides.
- THIRD_PARTY_LICENSES: the DeepFist helpers are no longer experimental.
- DeepCwEngine.{h,cpp}: notice that the AGPL-3.0 port was modified
  (per-character emissions, inferLogProbs/greedyEmissions), AGPL-3.0 §5(a).

Refs aethersdr#4817

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and older build dirs keep the DeepFist download. Principle VIII.

- DeepCwRxBackend::stop() clears Retry, detail and status, as
  DeepFistCwModel::stop() does; a stopped backend no longer shows a Retry
  button that cannot act.
- DeepCW reports no pitch, and Zero Beat is disabled for every neural
  decoder (CwDecodeSettings::neuralSelected()). Its window-wide peak bin is
  not a usable pitch estimate. A FlexRadio's own Autotune is unaffected.
- The DeepCW commit hold is a fixed 5 s; the environment override is gone.
- DEEPFIST_MODEL_BASE_URL is an override whose empty value selects the
  published release, so a build directory that cached the old empty value
  still downloads.
- Manual: received MQTT text and contact cards come from ggmorse only.
- Tests: deepcw_rx_backend_test (missing model -> unavailable -> Retry ->
  stop clears; cancel -> Retry -> stop clears) and a per-asset source case in
  deepfist_model_assets_test, both on injected HTTP replies. Mutation: with
  the stop() clearing removed, 2 failures (first "stop clears Retry, detail
  and status"); with the per-asset URL ignored, 2 failures ("asset with its
  own source is fetched from that source", "non-HTTPS asset source is
  refused"); both restored and passing.

Refs aethersdr#4817

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves the one conflict, docs/user/docs/cw-decoder.md: aethersdr#6298's decoder-pane
screenshot is kept, followed by the new "Choosing a decoder" section.

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

K5PTB commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Red-team review: #6300 at a1b32197

Issue fit (#4817): Mostly yes. The 2026-09-14 approval asked for four things, and this PR delivers three. DeepCW lands as a selectable backend behind CwRxModel, with ggmorse as the default and the TX decoder. Only the selected backend runs. The download path no longer depends on AETHER_ASR_ENABLED; that was the approval's blocker. Models come from upstream only, with no AetherSDR mirror. The approval's rename condition is not met, and its request to re-run the recorded take after any change to the harness has not been answered (blocker 2). DeepFist's release enablement is bundled in and disclosed. Its two gates, license and qualification, are still open by the PR's own account.

How this review was done: I read the diff and the touched files at the PR head, plus the upstream model sources. Nothing was built or run. This session could not execute the PR's code, so there was no local build, no test run, no mutation check, no bridge or demo session and no engine-boundary gate. Every runtime statement below is reasoned from the code and labeled that way. The automated code-review pass was not run either; I did the equivalent pass by hand.

Scope

File / group What it changes Claimed? Verdict
src/core/DeepCwEngine.*, DeepCwCommitter.* Ported engine, plus a sliding-window committer Yes In scope
src/models/DeepCwRxBackend.*, CwRxModel.cpp DeepCW backend and catalog entry Yes In scope
src/core/deepfist/DeepFistModelAssets.* Per-asset HTTPS source (used for LICENSE) Yes In scope
CMakeLists.txt Retires ENABLE_DEEPFIST_EXPERIMENT; neural decoders gate on ORT_FOUND; DEEPFIST_MODEL_BASE_URL now defaults to the release Yes In scope. It is a release decision (see below)
CMakeLists.txt:3391, tools/deepcw_replay.cpp Offline replay tool, EXCLUDE_FROM_ALL Only in commit 00d3e60f's headline, not the body Explained by the approval's regression-take request. Nit: say in the body what it is and how to run it
src/gui/*, src/models/CwDecodeSettings.h HAVE_DEEPFIST → HAVE_CW_RX_BACKENDS, a "neural" predicate, labels Yes In scope (predicate nit below)
THIRD_PARTY_LICENSES Entry 29 (DeepCW); DeepFist heading de-"experimental" Yes In scope (gap below)
Docs (BUILD-OPTIONS.md, docs/deepfist-cw-backend.md, docs/user/docs/cw-decoder.md) The three decoders Yes In scope (stale lines below)
tests/* New lifecycle test, per-asset source case, offline fix Yes In scope
src/models/DeepFistCwModel.cpp Comment only n/a Fine

No unrelated files, and CHANGELOG.md is untouched. New surface: the value "deepcw" in the existing CwDecoder document (approved), the AETHER_DEEPCW_MODEL_DIR dev env var, and a new meaning for DEEPFIST_MODEL_BASE_URL. All three are disclosed.

Blockers

1. On Windows, DeepCW cannot load its model when the profile path is not ASCII. src/core/DeepCwEngine.cpp:79

const std::wstring wpath(path.begin(), path.end());

path arrives as UTF-8 from modelPath.toStdString() (src/models/DeepCwRxBackend.cpp:203), and this line widens it one byte at a time. Take a profile such as C:\Users\José\AppData\Roaming\… or a CJK user name. The download succeeds, because Qt handles the path, and the SHA-256 check passes. Then Ort::Session gets a mangled path and the status reads "Model load failed". Retry re-verifies the cached file, which passes, and the load fails again every time. Both sibling ORT loaders get this right: SignalClassifier.cpp:32 uses QString::toStdWString(), and third_party/deepfist/DeepFistModel.cpp:73 uses MultiByteToWideChar(CP_UTF8, …). Reasoned from code; no Windows run. A C++20 fix that keeps the engine Qt-free (add #include <filesystem>; written but not compiled):

#ifdef _WIN32
        // path is UTF-8 (QString::toStdString); widen it as UTF-8, not byte by byte.
        const std::wstring wpath =
            std::filesystem::path(std::u8string(path.begin(), path.end())).wstring();
        m_session = std::make_unique<Ort::Session>(m_env, wpath.c_str(), m_sessionOpts);
#else

2. The recorded weak-signal take has not been re-run, although the approval requires it for any change to the harness. The approval says: "please carry @skerker's recorded take forward as the regression case … any harness change should be re-run against it." This PR replaces the prototype's emission loop with DeepCwCommitter, which is exactly such a change. tools/deepcw_replay.cpp exists to produce the comparison, with its -window, -grow and commit modes. The body reports no results from the 497 s take, only display delay on the demo's clean tone. The committer's central claim is that held text is corrected by later audio and that no character is cut or repeated at a seam. Today nothing measures that claim (Principle VIII). The fix is to run the three modes on the take and put the outputs in the body.

Needs maintainer decision (@ten9876)

  • DeepFist license and qualification. These are the PR's own open items. One fact for the license question: the commit that produced exp27 is 5c63f6fc (2026-07-15, "This is the change that produced exp27"). Its LICENSE is MIT, and it differs from the pinned 061fc1d7 copy only in reading "Brent Crier (N9BC)", at 1075 bytes. The relicense to GPL-3.0-or-later is 3327d368, on 2026-07-20. That supports "MIT at the checkpoint". If MIT is confirmed, 5c63f6fc's LICENSE is the more exact file to pin. Separately, THIRD_PARTY_LICENSES covers the DeepFist native helpers (unnumbered, after entry 26) but has no download-on-demand entry for the DeepFist model weights. Silero, WeSpeaker, Moonshine and DeepCW (entry 29b) each have one. Once the license is settled, the weights need a numbered entry.
  • The rename condition. The approval's first condition was a neutral model manager with CW owning its own catalog. The PR reuses DeepFistModelAssets and types the DeepCW manifest as DeepFistModelAssets::Asset (DeepCwRxBackend.cpp:19-22). That is the same shape the approval objected to with deepCwModelTier() returning an AsrModelTier. The mechanism is already neutral in behavior, so the rename (for example to CwModelAssets) is mechanical. I recommend doing it here, before a third model makes it larger.
  • Compile proof before merge. I confirmed in this head's CI logs (run 37848296940) that build, check-macos and check-windows each log ONNX Runtime not found. None of DeepCwEngine, DeepCwCommitter, DeepCwRxBackend, the newly ORT-gated DeepFist path, or the ORT-gated test targets (deepfist_cw_model_test, deepcw_rx_backend_test) compiled in PR CI. That includes MSVC, where blocker 1 lives. No sanitizer lane runs the new worker thread either. The green checks therefore say nothing about this code. Also, main has moved 26 commits past the CI run's merge base (CI ran 2026-10-08 21:39; main is at 2026-10-09 20:30, d6f6a4f0), although the branch still merges cleanly. The body's suggestion is a release-workflow dispatch on this branch, and I think that should be a merge gate.
  • AGPL and aetherd. Entry 29 and the approval rest on "desktop use is unaffected". aetherd, the headless engine daemon that serves thin UI clients, links aethercore (CMakeLists.txt:1702), and aethercore now compiles DeepCwEngine in every ORT build. Someone who modifies and hosts aetherd would then be within AGPL §13's network clause. This is a question, not an objection. Either confirm that is acceptable, or keep the DeepCW sources out of what aetherd links.

Nits (non-blocking)

  • DeepCwCommitter has no automated test, although its DeepFist counterpart does. deepfist_committer_test runs DeepFist's committer model-free in the default CI graph. DeepCW's committer holds this PR's most intricate logic: the blank-run snap at DeepCwCommitter.cpp:74-78, the word-space acceptance window and the trim at push(). It has no coverage, and deepcw_rx_backend_test never gets past the 404. If decodeAndCommit took log-probs, or an injected source for them, instead of calling eng.inferLogProbs, a socket-free, model-free test could pin "no duplicate or dropped letter across hops and trims". greedyEmissions is already pure and compiles without ORT, so that test would run in PR CI, which is more than any of the PR's current tests do.
  • The neural predicate now matches by negation. CwDecodeSettings.h:21 and PanadapterApplet.cpp:876 changed from == "deepfist" to != "ggmorse". Suppose the stored key is one this build does not offer. refreshCwRxBackend() fails to select it and ggmorse keeps running, yet neuralSelected() returns true and Zero Beat stays disabled, with a tooltip blaming a neural decoder. The code's own comment says "the catalog is meant to grow". Suggested fix (needs #include "CwRxModel.h"):
        static bool neuralSelected()
        {
            const QString key = backend();
            return key != QLatin1String("ggmorse") && CwRxModel::availableBackends().contains(key);
        }
  • A reset discards up to 5 s of held DeepCW text. On every discontinuity, reset or stop, the committer is recreated (DeepCwRxBackend.cpp:221-223) or the worker is joined, and provisional text is dropped. DecoderPcmAdapter raises a discontinuity on any sample gap, so a retune just after the other station's last word loses that word. On a link with gaps less than about 7 s apart, DeepCW would never commit anything: it needs a 5 s window, plus the hold, plus a 2 s hop. DeepFist also invalidates on discontinuity, but its hold is much shorter. Consider emitting committer->flush(*m_engine).text before the committer is replaced. Reasoned from code; not tried on WAN.
  • The Zero Beat tooltip is inaccurate for DeepCW. VfoWidget.cpp:6194 says the neural decoders "provide none", but DeepCwCommitter::Result::pitchHz is computed on every decode and then discarded by the backend. Disabling Zero Beat is a fine choice; the wording should say that the pitch is not used.
  • Comments state history, or state things that are no longer true (AGENTS.md § Comments):
    • DeepCwEngine.h:27,32 and DeepCwEngine.cpp:281 describe a CwDecoder wrapper that resamples and adapts strings. In this tree, DeepCwRxBackend does that.
    • DeepFistModelAssets.cpp:40 still says "Empty until a reviewed, versioned release asset set has been published."
    • DeepCwRxBackend.cpp:194 ("K5PTB's DeepCW worker loop (prototype CwDecoder::decodeLoopDeep)"), DeepCwRxBackend.h:17 and tools/deepcw_replay.cpp:14 carry attribution and history, and refer to a function that is not in the tree. The AGPL provenance header in DeepCwEngine.* is legally required and should stay.
    • DeepCwCommitter.h's header comment runs to 12 lines.
  • User doc: cw-decoder.md:146 still says "Bundled directly — no external dependency", which is no longer true for the neural decoders. The doc also never says DeepCW only hears 400–1200 Hz (DeepCwEngine.h constants). An operator whose CW pitch is outside that band gets no text and no explanation.
  • Joins on the GUI thread. stopWorker() joins on the GUI thread, and the loop sleeps in an uninterruptible QThread::msleep(200) (DeepCwRxBackend.cpp:240). Every stop or backend switch can therefore block for up to about 200 ms plus one inference, or for the rest of a model load.

What I tried to break that held

  • The model pins. I fetched all four assets from the exact URLs in the PR and hashed them locally: DeepCW model.onnx at 9185d5da, deepfist.onnx and deepfist.onnx.json from exp27_bt-champion, and LICENSE at 061fc1d7. Every size and SHA-256 matches. None of the sources is an AetherSDR mirror. model.onnx.json at 9185d5da matches every DeepCwEngine constant: 3200 Hz, FFT 256, hop 48, 400–1200 Hz, 65 bins, 42 classes, blank 41, and the input and output layouts.
  • Tests reaching the network. This PR turns on HAVE_DEEPFIST in every ORT build, which activates deepfist selection in cw_rx_model_test and cw_pcm_consumer_test. Neither test starts the backend. Every deepfist_cw_model_test mode either injects the network or exits 77, and the contract case now passes an empty source. No test reaches GitHub.
  • Audio delivery. Both CwRxModel::feed and feedFixed24 are connected unconditionally (MainWindow_Session.cpp:2659-2661), so DeepCW does receive audio.
  • Resampler overrun. The worker can hand the resampler up to 96000 samples against maxBlockSamples = 4096. Resampler::process chunks its input, so this is safe.
  • MQTT and callsign spotting. Only the ggmorse text path publishes MQTT or feeds the spotter (appendCwText → cwRxTextDisplayed). coloredTextDecoded reaches neither.
  • Per-asset source. The HTTPS, host, query and fragment checks now apply to the final URL, and an empty base with no per-asset URL is still refused.
  • ASR coupling. HAVE_CW_RX_BACKENDS depends only on ORT_FOUND, with no AETHER_ASR_ENABLED anywhere on the path. The TX decoder is untouched.
  • Merge with current main: clean (git merge-tree).

Not tested: the body's mutation results, the demo-radio claims, display-delay figures, and persistence across a restart. All of these needed a build.


Red-team review by Claude Code for @K5PTB, written to be adversarial. Corrections are welcome wherever I attacked the wrong thing.

… manual no longer calls every decoder bundled. Principle VIII.

DeepCwEngine widened the UTF-8 model path byte by byte, so ONNX Runtime got a
mangled name ("Modèle" arrived as "Modᅢᄄle") and "Model load failed" on every
Retry. The path is now converted as UTF-8; non-Windows code is unchanged.

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

skerker commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

@K5PTB's Red-team review (2026-10-10, comment 6092500568), each point checked against the source at a1b32197:

  • Fixed in f58a19ab: on Windows, DeepCW could not load its model from a path containing a non-ASCII character (measured before and after, see Proof in the PR body). cw-decoder.md no longer says every decoder is bundled.
  • The recorded take: the 497 s take is in the regression set on Linux and Windows, decoded the way the app runs (commit, 5 s hold); its outputs are in the Linux and Windows zips.
  • Confirmed, not yet in this PR: a reset or stop drops DeepCW's held text (about 5–7 s); an unknown saved decoder key disables Zero Beat while ggmorse runs; the Zero Beat tooltip says the neural decoders have no pitch, although DeepCW computes one; a few comments describe history; the manual does not mention DeepCW's 400–1200 Hz range; stopping DeepCW can hold the GUI thread for up to about 200 ms plus one inference; DeepCwCommitter has no model-free test. These suit a follow-up PR unless a reviewer wants them here.
  • For a maintainer: the downloader name (DeepFistModelAssets, reused for DeepCW; asked on RFC: DeepCW neural CW decoder as an optional second CW backend #4817), and a THIRD_PARTY_LICENSES entry for the DeepFist weights once the license question in the PR body is settled.

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

@skerker
skerker marked this pull request as ready for review October 10, 2026 07:17
@skerker
skerker requested review from a team as code owners October 10, 2026 07:17

@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.

1. Issue fit

RFC #4817, approved by @ten9876 on 2026-09-14 as a backend framework with ggmorse the permanent default, and explicitly: "DeepFist joins through the same interface without a new RFC, once it has a published model release and Windows/Linux qualification." This PR is the right shape for that approval and lands DeepCW cleanly behind the existing CwRxBackend seam. Partially — the engineering conditions are met; the two decisions the author posed to @ten9876 on 2026-10-08 are still unanswered and one of them gates the diff.

Condition ledger against the approval:

Approval condition State
ggmorse default + permanent, TX stays ggmorse ✅ CwDecodeSettings.h:20 defaults "ggmorse"; TX decoder untouched
Only the selected decoder runs ✅ CwRxModel::selectBackend swaps m_backend, stops the old one
Upstream sources only, no AetherSDR mirror ✅ DeepCwRxBackend.cpp:36 (e04, commit-pinned), CMakeLists.txt:3023 (n9bc release)
Compare applet stays on the prototype branch ✅ grep -rn 'CwNeuralApplet|m_cwDecoderNeural' src/ → nothing
Backend key in AppSettings["CwDecoder"], extensible string ✅ unchanged
Decouple the download from AETHER_ASR_ENABLED ✅ the DeepFist downloader has no ASR dependency
AGPL attribution covering the ported code ✅ THIRD_PARTY_LICENSES entry 29 states both obligations separately; provenance header on DeepCwEngine.{h,cpp}
"Rename the download machinery to a neutral ModelManager" ❌ not done — DeepCW's model downloads through a class literally named DeepFistModelAssets (DeepCwRxBackend.cpp:46) and its requests go out as User-Agent: AetherSDR-DeepFist. This is the same type-abuse the triage flagged for AsrModelTier, one layer down. The author asks keep-or-rename; that is a maintainer call, and I'd note the smell is now visible in the wire traffic, not just the type name.
Follow-up issue for DeepCW's emission work, "filed with the PR" Not filed (search_issues finds only #4817). I don't think it's owed any more: DeepCwCommitter replaces the broken decodeLoopDeep emission rather than deferring it — the hold-and-re-read design is a direct fix for the dropped-revision defect behind @rfoust's 37 % column. Worth saying that in the body so the maintainer isn't looking for an issue that is now moot.
@skerker's recorded take carried forward as the regression case The harness exists (tools/deepcw_replay.cpp, three views incl. the old loop for A/B). The results of replaying that zip through it are not in the PR. That is the single most valuable missing piece of evidence here and it costs one command.

2. Scope

File / group What it changes Claimed? Verdict
src/core/DeepCwEngine.{h,cpp}, DeepCwCommitter.{h,cpp} the ported engine + the new streaming front yes In scope
src/models/DeepCwRxBackend.{h,cpp}, CwRxModel.cpp the third backend behind the interface yes In scope
CMakeLists.txt:3014-3040 retires ENABLE_DEEPFIST_EXPERIMENT; ORT_FOUND now defines HAVE_DEEPFIST/HAVE_DEEPCW/HAVE_CW_RX_BACKENDS yes (title) Needs maintainer decision — see Blocker 1. This flips DeepFist from default-OFF to on-in-every-release-binary; it is a separate decision from adding DeepCW and could be unbundled.
CMakeLists.txt:3388-3398 new deepcw_replay dev tool, EXCLUDE_FROM_ALL yes In scope (the RFC asked for a reproducible replay harness)
HAVE_DEEPFIST → HAVE_CW_RX_BACKENDS rename across MainWindow*, PanadapterApplet*, VfoWidget*, CwDecodeSettings.h mechanical; deepFistSelected() → neuralSelected() yes In scope — correct generalisation; the != "ggmorse" form is the right one for a catalog meant to grow
src/core/deepfist/DeepFistModelAssets.{h,cpp} per-asset absolute url yes In scope, and the validation stayed intact (see §5)
src/models/DeepFistCwModel.cpp one comment no Harmless comment-only
tests/*, tests.cmake per-asset-source test + DeepCW lifecycle test yes In scope
BUILD-OPTIONS.md, THIRD_PARTY_LICENSES, docs/deepfist-cw-backend.md, docs/user/docs/cw-decoder.md docs yes In scope

No unrelated files, no formatting churn, no CHANGELOG.md entry (correct — it must not be added). No new protocol verb, settings key, capability field or CLI flag. No removed guard: the only deleted gates are ENABLE_DEEPFIST_EXPERIMENT (Blocker 1) and the old base-URL validation, which is replaced by a stricter per-source check.

Socket-test disclosure (#5254): tests/deepcw_rx_backend_test.cpp is the one added network-shaped test. It is socket-free — DeepFistTestNetwork (tests/DeepFistDownloadTransport.h:60-72) subclasses QNetworkAccessManager and overrides createRequest(), so nothing binds, listens or connects; the base URL is https://fixture.invalid. No fake radio/amp/tuner peer anywhere. Target deepcw_rx_backend_test, TIMEOUT 20, registered at tests/tests.cmake:8691-8695 — but not run by any CI lane (Nit 1). Recording it here, not as a finding.

3. Blockers

1. CMakeLists.txt:3014-3030 — retiring ENABLE_DEEPFIST_EXPERIMENT ships DeepFist, auto-downloading, in every release binary on an unresolved license. Needs maintainer decision.

-option(ENABLE_DEEPFIST_EXPERIMENT "Build the local DeepFist CW prototype" OFF)
+if(ORT_FOUND)
+    target_compile_definitions(aethercore PUBLIC HAVE_DEEPFIST HAVE_DEEPCW HAVE_CW_RX_BACKENDS)

All three release workflows install ONNX Runtime — appimage.yml:239, macos-dmg.yml:261, windows-installer.yml:189 — so ORT_FOUND is true in every published build and the selector offers DeepFist with a live first-use download. Meanwhile:

  • THIRD_PARTY_LICENSES:622-646 still asserts plain MIT ("DeepFist native helpers", Copyright (c) 2026 Brent Crier) with no Source: line and no pinned revision — unlike every neighbouring entry and unlike the new entry 29, which does pin its source.
  • src/core/deepfist/DeepFistModelAssets.cpp:32-35 downloads that MIT text from n9bc/DeepFist@061fc1d7 — the repository's first commit:
    // The release publishes no LICENSE; these are the pinned bytes from the
    // repository's first commit, fetched from that commit.
  • The author's own open question, asked on #4817 twice (2026-09-28 and 2026-10-08) and never answered: the exp27_bt-champion checkpoint is dated 2026-07-15, and the repository relicensed MIT → GPL-3.0-or-later on 2026-07-20. Fetching the pre-relicense LICENSE blob does not establish that MIT covers this artifact; it establishes what the repo said five days earlier.

The 2026-09-14 approval let DeepFist in "without a new RFC" but conditioned it on "a published model release and Windows/Linux qualification" — it did not rule on the license, because the question did not exist yet. CONSTITUTION.md's Technology Constraints require the THIRD_PARTY_LICENSES update for vendored third-party material; shipping a download that the app presents as MIT-licensed, with no confirmation from upstream, is the part only a maintainer can sign off. @ten9876 — this is the decision the PR is actually waiting on, and it is separable: DeepCW could land on its own with ENABLE_DEEPFIST_EXPERIMENT intact, which would also decouple this from the DeepFist qualification question.

2. src/core/DeepCwEngine.cpp:231-234 and :336-337 — the new port trusts the ONNX output's self-described shape where its sibling validates it. (Confirmed by security-audit (CodeGuard); I verified the asymmetry.)

        const int outFrames = outShape.size() >= 2
            ? static_cast<int>(outShape[outShape.size() - 2]) : specFrames;
        *frames = outFrames;
        return std::vector<float>(lp, lp + static_cast<size_t>(outFrames) * kNumClasses);

The class dimension is never compared to kNumClasses (42) and GetElementCount() is never consulted, so a graph emitting [1, T, C] with C < 42 — or any rank < 2, which silently falls back to specFrames — reads past the output tensor. The DeepFist port in this same tree already does the check it needs:

// src/core/deepfist/DeepFistStream.cpp:122-123
            if (!model.infer(m_window.data(), kWindow, m_logits, frames, classes)
                || frames <= 0 || classes != 48) {

Not attacker-reachable from the app (DeepFistModelAssets verifies length + SHA-256 before the file is ever loaded), which is why I'd accept this as pre-merge hardening rather than a user-facing break — but deepcw_replay loads argv[1] as the model with no verification at all, and the header comment says "Model contract — pinned to model.onnx.json (verbatim), do not drift". A contract worth writing down in capitals is worth asserting. Drop-in, applied at both sites:

        const auto info = outputs[0].GetTensorTypeAndShapeInfo();
        const auto outShape = info.GetShape();
        if (outShape.size() != 3 || outShape[2] != kNumClasses || outShape[1] <= 0
            || info.GetElementCount() < outShape[1] * kNumClasses) {
            std::fprintf(stderr, "DeepCwEngine: unexpected log_probs shape\n");
            return {};
        }
        const int outFrames = static_cast<int>(outShape[1]);

4. Nits (non-blocking)

  • tests/tests.cmake:8676 — if(ORT_FOUND) means no CI lane compiles or runs any of this PR's core. ci.yml contains no onnxruntime reference at all (I grepped the whole file), and the three workflows that do install it are on: push: tags: ['v*']. So the seven green checks on f58a19ab prove the HAVE_DEEPFIST → HAVE_CW_RX_BACKENDS rename compiles — they do not compile DeepCwEngine.cpp, DeepCwCommitter.cpp or DeepCwRxBackend.cpp, and they do not run deepcw_rx_backend_test. Fixable without installing ORT, because the design already allows it: DeepCwEngine.h:35-43 keeps every Ort:: member behind #ifdef HAVE_ONNX and decode() returns empty without it. The precedent is in this same file — deepfist_model_assets_test (tests.cmake:8666-8668) compiles DeepFistModelAssets.cpp directly, outside the gate, and therefore does run in the default graph, which is exactly why its new per-asset-source cases are real coverage. Moving the three DeepCw sources and deepcw_rx_backend_test outside if(ORT_FOUND) (keeping HAVE_DEEPCW gating only the catalog entry and the UI) would put the download lifecycle, the committer and the CTC decode under every CI build.
  • src/core/DeepCwCommitter.{h,cpp} and DeepCwEngine::greedyEmissions have no test of any kind. The only consumers are the replay tool (EXCLUDE_FROM_ALL) and decodeLoop, and deepcw_rx_backend_test answers every request 404 so it never reaches either. This is the correctness heart of the PR — hold-based commit, the blank-run snap-back, word-space acceptance behind m_tLastChar, the drop-and-keep seam — and it is deterministic, Qt-free DSP/scheduling logic, which AGENTS.md's test-layer boundary puts squarely in socket-free CTest. Half of it needs no seam work at all: greedyEmissions(const float*, int, std::vector<uint8_t>*) and ctcDecode are pure functions over synthetic log-probs and need no ORT and no weights. The committer's hold logic does need a seam — decodeAndCommit takes a concrete const DeepCwEngine&, so there is no way to inject log-probs. Not withholding merge for it (no policy-compliant seam exists yet for the second half), but the first half is cheap and would be the test that catches a drift in the alphabet, the blank index or the snap-back rule.
  • tools/deepcw_replay.cpp:55-70 — unvalidated WAV chunk length: 16-byte heap over-read and an unbounded allocation. (Confirmed by security-audit (CodeGuard); I read it independently.) len comes straight off the file and is never checked before four fixed memcpys, the last at offset 14:
            std::vector<char> b(len);
            f.read(b.data(), len);
            std::memcpy(&bits, b.data() + 14, 2);
    A fmt chunk of length 0-15 reads past the allocation (len == 0 leaves b.data() null); std::vector<char> b(len) and std::vector<int16_t> s(len / 2) at :70 will each try up to a uint32_t-sized allocation and throw std::bad_alloc uncaught, which the no-exceptions house style doesn't expect. Dev-only target, no install(), so low severity — but the intended input is "a recorded take a reporter hands you", including the zip attached to #4817. Guard:
            if (len < 16) { std::fprintf(stderr, "short fmt chunk\n"); return false; }
            std::vector<char> b(len);
            if (!f.read(b.data(), len) || f.gcount() < 16) return false;
    and cap the data chunk against the remaining file size before sizing s.
  • src/core/deepfist/DeepFistModelAssets.cpp:40 — stale comment now that CMake substitutes the release URL: "Empty until a reviewed, versioned release asset set has been published." An empty base URL no longer reaches a product build.
  • src/core/deepfist/DeepFistModelAssets.cpp:149 — QStringLiteral("AetherSDR-DeepFist") is now the User-Agent on DeepCW's requests to raw.githubusercontent.com. Neutral string, or pass it in with the catalog.
  • src/models/DeepCwRxBackend.h:75 / .cpp:188-191 — the 4 s ring overflow erases the oldest samples silently and does not set m_resetRequested, so the committer keeps a time base that has lost audio in the middle of its window; on a slow box (the RPi 5 the RFC targets) that shows up as a wrong snap-back rather than a visible glitch. block.discontinuity correctly sets the flag two lines up — the overflow path deserves the same treatment, or at least a qCWarning(lcDsp).
  • src/core/DeepCwCommitter.cpp:36-46 — m_keep = std::min(m_window - hopSec, holdSec + leftContextSec) goes negative for hopSec > m_window, and static_cast<std::size_t>(m_keep * kRate) on a negative double is UB, giving a wildly out-of-range erase. Unreachable today (both callers pass only holdSec), but the 3-arg constructor is public with defaulted params. std::max(0.0, ...) closes it.
  • CMakeLists.txt:3389 — "Declared above the tests include: executables declared after it fail configure." I can't see what in tests/tests.cmake would do that, and the claim reads like a misdiagnosis of something else. Worth either explaining or dropping, since it now constrains where future targets may be declared.
  • CMakeLists.txt:3388-3398 — deepcw_replay is EXCLUDE_FROM_ALL and nothing in CI builds it, so a link break in it is invisible. It adds the r8brain include directory but no r8brain sources, relying on src/core/Resampler.cpp being header-complete. Please state in the body that it builds and links on your platform.
  • docs/deepfist-cw-backend.md:34 has two sentences run together on one line ("…below; a non-empty DEEPFIST_MODEL_BASE_URL replaces that directory. That release publishes no LICENSE;…"). Cosmetic.
  • Pre-existing, not this PR, mentioning only because the audit surfaced it next door: src/models/DeepFistCwModel.cpp:233-234 sizes mono as converted.size() / sizeof(float) and then memcpys converted.size() bytes into it — a 1-3 byte overflow if Resampler::process* ever returns a non-multiple of 4. Lands from #5716; worth its own one-liner sometime, not here.

5. What I tried to break

Findings above are reasoned from the head checkout at /tmp/aetherclaude/pr-6300; I have no build and no way to run the GUI, so nothing here was reproduced at runtime.

  • The downloader, hard, because it is the new attack surface and the new per-asset url field widens it. It holds. The HTTPS/host/query/fragment gate at DeepFistModelAssets.cpp:136-137 is applied to the resolved QUrl source, so an absolute per-asset URL gets exactly the same validation as baseUrl + name — I specifically looked for the common bug of validating the base and then fetching something else, and the hunk moves the check to the right side of that. asset.url.isEmpty() && m_baseUrl.isEmpty() still blocks the unconfigured case. The size cap at :164 precedes the write at :167, so a MITM or hostile mirror cannot make us write unbounded bytes; setReadBufferSize(64*1024) bounds memory; length and SHA-256 are checked at :187 before QSaveFile::commit() at :190 with setDirectWriteFallback(false), so nothing unverified ever appears at the final path; NoLessSafeRedirectPolicy forbids an https→http downgrade. Path traversal is rejected in ensure() at :83-85 before any QDir::filePath. No ignoreSslErrors/setPeerVerifyMode anywhere in the diff. And the two new cases in deepfist_model_assets_test.cpp:121-146 are genuine negative tests, not implementation echoes — the http:// one asserts plain.requests == 0, which fails if the gate is removed.
  • Out-of-bounds in the ported DSP. spectrogram's reflect-pad loops index audio3200[pad - i] and audio3200[last - 1 - i], which is where this class of port usually breaks; with the size() < kFftLength early return at DeepCwEngine.cpp:110 the worst case (exactly 256 samples) touches indices 128…1 and 127…254, both in bounds, and the pad arithmetic matches numpy mode="reflect" (left = x[pad]…x[1], right = x[n-2]…x[n-1-pad]). fftRadix2 is only ever called with the compile-time 256. blank[f-2] in DeepCwCommitter.cpp:76 is guarded by f >= kSnapBlankFrames (3) and f is capped by std::min(frames - 1, …), with a negative f from a short window simply skipping the loop; blank is sized frames at DeepCwEngine.cpp:252. The only bogus-f route I found needs a caller-supplied hopSec, which is the nit above.
  • Backend lifecycle at the usual failure points. Retry after a failed model load works because decodeLoop clears m_workerRun before returning, so retry()'s guard lets it through and stopWorker() joins the dead thread (DeepCwRxBackend.cpp:140-142, :205-208). The destructor disconnect()s before stop(), so no status publishes into a half-destroyed owner. Cancel mid-download bumps m_generation, and readAvailable/finishDownload both re-check a QPointer guard and the generation after every signal emission — I went looking for the destroy-during-progress-callback crash and it is explicitly handled at :159-172 and :178-185. m_assets is declared before m_directory in the header, so the manager gets the un-moved copy and the cache does not land in the process CWD.
  • Persisted-state migration across builds. An operator who selects deepcw on Linux and then opens an Intel-mac build (no ORT): CwRxModel::selectBackend returns false for an unknown key and the model stays on m_ggmorse, while HAVE_CW_RX_BACKENDS being undefined removes the selector and un-gates Zero Beat. Degrades correctly, no stuck state.
  • The two sources of truth for "a neural decoder is selected." PanadapterApplet::neuralEngineSelected() reads the combo's itemData; VfoWidget::refreshCwDecoderControls() reads CwDecodeSettings. They can in principle disagree, but selectCwRxBackend writes the setting and then broadcasts refreshCwDecoderControls() to every VFO across every pan (MainWindow_DigitalModes.cpp:107-119) — and the comment in refreshCwRxStatus shows someone already got bitten by the single-applet version of exactly this. I could not construct a divergence.
  • Docs claims I checked against code rather than taking on faith. "Characters are coloured by the model's own confidence … and are never hidden" — PanadapterApplet.cpp:923-937 confirms appendColoredCwText never compares against m_cwCostThreshold, and the cost arrives already inverted (1.0f - res.meanConf, DeepCwRxBackend.cpp:237), so the sensitivity slider is not inverted the way the triage warned. "MQTT / callsign cards come from ggmorse only" — the coloured path does not emit cwRxTextDisplayed, which is the signal those hang off.
  • Could not check: anything requiring a build or a run, so the deepcw_replay link, the real download from either upstream, the measured 7.6 s DeepCW latency, and the Linux/Windows qualification claims are all unverified by me. The replay results against @skerker's #4817 zip are the one piece of evidence that would settle the emission rewrite, and they are not in the PR.

6. Recommendation

Needs maintainer decision. The code is good work — the backend seam is reused rather than bent, the downloader's new per-asset source kept its validation and gained two real negative tests, the worker lifecycle survived the destroy-during-callback and retry-after-failed-load attacks I aimed at it, and DeepCwCommitter is a genuine fix for the dropped-revision emission defect that the approval was willing to merge around. What it cannot settle on its own is Blocker 1: retiring ENABLE_DEEPFIST_EXPERIMENT ships DeepFist in every release binary on a license question the author has now asked twice without an answer, and that is separable from adding DeepCW.

Concrete next step: @ten9876 to rule on the DeepFist checkpoint license and on the downloader keep-or-rename; and regardless of the ruling, fix Blocker 2 and move the three DeepCw sources plus deepcw_rx_backend_test outside if(ORT_FOUND) so CI compiles and exercises what this PR is actually about. Adding a greedyEmissions/ctcDecode test over synthetic log-probs would then come for free. Thanks for the thorough docs and for putting the open decisions on the issue before opening the PR rather than after — it made this much faster to review.

The CodeGuard bulk findings in the harness report (CS-VLN-CMDI-*, CS-VLN-SSRF-*, CS-VLN-XSS-EVENT, CS-PII-*, CS-MAL-SCRIPT) all sit in files this diff does not touch — the workflows, docs/user/screens.json, LogAnalyzer/index.js, check_release_assets.py — and are not attributable to this PR. The two leads that survived verification are credited inline above.


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

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Cisco CodeGuard — static analysis of this PR (1019 finding(s))

  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .claude/skills/tag-release/scripts/check_release_assets.py /tmp/aetherclaude/pr-6300/.claude/skills/tag-release/scripts/check_release_assets.py:70
  • [LOW] CS-PII-IP — PII detected: ip_address in .claude/skills/tag-release/scripts/check_release_assets.py /tmp/aetherclaude/pr-6300/.claude/skills/tag-release/scripts/check_release_assets.py:242
  • [LOW] CS-PII-IP — PII detected: ip_address in .claude/skills/tag-release/scripts/check_release_assets.py /tmp/aetherclaude/pr-6300/.claude/skills/tag-release/scripts/check_release_assets.py:242
  • [MEDIUM] CS-MAL-SCRIPT — Script detected: env python in .claude/skills/tag-release/scripts/check_release_assets.py /tmp/aetherclaude/pr-6300/.claude/skills/tag-release/scripts/check_release_assets.py:0
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/codeql.yml /tmp/aetherclaude/pr-6300/.github/workflows/codeql.yml:22
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/codeql.yml /tmp/aetherclaude/pr-6300/.github/workflows/codeql.yml:27
  • [CRITICAL] CS-VLN-CMDI-SUBSHELL — Vulnerability: CS-VLN-CMDI-SUBSHELL in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:464
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:14
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:36
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:81
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:172
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:291
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:314
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:322
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:337
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:367
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:382
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:437
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:499
  • [MEDIUM] CS-VLN-CMDI-ENVVAR — Vulnerability: CS-VLN-CMDI-ENVVAR in .github/workflows/static-checks.yml /tmp/aetherclaude/pr-6300/.github/workflows/static-checks.yml:502
  • [CRITICAL] CS-VLN-CMDI-SUBSHELL — Vulnerability: CS-VLN-CMDI-SUBSHELL in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:416
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:48
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:239
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:249
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:321
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:345
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in .github/workflows/windows-installer.yml /tmp/aetherclaude/pr-6300/.github/workflows/windows-installer.yml:619
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in docs/architecture/aetherd-touchpoint-tags.json /tmp/aetherclaude/pr-6300/docs/architecture/aetherd-touchpoint-tags.json:1115
  • [HIGH] CS-VLN-SSRF-LOCALHOST — Vulnerability: CS-VLN-SSRF-LOCALHOST in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2579
  • [HIGH] CS-VLN-SSRF-LOCALHOST — Vulnerability: CS-VLN-SSRF-LOCALHOST in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:4219
  • [MEDIUM] CS-PII-EMAIL — PII detected: email in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:3877
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:1122
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2579
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2625
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2659
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2756
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:2796
  • [LOW] CS-PII-IP — PII detected: ip_address in docs/user/screens.json /tmp/aetherclaude/pr-6300/docs/user/screens.json:4219
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in docs/user/src/components/LogAnalyzer/index.js /tmp/aetherclaude/pr-6300/docs/user/src/components/LogAnalyzer/index.js:61
  • [HIGH] CS-VLN-XSS-EVENT — Vulnerability: CS-VLN-XSS-EVENT in docs/user/src/components/LogAnalyzer/index.js /tmp/aetherclaude/pr-6300/docs/user/src/components/LogAnalyzer/index.js:169

Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.


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

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Antares Detector — candidate vulnerable file(s)

  • src/core/deepfist/DeepFistModelAssets.cpp

DeepFistModelAssets downloads model files from URLs using QNetworkAccessManager and QNetworkReply. The download logic constructs a QUrl from asset.url/baseUrl and issues an HTTPS GET; it reads bytes and writes them to a QSaveFile, which is a safe file write. No unsafe deserialization or path traversal is evident in this implementation.

Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting.


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

This branch has not been deployed

No deployments
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.

2 participants