# Cumulative Code Review Report — Batches 67–69 (cycle 1, test phase)

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

## Batch coverage

| Batch | Tasks | Theme |
|-------|-------|-------|
| 67 | AZ-406 | Blackbox test infrastructure bootstrap (Tier-1 docker-compose, Tier-2 scaffold, runner image, conftest, helpers, mock suite sat service, public-boundary scaffolds) |
| 68 | AZ-407, AZ-444, AZ-445 | Static fixture builders (tile-cache, age-injector, cold-boot, mavlink-passkey, cve-jpeg), Tier-2 orchestrator + on-Jetson delegate, CSV reporter + NFR recorder + evidence bundler refinements |
| 69 | AZ-408, AZ-410, AZ-411 | Runtime synthetic injectors (outlier, blackout_spoof, multi_segment, fc_proxy), FT-P-02 cumulative drift scenario + anchor-pair helper, FT-P-03/14 schema + WGS84 scenario + helper |

Cycle 1 product implementation (batches 64–66 footprint) is **out of
scope** for this cumulative review — those batches' files are under
`src/gps_denied_onboard/**`, which the test phase does not touch. Drift
between product and test phases is checked by the
`Architecture Compliance` phase's "no SUT imports in e2e/" invariant.

## Phase 1 — Context Loading

- Read `_docs/02_document/module-layout.md` § `blackbox_tests`
  (cross-cutting test harness).
- Read `_docs/02_document/architecture.md` § layering (note: blackbox_tests
  sits OUTSIDE the production layering table — see the module-layout
  "Layering note").
- Reviewed batch reports `batch_67_report.md` and `batch_68_report.md`.
- Reviewed task specs for AZ-406, AZ-407, AZ-408, AZ-410, AZ-411,
  AZ-444, AZ-445.

## Phase 2 — Spec Compliance

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

| Task | Status |
|------|--------|
| AZ-406 (test infra) | All ACs covered by batch 67 unit tests; harness scaffolds intentionally raise `NotImplementedError` with explicit ownership pointers to AZ-407/408/416/417/441. |
| AZ-407 (static fixtures) | All ACs covered; AZ-407 AC-4 SITL load deferred to AZ-419 (documented in batch 68 report). |
| AZ-408 (runtime injectors) | All ACs covered; see `batch_69_review.md`. |
| AZ-410 (FT-P-02) | Logic ACs (1, 2, 3, 4) covered by `test_anchor_pair_detector.py`; runtime AC-1.3 NOT COVERED (hardware-loop). |
| AZ-411 (FT-P-03/14) | Logic ACs (1, 2, 3) covered by `test_estimate_schema.py`; runtime single-image push NOT COVERED. |
| AZ-444 (Tier-2 harness) | AC-1, AC-6 covered; AC-2/3/4/5 NOT COVERED (hardware-loop). |
| AZ-445 (CSV reporter + NFR) | All four ACs covered by 9 unit tests; integration covered by `test_nfr_recorder_fixture_emits_artifacts_in_run`. |

No new Spec-Gap findings introduced by cross-batch interaction.

## Phase 3 — Code Quality (Cross-Batch View)

- Test pyramid is consistent across batches:
  - **Unit** tests under `e2e/_unit_tests/` exercise helpers and fixture
    builders in isolation (248 tests at end of batch 69, up from 97 at
    end of batch 67).
  - **Scenario** tests under `e2e/tests/<category>/` are gated on
    upstream helper availability via the `_harness_helpers_implemented`
    probe (introduced by AZ-410, reused by AZ-411). Pattern is consistent.
- Naming and docstring style consistent across batches.
- Error handling: every fixture builder raises typed errors with explicit
  remediation hints (FileNotFoundError + "build the X first").

## Phase 4 — Security (Cumulative)

No new findings:
- No subprocess(shell=True) anywhere in `e2e/`.
- MAVLink passkey file pairs (docker secret + runner-side fixture) are
  guarded by `test_passkey_files_match` (still passes after batch 68's
  comment-header introduction and batch 69's untouched delivery).
- CVE-2025-53644 synthetic JPEG generator is pinned by SHA-256
  (`test_committed_fixture_matches_generator`).

## Phase 5 — Performance (Cumulative)

- Test runtime grew from 12.59 s (batch 67, 97 tests) → 165 s (batch 69,
  248 tests). The growth is dominated by PIL JPEG encoding inside the
  injector unit tests; this is the documented trade-off for genuine
  determinism tests on the generator code paths.
- No N+1 patterns, no unbounded fetches, no blocking I/O in test bodies.

## Phase 6 — Cross-Task Consistency

- **API stability**: AZ-406's helper stubs (`FrameSourceReplayer`,
  `ImuReplayer`, `fdr_reader.iter_records`, `sitl_observer.get_observer`,
  `mavproxy_tlog_reader.iter_messages`) all still raise `NotImplementedError`
  with the original ownership tags. AZ-410 and AZ-411 scenario files
  correctly probe these via the `_harness_helpers_implemented` gate.
- **Scaffold-to-real evolution**: AZ-406's scaffold dataclasses for the
  injectors (`OutlierInjectionPlan` / `BlackoutSpoofPlan` /
  `MultiSegmentPlan`) were replaced in batch 69 by the AZ-408 spec's
  real shapes. The contract test (`test_injectors_contract.py`) was
  updated in lock-step — no orphaned old fields remain. This is the
  expected scaffold-to-real evolution pattern.
- **pytest plugin registration**: batch 67 introduced
  `csv_reporter` + `evidence_bundler`; batch 68 added `nfr_recorder`;
  batch 69 added `runner.helpers.injector_fixtures`. All four are
  registered in `runner.conftest.pytest_plugins` in the same place
  (consistent). No duplicate plugin registration.
- **No duplicate symbols across batches**: `derive_rng` (batch 69) is
  unique; `_common.haversine_m` is a deliberate dependency-isolation
  duplicate of `geo.distance_m` (batch 67 helper) — documented in the
  source docstring.

## Phase 7 — Architecture Compliance (Cumulative)

1. **Layer direction**: blackbox_tests sits outside production layering;
   only constraint is "no `gps_denied_onboard.*` imports". Enforced by
   `e2e/_unit_tests/test_no_sut_imports.py` (passes for all 21 changed
   files across batches 67–69).
2. **Public API respect**: cross-component imports inside `e2e/` are
   limited to `runner.helpers.*` (public) and `fixtures.injectors.*`
   (public package). The leading-underscore `_common.py` is the only
   private module and is consumed only inside the `fixtures.injectors`
   subpackage.
3. **No new cyclic dependencies**: full import graph remains a DAG:
   - `injectors._common` → (none — leaf)
   - `injectors.outlier|blackout_spoof|multi_segment` → `_common`
   - `injectors.fc_proxy` → (none — leaf)
   - `runner.helpers.injector_fixtures` → `injectors.*`
   - `runner.helpers.anchor_pair_detector` → `runner.helpers.geo`
   - `runner.helpers.estimate_schema` → (none — leaf)
   - `tests.positive.test_ft_p_02_*` → `runner.helpers.anchor_pair_detector` + runner stubs
   - `tests.positive.test_ft_p_03_14_*` → `runner.helpers.estimate_schema` + runner stubs
4. **Duplicate symbols across components**: none — every public name in
   `runner.helpers/*` and `fixtures.injectors/*` is unique.
5. **Cross-cutting concerns**: pytest plugin registration centralized
   in `runner.conftest`; no per-test local re-implementations.

Baseline delta: `_docs/02_document/architecture_compliance_baseline.md`
absent — section omitted (same as `batch_69_review.md`).

## Aggregate Verdict: PASS

No Critical, High, Medium, or Low findings across the cumulative scope
(batches 67–69). The test phase is internally consistent, the scaffold
→ real evolution between AZ-406 and AZ-408 was executed cleanly, and
public-boundary discipline is intact.

## Next Cumulative Review

K=3 default; next trigger after batches 70, 71, 72 complete.

## Test-Suite Snapshot (end of batch 69)

```
$ source .venv/bin/activate && python -m pytest e2e/_unit_tests/ -q
... 248 passed in 141.08s ...
```
