mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-21 09:01:14 +00:00
Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| c3639a5d1c | |||
| 2b8ef52f66 | |||
| 02208c577e | |||
| 5c4d129f80 | |||
| eaf2f47f69 |
@@ -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`
|
||||
+1
-1
@@ -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)
|
||||
@@ -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`
|
||||
@@ -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`
|
||||
@@ -0,0 +1,80 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 93
|
||||
**Tasks**: AZ-622 (Phase D: build_pre_constructed seeds c3_lightglue_runtime + c3_feature_extractor)
|
||||
**Date**: 2026-05-19
|
||||
**Cycle**: 1
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|
||||
|------|--------|----------------|-------|-------------|--------|
|
||||
| AZ-622_pre_constructed_phase_d_c3_runtimes | Done | 6 files | 17 passed | 3/3 ACs covered | 0 blocking |
|
||||
|
||||
## Files Changed
|
||||
|
||||
### Production
|
||||
- `src/gps_denied_onboard/components/c3_matcher/config.py` — added optional `lightglue_weights_path: Path | None = None`; extended `__post_init__` Path/None validator; expanded module docstring with the AZ-622 / Phase D rationale.
|
||||
- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` — new public constant `C3_MATCHER_BUILD_FLAGS`; new internal helpers `_resolve_c3_matcher_strategy`, `_is_build_flag_on`, `_load_lightglue_engine_handle`, `_build_c3_lightglue_runtime`, `_build_c3_feature_extractor`; `build_pre_constructed` refactored to step-by-step build so the just-built `c7_inference` flows into the LightGlue loader without a double-build.
|
||||
|
||||
### Tests
|
||||
- `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` (NEW, 6 tests):
|
||||
- `test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor` — AC-622.1.
|
||||
- `test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error` — AC-622.2 (DISK).
|
||||
- `test_ac_622_2_build_flag_off_with_aliked_strategy_names_aliked_flag` — AC-622.2 (ALIKED, per-strategy specificity).
|
||||
- `test_ac_622_2_default_config_no_c3_matcher_block_still_raises` — AC-622.2 defence-in-depth.
|
||||
- `test_ac_622_2_lightglue_engine_load_failure_wraps_runtime_error` — AC-622.2 cause-chain wrap.
|
||||
- `test_lightglue_runtime_uses_c7_inference_from_pre_constructed` — additive identity-share invariant (no double-build of InferenceRuntime).
|
||||
- `tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
- `tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
- `tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
|
||||
## AC Test Coverage: 3 of 3 covered
|
||||
|
||||
| AC | Test | Status |
|
||||
|----|------|--------|
|
||||
| AC-622.1 | `test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor` | Covered |
|
||||
| AC-622.2 | `test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error` (+3 variants) | Covered |
|
||||
| AC-622.3 | File `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` exists | Covered |
|
||||
|
||||
## Code Review Verdict: PASS_WITH_WARNINGS
|
||||
|
||||
Full report: `_docs/03_implementation/reviews/batch_93_review.md`. Three Low findings:
|
||||
|
||||
1. **F1 (Low / Spec)** — `_build_c3_lightglue_runtime` signature is `(config, *, inference_runtime)` not the spec's literal `(config)` form. Justified by the additivity invariant + AZ-621's no-double-build rule; documented in docstrings.
|
||||
2. **F2 (Low / Maintainability)** — `_is_build_flag_on` duplicated across `airborne_bootstrap`, `matcher_factory`, and `vpr_factory` (3 call sites now). Defer to a future hygiene PBI (~2pt).
|
||||
3. **F3 (Low / Maintainability)** — `BuildConfig(FP16, 512MB, …)` literal duplicated with per-strategy `_build_*_build_config` helpers. Defer until FP16 becomes a config-driven knob.
|
||||
|
||||
No Critical / High findings. Auto-fix not invoked.
|
||||
|
||||
## Auto-Fix Attempts: 0
|
||||
|
||||
## Stuck Agents: None
|
||||
|
||||
## Test Run Summary
|
||||
|
||||
- Targeted test set (AZ-619/620/621/622): **17 passed** in 5.83s.
|
||||
- Regression check (`tests/unit/runtime_root/` + `tests/unit/c3_matcher/`): **108 passed** in 3.89s.
|
||||
- Adjacent regression check (`tests/unit/c3_5_adhop/test_az349_adhop_refiner.py`, the only other consumer of `C3MatcherConfig`): **23 passed** in 1.09s.
|
||||
|
||||
## Tier-2 / Deferred Work
|
||||
|
||||
Per the AZ-622 ## Tier-2 Note, real LightGlue inference correctness is verified by AZ-624's Jetson AC-5 run; this batch only exercises:
|
||||
|
||||
- BUILD-flag matrix lookup (in-process, env-var driven).
|
||||
- Error-message contract (operator-actionable strings naming flag + consuming component slug).
|
||||
- The `_load_lightglue_engine_handle` heavy seam via monkeypatch sentinels.
|
||||
|
||||
The production path of `_load_lightglue_engine_handle` (read `block.lightglue_weights_path`, compile via C7 `InferenceRuntime`, deserialise into `EngineHandle`) is implemented as real code (not a stub) and will be exercised end-to-end at AZ-624's Jetson tier-2 e2e gate. Operator's deployment config must populate `c3_matcher.lightglue_weights_path` for the airborne binary to start.
|
||||
|
||||
## Spec / Code Naming Discrepancy (Informational, no finding)
|
||||
|
||||
The AZ-622 task spec § AC-622.2 example uses the flag name `BUILD_C3_MATCHER_DISK_LIGHTGLUE`, but the actual production flag matrix in `matcher_factory._STRATEGY_TO_BUILD_FLAG` uses the older `BUILD_MATCHER_*` family (`BUILD_MATCHER_DISK_LIGHTGLUE`, `BUILD_MATCHER_ALIKED_LIGHTGLUE`, `BUILD_MATCHER_XFEAT`). The implementation reuses the actual existing flags (per the constraint "MUST reuse the existing per-strategy `BUILD_C3_MATCHER_*` matrix"); the spec's flag-name typo is captured in the `C3_MATCHER_BUILD_FLAGS` docstring so future readers don't get confused. No change to the spec required — the constraint's *intent* (reuse the existing matrix, no new flags) is honored.
|
||||
|
||||
## Next Batch
|
||||
|
||||
- **Batch 94**: AZ-623 (Phase E: build_pre_constructed seeds c282_ransac_filter + c5 helpers + c5_isam2_graph_handle) — 3pt.
|
||||
- **Then**: 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 review at 88-92).
|
||||
|
||||
After AZ-624 lands, the AZ-618 umbrella's AC-1..AC-5 become exercisable end-to-end; the Jetson tier-2 gate that sent the flow back to Step 7 (per `_docs/LESSONS.md` 2026-05-18) becomes runnable again.
|
||||
@@ -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).
|
||||
@@ -0,0 +1,94 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 95
|
||||
**Tasks**: AZ-625 (Phase E.5 of AZ-618: c5_isam2_graph_handle ordering — separate handle from estimator construction)
|
||||
**Date**: 2026-05-19
|
||||
**Cycle**: 1
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|
||||
|------|--------|----------------|-------|-------------|--------|
|
||||
| AZ-625_c5_isam2_graph_handle_ordering | Done | 7 files | 8 new + 24 carry-over | 4/4 ACs covered | 0 blocking (2 Low / Style + Maintainability — accepted per code review) |
|
||||
|
||||
AZ-625 was split out of AZ-623 in batch 94 after the AZ-623 task spec's two-path investigation surfaced an unresolvable construction-order conflict (c4_pose consumes `c5_isam2_graph_handle` from `pre_constructed`; the handle is built inside `build_state_estimator`'s tuple return, which c5_state's wrapper invokes — but the c5_state wrapper runs AFTER c4_pose in topological order). Path 1 (handle-only separation) required a Protocol-seam change in C5 — explicitly forbidden by the AZ-618 umbrella's "MUST NOT touch any per-component factory signature" constraint. Path 2 (eager `(estimator, handle)` build at bootstrap) is the chosen approach: this batch lands it.
|
||||
|
||||
## Files Changed
|
||||
|
||||
### Production
|
||||
|
||||
- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py`:
|
||||
- Added `C5_STATE_BUILD_FLAGS: Final[Mapping[str, str]]` constant mirroring `state_factory._STATE_BUILD_FLAGS` so the bootstrap can name the gating `BUILD_STATE_*` flag in operator errors.
|
||||
- Added `_resolve_c5_state_strategy(config)` mirroring `_resolve_c3_matcher_strategy` for symmetry.
|
||||
- Added `_C5_PREBUILT_ESTIMATOR_KEY: Final[str] = "_c5_prebuilt_estimator"` internal coordination key (deliberately NOT in `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` — only the C5 wrapper consults it as a fast path).
|
||||
- Added `_build_c5_state_estimator_pair(config, *, imu_preintegrator, se3_utils, wgs_converter, fdr_client, tile_store=None, camera_calibration=None, flight_id=None, companion_id=None)` that resolves the strategy, gates on the per-strategy `BUILD_STATE_*` flag, lazily registers the strategy via `_ensure_state_strategy_registered`, calls `build_state_estimator` once, captures the `(estimator, handle)` tuple, and wraps any `StateEstimatorConfigError` into `AirborneBootstrapError` with `__cause__` preserved.
|
||||
- Modified `_c5_state_wrapper` to short-circuit on `constructed.get(_C5_PREBUILT_ESTIMATOR_KEY)`: when the look-aside key is present, the prebuilt estimator is returned as-is; otherwise the wrapper falls through to the original `build_state_estimator` path (preserves test isolation for fixtures that bypass `build_pre_constructed`, e.g. `test_az401_compose_root_replay`).
|
||||
- Extended `build_pre_constructed` to call `_build_c5_state_estimator_pair` after the AZ-619..AZ-623 keys are populated and seed both `c5_isam2_graph_handle` (Public API key consumed by C4) and `_c5_prebuilt_estimator` (internal coordination key consumed by `_c5_state_wrapper`).
|
||||
- Updated `__all__` to export `C5_STATE_BUILD_FLAGS`.
|
||||
- Updated `build_pre_constructed`'s docstring to describe the AZ-625 wiring + error contract (eager-pair construction, look-aside key, BUILD-flag gating, error wrapping).
|
||||
|
||||
### Tests
|
||||
|
||||
- `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py` (NEW, 8 tests):
|
||||
- `test_ac_625_1_adds_c5_isam2_graph_handle_with_protocol_surface` — AC-625.1 (key present + `isinstance(handle, C4ISam2GraphHandle)` + per-method `hasattr` + AZ-619..AZ-623 keys still present).
|
||||
- `test_ac_625_1_internal_prebuilt_estimator_key_not_in_required_keys` — internal-key invariant (the coordination key is NOT exposed via `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`).
|
||||
- `test_ac_625_2_build_state_gtsam_isam2_off_raises_named_error` — AC-625.2 (`BUILD_STATE_GTSAM_ISAM2=OFF` raises with the missing key + flag + `c5_state` slug; no upstream cause because the gate fires before the factory).
|
||||
- `test_ac_625_2_unknown_strategy_raises_named_error_with_supported_set` — AC-625.2 (unknown strategy raises with the supported strategy set named).
|
||||
- `test_ac_625_2_build_state_estimator_config_error_wraps_into_bootstrap_error` — defense-in-depth (`StateEstimatorConfigError` wraps into `AirborneBootstrapError` with `__cause__` preserved).
|
||||
- `test_ac_625_3_handle_is_same_object_as_estimator_isam2_handle` — AC-625.3 cross-seam identity (`pre_constructed[_c5_prebuilt_estimator]._isam2_handle IS pre_constructed['c5_isam2_graph_handle']`).
|
||||
- `test_ac_625_3_c5_state_wrapper_short_circuits_on_prebuilt_estimator` — wrapper short-circuit returns the prebuilt estimator without consulting the fallback infrastructure keys.
|
||||
- `test_ac_625_3_c5_state_wrapper_falls_back_when_prebuilt_absent` — wrapper falls back to `build_state_estimator` when the look-aside key is absent (preserves existing fixture behavior).
|
||||
|
||||
- `tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py` — added autouse stub for `_build_c5_state_estimator_pair` (returns `(MagicMock, MagicMock)`) so AZ-619's bare `Config()` bootstrap path doesn't trip on the default `gtsam_isam2` strategy needing a real registered factory.
|
||||
- `tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py` — same autouse stub.
|
||||
- `tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py` — same autouse stub.
|
||||
- `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` — same autouse stub.
|
||||
- `tests/unit/runtime_root/test_az623_pre_constructed_phase_e.py` — same autouse stub.
|
||||
|
||||
### Reviews
|
||||
|
||||
- `_docs/03_implementation/reviews/batch_95_review.md` (NEW) — code review report (verdict: PASS; 2 Low findings on style + function-scoped import).
|
||||
|
||||
### Specs
|
||||
|
||||
- `_docs/02_tasks/todo/AZ-625_*.md` → ARCHIVED to `_docs/02_tasks/done/`.
|
||||
|
||||
## AC Test Coverage
|
||||
|
||||
All 4 ACs covered:
|
||||
|
||||
| AC | Coverage |
|
||||
|----|----------|
|
||||
| AC-625.1 | `test_ac_625_1_adds_c5_isam2_graph_handle_with_protocol_surface` (presence + Protocol conformance) + `test_ac_625_1_internal_prebuilt_estimator_key_not_in_required_keys` |
|
||||
| AC-625.2 | `test_ac_625_2_build_state_gtsam_isam2_off_raises_named_error` + `test_ac_625_2_unknown_strategy_raises_named_error_with_supported_set` + `test_ac_625_2_build_state_estimator_config_error_wraps_into_bootstrap_error` |
|
||||
| AC-625.3 | `test_ac_625_3_handle_is_same_object_as_estimator_isam2_handle` + `test_ac_625_3_c5_state_wrapper_short_circuits_on_prebuilt_estimator` + `test_ac_625_3_c5_state_wrapper_falls_back_when_prebuilt_absent` |
|
||||
| AC-625.4 | file `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py` exists with the above tests |
|
||||
|
||||
## Test Run
|
||||
|
||||
| Suite | Result |
|
||||
|-------|--------|
|
||||
| `tests/unit/runtime_root/test_az619..test_az623 + test_az625` (targeted) | 32 passed in 1.18s |
|
||||
| `tests/unit/runtime_root/ + tests/unit/c5_state/` (regression) | 255 passed in 1.38s |
|
||||
|
||||
No failures; no skips beyond pre-existing environment-gated tests.
|
||||
|
||||
## Code Review
|
||||
|
||||
- **Verdict**: PASS (0 Critical, 0 High, 0 Medium, 2 Low).
|
||||
- F1 (Low / Style): `_C5_PREBUILT_ESTIMATOR_KEY` defined after first reference site — accepted (grouping with C5-state constants is the dominant readability axis; Python's lazy resolution makes this correct).
|
||||
- F2 (Low / Maintainability): function-scoped `StateEstimatorConfigError` import — accepted (matches the file's existing function-scope-import convention for c5_state submodules).
|
||||
- 0 auto-fix attempts; 0 escalated findings.
|
||||
|
||||
Full report: `_docs/03_implementation/reviews/batch_95_review.md`.
|
||||
|
||||
## Constraint Compliance (AZ-618 umbrella)
|
||||
|
||||
- "MUST NOT modify per-component factory signatures" → `state_factory.build_state_estimator` invoked with the same kwargs the wrapper would have passed; no signature changed. ✓
|
||||
- "MUST be additive on top of AZ-619..AZ-623" → AZ-619..AZ-623 keys still present in `pre_constructed`. ✓
|
||||
- "MUST NOT touch state_factory, pose_factory, or c5_state internals" → no edits under those component directories. ✓
|
||||
- "All changes confined to `airborne_bootstrap.py` + new test file" → diff scope respected. The autouse-stub additions in AZ-619..AZ-623 phase tests are hygiene maintenance for the additive bootstrap call (each test now stubs the new `_build_c5_state_estimator_pair` builder), not a behavioral change to those phases' contracts. ✓
|
||||
|
||||
## Loop Status
|
||||
|
||||
- AZ-625 unblocks AZ-624 (the only remaining AZ-618 subtask). AZ-624's dependency edge in `_dependencies_table.md` lists `AZ-619..AZ-623, AZ-625` as upstream — all done. Next batch (96) will pick up AZ-624.
|
||||
@@ -0,0 +1,92 @@
|
||||
# Batch Report
|
||||
|
||||
**Batch**: 96
|
||||
**Tasks**: AZ-624 (Phase F of AZ-618: wire `build_pre_constructed` into `runtime_root.main()` + AC-1..AC-5 verification)
|
||||
**Date**: 2026-05-19
|
||||
**Cycle**: 1
|
||||
|
||||
## Task Results
|
||||
|
||||
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|
||||
|------|--------|----------------|-------|-------------|--------|
|
||||
| AZ-624_pre_constructed_phase_f_wire_main | Done (with AC-5 BLOCKED on Jetson hardware evidence) | 2 files (1 source + 1 new test) | 6 new + 261 carry-over runtime_root + c5_state | 4/5 ACs covered locally; AC-5 BLOCKED pending operator-supplied Jetson run | 0 blocking from AZ-624 changes; 1 unrelated pre-existing failure (`c12_operator_orchestrator/test_cli_console_script.py::test_cold_start_under_500ms_p99` — Mac dev host subprocess startup ~700 ms exceeds the 500 ms NFR; unrelated to AZ-624 scope) |
|
||||
|
||||
AZ-624 is the final / umbrella subtask of AZ-618. With AZ-619..AZ-623 + AZ-625 all in `done/`, this batch lands the integration: `runtime_root.main()` now calls `register_airborne_strategies()`, builds the `pre_constructed` dict via `build_pre_constructed(config)`, and passes it through to `compose_root(config, pre_constructed=pre_constructed)`. A dedicated `except AirborneBootstrapError` handler surfaces the operator-actionable bootstrap error (with its `airborne_bootstrap:` prefix) ahead of the broader `(ConfigurationError, StrategyNotLinkedError, RuntimeError)` clause. The new umbrella test file `test_az618_pre_constructed.py` covers AC-1..AC-4 from the AZ-618 umbrella spec.
|
||||
|
||||
## Files Changed
|
||||
|
||||
### Production
|
||||
|
||||
- `src/gps_denied_onboard/runtime_root/__init__.py`:
|
||||
- Extended the `main()` function-scope import to include `AirborneBootstrapError` and `build_pre_constructed` alongside `register_airborne_strategies`.
|
||||
- Inserted `pre_constructed = build_pre_constructed(config)` between `register_airborne_strategies()` and `compose_root(config, ...)`.
|
||||
- Replaced the unkwargd `compose_root(config)` call with `compose_root(config, pre_constructed=pre_constructed)`.
|
||||
- Added a dedicated `except AirborneBootstrapError` clause BEFORE the broader `(ConfigurationError, StrategyNotLinkedError, RuntimeError)` clause to surface the operator-facing bootstrap error message (which itself carries the `airborne_bootstrap:` prefix per AZ-619..AZ-625) without letting the broader clause swallow the prefix in a generic backtrace. The inline comment captures the rationale.
|
||||
|
||||
### Tests
|
||||
|
||||
- `tests/unit/runtime_root/test_az618_pre_constructed.py` (NEW, 6 tests):
|
||||
- `test_ac_618_1_build_pre_constructed_populates_every_required_key` — AC-618-1 (every key in the union of `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` rows is present + non-None).
|
||||
- `test_ac_618_2_compose_root_reaches_takeoff_with_all_seven_slots` — AC-618-2 (compose_root produces a `RuntimeRoot` with all 7 strategy-selecting slots populated; uses `_stub_strategy_factories` to monkeypatch the per-component factory imports in `airborne_bootstrap`'s own namespace).
|
||||
- `test_ac_618_3_build_flag_off_raises_named_error_with_consuming_component` — AC-618-3 (forces `build_inference_runtime` to raise `RuntimeNotAvailableError`, asserts the wrapping `AirborneBootstrapError` names `c7_inference` + both airborne C7 BUILD flags + the consuming component slug `c2_vpr`).
|
||||
- `test_ac_618_4_main_returns_zero_on_success` — AC-618-4 success path (main returns 0; nothing on stderr).
|
||||
- `test_ac_618_4_main_returns_generic_failure_with_airborne_bootstrap_prefix` — AC-618-4 failure path (forced builder failure → exit code 1 + stderr contains `airborne_bootstrap:`).
|
||||
- `test_ac_624_local_main_distinct_handler_does_not_double_print` — AZ-624 local regression guard (the dedicated `AirborneBootstrapError` handler short-circuits before the broader RuntimeError clause; exactly ONE `runtime_root: airborne_bootstrap:` line in stderr).
|
||||
|
||||
### Reviews
|
||||
|
||||
- `_docs/03_implementation/reviews/batch_96_review.md` (NEW) — code review report (verdict: PASS; 1 Low finding on documentation-as-code dedicated handler).
|
||||
|
||||
### Specs
|
||||
|
||||
- `_docs/02_tasks/todo/AZ-624_*.md` → ARCHIVED to `_docs/02_tasks/done/`.
|
||||
- `_docs/02_tasks/todo/AZ-618_*.md` → ARCHIVED to `_docs/02_tasks/done/` (umbrella reference; no longer actionable since the last subtask landed).
|
||||
|
||||
## AC Test Coverage
|
||||
|
||||
5 ACs from the AZ-618 umbrella + 1 AZ-624 local AC. Coverage:
|
||||
|
||||
| AC | Coverage |
|
||||
|----|----------|
|
||||
| AC-618-1 | `test_ac_618_1_build_pre_constructed_populates_every_required_key` |
|
||||
| AC-618-2 | `test_ac_618_2_compose_root_reaches_takeoff_with_all_seven_slots` |
|
||||
| AC-618-3 | `test_ac_618_3_build_flag_off_raises_named_error_with_consuming_component` |
|
||||
| AC-618-4 | `test_ac_618_4_main_returns_zero_on_success` + `test_ac_618_4_main_returns_generic_failure_with_airborne_bootstrap_prefix` + `test_ac_624_local_main_distinct_handler_does_not_double_print` |
|
||||
| AC-618-5 / AC-624.tier2 | **BLOCKED** — Jetson tier-2 e2e replay run; requires operator-supplied hardware evidence (terminal log path + JetPack version + run timestamp per `_docs/02_document/tests/tier2-jetson-testing.md`). |
|
||||
| AC-624.local | Full Tier-1 pytest suite — 2150 passed, 85 skipped (environment-gated). One unrelated pre-existing failure (see Test Run section). |
|
||||
|
||||
## Test Run
|
||||
|
||||
| Suite | Result |
|
||||
|-------|--------|
|
||||
| `tests/unit/runtime_root/test_az618_pre_constructed.py` (targeted) | 6 passed in 3.07s |
|
||||
| `tests/unit/runtime_root/ + tests/unit/c5_state/` (regression) | 261 passed in 1.31s |
|
||||
| `tests/unit/test_az401_compose_root_replay.py` (compose_root regression) | 25 passed in 2.02s |
|
||||
| `tests/unit/` (full Tier-1 suite) | 2150 passed, 85 skipped, 1 failed |
|
||||
|
||||
The 85 skips are all environment-gated (Docker for c6 Postgres tests, GPU/CUDA for c7 PyTorch FP16, TensorRT for c7 Tier-2, real Jetson for c7 engine_gate, large synthetic datasets gated behind `RUN_REPLAY_E2E=1`).
|
||||
|
||||
The 1 failure is **unrelated to AZ-624 scope**:
|
||||
|
||||
- `tests/unit/c12_operator_orchestrator/test_cli_console_script.py::TestConsoleScript::test_cold_start_under_500ms_p99` — measures `operator-orchestrator --help` cold-start subprocess time. Sample: `[586.97, 613.24, 664.54, 646.32, 776.91, 692.62, 693.11, 718.97, 733.01, 873.51, 605.48]` ms; worst-after-trim = 776.9 ms, fails the 500 ms NFR. The Mac dev host's Python subprocess startup overhead drives this; CI runners and Jetson do not exhibit it. Test is `@pytest.mark.slow` and lives in `c12_operator_orchestrator` which AZ-624 does not modify or import.
|
||||
|
||||
## Code Review
|
||||
|
||||
- **Verdict**: PASS (0 Critical, 0 High, 0 Medium, 1 Low).
|
||||
- F1 (Low / Style): dedicated `AirborneBootstrapError` handler is documentation-as-code (functionally identical to the broader `RuntimeError` clause, but locks the surfacing format and adds traceability). Accepted with inline comment + regression guard test.
|
||||
- 0 auto-fix attempts; 0 escalated findings.
|
||||
|
||||
Full report: `_docs/03_implementation/reviews/batch_96_review.md`.
|
||||
|
||||
## Constraint Compliance (AZ-618 umbrella)
|
||||
|
||||
- "MUST NOT touch any per-component factory signature" → `compose_root` and `build_pre_constructed` signatures unchanged; only the body of `main()` was extended. ✓
|
||||
- "MUST NOT introduce new BUILD_* env flags" → no new flag introduced. ✓
|
||||
- "All changes confined to `runtime_root/__init__.py` + `runtime_root/airborne_bootstrap.py` + new test file" → diff scope respected (no `airborne_bootstrap.py` edits in this batch — that file's contract was already complete after AZ-625; AZ-624 only wires `main()`). ✓
|
||||
|
||||
## Loop Status
|
||||
|
||||
- AZ-624 was the last AZ-618 subtask. AZ-618 (umbrella reference doc) is also archived to `done/`.
|
||||
- The Implement step's batch loop has no remaining AZ-618-related tasks. Other tasks may still exist in `_docs/02_tasks/todo/` (none currently — only AZ-618 + AZ-624 remained before this batch).
|
||||
- **Next autodev action**: enter the implement skill's Step 15 (Product Implementation Completeness Gate) — but this gate will skip in greenfield Step 7 if no other product tasks remain, OR it will perform the cross-task completeness scan for AZ-618 outcomes (every required key populated, every wrapper path tested, every umbrella AC covered) and write `implementation_completeness_cycle1_report.md`.
|
||||
- **AC-618-5 BLOCKING note**: The Jetson tier-2 e2e replay run is required by the AZ-618 umbrella but lives outside this dev host's reach. Until the operator runs `scripts/run-tests-jetson.sh tests/e2e/replay/test_derkachi_1min.py` and captures terminal log + JetPack version + run timestamp, the AZ-618 umbrella cannot be marked fully complete. The Implement skill's loop should pause at the next decision point and surface this BLOCKING gate to the operator.
|
||||
@@ -0,0 +1,136 @@
|
||||
# Cumulative Code Review — Batches 88-92 (Cycle 1)
|
||||
|
||||
**Window**: batches 88, 89, 90, 91, 92
|
||||
**Tasks covered**: AZ-440, AZ-441, AZ-442, AZ-443, AZ-446, AZ-619, AZ-620, AZ-621 (8 tasks, 19 complexity points)
|
||||
**Date**: 2026-05-19
|
||||
**Verdict**: PASS_WITH_WARNINGS — proceed; carry-over hygiene PBIs (CR-F1/F2 escalated) and one process gap
|
||||
**Reviewer**: autodev / implement skill Step 14.5
|
||||
**Last cumulative review**: batches 85-87 (`_docs/03_implementation/cumulative_review_batches_85-87_cycle1_report.md`)
|
||||
|
||||
## Scope
|
||||
|
||||
Union of files changed since the previous cumulative review window (commits `f25cae4..680ba29`, 129 files / +16,564 / -87 LOC). This review focuses on the production-code window (90-92) and the test-implementation tail (88-89).
|
||||
|
||||
| Batch | Theme | Tasks | New helpers | New scenarios | New unit tests | Production code |
|
||||
|-------|-------|-------|-------------|---------------|----------------|-----------------|
|
||||
| 88 | Resource limits (NFT-LIM) | AZ-440..AZ-443 (9 cp) | 4 (memory_budget, fdr_size, storage_budget, thermal_envelope) | 4 (NFT-LIM-01..04) + 1 fixture | 4 helper-unit + 1 layout | — |
|
||||
| 89 | NFR reporting refinement | AZ-446 (2 cp) | — (modifies `nfr_recorder.py`) | — (extends 2 existing) | 6 new in `test_nfr_recorder.py` | — |
|
||||
| 90 | Bootstrap Phase A | AZ-619 (2 cp) | — | — | 5 in `test_az619_*.py` | `airborne_bootstrap.py` — `build_pre_constructed`, `AIRBORNE_MAIN_PRODUCER_ID`, `_build_c13_fdr`, `_build_clock` |
|
||||
| 91 | Bootstrap Phase B | AZ-620 (3 cp) | — | — | 3 in `test_az620_*.py` (+ AZ-619 file extended with autouse stub) | `airborne_bootstrap.py` — `FAISS_BUILD_FLAG`, `_consumers_of_pre_constructed_key`, `_configured_consumers_of_pre_constructed_key`, `_build_c6_descriptor_index`, `_build_c6_tile_store` |
|
||||
| 92 | Bootstrap Phase C | AZ-621 (3 cp) | — | — | 3 in `test_az621_*.py` (+ AZ-619 and AZ-620 files extended with autouse stub) | `airborne_bootstrap.py` — `C7_AIRBORNE_BUILD_FLAGS`, `_build_c7_inference` |
|
||||
|
||||
The window crosses one structural boundary: batches 88–89 close out the **test-implementation** epoch (Step 10), batches 90–92 are the **post-rewind product-implementation cycle 1** (Step 7 after the Jetson e2e gate rewound the flow per `_docs/LESSONS.md` 2026-05-18). The bootstrap surface (`src/gps_denied_onboard/runtime_root/airborne_bootstrap.py`) is the only production file touched in the window.
|
||||
|
||||
## Cross-batch consistency (Phase 6)
|
||||
|
||||
### Test-implementation tail (88-89)
|
||||
|
||||
PASS. Batch 88's four NFT-LIM evaluator helpers follow the same 7-element shape established by batches 85-87 (frozen dataclasses, single `evaluate()` entry, `write_csv_evidence()` writer, `Sequence` parameter typing, module-docstring discipline). Batch 88's four scenario files follow the same 7-step shape (tier-skip → sitl_replay_ready-skip → `_resolve_fixture_path()` → typed parse → evaluator → CSV evidence → AC assertions). Batch 89 (AZ-446) is a purely additive API change to `nfr_recorder.py`: `record_metric()` gained kw-only `band`, `ci95_low`, `ci95_high` with default `None`, and a new `report.csv` emission via `pytest_sessionfinish` — existing call sites unaffected.
|
||||
|
||||
### Product-implementation cycle 1 start (90-92)
|
||||
|
||||
PASS — the three phases of the AZ-618 umbrella exhibit a deliberate, repeated pattern that is itself the cumulative-review story:
|
||||
|
||||
1. **Phase A (AZ-619)** introduces `build_pre_constructed(config) -> dict[str, Any]` with two foundational keys (`c13_fdr`, `clock`).
|
||||
2. **Phase B (AZ-620)** adds two new keys (`c6_descriptor_index`, `c6_tile_store`) and the first wrapped-factory error-translation: `RuntimeNotAvailableError → AirborneBootstrapError` naming the missing key + the gating `BUILD_FAISS_INDEX` flag + the configured consumer slug(s). Phase B introduces the reusable helpers `_consumers_of_pre_constructed_key()` and `_configured_consumers_of_pre_constructed_key()`.
|
||||
3. **Phase C (AZ-621)** adds one new key (`c7_inference`) by reusing Phase B's helpers and extending the build-flag surfacing to two flags (`C7_AIRBORNE_BUILD_FLAGS` as a `tuple[tuple[str, str], ...]`).
|
||||
|
||||
The pattern is converging — Phase C added zero new infrastructure helpers and only one new constant. The reusable helpers placed in Phase B (`_configured_consumers_of_pre_constructed_key`) are exactly the right shape for Phase D/E/F to extend without duplication. **This is the strongest signal of design quality in the window.**
|
||||
|
||||
#### Minor naming asymmetry (informational, no finding)
|
||||
|
||||
`FAISS_BUILD_FLAG: Final[str]` (Phase B) vs. `C7_AIRBORNE_BUILD_FLAGS: Final[tuple[tuple[str, str], ...]]` (Phase C). Phase B has one flag; Phase C has two. The container types differ because the cardinalities differ. Not a finding — but if a future phase D/E/F introduces a third multi-flag constant, the project should consider a single uniform shape, e.g. `dict[str, tuple[(str, str), ...]]` keyed by infrastructure key, to eliminate the `_BUILD_FLAG` / `_BUILD_FLAGS` reader load.
|
||||
|
||||
#### Test-isolation pattern (informational, no finding)
|
||||
|
||||
Each phase's test file adds an **autouse stub fixture** for the next phase's builders so existing AC assertions remain valid as the bootstrap grows. AZ-619's `test_phase_a_only_seeds_two_keys` was relaxed to `test_phase_a_keys_remain_present_under_az620_additivity` (subset equality), and AZ-619/AZ-620 test files were extended with `_stub_c7_inference_builder` in batch 92. This is the right pattern for an additive umbrella — keeps each AC test focused on the keys it owns without coupling to later-phase env requirements (TensorRT, FAISS, Postgres).
|
||||
|
||||
## Cross-batch findings
|
||||
|
||||
### CR-F1 — `write_csv_evidence` duplication continues to scale (Medium / Maintainability — ESCALATED)
|
||||
|
||||
**Carry-over** from `cumulative_review_batches_85-87_cycle1_report.md` CR-F1.
|
||||
|
||||
Status update: the duplication is now spread across **17 evaluator helpers** (13 from batches 85-87 + 4 NFT-LIM helpers added by batch 88). Batch 89 (AZ-446) was speculatively expected to absorb this in `nfr_recorder.py` — but as captured in `_docs/03_implementation/reviews/batch_89_review.md` F2, AZ-446's spec is reporting-only (band/CI columns on per-metric `report.csv`), and the carry-over duplication lives in evaluator helpers + scenario boilerplate outside the AZ-446 envelope.
|
||||
|
||||
**Severity escalation rationale**: still Medium, but the carry-count went from 13 → 17 in one window. Every new NFT helper will reproduce the same ~30-line CSV-emit block. If batches 93-95 (AZ-622/623/624) introduce any new evaluator helpers (they should not — the umbrella is bootstrap wiring, not new NFT scenarios), the duplication will grow again.
|
||||
|
||||
**Proposed PBI**: hygiene — **3 points**. Add `e2e/runner/helpers/csv_evidence_writer.py` exposing `write_csv_evidence(out_path: Path, header: Sequence[str], rows: Sequence[Sequence[Any]], footer: Sequence[Sequence[Any]] | None = None) -> Path` and migrate the 17 helpers incrementally. Old per-helper API may co-exist during migration. Estimate: 17 helper files + 1 new helper + 1 unit test file.
|
||||
|
||||
### CR-F2 — `_resolve_fixture_path` duplication continues to scale (Medium / Maintainability — ESCALATED)
|
||||
|
||||
**Carry-over** from `cumulative_review_batches_85-87_cycle1_report.md` CR-F2.
|
||||
|
||||
Status update: same trajectory as CR-F1 — now in **17 scenario files** (13 from 85-87 + 4 NFT-LIM scenarios from batch 88). Every scenario defines an identical `_resolve_fixture_path() -> Path` differing only in env-var name + default filename + a `sitl_observer.replay_dir()` fallback.
|
||||
|
||||
**Severity escalation rationale**: as CR-F1.
|
||||
|
||||
**Proposed PBI**: hygiene — **2 points**. Add `e2e/runner/helpers/fixture_path.py` exposing `resolve(env_var_name: str, default_filename: str, sitl_observer_fallback: Callable[[], Path] | None = None) -> Path`. Pure refactor.
|
||||
|
||||
### CR-F3 — Per-batch review reports for batches 90 and 91 are missing on disk (Low / Process)
|
||||
|
||||
`_docs/03_implementation/reviews/` contains `batch_88_review.md`, `batch_89_review.md`, `batch_92_review.md` — but **NO `batch_90_review.md` or `batch_91_review.md`**. The batch reports for 90 and 91 say "self-reviewed inline" but the implement skill's Step 9 + Step 10 expect a canonical per-batch review artifact under `reviews/batch_NN_review.md`.
|
||||
|
||||
Likely cause: the previous session ran the implementation work and the inline self-review, but did not save the review report to disk. Combined with the batch 90 report being **a retroactive backfill** (its own header states this), this points to a process gap where the implement skill's writeback was partially skipped during the AZ-619/AZ-620 batches.
|
||||
|
||||
**Severity**: Low — the work itself is sound (verdicts in the batch reports are PASS with passing tests). But absent review artifacts complicate future cumulative reviews and the eventual product-implementation completeness gate. **No remediation expected for these specific batches** (the review can be reconstructed from batch reports + commit diffs at any point); the process correction is for future batches to land their review reports as part of the same commit as the code.
|
||||
|
||||
### CR-F4 — Outstanding hygiene PBIs from prior cumulative review still open (Info)
|
||||
|
||||
For tracking:
|
||||
|
||||
| PBI | Source | Estimate | Status |
|
||||
|-----|--------|----------|--------|
|
||||
| Shared `csv_evidence_writer` helper | CR-F1 (85-87) → escalated here | 3 cp | OPEN — escalated |
|
||||
| Shared `fixture_path.resolve` helper | CR-F2 (85-87) → escalated here | 2 cp | OPEN — escalated |
|
||||
| AZ-595 fixture builder materialization | CR-F3 (85-87) | 5 cp | OPEN — blocking 12 scenarios + 4 NFT-LIM scenarios |
|
||||
| `dns-blackhole` sidecar | CR-F4 (85-87) | 3 cp | OPEN — blocks NFT-SEC-05 live capture |
|
||||
|
||||
The AZ-595 fixture-builder backlog now has **17 dependent scenarios** (13 from prior window + 4 NFT-LIM added in batch 88). The next product cycle should sequence AZ-595 ahead of any retry of the Step 11 Run-Tests gate — without those fixtures, the Tier-1 docker harness will continue to `sitl_replay_ready`-skip 17+ scenarios.
|
||||
|
||||
### CR-F5 — `airborne_bootstrap.py` size + responsibility check (Info)
|
||||
|
||||
After Phase C, `airborne_bootstrap.py` is 637 lines: registration wiring (lines 199-420), `pre_constructed` builders (lines 423-587), and `register_airborne_strategies()` (lines 590-636). Phase D will add `c3_lightglue_runtime` + `c3_feature_extractor` builders; Phase E adds `c282_ransac_filter` + c5 helpers; Phase F wires `main()`. Projected size after Phase F: ~900-1000 lines.
|
||||
|
||||
The Single-Responsibility check passes today — the file's single responsibility is "airborne-binary composition: register strategies + build infrastructure dict". Phase D/E will add two more builders following the established pattern; this stays within scope. **Phase F (`main()` integration) is the size risk**: if it embeds CLI parsing + logging setup + `register_airborne_strategies()` + `build_pre_constructed()` + `compose_root()` orchestration, the file may exceed a comfortable single-file ceiling. **No finding today**; flag for the AZ-624 task spec / batch 95 review to evaluate whether `main()` belongs in a separate `airborne_main.py` adjacent to `airborne_bootstrap.py`.
|
||||
|
||||
### CR-F6 — All test runs in the window are green (Info)
|
||||
|
||||
| Batch | Test signal |
|
||||
|-------|-------------|
|
||||
| 88 | 207 helper unit tests pass in 0.36 s; full e2e unit suite **1223 passed in 154 s** |
|
||||
| 89 | +6 tests, full e2e unit suite **1229 passed in 134 s** |
|
||||
| 90 | 5 AZ-619 tests + AZ-591 regression (7/7) — **12/12 passed** |
|
||||
| 91 | 15/15 `tests/unit/runtime_root/` passed in 1.06 s; AZ-591 + AZ-619 + AZ-620 all green |
|
||||
| 92 | 18/18 `tests/unit/runtime_root/` passed in 0.68 s; AZ-591 (7/7) + AZ-619 (5/5) + AZ-620 (3/3) + AZ-621 (3/3) all green |
|
||||
|
||||
No new ruff errors on touched files across the window. Batch 91 also opportunistically cleaned up 12 pre-existing UP037 quoted-annotation lints in `airborne_bootstrap.py` (covered by `from __future__ import annotations`).
|
||||
|
||||
## Architecture Compliance (Phase 7)
|
||||
|
||||
`_docs/02_document/architecture_compliance_baseline.md` does NOT exist → no Baseline Delta section emitted.
|
||||
|
||||
Per-batch Phase-7 results (verified against this review's scope):
|
||||
|
||||
- **Batch 88 (e2e/** changes)**: all new files under `e2e/runner/helpers/` + `e2e/tests/resource_limit/` + `e2e/_unit_tests/helpers/` — owned by `blackbox_tests` (`Owns: e2e/**`). No `src/gps_denied_onboard` imports. No new cycles. F3 in batch 88 was an ownership drift in the **task spec** (suggested `tests/fixtures/...`); the implementation correctly moved the fixture to `e2e/fixtures/jetson/`.
|
||||
- **Batch 89 (`nfr_recorder.py` additive change)**: purely additive kw-only parameters + new public method. No new cross-component imports.
|
||||
- **Batch 90 (`airborne_bootstrap.py` Phase A)**: imports `runtime_root.fdr_client.client.make_fdr_client`, `clock.wall_clock.WallClock`, `runtime_root.register_strategy`. All Layer-5 sibling or lower. No cycles.
|
||||
- **Batch 91 (`airborne_bootstrap.py` Phase B)**: adds imports from `runtime_root.storage_factory` (Layer-5 sibling) and `runtime_root.errors`. All allowed.
|
||||
- **Batch 92 (`airborne_bootstrap.py` Phase C)**: adds import from `runtime_root.inference_factory` (Layer-5 sibling). All allowed. Per `_docs/03_implementation/reviews/batch_92_review.md`: imported symbol `build_inference_runtime` is in `inference_factory.__all__` — Public API respected.
|
||||
|
||||
**Cross-batch duplicate-symbol scan**: no class/function/constant introduced in this window collides with another component's symbol. The closest thing to a duplicate is the per-helper `write_csv_evidence` functions (CR-F1), but those are intentional pattern repetition within one component (`blackbox_tests`), not cross-component duplication.
|
||||
|
||||
**No new cyclic dependencies** introduced. The bootstrap module's import graph remains: `airborne_bootstrap → {clock, fdr_client, runtime_root.{storage_factory, inference_factory, errors, register_strategy}, runtime_root.{matcher,pose,refiner,rerank,state,vio,vpr}_factory}`. No back-edges.
|
||||
|
||||
## Final verdict
|
||||
|
||||
**PASS_WITH_WARNINGS** — proceed to batch 93.
|
||||
|
||||
Gate logic per code-review skill: 0 Critical, 0 High, 2 Medium (CR-F1 + CR-F2 carry-overs, both escalated), 1 Low (CR-F3 process), 3 Info — verdict is PASS_WITH_WARNINGS. Per implement skill Step 14.5 gate: PASS / PASS_WITH_WARNINGS → continue to next batch (Step 14 loop).
|
||||
|
||||
## Next-step recommendations
|
||||
|
||||
1. **Open hygiene PBI for CR-F1 + CR-F2** — combined 5 cp (or two separate 3+2 cp PBIs). Should land **before** Phase D/E/F if any new NFT helpers are introduced; otherwise can defer to after AZ-624.
|
||||
2. **Batch 93** = AZ-622 (Phase D, 3 cp). Single-task batch maintains the per-phase review focus that has worked through Phases A/B/C. Alternative: AZ-622 + AZ-623 (combined 6 cp) is acceptable — both extend the same bootstrap surface and the duplicate-symbol risk is low given the convergent pattern.
|
||||
3. **Do NOT combine all three remaining phases (D+E+F = 9 cp) into one batch** — AZ-624 (Phase F) wires `main()` and triggers the AC-1..AC-5 umbrella verification + Mandatory-Tier-2 Jetson run. Keeping AZ-624 as its own batch preserves a clean rollback boundary for the Jetson gate.
|
||||
4. **Process correction (CR-F3)**: future batches in this implementation arc should land `batch_NN_review.md` to `_docs/03_implementation/reviews/` as part of the same commit as the code, not as a self-review noted only in the batch report.
|
||||
@@ -0,0 +1,162 @@
|
||||
# Code Review Report — Batch 93
|
||||
|
||||
**Batch**: 93
|
||||
**Tasks**: AZ-622 (Phase D: build_pre_constructed seeds c3_lightglue_runtime + c3_feature_extractor)
|
||||
**Date**: 2026-05-19
|
||||
**Verdict**: PASS_WITH_WARNINGS
|
||||
**Reviewer**: autodev / implement skill Step 9
|
||||
**Cycle**: 1
|
||||
|
||||
## Scope
|
||||
|
||||
Files changed (per `git status`):
|
||||
|
||||
- `src/gps_denied_onboard/components/c3_matcher/config.py` — added optional `lightglue_weights_path: Path | None = None` field; extended `__post_init__` Path/None validator; expanded module docstring.
|
||||
- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` — new public constant `C3_MATCHER_BUILD_FLAGS`; new internal helpers `_resolve_c3_matcher_strategy`, `_is_build_flag_on`, `_load_lightglue_engine_handle`, `_build_c3_lightglue_runtime`, `_build_c3_feature_extractor`; refactored `build_pre_constructed` to step-by-step build so the just-built `c7_inference` flows into the LightGlue loader.
|
||||
- `tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
- `tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
- `tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py` — added autouse `_stub_c3_matcher_builders`.
|
||||
- `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` — new file: 6 tests covering AC-622.1, AC-622.2 (4 variants), and an identity-share invariant.
|
||||
|
||||
## Phase 1 — Context Loading
|
||||
|
||||
Read AZ-622 task spec (3pt; deps AZ-619/620/621/278 all in `done/`); umbrella spec AZ-618; matcher_factory's `_STRATEGY_TO_BUILD_FLAG`; LightGlueRuntime helper contract; OpenCvOrbExtractor helper docstring; existing AZ-619/620/621 test patterns. Confirmed:
|
||||
|
||||
- AC-622.1 = adds 2 keys on top of AZ-619..AZ-621; tests stub the heavy seam.
|
||||
- AC-622.2 = BUILD-flag mismatch surfaces clear `AirborneBootstrapError` naming the flag + consumer.
|
||||
- AC-622.3 = test file exists at the documented path.
|
||||
- Tier-2 Note = real LightGlue inference is verified at AZ-624 Jetson AC-5.
|
||||
|
||||
## Phase 2 — Spec Compliance
|
||||
|
||||
| AC | Test | Status |
|
||||
|----|------|--------|
|
||||
| AC-622.1 | `test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor` | Covered |
|
||||
| AC-622.2 | `test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error` (+ ALIKED variant + defence-in-depth + cause-chain wrap) | Covered |
|
||||
| AC-622.3 | File `tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py` exists | Covered |
|
||||
|
||||
Constraints check:
|
||||
|
||||
- **MUST NOT introduce new `BUILD_*` env flags** — PASS. The new `C3_MATCHER_BUILD_FLAGS` constant is a *consumer-side mirror* of the existing `matcher_factory._STRATEGY_TO_BUILD_FLAG` (`BUILD_MATCHER_DISK_LIGHTGLUE` / `BUILD_MATCHER_ALIKED_LIGHTGLUE` / `BUILD_MATCHER_XFEAT`). No new env var introduced.
|
||||
- **MUST reuse the existing per-strategy `BUILD_C3_MATCHER_*` matrix** — PASS in spirit. The actual production matrix is named `BUILD_MATCHER_*`, not `BUILD_C3_MATCHER_*` (the spec author's typo); the docstring of `C3_MATCHER_BUILD_FLAGS` calls this out so future readers don't get confused. See Finding F1 below.
|
||||
- **MUST be additive on top of AZ-619..AZ-621** — PASS. `build_pre_constructed`'s step-by-step build preserves every prior phase's key; the additivity test in `test_az619_*.py::test_phase_a_keys_remain_present_under_az620_additivity` continues to pass with the new builders in scope.
|
||||
- **The cross-component identity-share assertion (one instance shared across C3 + C2.5 in the actual graph) is verified at AZ-624** — Honored. The AZ-622 unit tests verify the LightGlueRuntime instance is constructed once per `build_pre_constructed` call; the wrapper-layer identity-share is left to AZ-624 per the task's Excluded section.
|
||||
|
||||
Spec drift surfaced as findings:
|
||||
|
||||
- **F1 (Low / Spec)** — see Findings section.
|
||||
|
||||
## Phase 3 — Code Quality
|
||||
|
||||
- **SOLID**: each new function has a single responsibility (resolve strategy, check flag, load engine, build runtime, build extractor, top-level assemble). No god-method introduced.
|
||||
- **Error handling**: every failure path produces an `AirborneBootstrapError` with three operator-actionable pieces (missing key, gating flag, consuming component) plus a remediation hint, matching the AZ-618 NFR contract. `RuntimeNotAvailableError` is preserved via `raise … from exc` so the upstream stack is recoverable.
|
||||
- **Naming**: `_resolve_c3_matcher_strategy`, `_load_lightglue_engine_handle`, `_build_c3_lightglue_runtime`, `_build_c3_feature_extractor` — all kebab-pattern consistent with prior phases (`_build_c6_descriptor_index`, `_build_c7_inference`).
|
||||
- **Complexity**: longest new function is `_build_c3_lightglue_runtime` at ~40 lines including docstring; cyclomatic complexity ≤ 4 per function.
|
||||
- **DRY**: `_is_build_flag_on` is a 2-line duplicate of `matcher_factory._is_build_flag_on`. See Finding F2.
|
||||
- **Test quality**: every test follows the Arrange/Act/Assert convention with comment markers; assertions check identity (`is`), type (`isinstance`), key membership, exception type, exception message content, and cause-chain — NOT just "no error thrown".
|
||||
- **Dead code**: no unused imports, no unreachable branches. The `del config` in `_build_c3_feature_extractor` is intentional (keeps the signature consistent with sibling builders).
|
||||
|
||||
## Phase 4 — Security Quick-Scan
|
||||
|
||||
- No SQL / command injection (no string-built queries, no `subprocess`, no `eval`/`exec`).
|
||||
- No hardcoded secrets, API keys, or paths to credential stores.
|
||||
- Input validation: `_load_lightglue_engine_handle` validates `weights_path is None` before calling `compile_engine`; the caller wraps any heavy-load failure into a structured error.
|
||||
- No sensitive data in error messages (path is referenced by name, not value).
|
||||
- No insecure deserialization — `deserialize_engine` is the existing C7 path that ships its own engine-gate validation (per AZ-301).
|
||||
|
||||
No security findings.
|
||||
|
||||
## Phase 5 — Performance Scan
|
||||
|
||||
- `build_pre_constructed` is a one-shot bootstrap call at process start; no hot-path concerns.
|
||||
- `_load_lightglue_engine_handle` is intentionally heavy (engine compile + deserialise) but is invoked once per process. The c7 `InferenceRuntime` is built once and reused for the LightGlue load — no double-build at this layer.
|
||||
- No O(n²) algorithms; no unbounded data fetching; no blocking I/O in async contexts (the bootstrap is synchronous by design).
|
||||
|
||||
No performance findings.
|
||||
|
||||
## Phase 6 — Cross-Task Consistency
|
||||
|
||||
- The new keys integrate with the existing `_c3_matcher_wrapper` (line 257 of `airborne_bootstrap.py`) which already extracts `c3_lightglue_runtime` from `constructed`. No signature collision.
|
||||
- The `_c2_5_rerank_wrapper` (line 232) extracts both `c3_lightglue_runtime` and `c3_feature_extractor` — both keys are now populated, matching the wrapper's expectation.
|
||||
- Prior-phase tests (AZ-619/620/621) now stub the new C3 builders via the same autouse pattern they already use for C6/C7. The pattern is uniform across all four phase test files.
|
||||
|
||||
## Phase 7 — Architecture Compliance
|
||||
|
||||
- **Layer direction**: `runtime_root.airborne_bootstrap` imports from `helpers/lightglue_runtime` (L1) and `helpers/feature_extractor` (L1). Allowed per `module-layout.md` rule 6 (composition root may import any L1 helper). PASS.
|
||||
- **Public API respect**: `LightGlueRuntime` is exported from `helpers/lightglue_runtime.py` `__all__`; `OpenCvOrbExtractor` is exported from `helpers/feature_extractor.py` `__all__`. Both imports go through documented Public API. PASS.
|
||||
- **No new cyclic module dependencies**: dependency chain `runtime_root.airborne_bootstrap → helpers/lightglue_runtime → _types/manifests + _types/matching` and `runtime_root.airborne_bootstrap → helpers/feature_extractor → _types/matching`. No reverse import. PASS.
|
||||
- **Duplicate symbols across components**: `_is_build_flag_on` exists in both `airborne_bootstrap.py` and `matcher_factory.py`. Both are *private* (underscore-prefixed) and live in the same `runtime_root/` subpackage — not a cross-component duplication. Severity Low; see Finding F2.
|
||||
- **Cross-cutting concerns not locally re-implemented**: the `BuildConfig(precision=PrecisionMode.FP16, workspace_mb=512, …)` constant in `_load_lightglue_engine_handle` mirrors the per-strategy `_build_disk_build_config` / `_build_aliked_build_config` shape in C3 strategy modules. Severity Low; see Finding F3.
|
||||
|
||||
No Critical or High Architecture findings.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| 1 | Low | Spec | `airborne_bootstrap.py:545` | `_build_c3_lightglue_runtime` signature differs from spec's single-arg form |
|
||||
| 2 | Low | Maintainability | `airborne_bootstrap.py` + `matcher_factory.py` | `_is_build_flag_on` duplicated across two runtime_root modules |
|
||||
| 3 | Low | Maintainability | `airborne_bootstrap.py:_load_lightglue_engine_handle` | `BuildConfig(precision=FP16, workspace_mb=512, …)` mirrors per-strategy `_build_*_build_config` constants |
|
||||
|
||||
### F1 — `_build_c3_lightglue_runtime` signature differs from spec's single-arg form (Low / Spec)
|
||||
|
||||
- **Location**: `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` (`_build_c3_lightglue_runtime(config: Config, *, inference_runtime: InferenceRuntime) -> LightGlueRuntime`).
|
||||
- **Description**: AZ-622 task spec § Scope.Included calls out internal builders as `_build_c3_lightglue_runtime(config)` and `_build_c3_feature_extractor(config)` — single-arg. The implementation adds an `inference_runtime` keyword-only argument so the just-built `pre_constructed['c7_inference']` can flow through without a double-build.
|
||||
- **Why this is the right interpretation**: AZ-622 also constrains "MUST be additive on top of AZ-619..AZ-621" and AZ-621's NFR guarantees one inference-runtime build per process. Re-calling `build_inference_runtime` from inside `_build_c3_lightglue_runtime` to satisfy the literal single-arg form would violate the additivity-and-no-double-build invariant. The kwarg keeps the surface honest about the dependency.
|
||||
- **Mitigation**: the divergence is documented inline in `_build_c3_lightglue_runtime`'s docstring and in `build_pre_constructed`'s docstring (line referencing "the C7 InferenceRuntime built for the c7_inference slot is reused as the engine source for the LightGlue matcher load").
|
||||
- **Suggestion**: none — the implementation is the more correct shape. If a future spec author updates the AZ-622 task file post-batch, fold the kwarg into the spec; otherwise leave as-is.
|
||||
- **Task reference**: AZ-622.
|
||||
|
||||
### F2 — `_is_build_flag_on` duplicated across `airborne_bootstrap.py` + `matcher_factory.py` (Low / Maintainability)
|
||||
|
||||
- **Location**: `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` + `src/gps_denied_onboard/runtime_root/matcher_factory.py` (both define the same 2-line predicate).
|
||||
- **Description**: Both `airborne_bootstrap` and `matcher_factory` now ship the same `_is_build_flag_on(flag_name: str) -> bool` predicate. The bootstrap intentionally avoids depending on `matcher_factory`'s private symbol so a future reorganisation of the matcher factory cannot break the bootstrap silently.
|
||||
- **Severity rationale**: the duplication is short (2 lines + docstring), private to a single subpackage, and prevents private-symbol coupling. Lifting it into a `runtime_root/_build_flags.py` (or `helpers/build_flags.py`) helper would be a hygiene improvement but not a correctness issue. Same pattern exists in `vpr_factory.py` (`vpr_factory._is_build_flag_on`) — at least 3 callers now duplicate the predicate.
|
||||
- **Suggestion**: add a follow-up hygiene PBI (sized 2 points) to extract `is_build_flag_on(flag_name: str) -> bool` into a shared helper module under `runtime_root/` (or `helpers/`). Defer to a future cumulative-review window.
|
||||
- **Task reference**: AZ-622, but the duplication predates this batch.
|
||||
|
||||
### F3 — `BuildConfig` literal duplicated with per-strategy build-config helpers (Low / Maintainability)
|
||||
|
||||
- **Location**: `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py::_load_lightglue_engine_handle`.
|
||||
- **Description**: `_load_lightglue_engine_handle` constructs `BuildConfig(precision=PrecisionMode.FP16, workspace_mb=512, calibration_dataset=None, optimization_profiles=())` inline. The same shape exists in `c3_matcher/disk_lightglue.py::_build_disk_build_config()` and `c3_matcher/aliked_lightglue.py::_build_aliked_build_config()`. If FP16 ever needs to be a config-driven knob (Tier-2 work), three places will drift.
|
||||
- **Severity rationale**: only matters when the build config becomes config-driven; today both per-strategy constants are literal-equal and the LightGlue engine is loaded with the same FP16 precision as DISK / ALIKED. Lifting into a shared helper would couple the bootstrap's internals to component-internal symbols — exactly the AZ-507 pattern we're trying to keep out of `runtime_root/`.
|
||||
- **Suggestion**: leave as-is for AZ-622. When the BuildConfig matrix becomes config-driven (likely a 2027 Tier-2 task), introduce a cross-cutting `helpers/build_config_for_engine.py` and migrate all three call sites at once.
|
||||
- **Task reference**: AZ-622, but the duplication predates this batch.
|
||||
|
||||
## Verdict Logic
|
||||
|
||||
- 0 Critical findings → not FAIL.
|
||||
- 0 High findings → not FAIL.
|
||||
- 3 Low findings → **PASS_WITH_WARNINGS**.
|
||||
|
||||
## Auto-Fix Eligibility
|
||||
|
||||
| Finding | Severity | Category | Auto-fix eligible? |
|
||||
|---------|----------|----------|--------------------|
|
||||
| F1 | Low | Spec | No — divergence is the more correct shape; spec needs updating |
|
||||
| F2 | Low | Maintainability | Defer — hygiene PBI for future cumulative-review window |
|
||||
| F3 | Low | Maintainability | Defer — wait for FP16-as-config knob requirement |
|
||||
|
||||
No findings escalate. Proceed to commit (Step 11).
|
||||
|
||||
## Test Verification
|
||||
|
||||
```
|
||||
pytest tests/unit/runtime_root/test_az619_pre_constructed_phase_a.py \
|
||||
tests/unit/runtime_root/test_az620_pre_constructed_phase_b.py \
|
||||
tests/unit/runtime_root/test_az621_pre_constructed_phase_c.py \
|
||||
tests/unit/runtime_root/test_az622_pre_constructed_phase_d.py -v
|
||||
=> 17 passed in 5.83s
|
||||
```
|
||||
|
||||
Broader regression check (runtime_root + c3_matcher + c3_5_adhop):
|
||||
|
||||
```
|
||||
pytest tests/unit/runtime_root/ tests/unit/c3_matcher/ -x --timeout=60
|
||||
=> 108 passed in 3.89s
|
||||
|
||||
pytest tests/unit/c3_5_adhop/test_az349_adhop_refiner.py
|
||||
=> 23 passed in 1.09s
|
||||
```
|
||||
|
||||
No regression introduced by the C3MatcherConfig schema extension.
|
||||
@@ -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.
|
||||
@@ -0,0 +1,107 @@
|
||||
# Code Review Report
|
||||
|
||||
**Batch**: 95
|
||||
**Tasks**: AZ-625 (Phase E.5 of AZ-618: c5_isam2_graph_handle ordering — separate handle from estimator construction)
|
||||
**Date**: 2026-05-19
|
||||
**Verdict**: PASS
|
||||
|
||||
## Phase 1: Context
|
||||
|
||||
Read in this review window:
|
||||
|
||||
- `_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md` (task spec; 4 ACs)
|
||||
- `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` (umbrella: "MUST NOT touch any per-component factory signature"; "All changes confined to runtime_root/airborne_bootstrap.py")
|
||||
- `_docs/03_implementation/batch_94_cycle1_report.md` (split rationale: original AZ-623 escalated the handle work; new AZ-625 captures the deferred wiring)
|
||||
- `src/gps_denied_onboard/components/c4_pose/_isam2_handle.py` (C4-side `ISam2GraphHandle` Protocol surface)
|
||||
- `src/gps_denied_onboard/components/c5_state/_isam2_handle.py` (`ISam2GraphHandleImpl.__init__(estimator)` — the Protocol-seam constraint that forbids handle-only construction)
|
||||
- `src/gps_denied_onboard/runtime_root/state_factory.py` (`build_state_estimator` return-tuple shape — confirms `(estimator, handle)` is the only construction site)
|
||||
- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` (mutated)
|
||||
- `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py` (new)
|
||||
- `tests/unit/runtime_root/test_az619..test_az623` (autouse fixtures stubbed to keep AZ-619..AZ-623 tests green)
|
||||
|
||||
## Phase 2: Spec Compliance
|
||||
|
||||
| AC | Status | Test | Notes |
|
||||
|----|--------|------|-------|
|
||||
| AC-625.1 (`c5_isam2_graph_handle` added on top of AZ-619..AZ-623; satisfies C4 `ISam2GraphHandle` Protocol) | Covered | `test_ac_625_1_adds_c5_isam2_graph_handle_with_protocol_surface` + `test_ac_625_1_internal_prebuilt_estimator_key_not_in_required_keys` | Presence asserted; `isinstance(handle, C4ISam2GraphHandle)` + per-method `hasattr` covers the runtime-checkable Protocol; AZ-619..AZ-623 keys still present (additivity). The internal `_c5_prebuilt_estimator` look-aside key is asserted NOT present in `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`. |
|
||||
| AC-625.2 (BUILD-flag OFF or unknown strategy → `AirborneBootstrapError` naming flag + `c5_state`) | Covered | `test_ac_625_2_build_state_gtsam_isam2_off_raises_named_error` + `test_ac_625_2_unknown_strategy_raises_named_error_with_supported_set` + `test_ac_625_2_build_state_estimator_config_error_wraps_into_bootstrap_error` | Three branches: explicit `BUILD_STATE_GTSAM_ISAM2=OFF`, unknown strategy (smuggled via `_resolve_c5_state_strategy` monkeypatch), and downstream `StateEstimatorConfigError` wrapping with `__cause__` preserved. Each asserts the missing key, the gating flag, and the consuming component slug `c5_state` are in the message. |
|
||||
| AC-625.3 (handle held by C4 IS the same object as estimator's `_isam2_handle`) | Covered | `test_ac_625_3_handle_is_same_object_as_estimator_isam2_handle` + `test_ac_625_3_c5_state_wrapper_short_circuits_on_prebuilt_estimator` + `test_ac_625_3_c5_state_wrapper_falls_back_when_prebuilt_absent` | Identity invariant verified at the look-aside-key seam (cheaper than standing up `compose_root` end-to-end, which would pull gtsam + FAISS + TensorRT — task spec's Tier-2 Note explicitly defers full e2e to AZ-624's Jetson AC-5). The fallback path is also covered so existing fixtures (e.g., `test_az401_compose_root_replay`) that bypass `build_pre_constructed` still work. |
|
||||
| AC-625.4 (test file under `tests/unit/runtime_root/test_az625_c5_isam2_graph_handle_ordering.py`) | Covered | file exists with 7 tests, all green | Filename matches AC. |
|
||||
|
||||
**Constraint compliance**:
|
||||
|
||||
- "MUST NOT modify per-component factory signatures" → `state_factory.build_state_estimator` is invoked with the same kwargs the wrapper would have passed; no signature changed. ✓
|
||||
- "MUST be additive on top of AZ-619..AZ-623" → AZ-619..AZ-623 keys still present in `pre_constructed`; `_C5_PREBUILT_ESTIMATOR_KEY` is internal coordination, not part of the public required-keys map. ✓
|
||||
- "MUST NOT touch state_factory, pose_factory, or c5_state internals" → no edits under those component directories. ✓
|
||||
- "All changes confined to airborne_bootstrap.py + new test file" (umbrella) → diff scope respected (existing AZ-619..AZ-623 phase tests adjusted only to add an autouse stub for the new builder, which is hygiene maintenance for the additive bootstrap call, not a behavioral change). ✓
|
||||
|
||||
## Phase 3: Code Quality
|
||||
|
||||
2 findings — all Low; none blocking:
|
||||
|
||||
- F1 (Low / Style): `_C5_PREBUILT_ESTIMATOR_KEY` is defined at module level after `_resolve_c5_state_strategy` but used earlier in the file inside `_c5_state_wrapper`. Python's lazy name resolution at call time makes this correct, but a forward reader has to scroll to confirm where the constant comes from. The placement keeps the C5-state-related constants grouped together (`C5_STATE_BUILD_FLAGS` + `_C5_PREBUILT_ESTIMATOR_KEY` + `_resolve_c5_state_strategy` + `_build_c5_state_estimator_pair`), which is the dominant readability axis. No change recommended.
|
||||
- F2 (Low / Maintainability): function-scoped `from gps_denied_onboard.components.c5_state.errors import StateEstimatorConfigError` inside `_build_c5_state_estimator_pair`. Could be hoisted to the module-level `if TYPE_CHECKING:` block since it's only used for `except` matching, but the existing `_ensure_state_strategy_registered` already function-scope-imports `gtsam_isam2_estimator` and `eskf_baseline` from the same component (intentional: keeps gtsam off the import graph until the strategy is selected). This file's pattern is "import inside the function that needs it"; keeping the new import at function scope matches that convention. No change recommended.
|
||||
|
||||
## Phase 4: Security
|
||||
|
||||
No findings.
|
||||
|
||||
- No SQL, no command exec, no `eval`/`exec`.
|
||||
- No new external input ingress; the strategy + flag values come from `Config` (operator-supplied YAML, validated upstream by `Config` dataclass) and `os.environ` (compile-time build flags).
|
||||
- Error messages include the missing flag name and supported strategy set — no secret leakage.
|
||||
|
||||
## Phase 5: Performance
|
||||
|
||||
No findings.
|
||||
|
||||
- The eager `(estimator, handle)` build at bootstrap moves work from "first call to `compose_root`" to "before `compose_root` runs". Same total work; no new hot-path cost.
|
||||
- The look-aside dict get + identity short-circuit in `_c5_state_wrapper` is O(1) and runs once per `compose_root` invocation.
|
||||
|
||||
## Phase 6: Cross-Task Consistency
|
||||
|
||||
Consistent with the AZ-619..AZ-623 builder pattern:
|
||||
|
||||
- `C5_STATE_BUILD_FLAGS` mirrors `C3_MATCHER_BUILD_FLAGS` (AZ-622) verbatim — both surface the per-strategy `BUILD_*` flag matrix at the airborne layer for operator-error messages, and both note that mutations MUST mirror the per-component factory's table.
|
||||
- `_build_c5_state_estimator_pair` follows the validation order shared by AZ-621 (`_build_c7_inference`) and AZ-622 (`_build_c3_lightglue_runtime`): resolve strategy → check flag matrix → check OFF gate → register strategy lazily → delegate to factory and wrap upstream errors.
|
||||
- The `_C5_PREBUILT_ESTIMATOR_KEY` look-aside key is documented as internal coordination at definition site AND in the wrapper docstring.
|
||||
|
||||
The 5 phase tests (AZ-619..AZ-623) had their autouse `_stub_c5_builders` fixture extended with one more `setattr` to stub `_build_c5_state_estimator_pair`. The stub is opaque (returns `(MagicMock, MagicMock)`), keeping each phase's assertions focused on its own contract. AZ-625's test file owns the (estimator, handle) pair contract.
|
||||
|
||||
## Phase 7: Architecture Compliance
|
||||
|
||||
- Layer direction: `airborne_bootstrap` (Layer 5 — entry / composition) imports `state_factory.build_state_estimator` (Layer 5 — runtime_root) and `components.c5_state.errors.StateEstimatorConfigError` (Layer 3 — domain). Layer 5 importing Layer 3 is allowed (composition root is the registration site per ADR-001). ✓
|
||||
- Public API respect: `c5_state.errors` is technically not in c5_state's "Public API" file list in `module-layout.md` (which lists only `__init__.py` and `interface.py`). However, the bootstrap is the documented exception per ADR-002 + ADR-001 (the build-flag gate / single-registration-site combo permits the airborne bootstrap to reach component internals to wrap their errors into `AirborneBootstrapError`). The existing line 440 `from gps_denied_onboard.components.c5_state import gtsam_isam2_estimator` already establishes this precedent in the same file; AZ-625 does not introduce a new exception. (Possible follow-up: list `errors.py` explicitly in c5_state's Public API row of `module-layout.md`. NOT in scope for AZ-625.)
|
||||
- No new module cycles introduced (the import graph still has `airborne_bootstrap → state_factory → c5_state`, no back-edge).
|
||||
- No duplicate symbols across components.
|
||||
- No cross-cutting concerns re-implemented locally.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| F1 | Low | Style | `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py:649` | `_C5_PREBUILT_ESTIMATOR_KEY` defined after first reference site |
|
||||
| F2 | Low | Maintainability | `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py:748` | Function-scoped `StateEstimatorConfigError` import (consistent with existing pattern) |
|
||||
|
||||
### Finding Details
|
||||
|
||||
**F1: `_C5_PREBUILT_ESTIMATOR_KEY` defined after first reference site** (Low / Style)
|
||||
|
||||
- Location: `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py:649`
|
||||
- Description: The constant is referenced at line 391 (`_c5_state_wrapper`) but defined at line 649. Python resolves the name lazily so this is correct.
|
||||
- Suggestion: leave as-is — grouping with other C5-state constants is the dominant readability axis.
|
||||
- Task: AZ-625
|
||||
|
||||
**F2: Function-scoped `StateEstimatorConfigError` import** (Low / Maintainability)
|
||||
|
||||
- Location: `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py:748`
|
||||
- Description: The exception is imported at function scope inside `_build_c5_state_estimator_pair`.
|
||||
- Suggestion: Matches the file's existing function-scope-import pattern for c5_state submodules; no change.
|
||||
- Task: AZ-625
|
||||
|
||||
## Verdict Logic
|
||||
|
||||
- 0 Critical, 0 High → not FAIL.
|
||||
- 0 Medium → not PASS_WITH_WARNINGS for medium.
|
||||
- 2 Low → still PASS overall (no Medium / High / Critical).
|
||||
|
||||
**Verdict: PASS**.
|
||||
@@ -0,0 +1,96 @@
|
||||
# Code Review Report
|
||||
|
||||
**Batch**: 96
|
||||
**Tasks**: AZ-624 (Phase F of AZ-618: wire `build_pre_constructed` into `runtime_root.main()` + AC-1..AC-5 verification)
|
||||
**Date**: 2026-05-19
|
||||
**Verdict**: PASS
|
||||
|
||||
## Phase 1: Context
|
||||
|
||||
Read in this review window:
|
||||
|
||||
- `_docs/02_tasks/todo/AZ-624_pre_constructed_phase_f_wire_main.md` (task spec; ACs)
|
||||
- `_docs/02_tasks/todo/AZ-618_airborne_bootstrap_pre_constructed.md` (umbrella; canonical AC-1..AC-5)
|
||||
- `_docs/03_implementation/batch_95_cycle1_report.md` (immediate predecessor — confirms AZ-625 unblocked AZ-624)
|
||||
- `src/gps_denied_onboard/runtime_root/__init__.py` (main() before + after; existing exception block; compose_root signature)
|
||||
- `src/gps_denied_onboard/runtime_root/airborne_bootstrap.py` (`build_pre_constructed`, `register_airborne_strategies`, `AirborneBootstrapError`)
|
||||
- `tests/unit/test_az401_compose_root_replay.py` (env / fixture pattern reused by the new umbrella test file)
|
||||
|
||||
## Phase 2: Spec Compliance
|
||||
|
||||
| AC | Status | Test | Notes |
|
||||
|----|--------|------|-------|
|
||||
| AC-618-1 (every required key populated, no None) | Covered | `test_ac_618_1_build_pre_constructed_populates_every_required_key` | Asserts the union of all `AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` rows is a subset of the returned dict + every key is non-None. |
|
||||
| AC-618-2 (compose_root reaches takeoff with 7 slots) | Covered | `test_ac_618_2_compose_root_reaches_takeoff_with_all_seven_slots` | Stubs the 7 strategy factories the airborne wrappers delegate to + builds a 7-block Config; asserts the returned `RuntimeRoot.components` contains all 7 slot names. |
|
||||
| AC-618-3 (BUILD-flag mismatch raises named error) | Covered | `test_ac_618_3_build_flag_off_raises_named_error_with_consuming_component` | Drops the autouse `_build_c7_inference` stub, forces `build_inference_runtime` to raise `RuntimeNotAvailableError`, asserts the wrapping error message names `c7_inference` + both airborne C7 BUILD flags + the consuming component slug `c2_vpr`. |
|
||||
| AC-618-4 (main() exit codes + stderr prefix) | Covered | `test_ac_618_4_main_returns_zero_on_success` + `test_ac_618_4_main_returns_generic_failure_with_airborne_bootstrap_prefix` + `test_ac_624_local_main_distinct_handler_does_not_double_print` | Three tests: success path returns 0 with no stderr; forced bootstrap failure returns `EXIT_GENERIC_FAILURE` and stderr contains the `airborne_bootstrap:` prefix; the dedicated handler does not double-print (regression guard for the catch ordering). |
|
||||
| AC-618-5 / AC-624.tier2 (Jetson tier-2 e2e) | BLOCKED | n/a (out-of-process) | Requires `scripts/run-tests-jetson.sh tests/e2e/replay/test_derkachi_1min.py` on real Jetson Orin Nano hardware (JetPack 6.2.2+b24). Cannot run from the Mac dev host. Operator-supplied evidence required: terminal log path + JetPack version + run timestamp. |
|
||||
| AC-624.local (full Tier-1 pytest green) | Pass with 1 unrelated pre-existing failure | full `tests/unit/` suite | 2150 passed, 85 skipped (environment-gated: GPU / Docker / Jetson / TensorRT). One unrelated failure: `tests/unit/c12_operator_orchestrator/test_cli_console_script.py::TestConsoleScript::test_cold_start_under_500ms_p99` — Mac dev host's Python subprocess startup is ~700 ms, exceeding the 500 ms NFR. This test exists in a component (`c12_operator_orchestrator`) AZ-624 does NOT modify or depend on; failure is a host-performance artifact, not a regression. Reported to the user; not blocking AZ-624. |
|
||||
|
||||
**Constraint compliance**:
|
||||
|
||||
- "MUST NOT touch any per-component factory signature" → `compose_root` and `build_pre_constructed` signatures unchanged; only the body of `main()` was extended. ✓
|
||||
- "MUST NOT introduce new BUILD_* env flags" → no new flag introduced. ✓
|
||||
- "All changes confined to `runtime_root/__init__.py` + `runtime_root/airborne_bootstrap.py` + new test file" → diff scope respected. ✓
|
||||
|
||||
## Phase 3: Code Quality
|
||||
|
||||
1 finding — Low / Style; not blocking:
|
||||
|
||||
- F1 (Low / Style): the dedicated `except AirborneBootstrapError` handler in `main()` produces stderr identical to the broader `(ConfigurationError, StrategyNotLinkedError, RuntimeError)` clause that already exists (since `AirborneBootstrapError` extends `RuntimeError` and the message itself carries the `airborne_bootstrap:` prefix). The dedicated clause is documentation-as-code: it makes the error path traceable in code review and locks the surfacing format against future broader-clause edits. No functional difference at runtime today; intentional. The inline comment captures the rationale.
|
||||
|
||||
## Phase 4: Security
|
||||
|
||||
No findings.
|
||||
|
||||
- No SQL, no command exec, no `eval`/`exec`.
|
||||
- No new external input ingress.
|
||||
- Error messages include the missing key + flag + component slug — no secret leakage (the bootstrap error format was already audited in AZ-619..AZ-625).
|
||||
|
||||
## Phase 5: Performance
|
||||
|
||||
No findings.
|
||||
|
||||
- The `build_pre_constructed(config)` call is a one-shot bootstrap; not in any hot path.
|
||||
- The new exception handler in `main()` is exercised only on a failure path; zero cost on the success path.
|
||||
|
||||
## Phase 6: Cross-Task Consistency
|
||||
|
||||
Consistent with the AZ-619..AZ-625 batch pattern:
|
||||
|
||||
- The umbrella test file uses the same env / config / monkeypatch pattern as the per-phase test files (`test_az619..test_az625`).
|
||||
- The `_stub_heavy_builders` helper mirrors the autouse fixtures from each phase test (one explicit stub per documented `_build_*` builder).
|
||||
- The `_stub_strategy_factories` helper mirrors the strategy-factory monkeypatch pattern from `test_az401_compose_root_replay.py`'s `_fake_replay_components_factory` — both replace heavy seams in the airborne_bootstrap namespace so wrappers return MagicMock sentinels.
|
||||
|
||||
The 1-line addition to `build_pre_constructed`-callers (none beyond `main()`) is consistent: the function's contract has not changed since AZ-619 — `main()` is the first non-test caller.
|
||||
|
||||
## Phase 7: Architecture Compliance
|
||||
|
||||
- Layer direction: `runtime_root/__init__.py::main()` (Layer 5 — entry / composition) imports from `runtime_root.airborne_bootstrap` (same Layer 5). ✓
|
||||
- Public API respect: the imports at the top of `main()`'s function body (`AirborneBootstrapError`, `build_pre_constructed`, `register_airborne_strategies`) are all in `airborne_bootstrap.__all__`. ✓
|
||||
- No new module cycles introduced. The `runtime_root/__init__.py` → `runtime_root/airborne_bootstrap.py` edge already existed.
|
||||
- No duplicate symbols across components.
|
||||
- No cross-cutting concerns re-implemented locally.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| F1 | Low | Style | `src/gps_denied_onboard/runtime_root/__init__.py:680` | Dedicated `AirborneBootstrapError` handler is documentation-as-code |
|
||||
|
||||
### Finding Details
|
||||
|
||||
**F1: Dedicated `AirborneBootstrapError` handler is documentation-as-code** (Low / Style)
|
||||
|
||||
- Location: `src/gps_denied_onboard/runtime_root/__init__.py:680`
|
||||
- Description: At runtime, the dedicated handler produces stderr identical to the broader `RuntimeError` clause (the bootstrap error message itself carries the `airborne_bootstrap:` prefix). The dedicated clause makes the error path traceable in code review and locks the surfacing format.
|
||||
- Suggestion: leave as-is — the inline comment captures the rationale; the regression guard test (`test_ac_624_local_main_distinct_handler_does_not_double_print`) ensures the catch ordering is not silently broken.
|
||||
- Task: AZ-624
|
||||
|
||||
## Verdict Logic
|
||||
|
||||
- 0 Critical, 0 High → not FAIL.
|
||||
- 0 Medium → not PASS_WITH_WARNINGS for medium.
|
||||
- 1 Low → still PASS overall.
|
||||
|
||||
**Verdict: PASS** (with one out-of-scope note: AC-618-5 / AC-624.tier2 BLOCKED on Jetson hardware evidence; user must supply).
|
||||
@@ -8,8 +8,8 @@ status: in_progress
|
||||
sub_step:
|
||||
phase: 16
|
||||
name: batch-loop
|
||||
detail: "batch 92 done (AZ-621 Phase C). Next: cumulative review for batches 85-92 (8 batches overdue, K=3 default missed), THEN batch 93 = AZ-622 (Phase D, 3pt) or AZ-622+AZ-623 (6pt). Fresh session recommended."
|
||||
detail: "AZ-618 umbrella complete; AC-618-5 BLOCKED on operator Jetson run"
|
||||
retry_count: 0
|
||||
cycle: 1
|
||||
tracker: jira
|
||||
last_completed_batch: 92
|
||||
last_completed_batch: 96
|
||||
|
||||
@@ -37,6 +37,16 @@ by C3.5; it lives in :class:`C3_5RefinerConfig` (sibling block) —
|
||||
not duplicated here. ``None`` is allowed as a placeholder for
|
||||
``BUILD_MATCHER_<variant>=OFF`` binaries; the factory rejects
|
||||
``None`` only for the SELECTED strategy at composition time.
|
||||
|
||||
``lightglue_weights_path`` (AZ-622 / Phase D) points at the LightGlue
|
||||
*matcher* engine — distinct from the per-strategy DISK / ALIKED
|
||||
*feature-extractor* engine. The airborne bootstrap
|
||||
(``runtime_root.airborne_bootstrap.build_pre_constructed``) reads
|
||||
this field to construct the single shared
|
||||
:class:`gps_denied_onboard.helpers.lightglue_runtime.LightGlueRuntime`
|
||||
that C3 + C2.5 both consume (R14 fix; one instance avoids double GPU
|
||||
memory). ``None`` is allowed; production main() (AZ-624) populates
|
||||
the path before calling ``build_pre_constructed``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -69,6 +79,7 @@ class C3MatcherConfig:
|
||||
disk_weights_path: Path | None = None
|
||||
aliked_weights_path: Path | None = None
|
||||
xfeat_weights_path: Path | None = None
|
||||
lightglue_weights_path: Path | None = None
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if self.strategy not in KNOWN_STRATEGIES:
|
||||
@@ -91,7 +102,12 @@ class C3MatcherConfig:
|
||||
"C3MatcherConfig.ransac_threshold_px must be > 0; "
|
||||
f"got {self.ransac_threshold_px}"
|
||||
)
|
||||
for name in ("disk_weights_path", "aliked_weights_path", "xfeat_weights_path"):
|
||||
for name in (
|
||||
"disk_weights_path",
|
||||
"aliked_weights_path",
|
||||
"xfeat_weights_path",
|
||||
"lightglue_weights_path",
|
||||
):
|
||||
value = getattr(self, name)
|
||||
if value is not None and not isinstance(value, Path):
|
||||
raise ConfigError(
|
||||
|
||||
@@ -344,9 +344,7 @@ def _compose(
|
||||
for slug, strategy in selections.items()
|
||||
}
|
||||
order = _topo_order(resolved.keys(), resolved)
|
||||
constructed: dict[str, Any] = (
|
||||
dict(pre_constructed) if pre_constructed is not None else {}
|
||||
)
|
||||
constructed: dict[str, Any] = dict(pre_constructed) if pre_constructed is not None else {}
|
||||
for slug in order:
|
||||
registration = resolved[slug]
|
||||
try:
|
||||
@@ -454,11 +452,7 @@ def compose_root(
|
||||
have to satisfy the full OpenCV / pymavlink / FDR side-effects of
|
||||
the real strategies.
|
||||
"""
|
||||
extra_env = (
|
||||
("MAVLINK_SIGNING_KEY",)
|
||||
if config.mode == "live"
|
||||
else ()
|
||||
)
|
||||
extra_env = ("MAVLINK_SIGNING_KEY",) if config.mode == "live" else ()
|
||||
if config.mode == "replay":
|
||||
replay_factory = replay_components_factory or build_replay_components
|
||||
replay_components, replay_order = replay_factory(config)
|
||||
@@ -478,9 +472,7 @@ def compose_root(
|
||||
)
|
||||
merged: dict[str, Any] = dict(replay_components)
|
||||
merged.update(components)
|
||||
full_order = tuple(replay_order) + tuple(
|
||||
slug for slug in order if slug not in replay_order
|
||||
)
|
||||
full_order = tuple(replay_order) + tuple(slug for slug in order if slug not in replay_order)
|
||||
return RuntimeRoot(
|
||||
binary="airborne",
|
||||
profile=os.environ["GPS_DENIED_FC_PROFILE"],
|
||||
@@ -659,6 +651,8 @@ def main(config: Config | None = None) -> int:
|
||||
"""
|
||||
from gps_denied_onboard.replay_input import ReplayInputAdapterError
|
||||
from gps_denied_onboard.runtime_root.airborne_bootstrap import (
|
||||
AirborneBootstrapError,
|
||||
build_pre_constructed,
|
||||
register_airborne_strategies,
|
||||
)
|
||||
|
||||
@@ -666,10 +660,18 @@ def main(config: Config | None = None) -> int:
|
||||
if config is None:
|
||||
config = load_config(env=os.environ, paths=())
|
||||
register_airborne_strategies()
|
||||
compose_root(config)
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
compose_root(config, pre_constructed=pre_constructed)
|
||||
except ReplayInputAdapterError as exc:
|
||||
print(f"runtime_root: replay sync impossible: {exc}", file=sys.stderr)
|
||||
return EXIT_FDR_OPEN_FAILURE
|
||||
except AirborneBootstrapError as exc:
|
||||
# Surface the operator-actionable bootstrap error directly so the
|
||||
# operator sees the "airborne_bootstrap: ..." prefix the message
|
||||
# already carries (named missing dep + consuming component slug
|
||||
# + actionable fix), rather than the raw RuntimeError fallback.
|
||||
print(f"runtime_root: {exc}", file=sys.stderr)
|
||||
return EXIT_GENERIC_FAILURE
|
||||
except (ConfigurationError, StrategyNotLinkedError, RuntimeError) as exc:
|
||||
print(f"runtime_root: {exc}", file=sys.stderr)
|
||||
return EXIT_GENERIC_FAILURE
|
||||
|
||||
@@ -48,13 +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
|
||||
@@ -71,19 +82,50 @@ from gps_denied_onboard.runtime_root.vio_factory import build_vio_strategy
|
||||
from gps_denied_onboard.runtime_root.vpr_factory import build_vpr_strategy
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from gps_denied_onboard._types.manifests import EngineHandle
|
||||
from gps_denied_onboard.components.c7_inference import InferenceRuntime
|
||||
from gps_denied_onboard.config import Config
|
||||
from gps_denied_onboard.helpers.feature_extractor import FeatureExtractor
|
||||
|
||||
__all__ = [
|
||||
"AIRBORNE_MAIN_PRODUCER_ID",
|
||||
"AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS",
|
||||
"C3_MATCHER_BUILD_FLAGS",
|
||||
"C5_STATE_BUILD_FLAGS",
|
||||
"C7_AIRBORNE_BUILD_FLAGS",
|
||||
"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.
|
||||
|
||||
@@ -114,6 +156,55 @@ flag would unblock the bootstrap with a different runtime selection.
|
||||
"""
|
||||
|
||||
|
||||
C5_STATE_BUILD_FLAGS: Final[Mapping[str, str]] = {
|
||||
"gtsam_isam2": "BUILD_STATE_GTSAM_ISAM2",
|
||||
"eskf": "BUILD_STATE_ESKF",
|
||||
}
|
||||
"""Per-strategy ``BUILD_STATE_*`` flag matrix consumed by the airborne
|
||||
c5_state estimator pair builder (AZ-625 / Phase E.5).
|
||||
|
||||
Mirrors :data:`gps_denied_onboard.runtime_root.state_factory._STATE_BUILD_FLAGS`
|
||||
verbatim — both this constant and the state factory's table read the same
|
||||
compile-time flags. ANY mutation of this matrix MUST be mirrored in
|
||||
``state_factory._STATE_BUILD_FLAGS`` (and vice versa).
|
||||
|
||||
Surfaced here so :func:`_build_c5_state_estimator_pair` can name the
|
||||
gating flag in an :class:`AirborneBootstrapError` (AC-625.2) when the
|
||||
configured C5 state strategy's flag is OFF in this binary, *before*
|
||||
:func:`build_state_estimator` has a chance to raise the lower-level
|
||||
:class:`StateEstimatorConfigError` (which is the
|
||||
state-factory-internal error type, not the operator-facing
|
||||
bootstrap-error contract this module owns).
|
||||
"""
|
||||
|
||||
|
||||
C3_MATCHER_BUILD_FLAGS: Final[Mapping[str, str]] = {
|
||||
"disk_lightglue": "BUILD_MATCHER_DISK_LIGHTGLUE",
|
||||
"aliked_lightglue": "BUILD_MATCHER_ALIKED_LIGHTGLUE",
|
||||
"xfeat": "BUILD_MATCHER_XFEAT",
|
||||
}
|
||||
"""Per-strategy ``BUILD_MATCHER_*`` flag matrix consumed by the airborne
|
||||
LightGlue-runtime builder (AZ-622 / Phase D).
|
||||
|
||||
Mirrors :data:`gps_denied_onboard.runtime_root.matcher_factory.\
|
||||
_STRATEGY_TO_BUILD_FLAG` verbatim — both this constant and the matcher
|
||||
factory's table read the same compile-time flags. ANY mutation of this
|
||||
matrix MUST be mirrored in ``matcher_factory._STRATEGY_TO_BUILD_FLAG``
|
||||
(and vice versa).
|
||||
|
||||
Surfaced here so :func:`_build_c3_lightglue_runtime` can name the
|
||||
gating flag in an :class:`AirborneBootstrapError` (AC-622.2) when the
|
||||
configured C3 matcher strategy's flag is OFF in this binary.
|
||||
|
||||
Note on flag naming: AZ-622's task spec uses the name
|
||||
``BUILD_C3_MATCHER_DISK_LIGHTGLUE`` informally, but the actual
|
||||
production flag matrix is the older ``BUILD_MATCHER_*`` family
|
||||
(``matcher_factory._STRATEGY_TO_BUILD_FLAG``). Reusing those flags is
|
||||
the spirit of the AZ-622 ``MUST reuse the existing per-strategy
|
||||
BUILD_C3_MATCHER_* matrix`` constraint.
|
||||
"""
|
||||
|
||||
|
||||
AIRBORNE_MAIN_PRODUCER_ID: Final[str] = "airborne_main"
|
||||
"""Producer ID for the per-binary shared FdrClient placed under
|
||||
``pre_constructed['c13_fdr']``.
|
||||
@@ -196,9 +287,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())
|
||||
@@ -229,16 +318,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(
|
||||
@@ -251,12 +334,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")
|
||||
@@ -271,9 +350,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")
|
||||
@@ -291,9 +368,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(
|
||||
@@ -308,9 +383,19 @@ 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"
|
||||
)
|
||||
# AZ-625 fast path: when build_pre_constructed has eagerly built the
|
||||
# (estimator, handle) pair, the estimator lives under the private
|
||||
# _c5_prebuilt_estimator key. Returning the prebuilt instance keeps
|
||||
# c4_pose's c5_isam2_graph_handle pointing at the SAME estimator's
|
||||
# _isam2_handle — the AC-625.3 cross-seam identity invariant.
|
||||
prebuilt = constructed.get(_C5_PREBUILT_ESTIMATOR_KEY)
|
||||
if prebuilt is not None:
|
||||
return prebuilt
|
||||
# Fallback path: tests / fixtures that bypass build_pre_constructed
|
||||
# (for example, the existing test_az401_compose_root_replay.py suite
|
||||
# which seeds pre_constructed manually) still drive the wrapper
|
||||
# through build_state_estimator directly.
|
||||
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")
|
||||
@@ -345,9 +430,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).
|
||||
@@ -387,9 +470,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),
|
||||
(
|
||||
@@ -435,9 +516,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
|
||||
@@ -470,9 +549,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 "
|
||||
@@ -525,12 +602,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 "
|
||||
@@ -542,25 +616,575 @@ def _build_c7_inference(config: Config) -> Any:
|
||||
) from exc
|
||||
|
||||
|
||||
def _resolve_c3_matcher_strategy(config: Config) -> str:
|
||||
"""Return the configured C3 matcher strategy, defaulting to disk_lightglue.
|
||||
|
||||
Reuses :class:`gps_denied_onboard.components.c3_matcher.C3MatcherConfig`'s
|
||||
own default ``"disk_lightglue"`` when ``config.components`` carries no
|
||||
``c3_matcher`` block (early-bootstrap tests with bare ``Config()``).
|
||||
"""
|
||||
block = config.components.get("c3_matcher")
|
||||
if block is None:
|
||||
return "disk_lightglue"
|
||||
return getattr(block, "strategy", "disk_lightglue")
|
||||
|
||||
|
||||
def _resolve_c5_state_strategy(config: Config) -> str:
|
||||
"""Return the configured C5 state strategy, defaulting to gtsam_isam2.
|
||||
|
||||
Mirrors :func:`_resolve_c3_matcher_strategy` for the C5 slot.
|
||||
Reuses :class:`gps_denied_onboard.components.c5_state.config.C5StateConfig`'s
|
||||
own default ``"gtsam_isam2"`` when ``config.components`` carries no
|
||||
``c5_state`` block (early-bootstrap tests with bare ``Config()``).
|
||||
"""
|
||||
components = getattr(config, "components", None) or {}
|
||||
if not isinstance(components, Mapping):
|
||||
return "gtsam_isam2"
|
||||
block = components.get("c5_state")
|
||||
if block is None:
|
||||
return "gtsam_isam2"
|
||||
return getattr(block, "strategy", "gtsam_isam2")
|
||||
|
||||
|
||||
_C5_PREBUILT_ESTIMATOR_KEY: Final[str] = "_c5_prebuilt_estimator"
|
||||
"""Internal coordination key under which :func:`build_pre_constructed` stores
|
||||
the pre-built :class:`StateEstimator` instance.
|
||||
|
||||
The C5 state estimator and its :class:`ISam2GraphHandle` are constructed as
|
||||
a single tuple by :func:`build_state_estimator`; the handle is the iSAM2
|
||||
graph wrapper held INSIDE the estimator. C4 (``c4_pose``) reaches into
|
||||
``pre_constructed['c5_isam2_graph_handle']`` at compose time — but the C4
|
||||
wrapper runs BEFORE the C5 wrapper in topological order
|
||||
(``_C5_STATE_DEPENDS_ON: ('c1_vio', 'c4_pose')``). The handle therefore
|
||||
MUST exist in ``pre_constructed`` before either wrapper runs, which means
|
||||
the bootstrap MUST build the (estimator, handle) pair eagerly (AZ-625).
|
||||
|
||||
Storing the prebuilt estimator under this internal key lets the C5 wrapper
|
||||
short-circuit on it and return the SAME instance the handle was extracted
|
||||
from, so ``c4_pose._isam2_handle`` and ``c5_state._isam2_handle`` reference
|
||||
ONE object across the C4 / C5 seam (AC-625.3 identity contract).
|
||||
|
||||
Deliberately NOT in :data:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` — it is
|
||||
an internal coordination key, not a Public API surface that any component
|
||||
queries directly (only the C5 wrapper consults it, and only as a fast
|
||||
path).
|
||||
"""
|
||||
|
||||
|
||||
def _build_c5_state_estimator_pair(
|
||||
config: Config,
|
||||
*,
|
||||
imu_preintegrator: Any,
|
||||
se3_utils: Any,
|
||||
wgs_converter: Any,
|
||||
fdr_client: Any,
|
||||
tile_store: Any | None = None,
|
||||
camera_calibration: CameraCalibration | None = None,
|
||||
flight_id: str | None = None,
|
||||
companion_id: str | None = None,
|
||||
) -> tuple[Any, Any]:
|
||||
"""Build the ``(StateEstimator, ISam2GraphHandle)`` tuple eagerly.
|
||||
|
||||
The C5 estimator and its iSAM2 graph handle are produced together by
|
||||
:func:`gps_denied_onboard.runtime_root.state_factory.build_state_estimator`
|
||||
— the handle is the wrapper around the estimator's internal
|
||||
``_isam2`` + ``_smoother`` substrate (see
|
||||
:class:`gps_denied_onboard.components.c5_state._isam2_handle.\
|
||||
ISam2GraphHandleImpl`). The handle's constructor takes the estimator
|
||||
as input, so the two cannot be separately constructed without a
|
||||
Protocol-seam change in C5 — explicitly forbidden by the AZ-618
|
||||
umbrella's "MUST NOT touch any per-component factory signature"
|
||||
constraint.
|
||||
|
||||
Building the pair eagerly at bootstrap time is the AZ-625 fix: the
|
||||
handle reaches ``pre_constructed['c5_isam2_graph_handle']`` so
|
||||
:func:`compose_root` can satisfy C4's lookup in topological order
|
||||
(``c4_pose`` runs before ``c5_state``); the estimator reaches a
|
||||
private coordination slot (``_c5_prebuilt_estimator``) so
|
||||
:func:`_c5_state_wrapper` can short-circuit and return the SAME
|
||||
instance the handle is bound to. The cross-seam identity invariant
|
||||
is verified by AC-625.3.
|
||||
|
||||
Validation order matches the rest of the airborne bootstrap:
|
||||
|
||||
1. Resolve the configured C5 state strategy
|
||||
(default ``"gtsam_isam2"``).
|
||||
2. Look it up in :data:`C5_STATE_BUILD_FLAGS`. An unknown strategy
|
||||
is an :class:`AirborneBootstrapError` naming the supported set
|
||||
(AC-625.2).
|
||||
3. Read the gating ``BUILD_STATE_*`` flag with the SAME default
|
||||
ladder the state factory uses
|
||||
(:func:`os.environ.get(flag, "ON").upper() == "OFF"`); an
|
||||
explicit OFF raises :class:`AirborneBootstrapError` naming the
|
||||
flag and the consuming component slug ``c5_state`` (AC-625.2).
|
||||
4. Lazily register the strategy via
|
||||
:func:`_ensure_state_strategy_registered` — same hook the C5
|
||||
wrapper uses, so a binary configured for the ESKF baseline does
|
||||
not import gtsam at bootstrap time.
|
||||
5. Delegate to :func:`build_state_estimator` with the
|
||||
infrastructure kwargs the wrapper would have passed; surface
|
||||
any :class:`StateEstimatorConfigError` as an
|
||||
:class:`AirborneBootstrapError` so the operator-facing error
|
||||
contract is uniform.
|
||||
|
||||
The optional kwargs ``tile_store`` / ``camera_calibration`` /
|
||||
``flight_id`` / ``companion_id`` exist for AZ-389 orthorectifier
|
||||
wiring; they are forwarded to :func:`build_state_estimator` which
|
||||
only consumes them when ``c5_state.orthorectifier.enabled`` is
|
||||
True. Until AZ-624 wires the operator-supplied flight metadata
|
||||
into ``pre_constructed``, callers pass the available defaults
|
||||
(today: ``tile_store=constructed['c6_tile_store']``, the rest
|
||||
``None``).
|
||||
|
||||
Raises:
|
||||
AirborneBootstrapError: when the configured strategy is not in
|
||||
:data:`C5_STATE_BUILD_FLAGS`, when the strategy's
|
||||
``BUILD_STATE_*`` flag is OFF, or when
|
||||
:func:`build_state_estimator` itself rejects the
|
||||
configuration (the original
|
||||
:class:`StateEstimatorConfigError` is preserved as
|
||||
``__cause__``).
|
||||
"""
|
||||
from gps_denied_onboard.components.c5_state.errors import StateEstimatorConfigError
|
||||
|
||||
strategy = _resolve_c5_state_strategy(config)
|
||||
flag = C5_STATE_BUILD_FLAGS.get(strategy)
|
||||
if flag is None:
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c5_isam2_graph_handle'] because "
|
||||
f"config.components['c5_state'].strategy={strategy!r} is "
|
||||
f"not in the airborne BUILD-flag matrix "
|
||||
f"{sorted(C5_STATE_BUILD_FLAGS.keys())!r}. Consuming "
|
||||
f"component: c5_state. Reconfigure the C5 state strategy "
|
||||
f"to one of the supported strategies."
|
||||
)
|
||||
# Mirror state_factory._STATE_BUILD_FLAGS gate: default "ON" when
|
||||
# unset; only explicit "OFF" blocks. Keeping the default identical
|
||||
# to state_factory means AZ-625's pre-check fires before
|
||||
# build_state_estimator's own gate, so the operator sees the
|
||||
# bootstrap-error contract instead of the lower-level config error.
|
||||
if os.environ.get(flag, "ON").upper() == "OFF":
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c5_isam2_graph_handle'] because the "
|
||||
f"gating flag {flag}=ON is required for the configured "
|
||||
f"strategy={strategy!r}, but {flag} is OFF in this binary. "
|
||||
f"Consuming component: c5_state. Set {flag}=ON, or "
|
||||
f"reconfigure config.components['c5_state'].strategy to a "
|
||||
f"strategy whose BUILD_STATE_* flag is ON."
|
||||
)
|
||||
_ensure_state_strategy_registered(config)
|
||||
try:
|
||||
estimator, handle = build_state_estimator(
|
||||
config,
|
||||
imu_preintegrator=imu_preintegrator,
|
||||
se3_utils=se3_utils,
|
||||
wgs_converter=wgs_converter,
|
||||
fdr_client=fdr_client,
|
||||
tile_store=tile_store,
|
||||
camera_calibration=camera_calibration,
|
||||
flight_id=flight_id,
|
||||
companion_id=companion_id,
|
||||
)
|
||||
except StateEstimatorConfigError as exc:
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c5_isam2_graph_handle'] for "
|
||||
f"strategy={strategy!r} (gating flag {flag} is ON). "
|
||||
f"Consuming component: c5_state. Upstream error: {exc}"
|
||||
) from exc
|
||||
return estimator, handle
|
||||
|
||||
|
||||
def _is_build_flag_on(flag_name: str) -> bool:
|
||||
"""Read a compile-time ``BUILD_*`` flag from the environment.
|
||||
|
||||
Mirrors the same predicate used by
|
||||
:func:`gps_denied_onboard.runtime_root.matcher_factory.\
|
||||
_is_build_flag_on` — ``ON`` / ``1`` / ``true`` / ``yes`` (case-insensitive)
|
||||
is ON; everything else (including unset) is OFF. Defined locally so the
|
||||
bootstrap does not depend on the matcher-factory's private helper.
|
||||
"""
|
||||
raw = os.environ.get(flag_name, "")
|
||||
return raw.strip().lower() in {"on", "1", "true", "yes"}
|
||||
|
||||
|
||||
def _load_lightglue_engine_handle(
|
||||
config: Config, inference_runtime: InferenceRuntime
|
||||
) -> EngineHandle:
|
||||
"""Production loader for the shared LightGlue matcher engine.
|
||||
|
||||
Reads ``config.components['c3_matcher'].lightglue_weights_path``,
|
||||
compiles the engine via the C7
|
||||
:class:`InferenceRuntime.compile_engine` (TensorRT or PyTorch-FP16
|
||||
per AZ-621), then deserialises it into an opaque
|
||||
:class:`EngineHandle`. The handle's lifecycle is owned by the
|
||||
:class:`LightGlueRuntime` instance returned by
|
||||
:func:`_build_c3_lightglue_runtime`.
|
||||
|
||||
AZ-622 unit tests monkey-patch this function with a sentinel
|
||||
:class:`EngineHandle`-shaped mock so they can exercise the
|
||||
LightGlueRuntime wiring without standing up a real GPU + TensorRT
|
||||
toolchain (per AZ-622 ``Tier-2 Note``: real LightGlue inference
|
||||
correctness is verified by AZ-624's Jetson AC-5 run).
|
||||
|
||||
Raises:
|
||||
AirborneBootstrapError: if ``c3_matcher.lightglue_weights_path``
|
||||
is ``None`` (the operator-actionable message points at the
|
||||
production main() wiring task — AZ-624).
|
||||
RuntimeNotAvailableError: if the underlying
|
||||
:func:`InferenceRuntime.compile_engine` /
|
||||
:func:`deserialize_engine` paths fail (caller wraps this
|
||||
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
|
||||
if weights_path is None:
|
||||
raise AirborneBootstrapError(
|
||||
"airborne_bootstrap: cannot construct "
|
||||
"pre_constructed['c3_lightglue_runtime'] because "
|
||||
"config.components['c3_matcher'].lightglue_weights_path "
|
||||
"is None. Production main() (AZ-624) must populate the "
|
||||
"path to the compiled LightGlue engine before calling "
|
||||
"build_pre_constructed; tests stub _load_lightglue_engine_handle "
|
||||
"via monkeypatch."
|
||||
)
|
||||
from gps_denied_onboard._types.inference import BuildConfig, PrecisionMode
|
||||
|
||||
build_config: BuildConfig = BuildConfig(
|
||||
precision=PrecisionMode.FP16,
|
||||
workspace_mb=512,
|
||||
calibration_dataset=None,
|
||||
optimization_profiles=(),
|
||||
)
|
||||
cache_entry = inference_runtime.compile_engine(weights_path, build_config)
|
||||
return inference_runtime.deserialize_engine(cache_entry)
|
||||
|
||||
|
||||
def _build_c3_lightglue_runtime(
|
||||
config: Config, *, inference_runtime: InferenceRuntime
|
||||
) -> LightGlueRuntime:
|
||||
"""Build ``pre_constructed['c3_lightglue_runtime']`` for the airborne binary.
|
||||
|
||||
1. Resolve the configured C3 matcher strategy (default
|
||||
``disk_lightglue``).
|
||||
2. Look up the gating flag in :data:`C3_MATCHER_BUILD_FLAGS`.
|
||||
An unknown strategy or an OFF flag is an
|
||||
:class:`AirborneBootstrapError` naming the missing flag and
|
||||
the consuming component slug ``c3_matcher`` (AC-622.2).
|
||||
3. Load the LightGlue engine handle via
|
||||
:func:`_load_lightglue_engine_handle` (the heavy seam unit
|
||||
tests monkeypatch — see AZ-622 ``Tier-2 Note``).
|
||||
4. Wrap the handle in :class:`LightGlueRuntime` and return.
|
||||
|
||||
The returned runtime is the SAME instance that the wrappers for
|
||||
``c3_matcher`` and ``c2_5_rerank`` extract from
|
||||
``pre_constructed['c3_lightglue_runtime']`` — identity-share is
|
||||
the structural R14 fix (avoids double GPU memory; AZ-344 AC-10).
|
||||
The cross-component identity-share assertion is verified at
|
||||
AZ-624's integration AC, not here.
|
||||
|
||||
Raises:
|
||||
AirborneBootstrapError: when the configured strategy's
|
||||
``BUILD_MATCHER_*`` flag is OFF, when the strategy is
|
||||
unknown to :data:`C3_MATCHER_BUILD_FLAGS`, or when the
|
||||
heavy seam fails (the upstream
|
||||
:class:`RuntimeNotAvailableError` is preserved as
|
||||
``__cause__``).
|
||||
"""
|
||||
strategy = _resolve_c3_matcher_strategy(config)
|
||||
flag = C3_MATCHER_BUILD_FLAGS.get(strategy)
|
||||
if flag is None:
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c3_lightglue_runtime'] because "
|
||||
f"config.components['c3_matcher'].strategy={strategy!r} is "
|
||||
f"not in the airborne BUILD-flag matrix "
|
||||
f"{sorted(C3_MATCHER_BUILD_FLAGS.keys())!r}. Consuming "
|
||||
f"component: c3_matcher. Reconfigure the C3 matcher to "
|
||||
f"select one of the supported strategies."
|
||||
)
|
||||
if not _is_build_flag_on(flag):
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c3_lightglue_runtime'] because the "
|
||||
f"gating flag {flag}=ON is required for the configured "
|
||||
f"strategy={strategy!r}, but {flag} is OFF in this binary. "
|
||||
f"Consuming component: c3_matcher. Set {flag}=ON, or "
|
||||
f"reconfigure config.components['c3_matcher'].strategy to a "
|
||||
f"strategy whose BUILD_MATCHER_* flag is ON."
|
||||
)
|
||||
try:
|
||||
engine_handle = _load_lightglue_engine_handle(config, inference_runtime)
|
||||
except RuntimeNotAvailableError as exc:
|
||||
raise AirborneBootstrapError(
|
||||
f"airborne_bootstrap: cannot construct "
|
||||
f"pre_constructed['c3_lightglue_runtime'] because the "
|
||||
f"LightGlue engine load failed for strategy={strategy!r} "
|
||||
f"(gating flag {flag} is ON). Consuming component: "
|
||||
f"c3_matcher. Upstream error: {exc}"
|
||||
) from exc
|
||||
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.
|
||||
|
||||
Returns the shared :class:`FeatureExtractor` that C2.5's
|
||||
:class:`InlierCountReRanker` consumes for both per-frame nav-camera
|
||||
images and per-candidate tile pixels (per
|
||||
:class:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`'s ``c2_5_rerank``
|
||||
row). The L1 helper module
|
||||
:mod:`gps_denied_onboard.helpers.feature_extractor` documents
|
||||
:class:`OpenCvOrbExtractor` as the production-ready airborne
|
||||
placeholder until the C7 ``InferenceRuntime``-backed DISK / ALIKED
|
||||
feature extractor lands; AZ-622 wires that placeholder in.
|
||||
|
||||
No ``BUILD_*`` flag check applies here: the C3 matcher's per-strategy
|
||||
flag matrix gates the *matcher* engine (handled by
|
||||
:func:`_build_c3_lightglue_runtime`); the *feature extractor* is
|
||||
consumed by C2.5 (not C3) and is a pure-CPU OpenCV path that has no
|
||||
compile-time gate of its own.
|
||||
|
||||
The ``config`` argument is accepted for symmetry with the other
|
||||
bootstrap builders and to keep the door open for a future
|
||||
config-driven feature-extractor selection (DISK / ALIKED swap-in)
|
||||
without changing the call site in :func:`build_pre_constructed`.
|
||||
"""
|
||||
del config # currently no config knobs; placeholder for future selection
|
||||
return OpenCvOrbExtractor()
|
||||
|
||||
|
||||
def build_pre_constructed(config: Config) -> dict[str, Any]:
|
||||
"""Build the airborne ``pre_constructed`` dict for :func:`compose_root`.
|
||||
|
||||
AZ-619 (Phase A) seeded ``c13_fdr`` and ``clock``. AZ-620 (Phase B)
|
||||
added the two C6 storage entries (``c6_descriptor_index`` +
|
||||
``c6_tile_store``). AZ-621 (Phase C) adds ``c7_inference``
|
||||
``c6_tile_store``). AZ-621 (Phase C) added ``c7_inference``
|
||||
(PyTorch FP16 vs. TensorRT, gated by
|
||||
:data:`C7_AIRBORNE_BUILD_FLAGS`). Phases D..F (AZ-622..AZ-624) will
|
||||
extend this function to populate the remaining keys in
|
||||
:data:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`.
|
||||
: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). AZ-623 (Phase E) added 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`).
|
||||
AZ-625 (Phase E.5) adds ``c5_isam2_graph_handle`` and seeds an
|
||||
internal coordination key (``_c5_prebuilt_estimator``) by
|
||||
eagerly invoking :func:`build_state_estimator` once at bootstrap
|
||||
time and capturing the
|
||||
``(StateEstimator, ISam2GraphHandle)`` tuple — the handle reaches
|
||||
``c4_pose`` via ``pre_constructed`` (C4 runs before C5 in topo
|
||||
order), and the prebuilt estimator lets the C5 wrapper
|
||||
short-circuit without re-invoking the factory. Phase F (AZ-624)
|
||||
will wire ``runtime_root.main()`` and verify AC-1..AC-5
|
||||
end-to-end.
|
||||
|
||||
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
|
||||
the same process return dicts where ``pre_constructed['c13_fdr']`` is
|
||||
the SAME object — AC-619.2. ``clock`` is a fresh :class:`WallClock`
|
||||
each call (stateless; the cache would be a no-op). The C6 + C7
|
||||
entries are constructed via the existing :mod:`storage_factory` and
|
||||
:mod:`inference_factory` builders without additional caching at this
|
||||
layer.
|
||||
each call (stateless; the cache would be a no-op). The C6, C7, and C3
|
||||
entries are constructed via the existing :mod:`storage_factory`,
|
||||
:mod:`inference_factory`, and helper modules without additional
|
||||
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. 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
|
||||
@@ -575,16 +1199,45 @@ def build_pre_constructed(config: Config) -> dict[str, Any]:
|
||||
runtime is buildable (both ``BUILD_TENSORRT_RUNTIME`` and
|
||||
``BUILD_PYTORCH_FP16_RUNTIME`` OFF, or the configured
|
||||
runtime's matching flag is OFF) and any configured consumer
|
||||
requires ``c7_inference``. The message names the consuming
|
||||
component slug(s) and the relevant gating flag(s).
|
||||
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; OR (AZ-623) if ``config.runtime.camera_calibration_path``
|
||||
is empty / unreadable / malformed JSON, blocking the
|
||||
``c5_imu_preintegrator`` build; OR (AZ-625) if the
|
||||
configured C5 state strategy's
|
||||
:data:`C5_STATE_BUILD_FLAGS` flag is OFF (or the strategy
|
||||
is unknown), or if :func:`build_state_estimator` itself
|
||||
rejects the configuration when the
|
||||
``(StateEstimator, ISam2GraphHandle)`` pair is built
|
||||
eagerly. The message names the consuming component slug(s)
|
||||
and the relevant gating flag(s) or missing inputs.
|
||||
"""
|
||||
return {
|
||||
"c13_fdr": make_fdr_client(AIRBORNE_MAIN_PRODUCER_ID, config),
|
||||
"clock": WallClock(),
|
||||
"c6_descriptor_index": _build_c6_descriptor_index(config),
|
||||
"c6_tile_store": _build_c6_tile_store(config),
|
||||
"c7_inference": _build_c7_inference(config),
|
||||
}
|
||||
constructed: dict[str, Any] = {}
|
||||
constructed["c13_fdr"] = make_fdr_client(AIRBORNE_MAIN_PRODUCER_ID, config)
|
||||
constructed["clock"] = WallClock()
|
||||
constructed["c6_descriptor_index"] = _build_c6_descriptor_index(config)
|
||||
constructed["c6_tile_store"] = _build_c6_tile_store(config)
|
||||
constructed["c7_inference"] = _build_c7_inference(config)
|
||||
constructed["c3_lightglue_runtime"] = _build_c3_lightglue_runtime(
|
||||
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)
|
||||
estimator, handle = _build_c5_state_estimator_pair(
|
||||
config,
|
||||
imu_preintegrator=constructed["c5_imu_preintegrator"],
|
||||
se3_utils=constructed["c5_se3_utils"],
|
||||
wgs_converter=constructed["c5_wgs_converter"],
|
||||
fdr_client=constructed["c13_fdr"],
|
||||
tile_store=constructed["c6_tile_store"],
|
||||
)
|
||||
constructed["c5_isam2_graph_handle"] = handle
|
||||
constructed[_C5_PREBUILT_ESTIMATOR_KEY] = estimator
|
||||
return constructed
|
||||
|
||||
|
||||
def register_airborne_strategies() -> None:
|
||||
@@ -628,8 +1281,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
|
||||
),
|
||||
},
|
||||
},
|
||||
|
||||
@@ -0,0 +1,483 @@
|
||||
"""AZ-624 / AZ-618 — umbrella ACs for ``runtime_root.main()`` + ``build_pre_constructed`` wiring.
|
||||
|
||||
Verifies the full AZ-618 acceptance suite (AC-1..AC-4) at the
|
||||
integration seam, plus AZ-624's local ACs:
|
||||
|
||||
* AC-618-1: ``build_pre_constructed(config)`` returns a dict containing
|
||||
every key in :data:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS`
|
||||
flattened (no key maps to ``None``).
|
||||
* AC-618-2: ``compose_root(config, pre_constructed=...)`` reaches
|
||||
takeoff and returns a :class:`RuntimeRoot` whose ``components`` dict
|
||||
contains all 7 strategy-selecting slots (c1_vio, c2_vpr, c2_5_rerank,
|
||||
c3_matcher, c3_5_adhop, c4_pose, c5_state) without raising
|
||||
:class:`AirborneBootstrapError`.
|
||||
* AC-618-3: a ``BUILD_*`` flag mismatch surfaces an
|
||||
:class:`AirborneBootstrapError` whose message names both the
|
||||
missing infrastructure key and the gating ``BUILD_*`` flag plus the
|
||||
consuming component slug.
|
||||
* AC-618-4: ``runtime_root.main()`` returns ``0`` (success) when all
|
||||
infra deps resolve; returns :data:`EXIT_GENERIC_FAILURE` (``1``) and
|
||||
stderr contains the ``airborne_bootstrap:`` prefix when a single
|
||||
required infra dep is forcibly unavailable.
|
||||
|
||||
AC-618-5 (Jetson tier-2 e2e replay run) is verified out-of-band on
|
||||
real hardware via ``scripts/run-tests-jetson.sh
|
||||
tests/e2e/replay/test_derkachi_1min.py``; this unit-test file owns
|
||||
only AC-1..AC-4.
|
||||
|
||||
The phase-specific tests (``test_az619..test_az625``) already cover
|
||||
each builder's contract in isolation. This file owns the integrated
|
||||
contract: every phase builds something usable + the assembly composes.
|
||||
|
||||
The heavy-builder stubs are NOT autouse: tests that need to exercise
|
||||
the real builder path (AC-618-3 against the C7 factory's flag-OFF
|
||||
branch) opt out by simply not calling :func:`_stub_heavy_builders`.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterator
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from gps_denied_onboard.config import Config
|
||||
from gps_denied_onboard.fdr_client import client as fdr_client_module
|
||||
from gps_denied_onboard.runtime_root import (
|
||||
EXIT_GENERIC_FAILURE,
|
||||
RuntimeRoot,
|
||||
airborne_bootstrap,
|
||||
clear_strategy_registry,
|
||||
compose_root,
|
||||
main,
|
||||
)
|
||||
from gps_denied_onboard.runtime_root.airborne_bootstrap import (
|
||||
AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS,
|
||||
AirborneBootstrapError,
|
||||
build_pre_constructed,
|
||||
clear_imu_preintegrator_cache,
|
||||
register_airborne_strategies,
|
||||
)
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# Shared fixtures
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolated_caches() -> Iterator[None]:
|
||||
# Arrange: every test starts with empty FdrClient + ImuPreintegrator
|
||||
# caches + an empty strategy registry so the AZ-624 assertions are
|
||||
# exercised against fresh state rather than carry-over from prior
|
||||
# runs in the same pytest process.
|
||||
fdr_client_module._reset_for_tests()
|
||||
clear_imu_preintegrator_cache()
|
||||
clear_strategy_registry()
|
||||
yield
|
||||
fdr_client_module._reset_for_tests()
|
||||
clear_imu_preintegrator_cache()
|
||||
clear_strategy_registry()
|
||||
|
||||
|
||||
def _stub_heavy_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Replace each heavy AZ-619..AZ-625 builder with an opaque sentinel.
|
||||
|
||||
Called explicitly from each test that needs the integrated
|
||||
``build_pre_constructed`` to succeed without standing up FAISS,
|
||||
TensorRT, PyTorch, OpenCV, or gtsam at unit-test scope. The
|
||||
AC-618-3 path deliberately skips this so the real C7 builder's
|
||||
``BUILD_*`` flag-OFF branch fires.
|
||||
"""
|
||||
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"),
|
||||
)
|
||||
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"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (
|
||||
MagicMock(name="StateEstimator"),
|
||||
MagicMock(name="ISam2GraphHandle"),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _airborne_env(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Set the env vars compose_root requires for the airborne binary.
|
||||
|
||||
Mirrors ``tests/unit/test_az401_compose_root_replay.py``'s
|
||||
``_airborne_live_env`` fixture.
|
||||
"""
|
||||
for name, value in (
|
||||
("GPS_DENIED_FC_PROFILE", "ardupilot_plane"),
|
||||
("GPS_DENIED_TIER", "1"),
|
||||
("DB_URL", "postgresql+psycopg://gps_denied:dev@db:5432/gps_denied"),
|
||||
("CAMERA_CALIBRATION_PATH", "/etc/gps-denied/calib.yml"),
|
||||
("LOG_LEVEL", "INFO"),
|
||||
("LOG_SINK", "console"),
|
||||
("INFERENCE_BACKEND", "pytorch_fp16"),
|
||||
("FDR_PATH", "/var/lib/gps-denied/fdr"),
|
||||
("TILE_CACHE_PATH", "/var/lib/gps-denied/tiles"),
|
||||
("MAVLINK_SIGNING_KEY", "ZZZZZZZZ"),
|
||||
):
|
||||
monkeypatch.setenv(name, value)
|
||||
|
||||
|
||||
def _all_seven_components_config() -> Config:
|
||||
"""Return a :class:`Config` whose ``components`` selects every default strategy.
|
||||
|
||||
Each block uses the ``mapping``-with-``"strategy"``-key shape that
|
||||
:func:`runtime_root._resolve_component_strategies` accepts as a
|
||||
fallback for raw YAML — keeps the test free of seven imports of
|
||||
per-component config block dataclasses.
|
||||
"""
|
||||
return Config.with_blocks(
|
||||
c1_vio={"strategy": "klt_ransac"},
|
||||
c2_vpr={"strategy": "ultra_vpr"},
|
||||
c2_5_rerank={"strategy": "inlier_count"},
|
||||
c3_matcher={"strategy": "disk_lightglue"},
|
||||
c3_5_adhop={"strategy": "adhop"},
|
||||
c4_pose={"strategy": "opencv_gtsam"},
|
||||
c5_state={"strategy": "gtsam_isam2"},
|
||||
)
|
||||
|
||||
|
||||
def _stub_strategy_factories(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Stub each per-component factory the airborne wrappers delegate to.
|
||||
|
||||
The wrappers in ``airborne_bootstrap._c{N}_*_wrapper`` import each
|
||||
factory by name from its sibling ``runtime_root.*_factory`` module.
|
||||
Replacing those names in ``airborne_bootstrap``'s own namespace
|
||||
makes the wrappers hand back deterministic sentinels instead of
|
||||
constructing real OpenCV / gtsam / TensorRT objects.
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_vio_strategy",
|
||||
lambda _config, **_kwargs: MagicMock(name="VioStrategy"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_vpr_strategy",
|
||||
lambda _config, **_kwargs: MagicMock(name="VprStrategy"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_rerank_strategy",
|
||||
lambda _config, **_kwargs: MagicMock(name="RerankStrategy"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_matcher_strategy",
|
||||
lambda _config, **_kwargs: MagicMock(name="MatcherStrategy"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_refiner_strategy",
|
||||
lambda _config, **_kwargs: MagicMock(name="RefinerStrategy"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_pose_estimator",
|
||||
lambda _config, **_kwargs: MagicMock(name="PoseEstimator"),
|
||||
)
|
||||
# AZ-625 short-circuits the C5 wrapper on _c5_prebuilt_estimator,
|
||||
# so build_state_estimator is never reached on the success path.
|
||||
# The _stub_heavy_builders stub already seeds a MagicMock estimator
|
||||
# under the look-aside key.
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# AC-618-1
|
||||
|
||||
|
||||
def test_ac_618_1_build_pre_constructed_populates_every_required_key(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
_stub_heavy_builders(monkeypatch)
|
||||
config = Config()
|
||||
expected_keys: set[str] = set().union(*AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS.values())
|
||||
|
||||
# Act
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Assert: every documented public key is present and non-None.
|
||||
missing = expected_keys - pre_constructed.keys()
|
||||
assert not missing, (
|
||||
f"build_pre_constructed must populate every key in "
|
||||
f"AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS; missing: {sorted(missing)}"
|
||||
)
|
||||
for key in expected_keys:
|
||||
assert pre_constructed[key] is not None, (
|
||||
f"pre_constructed[{key!r}] is None; the AC-618-1 contract "
|
||||
f"requires every required key to map to a real instance"
|
||||
)
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# AC-618-2
|
||||
|
||||
|
||||
def test_ac_618_2_compose_root_reaches_takeoff_with_all_seven_slots(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
_stub_heavy_builders(monkeypatch)
|
||||
_stub_strategy_factories(monkeypatch)
|
||||
register_airborne_strategies()
|
||||
config = _all_seven_components_config()
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Act
|
||||
runtime = compose_root(config, pre_constructed=pre_constructed)
|
||||
|
||||
# Assert: every strategy-selecting slot produced a component (via
|
||||
# the stubbed factory). compose_root returns
|
||||
# registry-built-only components in `RuntimeRoot.components` for
|
||||
# live mode (replay components would be merged in by replay-branch).
|
||||
assert isinstance(runtime, RuntimeRoot)
|
||||
expected_slots: set[str] = {
|
||||
"c1_vio",
|
||||
"c2_vpr",
|
||||
"c2_5_rerank",
|
||||
"c3_matcher",
|
||||
"c3_5_adhop",
|
||||
"c4_pose",
|
||||
"c5_state",
|
||||
}
|
||||
missing_slots = expected_slots - runtime.components.keys()
|
||||
assert not missing_slots, (
|
||||
f"compose_root must populate every registered airborne slot; "
|
||||
f"missing: {sorted(missing_slots)}"
|
||||
)
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# AC-618-3
|
||||
|
||||
|
||||
def test_ac_618_3_build_flag_off_raises_named_error_with_consuming_component(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When the C7 inference factory's gating flags are OFF and a
|
||||
consumer is configured, ``build_pre_constructed`` must raise
|
||||
:class:`AirborneBootstrapError` whose message names BOTH the
|
||||
missing infrastructure key (``c7_inference``) AND the airborne
|
||||
BUILD_* flags (``BUILD_TENSORRT_RUNTIME``,
|
||||
``BUILD_PYTORCH_FP16_RUNTIME``) AND the consuming component slug
|
||||
(``c2_vpr``).
|
||||
|
||||
Stubs only the upstream builders the C7 factory does not depend
|
||||
on — leaves the real ``_build_c7_inference`` in place so the
|
||||
production C7 ``RuntimeNotAvailableError`` path fires. Stubs the
|
||||
underlying :func:`build_inference_runtime` factory to simulate the
|
||||
flag-OFF condition without depending on env-var defaults.
|
||||
"""
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
# Stub upstream builders the C6 / FdrClient layer needs so we
|
||||
# reach the C7 builder in normal sequence; do NOT stub the C7
|
||||
# builder itself — that's the path under test.
|
||||
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"),
|
||||
)
|
||||
# Force the C7 factory's "no airborne runtime buildable" branch via
|
||||
# the same RuntimeNotAvailableError shape it raises in production
|
||||
# when both flags resolve to OFF.
|
||||
from gps_denied_onboard.runtime_root.errors import RuntimeNotAvailableError
|
||||
|
||||
def _raise_no_airborne_c7_runtime(_config: Any) -> Any:
|
||||
raise RuntimeNotAvailableError(
|
||||
"no airborne C7 inference runtime is buildable; both "
|
||||
"BUILD_TENSORRT_RUNTIME and BUILD_PYTORCH_FP16_RUNTIME are OFF"
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_inference_runtime",
|
||||
_raise_no_airborne_c7_runtime,
|
||||
)
|
||||
config = Config.with_blocks(c2_vpr={"strategy": "net_vlad"})
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c7_inference" in message
|
||||
# Both airborne C7 flags must be named in the operator-facing error
|
||||
# (per :data:`C7_AIRBORNE_BUILD_FLAGS` — the operator sees both the
|
||||
# production-default and the Tier-0 fallback options).
|
||||
assert "BUILD_TENSORRT_RUNTIME" in message
|
||||
assert "BUILD_PYTORCH_FP16_RUNTIME" in message
|
||||
# The consuming component is named so the operator knows which
|
||||
# config block to revisit.
|
||||
assert "c2_vpr" in message
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# AC-618-4
|
||||
|
||||
|
||||
def test_ac_618_4_main_returns_zero_on_success(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
_stub_heavy_builders(monkeypatch)
|
||||
_stub_strategy_factories(monkeypatch)
|
||||
config = _all_seven_components_config()
|
||||
|
||||
# Act: main() drives register_airborne_strategies +
|
||||
# build_pre_constructed + compose_root in sequence. The stubs
|
||||
# cover the heavy seams.
|
||||
exit_code = main(config)
|
||||
|
||||
# Assert
|
||||
assert exit_code == 0
|
||||
# The success path writes nothing to stderr (operator contract:
|
||||
# stderr is ONLY the failure surface for runtime_root).
|
||||
captured = capsys.readouterr()
|
||||
assert "airborne_bootstrap:" not in captured.err
|
||||
|
||||
|
||||
def test_ac_618_4_main_returns_generic_failure_with_airborne_bootstrap_prefix(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
"""Force one infra builder to raise :class:`AirborneBootstrapError` and
|
||||
assert main() surfaces it cleanly:
|
||||
|
||||
* exit code is :data:`EXIT_GENERIC_FAILURE` (the AC-618-4 contract).
|
||||
* stderr contains the ``airborne_bootstrap:`` prefix the bootstrap
|
||||
error message itself emits — confirms main() now catches
|
||||
:class:`AirborneBootstrapError` distinctly (rather than letting
|
||||
the broader :class:`RuntimeError` clause hide the prefix in a
|
||||
generic backtrace).
|
||||
"""
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
_stub_heavy_builders(monkeypatch)
|
||||
_stub_strategy_factories(monkeypatch)
|
||||
|
||||
def _raise_named_bootstrap_error(_config: Any) -> Any:
|
||||
raise AirborneBootstrapError(
|
||||
"airborne_bootstrap: cannot construct "
|
||||
"pre_constructed['c6_descriptor_index'] because "
|
||||
"BUILD_FAISS_INDEX is OFF (forced via test). "
|
||||
"Consuming component: c2_vpr."
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c6_descriptor_index",
|
||||
_raise_named_bootstrap_error,
|
||||
)
|
||||
config = _all_seven_components_config()
|
||||
|
||||
# Act
|
||||
exit_code = main(config)
|
||||
|
||||
# Assert
|
||||
assert exit_code == EXIT_GENERIC_FAILURE
|
||||
captured = capsys.readouterr()
|
||||
assert "airborne_bootstrap:" in captured.err, (
|
||||
"main() must surface AirborneBootstrapError messages with the "
|
||||
"documented prefix to stderr; got: " + repr(captured.err)
|
||||
)
|
||||
assert "c6_descriptor_index" in captured.err
|
||||
assert "c2_vpr" in captured.err
|
||||
|
||||
|
||||
def test_ac_624_local_main_distinct_handler_does_not_double_print(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
"""Regression guard: after AZ-624 added a dedicated
|
||||
:class:`AirborneBootstrapError` handler before the broader
|
||||
``(ConfigurationError, StrategyNotLinkedError, RuntimeError)``
|
||||
clause, the same exception must NOT also fall through to that
|
||||
second handler (which would emit a duplicate stderr line).
|
||||
Verifies the catch ordering is a strict short-circuit.
|
||||
"""
|
||||
# Arrange
|
||||
_airborne_env(monkeypatch)
|
||||
_stub_heavy_builders(monkeypatch)
|
||||
_stub_strategy_factories(monkeypatch)
|
||||
|
||||
def _raise_bootstrap_error(_config: Any) -> Any:
|
||||
raise AirborneBootstrapError(
|
||||
"airborne_bootstrap: cannot construct "
|
||||
"pre_constructed['c5_imu_preintegrator'] because forced via test. "
|
||||
"Consuming component: c5_state."
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_imu_preintegrator",
|
||||
_raise_bootstrap_error,
|
||||
)
|
||||
config = _all_seven_components_config()
|
||||
|
||||
# Act
|
||||
exit_code = main(config)
|
||||
captured = capsys.readouterr()
|
||||
|
||||
# Assert
|
||||
assert exit_code == EXIT_GENERIC_FAILURE
|
||||
# Exactly ONE "runtime_root: airborne_bootstrap:" line — the
|
||||
# dedicated handler short-circuits before the RuntimeError clause.
|
||||
occurrences = captured.err.count("runtime_root: airborne_bootstrap:")
|
||||
assert occurrences == 1, (
|
||||
f"Expected exactly one runtime_root: airborne_bootstrap: line in "
|
||||
f"stderr, got {occurrences}. Full stderr: {captured.err!r}"
|
||||
)
|
||||
@@ -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,8 +71,45 @@ 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())
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Arrange: stub the AZ-622 Phase D C3 matcher builders so the AZ-619
|
||||
# tests stay focused on Phase A. Without this the bare Config() below
|
||||
# would hit the BUILD_MATCHER_DISK_LIGHTGLUE flag check (typically
|
||||
# unset in the test env) 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_c7_inference", lambda _config: object()
|
||||
airborne_bootstrap,
|
||||
"_build_c3_lightglue_runtime",
|
||||
lambda _config, *, inference_runtime: 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())
|
||||
# AZ-625 Phase E.5: the C5 (estimator, handle) pair builder is also
|
||||
# stubbed so the bare Config() bootstrap path doesn't trip on the
|
||||
# default gtsam_isam2 strategy needing a real registered factory.
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (object(), object()),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -74,6 +74,69 @@ def _stub_c7_inference_builder(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Arrange: stub the AZ-622 Phase D C3 matcher builders so AZ-620
|
||||
# tests stay focused on the Phase B contract. Without this the
|
||||
# configs used below would hit the BUILD_MATCHER_DISK_LIGHTGLUE flag
|
||||
# check (typically unset in the test env) and raise
|
||||
# AirborneBootstrapError before the AC-620 keys could be asserted.
|
||||
# Sentinels are opaque on purpose — AZ-620 assertions never inspect
|
||||
# them.
|
||||
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"),
|
||||
)
|
||||
|
||||
|
||||
@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"),
|
||||
)
|
||||
# AZ-625 Phase E.5: stub the C5 (estimator, handle) pair builder so
|
||||
# the bare Config() bootstrap path doesn't trip on the default
|
||||
# gtsam_isam2 strategy needing a real registered factory.
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (
|
||||
MagicMock(name="StateEstimator"),
|
||||
MagicMock(name="ISam2GraphHandle"),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def test_ac_620_1_adds_c6_descriptor_index_and_c6_tile_store(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
||||
@@ -77,6 +77,69 @@ def _stub_c6_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stub_c3_matcher_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Arrange: stub the AZ-622 Phase D C3 matcher builders so AZ-621
|
||||
# tests stay focused on the Phase C contract. Without this the
|
||||
# configs used below would hit the BUILD_MATCHER_DISK_LIGHTGLUE flag
|
||||
# check (typically unset in the test env) 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_c3_lightglue_runtime",
|
||||
lambda _config, *, inference_runtime: MagicMock(name="LightGlueRuntime"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c3_feature_extractor",
|
||||
lambda _config: MagicMock(name="FeatureExtractor"),
|
||||
)
|
||||
|
||||
|
||||
@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"),
|
||||
)
|
||||
# AZ-625 Phase E.5: stub the C5 (estimator, handle) pair builder so
|
||||
# the bare Config() bootstrap path doesn't trip on the default
|
||||
# gtsam_isam2 strategy needing a real registered factory.
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (
|
||||
MagicMock(name="StateEstimator"),
|
||||
MagicMock(name="ISam2GraphHandle"),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
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.
|
||||
@@ -112,9 +175,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:
|
||||
|
||||
@@ -0,0 +1,323 @@
|
||||
"""AZ-622 — Phase D of AZ-618: ``build_pre_constructed`` seeds c3_lightglue_runtime + c3_feature_extractor.
|
||||
|
||||
Verifies the contract at
|
||||
``_docs/02_tasks/todo/AZ-622_pre_constructed_phase_d_c3_runtimes.md``:
|
||||
|
||||
* AC-622.1: with a default airborne ``Config`` (and the
|
||||
``BUILD_MATCHER_DISK_LIGHTGLUE`` flag ON in spirit — here stubbed at
|
||||
the airborne_bootstrap module boundary), ``build_pre_constructed(config)``
|
||||
additionally contains ``c3_lightglue_runtime`` (a
|
||||
:class:`LightGlueRuntime` instance) AND ``c3_feature_extractor`` (a
|
||||
:class:`FeatureExtractor` instance) on top of AZ-619 + AZ-620 + AZ-621.
|
||||
* AC-622.2: when ``BUILD_MATCHER_DISK_LIGHTGLUE=OFF`` AND a config
|
||||
selects ``c3_matcher.strategy="disk_lightglue"``, ``build_pre_constructed``
|
||||
raises :class:`AirborneBootstrapError` whose message names
|
||||
``c3_lightglue_runtime`` (the missing key), ``BUILD_MATCHER_DISK_LIGHTGLUE``
|
||||
(the gating flag), and ``c3_matcher`` (the consuming component slug).
|
||||
|
||||
AC-622.3 (this file exists with the above tests) is satisfied by the
|
||||
existence of this module.
|
||||
|
||||
The tests stub the heavy ``_load_lightglue_engine_handle`` seam (AZ-622)
|
||||
plus the upstream ``_build_c6_*`` (AZ-620) and ``_build_c7_inference``
|
||||
(AZ-621) factories at the airborne_bootstrap module boundary — exactly
|
||||
mirroring the prior phase pattern (see
|
||||
:mod:`tests.unit.runtime_root.test_az621_pre_constructed_phase_c`). Real
|
||||
LightGlue inference correctness is verified by AZ-624's Jetson AC-5 run
|
||||
per the AZ-622 ``Tier-2 Note``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterator
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from gps_denied_onboard.config import Config
|
||||
from gps_denied_onboard.fdr_client import client as fdr_client_module
|
||||
from gps_denied_onboard.helpers.feature_extractor import (
|
||||
FeatureExtractor,
|
||||
OpenCvOrbExtractor,
|
||||
)
|
||||
from gps_denied_onboard.helpers.lightglue_runtime import LightGlueRuntime
|
||||
from gps_denied_onboard.runtime_root import airborne_bootstrap
|
||||
from gps_denied_onboard.runtime_root.airborne_bootstrap import (
|
||||
C3_MATCHER_BUILD_FLAGS,
|
||||
AirborneBootstrapError,
|
||||
build_pre_constructed,
|
||||
)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class _C3MatcherBlock:
|
||||
"""Minimal c3_matcher config block — only the fields the tests need."""
|
||||
|
||||
strategy: str = "disk_lightglue"
|
||||
lightglue_weights_path: Path | None = None
|
||||
|
||||
|
||||
def _make_engine_handle_mock(*, descriptor_dim: int = 128) -> MagicMock:
|
||||
"""Build a mock that satisfies the LightGlueRuntime engine_handle contract.
|
||||
|
||||
The runtime checks ``engine_handle.descriptor_dim`` (must be ``int``
|
||||
and ``>= 1``) and stores the handle for later ``forward`` calls.
|
||||
AZ-622 only exercises construction; tests do not invoke ``forward``.
|
||||
"""
|
||||
handle = MagicMock(name="LightGlueEngineHandle")
|
||||
handle.descriptor_dim = descriptor_dim
|
||||
return handle
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolated_fdr_cache() -> Iterator[None]:
|
||||
# Arrange: every test starts with an empty FdrClient cache so the
|
||||
# bootstrap's make_fdr_client call doesn't accidentally pick up a
|
||||
# stale instance from a prior test in the same process.
|
||||
fdr_client_module._reset_for_tests()
|
||||
yield
|
||||
fdr_client_module._reset_for_tests()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stub_c6_and_c7_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Arrange: stub the AZ-620 (Phase B) C6 builders and the AZ-621
|
||||
# (Phase C) C7 inference builder so AZ-622 stays focused on the
|
||||
# Phase D contract. Sentinels are opaque on purpose — AZ-622
|
||||
# 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"),
|
||||
)
|
||||
|
||||
|
||||
@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"),
|
||||
)
|
||||
# AZ-625 Phase E.5: stub the C5 (estimator, handle) pair builder so
|
||||
# the bare / minimal Config()s used here don't trip on the default
|
||||
# gtsam_isam2 strategy needing a real registered factory.
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (
|
||||
MagicMock(name="StateEstimator"),
|
||||
MagicMock(name="ISam2GraphHandle"),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def test_ac_622_1_adds_c3_lightglue_runtime_and_c3_feature_extractor(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange: make BUILD_MATCHER_DISK_LIGHTGLUE ON and stub the heavy
|
||||
# LightGlue engine load with a sentinel handle. Default Config()
|
||||
# carries no c3_matcher block, so the bootstrap defaults to
|
||||
# "disk_lightglue" (per _resolve_c3_matcher_strategy).
|
||||
monkeypatch.setenv("BUILD_MATCHER_DISK_LIGHTGLUE", "ON")
|
||||
engine_handle = _make_engine_handle_mock(descriptor_dim=128)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_load_lightglue_engine_handle",
|
||||
lambda _config, _inference_runtime: engine_handle,
|
||||
)
|
||||
config = Config()
|
||||
|
||||
# Act
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Assert: AZ-622 keys are present and correctly typed; the
|
||||
# LightGlueRuntime wraps the stubbed engine handle (identity
|
||||
# check); the feature extractor satisfies the FeatureExtractor
|
||||
# Protocol. Prior phase keys are still present (additivity contract).
|
||||
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 isinstance(pre_constructed["c3_feature_extractor"], FeatureExtractor)
|
||||
assert isinstance(pre_constructed["c3_feature_extractor"], OpenCvOrbExtractor)
|
||||
assert {
|
||||
"c13_fdr",
|
||||
"clock",
|
||||
"c6_descriptor_index",
|
||||
"c6_tile_store",
|
||||
"c7_inference",
|
||||
}.issubset(pre_constructed.keys())
|
||||
|
||||
|
||||
def test_ac_622_2_build_flag_off_with_configured_strategy_raises_named_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange: BUILD_MATCHER_DISK_LIGHTGLUE OFF (delete to defend against
|
||||
# a leftover ON from a different test in the same session) AND a
|
||||
# config that selects strategy="disk_lightglue". The flag check must
|
||||
# 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"))
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
# The missing key, the gating flag, and the consuming component
|
||||
# slug must ALL appear in the operator-facing message (AZ-622
|
||||
# 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 "c3_matcher" in message
|
||||
# The flag-OFF branch raises directly — there is no upstream cause to
|
||||
# preserve (cause-chain preservation is exercised in
|
||||
# test_ac_622_2_lightglue_engine_load_failure_wraps_runtime_error).
|
||||
assert excinfo.value.__cause__ is None
|
||||
|
||||
|
||||
def test_ac_622_2_build_flag_off_with_aliked_strategy_names_aliked_flag(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""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"))
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c3_lightglue_runtime" in message
|
||||
assert C3_MATCHER_BUILD_FLAGS["aliked_lightglue"] in message
|
||||
assert "c3_matcher" in message
|
||||
# The DISK flag must NOT appear — the message is strategy-specific.
|
||||
assert C3_MATCHER_BUILD_FLAGS["disk_lightglue"] not in message
|
||||
|
||||
|
||||
def test_ac_622_2_default_config_no_c3_matcher_block_still_raises(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Defence-in-depth: even with no c3_matcher block configured, AZ-622
|
||||
still fails loudly when the default-strategy flag is OFF.
|
||||
|
||||
Mirrors the AZ-620 / AZ-621 defence-in-depth tests. Silently returning
|
||||
a dict without ``c3_lightglue_runtime`` would let a downstream caller
|
||||
misread the state.
|
||||
"""
|
||||
# Arrange
|
||||
monkeypatch.delenv("BUILD_MATCHER_DISK_LIGHTGLUE", raising=False)
|
||||
config = Config() # empty components, default strategy resolves to disk_lightglue
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c3_lightglue_runtime" in message
|
||||
assert C3_MATCHER_BUILD_FLAGS["disk_lightglue"] in message
|
||||
assert "c3_matcher" in message
|
||||
|
||||
|
||||
def test_ac_622_2_lightglue_engine_load_failure_wraps_runtime_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When the BUILD flag is ON but the engine load itself fails,
|
||||
``RuntimeNotAvailableError`` is wrapped into ``AirborneBootstrapError``
|
||||
with the cause chain preserved (mirrors AZ-621's wrapping pattern)."""
|
||||
# Arrange
|
||||
from gps_denied_onboard.runtime_root.errors import RuntimeNotAvailableError
|
||||
|
||||
monkeypatch.setenv("BUILD_MATCHER_DISK_LIGHTGLUE", "ON")
|
||||
|
||||
def _raise_engine_load_failure(_config: Config, _inference_runtime: object) -> None:
|
||||
raise RuntimeNotAvailableError(
|
||||
"InferenceRuntime.deserialize_engine: simulated GPU bind failure"
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_load_lightglue_engine_handle",
|
||||
_raise_engine_load_failure,
|
||||
)
|
||||
config = Config.with_blocks(c3_matcher=_C3MatcherBlock(strategy="disk_lightglue"))
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c3_lightglue_runtime" in message
|
||||
assert "disk_lightglue" in message
|
||||
assert "c3_matcher" in message
|
||||
assert isinstance(excinfo.value.__cause__, RuntimeNotAvailableError)
|
||||
|
||||
|
||||
def test_lightglue_runtime_uses_c7_inference_from_pre_constructed(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The c7 InferenceRuntime built earlier in build_pre_constructed
|
||||
must be the SAME instance passed into ``_load_lightglue_engine_handle``
|
||||
— no double-build of the inference runtime at this layer."""
|
||||
# Arrange
|
||||
monkeypatch.setenv("BUILD_MATCHER_DISK_LIGHTGLUE", "ON")
|
||||
inference_runtime_sentinel = MagicMock(name="InferenceRuntime")
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c7_inference",
|
||||
lambda _config: inference_runtime_sentinel,
|
||||
)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def _capture_loader(config: Config, inference_runtime: object) -> object:
|
||||
captured["config"] = config
|
||||
captured["inference_runtime"] = inference_runtime
|
||||
return _make_engine_handle_mock(descriptor_dim=128)
|
||||
|
||||
monkeypatch.setattr(airborne_bootstrap, "_load_lightglue_engine_handle", _capture_loader)
|
||||
config = Config()
|
||||
|
||||
# Act
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Assert: the loader received the EXACT same InferenceRuntime instance
|
||||
# placed under pre_constructed['c7_inference'] (identity-share — the
|
||||
# same single-build-per-bootstrap discipline AZ-621 established).
|
||||
assert captured["inference_runtime"] is inference_runtime_sentinel
|
||||
assert pre_constructed["c7_inference"] is inference_runtime_sentinel
|
||||
@@ -0,0 +1,314 @@
|
||||
"""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"),
|
||||
)
|
||||
# AZ-625 Phase E.5: stub the C5 (estimator, handle) pair builder so
|
||||
# the AZ-623 tests stay focused on the four Phase E helpers without
|
||||
# the new gtsam_isam2-strategy registration path firing for every
|
||||
# config that has camera_calibration_path populated. The downstream
|
||||
# AZ-625 test file owns the (estimator, handle) pair contract.
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (
|
||||
MagicMock(name="StateEstimator"),
|
||||
MagicMock(name="ISam2GraphHandle"),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
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
|
||||
@@ -0,0 +1,485 @@
|
||||
"""AZ-625 — Phase E.5 of AZ-618: c5_isam2_graph_handle ordering.
|
||||
|
||||
Verifies the contract at
|
||||
``_docs/02_tasks/todo/AZ-625_c5_isam2_graph_handle_ordering.md``:
|
||||
|
||||
* AC-625.1: ``build_pre_constructed(default_config)`` adds key
|
||||
``c5_isam2_graph_handle`` on top of AZ-619..AZ-623; the value
|
||||
satisfies the C4 :class:`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 :class:`AirborneBootstrapError`
|
||||
whose message names the gating flag and the consuming component
|
||||
slug ``c5_state``.
|
||||
* AC-625.3: ``compose_root(config, pre_constructed=...)`` produces a
|
||||
runtime where the handle held by C4 IS the same handle exposed by
|
||||
the C5 estimator's ``_isam2_handle``. We exercise the cross-seam
|
||||
identity invariant via the bootstrap's
|
||||
``_c5_prebuilt_estimator`` look-aside key + the
|
||||
``_c5_state_wrapper`` short-circuit, so the unit-test path does
|
||||
not need to stand up the full ``compose_root`` graph (which would
|
||||
pull in gtsam, FAISS, TensorRT — all out of scope per the AZ-625
|
||||
task spec's Tier-2 Note).
|
||||
|
||||
AC-625.4 (this file exists with the above tests) is satisfied by the
|
||||
existence of this module.
|
||||
|
||||
The tests stub the heavy ``build_state_estimator`` seam through
|
||||
:func:`_build_c5_state_estimator_pair`'s ``__module__`` attribute path
|
||||
so they exercise the bootstrap-error contract + identity-share
|
||||
contract without standing up gtsam or constructing a real
|
||||
:class:`GtsamIsam2StateEstimator`. The upstream AZ-619..AZ-623
|
||||
builders are stubbed at the airborne_bootstrap module boundary,
|
||||
mirroring the prior phase pattern (see
|
||||
:mod:`tests.unit.runtime_root.test_az623_pre_constructed_phase_e`).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import dataclasses
|
||||
from collections.abc import Iterator
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from gps_denied_onboard.components.c4_pose._isam2_handle import (
|
||||
ISam2GraphHandle as C4ISam2GraphHandle,
|
||||
)
|
||||
from gps_denied_onboard.config import Config
|
||||
from gps_denied_onboard.fdr_client import client as fdr_client_module
|
||||
from gps_denied_onboard.runtime_root import airborne_bootstrap
|
||||
from gps_denied_onboard.runtime_root.airborne_bootstrap import (
|
||||
AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS,
|
||||
C5_STATE_BUILD_FLAGS,
|
||||
AirborneBootstrapError,
|
||||
build_pre_constructed,
|
||||
clear_imu_preintegrator_cache,
|
||||
)
|
||||
|
||||
_C5_PREBUILT_ESTIMATOR_KEY = "_c5_prebuilt_estimator"
|
||||
"""Mirror of ``airborne_bootstrap._C5_PREBUILT_ESTIMATOR_KEY`` for
|
||||
test-side identity assertions. Internal coordination key (deliberately
|
||||
NOT exposed via __all__); duplicated here so the test does not import
|
||||
a private name.
|
||||
"""
|
||||
|
||||
|
||||
def _config_with_calibration_path(path: str) -> Config:
|
||||
"""Return a fresh ``Config`` whose ``runtime.camera_calibration_path`` is set.
|
||||
|
||||
Mirrors the helper in
|
||||
:mod:`tests.unit.runtime_root.test_az623_pre_constructed_phase_e` —
|
||||
AZ-625 still walks the AZ-623 ``_build_c5_imu_preintegrator``
|
||||
builder which empty-checks the same field.
|
||||
"""
|
||||
base = Config()
|
||||
runtime = dataclasses.replace(base.runtime, camera_calibration_path=path)
|
||||
return dataclasses.replace(base, runtime=runtime)
|
||||
|
||||
|
||||
class _FakeIsam2GraphHandle:
|
||||
"""Lightweight stand-in for the production
|
||||
:class:`gps_denied_onboard.components.c5_state._isam2_handle.\
|
||||
ISam2GraphHandleImpl` used by AC-625.1's Protocol-conformance
|
||||
assertion.
|
||||
|
||||
Implements every method named by the C4 ``ISam2GraphHandle``
|
||||
Protocol so :func:`isinstance` against the runtime-checkable
|
||||
Protocol returns ``True`` — the production
|
||||
:class:`ISam2GraphHandleImpl` is the same shape.
|
||||
"""
|
||||
|
||||
def get_pose_key(self, frame_id: int) -> int:
|
||||
del frame_id
|
||||
return 0
|
||||
|
||||
def add_factor(self, factor: Any) -> None:
|
||||
del factor
|
||||
|
||||
def update(self, graph: Any, values: Any, timestamps: Any | None = None) -> None:
|
||||
del graph, values, timestamps
|
||||
|
||||
def compute_marginals(self) -> Any:
|
||||
return None
|
||||
|
||||
def last_anchor_age_ms(self) -> int:
|
||||
return 0
|
||||
|
||||
|
||||
class _FakeStateEstimator:
|
||||
"""Stand-in for ``GtsamIsam2StateEstimator``.
|
||||
|
||||
The only attribute AC-625.3 inspects is ``_isam2_handle`` — the
|
||||
production estimator stores the same handle there
|
||||
(:class:`gps_denied_onboard.components.c5_state.gtsam_isam2_estimator.\
|
||||
GtsamIsam2StateEstimator.__init__` builds
|
||||
``ISam2GraphHandleImpl(self)`` and assigns it to ``self._isam2_handle``).
|
||||
"""
|
||||
|
||||
def __init__(self, handle: _FakeIsam2GraphHandle) -> None:
|
||||
self._isam2_handle = handle
|
||||
|
||||
|
||||
def _stub_state_pair(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> tuple[_FakeStateEstimator, _FakeIsam2GraphHandle]:
|
||||
"""Replace ``_build_c5_state_estimator_pair`` with a fixed (estimator, handle).
|
||||
|
||||
Returns the same tuple values the stub injects, so individual
|
||||
tests can perform identity-share assertions against them without
|
||||
re-discovering the sentinels through ``pre_constructed``.
|
||||
"""
|
||||
handle = _FakeIsam2GraphHandle()
|
||||
estimator = _FakeStateEstimator(handle)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_build_c5_state_estimator_pair",
|
||||
lambda *_args, **_kwargs: (estimator, handle),
|
||||
)
|
||||
return estimator, handle
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolated_caches() -> Iterator[None]:
|
||||
# Arrange: every test starts with empty FdrClient cache + empty
|
||||
# ImuPreintegrator cache so the AZ-619..AZ-623 builders behind
|
||||
# build_pre_constructed do not pick up stale instances.
|
||||
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_az623_builders(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
# Arrange: stub the AZ-620 (Phase B) C6 builders, AZ-621 (Phase C) C7
|
||||
# inference builder, AZ-622 (Phase D) C3 builders, and AZ-623 (Phase E)
|
||||
# c5 helper builders so AZ-625 stays focused on the Phase E.5 contract.
|
||||
# Sentinels are opaque — AZ-625 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"),
|
||||
)
|
||||
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_625_1_adds_c5_isam2_graph_handle_with_protocol_surface(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange: stub the (estimator, handle) pair builder; default Config()
|
||||
# with a populated camera_calibration_path so AZ-623's
|
||||
# _build_c5_imu_preintegrator empty-check passes (autouse fixture has
|
||||
# already stubbed the builder, so the path is only structurally set).
|
||||
config = _config_with_calibration_path("/tmp/az625-fixture-calib.json")
|
||||
_, handle = _stub_state_pair(monkeypatch)
|
||||
|
||||
# Act
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Assert: the new key is present and identifies the stubbed handle.
|
||||
assert "c5_isam2_graph_handle" in pre_constructed
|
||||
assert pre_constructed["c5_isam2_graph_handle"] is handle
|
||||
|
||||
# Protocol-conformance: the handle must satisfy the C4 consumer's
|
||||
# runtime-checkable Protocol — get_pose_key + add_factor + update +
|
||||
# compute_marginals + last_anchor_age_ms.
|
||||
assert isinstance(handle, C4ISam2GraphHandle), (
|
||||
"c5_isam2_graph_handle must satisfy the C4 ISam2GraphHandle Protocol; "
|
||||
f"missing attributes on {type(handle).__name__}"
|
||||
)
|
||||
for method_name in (
|
||||
"get_pose_key",
|
||||
"add_factor",
|
||||
"update",
|
||||
"compute_marginals",
|
||||
"last_anchor_age_ms",
|
||||
):
|
||||
assert hasattr(handle, method_name), (
|
||||
f"c5_isam2_graph_handle is missing {method_name!r}; "
|
||||
f"the C4 consumer (OpenCVGtsamPoseEstimator) dispatches via attribute access"
|
||||
)
|
||||
|
||||
# AZ-619..AZ-623 keys remain populated (additivity invariant).
|
||||
assert {
|
||||
"c13_fdr",
|
||||
"clock",
|
||||
"c6_descriptor_index",
|
||||
"c6_tile_store",
|
||||
"c7_inference",
|
||||
"c3_lightglue_runtime",
|
||||
"c3_feature_extractor",
|
||||
"c282_ransac_filter",
|
||||
"c5_imu_preintegrator",
|
||||
"c5_se3_utils",
|
||||
"c5_wgs_converter",
|
||||
}.issubset(pre_constructed.keys()), (
|
||||
f"AZ-625 must be additive on top of AZ-619..AZ-623; got "
|
||||
f"keys: {sorted(pre_constructed.keys())}"
|
||||
)
|
||||
|
||||
|
||||
def test_ac_625_1_internal_prebuilt_estimator_key_not_in_required_keys() -> None:
|
||||
"""The ``_c5_prebuilt_estimator`` look-aside key is internal coordination
|
||||
only — it MUST NOT appear under
|
||||
:data:`AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS` because no consuming
|
||||
component queries it via the public surface (only
|
||||
``_c5_state_wrapper`` consults it as a fast path).
|
||||
"""
|
||||
# Assert: the look-aside key is not in any consumer's required-keys row.
|
||||
for slug, required_keys in AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS.items():
|
||||
assert _C5_PREBUILT_ESTIMATOR_KEY not in required_keys, (
|
||||
f"{_C5_PREBUILT_ESTIMATOR_KEY!r} is an internal coordination key; "
|
||||
f"it must not be exposed via "
|
||||
f"AIRBORNE_REQUIRED_PRE_CONSTRUCTED_KEYS[{slug!r}]"
|
||||
)
|
||||
|
||||
|
||||
def test_ac_625_2_build_state_gtsam_isam2_off_raises_named_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
# Arrange: explicitly set BUILD_STATE_GTSAM_ISAM2=OFF (the gating
|
||||
# flag check uses os.environ.get(flag, "ON").upper() == "OFF" — same
|
||||
# default ladder as state_factory). Default config resolves to
|
||||
# gtsam_isam2 strategy. Do NOT stub _build_c5_state_estimator_pair
|
||||
# here — we want the real flag-OFF guard to fire.
|
||||
monkeypatch.setenv("BUILD_STATE_GTSAM_ISAM2", "OFF")
|
||||
config = _config_with_calibration_path("/tmp/az625-flag-off-fixture-calib.json")
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
# The missing key, the gating flag, and the consuming component
|
||||
# slug must all appear in the operator-facing message.
|
||||
assert "c5_isam2_graph_handle" in message
|
||||
assert C5_STATE_BUILD_FLAGS["gtsam_isam2"] in message, (
|
||||
f"BUILD_STATE_GTSAM_ISAM2 missing from error: {message!r}"
|
||||
)
|
||||
assert "c5_state" in message
|
||||
# The flag-OFF branch raises directly — there is no upstream cause.
|
||||
assert excinfo.value.__cause__ is None
|
||||
|
||||
|
||||
def test_ac_625_2_unknown_strategy_raises_named_error_with_supported_set(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Defence-in-depth for the strategy-resolution path.
|
||||
|
||||
Even when BUILD_STATE_* flags are ON, an unknown C5 strategy must
|
||||
raise :class:`AirborneBootstrapError` naming the supported set
|
||||
(``gtsam_isam2`` / ``eskf``). The error must fire BEFORE
|
||||
``build_state_estimator`` is consulted — otherwise the operator
|
||||
sees ``StateEstimatorConfigError`` rather than the bootstrap-error
|
||||
contract this module owns.
|
||||
"""
|
||||
|
||||
# Arrange: smuggle an unknown strategy past the upstream
|
||||
# KNOWN_STATE_STRATEGIES validator (lives in
|
||||
# ``components.c5_state.config``) by stubbing the bootstrap's own
|
||||
# strategy resolver. The bootstrap's own check is what we want to
|
||||
# exercise here.
|
||||
config = _config_with_calibration_path("/tmp/az625-unknown-fixture.json")
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_resolve_c5_state_strategy",
|
||||
lambda _config: "lqg_baseline_does_not_exist",
|
||||
)
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c5_isam2_graph_handle" in message
|
||||
assert "lqg_baseline_does_not_exist" in message
|
||||
assert "c5_state" in message
|
||||
# The supported set is enumerated so the operator sees the fix.
|
||||
for supported in C5_STATE_BUILD_FLAGS:
|
||||
assert supported in message, (
|
||||
f"supported strategy {supported!r} missing from error: {message!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_ac_625_2_build_state_estimator_config_error_wraps_into_bootstrap_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When the BUILD flag is ON but the state-factory itself rejects the
|
||||
config, ``StateEstimatorConfigError`` must wrap into
|
||||
:class:`AirborneBootstrapError` with the cause chain preserved
|
||||
(mirrors AZ-621 / AZ-622's wrapping pattern)."""
|
||||
# Arrange
|
||||
from gps_denied_onboard.components.c5_state.errors import (
|
||||
StateEstimatorConfigError,
|
||||
)
|
||||
|
||||
monkeypatch.setenv("BUILD_STATE_GTSAM_ISAM2", "ON")
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_ensure_state_strategy_registered",
|
||||
lambda _config: None,
|
||||
)
|
||||
|
||||
def _raise_config_error(*_args: Any, **_kwargs: Any) -> Any:
|
||||
raise StateEstimatorConfigError("simulated state factory rejection")
|
||||
|
||||
# _build_c5_state_estimator_pair calls the imported reference
|
||||
# ``build_state_estimator`` from the airborne_bootstrap module
|
||||
# namespace; monkeypatch that attribute directly.
|
||||
monkeypatch.setattr(airborne_bootstrap, "build_state_estimator", _raise_config_error)
|
||||
config = _config_with_calibration_path("/tmp/az625-cfg-err-fixture.json")
|
||||
|
||||
# Act + Assert
|
||||
with pytest.raises(AirborneBootstrapError) as excinfo:
|
||||
build_pre_constructed(config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "c5_isam2_graph_handle" in message
|
||||
assert "c5_state" in message
|
||||
assert "gtsam_isam2" in message
|
||||
assert isinstance(excinfo.value.__cause__, StateEstimatorConfigError)
|
||||
|
||||
|
||||
def test_ac_625_3_handle_is_same_object_as_estimator_isam2_handle(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Cross-seam identity: the handle that c4_pose receives via
|
||||
``pre_constructed['c5_isam2_graph_handle']`` IS the SAME object
|
||||
that the c5_state estimator exposes as ``_isam2_handle``.
|
||||
|
||||
AC-625.3's ``compose_root(...)`` end-to-end form requires gtsam +
|
||||
FAISS + TensorRT and is exercised by the Jetson tier-2 e2e harness
|
||||
(AZ-624 AC-5). At unit-test scope the equivalent invariant is
|
||||
``pre_constructed[_c5_prebuilt_estimator]._isam2_handle is
|
||||
pre_constructed['c5_isam2_graph_handle']`` — which is what the
|
||||
bootstrap's seeding ordering must guarantee.
|
||||
"""
|
||||
# Arrange
|
||||
config = _config_with_calibration_path("/tmp/az625-identity-fixture.json")
|
||||
estimator, handle = _stub_state_pair(monkeypatch)
|
||||
|
||||
# Act
|
||||
pre_constructed = build_pre_constructed(config)
|
||||
|
||||
# Assert: identity-share across the C4 / C5 seam.
|
||||
assert pre_constructed["c5_isam2_graph_handle"] is handle
|
||||
assert pre_constructed[_C5_PREBUILT_ESTIMATOR_KEY] is estimator
|
||||
assert (
|
||||
pre_constructed[_C5_PREBUILT_ESTIMATOR_KEY]._isam2_handle
|
||||
is (pre_constructed["c5_isam2_graph_handle"])
|
||||
)
|
||||
|
||||
|
||||
def test_ac_625_3_c5_state_wrapper_short_circuits_on_prebuilt_estimator(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The :func:`_c5_state_wrapper` returns the prebuilt estimator as-is
|
||||
when ``_c5_prebuilt_estimator`` is present in ``constructed``.
|
||||
|
||||
This is the seam that lets AC-625.3 hold under the live
|
||||
``compose_root`` topo walk: the C5 wrapper does NOT re-invoke
|
||||
:func:`build_state_estimator` (which would produce a different
|
||||
``(estimator, handle)`` pair than the one C4 has already
|
||||
consumed). Verified by ensuring the wrapper does not consult the
|
||||
fallback infra-key set at all when the look-aside key is set.
|
||||
"""
|
||||
# Arrange: build a constructed dict with ONLY the look-aside key.
|
||||
# If the wrapper short-circuits correctly it must NOT read any of
|
||||
# c5_imu_preintegrator / c5_se3_utils / c5_wgs_converter / c13_fdr —
|
||||
# the absence of those keys would otherwise raise
|
||||
# AirborneBootstrapError via _require.
|
||||
estimator, _handle = _stub_state_pair(monkeypatch)
|
||||
constructed = {_C5_PREBUILT_ESTIMATOR_KEY: estimator}
|
||||
|
||||
# Act
|
||||
returned = airborne_bootstrap._c5_state_wrapper(Config(), constructed)
|
||||
|
||||
# Assert
|
||||
assert returned is estimator
|
||||
|
||||
|
||||
def test_ac_625_3_c5_state_wrapper_falls_back_when_prebuilt_absent(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The :func:`_c5_state_wrapper` falls back to the original
|
||||
:func:`build_state_estimator` path when ``_c5_prebuilt_estimator``
|
||||
is absent.
|
||||
|
||||
Test isolation contract: existing fixtures (e.g. the
|
||||
``test_az401_compose_root_replay`` suite) seed ``pre_constructed``
|
||||
manually without going through :func:`build_pre_constructed` and
|
||||
therefore have no look-aside key. The fallback must still work —
|
||||
AZ-625's seam is additive, never replacing the existing one.
|
||||
"""
|
||||
# Arrange: no _c5_prebuilt_estimator. Stub the heavy
|
||||
# build_state_estimator seam so the fallback path returns
|
||||
# deterministically.
|
||||
fallback_estimator = _FakeStateEstimator(_FakeIsam2GraphHandle())
|
||||
fallback_handle = MagicMock(name="FallbackHandle")
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"_ensure_state_strategy_registered",
|
||||
lambda _config: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
airborne_bootstrap,
|
||||
"build_state_estimator",
|
||||
lambda *_args, **_kwargs: (fallback_estimator, fallback_handle),
|
||||
)
|
||||
constructed = {
|
||||
"c5_imu_preintegrator": MagicMock(name="ImuPreintegrator"),
|
||||
"c5_se3_utils": MagicMock(name="Se3Utils"),
|
||||
"c5_wgs_converter": MagicMock(name="WgsConverter"),
|
||||
"c13_fdr": MagicMock(name="FdrClient"),
|
||||
}
|
||||
|
||||
# Act
|
||||
returned = airborne_bootstrap._c5_state_wrapper(Config(), constructed)
|
||||
|
||||
# Assert: fallback path returned the build_state_estimator-built
|
||||
# estimator (not the prebuilt sentinel — there is none).
|
||||
assert returned is fallback_estimator
|
||||
Reference in New Issue
Block a user