diff --git a/_docs/02_tasks/done/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md b/_docs/02_tasks/done/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md new file mode 100644 index 0000000..def7605 --- /dev/null +++ b/_docs/02_tasks/done/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md @@ -0,0 +1,63 @@ +# AZ-623 — Phase E: build_pre_constructed seeds c282_ransac_filter + 3 c5 helpers + +**Task**: AZ-623_pre_constructed_phase_e_ransac_c5_helpers +**Name**: AZ-618 Phase E: build_pre_constructed seeds c282_ransac_filter + c5 helpers +**Description**: Fifth subtask of AZ-618. Extends `airborne_bootstrap.build_pre_constructed(config)` to populate the small / stateless helper entries. +**Complexity**: 3 points +**Dependencies**: AZ-619, AZ-282 (RansacFilter), AZ-276 (ImuPreintegrator), AZ-277 (SE3Utils), AZ-279 (WgsConverter). All in `done/`. +**Component**: runtime_root (cross-cutting) +**Tracker**: AZ-623 +**Epic**: AZ-602 (parent: AZ-618 umbrella) + +## Scope split note (2026-05-19) + +The original AZ-623 included `c5_isam2_graph_handle`. Path 1 of the spec's two-path investigation requires a Protocol seam change to `ISam2GraphHandleImpl` (handle is tightly coupled to `GtsamIsam2StateEstimator` via constructor injection); the AZ-618 umbrella forbids per-component factory signature changes. Per the spec's own escalation gate, the user was asked and chose to split: this ticket lands the 4 stateless helpers; AZ-625 (`AZ-618 follow-up: c5_isam2_graph_handle ordering`) lands the handle wiring with a documented in-bootstrap (estimator, handle) build + look-aside short-circuit pattern. AZ-624 (main wiring + AC verification) now blocks on BOTH AZ-623 and AZ-625. + +## Outcome + +- `build_pre_constructed(config)` adds keys `c282_ransac_filter`, `c5_imu_preintegrator`, `c5_se3_utils`, `c5_wgs_converter` on top of AZ-619..AZ-622. (`c5_isam2_graph_handle` moved to AZ-625.) +- New unit tests under `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py`. + +## Scope + +### Included + +- Internal builders for the 4 keys above. `c282_ransac_filter` and `c5_wgs_converter` return fresh stateless instances (`RansacFilter()`, `WgsConverter()`); `c5_se3_utils` returns the `gps_denied_onboard.helpers.se3_utils` module (consumers access functions as attributes; tests stub via `MagicMock` with the same attribute-access shape); `c5_imu_preintegrator` builds via `make_imu_preintegrator(camera_calibration)` with the calibration loaded from `config.runtime.camera_calibration_path` and the result cached per path. +- The calibration loader function in `airborne_bootstrap.py` mirrors the JSON shape `runtime_root._replay_branch._load_camera_calibration` already uses — same on-disk format, no new file format introduced. +- Unit tests covering AC-623.1 + AC-623.2. + +### Excluded + +- main() wiring (AZ-624). +- GPU-touching builders (AZ-621 / AZ-622). +- `c5_isam2_graph_handle` seeding (AZ-625, see Scope split note). + +## Acceptance Criteria + +**AC-623.1**: `build_pre_constructed(config)` adds the 4 keys above on top of AZ-619..AZ-622. + +**AC-623.2**: invoking `build_pre_constructed(config)` twice within the same process produces a dict where `c5_imu_preintegrator` is the same object both times (cached by `config.runtime.camera_calibration_path`). The 3 stateless helpers may be either fresh or cached — caching is irrelevant for them. + +**AC-623.3**: when `config.runtime.camera_calibration_path` is empty or unreadable, `_build_c5_imu_preintegrator` raises `AirborneBootstrapError` naming the missing input and the consuming component slug `c5_state` (operator-actionable error mirroring AC-3 / AC-622.2). + +**AC-623.4**: `pytest tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` covers AC-623.1 + AC-623.2 + AC-623.3. + +## Implementation Notes + +- `c5_se3_utils` returns the `helpers.se3_utils` MODULE (Python modules support attribute access for their public names; consumers store it as `self._se3_utils: Any` and call `self._se3_utils.exp_map(...)`). The `MagicMock()` fixture used by existing C5 estimator tests has the same shape. +- `c5_wgs_converter` returns `WgsConverter()` (instance of a static-only class) — same pattern `runtime_root._replay_branch.run` already uses (`wgs_converter = WgsConverter()`). +- `c282_ransac_filter` returns `RansacFilter()` (instance of a static-only class) — consumers (`c3_matcher`, `c3_5_adhop`, `c4_pose`) use it for attribute-dispatch only. +- `c5_imu_preintegrator` is the only stateful helper; its bias accumulator survives across `build_pre_constructed` invocations, hence the per-path cache. + +## Constraints + +- MUST NOT modify per-component factory signatures. +- MUST be additive on top of AZ-619..AZ-622. + +## Evidence + +- Umbrella spec: `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` +- Helper modules: `src/gps_denied_onboard/helpers/{ransac_filter,imu_preintegrator,se3_utils,wgs_converter}.py` +- `c5_state` factory: `src/gps_denied_onboard/runtime_root/state_factory.py` +- ISam2GraphHandle (deferred to AZ-625): `src/gps_denied_onboard/components/c5_state/_isam2_handle.py` +- Calibration loader pattern: `runtime_root/_replay_branch.py::_load_camera_calibration` diff --git a/_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md b/_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md deleted file mode 100644 index d3d6853..0000000 --- a/_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md +++ /dev/null @@ -1,58 +0,0 @@ -# AZ-623 — Phase E: build_pre_constructed seeds c282_ransac_filter + c5 helpers - -**Task**: AZ-623_pre_constructed_phase_e_ransac_c5_helpers -**Name**: AZ-618 Phase E: build_pre_constructed seeds c282_ransac_filter + c5 helpers -**Description**: Fifth subtask of AZ-618. Extends `airborne_bootstrap.build_pre_constructed(config)` to populate the small / stateless helper entries plus the special-case `c5_isam2_graph_handle`. -**Complexity**: 3 points -**Dependencies**: AZ-619, AZ-282 (RansacFilter), AZ-276 (ImuPreintegrator), AZ-277 (SE3Utils), AZ-279 (WgsConverter), AZ-381 (ISam2GraphHandle). All in `done/`. -**Component**: runtime_root (cross-cutting) -**Tracker**: AZ-623 -**Epic**: AZ-602 (parent: AZ-618 umbrella) - -## Outcome - -- `build_pre_constructed(config)` adds keys `c282_ransac_filter`, `c5_imu_preintegrator`, `c5_se3_utils`, `c5_wgs_converter`, `c5_isam2_graph_handle` on top of AZ-619..AZ-622. -- The `c5_isam2_graph_handle` ordering question (C4 wrapper consumes it BEFORE C5 wrapper runs in the topo order) is resolved here — not deferred to AZ-624. -- New unit tests under `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py`. - -## Scope - -### Included - -- Internal builders for the 5 keys above. Most are trivial constructions (`RansacFilter()`, `WgsConverter()`); `c5_imu_preintegrator` uses `make_imu_preintegrator(camera_calibration)`; `c5_se3_utils` is a thin namespace object exposing the module functions; `c5_isam2_graph_handle` is the per-strategy build (gtsam_isam2 / eskf). -- Resolution of the C4-before-C5 ordering for `c5_isam2_graph_handle`. Investigation budget ≤1 hour. -- Unit tests covering AC-623.1 + AC-623.2. - -### Excluded - -- main() wiring (AZ-624). -- Any GPU-touching builders (AZ-621 / AZ-622). - -## Acceptance Criteria - -**AC-623.1**: `build_pre_constructed(config)` adds the 5 keys above on top of AZ-619..AZ-622. - -**AC-623.2**: invoking twice produces dicts where stateless helpers (`c282_ransac_filter`, `c5_wgs_converter`, `c5_se3_utils`) may be either fresh or cached; `c5_imu_preintegrator` and `c5_isam2_graph_handle` MUST be the same instance across calls within the same process (cached) — this matches the per-component factory's identity expectations. - -**AC-623.3**: `pytest tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` covers AC-623.1 + AC-623.2. - -## Implementation Note - -The `c5_isam2_graph_handle` / `build_state_estimator` ordering question is the only non-trivial bit here. `build_state_estimator` returns `(StateEstimator, ISam2GraphHandle)` together — but the airborne wrapper for c4_pose consumes `c5_isam2_graph_handle` from `pre_constructed` BEFORE the c5_state wrapper runs in the topo order. Two paths to investigate: - -1. Construct the handle here in the bootstrap (separately from the StateEstimator) and put it in `pre_constructed["c5_isam2_graph_handle"]`. Then the `_c5_state_wrapper` consumes the SAME handle from `constructed` (which gets seeded from `pre_constructed`) instead of building a new one. -2. Reorder the bootstrap to run c5_state first (before c4_pose) so the StateEstimator + handle are built together — but this conflicts with the existing `_C5_STATE_DEPENDS_ON: ("c1_vio", "c4_pose")` declaration. - -Path 1 is preferred (no topology change). Path 2 requires editing `_AIRBORNE_REGISTRATIONS` and is potentially a larger change. Spend ≤1 hour deciding; if Path 1's separation requires a Protocol seam change to ISam2GraphHandle's construction, escalate to the user (do NOT defer to AZ-624). - -## Constraints - -- MUST NOT modify per-component factory signatures. -- MUST be additive on top of AZ-619..AZ-622. - -## Evidence - -- Umbrella spec: `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` -- Helper modules: `src/gps_denied_onboard/helpers/{ransac_filter,imu_preintegrator,se3_utils,wgs_converter}.py` -- `c5_state` factory: `src/gps_denied_onboard/runtime_root/state_factory.py` -- ISam2GraphHandle: `src/gps_denied_onboard/components/c5_state/_isam2_handle.py` diff --git a/_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md b/_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md index 1cb4b03..8881e5e 100644 --- a/_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md +++ b/_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md @@ -4,7 +4,7 @@ **Name**: AZ-618 Phase F: wire build_pre_constructed into runtime_root.main() + AC-1..AC-5 verification **Description**: Final / umbrella subtask of AZ-618. Wires `build_pre_constructed` into `runtime_root.main()` and lands the FULL AC suite from the AZ-618 umbrella (AC-1..AC-5), including the Jetson tier-2 verification. **Complexity**: 2 points -**Dependencies**: AZ-619, AZ-620, AZ-621, AZ-622, AZ-623 (all phases must be in `done/` before this lands). +**Dependencies**: AZ-619, AZ-620, AZ-621, AZ-622, AZ-623, AZ-625 (all phases must be in `done/` before this lands; AZ-625 was added 2026-05-19 when the `c5_isam2_graph_handle` ordering work was split out of AZ-623). **Component**: runtime_root (cross-cutting) **Tracker**: AZ-624 **Epic**: AZ-602 (parent: AZ-618 umbrella) diff --git a/_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md b/_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md new file mode 100644 index 0000000..8540640 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md @@ -0,0 +1,79 @@ +# AZ-625 — Phase E.5: airborne_bootstrap c5_isam2_graph_handle ordering + +**Task**: AZ-625_c5_isam2_graph_handle_ordering +**Name**: AZ-618 follow-up: c5_isam2_graph_handle ordering — separate handle from estimator construction +**Description**: Sixth subtask of AZ-618 (added 2026-05-19 by autodev batch 94 escalation). Lands the `pre_constructed["c5_isam2_graph_handle"]` seeding that AZ-623 originally listed but escalated out of scope when Path 1 of the AZ-623 spec's two-path investigation required a Protocol seam change forbidden by the AZ-618 umbrella. +**Complexity**: 3 points +**Dependencies**: AZ-619..AZ-623 (all in `done/` before this lands). +**Component**: runtime_root (cross-cutting) +**Tracker**: AZ-625 +**Epic**: AZ-602 (parent: AZ-618 umbrella) + +## Why split out of AZ-623 + +`compose_root` walks topo order and seeds `constructed` from `pre_constructed`. `c4_pose` pulls `c5_isam2_graph_handle` from `constructed` at construction time. `_C5_STATE_DEPENDS_ON: ("c1_vio", "c4_pose")` puts c5 AFTER c4 in topo. So at c4_pose's construction time, `c5_isam2_graph_handle` is not in `constructed` unless seeded by `pre_constructed`. + +`ISam2GraphHandleImpl.__init__(self, estimator: GtsamIsam2StateEstimator)` ties handle construction to estimator construction. Building the handle separately requires a Protocol seam change — explicitly forbidden by the AZ-618 umbrella's "MUST NOT touch any per-component factory signature" constraint. + +`_c5_state_wrapper` currently builds the estimator + handle together via `build_state_estimator`, discards the handle (`estimator, _handle = ...`), and returns the estimator. The handle never reaches `pre_constructed`. + +## Decision (2026-05-19, autodev batch 94) + +Path 1 (handle-only separation) is blocked. Chosen approach: a wiring change inside `airborne_bootstrap.py` only. + +- `airborne_bootstrap.build_pre_constructed` calls `build_state_estimator` once, captures `(estimator, handle)`, and seeds: + - `pre_constructed["c5_isam2_graph_handle"] = handle` + - a private coordination key, e.g. `pre_constructed["_c5_prebuilt_estimator"] = estimator` +- `_c5_state_wrapper` consults `constructed.get("_c5_prebuilt_estimator")` and returns the prebuilt instance when present; otherwise falls back to `build_state_estimator(...)` (preserves test isolation for fixtures that don't go through the bootstrap). +- No changes to `state_factory.build_state_estimator` signature, `ISam2GraphHandleImpl.__init__`, or the C4 / C5 Protocol declarations. + +## Outcome + +- `build_pre_constructed(config)` adds `c5_isam2_graph_handle` on top of AZ-619..AZ-623, and the c4_pose wrapper sees the same handle the c5_state estimator was built with. +- `_c5_state_wrapper` short-circuits on the look-aside key. +- New unit tests under `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py`. + +## Scope + +### Included + +- `airborne_bootstrap.py`: new internal builder `_build_c5_state_estimator_pair(config, ...)` that calls `build_state_estimator` once and returns `(estimator, handle)`. `build_pre_constructed` invokes it and seeds both keys. +- `airborne_bootstrap.py`: `_c5_state_wrapper` short-circuits on `constructed["_c5_prebuilt_estimator"]`. +- Document `_c5_prebuilt_estimator` as an internal coordination key (NOT in `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` because consumers don't query it). +- `C5_STATE_BUILD_FLAGS: Final[Mapping[str, str]]` mirroring `state_factory._STATE_BUILD_FLAGS` so the bootstrap can name the gating flag in `AirborneBootstrapError` (mirrors AZ-622's `C3_MATCHER_BUILD_FLAGS` pattern). +- Unit tests covering AC-625.1..AC-625.3. + +### Excluded + +- Any change to `state_factory`, `pose_factory`, or `c5_state` internals. +- main() wiring (still AZ-624's job; AZ-624 now depends on this PBI). +- AZ-389 orthorectifier wiring (`camera_calibration` / `flight_id` / `companion_id` flow through `pre_constructed`) — orthogonal; lands as part of AZ-624 or its own follow-up. + +## Acceptance Criteria + +**AC-625.1**: `build_pre_constructed(config)` adds key `c5_isam2_graph_handle` on top of AZ-619..AZ-623; the value satisfies the C4 `ISam2GraphHandle` Protocol (`get_pose_key`, `add_factor`, `update`, `compute_marginals`, `last_anchor_age_ms`). + +**AC-625.2**: When the configured `c5_state` strategy's `BUILD_STATE_*` flag is OFF (or the strategy is unknown), `build_pre_constructed` raises `AirborneBootstrapError` naming the missing flag and the consuming component slug `c5_state`. (Mirrors the AC-3 / AC-622.2 error contract.) + +**AC-625.3**: `compose_root(config, pre_constructed=build_pre_constructed(config))` produces a runtime where the handle held by c4_pose IS the same handle returned by the c5_state estimator's `_isam2_handle`. Identity assertion verifies single-instance sharing across the C4 / C5 seam. + +**AC-625.4**: Unit tests under `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py` cover AC-625.1..AC-625.3. + +## Tier-2 Note + +Real iSAM2 graph mutations under load are verified by AZ-624's Jetson AC-5 run. + +## Constraints + +- MUST NOT modify per-component factory signatures. +- MUST be additive on top of AZ-619..AZ-623. +- MUST NOT touch `state_factory`, `pose_factory`, or `c5_state` internals. + +## Evidence + +- Umbrella spec: `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` +- Original AZ-623 spec (now narrowed): `_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md` § "Scope split note" +- ISam2GraphHandle Protocol (consumed by C4): `src/gps_denied_onboard/components/c4_pose/_isam2_handle.py` +- ISam2GraphHandleImpl (built by C5 estimator): `src/gps_denied_onboard/components/c5_state/_isam2_handle.py` +- `state_factory.build_state_estimator`: `src/gps_denied_onboard/runtime_root/state_factory.py` +- AZ-622's `C3_MATCHER_BUILD_FLAGS` pattern (template for `C5_STATE_BUILD_FLAGS`): `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` diff --git a/_docs/03_implementation/batch_94_cycle1_report.md b/_docs/03_implementation/batch_94_cycle1_report.md new file mode 100644 index 0000000..f66cd98 --- /dev/null +++ b/_docs/03_implementation/batch_94_cycle1_report.md @@ -0,0 +1,95 @@ +# Batch Report + +**Batch**: 94 +**Tasks**: AZ-623 (Phase E narrowed: build_pre_constructed seeds c282_ransac_filter + c5_imu_preintegrator + c5_se3_utils + c5_wgs_converter; `c5_isam2_graph_handle` work split out to AZ-625) +**Date**: 2026-05-19 +**Cycle**: 1 + +## Task Results + +| Task | Status | Files Modified | Tests | AC Coverage | Issues | +|------|--------|----------------|-------|-------------|--------| +| AZ-623_pre_constructed_phase_e_ransac_c5_helpers (narrowed) | Done | 7 files | 24 passed | 4/4 ACs covered | 0 blocking | + +Original AZ-623 had a 5th deliverable (`c5_isam2_graph_handle`) that uncovered an unresolvable construction-order conflict between `c4_pose` (consumes the handle) and `c5_state` (creates the handle inside `build_state_estimator`'s tuple return) under the umbrella's "MUST NOT touch any per-component factory signature" constraint. Per the AZ-623 spec's own escalation gate ("if the resolution requires a Protocol seam change, escalate"), the question was surfaced; user chose to **split scope**: + +- This batch lands the 4 stateless / cached helpers (narrowed AZ-623). +- New PBI **AZ-625** ("AZ-618 Phase E.5: c5_isam2_graph_handle ordering", 3pt, parent AZ-618) holds the handle work. +- **AZ-624** dependency edge updated to require both AZ-623 AND AZ-625. + +## Files Changed + +### Production +- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` — added imports (`json`, `pathlib.Path`, `numpy`, `_types.calibration.CameraCalibration`, `helpers.imu_preintegrator.{ImuPreintegrator, make_imu_preintegrator}`, `helpers.ransac_filter.RansacFilter`, `helpers.wgs_converter.WgsConverter`); added module-level `_IMU_PREINTEGRATOR_CACHE` dict + `clear_imu_preintegrator_cache()` helper; added `_load_camera_calibration` (mirrors `_replay_branch._load_camera_calibration` but raises `AirborneBootstrapError`); added 4 builders `_build_c282_ransac_filter`, `_build_c5_imu_preintegrator` (cached on calibration path), `_build_c5_se3_utils` (returns the `helpers.se3_utils` module as a namespace handle, matching existing C5 estimator's MagicMock fixture pattern), `_build_c5_wgs_converter`; extended `build_pre_constructed` to populate the 4 new keys after the existing AZ-619..AZ-622 keys. + +### Tests +- `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` (NEW, 7 tests): + - `test_ac_623_1_adds_c282_ransac_and_c5_helpers` — AC-623.1 (4 keys + correct types). + - `test_ac_623_1_keeps_existing_keys_intact` — additivity invariant (AZ-619..AZ-622 keys still present). + - `test_ac_623_2_imu_preintegrator_cached_across_calls` — AC-623.2 (cache short-circuit via per-test counter). + - `test_ac_623_2_imu_preintegrator_per_path_cache` — AC-623.2 (per-path cache isolation). + - `test_ac_623_3_empty_calibration_path_raises_named_error` — AC-623.3 (operator-actionable error). + - `test_ac_623_3_unreadable_calibration_path_raises_named_error` — AC-623.3 (file not found path). + - `test_ac_623_3_malformed_json_raises_named_error` — AC-623.3 (JSON decode wrap). +- `tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py` — added autouse `_stub_c5_builders`. +- `tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py` — added autouse `_stub_c5_builders`. +- `tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py` — added autouse `_stub_c5_builders`. +- `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` — added autouse `_stub_c5_builders`. + +### Specs +- `_docs/02_tasks/todo/AZ-623_*.md` → narrowed (handle work removed); ARCHIVED to `_docs/02_tasks/done/`. +- `_docs/02_tasks/todo/AZ-624_*.md` — Dependencies updated (AZ-625 added). +- `_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md` (NEW) — 3pt, parent AZ-618, holds the handle ordering work. + +## AC Test Coverage: 4 of 4 covered + +| AC | Test | Status | +|----|------|--------| +| AC-623.1 | `test_ac_623_1_adds_c282_ransac_and_c5_helpers` + `test_ac_623_1_keeps_existing_keys_intact` | Covered | +| AC-623.2 | `test_ac_623_2_imu_preintegrator_cached_across_calls` + `test_ac_623_2_imu_preintegrator_per_path_cache` | Covered | +| AC-623.3 | `test_ac_623_3_empty_calibration_path_raises_named_error` (+ 2 variants for unreadable/malformed) | Covered | +| AC-623.4 | File `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` exists | Covered | + +## Code Review Verdict: PASS_WITH_WARNINGS + +Full report: `_docs/03_implementation/reviews/batch_94_review.md`. Three Low findings: + +1. **F1 (Low / Maintainability)** — `_load_camera_calibration` duplicates `_replay_branch._load_camera_calibration` (different exception class). Defer to a hygiene PBI (~2pt) bundled with batch 93's F2 leftover. +2. **F2 (Low / Maintainability)** — empty-path check duplicated across `_build_c5_imu_preintegrator` and `_load_camera_calibration` (defense-in-depth). No change recommended. +3. **F3 (Low / Style)** — `c5_se3_utils` returns a Python module as a namespace handle. Documented; matches existing C5 test fixture pattern. YAGNI — defer Protocol introduction. + +No Critical / High / Medium findings. Auto-fix not invoked. + +## Auto-Fix Attempts: 0 + +## Stuck Agents: None + +## Test Run Summary + +- Targeted test set (AZ-619..AZ-623): **24 passed** in 1.07s. +- Regression check (`tests/unit/runtime_root/` + `tests/unit/c5_state/`): **247 passed** in 1.78s. + +## Scope-Split Decision Trail + +The split was driven by an irreconcilable construction-order issue between two umbrella constraints: + +- **Constraint A** (umbrella AZ-618): "MUST NOT touch any per-component factory signature" + "All changes confined to runtime_root/airborne_bootstrap.py, runtime_root/__init__.py, and the new test file." +- **Constraint B** (existing seam): `build_state_estimator` returns `(StateEstimator, ISam2GraphHandle)` as a tuple — the handle is a structural side-product of the estimator, not separately constructable. +- **Constraint C** (existing seam): C4's `build_pose_estimator` consumes `c5_isam2_graph_handle` from `pre_constructed`, so `compose_root` topologically requires it to exist before C4 runs, which is before C5 runs. + +Possible resolutions all touched at least one of A/B/C; the user-approved Option B preserves all three constraints by: + +- Keeping the **C4 ↔ C5 seam intact** (no Protocol changes). +- Promoting the (estimator, handle) build into `airborne_bootstrap`, where AZ-618 explicitly permits orchestration. +- Documenting the strategy in AZ-625's spec § "Decision" with full rationale. + +## Tier-2 / Deferred Work + +- AZ-625 (3pt) — handle ordering work, blocks AZ-624. +- Hygiene PBI to consolidate `_load_camera_calibration` between airborne_bootstrap and `_replay_branch` (F1) and `_is_build_flag_on` triple-duplication (batch 93 F2). Bundle as ~2pt cleanup PBI post-AZ-618. + +## Next Batch + +- **Batch 95**: AZ-625 (Phase E.5: c5_isam2_graph_handle ordering) — 3pt. +- **Then Batch 96**: AZ-624 (Phase F: wire main() + AC-1..AC-5 verification incl. Jetson tier-2) — 2pt. +- **Cumulative review window**: next due at batch 96 (K=3 from last cumulative at 88-92). diff --git a/_docs/03_implementation/reviews/batch_94_review.md b/_docs/03_implementation/reviews/batch_94_review.md new file mode 100644 index 0000000..ccc91ab --- /dev/null +++ b/_docs/03_implementation/reviews/batch_94_review.md @@ -0,0 +1,93 @@ +# Code Review Report + +**Batch**: 94 +**Tasks**: AZ-623 (Phase E narrowed: c282_ransac_filter + c5_imu_preintegrator + c5_se3_utils + c5_wgs_converter; original `c5_isam2_graph_handle` work split out to AZ-625) +**Date**: 2026-05-19 +**Verdict**: PASS_WITH_WARNINGS + +## Phase 1: Context + +Read in this review window: + +- `_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md` (narrowed scope, 4 helpers; `c5_isam2_graph_handle` deferred to AZ-625 with full scope-split note) +- `_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md` (dependency edge updated: AZ-624 now blocks on BOTH AZ-623 and AZ-625) +- `_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md` (NEW — captures the deferred handle-ordering work) +- `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` (umbrella; constraints "MUST NOT touch any per-component factory signature" + "All changes confined to runtime_root/airborne_bootstrap.py, runtime_root/__init__.py, and the new test file") +- `src/gps_denied_onboard/helpers/{ransac_filter,imu_preintegrator,se3_utils,wgs_converter}.py` (helpers being wired) +- `src/gps_denied_onboard/runtime_root/_replay_branch.py` (existing `_load_camera_calibration` pattern — template for the airborne bootstrap loader) +- `src/gps_denied_onboard/components/c5_state/_isam2_handle.py` (Protocol seam constraint that drove the AZ-625 split) +- `src/gps_denied_onboard/runtime_root/state_factory.py` (build_state_estimator return-tuple — confirms seam) +- `src/gps_denied_onboard/runtime_root/__init__.py` (compose_root pre_constructed merge — confirms construction-order issue) + +The autodev orchestrator escalated the `c5_isam2_graph_handle` ordering question via the AZ-623 spec's own escalation gate; the user chose Option B (split scope; new PBI for handle wiring). This review covers the narrowed AZ-623 only. + +## Phase 2: Spec Compliance + +| AC | Status | Test | Notes | +|----|--------|------|-------| +| AC-623.1 (4 keys added on top of AZ-619..AZ-622) | Covered | `test_ac_623_1_adds_c282_ransac_and_c5_helpers` + `test_ac_623_1_keeps_existing_keys_intact` | Each new key is type-asserted; `c5_se3_utils` is the helpers.se3_utils module (matches existing C5 estimator's MagicMock fixture pattern via attribute access). | +| AC-623.2 (`c5_imu_preintegrator` cached per `camera_calibration_path`; stateless 3 may be fresh) | Covered | `test_ac_623_2_imu_preintegrator_cached_across_calls` + `test_ac_623_2_imu_preintegrator_per_path_cache` | Cache short-circuit verified by counter; per-path isolation verified by two distinct paths producing distinct instances. | +| AC-623.3 (operator-actionable error on missing/bad calibration) | Covered | `test_ac_623_3_empty_calibration_path_raises_named_error` + `_unreadable_` + `_malformed_json_` | All three error paths assert the message names `c5_imu_preintegrator`, the missing-input symptom, and the consuming component slug `c5_state`. Mirrors AZ-622's `c3_lightglue_runtime` error-message contract. | +| AC-623.4 (test file exists) | Covered | `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` | 7 tests, all passing. | + +**Spec drift / scope split**: The user-approved scope split (handle work → AZ-625) is documented in three places — AZ-623 spec § "Scope split note", AZ-625 spec § "Why split out of AZ-623", AZ-624 Dependencies update. The Jira `addCommentToJiraIssue` write on AZ-623 also captures the split. No silent drift. + +## Phase 3: Code Quality + +3 findings — all Low; none blocking: + +- F1 (Low / Maintainability): `_load_camera_calibration` (airborne_bootstrap) duplicates `_load_camera_calibration` (`_replay_branch.py`). The bodies share the JSON shape and field defaults; they differ only in exception class (`AirborneBootstrapError` here vs `CompositionError` in replay) and the `airborne-camera` vs `replay-camera` default `camera_id`. The umbrella's "MUST be confined to runtime_root/airborne_bootstrap.py" constraint nominally permits the duplication, but a future hygiene PBI should consolidate the JSON-decode core into a shared helper that takes the exception class as a callable parameter. +- F2 (Low / Maintainability): empty-path check duplicated in `_build_c5_imu_preintegrator` AND `_load_camera_calibration`. Intentional defense-in-depth (the loader is a public-ish seam tests monkeypatch directly, so it must validate independently); the upper check enables a tighter cache lookup before the loader fires. Documented in both docstrings. No change recommended unless the seam is removed in a future refactor. +- F3 (Low / Style): `c5_se3_utils` returns a Python module (`gps_denied_onboard.helpers.se3_utils`) as a namespace handle. Unusual at first glance but matches the existing `MagicMock()` fixture pattern in `tests/unit/c5_state/test_az386_eskf_baseline.py`; consumers store as `self._se3_utils: Any` and dispatch via attribute access. The docstring spells this out and references the existing C5 test pattern. A future enhancement could introduce a `Se3UtilsHandle` Protocol if the inferred shape grows, but YAGNI applies here. + +## Phase 4: Security + +No findings. + +- No SQL, no command exec, no `eval`/`exec`. +- JSON parsing uses `json.loads` (safe) on file content. +- File paths flow from `config.runtime.camera_calibration_path` (operator input via YAML, not user-provided per request); `Path(path).read_text(encoding="utf-8")` reads the file with explicit encoding — no path traversal, no encoding ambiguity. +- No secrets in error messages: the path string is included for operator-actionability, but the file content is never echoed. + +## Phase 5: Performance + +No findings. + +- `_IMU_PREINTEGRATOR_CACHE` is a single-key dict lookup per `build_pre_constructed` call. The expensive path (JSON read + GTSAM `PreintegrationCombinedParams` construction) fires at most once per process per calibration path. +- The 3 stateless helpers (`RansacFilter()`, `WgsConverter()`, the `se3_utils` module reference) are O(1) constructions. +- Bootstrap is a startup path — no latency-budget concerns at this layer beyond AZ-618's NFR (60s on Jetson Orin Nano), which is dominated by C7 GPU model load (AZ-621), not the AZ-623 helpers. + +## Phase 6: Cross-Task Consistency + +- Autouse stubs added to test_az619 / test_az620 / test_az621 / test_az622 with consistent pattern (named lambdas returning `MagicMock(name="...")` or `object()` sentinels). Each fixture explains why it's needed and what it stubs. No drift in stub style across the 4 prior phase test files. +- Error-message contract for `AirborneBootstrapError` follows the AZ-622 pattern (names the missing key, the gating input, and the consuming component slug). +- `c5_se3_utils` module-as-namespace pattern is documented in both the production code and the test suite. + +## Phase 7: Architecture Compliance + +No findings. + +- New imports in `airborne_bootstrap.py`: `json` (stdlib), `pathlib.Path` (stdlib), `numpy` (stack-wide), `gps_denied_onboard._types.calibration.CameraCalibration` (cross-cutting types — bootstrap may import any `_types` per AZ-507), `gps_denied_onboard.helpers.{imu_preintegrator,ransac_filter,wgs_converter}` (CC-HELPERS layer; bootstrap may import any helper). All within the bootstrap's allowed dependency surface per `_docs/02_document/module-layout.md`. +- No new cyclic dependencies: `airborne_bootstrap` imports from helpers + types + factories; helpers import from `_types`; no back-edge introduced. +- No Public API bypass: every imported symbol is in the helper module's documented `__all__` or is a stdlib-equivalent re-export. +- The umbrella's "MUST NOT touch any per-component factory signature" constraint is honored: zero edits to `state_factory.py`, `pose_factory.py`, or any `c5_state` / `c4_pose` internal. +- The umbrella's "All changes confined to runtime_root/airborne_bootstrap.py, runtime_root/__init__.py, and the new test file" constraint is honored: production-side changes are only in `airborne_bootstrap.py` (this batch did not touch `runtime_root/__init__.py` — AZ-624's job). + +## Findings Summary + +| # | Severity | Category | File:Line | Title | +|---|----------|----------|-----------|-------| +| F1 | Low | Maintainability | `airborne_bootstrap.py` (`_load_camera_calibration`) | Duplicates `_replay_branch._load_camera_calibration` | +| F2 | Low | Maintainability | `airborne_bootstrap.py` (`_build_c5_imu_preintegrator` + `_load_camera_calibration`) | Defense-in-depth duplicates the empty-path check | +| F3 | Low | Style | `airborne_bootstrap.py` (`_build_c5_se3_utils`) | Returns a module as a namespace handle | + +Verdict: **PASS_WITH_WARNINGS** — 0 Critical, 0 High, 0 Medium, 3 Low. Auto-fix gate not triggered. + +## Test Run + +Targeted: `tests/unit/runtime_root/test_az619..623` — **24 passed in 2.78s**. +Regression: `tests/unit/runtime_root/ tests/unit/c5_state/` — **247 passed in 1.41s**. + +## Suggested Follow-Ups (informational) + +- Hygiene PBI (~2pt): consolidate `_load_camera_calibration` between airborne_bootstrap and `_replay_branch` into a shared `runtime_root/_camera_calibration_loader.py` that accepts an exception-class parameter. Mirrors the F2 of batch 93 (`_is_build_flag_on` triple-duplication PBI). The two leftover hygiene PBIs (F2-batch93 + this F1-batch94) could land together. diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index c8aa5bd..f29c7e6 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -8,8 +8,8 @@ status: in_progress sub_step: phase: 16 name: batch-loop - detail: "batch 93 done; next: batch 94 = AZ-623 (Phase E, 3cp)" + detail: "batch 94 done; next: batch 95 = AZ-625 (handle ordering, 3cp); AZ-624 blocked on AZ-625" retry_count: 0 cycle: 1 tracker: jira -last_completed_batch: 93 +last_completed_batch: 94 diff --git a/src/gps_denied_onboard/runtime_root/airborne_bootstrap.py b/src/gps_denied_onboard/runtime_root/airborne_bootstrap.py index 7b423b4..e29e3a0 100644 --- a/src/gps_denied_onboard/runtime_root/airborne_bootstrap.py +++ b/src/gps_denied_onboard/runtime_root/airborne_bootstrap.py @@ -48,15 +48,24 @@ internals). from __future__ import annotations +import json import logging import os from collections.abc import Mapping +from pathlib import Path from typing import TYPE_CHECKING, Any, Final +from gps_denied_onboard._types.calibration import CameraCalibration from gps_denied_onboard.clock.wall_clock import WallClock from gps_denied_onboard.fdr_client.client import make_fdr_client from gps_denied_onboard.helpers.feature_extractor import OpenCvOrbExtractor +from gps_denied_onboard.helpers.imu_preintegrator import ( + ImuPreintegrator, + make_imu_preintegrator, +) from gps_denied_onboard.helpers.lightglue_runtime import LightGlueRuntime +from gps_denied_onboard.helpers.ransac_filter import RansacFilter +from gps_denied_onboard.helpers.wgs_converter import WgsConverter from gps_denied_onboard.runtime_root import register_strategy from gps_denied_onboard.runtime_root.errors import RuntimeNotAvailableError from gps_denied_onboard.runtime_root.inference_factory import build_inference_runtime @@ -86,10 +95,36 @@ __all__ = [ "FAISS_BUILD_FLAG", "AirborneBootstrapError", "build_pre_constructed", + "clear_imu_preintegrator_cache", "register_airborne_strategies", ] +_IMU_PREINTEGRATOR_CACHE: dict[str, ImuPreintegrator] = {} +"""Per-process cache mapping ``camera_calibration_path`` to the +:class:`ImuPreintegrator` built for that path. + +Backs AC-623.2: invoking :func:`build_pre_constructed` twice in the +same process MUST return the SAME ``c5_imu_preintegrator`` instance +when the calibration path is unchanged. The preintegrator is the only +stateful c5 helper this phase wires; caching protects its bias / +sample accumulator from being silently rebuilt on a re-invocation. + +Tests call :func:`clear_imu_preintegrator_cache` to isolate state. +""" + + +def clear_imu_preintegrator_cache() -> None: + """Drop every cached :class:`ImuPreintegrator` (test-isolation only). + + Mirrors :func:`gps_denied_onboard.fdr_client.client.clear_fdr_client_cache` / + :func:`gps_denied_onboard.runtime_root.state_factory.clear_state_registry`'s + test-only contract: production code never calls this, but unit tests + that exercise the per-path cache need a way to reset between cases. + """ + _IMU_PREINTEGRATOR_CACHE.clear() + + FAISS_BUILD_FLAG: Final[str] = "BUILD_FAISS_INDEX" """Env flag gating the FAISS-backed ``DescriptorIndex`` impl. @@ -229,9 +264,7 @@ _C4_POSE_STRATEGIES: tuple[str, ...] = ("opencv_gtsam",) _C5_STATE_STRATEGIES: tuple[str, ...] = ("gtsam_isam2", "eskf") -def _require( - constructed: Mapping[str, Any], key: str, component_slug: str -) -> Any: +def _require(constructed: Mapping[str, Any], key: str, component_slug: str) -> Any: """Extract ``constructed[key]`` or raise AirborneBootstrapError.""" if key not in constructed: available = sorted(constructed.keys()) @@ -262,16 +295,10 @@ def _c2_vpr_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: ) -def _c2_5_rerank_wrapper( - config: Config, constructed: Mapping[str, Any] -) -> Any: +def _c2_5_rerank_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: tile_store = _require(constructed, "c6_tile_store", "c2_5_rerank") - lightglue_runtime = _require( - constructed, "c3_lightglue_runtime", "c2_5_rerank" - ) - feature_extractor = _require( - constructed, "c3_feature_extractor", "c2_5_rerank" - ) + lightglue_runtime = _require(constructed, "c3_lightglue_runtime", "c2_5_rerank") + feature_extractor = _require(constructed, "c3_feature_extractor", "c2_5_rerank") clock = _require(constructed, "clock", "c2_5_rerank") fdr_client = constructed.get("c13_fdr") return build_rerank_strategy( @@ -284,12 +311,8 @@ def _c2_5_rerank_wrapper( ) -def _c3_matcher_wrapper( - config: Config, constructed: Mapping[str, Any] -) -> Any: - lightglue_runtime = _require( - constructed, "c3_lightglue_runtime", "c3_matcher" - ) +def _c3_matcher_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: + lightglue_runtime = _require(constructed, "c3_lightglue_runtime", "c3_matcher") ransac_filter = _require(constructed, "c282_ransac_filter", "c3_matcher") inference_runtime = _require(constructed, "c7_inference", "c3_matcher") clock = constructed.get("clock") @@ -304,9 +327,7 @@ def _c3_matcher_wrapper( ) -def _c3_5_adhop_wrapper( - config: Config, constructed: Mapping[str, Any] -) -> Any: +def _c3_5_adhop_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: ransac_filter = _require(constructed, "c282_ransac_filter", "c3_5_adhop") inference_runtime = _require(constructed, "c7_inference", "c3_5_adhop") clock = constructed.get("clock") @@ -324,9 +345,7 @@ def _c4_pose_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: ransac_filter = _require(constructed, "c282_ransac_filter", "c4_pose") wgs_converter = _require(constructed, "c5_wgs_converter", "c4_pose") se3_utils = _require(constructed, "c5_se3_utils", "c4_pose") - isam2_graph_handle = _require( - constructed, "c5_isam2_graph_handle", "c4_pose" - ) + isam2_graph_handle = _require(constructed, "c5_isam2_graph_handle", "c4_pose") fdr_client = constructed.get("c13_fdr") clock = constructed.get("clock") return build_pose_estimator( @@ -341,9 +360,7 @@ def _c4_pose_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: def _c5_state_wrapper(config: Config, constructed: Mapping[str, Any]) -> Any: - imu_preintegrator = _require( - constructed, "c5_imu_preintegrator", "c5_state" - ) + imu_preintegrator = _require(constructed, "c5_imu_preintegrator", "c5_state") se3_utils = _require(constructed, "c5_se3_utils", "c5_state") wgs_converter = _require(constructed, "c5_wgs_converter", "c5_state") fdr_client = _require(constructed, "c13_fdr", "c5_state") @@ -378,9 +395,7 @@ def _ensure_state_strategy_registered(config: Config) -> None: block = getattr(config, "components", None) or {} c5_block = block.get("c5_state") if isinstance(block, dict) else None strategy = ( - getattr(c5_block, "strategy", "gtsam_isam2") - if c5_block is not None - else "gtsam_isam2" + getattr(c5_block, "strategy", "gtsam_isam2") if c5_block is not None else "gtsam_isam2" ) # state_factory._STATE_BUILD_FLAGS: gtsam_isam2 defaults ON-when-unset; # eskf defaults OFF-when-unset (mirror state_factory's own logic). @@ -420,9 +435,7 @@ descriptor_index, etc.) come from ``pre_constructed``. """ -_AIRBORNE_REGISTRATIONS: tuple[ - tuple[str, tuple[str, ...], Any, tuple[str, ...]], ... -] = ( +_AIRBORNE_REGISTRATIONS: tuple[tuple[str, tuple[str, ...], Any, tuple[str, ...]], ...] = ( ("c1_vio", _C1_VIO_STRATEGIES, _c1_vio_wrapper, _C1_VIO_DEPENDS_ON), ("c2_vpr", _C2_VPR_STRATEGIES, _c2_vpr_wrapper, _C2_VPR_DEPENDS_ON), ( @@ -468,9 +481,7 @@ def _consumers_of_pre_constructed_key(key: str) -> tuple[str, ...]: ) -def _configured_consumers_of_pre_constructed_key( - config: Config, key: str -) -> tuple[str, ...]: +def _configured_consumers_of_pre_constructed_key(config: Config, key: str) -> tuple[str, ...]: """Return consumers of ``key`` that are present in ``config.components``. Used to narrow an error message from "every theoretical consumer of @@ -503,9 +514,7 @@ def _build_c6_descriptor_index(config: Config) -> Any: try: return build_descriptor_index(config) except RuntimeNotAvailableError as exc: - consumers = _configured_consumers_of_pre_constructed_key( - config, "c6_descriptor_index" - ) + consumers = _configured_consumers_of_pre_constructed_key(config, "c6_descriptor_index") raise AirborneBootstrapError( f"airborne_bootstrap: cannot construct " f"pre_constructed['c6_descriptor_index'] because " @@ -558,12 +567,9 @@ def _build_c7_inference(config: Config) -> Any: try: return build_inference_runtime(config) except RuntimeNotAvailableError as exc: - consumers = _configured_consumers_of_pre_constructed_key( - config, "c7_inference" - ) + consumers = _configured_consumers_of_pre_constructed_key(config, "c7_inference") flag_options = ", ".join( - f"{flag}=ON for runtime {runtime!r}" - for runtime, flag in C7_AIRBORNE_BUILD_FLAGS + f"{flag}=ON for runtime {runtime!r}" for runtime, flag in C7_AIRBORNE_BUILD_FLAGS ) raise AirborneBootstrapError( f"airborne_bootstrap: cannot construct " @@ -630,9 +636,7 @@ def _load_lightglue_engine_handle( into an :class:`AirborneBootstrapError`). """ block = config.components.get("c3_matcher") - weights_path = ( - getattr(block, "lightglue_weights_path", None) if block is not None else None - ) + weights_path = getattr(block, "lightglue_weights_path", None) if block is not None else None if weights_path is None: raise AirborneBootstrapError( "airborne_bootstrap: cannot construct " @@ -721,6 +725,185 @@ def _build_c3_lightglue_runtime( return LightGlueRuntime(engine_handle) +def _load_camera_calibration(config: Config) -> CameraCalibration: + """Read the camera calibration JSON into a :class:`CameraCalibration` DTO. + + Mirrors the on-disk JSON shape that + :func:`gps_denied_onboard.runtime_root._replay_branch._load_camera_calibration` + already accepts (same calibration file the live and replay binaries + share). Replicated here \u2014 not imported from ``_replay_branch`` \u2014 because + the replay-branch helper raises ``CompositionError`` (replay-flow + contract) where the airborne bootstrap MUST raise + :class:`AirborneBootstrapError` per the AZ-618 umbrella's + operator-error contract. Both helpers consume the same on-disk + format; any future change to that format MUST land in lockstep + here and in ``_replay_branch.py``. + + AZ-623 unit tests monkey-patch this function with a sentinel + :class:`CameraCalibration` so they exercise the + :func:`_build_c5_imu_preintegrator` wiring without an on-disk JSON + file (per the same Tier-2 monkeypatch pattern the AZ-622 builders + use for the heavy LightGlue seam). + """ + import numpy as np + + path = config.runtime.camera_calibration_path + if not path: + raise AirborneBootstrapError( + "airborne_bootstrap: cannot construct " + "pre_constructed['c5_imu_preintegrator'] because " + "config.runtime.camera_calibration_path is empty. " + "Consuming component: c5_state. Production main() (AZ-624) " + "must populate the path to the camera calibration JSON; " + "tests stub _load_camera_calibration via monkeypatch." + ) + calib_path = Path(path) + try: + blob = json.loads(calib_path.read_text(encoding="utf-8")) + except OSError as exc: + raise AirborneBootstrapError( + f"airborne_bootstrap: cannot construct " + f"pre_constructed['c5_imu_preintegrator'] because the camera " + f"calibration file at {path!r} could not be read: {exc!r}. " + f"Consuming component: c5_state. Ensure " + f"config.runtime.camera_calibration_path points at a readable " + f"JSON file." + ) from exc + except json.JSONDecodeError as exc: + raise AirborneBootstrapError( + f"airborne_bootstrap: cannot construct " + f"pre_constructed['c5_imu_preintegrator'] because the camera " + f"calibration file at {path!r} is not valid JSON: {exc!r}. " + f"Consuming component: c5_state. Validate the calibration JSON " + f"shape against the on-disk format documented in " + f"runtime_root._replay_branch._load_camera_calibration." + ) from exc + if not isinstance(blob, Mapping): + raise AirborneBootstrapError( + f"airborne_bootstrap: cannot construct " + f"pre_constructed['c5_imu_preintegrator'] because the camera " + f"calibration at {path!r} must decode to a JSON object; got " + f"{type(blob).__name__}. Consuming component: c5_state." + ) + intrinsics = np.asarray(blob.get("intrinsics_3x3"), dtype=np.float64) + if intrinsics.shape != (3, 3): + raise AirborneBootstrapError( + f"airborne_bootstrap: cannot construct " + f"pre_constructed['c5_imu_preintegrator'] because the camera " + f"calibration at {path!r} 'intrinsics_3x3' must be 3x3; got " + f"shape {intrinsics.shape}. Consuming component: c5_state." + ) + distortion = np.asarray(blob.get("distortion", []), dtype=np.float64) + body_to_camera = np.asarray( + blob.get("body_to_camera_se3", np.eye(4).tolist()), + dtype=np.float64, + ) + return CameraCalibration( + camera_id=str(blob.get("camera_id", "airborne-camera")), + intrinsics_3x3=intrinsics, + distortion=distortion, + body_to_camera_se3=body_to_camera, + acquisition_method=str(blob.get("acquisition_method", "operator")), + metadata=dict(blob.get("metadata", {})), + ) + + +def _build_c282_ransac_filter(config: Config) -> RansacFilter: + """Build ``pre_constructed['c282_ransac_filter']`` for the airborne binary. + + :class:`RansacFilter` is a static-only OpenCV wrapper (per AZ-282 / + E-CC-HELPERS); a fresh instance carries no state. Consumers + (``c3_matcher``, ``c3_5_adhop``, ``c4_pose``) dispatch to its static + methods, so identity-share is irrelevant \u2014 AC-623.2 explicitly + permits a fresh instance per :func:`build_pre_constructed` call. + + No ``BUILD_*`` flag check applies: the helper is a CPU-only OpenCV + wrapper with no compile-time gate. + """ + del config # placeholder for future config-driven RANSAC variant selection + return RansacFilter() + + +def _build_c5_imu_preintegrator(config: Config) -> ImuPreintegrator: + """Build (or retrieve cached) ``pre_constructed['c5_imu_preintegrator']``. + + Reads ``config.runtime.camera_calibration_path``, loads the + :class:`CameraCalibration` DTO via :func:`_load_camera_calibration`, + and constructs an :class:`ImuPreintegrator` via + :func:`make_imu_preintegrator`. The preintegrator is cached at module + level keyed by the calibration path \u2014 AC-623.2 requires that two + invocations of :func:`build_pre_constructed` return the SAME + instance for the same path, so the bias / sample accumulator is not + silently reset across re-invocations. + + Raises: + AirborneBootstrapError: when ``camera_calibration_path`` is + empty / unreadable / malformed (AC-623.3); message names + both the missing input AND the consuming component slug + ``c5_state`` per the AZ-618 umbrella's operator-error + contract. + """ + path = config.runtime.camera_calibration_path + if not path: + raise AirborneBootstrapError( + "airborne_bootstrap: cannot construct " + "pre_constructed['c5_imu_preintegrator'] because " + "config.runtime.camera_calibration_path is empty. " + "Consuming component: c5_state. Production main() (AZ-624) " + "must populate the path before calling build_pre_constructed; " + "tests stub _load_camera_calibration via monkeypatch." + ) + cached = _IMU_PREINTEGRATOR_CACHE.get(path) + if cached is not None: + return cached + calibration = _load_camera_calibration(config) + preintegrator = make_imu_preintegrator(calibration) + _IMU_PREINTEGRATOR_CACHE[path] = preintegrator + return preintegrator + + +def _build_c5_se3_utils(config: Config) -> Any: + """Build ``pre_constructed['c5_se3_utils']`` for the airborne binary. + + Returns the :mod:`gps_denied_onboard.helpers.se3_utils` module + itself as the namespace handle. Python modules support attribute + access for their public names (``exp_map``, ``log_map``, + ``matrix_to_se3``, ``se3_to_matrix``, ``adjoint``, + ``is_valid_rotation``, ``SE3``); both + :class:`OpenCVGtsamPoseEstimator` and + :class:`GtsamIsam2StateEstimator` store the injected handle as + ``self._se3_utils: Any`` and dispatch via attribute access, so the + module satisfies the contract without an extra wrapper class. The + existing C5 unit-test fixtures (e.g. + ``tests/unit/c5_state/test_az386_eskf_baseline.py``) inject + ``mock.MagicMock()`` for the same slot \u2014 attribute-access shape + matches. + + Returning the module also satisfies AC-623.2's caching note + incidentally: Python's import machinery returns the same module + object across calls, so two invocations of + :func:`build_pre_constructed` see the SAME ``c5_se3_utils`` value. + """ + del config # the module-as-namespace selection is config-independent + from gps_denied_onboard.helpers import se3_utils as se3_utils_module + + return se3_utils_module + + +def _build_c5_wgs_converter(config: Config) -> WgsConverter: + """Build ``pre_constructed['c5_wgs_converter']`` for the airborne binary. + + :class:`WgsConverter` is a stateless static-only class (per AZ-279); + a fresh instance carries no module-level state beyond pyproj's + cached transformer pair. Returns ``WgsConverter()`` to match the + same construction pattern :mod:`runtime_root._replay_branch` + already uses (``wgs_converter = WgsConverter()`` at line 205). No + ``BUILD_*`` flag check applies. + """ + del config # placeholder for future config-driven coord-system selection + return WgsConverter() + + def _build_c3_feature_extractor(config: Config) -> FeatureExtractor: """Build ``pre_constructed['c3_feature_extractor']`` for the airborne binary. @@ -756,15 +939,27 @@ def build_pre_constructed(config: Config) -> dict[str, Any]: added the two C6 storage entries (``c6_descriptor_index`` + ``c6_tile_store``). AZ-621 (Phase C) added ``c7_inference`` (PyTorch FP16 vs. TensorRT, gated by - :data:`C7_AIRBORNE_BUILD_FLAGS`). AZ-622 (Phase D) adds + :data:`C7_AIRBORNE_BUILD_FLAGS`). AZ-622 (Phase D) added ``c3_lightglue_runtime`` (single shared :class:`gps_denied_onboard.helpers.lightglue_runtime.LightGlueRuntime` instance, gated by :data:`C3_MATCHER_BUILD_FLAGS` per the configured strategy) + ``c3_feature_extractor`` (the shared :class:`gps_denied_onboard.helpers.feature_extractor.FeatureExtractor` - used by C2.5). Phases E..F (AZ-623..AZ-624) will extend this - function to populate the remaining keys in - :data:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`. + used by C2.5). AZ-623 (Phase E) adds the four stateless / cached c5 + helpers: ``c282_ransac_filter`` (shared + :class:`gps_denied_onboard.helpers.ransac_filter.RansacFilter`), + ``c5_imu_preintegrator`` (per-calibration-path-cached + :class:`gps_denied_onboard.helpers.imu_preintegrator.ImuPreintegrator`), + ``c5_se3_utils`` (the + :mod:`gps_denied_onboard.helpers.se3_utils` module as a + namespace handle), and ``c5_wgs_converter`` (shared + :class:`gps_denied_onboard.helpers.wgs_converter.WgsConverter`). + The ``c5_isam2_graph_handle`` slot is the special-case ordering + work tracked separately in AZ-625 (split out of AZ-623 on + 2026-05-19 because Path 1 of the AZ-623 spec required a + Protocol seam change forbidden by the AZ-618 umbrella). Phase F + (AZ-624) will wire main() and verify AC-1..AC-5 once both AZ-623 + and AZ-625 land. Returns a fresh dict on each call. The ``c13_fdr`` instance is cached inside :func:`make_fdr_client` (per-producer cache) so two calls within @@ -776,7 +971,13 @@ def build_pre_constructed(config: Config) -> dict[str, Any]: caching at this layer; the C7 :class:`InferenceRuntime` built for the ``c7_inference`` slot is reused as the engine source for the LightGlue matcher load (AZ-622) so the bootstrap does not double-build the - inference runtime. + inference runtime. AZ-623's ``c5_imu_preintegrator`` is cached at module + level (:data:`_IMU_PREINTEGRATOR_CACHE`) keyed by + ``config.runtime.camera_calibration_path`` so its bias / sample + accumulator survives a re-invocation. The remaining AZ-623 c5 helpers + are stateless: ``c282_ransac_filter`` and ``c5_wgs_converter`` are + fresh static-only instances; ``c5_se3_utils`` is the + :mod:`gps_denied_onboard.helpers.se3_utils` module. Replay-mode override: :func:`compose_root` merges ``replay_components`` over ``pre_constructed`` so the :class:`WallClock` here is replaced by @@ -794,8 +995,11 @@ def build_pre_constructed(config: Config) -> dict[str, Any]: requires ``c7_inference``; OR if the configured C3 matcher strategy's :data:`C3_MATCHER_BUILD_FLAGS` flag is OFF (or the strategy is unknown), or if the LightGlue engine load - fails. The message names the consuming component slug(s) - and the relevant gating flag(s). + fails; OR (AZ-623) if ``config.runtime.camera_calibration_path`` + is empty / unreadable / malformed JSON, blocking the + ``c5_imu_preintegrator`` build. The message names the + consuming component slug(s) and the relevant gating flag(s) + or missing inputs. """ constructed: dict[str, Any] = {} constructed["c13_fdr"] = make_fdr_client(AIRBORNE_MAIN_PRODUCER_ID, config) @@ -807,6 +1011,10 @@ def build_pre_constructed(config: Config) -> dict[str, Any]: config, inference_runtime=constructed["c7_inference"] ) constructed["c3_feature_extractor"] = _build_c3_feature_extractor(config) + constructed["c282_ransac_filter"] = _build_c282_ransac_filter(config) + constructed["c5_imu_preintegrator"] = _build_c5_imu_preintegrator(config) + constructed["c5_se3_utils"] = _build_c5_se3_utils(config) + constructed["c5_wgs_converter"] = _build_c5_wgs_converter(config) return constructed @@ -851,8 +1059,7 @@ def register_airborne_strategies() -> None: "kv": { "slots": [slug for slug, *_ in _AIRBORNE_REGISTRATIONS], "total_registrations": sum( - len(strategies) - for _, strategies, *_ in _AIRBORNE_REGISTRATIONS + len(strategies) for _, strategies, *_ in _AIRBORNE_REGISTRATIONS ), }, }, diff --git a/tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py b/tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py index 14e30dc..3bd50dd 100644 --- a/tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py +++ b/tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py @@ -59,12 +59,8 @@ def _stub_c6_builders(monkeypatch: pytest.MonkeyPatch) -> None: # Config() used below would hit KeyError inside storage_factory's # config.components["c6_tile_cache"] lookup. Sentinel objects are # opaque on purpose — the AZ-619 assertions never inspect them. - monkeypatch.setattr( - airborne_bootstrap, "_build_c6_descriptor_index", lambda _config: object() - ) - monkeypatch.setattr( - airborne_bootstrap, "_build_c6_tile_store", lambda _config: object() - ) + monkeypatch.setattr(airborne_bootstrap, "_build_c6_descriptor_index", lambda _config: object()) + monkeypatch.setattr(airborne_bootstrap, "_build_c6_tile_store", lambda _config: object()) @pytest.fixture(autouse=True) @@ -75,9 +71,7 @@ def _stub_c7_inference_builder(monkeypatch: pytest.MonkeyPatch) -> None: # config.components["c7_inference"] lookup, AND the airborne BUILD_* # flags are typically unset in the test env. The sentinel is opaque # on purpose — AZ-619 assertions never inspect it. - monkeypatch.setattr( - airborne_bootstrap, "_build_c7_inference", lambda _config: object() - ) + monkeypatch.setattr(airborne_bootstrap, "_build_c7_inference", lambda _config: object()) @pytest.fixture(autouse=True) @@ -93,9 +87,22 @@ def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None: "_build_c3_lightglue_runtime", lambda _config, *, inference_runtime: object(), ) - monkeypatch.setattr( - airborne_bootstrap, "_build_c3_feature_extractor", lambda _config: object() - ) + monkeypatch.setattr(airborne_bootstrap, "_build_c3_feature_extractor", lambda _config: object()) + + +@pytest.fixture(autouse=True) +def _stub_c5_builders(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange: stub the AZ-623 Phase E c5 / RANSAC builders so the AZ-619 + # tests stay focused on Phase A. Without this the bare Config() below + # would hit _build_c5_imu_preintegrator's + # config.runtime.camera_calibration_path empty-check and raise + # AirborneBootstrapError before the AC-619 keys could be asserted. + # Sentinels are opaque on purpose — AZ-619 assertions never inspect + # them. + monkeypatch.setattr(airborne_bootstrap, "_build_c282_ransac_filter", lambda _config: object()) + monkeypatch.setattr(airborne_bootstrap, "_build_c5_imu_preintegrator", lambda _config: object()) + monkeypatch.setattr(airborne_bootstrap, "_build_c5_se3_utils", lambda _config: object()) + monkeypatch.setattr(airborne_bootstrap, "_build_c5_wgs_converter", lambda _config: object()) def test_ac_619_1_default_config_seeds_c13_fdr_and_clock() -> None: diff --git a/tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py b/tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py index b918dc1..a995389 100644 --- a/tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py +++ b/tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py @@ -95,6 +95,37 @@ def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None: ) +@pytest.fixture(autouse=True) +def _stub_c5_builders(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange: stub the AZ-623 Phase E c5 / RANSAC builders so AZ-620 + # tests stay focused on the Phase B contract. Without this the configs + # used below would hit _build_c5_imu_preintegrator's + # config.runtime.camera_calibration_path empty-check and raise + # AirborneBootstrapError before the AC-620 keys could be asserted. + # MagicMock sentinels are opaque on purpose — AZ-620 assertions + # never inspect them. + monkeypatch.setattr( + airborne_bootstrap, + "_build_c282_ransac_filter", + lambda _config: MagicMock(name="RansacFilter"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_imu_preintegrator", + lambda _config: MagicMock(name="ImuPreintegrator"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_se3_utils", + lambda _config: MagicMock(name="Se3Utils"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_wgs_converter", + lambda _config: MagicMock(name="WgsConverter"), + ) + + def test_ac_620_1_adds_c6_descriptor_index_and_c6_tile_store( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py b/tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py index 2aae8ec..9700c26 100644 --- a/tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py +++ b/tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py @@ -98,6 +98,37 @@ def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None: ) +@pytest.fixture(autouse=True) +def _stub_c5_builders(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange: stub the AZ-623 Phase E c5 / RANSAC builders so AZ-621 + # tests stay focused on the Phase C contract. Without this the configs + # used below would hit _build_c5_imu_preintegrator's + # config.runtime.camera_calibration_path empty-check and raise + # AirborneBootstrapError before the AC-621 keys could be asserted. + # Sentinels are opaque on purpose — AZ-621 assertions never inspect + # them. + monkeypatch.setattr( + airborne_bootstrap, + "_build_c282_ransac_filter", + lambda _config: MagicMock(name="RansacFilter"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_imu_preintegrator", + lambda _config: MagicMock(name="ImuPreintegrator"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_se3_utils", + lambda _config: MagicMock(name="Se3Utils"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_wgs_converter", + lambda _config: MagicMock(name="WgsConverter"), + ) + + def test_ac_621_1_adds_c7_inference(monkeypatch: pytest.MonkeyPatch) -> None: # Arrange: stub build_inference_runtime to a sentinel so we can # assert wiring without standing up real GPU/TensorRT/PyTorch. @@ -133,9 +164,7 @@ def test_ac_621_2_both_build_flags_off_with_configured_consumer_raises_named_err "build_inference_runtime", _raise_no_c7_runtime_available, ) - config = Config.with_blocks( - c3_matcher=_C3MatcherBlock(strategy="disk_lightglue") - ) + config = Config.with_blocks(c3_matcher=_C3MatcherBlock(strategy="disk_lightglue")) # Act + Assert with pytest.raises(AirborneBootstrapError) as excinfo: diff --git a/tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py b/tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py index a5de7c5..697d317 100644 --- a/tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py +++ b/tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py @@ -104,6 +104,35 @@ def _stub_c6_and_c7_builders(monkeypatch: pytest.MonkeyPatch) -> None: ) +@pytest.fixture(autouse=True) +def _stub_c5_builders(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange: stub the AZ-623 Phase E c5 / RANSAC builders so AZ-622 + # tests stay focused on the Phase D contract. Without this the configs + # used below would hit _build_c5_imu_preintegrator's + # config.runtime.camera_calibration_path empty-check and raise + # AirborneBootstrapError before the AC-622 keys could be asserted. + monkeypatch.setattr( + airborne_bootstrap, + "_build_c282_ransac_filter", + lambda _config: MagicMock(name="RansacFilter"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_imu_preintegrator", + lambda _config: MagicMock(name="ImuPreintegrator"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_se3_utils", + lambda _config: MagicMock(name="Se3Utils"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c5_wgs_converter", + lambda _config: MagicMock(name="WgsConverter"), + ) + + def test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -130,10 +159,7 @@ def test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor( assert "c3_lightglue_runtime" in pre_constructed assert "c3_feature_extractor" in pre_constructed assert isinstance(pre_constructed["c3_lightglue_runtime"], LightGlueRuntime) - assert ( - pre_constructed["c3_lightglue_runtime"].descriptor_dim() - == engine_handle.descriptor_dim - ) + assert pre_constructed["c3_lightglue_runtime"].descriptor_dim() == engine_handle.descriptor_dim assert isinstance(pre_constructed["c3_feature_extractor"], FeatureExtractor) assert isinstance(pre_constructed["c3_feature_extractor"], OpenCvOrbExtractor) assert { @@ -154,9 +180,7 @@ def test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error( # fire BEFORE _load_lightglue_engine_handle is consulted, so we don't # need to stub the loader for this branch. monkeypatch.delenv("BUILD_MATCHER_DISK_LIGHTGLUE", raising=False) - config = Config.with_blocks( - c3_matcher=_C3MatcherBlock(strategy="disk_lightglue") - ) + config = Config.with_blocks(c3_matcher=_C3MatcherBlock(strategy="disk_lightglue")) # Act + Assert with pytest.raises(AirborneBootstrapError) as excinfo: @@ -168,9 +192,7 @@ def test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error( # AC-622.2 + AZ-618 NFR "operator-facing error contract"). assert "c3_lightglue_runtime" in message expected_flag = C3_MATCHER_BUILD_FLAGS["disk_lightglue"] - assert expected_flag in message, ( - f"{expected_flag!r} missing from error: {message!r}" - ) + assert expected_flag in message, f"{expected_flag!r} missing from error: {message!r}" assert "c3_matcher" in message # The flag-OFF branch raises directly — there is no upstream cause to # preserve (cause-chain preservation is exercised in @@ -184,9 +206,7 @@ def test_ac_622_2_build_flag_off_with_aliked_strategy_names_aliked_flag( """Per-strategy flag specificity: aliked_lightglue surfaces ALIKED's flag.""" # Arrange: ALIKED strategy with its own gating flag OFF. monkeypatch.delenv("BUILD_MATCHER_ALIKED_LIGHTGLUE", raising=False) - config = Config.with_blocks( - c3_matcher=_C3MatcherBlock(strategy="aliked_lightglue") - ) + config = Config.with_blocks(c3_matcher=_C3MatcherBlock(strategy="aliked_lightglue")) # Act + Assert with pytest.raises(AirborneBootstrapError) as excinfo: @@ -245,9 +265,7 @@ def test_ac_622_2_lightglue_engine_load_failure_wraps_runtime_error( "_load_lightglue_engine_handle", _raise_engine_load_failure, ) - config = Config.with_blocks( - c3_matcher=_C3MatcherBlock(strategy="disk_lightglue") - ) + config = Config.with_blocks(c3_matcher=_C3MatcherBlock(strategy="disk_lightglue")) # Act + Assert with pytest.raises(AirborneBootstrapError) as excinfo: @@ -281,9 +299,7 @@ def test_lightglue_runtime_uses_c7_inference_from_pre_constructed( captured["inference_runtime"] = inference_runtime return _make_engine_handle_mock(descriptor_dim=128) - monkeypatch.setattr( - airborne_bootstrap, "_load_lightglue_engine_handle", _capture_loader - ) + monkeypatch.setattr(airborne_bootstrap, "_load_lightglue_engine_handle", _capture_loader) config = Config() # Act diff --git a/tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py b/tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py new file mode 100644 index 0000000..aa32034 --- /dev/null +++ b/tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py @@ -0,0 +1,301 @@ +"""AZ-623 — Phase E of AZ-618: ``build_pre_constructed`` seeds c282_ransac_filter + 3 c5 helpers. + +Verifies the contract at +``_docs/02_tasks/todo/AZ-623_pre_constructed_phase_e_ransac_c5_helpers.md`` +(scope-narrowed 2026-05-19; ``c5_isam2_graph_handle`` deferred to AZ-625): + +* AC-623.1: ``build_pre_constructed(default_config)`` adds + ``c282_ransac_filter`` (a :class:`RansacFilter` instance), + ``c5_imu_preintegrator`` (an :class:`ImuPreintegrator` instance), + ``c5_se3_utils`` (the :mod:`gps_denied_onboard.helpers.se3_utils` + module), and ``c5_wgs_converter`` (a :class:`WgsConverter` instance) + on top of AZ-619..AZ-622. +* AC-623.2: invoking twice in the same process produces dicts where + ``c5_imu_preintegrator`` is the SAME instance both calls (cached by + ``config.runtime.camera_calibration_path``); the 3 stateless helpers + may be either fresh or cached. +* AC-623.3: when ``config.runtime.camera_calibration_path`` is empty or + unreadable, ``_build_c5_imu_preintegrator`` raises + :class:`AirborneBootstrapError` whose message names the missing input + AND the consuming component slug ``c5_state``. + +AC-623.4 (this file exists with the above tests) is satisfied by the +existence of this module. + +The tests stub the heavy ``_load_camera_calibration`` seam so they +exercise the helper-wiring contract without standing up an on-disk +calibration JSON. The upstream AZ-619..AZ-622 builders are stubbed at +the airborne_bootstrap module boundary, mirroring the prior phase +pattern (see :mod:`tests.unit.runtime_root.test_az622_pre_constructed_phase_d`). +""" + +from __future__ import annotations + +import dataclasses +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import MagicMock + +import numpy as np +import pytest + +from gps_denied_onboard._types.calibration import CameraCalibration +from gps_denied_onboard.config import Config +from gps_denied_onboard.fdr_client import client as fdr_client_module +from gps_denied_onboard.helpers import se3_utils as se3_utils_module +from gps_denied_onboard.helpers.imu_preintegrator import ImuPreintegrator +from gps_denied_onboard.helpers.ransac_filter import RansacFilter +from gps_denied_onboard.helpers.wgs_converter import WgsConverter +from gps_denied_onboard.runtime_root import airborne_bootstrap +from gps_denied_onboard.runtime_root.airborne_bootstrap import ( + AirborneBootstrapError, + build_pre_constructed, + clear_imu_preintegrator_cache, +) + + +def _config_with_calibration_path(path: str) -> Config: + """Return a fresh ``Config`` whose ``runtime.camera_calibration_path`` is set. + + ``RuntimeConfig`` is a frozen dataclass; mutate via + :func:`dataclasses.replace` rather than ``object.__setattr__``. + """ + base = Config() + runtime = dataclasses.replace(base.runtime, camera_calibration_path=path) + return dataclasses.replace(base, runtime=runtime) + + +def _make_calibration(camera_id: str = "az623-test-camera") -> CameraCalibration: + """Sentinel CameraCalibration matching the C5 estimator's expected shape. + + The bias / sample accumulator inside :class:`ImuPreintegrator` only + needs intrinsics-shape inputs to be present; AZ-623 tests do not + integrate samples, so the contents are irrelevant beyond passing + :func:`make_imu_preintegrator`'s own internal validators. + """ + return CameraCalibration( + camera_id=camera_id, + intrinsics_3x3=np.eye(3, dtype=np.float64), + distortion=np.zeros(5, dtype=np.float64), + body_to_camera_se3=np.eye(4, dtype=np.float64), + acquisition_method="operator", + metadata={}, + ) + + +@pytest.fixture(autouse=True) +def _isolated_caches() -> Iterator[None]: + # Arrange: every test starts with empty FdrClient cache + empty + # ImuPreintegrator cache so AC-623.2's "same instance across calls" + # assertion is exercised against fresh state. + fdr_client_module._reset_for_tests() + clear_imu_preintegrator_cache() + yield + fdr_client_module._reset_for_tests() + clear_imu_preintegrator_cache() + + +@pytest.fixture(autouse=True) +def _stub_az619_to_az622_builders(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange: stub the AZ-619 (clock seeded by build_pre_constructed + # directly via WallClock — no builder), AZ-620 (Phase B) C6 builders, + # AZ-621 (Phase C) C7 inference builder, and AZ-622 (Phase D) C3 + # builders so AZ-623 stays focused on the Phase E contract. Sentinels + # are opaque on purpose — AZ-623 assertions never inspect them. + monkeypatch.setattr( + airborne_bootstrap, + "_build_c6_descriptor_index", + lambda _config: MagicMock(name="DescriptorIndex"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c6_tile_store", + lambda _config: MagicMock(name="TileStore"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c7_inference", + lambda _config: MagicMock(name="InferenceRuntime"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c3_lightglue_runtime", + lambda _config, *, inference_runtime: MagicMock(name="LightGlueRuntime"), + ) + monkeypatch.setattr( + airborne_bootstrap, + "_build_c3_feature_extractor", + lambda _config: MagicMock(name="FeatureExtractor"), + ) + + +def test_ac_623_1_adds_c282_ransac_and_c5_helpers( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Arrange: a Config with a populated camera_calibration_path so + # _build_c5_imu_preintegrator's empty-check passes. Stub + # _load_camera_calibration to return a sentinel calibration so the + # bootstrap does not touch the filesystem. + config = _config_with_calibration_path("/tmp/az623-fixture-calib.json") + monkeypatch.setattr( + airborne_bootstrap, + "_load_camera_calibration", + lambda _config: _make_calibration(), + ) + + # Act + pre_constructed = build_pre_constructed(config) + + # Assert: each new key is present and typed per AC-623.1. + assert "c282_ransac_filter" in pre_constructed + assert isinstance(pre_constructed["c282_ransac_filter"], RansacFilter) + + assert "c5_imu_preintegrator" in pre_constructed + assert isinstance(pre_constructed["c5_imu_preintegrator"], ImuPreintegrator) + + assert "c5_se3_utils" in pre_constructed + se3_utils = pre_constructed["c5_se3_utils"] + # The se3_utils handle is the helpers.se3_utils module — a module is + # the simplest namespace object that exposes exp_map, log_map etc. + # as attributes (matches the MagicMock fixture pattern existing C5 + # tests already use). + assert se3_utils is se3_utils_module + for func_name in ("exp_map", "log_map", "matrix_to_se3", "se3_to_matrix"): + assert hasattr(se3_utils, func_name), ( + f"c5_se3_utils handle is missing {func_name!r}; " + f"consumers (build_state_estimator, build_pose_estimator) " + f"dispatch via attribute access." + ) + + assert "c5_wgs_converter" in pre_constructed + assert isinstance(pre_constructed["c5_wgs_converter"], WgsConverter) + + +def test_ac_623_1_keeps_existing_keys_intact(monkeypatch: pytest.MonkeyPatch) -> None: + # Arrange + config = _config_with_calibration_path("/tmp/az623-fixture-calib.json") + monkeypatch.setattr( + airborne_bootstrap, + "_load_camera_calibration", + lambda _config: _make_calibration(), + ) + + # Act + pre_constructed = build_pre_constructed(config) + + # Assert: AZ-619..AZ-622 keys remain populated; AZ-623 is additive. + assert { + "c13_fdr", + "clock", + "c6_descriptor_index", + "c6_tile_store", + "c7_inference", + "c3_lightglue_runtime", + "c3_feature_extractor", + }.issubset(pre_constructed.keys()), ( + f"AZ-623 must be additive on top of AZ-619..AZ-622; got " + f"keys: {sorted(pre_constructed.keys())}" + ) + + +def test_ac_623_2_imu_preintegrator_cached_across_calls( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Arrange: stub the calibration loader with a counter so we can + # assert the cache short-circuit fires on the second call. + config = _config_with_calibration_path("/tmp/az623-cache-fixture.json") + load_count = {"n": 0} + + def _counting_load(_config: Config) -> CameraCalibration: + load_count["n"] += 1 + return _make_calibration() + + monkeypatch.setattr(airborne_bootstrap, "_load_camera_calibration", _counting_load) + + # Act + first = build_pre_constructed(config) + second = build_pre_constructed(config) + + # Assert: cache holds the same ImuPreintegrator instance; the + # calibration loader fires exactly once across the two calls. + assert first["c5_imu_preintegrator"] is second["c5_imu_preintegrator"], ( + "AC-623.2 requires c5_imu_preintegrator to be the same instance " + "across two build_pre_constructed calls (cache keyed on " + "camera_calibration_path)" + ) + assert load_count["n"] == 1, ( + f"calibration loader should fire once (cached on second call); " + f"got {load_count['n']} invocations" + ) + + +def test_ac_623_2_imu_preintegrator_per_path_cache( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Arrange: two configs with DIFFERENT calibration paths. The cache + # must produce DIFFERENT preintegrator instances (one per path). + config_a = _config_with_calibration_path("/tmp/az623-path-a.json") + config_b = _config_with_calibration_path("/tmp/az623-path-b.json") + monkeypatch.setattr( + airborne_bootstrap, + "_load_camera_calibration", + lambda _config: _make_calibration(), + ) + + # Act + a = build_pre_constructed(config_a) + b = build_pre_constructed(config_b) + + # Assert + assert a["c5_imu_preintegrator"] is not b["c5_imu_preintegrator"], ( + "Two distinct calibration paths must produce two distinct " + "ImuPreintegrator instances; the cache key is the path." + ) + + +def test_ac_623_3_empty_calibration_path_raises_named_error() -> None: + # Arrange: default Config() has camera_calibration_path="". + config = Config() + + # Act + Assert + with pytest.raises(AirborneBootstrapError) as exc_info: + build_pre_constructed(config) + msg = str(exc_info.value) + assert "c5_imu_preintegrator" in msg, msg + assert "camera_calibration_path" in msg, msg + assert "c5_state" in msg, msg + + +def test_ac_623_3_unreadable_calibration_path_raises_named_error( + tmp_path: Path, +) -> None: + # Arrange: pass a path that does not exist on disk; the OSError + # surfaces as AirborneBootstrapError with the operator-actionable + # message. + missing = tmp_path / "this-file-does-not-exist.json" + config = _config_with_calibration_path(str(missing)) + + # Act + Assert + with pytest.raises(AirborneBootstrapError) as exc_info: + build_pre_constructed(config) + msg = str(exc_info.value) + assert "c5_imu_preintegrator" in msg + assert "c5_state" in msg + assert str(missing) in msg + + +def test_ac_623_3_malformed_json_calibration_raises_named_error( + tmp_path: Path, +) -> None: + # Arrange + bad = tmp_path / "bad-calib.json" + bad.write_text("{ this is not valid json", encoding="utf-8") + config = _config_with_calibration_path(str(bad)) + + # Act + Assert + with pytest.raises(AirborneBootstrapError) as exc_info: + build_pre_constructed(config) + msg = str(exc_info.value) + assert "c5_imu_preintegrator" in msg + assert "c5_state" in msg + assert "not valid JSON" in msg