azaion/gps-denied-onboard
1
0
Fork 0
mirror of https://github.com/azaion/gps-denied-onboard.git synced 2026-06-22 15:21:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

[AZ-527] Archive AZ-527 + batch 52 report + state bump

Browse Source 
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-14 00:51:19 +03:00
parent 235eb4549e
commit 2ce300ddb1
3 changed files with 75 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
_docs/02_tasks/todo/AZ-527_hygiene_engine_dim_assertion_consolidation.md
-120
View File
@@ -1,120 +0,0 @@
# Hygiene — Consolidate `_assert_engine_output_dim` into a c2-internal helper
**Task**: AZ-527_hygiene_engine_dim_assertion_consolidation
**Name**: c2_vpr engine output-dim assertion helper consolidation
**Description**: Replace the seven duplicated `_assert_engine_output_dim(...)` definitions across the c2_vpr secondary VPR strategy modules (`ultra_vpr.py`, `net_vlad.py`, `mega_loc.py`, `mix_vpr.py`, `sela_vpr.py`, `eigen_places.py`, `salad.py`) with a single c2-internal helper at `src/gps_denied_onboard/components/c2_vpr/_engine_dim_assertion.py`. Closes cumulative review batches 49–51 Finding F1 (Medium / Maintainability).
**Complexity**: 2 points
**Dependencies**: AZ-340 (the trigger that escalated the duplication from 4-way to 7-way; all 7 callers now exist). All other 6 callers are already in `done/` (AZ-337, AZ-338, AZ-339).
**Component**: c2_vpr (epic AZ-255 / E-C2)
**Tracker**: AZ-527
**Epic**: AZ-255 (E-C2)
### Document Dependencies
- `_docs/03_implementation/cumulative_review_batches_49-51_cycle1_report.md` § F1 — the finding being closed.
- `_docs/02_document/components/02_c2_vpr/description.md` § 6 — the c2_vpr component description (helper goes inside the c2_vpr boundary).
- `_docs/02_tasks/done/AZ-337_c2_ultra_vpr_strategy.md`, `AZ-338_c2_netvlad_strategy.md`, `AZ-339_c2_mega_loc_mix_vpr.md`, `AZ-340_c2_selavpr_eigenplaces_salad.md` — the four task-specs whose AC-6 inference-output-dim contracts must continue to pass unmodified.
## Problem
Seven c2_vpr secondary VPR strategy modules each define a private `_assert_engine_output_dim` helper with byte-identical bodies modulo a single integer constant:
| File | `DESCRIPTOR_DIM` |
|------|------------------|
| `src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py` (AZ-337, B47) | 512 |
| `src/gps_denied_onboard/components/c2_vpr/net_vlad.py` (AZ-338, B46) | 4096 |
| `src/gps_denied_onboard/components/c2_vpr/mega_loc.py` (AZ-339, B50) | 4096 |
| `src/gps_denied_onboard/components/c2_vpr/mix_vpr.py` (AZ-339, B50) | 8448 |
| `src/gps_denied_onboard/components/c2_vpr/sela_vpr.py` (AZ-340, B51) | 512 |
| `src/gps_denied_onboard/components/c2_vpr/eigen_places.py` (AZ-340, B51) | 2048 |
| `src/gps_denied_onboard/components/c2_vpr/salad.py` (AZ-340, B51) | 8448 |
Each copy runs a zero-init dry-run inference probe at `create()` time, asserts the engine output dict carries an `"embedding"` key, and asserts the output ndarray shape is `(1, DESCRIPTOR_DIM)`. The error-message strings and the shape-check logic are byte-identical. Each copy carries an inline comment referencing this AZ-527 ticket.
Cumulative review batches 46-48 originally flagged the duplication at 2-way (Low). Cumulative review batches 49-51 escalated to Medium when the duplication grew to 7-way after AZ-339 + AZ-340 land. This PBI closes the pattern before any future c2_vpr backbone (or hypothetical c2-side adjacent work) adds an 8th copy.
## Outcome
- A new c2-internal module `src/gps_denied_onboard/components/c2_vpr/_engine_dim_assertion.py` exposes a single function `assert_engine_output_dim(inference_runtime, handle, preprocessor, descriptor_dim, *, output_key="embedding", input_key="input") -> None` parametrised by the dim and the key names. Raises `gps_denied_onboard.config.schema.ConfigError` on mismatch (preserving the existing error envelope).
- The seven local `_assert_engine_output_dim` definitions are deleted; consumers import the helper. Module-level callers preserve the local symbol via `from gps_denied_onboard.components.c2_vpr._engine_dim_assertion import assert_engine_output_dim` (with a thin one-liner wrapper if call-site stability is preferred).
- The seven inline `AZ-527 (planned)` comments are deleted (they will be obsolete).
- A new unit test `tests/unit/c2_vpr/test_az527_engine_dim_assertion.py` covers AC-1..AC-4 below.
- The existing AZ-337 / 338 / 339 / 340 AC-6 tests pass unmodified.
## Scope
### Included
- Add `src/gps_denied_onboard/components/c2_vpr/_engine_dim_assertion.py` with the helper function (signature above). Import surface: `gps_denied_onboard.config.schema.ConfigError` only — no other component imports.
- Migrate the seven c2_vpr strategy modules to import the helper. Delete the local definitions and the local `AZ-527 (planned)` comments.
- Add `tests/unit/c2_vpr/test_az527_engine_dim_assertion.py` with AC-1..AC-4 (success path with matching shape, wrong shape raises `ConfigError`, missing output key raises `ConfigError`, success with non-default `output_key` / `input_key`).
- Re-run the existing `tests/unit/c2_vpr/test_ultra_vpr.py`, `test_net_vlad.py`, `test_az339_mega_loc_mix_vpr.py`, `test_az340_sela_vpr_eigen_places_salad.py` AC-6 sub-tests **unmodified** to verify that the consolidation preserves behavior at every call site.
- Optionally add a one-line mention of the helper in `_docs/02_document/components/02_c2_vpr/description.md` § 6 (Internal helpers).
### Excluded
- Sharing the helper across other components (c3, c4, c5). C7 already has its own engine-shape assertions inside the runtime; this PBI is c2-internal only.
- Changing the `"embedding"` output key contract or the `"input"` input key contract (those are runtime/engine conventions enforced elsewhere).
- Refactoring the `BackbonePreprocessor` Protocol — the helper still calls `preprocessor.input_shape()` to size the dry-run probe.
- Hoisting the helper to `src/gps_denied_onboard/helpers/` — engine-output-shape contracts are a c2 internal concern, not a cross-component shared helper.
## Acceptance Criteria
**AC-1: Helper exists at the canonical path with the expected signature**
Given a fresh checkout
When `from gps_denied_onboard.components.c2_vpr._engine_dim_assertion import assert_engine_output_dim` is run
Then the import succeeds; the function signature is `(inference_runtime, handle, preprocessor, descriptor_dim, *, output_key="embedding", input_key="input")` and the function returns `None` on success
**AC-2: Wrong shape raises `ConfigError` with the expected envelope**
Given a fake `InferenceRuntime` whose `infer()` returns `{"embedding": np.zeros((1, 999), dtype=np.float16)}` for any of the 7 backbones
When `assert_engine_output_dim(..., descriptor_dim=512)` is called
Then a `ConfigError` is raised; the message contains both the expected dim (`512`) and the actual dim (`999`); the error type matches the contract `gps_denied_onboard.config.schema.ConfigError`
**AC-3: Missing output key raises `ConfigError`**
Given a fake `InferenceRuntime` whose `infer()` returns `{"wrong_key": np.zeros((1, 512), dtype=np.float16)}`
When `assert_engine_output_dim(..., descriptor_dim=512)` is called
Then a `ConfigError` is raised; the message names the missing `output_key`
**AC-4: All seven local `_assert_engine_output_dim` definitions are removed**
Given a `grep -rn "def _assert_engine_output_dim\|def assert_engine_output_dim" src/` after the task lands
When the search runs
Then matches appear only inside `src/gps_denied_onboard/components/c2_vpr/_engine_dim_assertion.py`; zero matches in `ultra_vpr.py`, `net_vlad.py`, `mega_loc.py`, `mix_vpr.py`, `sela_vpr.py`, `eigen_places.py`, `salad.py`
**AC-5: All AZ-337 / 338 / 339 / 340 AC-6 sub-tests pass unmodified**
Given the existing `tests/unit/c2_vpr/test_ultra_vpr.py`, `test_net_vlad.py`, `test_az339_mega_loc_mix_vpr.py`, `test_az340_sela_vpr_eigen_places_salad.py`
When the suites run after this task
Then every previously-passing AC-6 (engine output-dim) sub-test still passes; no test file is modified
**AC-6: AZ-270 + AZ-507 layer lints pass**
Given the helper lives inside the c2_vpr component (Layer 3) and only imports from `gps_denied_onboard.config.schema` (Layer 1) and stdlib
When `tests/unit/test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies` runs
Then the test passes (helper is c2-internal, not a Public API surface, and respects layer direction)
## Constraints
- The helper module is c2-internal (`_engine_dim_assertion.py` underscore prefix). No other component may import it.
- `descriptor_dim`, `output_key`, `input_key` are passed as parameters — the helper has zero module-level constants describing per-backbone conventions.
- Error envelope preserved: raise `gps_denied_onboard.config.schema.ConfigError` (NOT `ValueError`, NOT a custom new error type). Error messages must continue to name both the expected and actual descriptor dims, plus the missing key when applicable.
- The dry-run probe input shape comes from `preprocessor.input_shape()` (the `BackbonePreprocessor` Protocol method). The helper does NOT hardcode any input shape — backbone-specific input shapes remain a per-strategy concern.
- Test additions go into a new `test_az527_engine_dim_assertion.py`; AZ-337 / 338 / 339 / 340 test files are NOT touched.
## Risks & Mitigation
**Risk 1: A 7-way refactor accidentally drops one call site, breaking `create()` for one strategy**
- *Risk*: One of the 7 modules forgets to call the helper or calls it with the wrong dim.
- *Mitigation*: AC-5 (all 4 existing AC-6 test files pass unmodified) is the safety net — if any strategy regresses, its AC-6 sub-test will fail.
**Risk 2: A future emitter forgets the helper exists and adds an 8th local copy**
- *Risk*: The pattern recurs as new c2_vpr backbones land (none planned in current scope, but possible for hypothetical c2-side adjacent work).
- *Mitigation*: Add an AST-walk regression guard inside `test_az527_engine_dim_assertion.py` that asserts zero stray `_assert_engine_output_dim` / `assert_engine_output_dim` definitions outside the helper module — modeled on the AZ-508 / AZ-526 `test_no_local_iso_ts_*_definitions_remain` pattern.
**Risk 3: The helper accidentally becomes a cross-component shared helper**
- *Risk*: A future C3/C4/C5 strategy author imports the helper from c2_vpr and creates a cross-component coupling.
- *Mitigation*: The helper is named `_engine_dim_assertion.py` (underscore prefix) inside the c2_vpr/ component folder, not in `helpers/`. The AZ-507 layer lint forbids cross-component imports of c2_vpr internals. If a C3/C4/C5 needs the same shape, the right move is to factor a c2-or-helpers Layer-1 module — out of scope here.
## Runtime Completeness
- **Named capability**: a c2-internal helper `assert_engine_output_dim(inference_runtime, handle, preprocessor, descriptor_dim, *, output_key, input_key) -> None` that runs a zero-init dry-run inference and validates the output dict shape against the expected descriptor dim.
- **Production code that must exist**: real `assert_engine_output_dim` function in `_engine_dim_assertion.py`; real import + call in each of the 7 strategy modules.
- **Allowed external stubs**: none for production code. The unit test uses fake `InferenceRuntime` and fake `BackbonePreprocessor` Protocol implementations (matching the existing AZ-337..AZ-340 fakes).
- **Unacceptable substitutes**: keeping one or more local definitions "for parity"; raising `ValueError` instead of `ConfigError`; hoisting the helper to `helpers/`; changing the `"embedding"` / `"input"` key defaults to per-backbone names (the parameters are the right tool).