[on8st]

Refs #5463. Not a fix for #5463.

Limits first. Comments only: no build output, no reading, no transmit level
changes. It does not answer the open question on #5463 (a gate ahead of the
level chain, or a different mic-slider law; that is yours), and it is not the
keyed run we owe there. The ALC Gain gauge's face stays −20..+40 dB, so its
upper two thirds, including the yellow (+20) and red (+30) zones, are still
never reached. Rescaling it is a visible change to the panel, so it is not
bundled here.

What it corrects. The HL2 ALC caps its target at unity
(Hl2TxAlc::process, std::min(settings.targetPeak / levels.micPeak, 1.0)),
and Hl2TxDsp::Config has no alcMaxGainDb. Five comments still said
otherwise:

• PhoneCwApplet.cpp, at kAlcGainGaugeMinDb/MaxDb and in
  buildPhonePanel(): the +40 dB top was "the HL2 modulator's makeup ceiling
  (Hl2TxDsp::Config::alcMaxGainDb)" and the yellow/red zones were "makeup
  spent". They now say readings stay at or below 0 dB, as Hl2Backend
  declares TX:ALCGAIN (−20..0), and that −20 is a presentation floor.
• PhoneCwApplet.cpp, the hover formatter: "the ALC both adds makeup and takes
  level away". Now: a reduction reads negative.
• PhoneCwApplet::updateAlcGain(): "a gain pressed against the modulator's
  ceiling". Now: HGauge clamps, so a reduction past −20 dB shows as an empty bar.
• MainWindow_DspApplets.cpp, the speech-processor presets: "the modulator's
  ALC, which applies its own makeup gain on top". Now: it only reduces. The
  preset values are unchanged. Their old reason, that Flex numbers "would
  compress twice", rested on that makeup, and this PR does not offer a new one.

Tests. None added, since no behaviour changes. Static checks (check_*.py --strict) are clean, and
check_a11y.py reports the same two warnings on these files as on main.

Full ctest on 06e78d8a (on main 4e547347): 754 of 756 pass on this Mac. The two that fail are known failures here, and this change touches neither: peripheral_auth_dialog_interaction_test (the #6008 clipped-page check, in the per-case test #6156 introduced) and unified_title_bar_test (its two #6224 checks).

73, ON8ST

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

No Fixes line — the commit says Refs #5463, and this is the comment-side cleanup left over from 5607b565f ("make the ALC reduction-only", #5646), which removed Hl2TxDsp::Config::alcMaxGainDb from the tree but left five comments describing it. Bug fix with a clear root cause, so GOVERNANCE.md wants no RFC. Yes — and every factual claim in the new text checks out against the source:

• src/core/backends/hl2/Hl2TxAlc.h — const double target = std::min(settings.targetPeak / levels.micPeak, 1.0);, plus m_gain = 1.0 on the disabled path and reset(). Never above unity, as the PR says.
• Hl2TxDsp::alcGainDb() is 20.0 * std::log10(std::max(1e-9, m_alc.gain())) → ≤ 0 for all reachable gains.
• src/core/backends/hl2/Hl2Backend.cpp:8048-8049 — def(9, "TX", "ALCGAIN", "dB", -20.0, 0.0, ...), with a comment already saying "Top is 0 dB because the ALC only reduces". The new PhoneCwApplet.cpp:36 text matches the declaration exactly.
• grep -rn alcMaxGainDb src/ → zero hits. The symbol the old comments cited is genuinely gone.
• HGauge::fractionFor() is qBound(0.0f, (v - m_min) / (m_max - m_min), 1.0f) — so the PhoneCwApplet.cpp:1448 claim "HGauge clamps to its own range, so a reduction deeper than the -20 dB floor renders as an empty bar" is correct (m_value itself stays unclamped, so the hover readout still shows the true figure — which is the better behaviour and worth keeping).

Scope

File / group	What it changes	Claimed?	Verdict
src/gui/PhoneCwApplet.cpp	Four comments on the ALC Gain gauge face, zones, hover sign and clamp	Yes — title and body	In scope
src/gui/MainWindow_DspApplets.cpp	One comment on the speech-processor presets	Yes	In scope

Everything in the diff is explained by the stated fix. I verified the "comments only / no behaviour differs" claim structurally rather than taking it: git show HEAD -- src/gui/ | grep -E "^[+-]" | grep -v "^[+-][+-]" | grep -vE "^[+-]\s*//" | grep -vE "^[+-]\s*$" returns nothing — every added and removed line is a // comment or blank. No CHANGELOG.md entry (correct). No new public surface, no capability, no settings key, no default changed.

On the removals: nothing deleted names a symptom or guards a recurrence. The removed lines describe a condition ("the ALC has run out of gain", "half the makeup spent") that the reduction-only stage can no longer produce.

I also checked docs/radio-certification.md:134, which still names Hl2TxDsp::Config::alcMaxGainDb and "40 dB of upward makeup". That one is intentional, not a straggler — the surrounding block is explicitly historical and closes with "once the stage is made reduction-only the signature disappears entirely", and lines 60-76 already record the narrowing to −20..0. No change wanted there.

Blockers

None.

Nits (all non-blocking)

• src/models/MeterModel.h:235-236 — the same stale claim, one layer down, left behind. This is the accessor that feeds the gauge this PR is correcting:

      // Gain the TX ALC is applying, in dB (0 = unity, positive = makeup, negative =
      // reduction). A different measurement from swAlc(), which is the post-ALC level

  "positive = makeup" is the exact belief the PR is removing from PhoneCwApplet.cpp, and today the HL2 is the only radio that publishes TX:ALCGAIN (MainWindow.cpp:8085, PhoneCwApplet.h:64), so nothing can produce a positive value. It reads as deliberately family-neutral, which is a defensible choice for a model accessor — but it now contradicts the GUI comment directly above the widget it drives. One clause would settle it:

      // Gain the TX ALC is applying, in dB (0 = unity, negative = reduction; the
      // HL2, the only radio publishing this meter, caps at unity and so never
      // reports makeup). A different measurement from swAlc(), which is the post-ALC
      // level and sits near target regardless. Not unit-converted: every radio
      // reports it in dB.

• src/gui/MainWindow_DspApplets.cpp:822-824 — the correction deletes the reason and keeps the conclusion. The old text gave a mechanism for the gentler presets ("Matching Flex numbers here would compress twice"). The new text keeps "Gentler than a Flex's own speech processor" and then explains that the downstream ALC does nothing — which argues the opposite way, so the comment now reads as a non-sequitur to the next person who opens it. The preset values (−18/2.5/+4 → −30/6/+12) are correctly left alone; what's missing is one sentence on why they are still right with no makeup downstream. If the honest answer is "they have not been re-tuned since #5646", that is worth saying out loud and filing, since it is the operator-audible half of the same stale belief.

• src/gui/PhoneCwApplet.cpp:33-40 and :302-310 — the comments now document a face that is two-thirds unreachable; is narrowing it the intended follow-up? With readings pinned at or below 0 dB on a −20..+40 face, the ticks at +10/+20/+30/+40 and both coloured zones can never be reached, while the one condition an operator wants flagged — deep reduction, i.e. overdriving — sits on the uncoloured side. The body is upfront that "the gauge face stays −20..+40", so I read this as a maintainer call rather than a defect: should the face be narrowed to the −20..0 that Hl2Backend.cpp:8049 declares, with the warn/red zones moved onto the reduction side, in its own issue? Accurately documenting dead configuration is the right move for a comment-only PR; I just don't think the comment should be where it ends.

• tests/phone_cw_level_meter_state_test.cpp:149 drives a value the new comments say cannot occur. applet.updateAlcGain(12.0f); then check(alcGainGauge->filledFraction() > 0.5f, ...) — +12 dB on the −20..+40 face is fraction 0.533, so the assertion depends on the unreachable upper half of the scale. The test is about plumbing, not physics, so it is not wrong today, but a -12.0f would exercise the range the meter actually produces. Flagging it mainly because it tells you the cost of the previous nit: narrowing the face to −20..0 breaks this test, so the two want to move together.

Other tools' findings

CodeGuard's three CS-VLN-CMDI-BACKTICK CRITICALs are false positives — all three refuted, credited to security-audit (CodeGuard). Each cited line is inside a C++ // comment, and the backticks are Markdown-style quoting of Flex SmartSDR command-plane verbs discussed in prose: MainWindow_DspApplets.cpp:784 (`transmit set speech_processor_*`), :863 (`eq TXsc/RXsc`), PhoneCwApplet.cpp:165 (`mic acc`). C++ has no backtick operator, and neither translation unit contains any process or shell sink — QProcess, system(, popen, exec[lvp], startDetached all return zero matches. The detector appears to be a lexical match on the character that does not distinguish comment from code, which is why a comment-only PR tripped it three times. Nothing else surfaced in the changed hunks: the preset table is indexed by qBound(0, tx.speechProcessorLevel(), 2) against kPresets[3], so it cannot run off the end.

What I tried to break

• The central claim, that the HL2 ALC only reduces. Read Hl2TxAlc::process end to end looking for a path above unity: the release branch moves toward target, which is itself min(…, 1.0); the attack branch only fires when target < m_gain; attackKnots only emits knots for samples the previous gain would push over target; the disabled path is a hard 1.0; reset() is 1.0. No path exceeds unity. Survived.
• The build-arm trap (AETHER_HL2_TX_TXA, ON by default, selects WDSP TXA over the in-tree phasing modulator). If the ALC lived in only one arm the comment would be true in half the builds. It does not: m_alc.process(...) at Hl2TxDsp.cpp:638 is outside every #if, and :649-651 states the level chain is "shared by both builds; the modulator below is the only thing AETHER_HL2_TX_TXA selects". Hl2TxDsp.h:60 adds that WDSP's own create_txa alc runs at max_gain=1.0 with the leveler off, so no makeup hides downstream in the TXA arm either. Survived.
• The second host-modulating backend. applySpeechProcessorToClientComp is gated on hostModulates && canTransmit, not on family, and ANAN declares hostModulates = true (AnanBackend.cpp:483) — so naming Hl2TxAlc would be wrong there. But ANAN is canTransmit = false (:481, "P2Client has no PTT capability"), so the HL2 is the only backend that reaches this code today and the comment is accurate. Worth knowing it goes stale the day ANAN transmits; I did not count that as a finding on a comment PR.
• The -20..0 declaration and the "empty bar" claim. Read Hl2Backend::defineMeters for the real range and HGauge::fractionFor/setValue for the clamp rather than inferring from the constructor. Confirmed, and confirmed no setFillFromRight on m_alcGainGauge — with that flag set, a floor clamp would paint a full bar and the new comment would be backwards. Survived.
• The zone figures. HGauge(min, max, redStart, …, yellowStart) with 30.0f and 20.0f → red +30, yellow +20, exactly as the new comment says. Survived.
• "No behaviour differs." Proved by the grep above rather than assumed. CI is 5/5 green at 06e78d8 (build, check-macos, check-windows, Static checks, Sanitizer option configures) — though on a comment-only diff that is a near-tautology, and none of those lanes run ctest, so green says nothing about the test nit above.
• What I could not check: nothing runtime. I have no build and no HL2, so every claim here is reasoned from the source in the head checkout at /tmp/aetherclaude/pr-6348, not reproduced on hardware. In particular I did not observe the gauge rendering a deep reduction, and I did not run phone_cw_level_meter_state_test.
• Socket tests: none added, modified or removed by this PR.

Recommendation

Approve with nits. This is a careful, well-targeted correction: four comments that described a makeup ceiling deleted a year ago now say what the code does, and the author resisted the temptation to "fix" the gauge face while in there. The claims are individually verifiable and I could not falsify any of them. The one thing I would ask for before merge is the MeterModel.h:235 clause, since leaving "positive = makeup" in the accessor that feeds this very gauge reinstates half the belief the PR exists to remove; the preset rationale sentence is cheap enough to fold in at the same time. The gauge-face question and the +12.0f fixture belong in a follow-up issue together, and that is a maintainer call, not a condition on this diff.

Thanks for chasing the comments down to the symbol level rather than just softening the wording — citing std::min(targetPeak / micPeak, 1.0) in the commit message is what made this quick to verify.

---

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

[aethersdr-agent]

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

• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow_DspApplets.cpp /tmp/aetherclaude/pr-6348/src/gui/MainWindow_DspApplets.cpp:784
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow_DspApplets.cpp /tmp/aetherclaude/pr-6348/src/gui/MainWindow_DspApplets.cpp:863
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/PhoneCwApplet.cpp /tmp/aetherclaude/pr-6348/src/gui/PhoneCwApplet.cpp:165

_{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: $10.8145 · model: claude-opus-5}

[on8st]

Thanks for the review. Fixed in 88faa52c, a new commit on top (no rebase). The diff is still comments only by your own check.

• MeterModel.h, alcGainDb(): "positive = makeup" is gone. It now says that only the HL2 publishes this meter and that its ALC caps at unity, so the value is never positive. I left out "every radio reports it in dB", which sat oddly beside "the only radio".
• The preset comment: you are right that it kept the conclusion and lost the reason. I could not verify "gentler than a Flex's own speech processor" from the tree, so that claim is removed. The comment now says only what the code shows: the makeup comes from this stage, and the ALC downstream only reduces.

The honest answer to your question is the one you guessed. The preset values came in with #4609 (2a76852a) and have not changed since. #5646 made the ALC reduction-only on 2026-09-16 and did not touch them. So they were tuned while the ALC still added makeup and were not re-tuned afterwards. Whether they are still right is not something I can show offline; it needs a keyed listening or measurement pass. I left that out of the comment and am saying it here instead. If you want an issue for it, say so.

Not touched, as you put it: the gauge face and its zones, and the +12.0f fixture in phone_cw_level_meter_state_test. Those move together and are your call.

Full ctest on 88faa52c: 754 of 756, the same two clean-main failures (#6008, #6224).

73, ON8ST

[rfoust]

The comment corrections match the implementation and remain comments-only against the refreshed base 16cfa80b. This addresses the stated documentation cleanup; it does not resolve #5463's room-noise symptom.

Scope

File	Change	Verdict
src/gui/PhoneCwApplet.cpp	Corrects ALC gain, gauge range, sign and clamp explanations	In scope
src/gui/MainWindow_DspApplets.cpp	Removes unsupported makeup/preset rationale	In scope
src/models/MeterModel.h	Corrects the ALC gain accessor description	In scope

Everything in the diff is explained by the stated scope. No blockers or new nits found. The gauge range and preset qualification remain disclosed follow-up concerns, not conditions on this comments-only change.

Verified empirically: clean merge construction against base 16cfa80b46466938a523c8e704a1e19cb7f34ba7 (tree 912b5033834adb47d04184b52dc5a73f5a639df8), all non-comment lines identical to that base, and an ARM64 build/run of the socket-free hl2_tx_alc_test: all ALC path checks passed. The tested ALC implementation is unchanged at the refreshed merge. All five reported CI checks pass at head 88faa52c, although those runs predate the latest main.

Tried to falsify the unity ceiling via attack/release/disabled/reset paths; checked quiet pass-through, over-level/onset/noise target limits, both modulator paths sharing the ALC, meter publisher uniqueness, and gauge clamp arithmetic. The source and focused test support the new comments. The earlier bot backtick alerts point to C++ comments and are false positives.

No tests are added or modified and no socket test was executed. No full app build or GUI session was performed for this comments-only diff; gauge rendering claims were checked from code. Automated review invocation was unavailable. Approving at the operator's explicit request.