# Code Review Report — Batch 57

**Batch**: 57
**Tasks**: AZ-345 (C3 DISK+LightGlue), AZ-346 (C3 ALIKED+LightGlue), AZ-347 (C3 XFeat), AZ-349 (C3.5 AdHoP refiner)
**Date**: 2026-05-14
**Verdict**: PASS_WITH_WARNINGS
**Mode**: Full (per-batch)

## Phase Summary

| Phase                              | Result   |
|------------------------------------|----------|
| 1. Context Loading                 | OK       |
| 2. Spec Compliance                 | OK (47/47 ACs implemented + tested — see breakdown) |
| 3. Code Quality                    | OK (one Low: cross-module underscore-prefixed imports) |
| 4. Security Quick-Scan             | OK       |
| 5. Performance Scan                | OK (real-hardware budgets deferred to C3-PT-01 / C3.5-PT-01) |
| 6. Cross-Task Consistency          | OK (4 strategies share `RansacFilter`, `RollingHealthWindow`, `LightGlueRuntime`, `FdrClient`) |
| 7. Architecture Compliance         | OK (consumer-side `InferenceRuntimeCut` Protocols added per AZ-507) |

## AC Coverage Breakdown

| Task   | AC count                                | Covered |
|--------|-----------------------------------------|---------|
| AZ-345 | 12 (AC-1..AC-12)                        | 12/12   |
| AZ-346 | 14 (AC-1..AC-12 + AC-special-1/2)       | 14/14   |
| AZ-347 | 13 (AC-1..AC-12 + AC-special-1)         | 13/13 (AC-special-2 informational; not gated per spec) |
| AZ-349 | 11 (AC-1..AC-11)                        | 11/11   |
| **Total** | **47**                              | **47/47** |

Test files added:

- `tests/unit/c3_matcher/test_az345_346_347_concrete_matchers.py` — 43 tests, parametrised across the three strategies for AC-1..AC-12 plus three strategy-specific tests (AZ-346 AC-special-1 ×2, AZ-347 AC-special-1).
- `tests/unit/c3_5_adhop/test_az349_adhop_refiner.py` — 23 tests covering AC-1..AC-11 + extra-safety checks for bad threshold, bad refined shape, and non-finite outputs.

## Findings

| # | Severity | Category        | File:Line | Title |
|---|----------|-----------------|-----------|-------|
| 1 | Low      | Style           | `components/c3_matcher/xfeat.py:33-40` | XFeat imports underscore-prefixed helpers (`_emit_backbone_error`, `_emit_frame_done`, `_emit_insufficient_inliers`, `_fail_all`) from sibling `_pipeline.py` |
| 2 | Low      | Maintainability | `components/c3_matcher/_engine_output_assertion.py:60` | Probe tensor is FP32; if a future ALIKED engine is compiled FP16-only, the probe input dtype must follow |
| 3 | Low      | Scope           | AZ-347 AC-special-2 | Latency comparison (XFeat < DISK+LightGlue p95) is informational per spec — no test exists; flagged for traceability |

### Finding Details

**F1: XFeat imports underscore-prefixed helpers from `_pipeline.py`** (Low / Style)

- Location: `src/gps_denied_onboard/components/c3_matcher/xfeat.py:33-40`
- Description: `_pipeline.py` is the shared per-frame orchestration for DISK+LightGlue and ALIKED+LightGlue (AZ-345/346). XFeat (AZ-347) uses a different per-candidate loop because its backbone fuses extraction + matching into one pass, but it still re-uses four error/emission helpers (`_emit_backbone_error`, `_emit_frame_done`, `_emit_insufficient_inliers`, `_fail_all`) for FDR-byte-identical telemetry. The underscore prefix on these helpers means "internal to `c3_matcher`" — sibling files importing them is permitted by Python convention, but readers may assume `_pipeline` is private to its `__all__` exports.
- Suggestion: (a) Add a docstring note in `_pipeline.py` that lists the in-component consumers of the underscore helpers, OR (b) drop the leading underscore from the four helpers (they are component-internal but cross-file public). Option (a) preferred — keeps the surface unchanged.
- Task: AZ-347

**F2: Probe tensor dtype is FP32; production ALIKED engines may be FP16** (Low / Maintainability)

- Location: `src/gps_denied_onboard/components/c3_matcher/_engine_output_assertion.py:60`
- Description: `assert_keypoint_engine_output_schema` allocates a zero-init probe with `dtype=np.float32`. Production ALIKED engines compiled by C10 (AZ-321) use `PrecisionMode.FP16` per `_build_aliked_build_config()`. If a TRT engine's input binding rejects an FP32 tensor (TRT 10.x is strict about binding dtype), the probe will fail at startup with a confusing error rather than the intended schema check.
- Suggestion: Follow the C2 pattern (`assert_engine_output_dim` uses `np.float16` for FP16 backbones); thread the engine's input dtype through `assert_keypoint_engine_output_schema` and synthesise the probe in the matching dtype. Alternative: probe with FP16 by default and document the assumption. No fix required this cycle — the existing tests cover the schema-mismatch path with a fake runtime that accepts any dtype.
- Task: AZ-346

**F3: AZ-347 AC-special-2 is informational, not tested** (Low / Scope)

- Location: `_docs/02_tasks/todo/AZ-347_c3_xfeat.md:91-94`
- Description: The task spec explicitly marks AC-special-2 as "informational metric; if XFeat is NOT faster, that's a backbone misconfiguration, not a contract violation. Documented in the test report; does NOT block this AC." No test exists today. Flagged for traceability so the omission is visible in the next cumulative review.
- Suggestion: No action. Latency benchmarks belong to C3-PT-01 (E-BBT, deferred). Documented here to keep the AC coverage matrix honest.
- Task: AZ-347

## Verdict

**PASS_WITH_WARNINGS** — every AC has a covering test; the three Low findings are observability / future-proofing concerns, not contract violations or runtime bugs. No Critical / High / Architecture findings.

Auto-fix would not improve any of the Low findings without expanding scope. Proceed to commit.