mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-22 07:21:13 +00:00
[AZ-335] C1 warm-start hint persistence + F8 reboot recovery wiring
Adds JsonSidecarWarmStartHintStore (atomic JSON + SHA-256 sidecar via AZ-280) inside c1_vio, plus the cross-strategy WarmStartWiredStrategy wrapper + prime_warm_start_from_disk / prime_warm_start_from_fc hooks at runtime_root. AC-7 post-reset covariance inflation and AC-8 "no fake confidence" baseline floor are enforced at the wiring layer so no strategy module needed edits. Adds three c1_vio config fields (warm_start_store_dir, warm_start_save_period_frames, post_reset_covariance_inflation_factor) and registers the new FDR kind vio.warm_start. 34 unit tests cover all 10 ACs + 3 NFRs. Verdict PASS_WITH_WARNINGS — see _docs/03_implementation/reviews/batch_56_review.md for the four non-blocking documentation findings (F1 cold-start log kind shorthand, F2 strategy-frame pose semantics, F3 dev-hardware perf smoke, F4 runtime_root importing c1-internal _facade_spine for shared FDR conventions). Closes AZ-335; depends on AZ-528 (batch 55). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,165 @@
|
||||
# Batch 56 — Cycle 1 Report
|
||||
|
||||
**Date**: 2026-05-14
|
||||
**Tasks**: AZ-335 (C1 Warm-Start + F8 Reboot Recovery)
|
||||
**Verdict**: COMPLETE — PASS_WITH_WARNINGS
|
||||
|
||||
## Summary
|
||||
|
||||
Implemented the cross-strategy warm-start hint persistence layer, the
|
||||
F2 takeoff (FC EKF) and F8 reboot (disk) prime hooks, and the AC-5.3
|
||||
"no fake confidence" covariance enforcement at the runtime composition
|
||||
layer. The persistence layer is c1-internal
|
||||
(`components/c1_vio/warm_start_store.py`); the cross-strategy wiring
|
||||
(wrapper + prime hooks) lives at the composition root
|
||||
(`runtime_root/warm_start_wiring.py`) so any concrete `VioStrategy`
|
||||
gains warm-start behaviour without per-strategy edits. AC-5.3 is
|
||||
enforced via a wrapper-owned post-reset covariance inflation +
|
||||
baseline floor — not by mutating any strategy. Default ships with the
|
||||
`JsonSidecarWarmStartHintStore` (atomic JSON + SHA-256 sidecar via
|
||||
AZ-280); a future Redis-backed store can plug in via the same
|
||||
`WarmStartHintStore` Protocol without touching the wiring.
|
||||
|
||||
Closes the AZ-335 dependency chain: AZ-331 / AZ-332 / AZ-333 / AZ-334
|
||||
(strategies) + AZ-263 / AZ-269 / AZ-266 / AZ-270 (bootstrap +
|
||||
config + log + compose lint) + AZ-280 (sha256 sidecar) + AZ-272 (FDR
|
||||
schema). Runs immediately after AZ-528 (batch 55) — no other c1_vio
|
||||
work was blocked behind AZ-335.
|
||||
|
||||
## Files added / modified
|
||||
|
||||
### Added (3)
|
||||
|
||||
- `src/gps_denied_onboard/components/c1_vio/warm_start_store.py` — 440
|
||||
lines. Exports `HINT_FILENAME`, `HINT_SCHEMA_VERSION`,
|
||||
`LoadedWarmStartHint` dataclass, `WarmStartHintStore` Protocol,
|
||||
`WarmStartFcSource` Protocol (consumer-side cut over C8 FcAdapter
|
||||
per AZ-507), and the default `JsonSidecarWarmStartHintStore` impl.
|
||||
JSON schema v1: `version`, `calibration_id` (Risk-2 mitigation),
|
||||
`pre_reboot_covariance_norm` (AC-8 floor), `pose` block (4×4 matrix
|
||||
+ velocity + bias + ns timestamp).
|
||||
- `src/gps_denied_onboard/runtime_root/warm_start_wiring.py` — 563
|
||||
lines. Exports `WARM_START_PRODUCER_ID`, `WarmStartWiredStrategy`
|
||||
(the wrapper that adds AC-6 throttled save + AC-7 inflation + AC-8
|
||||
floor on top of any inner `VioStrategy`),
|
||||
`prime_warm_start_from_disk` (F8 hook), and
|
||||
`prime_warm_start_from_fc` (F2 hook). Single point of FDR record
|
||||
emission via `_emit_prime_fdr` and single point of INFO/WARN log
|
||||
emission via `_emit_prime_log`.
|
||||
- `tests/unit/c1_vio/test_az335_warm_start.py` — 34 unit tests
|
||||
covering all 10 ACs + 3 NFRs. Local fakes for `VioStrategy` and
|
||||
`WarmStartFcSource`; real `Sha256Sidecar` on `tmp_path` for the
|
||||
store tests so AC-1 / AC-2 / AC-10 atomicity contracts are
|
||||
exercised against the production helper.
|
||||
|
||||
### Modified (3)
|
||||
|
||||
- `src/gps_denied_onboard/components/c1_vio/config.py` — added
|
||||
`warm_start_store_dir` (default `/var/lib/gps_denied_onboard/warm_start/`),
|
||||
`warm_start_save_period_frames` (default 5),
|
||||
`post_reset_covariance_inflation_factor` (default 2.0). Each new
|
||||
field has a `__post_init__` validation matching the existing
|
||||
pattern.
|
||||
- `src/gps_denied_onboard/fdr_client/records.py` — registered the new
|
||||
FDR kind `vio.warm_start` in `KNOWN_PAYLOAD_KEYS` with the
|
||||
frozen schema {`source`, `strategy_label`, `bias_norm`,
|
||||
`staleness_ns`, `pre_reboot_covariance_norm`}.
|
||||
- `tests/unit/test_az272_fdr_record_schema.py` — added the per-kind
|
||||
fixture branch for `vio.warm_start` so the AC-1 round-trip suite
|
||||
stays exhaustive over `KNOWN_KIND`.
|
||||
|
||||
## Tests
|
||||
|
||||
- `tests/unit/c1_vio/test_az335_warm_start.py` — 34 new tests, all
|
||||
pass (4.01 s).
|
||||
- Adjacent regression sweep (`tests/unit/c1_vio/`,
|
||||
`tests/unit/c13_fdr/`, `tests/unit/composition_root/`,
|
||||
`test_az272_fdr_record_schema`, `test_az269_config_loader`,
|
||||
`test_az270_compose_root`, `test_az273_fdr_client_ringbuf`,
|
||||
`test_az266_logging_schema`, `test_ac1_scaffold_layout`) — 356
|
||||
pass + 6 tier-2 skipped (unchanged from pre-AZ-335 state).
|
||||
|
||||
## AC traceability
|
||||
|
||||
| AC | Status | Test |
|
||||
|-------|--------|-------------------------------------------------------------------|
|
||||
| AC-1 | ✓ | `TestStoreAc1RoundTrip` (3 tests; deep-equal + file presence) |
|
||||
| AC-2 | ✓ | `TestStoreAc2Corrupted` (3 tests; sha mismatch + bad envelope) |
|
||||
| AC-3 | ✓ | `TestWiringAc3ColdStart::test_cold_start_does_not_invoke_reset` |
|
||||
| AC-4 | ✓ | `TestWiringAc4F8Reboot::test_f8_reboot_loads_hint_calls_reset_emits_fdr` |
|
||||
| AC-5 | ✓ | `TestWiringAc5F2Takeoff::test_f2_takeoff_fetches_fc_calls_reset_persists` |
|
||||
| AC-6 | ✓ | `TestWiringAc6PerFrameSave` (2 tests; period=5 + period=1) |
|
||||
| AC-7 | ✓ | `TestWiringAc7PostResetInflation` (2 tests; with/without reset) |
|
||||
| AC-8 | ✓ | `TestWiringAc8CovarianceFloor` (2 tests; floor active + dormant) |
|
||||
| AC-9 | ✓ | `TestStoreAc9Clear` (3 tests; remove + log + idempotent) |
|
||||
| AC-10 | ✓ | `TestStoreAc10Atomicity::test_kill_mid_save_leaves_prior_hint_loadable` |
|
||||
| NFR-perf-save | ✓ | `TestStoreNfrPerf::test_nfr_perf_save_p99_under_50ms` |
|
||||
| NFR-perf-load | ✓ | `TestStoreNfrPerf::test_nfr_perf_load_p99_under_20ms` |
|
||||
| NFR-no-crash | ✓ | `TestWiringNfrNoCrash` (4 tests; FC raise/None + save fail + reset fail) |
|
||||
| Risk-2 (calib) | ✓ | `TestStoreAc3CalibrationMismatch::test_calibration_mismatch_returns_none_with_specific_warn` |
|
||||
|
||||
## Code review
|
||||
|
||||
See `_docs/03_implementation/reviews/batch_56_review.md` — verdict
|
||||
**PASS_WITH_WARNINGS**, 1 Medium + 3 Low findings, all
|
||||
informational / documentation-tightening:
|
||||
|
||||
- F1 (Style, Low): AC-3 spec text shorthand vs source-suffixed log
|
||||
kind — recommend updating spec phrasing in cycle 2.
|
||||
- F2 (Maintainability, Medium): per-frame save uses strategy-frame
|
||||
pose as `body_T_world`; semantically defensible because the
|
||||
strategy's "internal frame" persists across F8 reload via the
|
||||
saved pose; recommend an inline 3-line comment explaining the
|
||||
design choice.
|
||||
- F3 (Spec-Gap, Low): NFR perf tests are dev-hardware smoke; full
|
||||
Tier-2 NVMe perf gate is owned by C1-PT-01 (deferred to E-BBT).
|
||||
- F4 (Architecture, Low): `runtime_root/warm_start_wiring.py`
|
||||
imports c1-internal `_facade_spine` for shared FDR conventions;
|
||||
allowed by module-layout §6, but noted for a possible future
|
||||
promotion of `bias_norm` to `helpers/imu_bias.py`.
|
||||
|
||||
## Outcomes & lessons
|
||||
|
||||
- The Protocol-cut-at-consumer pattern (defining `WarmStartFcSource`
|
||||
inside `c1_vio/warm_start_store.py` instead of importing the
|
||||
concrete C8 `FcAdapter`) is the right shape for AZ-507 compliance.
|
||||
The composition root will wire a thin adapter from C8's actual
|
||||
`FcAdapter` to this Protocol. The AZ-335 wiring tests inject a
|
||||
fake matching the surface directly — no C8 dependency in the test.
|
||||
- Wrapping (rather than per-strategy mixing) for cross-strategy
|
||||
concerns scales: AC-7 inflation + AC-8 floor + AC-6 throttled save
|
||||
all live in one 240-line wrapper class with one inner
|
||||
`VioStrategy` field. The three strategies (OKVIS2 / VINS-Mono /
|
||||
KLT-RANSAC) needed zero edits.
|
||||
- AC-7 and AC-8 stack cleanly: inflation is applied first, then if
|
||||
the inflated norm is below the AC-8 floor it is scaled up to the
|
||||
floor. Both operations preserve SPD because they're positive
|
||||
scalar multiplications. No matrix re-decomposition required.
|
||||
- The AC-NFR-no-crash policy (catch + log + return False; never
|
||||
propagate) is enforced at every prime hook seam: FC source raise,
|
||||
FC source returns None, store.save raises, inner.reset raises.
|
||||
Each path emits a distinct log `kind` so post-mortem can
|
||||
partition the failure mode.
|
||||
|
||||
## Outstanding
|
||||
|
||||
- F1 / F2 / F3 / F4 from this batch's review — non-blocking;
|
||||
recommend folding into a future hygiene PBI alongside any AZ-345+
|
||||
c3 work that touches the same `vio.warm_start` FDR namespace.
|
||||
- The composition root's `compose_*` binaries do NOT yet wire a
|
||||
`WarmStartWiredStrategy` over the `vio_factory` output. The wiring
|
||||
is in place; the actual call site (`runtime_root/runtime.py` or
|
||||
the per-binary compose script) needs to construct the
|
||||
`WarmStartWiredStrategy` + `JsonSidecarWarmStartHintStore` and
|
||||
call the F8 prime hook before the first `process_frame`. This is
|
||||
out of scope for AZ-335 (the spec only delivers the wiring
|
||||
module, not the per-binary integration); the integration belongs
|
||||
to the next-cycle compose-root task that adds the F2/F8 hook
|
||||
invocations alongside the existing strategy build.
|
||||
|
||||
## Next batch
|
||||
|
||||
AZ-345 (C3 DISK + LightGlue Primary Matcher, 5 points) is the next
|
||||
unblocked product PBI per `_dependencies_table.md`. All its
|
||||
dependencies (AZ-263, AZ-269, AZ-278, AZ-282, AZ-298, AZ-299,
|
||||
AZ-303, AZ-281, AZ-321, AZ-266, AZ-272, AZ-344) are complete.
|
||||
@@ -0,0 +1,69 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user