# Cumulative Code Review Report — Batches 70–72 (cycle 1, test phase)

**Date**: 2026-05-16
**Mode**: cumulative
**Scope**: union of files changed in batches 70, 71, 72 of cycle 1
(the test-implementation phase batches that followed the
`batches_67-69` cumulative review).
**Verdict**: PASS

## Batch coverage

| Batch | Tasks | Theme |
|-------|-------|-------|
| 70 | AZ-409, AZ-412, AZ-413 | Still-image accuracy (FT-P-01), Derkachi frame-to-frame registration (FT-P-04), satellite anchor + MRE budgets (FT-P-05 + FT-P-06) |
| 71 | AZ-414, AZ-415, AZ-418 | Sharp-turn recovery + failure twin (FT-P-07 + FT-N-02), multi-segment relocalisation (FT-P-08), GTSAM smoothing-loop look-back (FT-P-10) |
| 72 | AZ-416, AZ-417, AZ-419 | ArduPilot GPS_INPUT contract + signing handshake (FT-P-09-AP), iNav MSP2_SENSOR_GPS contract (FT-P-09-iNav), cold-start initialization (FT-P-11 — 3 origin_source variants + no-origin abort) |

Cycle 1 product implementation under `src/gps_denied_onboard/**` is
out of scope; drift between product and test phases is checked by
`test_no_sut_imports.py` (passing).

## Phase 1 — Context Loading

* Read `_docs/02_document/module-layout.md` § `blackbox_tests`.
* Read `_docs/02_document/architecture.md` § layering.
* Reviewed batch reports `batch_70_report.md`, `batch_71_report.md`,
  `batch_72_report.md` (in-progress draft).
* Reviewed task specs AZ-409, AZ-410 (prior), AZ-411 (prior), AZ-412,
  AZ-413, AZ-414, AZ-415, AZ-416, AZ-417, AZ-418, AZ-419.
* Cross-referenced the prior `cumulative_review_batches_67-69`
  conclusions to verify the K=3 cumulative cadence is honoured.

## Phase 2 — Spec Compliance

Per-task AC coverage at the end of batch 72:

| Task | Status |
|------|--------|
| AZ-409 (FT-P-01) | Helper + scenario + 20 unit tests; AC-1..AC-7 covered |
| AZ-412 (FT-P-04) | Helper + scenario + 26 unit tests; AC-1..AC-5 covered |
| AZ-413 (FT-P-05 + FT-P-06) | Helper + 2 scenarios + 22 unit tests; AC-1..AC-4 covered (FT-P-06 piggybacks on FT-P-04 + FT-P-05 evidence CSVs) |
| AZ-414 (FT-P-07 + FT-N-02) | Helper + 2 scenarios + 30 unit tests; AC-1..AC-7 (FT-P-07) AND AC-1..AC-7 (FT-N-02) covered via the shared `sharp_turn_detector` helper |
| AZ-415 (FT-P-08) | Helper + scenario + 16 unit tests; AC-1..AC-4 covered |
| AZ-416 (FT-P-09-AP) | Helper + scenario + 22 unit tests (ap_contract_evaluator) + 6 unit tests (mavproxy_tlog_reader); AC-1..AC-5 + D-C8-9 covered |
| AZ-417 (FT-P-09-iNav) | Helper + scenario + 14 unit tests; AC-1..AC-4 covered |
| AZ-418 (FT-P-10) | Helper + scenario + 15 unit tests; AC-1..AC-3 covered |
| AZ-419 (FT-P-11) | Helper + 2 scenarios + 19 unit tests; AC-1..AC-5 covered (3 origin_source parametrize variants + 1 no-origin abort scenario) |

All scenarios are skip-gated on the AZ-441 / AZ-407 leftovers
(`frame_source_replay`, `imu_replay`, `fdr_reader`, `sitl_observer`
ext methods); pure-logic acceptance is fully covered in the
`e2e/_unit_tests/helpers/` test files.

## Phase 3 — Code Quality

* **Single responsibility**: each helper owns ONE analytic concern:
  - `accuracy_evaluator` — still-image Vincenty + pass-count rules
  - `registration_classifier` — IMU-derived attitude + normal-segment
    classification + success ratio
  - `mre_evaluator` — per-image cross-domain + 95th-percentile MRE
  - `anchor_pair_detector` — drift binning + monotonicity
  - `estimate_schema` — schema validation + WGS84 range + int32
    decoding
  - `sharp_turn_detector` — gyro_z run detection + during-turn label/cov
    + recovery lag/drift/heading
  - `multi_segment_evaluator` — multi-window relocalisation
  - `smoothing_evaluator` — raw + smoothed pose pair + improvement rate
  - `mavproxy_tlog_reader` — pymavlink tlog frame iteration
  - `ap_contract_evaluator` — signing handshake + GPS_INPUT rate +
    EK3 source-set + GPS_RAW_INT health
  - `msp_frame_observer` — MSP rate + iNav GPS state evaluation
  - `cold_start_evaluator` — Manifest build/read + cold-boot snapshot
    parse + first-estimate / no-origin / bounded-delta evaluation
* **No suppressed errors**: the only narrow `try`/`except` is in
  `mavproxy_tlog_reader.iter_messages` for pymavlink's `BAD_DATA` +
  per-message `to_dict` exceptions — documented in the docstring as
  the standard pymavlink iteration idiom.
* **AAA discipline**: all 460 unit tests use `# Arrange / # Act /
  # Assert`.
* **No narration comments** in any new module; docstrings carry
  intent + AC mapping + Mode B Facts where relevant (Fact #107 in
  `smoothing_evaluator`, Fact #109 noted in scenario docstrings of
  AZ-416 + AZ-417, ADR-010 Principle #11 in `cold_start_evaluator`).

## Phase 4 — Security

* **`test_no_sut_imports.py` passes** — no e2e helper or test file
  imports `src/gps_denied_onboard`.
* **Signing channel observability**: AZ-416 helper observes signed
  frames + BAD_SIGNATURE STATUSTEXT events without ever validating
  the signature itself (that's pymavlink + AP-side wiring). The
  scenario "Forbidden" list (no bypass to unsigned channel) is
  honoured — `passes` returns False if any `BAD_SIGNATURE` STATUSTEXT
  appears in the handshake window OR no signed frame arrives.
* **Test passkey hygiene**: `test_passkey_files_match` (pre-existing)
  still passes; AZ-416 scenario consumes the docker-secret fixture
  only.
* **No credentials in source**: confirmed by grep across all batch
  72 added modules.

## Phase 5 — Performance

* Across all 12 helpers added in batches 70-72, every analyser is O(N)
  over its input.
* `mavproxy_tlog_reader` materialises to a list ONCE per scenario via
  `ap_contract_evaluator.collect_messages_to_list` so multiple
  analysers can share the result — the alternative (re-iterating the
  generator) would re-open the pymavlink connection per analyser.
* No nested CSV reads or repeated geodesic recomputations in any
  helper across the three batches.

## Phase 6 — Cross-Task Consistency

Verified across all 9 tasks in the 70-72 window:

* **Skip gate pattern**: every scenario uses an
  `_*_harness_implemented` fixture that probes one or more
  `NotImplementedError`-raising helpers and skips with a single,
  spec-referenced message naming the upstream owner (AZ-441 / AZ-407)
  and the pure-logic unit-test file that DOES cover the AC.
* **Constants discipline**: every scenario assertion message
  references the helper's exported constant by name (e.g.
  `ace.HANDSHAKE_BUDGET_S`, `cse.BOUNDED_DELTA_TRIGGER_M`,
  `std.MAX_RECOVERY_FRAMES_SAFETY_MS`), not magic numbers.
* **Evidence emission**: every scenario emits per-scenario NFR metrics
  via `nfr_recorder.record_metric(name, value, ac_id=…)`. Per-test CSV
  artifacts use `write_csv_evidence(out, …)` returning the path —
  same idiom in `accuracy_evaluator`, `mre_evaluator`,
  `multi_segment_evaluator`, `smoothing_evaluator`,
  `sharp_turn_detector`.
* **Trace markers**: every scenario uses `@pytest.mark.traces_to(...)`
  with comma-separated AC IDs, matching the
  `monorepo-document`-owned traceability format used by batches 67-69.
* **Helper return shape**: every analyser returns a frozen
  `@dataclass` with a `passes` (or `passes_distance`, `passes_rate`,
  etc.) property — so the scenario assertion is one boolean check
  with a structured-data message.
* **No drift in shared types**: `TlogMessage` (AZ-406 surface, AZ-416
  body) used identically across `mavproxy_tlog_reader.count_by_type`
  and `ap_contract_evaluator.*` analysers.

## Phase 7 — Architecture Compliance

* **Module-layout invariant**: every new helper is under
  `e2e/runner/helpers/`; every new scenario under
  `e2e/tests/{positive,negative}/`; every new unit test under
  `e2e/_unit_tests/helpers/`. `test_directory_layout.py` parametrize
  list updated to enforce the invariant — 75 path entries pass.
* **Public-boundary**: every scenario uses only the FDR `record_type`
  + `payload` dict schema, outbound estimate stream, and SITL
  observer surface; no SUT internals consumed.
* **Backwards compat with AZ-406 surface**: `mavproxy_tlog_reader`
  filled in its body without changing the `TlogMessage` dataclass
  shape or the `iter_messages` / `count_by_type` signatures, so
  downstream consumers (FT-P-03/14 schema scenario, others) keep
  working.

## Phase 8 — Test Suite Health Trend

| Batch end | Total tests | Delta |
|-----------|-------------|-------|
| 69 | 257 | (baseline) |
| 70 | 325 | +68 |
| 71 | 393 | +68 |
| 72 | 460 | +67 |

Net: +203 unit tests across batches 70-72 / 12 new helper modules + 9
new scenario files + 1 modified scenario file (FT-P-09-AP wired up
through the previously stub-only `mavproxy_tlog_reader`).

Pre-existing macOS-only `/e2e-results` plugin issue in scenario
invocation outside Docker is unaffected by all batch 70-72 changes;
unit suite untouched by it.

## Cross-Batch Consistency Verdict

PASS — no behavioural drift between batches; helper module shape +
scenario skeleton + skip-gate pattern + constants discipline + NFR
metrics format + traces_to marker format all identical across the 9
tasks.

## Architecture Compliance Verdict

PASS — public-boundary blackbox stance preserved across all 12 new
helpers; pymavlink boundary correctly placed at the tlog reader;
ADR-010 Principle #11 amended explicitly encoded in
`cold_start_evaluator`; Mode B Fact #107 preserved in
`smoothing_evaluator` docstring.

## Final Verdict

**PASS** — Batches 70-72 (AZ-409, AZ-412, AZ-413, AZ-414, AZ-415,
AZ-416, AZ-417, AZ-418, AZ-419 — 9 tasks / 27 cp) ready for the next
K=3 cumulative review at end of batch 75.
