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

Code Review Report — Batch 56

Batch: 56 Tasks: AZ-335 (C1 Warm-Start Hint Persistence + F8 Reboot Recovery) Date: 2026-05-14 Verdict: PASS_WITH_WARNINGS Mode: Full (per-batch)

Phase Summary

Phase	Result
1. Context Loading	OK
2. Spec Compliance	OK (10/10 ACs implemented + tested; 3 NFRs covered)
3. Code Quality	OK
4. Security Quick-Scan	OK
5. Performance Scan	OK
6. Cross-Task Consistency	OK
7. Architecture Compliance	1 Low note (F4)

Findings

#	Severity	Category	File:Line	Title
1	Low	Style	runtime_root/warm_start_wiring.py:82	AC-3 spec text says log kind c1.warm_start.cold_start; impl uses c1.warm_start.cold_start_no_hint
2	Medium	Maintainability	runtime_root/warm_start_wiring.py:267-272	Per-frame save uses VioOutput.relative_pose_T directly as WarmStartPose.body_T_world without explicit baseline composition
3	Low	Spec-Gap	tests/unit/c1_vio/test_az335_warm_start.py:TestStoreNfrPerf	NFR perf tests are dev-hardware smoke; full Tier-2 NVMe perf is deferred to C1-PT-01
4	Low	Architecture	runtime_root/warm_start_wiring.py:54	runtime_root imports c1-internal _facade_spine (bias_norm, now_iso)

Finding Details

F1: AC-3 log kind shorthand vs source-suffixed kind (Low / Style)

• Location: src/gps_denied_onboard/runtime_root/warm_start_wiring.py:82, mirrored in _emit_prime_log k-builder
• Description: AZ-335 spec AC-3 requires INFO log kind="c1.warm_start.cold_start". The spec Outcome § also names the cold-start source tag as cold_start_no_hint (line 44 of AZ-335_c1_warm_start_recovery.md). The implementation builds the log kind as f"c1.warm_start.{source}" to keep the family namespace consistent (so all three sources — f2_takeoff_fc, f8_reboot_disk, cold_start_no_hint — produce log kinds that match their FDR source field). The result is c1.warm_start.cold_start_no_hint, which is more discriminating than the AC-3 shorthand but doesn't match it character-for-character.
• Suggestion: Either (a) tighten AC-3's spec text in the next revision of AZ-335_c1_warm_start_recovery.md to say c1.warm_start.cold_start_no_hint, or (b) emit c1.warm_start.cold_start and keep the FDR record's source field as cold_start_no_hint. Option (a) preferred — the source-suffixed kind is genuinely more useful for log filtering.
• Task: AZ-335

F2: Per-frame save uses strategy-frame pose as body_T_world (Medium / Maintainability)

• Location: src/gps_denied_onboard/runtime_root/warm_start_wiring.py:267-272 (_save_hint_from_output)
• Description: AZ-335 spec line 41 says "every emitted VioOutput from process_frame is converted into a WarmStartPose (relative-pose chained against the prior baseline by the runtime root, plus the latest imu_bias from the same VioOutput)". Per _types.nav.VioOutput docstring, relative_pose_T is "the strategy's current pose ... expressed in the strategy's own internal frame". The implementation passes out.relative_pose_T straight into WarmStartPose.body_T_world without composing against a takeoff baseline. This is semantically defensible because the strategy's "internal frame" persists across F8 reload: at F2 takeoff the FC EKF seeds the strategy's frame to world, and on F8 reload the saved hint reinstalls that same frame's most-recent pose so the strategy "continues from where it left off". But the spec phrasing implies an explicit baseline-compose step that the wiring layer would own. No AC tests this composition, so the gap is informational, not contractual.
• Suggestion: Either (a) document the design choice inline in _save_hint_from_output (a 3-line comment explaining why the strategy-frame pose IS the right hint without explicit composition), or (b) revise the spec line 41 prose in cycle 2 to match the as-built behaviour. Recommend (a) — adds zero runtime cost, prevents future maintainers from "fixing" the gap.
• Task: AZ-335

F3: NFR perf tests are dev-hardware smoke (Low / Spec-Gap)

• Location: tests/unit/c1_vio/test_az335_warm_start.py::TestStoreNfrPerf
• Description: Spec NFR-perf-save (p99 ≤ 50 ms) and NFR-perf-load (p99 ≤ 20 ms) are explicitly Tier-2-NVMe budgets. The unit test uses 200 iterations on whatever filesystem tmp_path resolves to (developer hardware) and asserts the p99 is below the same threshold. This is sufficient to catch egregious regressions but is NOT the production NFR check.
• Suggestion: Tier-2 measurement is the responsibility of C1-PT-01 (Tier-2 perf gate; deferred to E-BBT). Keep the dev smoke as-is; do not expand here.
• Task: AZ-335

F4: runtime_root imports c1-internal _facade_spine (Low / Architecture)

• Location: src/gps_denied_onboard/runtime_root/warm_start_wiring.py:54
• Description: runtime_root/warm_start_wiring.py imports bias_norm and now_iso from gps_denied_onboard.components.c1_vio._facade_spine. Per module-layout.md §6 + §9, runtime_root is the composition root and may import any component's internal modules — so this is allowed. The note is recorded because importing an underscore-prefixed (c1-internal-by-convention) module from runtime_root is unusual: most runtime_root files only import each component's interface.py plus the concrete strategy modules.
• Rationale for the choice: the AZ-335 wiring emits vio.warm_start FDR records that share the same kind="vio.*" namespace and timestamp/bias-norm conventions as the c1-strategy-internal vio.health records (AZ-528 / _facade_spine). Sharing the producer functions guarantees forensic logs across the family stay byte-identical in formatting. Inlining the two helpers in warm_start_wiring.py would introduce 6 lines of duplication and a future drift risk.
• Suggestion: Keep the import. If a future cycle wants to formalize, promote bias_norm + now_iso into a shared helper module (e.g., helpers/iso_timestamps.py already exists for ISO-8601 handling per AZ-526; bias_norm could move to helpers/imu_bias.py).
• Task: AZ-335

Verdict logic

• 0 Critical, 0 High → not FAIL
• 1 Medium + 3 Low → PASS_WITH_WARNINGS
• All findings are non-blocking and documented for cycle-2 follow-up.

Auto-fix Gate

Not applicable (no FAIL findings). All notes are informational / documentation-tightening.