fix(modem): a cap-flushed burst is not evidence about the rate ladder (#1255) - #1307
Merged
Conversation
…#1255) `accumulate_routed` returned `Ok(Some(burst))` for two opposite events — the carrier dropped (a complete transmission) and the accumulator hit its runaway cap (the carrier was STILL UP, so the slab is not one transmission) — with nothing to tell them apart. A failed decode of a capped slab therefore drove `RxOutcome::Failed`. That does two things and only one is bounded. The NACK keying is capped by `OTA_NACK_BUDGET` in the daemon; the rate controller's demotion is not, so successive capped slabs walk `recommended_level` down and the next real ACK carries it to the peer. The engine now records the flush reason and consults it in `ota_decode_and_ack_inner`, returning `ack: None`. That lands in the discrimination #1123 already built — the daemon's `ladder_frame` branch already means "key nothing, leave the budget alone" — so there are zero daemon changes and no `BurstEnd` type threaded through ~19 test files. The decode still runs, deliberately: when the squelch sits below the band floor EVERY burst is a cap flush (#1254's regime), so skipping it would make the daemon deaf on a hot band rather than merely quieter. A control pins that a frame at the head of a capped slab still decodes, which also falsifies the issue's premise that a capped burst is never a legitimate frame. Implements: REQ-DCD-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1255, re-scoped: from "add a
BurstEndtype" to "a cap flush is not ladder evidence". Two of the issue's premises did not survive checking — see below.What was wrong
accumulate_routedreturnedOk(Some(burst))for two opposite events with nothing to tell them apart:A failed decode of a capped slab therefore drove
RxOutcome::Failed.The severity is the controller, not the keying
I filed this thinking of spent RF. That half is bounded:
server.rscaps NACK keying atOTA_NACK_BUDGETconsecutive failures. The rate controller's demotion is not — successive capped slabs walkrecommended_leveldown, and the next real ACK carries it to the peer. That is the un-budgeted half, and it is the reason to fix this.The shape: no new type, no daemon change
The engine already owns both ends. It records the flush reason and consults it in
ota_decode_and_ack_inner, returningack: None— which lands in the discrimination #1123 already built, whereladder_frame = res.ack.is_some()already means "key nothing, leave the budget alone".So: zero daemon changes, and no
BurstEndthreaded through the ~19 test files that callaccumulate_capture. The flag istake()n before the OTA borrow so a later hand-built burst cannot read a staletrue.Two of the issue's premises were wrong
"A capped burst is never one legitimate frame." It can be — a frame at the head of the slab, followed by carrier that outlasts it, is whole and decodable. There is a test for exactly this.
"Skip or shorten the expensive decode pass on it." This would have been actively harmful: when the squelch sits below the band floor, every burst is a cap flush — that is #1254's regime, fixed earlier the same day — so skipping the decode would have made the daemon deaf on a hot band rather than merely quieter. The decode still runs; only the ladder evidence is suppressed.
And I retract my own argument for keeping it. I justified it with the QRM case (a frame arriving while an interferer holds the channel). The DCD is measured post-notch, so a notchable interferer never holds it open and REQ-QRM-01's rescue case never produces a cap flush. The right reasons are stuck-DCD and the boundary frame.
Scope of "keep the decode", so the next reader does not overclaim it: both scan phases bound onsets to
4 × acq_samples— ~0.5 s at BPSK250, ~4 s at BPSK31 — against a 37–300 s slab. Keeping the decode preserves boundary frames; it does not reach a frame embedded mid-carrier.Tests
Three, all through
accumulate_capture, because the flush reason exists nowhere else:recommended_level(the fix);Sabotage-verified: disabling the check fails (1) with "a cap-flushed slab that decoded nothing produced an ACK frame", while (2) and (3) stay green.
Two fixture notes worth keeping:
Cost, flagged rather than buried
This file runs ~75 s, and that lands on both the gate and the pre-push hook. It is inherent:
ota_decode_burstscans the candidate rungs and the cost is flat in slab length — I shrank the slab 8× (2.39 M → 298 k) and it did not move. Kept in the gate rather than#[ignore]d, because a held-out test for a small property is decay wearing preservation's clothes; the cost is recorded in the acceptance row.Incidentally the first committed evidence consistent with #1249's "~25–31 s, flat in buffer length", which was one uncommitted ad-hoc run.
GATE: PASS 323bca8890f0f7a0843fd7ca41a80ee8c0536b13 clean— 326 suites, 2500 passed, 0 failed.(The first gate run failed on two things of mine, neither in the code under test: an unused binding clippy caught only with
--all-targets, and the trace checker readingCAP-flushedin a comment as a capability ID.)Consumer
server.rs:903accumulate_captureon the daemon's rx ticker →ota_decode_burst(:956) → theladder_framebranch (:970). The thing consumed isres.ack, which is why this needs no daemon change.Prior art
#1249 sized the cap; #1254 removed the cold-start route to the same spent-RF outcome; #1123 is the discrimination this routes into rather than duplicating.
Twins
decode_burst(the non-OTA arm) has the same blindness but keys nothing, so only the CPU half applies. The monitor path sees every capped burst and also keys nothing.openpulse-ardopandopenpulse-kissuse thereceive*family and never reachaccumulate_capture— checked, not assumed.Implements: REQ-DCD-01
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6