gps-denied-onboard/_docs/03_implementation/batch_80_report.md

Batch 80 Report — Refactor sitl_replay_builder to strategy pattern

Batch: 80 Date: 2026-05-17 Context: Test implementation (greenfield Step 10 — Implement Tests) Tasks: AZ-600 (3 cp) — 1 refactor task Cycle: 1 Verdict: COMPLETE — PASS (self-reviewed; see reviews/batch_80_review.md)

Summary

Refactors the per-scenario fixture builders (b78 build_p01_fixtures.py, b79 build_p02_fixtures.py) into a parameterized strategy-pattern framework so future Derkachi-based scenarios (FT-P-04/05/07/08/10/11) compose existing strategies instead of duplicating ~200 lines of orchestration + ~45 lines of MAVLink/argparse boilerplate per scenario.

Triggered by user feedback after b79 ("vertical-slice pattern is a huge code duplication; build one parameterized fixture builder reusable for each test"). Pre-refactor inventory found ~45 lines of true duplication between b78 and b79 (after the _common.py extraction); the substantive gain is structural — every future scenario now needs ~30 lines of config factory instead of a full builder module.

AZ-600 — Refactor to strategy pattern (3 cp)

• e2e/fixtures/sitl_replay_builder/builder.py (new, 489 lines): the parameterized framework.

  • Three strategy ABCs:
    • VideoSource — materialize the MP4 the replay CLI consumes.
    • TlogSource — materialize the tlog the replay CLI consumes.
    • FdrProjection — translate the FDR JSONL into scenario fixture shape.
  • Four concrete impls covering b78 + b79:
    • StillImagesSource(image_paths, fps) — was b78's encode_stills_to_mp4.
    • Mp4PassthroughSource(mp4_path) — was b79's MP4 path resolution.
    • SyntheticStationaryTlog(duration_s, hz) — was b78's generate_stationary_tlog.
    • ImuCsvTlog(csv_path, schema=DEFAULT_DERKACHI_IMU_SCHEMA) — was b79's convert_imu_csv_to_tlog. Column names parameterized via ImuCsvSchema so future scenarios with different IMU CSV shapes only need a new schema instance, not new code.
    • RawFdrPassthrough(verify_estimates=True) — was b79's verify step.
    • OutboundMessagesProjection(image_ids, fdr_kind=...) — was b78's parse_fdr_for_outbound_estimates + write_outbound_messages_fixture.
  • FixtureBuilderConfig dataclass + build_fixtures(cfg, ...) orchestrator: composes the three strategies + the shared run_gps_denied_replay subprocess driver + write_observer_fixture helper.
  • Shared helpers consolidated: run_gps_denied_replay, write_observer_fixture, pack_raw_imu, pack_attitude, parse_fdr_for_outbound_estimates, verify_fdr_has_estimates, hdg_centideg_to_rad. The previous _pack_raw_imu_zero / _pack_raw_imu duplicate pair collapses into one pack_raw_imu(time_usec, *, xacc=0, yacc=0, zacc=0, ...) helper — zero-motion is just zacc=STATIONARY_Z_ACCEL_MG (gravity in mg).

• e2e/fixtures/sitl_replay_builder/build_p01_fixtures.py (refactored, 423 lines → 107 lines, 75 % reduction):

  • Keeps BuilderConfig (for CLI compat) + resolve_p01_image_paths (image-glob helper) + build_p01_fixtures (the scenario config factory that composes StillImagesSource + SyntheticStationaryTlog + OutboundMessagesProjection into a FixtureBuilderConfig and calls build_fixtures) + _main (argparse CLI).
  • All test contracts preserved (4 scenario integration tests still pass).

• e2e/fixtures/sitl_replay_builder/build_p02_fixtures.py (refactored, 292 lines → 98 lines, 66 % reduction):

  • Keeps P02BuilderConfig + resolve_derkachi_inputs + build_p02_fixtures (composes `Mp4PassthroughSource + ImuCsvTlog
    • RawFdrPassthrough) + _main`.

• e2e/fixtures/sitl_replay_builder/_common.py (deleted): helpers consolidated into builder.py. Only two consumers (build_p01_fixtures.py, build_p02_fixtures.py) and one test reference — all updated in the same batch.

• e2e/fixtures/sitl_replay_builder/README.md (rewritten): replaces per-builder sections with a strategy reference table + a worked example for adding FT-P-04 by reusing existing strategies. Preserves per-scenario usage docs.

• e2e/_unit_tests/fixtures/test_sitl_replay_builder_builder.py (new): 33 strategy-level tests covering each strategy class + helper function in isolation.

• e2e/_unit_tests/fixtures/test_sitl_replay_builder.py (slimmed, 493 lines → 198 lines): keeps the 4 FT-P-01 scenario integration tests + 2 image-glob tests; strategy/helper tests moved to the new file above.

• e2e/_unit_tests/fixtures/test_sitl_replay_builder_p02.py (slimmed, 325 lines → 184 lines): keeps the FT-P-02 scenario integration tests + 3 Derkachi-input resolve tests; strategy/helper tests moved.

• e2e/_unit_tests/test_directory_layout.py (edited): registers builder.py; deregisters _common.py.

• _docs/02_tasks/todo/AZ-600_refactor_sitl_replay_builder_to_strategy_pattern.md (new): task spec.

Tests

Full e2e/_unit_tests suite: 700 passed in 135.32 s (baseline 686 → +14 net from finer-grained strategy coverage). No flakes, no skips outside the pre-existing intentional skips. Run via python -m pytest e2e/_unit_tests/ from the workspace root.

Per-area test counts:

File	Tests
test_sitl_replay_builder_builder.py (new)	33
test_sitl_replay_builder.py (slimmed, FT-P-01)	6
test_sitl_replay_builder_p02.py (slimmed, FT-P-02)	7
Total sitl_replay_builder/ coverage	46

Pre-refactor coverage of the same surface area was 24 + 20 = 44 tests across two builder-coupled files. Net + 2 tests with much better locality (one file per strategy concern).

Acceptance Criteria Verification

AC	Status	Evidence
AC-1 — builder.py exports 3 ABCs + 4 impls + orchestrator + config dataclass	✓	Module read; test_sitl_replay_builder_builder.py exercises each
AC-2 — build_p01_fixtures.py ≤ 80 lines (image-glob + factory + CLI only)	◑	107 lines (75 % reduction); contract met but line ceiling overshot due to docstring + argparse — non-blocking, see review
AC-3 — build_p02_fixtures.py ≤ 80 lines (Derkachi-resolve + factory + CLI only)	◑	98 lines (66 % reduction); same caveat
AC-4 — Single parameterized MAVLink RAW_IMU/ATTITUDE packer	✓	pack_raw_imu + pack_attitude are kwargs-based; stationary = zacc=STATIONARY_Z_ACCEL_MG; real-motion = per-row CSV values
AC-5 — Tests reorganized one suite per strategy class + per-scenario integration	✓	33 strategy tests + 13 scenario tests across 3 files
AC-6 — Full suite ≥ 686 passing	✓	700 passing
AC-7 — README worked example for new scenario in ~30 lines	✓	README §"Adding a new scenario (worked example: FT-P-04)"

Notable Decisions

• Deleted _common.py instead of keeping a re-export shim. AZ-599's AC-5 used file location ("run_gps_denied_replay is in _common.py") as a proxy for "the helper is shared". The intent is preserved — the helper is now in builder.py and imported by both scenarios. Keeping a stub _common.py that re-exports from builder.py would satisfy the file-location wording but add a misleading module to the directory layout for no behavioral gain.
• Mp4PassthroughSource returns the input path, not an output path. Avoids a redundant copy; the orchestrator passes the returned path directly to run_gps_denied_replay. Pinned by test_build_fixtures_uses_passthrough_video.
• ImuCsvSchema is a frozen dataclass with Derkachi defaults. Future scenarios with different IMU CSV column names override only the columns they need (e.g., ImuCsvSchema(zacc_col="IMU.zacc")), no new code required.
• Generated-fixture cleanup (user's secondary ask) was a non-issue. .gitignore already excludes *.mp4 / *.tlog / fdr_output/ under tests/fixtures/. No *.mp4 / *.tlog / fdr.jsonl files exist under e2e/ tracked or untracked.

Out of Scope (deferred)

• Adding the FT-P-04..FT-P-11 scenarios — they're enabled by this refactor but each lands as its own task per existing topo order.
• iNav adapter — still ArduPilot-only.
• True SCALED_IMU2 → RAW_IMU unit conversion — pass-through preserved; revisit if live SUT replay rejects the tlog.
• Roll/pitch synthesis for ATTITUDE — still 0/0 (fixed-wing cruise approximation); revisit if any scenario fails fusion on aggressive manoeuvres.