fix(modem): the FSK4-ACK scan reached 2 s of a 9 s window, and restarted every tick (#1247)

[dc0sk]

Closes #1247.

What was wrong

decode_fsk4_ack_in_stream capped its trial-decode onset scan at FSK4_ACK_SEARCH_SAMPLES = 16_000 — 2 s at 8 kHz — inside a listen window the same function sizes at 4 s, or 9 s when the profile carries the MFSK16 sub-floor rung, which hpx_hf does.

Nothing else could rescue a later ACK: the whole-buffer decode cannot carry a noisy lead (that is why the scan exists), a normal-rung ACK is transmitted as FSK4 rather than K=3, and a retry opens a fresh buffer instead of accumulating.

Measuring first inverted the fix

The issue proposes deriving the cap from the listen window. That is the wrong fix, and the measurement is why: the caller re-invokes the scan on the whole accumulated buffer once per frame of growth (next_fsk4_at), so the cap bounds a redundancy, not a single pass. Raising it multiplies an already-quadratic cost on the thread that must keep draining the capture stream.

Scanning incrementally — resuming past offsets already tried — covers the entire window for less total work, and the constant disappears rather than being re-tuned. There is no cap left to justify.

An outcome-level defect the issue does not mention

The scan returns the first CRC-valid offset. When that ACK failed the caller's session check, the old scan restarted from zero on the next tick and found the same foreign ACK again — every tick, deterministically, for the whole window. A real ACK behind it was never reached.

That is a live lock, not a probabilistic miss. It is reachable only in the session-hash mode; with an E7 MAC key a foreign ACK fails the MAC inside the trial decode and never returns at all.

Why resuming is equivalent, and why that is pinned

A resumed scan matches a rescan only if a trial decode gives the same answer later in the listen. fsk4_ack_at takes &self and mutates nothing — but its demod config carries afc_correction_hz, and the per-chunk decode_fsk4_ack feeds update_afc_estimate on every chunk. The correction stays put only because Fsk4Plugin does not override estimate_afc_hz and inherits the trait's None.

That is an inherited property, not a designed one. Give FSK4-ACK an AFC estimator and resumed and from-scratch scans diverge silently. So it is pinned by a test, following #1177's precedent of pinning the whitening property its gateless scan depends on.

Two defects sabotage found in this work that the passing tests did not

The retry loop hung rather than failed. Sabotaging the resume made the listen spin forever: the inner loop assumed the resume offset always advances. It does, but the loop now enforces it instead of assuming it.

The first fixture was vacuous. With the scan fully sabotaged, all three tests still passed. A FSK4 ACK is almost exactly one chunk long, so chunk-aligned ACKs are decoded by the per-chunk whole-buffer path and the scan never runs. Every offset is now deliberately off the chunk grid, and the reason is written into the fixture.

A third pass tightened the starvation fixture to sit inside the old cap, so it isolates the resume defect. Before that it failed under both sabotages and the two mechanisms were indistinguishable.

Tests

• ack_scan_tests — the offset planner is a pure function, so never-repeat and never-skip are gated directly rather than through an instrument, over uneven growth including sub-step growth and off-grid lengths. (ScanPlanner already does this for the data path; prior art I had missed.) Plus the AFC pin.
• fsk4_ack_scan_reaches_the_whole_window.rs — cap removal, an early-ACK control, and the foreign-ACK starvation, all through the production entry with the capture delivered in chunks via push_frame. The unpaced loopback returns a whole capture in one read, which is why every pre-existing ACK test could not see either defect.

Sabotage-verified with clean attribution: restarting from zero fails only the starvation gate; reinstating the cap fails only the cap gate.

Cost — not the justification, and not claimed as one

One pass is ~0.1 s; today's rescan measured 0.5–1.2 s over a chunked 9 s listen, with a 2× spread I cannot attribute. On hpx_hf the K=3 MFSK16 arm dominates the listen at ~2.5 s regardless, so "cheaper than today" is true of the FSK4 arm in isolation and false of the listen. The only cost claim being made is that raising the cap would have made an already-quadratic path worse, which needs no absolute number.

Consumer

server.rs:1912 is the only production caller of receive_ota_ack_within. My design artifact also claimed the CLI ARQ path reaches it; it does not — that goes through transmit_arq → receive_ack_with_short_fec, which is #1315.

Prior art

#1177 removed this scan's energy gate and pins the whitening property that makes a gateless scan safe — the same pinning idiom used here for the AFC property. ScanPlanner (engine.rs:77) is the never-retry-a-start-position precedent. The K=3 throttle comment already reasoned about O(n²)-over-the-window for the sibling path; the FSK4 side got a constant instead.

Twins

decode_mfsk16_k3_ack has no cap — it walks the whole buffer — so it has no unreachable region and this is not wider on that axis. The real twin is #1315: receive_ack_with_short_fec (ARDOP production, and the CLI's transmit_arq) has no in-stream acquisition at all, which is #1177's shape unfixed, compounded by stage_capture_input reopening the stream per read. Different function, different frame type, own callers — filed rather than folded in.

Implements: REQ-FUN-09

🤖 Generated with Claude Code

https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6