ARQ retry-limit sweep + post-ARQ FEC dimensioning (#362 steps 1+4)

[josephnef]

Steps 1 and 4 of #362 (issue stays open for the rate-fallback and deadline research, steps 2–3).

What's added

• tests/arq_retry_sweep.sh — runs the ARQ end-to-end delivery bench: three-ledger harness; count spsc-fat pool-exhaustion drops #359 arq_e2e bench once per DEVOURER_TX_RETRY_LIMIT, tabulates delivered% / drops / mean retries (the airtime proxy), and hands each run to the residual analyzer.
• tests/arq_fec_dimension.py — post-ARQ residual gap-length distribution per run: the quantity a wfb-style (K,N) block FEC must cover. Header states the caveats: single-gap-per-window mapping, near-field scope, and unreported-frames-count-as-lost (conservative — the safe direction for dimensioning).
• docs/scheduled-mac.md — the measured curve + recommendation next to the existing retry-knob paragraph.

Measured (8812CU retrying TX → 8812EU duplex ground airing PixelPilot-shaped feedback bursts, ~1 k fps collision regime, near-field)

limit	delivered%	drops	mean retries	residual	gap P99.9	FEC to cover
3	99.72	352	0.035	0.26%	62¹	—
8	99.97	34	0.039	0.03%	3	K=8 → N=11
16	100.00	0	0.054	0.00%	—	residual gap-free
32	100.00	0	0.063	—	—	+17% retries over 16 for nothing

¹ single 62-frame gap in one run — one occurrence, P99 is still 3; not treated as a distribution feature.

Queue-time p99 is flat (~10–11 fw units) across all limits; only the rare worst case doubles (18 → 35) and then stops growing — deep retries are rare enough that latency does not enter the trade.

Recommendation carried in the docs: DEVOURER_TX_RETRY_LIMIT=16 on an ARQ link (zero bench loss at +5.4% retry airtime), or 8 plus a light FEC floor (K8/N11-class) where airtime is precious. Retries are backoff-spaced, so the field default of 3 can burn entirely inside one 2–3 ms ground-station feedback burst — that is where its 0.26% residual comes from.

Also re-derived from the recorded #359/#366 ledgers (posted to #362): the stall-regime contrast — plain-async stalls produce ~110-frame contiguous gaps no sane FEC rate covers (rate 0.07), while the #366 backpressure policy bounds the same stalls to ≤13-frame gaps. FEC should be sized to the collision residual; stall regimes are a ring-policy problem.

Validation

Four full on-air bench runs (one per limit, ~129 k frames each, frame-exact three-ledger accounting from #359); analyzer parity-checked against the exploratory version on the recorded baseline. ctest 48/48 (no library changes in this PR).

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add ARQ retry-limit sweep harness and post-ARQ FEC residual gap analyzer

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Add a sweep script to benchmark delivery vs airtime across TX retry limits
• Add a residual gap-length analyzer to size post-ARQ block FEC (K,N) coverage
• Document measured curve and recommend retry-limit defaults for ARQ links

Diagram

graph TD
  S["arq_retry_sweep.sh"] --> B["arq_e2e_delivery.sh"] --> R[("Run ledgers")]
  R --> A["arq_fec_dimension.py"] --> O[["Sweep summary"]] --> D[["scheduled-mac.md"]]

  subgraph Legend
    direction LR
    _script["Script"] ~~~ _data[("Recorded data")] ~~~ _doc[["Documentation"]]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fold residual-gap analysis into arq_e2e_analyze.py

• ➕ Avoids duplicating tag-unwrapping / tail-guard conventions across scripts
• ➕ Single tool can emit both delivery verdicts and residual/FEC sizing in one run
• ➖ Makes the main analyzer heavier and less focused on its current “join and verdict” role
• ➖ May complicate output formatting/CLI stability for existing workflows

2. Model multi-gap-per-block FEC coverage (windowed simulation)

• ➕ More accurate than the current “single-gap-per-window” approximation for some loss patterns
• ➕ Can directly output recommended (K,N) pairs under explicit assumptions
• ➖ More complex and easier to misinterpret without carefully matching the real interleaving/windowing strategy
• ➖ Likely overkill for the stated goal of conservative, quick dimensioning from bench data

Recommendation: The PR’s approach is reasonable for a lightweight, conservative engineering toolchain: a retry-limit sweep that reuses the existing bench and a separate analyzer focused on residual gap run-lengths. If this workflow becomes a recurring gate, consider consolidating shared conventions (tag unwrap, tail guard, report semantics) into a small shared module or into arq_e2e_analyze.py to reduce drift.

Files changed (3) +185 / -0

Tests (2) +173 / -0

arq_fec_dimension.pyNew post-ARQ residual gap-length analyzer for FEC sizing +108/-0

New post-ARQ residual gap-length analyzer for FEC sizing

• Introduces a Python tool that reconstructs an undelivered-frame set from DUT rx.seq and drone tx.report events (treating missing report verdicts as undelivered, conservatively). Computes run-length (gap) distribution percentiles and maps P99.9 single-gap length to an FEC requirement (N-K) for configurable K values.

tests/arq_fec_dimension.py

arq_retry_sweep.shNew harness to sweep DEVOURER_TX_RETRY_LIMIT across arq_e2e runs +65/-0

New harness to sweep DEVOURER_TX_RETRY_LIMIT across arq_e2e runs

• Adds a root-run shell script that executes the existing arq_e2e_delivery bench once per retry limit, captures logs, and prints a summary table (delivered%, drops, mean retries/frame). Invokes the new FEC dimensioning analyzer across the collected run directories and appends results to the sweep summary output.

tests/arq_retry_sweep.sh

Documentation (1) +12 / -0

scheduled-mac.mdDocument measured retry-limit curve and recommended defaults +12/-0

Document measured retry-limit curve and recommended defaults

• Adds a new paragraph explaining why low retry limits can fail inside a short feedback burst and summarizes measured delivery/residual/airtime results for limits 3/8/16/32. Records the recommended settings (16 for ARQ links, or 8 with a light FEC floor) and points to the new residual gap analyzer script.

docs/scheduled-mac.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (1) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. arq_retry_sweep.sh omits per-arm curve ✓ Resolved 📎 Requirement gap ⚙ Maintainability

Description

The retry-limit sweep summary is computed as a single aggregate over drone.jsonl and does not
break out results for both burst arms (6M:10 and 6M:30) as required. This prevents publishing a
delivered% vs airtime-cost curve per burst arm.

Code

tests/arq_retry_sweep.sh[R39-42]

+  D=${RUNDIR[$L]}
+  python3 - "$D/drone.jsonl" "$L" <<'PYEOF'
+import json, sys
+n = ok = drops = 0

Evidence

The compliance rule requires sweep results to be published for both burst arms (6M:10 and
6M:30). tests/arq_e2e_delivery.sh defines those arms via its default PHASES, but
tests/arq_retry_sweep.sh tabulates a single delivered%/mean-retries line per retry limit by
scanning only tx.report events in drone.jsonl, and the added docs paragraph also reports a
single combined curve without per-arm breakdown.

Publish retry-limit sweep results and recommended default for ARQ links
tests/arq_e2e_delivery.sh[52-56]
tests/arq_retry_sweep.sh[36-59]
docs/scheduled-mac.md[181-191]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
PR Compliance ID 1 requires publishing sweep results for RETRY_LIMIT ∈ {3, 8, 16, 32} across both burst arms (`6M:10` and `6M:30`). The new `tests/arq_retry_sweep.sh` currently computes `delivered%` and `mean_retries` as a single aggregate over all `tx.report` events, so the output cannot be attributed to each burst arm.
## Issue Context
`tests/arq_e2e_delivery.sh` runs multiple phases (default includes `6M:10` and `6M:30`) and `tests/arq_e2e_analyze.py` already outputs per-phase `ok%`, but the sweep script ignores that per-phase output.
## Fix Focus Areas
- tests/arq_retry_sweep.sh[37-65]
- tests/arq_e2e_delivery.sh[52-56]
- docs/scheduled-mac.md[181-191]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. arq_e2e_analyze.py lacks histogram 📎 Requirement gap ⚙ Maintainability

Description

PR Compliance ID 4 requires extending tests/arq_e2e_analyze.py to emit a post-ARQ residual
gap-length histogram, but this PR adds a separate analyzer (tests/arq_fec_dimension.py) instead.
As a result, the required histogram is not produced by the mandated analyzer entry point.

Code

tests/arq_fec_dimension.py[R4-9]

+For each recorded arq_e2e run directory: reconstruct the undelivered set
+(report ok=0, plus unreported-and-undelivered — conservative, slightly
+overcounts), then measure the RUN-LENGTH distribution of consecutive
+undelivered frame indices. That is the quantity wfb-style block FEC cares
+about: a (K,N) block recovers up to N-K losses per block window, so the
+residual gap-length percentiles map directly onto the N-K needed.

Evidence

The rule requires histogram emission by tests/arq_e2e_analyze.py. The PR instead adds
tests/arq_fec_dimension.py, which computes a histogram (Counter(gaps)) and prints P50/P99/P99.9,
while tests/arq_e2e_analyze.py continues to emit only its existing per-phase table and verdict
event, not a residual gap-length histogram.

Publish post-ARQ residual gap-length histogram and derive FEC K/N pairing to cover P99.9
tests/arq_fec_dimension.py[87-103]
tests/arq_e2e_analyze.py[277-337]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
PR Compliance ID 4 explicitly requires `tests/arq_e2e_analyze.py` to be extended to emit the gap-length histogram of undelivered frames after ARQ. The PR introduces `tests/arq_fec_dimension.py` to compute this, but does not add histogram emission to `tests/arq_e2e_analyze.py`.
## Issue Context
`tests/arq_fec_dimension.py` already computes the run-length distribution and percentiles and maps P99.9 to an `N-K` requirement. To satisfy the compliance item as written, that histogram (and/or the derived P99.9) should be emitted from `tests/arq_e2e_analyze.py` (either directly or by invoking/shared-importing this logic).
## Fix Focus Areas
- tests/arq_fec_dimension.py[27-105]
- tests/arq_e2e_analyze.py[277-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. No tagged-report validation ✓ Resolved 🐞 Bug ☼ Reliability

Description

tests/arq_fec_dimension.py skips tx.report events without a halmac tag and never validates that any
tagged reports were processed, so the residual calculation can silently degrade into a
biased/invalid reconstruction. This is inconsistent with arq_e2e_analyze.py, which treats “no tagged
tx.report” as a fatal condition because the ledgers can’t be joined.

Code

tests/arq_fec_dimension.py[R47-50]

+            t = ev.get("tag")
+            if t is None:
+                continue
+            if prev is not None:

Evidence

arq_fec_dimension depends on tag to unwrap reports into a frame index (r) but only continues
on missing tags and never checks whether any usable reports exist; arq_e2e_analyze documents that
tagless reports make joining impossible and returns a failure in that case.

tests/arq_fec_dimension.py[36-58]
tests/arq_e2e_analyze.py[56-67]
tests/arq_e2e_analyze.py[148-150]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/arq_fec_dimension.py` ignores `tx.report` events that lack `tag` and then proceeds even if **zero** tagged reports were found. In that situation, the `rep` map is empty and `miss` becomes effectively “everything not in dut (except where `rep.get(k)` is True, which never happens)”, producing misleading residual/gap outputs instead of an explicit invalid-input error.
### Issue Context
The existing analyzer (`tests/arq_e2e_analyze.py`) explicitly treats "NO tx.report events with tags" as a failure because without tags the join/index reconstruction is impossible.
### Fix Focus Areas
- tests/arq_fec_dimension.py[36-58]
- tests/arq_e2e_analyze.py[56-67]
- tests/arq_e2e_analyze.py[148-150]
### Suggested change
- Track `n_tagged_reports` (or `prev is not None`) while reading `drone.jsonl`.
- If `n_tagged_reports == 0`, print a clear error (e.g. `VERDICT: NO tx.report events with tags`) and exit non-zero (or raise).
- Optionally also validate `dut` has at least some `rx.seq` rows and fail fast if not.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (2)

4. Incorrect percentile index ✓ Resolved 🐞 Bug ≡ Correctness

Description

tests/arq_fec_dimension.py computes percentiles with int(p * n) as a 0-based index, which is
off-by-one and biases quantiles upward (e.g., P50 of a 2-sample list returns the maximum). This can
misstate the printed P50/P99/P99.9 and overstate the derived FEC N-K requirement.

Code

tests/arq_fec_dimension.py[R75-78]

+def pct(sorted_g, p):
+    if not sorted_g:
+        return 0
+    return sorted_g[min(len(sorted_g) - 1, int(p * len(sorted_g)))]

Evidence

The percentile helper is used directly for P50/P99/P99.9 printing and for need = pct(g, .999); the
current index formula is visibly int(p*n) which selects too-high order statistics for common
percentile conventions.

tests/arq_fec_dimension.py[75-78]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pct()` uses `int(p * len(sorted_g))` to select an element, which is not a standard discrete percentile definition and is off-by-one for many `p` values. This biases results upward (notably for small samples) and affects both reported percentiles and the `need = pct(g, .999)` sizing.
### Issue Context
This script is used for FEC dimensioning; the quantile convention should be explicit and stable. A common discrete approach is nearest-rank: `ceil(p*n)` (1-based), converted to 0-based index `ceil(p*n)-1`.
### Fix Focus Areas
- tests/arq_fec_dimension.py[75-78]
### Suggested change
- Implement nearest-rank indexing:
- `import math`
- clamp `p` into `[0, 1]`
- `n = len(sorted_g)`
- `idx = max(0, min(n - 1, math.ceil(p * n) - 1))`
- `return sorted_g[idx]`
- (Alternative) Use `statistics.quantiles` with a documented method, but keep it dependency-free.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Rundir selection race ✓ Resolved 🐞 Bug ☼ Reliability

Description

tests/arq_retry_sweep.sh infers the run directory by selecting the newest /tmp/arq-e2e/*/ after
each run, which can pick the wrong directory if other runs exist or another process updates mtimes.
This silently corrupts the per-limit summary and FEC residual analysis.

Code

tests/arq_retry_sweep.sh[R29-32]

+  RETRY_LIMIT="$L" bash "$ROOT/tests/arq_e2e_delivery.sh" \
+      >"$OUT/limit_$L.log" 2>&1 || {
+    echo "run failed (see $OUT/limit_$L.log)"; exit 1; }
+  RUNDIR[$L]=$(ls -td /tmp/arq-e2e/*/ | head -1)

Evidence

The sweep uses a newest-directory heuristic to populate RUNDIR, while the underlying bench script
already exposes an OUT variable that can be set to avoid guessing.

tests/arq_retry_sweep.sh[26-34]
tests/arq_e2e_delivery.sh[71-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The sweep script guesses the just-finished run’s output directory via `ls -td /tmp/arq-e2e/*/ | head -1`. This is race-prone and can mismatch retry limits to the wrong run.
### Issue Context
`tests/arq_e2e_delivery.sh` already supports caller-controlled output via the `OUT` env var, so the sweep can create a deterministic per-limit output path.
### Fix Focus Areas
- tests/arq_retry_sweep.sh[26-34]
- tests/arq_e2e_delivery.sh[71-72]
### Suggested change
- Inside the loop, set something like:
- `RUN_OUT="$OUT/limit_$L"`
- `OUT="$RUN_OUT" RETRY_LIMIT="$L" bash "$ROOT/tests/arq_e2e_delivery.sh" ...`
- `RUNDIR[$L]="$RUN_OUT"`
- Remove the `ls -td ...` heuristic entirely.
- Optionally validate that `$RUN_OUT/drone.jsonl` exists before recording it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}