gps-denied-onboard/_docs/03_implementation/reviews/batch_97_review.md

Code Review Report

Batch: 97 — AZ-687 (build_pre_constructed replay-mode guard for c6 descriptor index) Date: 2026-05-19 Verdict: PASS

Inputs

• Task spec: _docs/02_tasks/todo/AZ-687_pre_constructed_replay_mode_guard.md
• Changed files:
  • src/gps_denied_onboard/runtime_root/airborne_bootstrap.py (modified — helper added, three guarded conditionals around the existing seeds)
  • tests/unit/runtime_root/test_az687_pre_constructed_replay_mode.py (new — three tests covering AC-687-1 + inverse + AC-687-2)
• Project restrictions: _docs/00_problem/restrictions.md (re-read)
• Solution overview: _docs/01_solution/solution.md (re-read)

Findings

None.

Phase Walkthrough

Phase 1: Context Loading

Spec read: AZ-687 is a 2-point cross-cutting task surfacing the KeyError: 'c6_tile_cache' regression from the AZ-618 Jetson Tier-2 e2e run on 2026-05-19. The guard belongs at the BUILD-PRE-CONSTRUCTED layer, not at _c6_config (the existing docstring rejects silent fallback there). Scope is precisely: runtime_root/airborne_bootstrap.py::build_pre_constructed plus a new unit test under tests/unit/runtime_root/.

Phase 2: Spec Compliance

AC	Status	Test
AC-687-1 (replay-mode without c6_tile_cache block does not raise; skipped slots absent from dict)	Satisfied	test_ac_687_1_replay_mode_without_c6_block_does_not_raise_keyerror (asserts no KeyError; asserts the five guarded keys are absent; asserts builders not invoked) + test_ac_687_1_replay_mode_with_c6_block_present_still_builds_c6_seeds (inverse — guard is conditional on block absence, not just on mode)
AC-687-2 (live-mode regression: every key in set.union(*AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS.values()) still seeded; no key None)	Satisfied	test_ac_687_2_live_mode_seeds_every_required_key
AC-687-3 (Jetson Tier-2 e2e replay crosses replay.compose_root.ready AND replay.input.frame_emitted on AC-1/AC-2/AC-5/AC-6)	Pending — out-of-band hardware run required. Will be collected by the user via scripts/run-tests-jetson.sh tests/e2e/replay/test_derkachi_1min.py; evidence destination _docs/03_implementation/jetson_runs/2026-05-19_az687_tier2_run.txt.
AC-687-4 (full Tier-1 pytest suite green)	Satisfied — pytest tests/unit/ reports 2153 passed, 85 skipped, 1 deselected (pre-existing test_cold_start_under_500ms_p99 perf flake — operator-orchestrator CLI subprocess startup latency on busy dev macOS; not related to AZ-687).

Constraint compliance:

• _c6_config untouched — silent fallback explicitly avoided.
• No new BUILD_* env flags introduced.
• No component factory signatures changed.
• No per-component config schema changes.

Scope discipline: implementation is exactly two files; no scope creep, no opportunistic refactors.

Phase 3: Code Quality

• SRP: _replay_omits_component_block has one reason to change (the predicate's truth table). build_pre_constructed retains its existing responsibility; the conditionals are inline, not factored into a new function.
• Error handling: no silent suppression. KeyError from _c6_config is still surfaced in any environment where the block is expected (live mode or replay-with-block).
• Naming: _replay_omits_component_block reads as a complete sentence at the call site (if not _replay_omits_component_block(config, "c6_tile_cache"): ...). The single-line docstrings on each conditional block name the invariant the skip preserves.
• Complexity: helper is 7 LOC (excluding docstring). build_pre_constructed grew by 3 if branches; cyclomatic complexity well under 10.
• DRY: the predicate is shared across the three skip sites; the conditionals stay inline (a higher abstraction would obscure the per-seed semantics).
• Test quality: tests assert specific behavior (which builders fired, which keys are present/absent), not just "no exception". _stub_guarded_builders returns a mutable invocation map so the assertions are precise.
• Dead code: none.
• Comment discipline: the inline comments document the WHY (why the skip is safe — wrappers gated on config.components), not the WHAT.

Phase 4: Security Quick-Scan

No SQL, no subprocess, no eval/exec, no secrets, no new external inputs. Helper inspects config.mode (typed Literal["live", "replay"]) and config.components (typed Mapping[str, Any]); both are already validated by Config.__post_init__.

Phase 5: Performance Scan

Helper is O(1): one attribute read + one dict membership check. No hot-path impact (bootstrap runs once per process). No N+1 risk, no unbounded fetch, no async-blocking.

Phase 6: Cross-Task Consistency

Single-task batch; N/A.

Phase 7: Architecture Compliance

• Layer direction: runtime_root is the composition root (ADR-001/ADR-009) — it MAY import concrete strategies across components. The change touches only the composition root and adds no new cross-component imports.
• Public API respect: no new imports introduced.
• No new cyclic dependencies: _replay_omits_component_block reads Config only via getattr + dict membership; no module-level imports added.
• Duplicate symbols: _replay_omits_component_block is unique to airborne_bootstrap.py.
• Cross-cutting concerns not locally re-implemented: the mode + block-presence predicate is a composition-root concern (the composition root is the only layer that owns the config.mode branch); placing it in airborne_bootstrap.py is correct per _docs/02_document/module-layout.md shared/runtime_root entry.

Baseline delta: no _docs/02_document/architecture_compliance_baseline.md exists yet, so no delta is computed (baseline mode has not been run).

Verdict Justification

Zero Critical / High findings (no Spec-Gap, no Bug, no Security, no Architecture violation). Zero Medium / Low findings. Verdict: PASS — no action required before commit.