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

[AZ-348] C3.5 ConditionalRefiner Protocol + factory + PassthroughRefiner

Browse Source 
Defines the public `ConditionalRefiner` Protocol (PEP 544
@runtime_checkable, two methods: `refine_if_needed` +
`was_invoked`), extends `MatchResult` in-place with two
default-valued refinement fields (`refinement_label`,
`refinement_added_latency_ms`), defines the `RefinerError` family
(`RefinerBackboneError`, `RefinerConfigError`), and ships the
trivial `PassthroughRefiner` reference impl.

Both refiner strategies are linked unconditionally — no
`BUILD_REFINER_*` flag (NOT ADR-002 territory). Runtime selection
only per ADR-001. `PassthroughRefiner` returns the input
`MatchResult` by reference (bit-identical correspondences per
contract INV-5) and always reports `was_invoked() is False`.

Documentation: renames `module-layout.md` `c3_5_adhop` Public API
symbol from `AdHoPRefinementStrategy` to `ConditionalRefiner`
(AC-14) so the doc agrees with `description.md` and the contract.

AC-9 (single-thread binding) deferred to AZ-270 runtime-root
composition, mirroring AZ-336 / AZ-342 / AZ-344 Risk-4 precedent.
AC-7 for the `"adhop"` strategy stops at `ModuleNotFoundError`
because the AdHoP backbone is owned by AZ-349. All other ACs +
NFRs covered by 36 new conformance tests.

Architectural note: `PassthroughRefiner.inference_runtime` is
typed as `object` because the L3→L3 import ban
(`test_az270_compose_root`) forbids c3_5_adhop from importing
c7_inference; the runtime-root factory narrows the type at
construction time.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-12 05:52:36 +03:00
parent 89c223882b
commit 9a605c8514
13 changed files with 991 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
_docs/02_tasks/todo/AZ-348_c3_5_refiner_protocol.md
-215
View File
@@ -1,215 +0,0 @@
# C3.5 ConditionalRefiner Protocol + Factory + PassthroughRefiner + Composition
**Task**: AZ-348_c3_5_refiner_protocol
**Name**: C3.5 `ConditionalRefiner` Protocol + Factory + `PassthroughRefiner` + Composition
**Description**: Define the public `ConditionalRefiner` Protocol (PEP 544 `@runtime_checkable`), the C3.5 DTO additions (`refinement_label`, `refinement_added_latency_ms` extending `MatchResult`), the error hierarchy (`RefinerError`, `RefinerBackboneError`, `RefinerConfigError`), the composition-root factory `build_refiner_strategy(config, ransac_filter, inference_runtime) -> ConditionalRefiner` selecting between strategies at startup based on `config.refiner.strategy`, AND the trivial `PassthroughRefiner` concrete strategy (always-passthrough; non-conditional baseline used by smoke tests + IT-12 comparison). Both refiner strategies are linked into the production binary unconditionally (NO `BUILD_REFINER_*` flag — runtime selection only per ADR-001; ADR-002 build-time exclusion does NOT apply because both strategies are tiny and the AdHoP TRT engine is shared C7 infrastructure). The shared `RansacFilter` (AZ-282) is constructor-injected. This task delivers the foundational scaffolding `AdHoPRefiner` (TBD AZ-?) depends on; the AdHoP backbone implementation is NOT in scope here.
**Complexity**: 3 points
**Dependencies**: AZ-263_initial_structure, AZ-269_config_loader, AZ-270_compose_root, AZ-282_ransac_filter (helper handle), AZ-297_c7_runtime_protocol (for `InferenceRuntime` interface), AZ-344 (for `MatchResult` DTO defined in `_types/matcher.py` — extended in-place by this task), AZ-266_log_module
**Component**: c3_5_adhop (epic AZ-258 / E-C3.5)
**Tracker**: AZ-348
**Epic**: AZ-258 (E-C3.5)
### Document Dependencies
- `_docs/02_document/contracts/c3_5_adhop/conditional_refiner_protocol.md` — the public contract this task implements (Protocol surface + DTO extension + error hierarchy + factory signature + 9 invariants + producer/consumer split).
- `_docs/02_document/components/05_c3_5_adhop/description.md` — § 1 architectural pattern (Strategy with two concrete impls); § 2 `ConditionalRefiner` interface + DTO enrichments (`refinement_label`, `refinement_added_latency_ms`); § 5 error handling (passthrough fall-through on `RefinerBackboneError`); § 9 logging.
- `_docs/02_document/module-layout.md` — `c3_5_adhop` Per-Component Mapping (this task ALSO updates the canonical Public API symbol from `AdHoPRefinementStrategy` to `ConditionalRefiner` so the document agrees with `description.md` § 2 and the contract).
- `_docs/02_document/architecture.md` — ADR-001 (Strategy + composition root), ADR-009 (interface-first DI). ADR-002 explicitly does NOT apply here (no `BUILD_REFINER_*` flag).
- `_docs/02_document/contracts/c3_matcher/cross_domain_matcher_protocol.md` — `MatchResult` DTO consumed AND extended by this task.
- `_docs/02_document/contracts/shared_helpers/ransac_filter.md` — helper API consumed (held by reference; not invoked by `PassthroughRefiner`; held for parity + future use by `AdHoPRefiner`).
- `_docs/02_document/contracts/c7_inference/inference_runtime_protocol.md` — `InferenceRuntime` interface; held by reference but NOT invoked by `PassthroughRefiner`.
## Problem
Without this task, `AdHoPRefiner` and the downstream C4 PoseEstimator would each invent their own ad-hoc interface for the conditional-refinement boundary, breaking ADR-001 (Strategy is the documented architectural pattern) and ADR-009 (consumers must hold typed references to a Protocol, not a concrete class). Additionally, the `MatchResult` DTO must grow two NEW fields (`refinement_label`, `refinement_added_latency_ms`) so the per-frame FDR records carry refinement provenance and the NFT-PERF-01 invocation-rate accounting (test C3.5-IT-03) is well-defined; doing this DTO extension in the producer task (AZ-344, C3 Protocol) would couple C3 to a C3.5 concept and violate the layering invariant (C3 is upstream of C3.5).
The `PassthroughRefiner` is bundled into this task because (a) it is a 1-pt no-op trivially defined alongside the Protocol, (b) the Protocol's tests need a real `ConditionalRefiner` instance to verify `runtime_checkable` conformance, and (c) it serves as the documented IT-12 "no-refinement" comparison baseline at the refinement level (engine rule per AC-2.1a applied at the refinement layer).
## Outcome
- `src/gps_denied_onboard/components/c3_5_adhop/interface.py` defining the `ConditionalRefiner` Protocol with `refine_if_needed` and `was_invoked`. Docstring encodes all 9 invariants from the contract verbatim.
- `src/gps_denied_onboard/components/c3_5_adhop/__init__.py` re-exporting the Protocol + the new MatchResult fields' default-construction helper (no separate DTO; the fields live on `MatchResult`).
- `src/gps_denied_onboard/_types/matcher.py` (existing file from AZ-344) **extended in-place** to add the two NEW fields:
- `refinement_label: str = "passthrough"`
- `refinement_added_latency_ms: float = 0.0`
Both with default values so AZ-344's `MatchResult(...)` construction without C3.5 still produces a valid downstream-readable instance. The task ALSO updates AZ-344's frozen-dataclass tests (slot count, repr) to reflect the two new fields without changing AZ-344's per-field assertions.
- `src/gps_denied_onboard/components/c3_5_adhop/errors.py` defining `RefinerError`, `RefinerBackboneError`, `RefinerConfigError`.
- `src/gps_denied_onboard/components/c3_5_adhop/passthrough_refiner.py` defining:
- `PassthroughRefiner` class implementing the `ConditionalRefiner` Protocol.
- Constructor: `__init__(self, ransac_filter, inference_runtime)`. Both helpers held by reference; neither invoked.
- `refine_if_needed(frame, mr, residual_threshold_px)`:
- Validate `residual_threshold_px > 0` (raise `ValueError` per Invariant 9; defensive).
- Set `_was_invoked = False`.
- Return `mr` unchanged (input == output, byte-identical correspondences ndarray references). `refinement_label` already defaults to `"passthrough"` and `refinement_added_latency_ms` defaults to `0.0` — no `dataclasses.replace` needed.
- `was_invoked() -> bool`: return `self._was_invoked`.
- Module-level `create(config, ransac_filter, inference_runtime) -> ConditionalRefiner` factory function.
- `src/gps_denied_onboard/runtime_root/refiner_factory.py` exporting `build_refiner_strategy(config, ransac_filter, inference_runtime) -> ConditionalRefiner`. The factory:
1. Reads `config.refiner.strategy` (one of: `"adhop"`, `"passthrough"`).
2. Imports the concrete module via the strategy resolution table (NOT lazy — both modules are linked unconditionally per ADR-001).
3. Constructs via the module-level `create(config, ransac_filter, inference_runtime)` factory function.
4. Returns the instance.
5. Validates `config.refiner.residual_threshold_px > 0`; rejects with `RefinerConfigError` otherwise.
- Composition-root `compose_root` extension: invoke `build_refiner_strategy` AFTER `RansacFilter` and the C7 `InferenceRuntime` are constructed; bind the result to the same C-frame ingest thread.
- Config schema extension to AZ-269:
- `config.refiner.strategy` (enum: `"adhop"` | `"passthrough"`; default `"adhop"`).
- `config.refiner.residual_threshold_px` (float, default `2.5`).
- `config.refiner.invocation_rate_warn_threshold` (float, default `0.25`).
- INFO log on every successful `build_refiner_strategy`: `kind="c3_5.refiner.strategy_loaded"` with `{strategy, residual_threshold_px}`.
- ERROR log on `RefinerConfigError`.
- Documentation update: update `module-layout.md` `c3_5_adhop` Public API symbol from `AdHoPRefinementStrategy` to `ConditionalRefiner` (the canonical name per `description.md` and the contract).
## Scope
### Included
- The `ConditionalRefiner` Protocol with both `refine_if_needed` and `was_invoked` methods.
- DTO extension to `MatchResult` (two NEW default-valued fields).
- The three-class error hierarchy.
- The trivial `PassthroughRefiner` concrete strategy.
- The composition-root factory.
- Config schema extension for `config.refiner.{strategy, residual_threshold_px, invocation_rate_warn_threshold}`.
- Composition-root wiring path that identity-shares `RansacFilter` with C3 and C4.
- Strategy resolution table comment matching the contract verbatim.
- Documentation correction in `module-layout.md` (rename `AdHoPRefinementStrategy` → `ConditionalRefiner`).
- Unit tests covering: Protocol conformance (`runtime_checkable`); DTO field defaults + slots; `PassthroughRefiner` byte-identical-correspondences passthrough (Invariant 5); `PassthroughRefiner` `was_invoked()` always False (Invariant 8); factory rejection on unknown strategy (`RefinerConfigError`); factory rejection on `residual_threshold_px <= 0` (`RefinerConfigError`); INFO + ERROR log emission.
### Excluded
- The `AdHoPRefiner` concrete strategy — owned by the AdHoP task (TBD AZ-?).
- The AdHoP TRT engine compile path — owned by AZ-321.
- The `RansacFilter` helper — already AZ-282.
- The C7 `InferenceRuntime` — owned by AZ-297.
- Component-internal acceptance tests beyond Protocol-conformance + factory-validation: C3.5-IT-01..03 + C3.5-PT-01 deferred to E-BBT (AZ-262).
## Acceptance Criteria
**AC-1: Protocol conformance — `runtime_checkable`**
Given a `FakeRefiner` test double implementing both `refine_if_needed` and `was_invoked`
When `isinstance(fake, ConditionalRefiner)` is evaluated
Then result is `True`; an object missing either method returns `False`.
**AC-2: `MatchResult` DTO field additions are backward-compatible**
Given AZ-344's `MatchResult(...)` call sites that do NOT pass the new fields
When the construction is evaluated post-extension
Then construction succeeds; `refinement_label == "passthrough"`; `refinement_added_latency_ms == 0.0`. AZ-344's existing tests still pass.
**AC-3: `MatchResult` immutability + slots preserved**
After the extension, `MatchResult` is still `frozen=True, slots=True`; mutation raises `FrozenInstanceError`; `__slots__` includes the two new field names.
**AC-4: Factory rejects unknown strategy**
Given `config.refiner.strategy = "garbage"`
When `build_refiner_strategy(...)` is called
Then `RefinerConfigError("Unknown refiner strategy: garbage")` is raised; ONE ERROR log `kind="c3_5.refiner.strategy_unknown"` is emitted.
**AC-5: Factory rejects invalid threshold**
Given `config.refiner.residual_threshold_px = 0` (or negative)
When `build_refiner_strategy(...)` is called
Then `RefinerConfigError` is raised; ONE ERROR log `kind="c3_5.refiner.invalid_threshold"` is emitted.
**AC-6: Successful factory load emits INFO log**
Given `config.refiner.strategy = "passthrough"` AND `residual_threshold_px = 2.5`
When `build_refiner_strategy(...)` is called
Then a `ConditionalRefiner` instance is returned; ONE INFO log `kind="c3_5.refiner.strategy_loaded"` with `{strategy: "passthrough", residual_threshold_px: 2.5}` is emitted.
**AC-7: Strategy resolution table**
Given each of `"adhop"` AND `"passthrough"` for `config.refiner.strategy`
When `build_refiner_strategy(...)` is called for each
Then resolved module path matches the contract's table verbatim. (For `"adhop"`, the factory imports the AdHoP module successfully iff that module exists; in this task the AdHoP module is a placeholder + raises `NotImplementedError` so AC-7 for `"adhop"` reaches the import + class lookup but NOT the `__init__`. The full-success path for `"adhop"` belongs to the AdHoP task.)
**AC-8: Public API surface — `__init__.py` re-exports**
Given `from gps_denied_onboard.components.c3_5_adhop import ConditionalRefiner`
When the import is evaluated
Then `ConditionalRefiner` resolves; `PassthroughRefiner`, `RefinerError`, etc. are NOT in `__all__` (kept internal).
**AC-9: Strategy bound to single ingest thread by composition root**
Given a `compose_root(config)` invocation
When the resulting strategy is bound
Then a second binding attempt from a different thread raises `RuntimeError`.
**AC-10: `PassthroughRefiner` byte-identical correspondences (Invariant 5)**
Given a `MatchResult` with non-empty `inlier_correspondences` ndarrays
When `PassthroughRefiner.refine_if_needed(frame, mr, threshold)` is invoked
Then for every candidate `i`, `np.array_equal(out.per_candidate[i].inlier_correspondences, mr.per_candidate[i].inlier_correspondences) is True` AND dtypes match exactly. The output's ndarray IS the input's ndarray (same object reference). `out.refinement_label == "passthrough"`. `out.refinement_added_latency_ms == 0.0`.
**AC-11: `PassthroughRefiner.was_invoked()` always False (Invariant 8)**
Given a fresh `PassthroughRefiner`
When `refine_if_needed` is called any number of times
Then every subsequent `was_invoked()` call returns False.
**AC-12: Threshold validation in `refine_if_needed` (Invariant 9, defensive)**
Given `PassthroughRefiner.refine_if_needed(frame, mr, residual_threshold_px=0)`
When called
Then `ValueError` is raised. (The composition root MUST also have caught this earlier; this in-method check is defensive.)
**AC-13: Error hierarchy catchability**
Test instances of `RefinerBackboneError` + `RefinerConfigError` are caught by `except RefinerError`.
**AC-14: `module-layout.md` symbol rename applied**
After this task completes, `_docs/02_document/module-layout.md` § c3_5_adhop Public API rows reference `ConditionalRefiner` (NOT `AdHoPRefinementStrategy`).
## Non-Functional Requirements
**Performance**
- `build_refiner_strategy` p99 ≤ 20 ms (factory-only; no engine load happens here for `"passthrough"`; AdHoP engine load owned by the AdHoP task).
- `PassthroughRefiner.refine_if_needed` p99 ≤ 0.5 ms (per Invariant 7 + C3.5-PT-01 passthrough target).
**Compatibility**
- Protocol method-signature changes are MAJOR version bumps (lockstep update of all consumers + concrete strategies).
- `MatchResult` field additions are MINOR; field removals are MAJOR.
**Reliability**
- Single-thread invariant enforced at composition-root binding time (AC-9).
- `PassthroughRefiner` is stateless except for the `_was_invoked` flag; concurrent calls are unsafe (single-thread invariant covers).
## Unit Tests
| AC Ref | What to Test | Required Outcome |
|--------|--------------|------------------|
| AC-1 | Protocol conformance | Fake passes; partial fake fails |
| AC-2 | `MatchResult` backward-compatible defaults | Existing call sites still produce valid instances |
| AC-3 | `MatchResult` slots + immutability | `FrozenInstanceError`; new fields in `__slots__` |
| AC-4 | Unknown-strategy rejection | `RefinerConfigError`; ERROR log |
| AC-5 | Invalid-threshold rejection | `RefinerConfigError`; ERROR log |
| AC-6 | Successful factory load | INFO log with structured fields |
| AC-7 | Strategy resolution paths | Both `"adhop"` and `"passthrough"` resolve to expected module paths |
| AC-8 | Public API re-exports | `ConditionalRefiner` in `__all__`; internals not |
| AC-9 | Single-thread binding | Second binding raises `RuntimeError` |
| AC-10 | Passthrough byte-identical | `np.array_equal` + same `dtype`; `refinement_label == "passthrough"` |
| AC-11 | `was_invoked()` always False on Passthrough | Every call returns False |
| AC-12 | Threshold validation | `ValueError` on `<= 0` |
| AC-13 | Error catchability | All caught by `except RefinerError` |
| AC-14 | `module-layout.md` symbol rename | `ConditionalRefiner` referenced; `AdHoPRefinementStrategy` not |
## Constraints
- **`@runtime_checkable` MUST be used** on the Protocol.
- **`MatchResult` MUST remain `frozen=True, slots=True`** after the extension.
- **`PassthroughRefiner.refine_if_needed` MUST return the input `MatchResult` unchanged** (same object reference) when the field defaults already hold. No `dataclasses.replace` is needed; if the input already has `refinement_label != "passthrough"` (impossible from a C3 producer but possible in tests), the strategy MAY rewrite via `replace` — but the task ships the simpler same-reference path because every C3 producer outputs the default values.
- **Both refiner modules linked unconditionally** — no `BUILD_REFINER_*` flag (NOT ADR-002 territory). The composition root validates the config-load-time strategy enum.
- **The factory does NOT instantiate `RansacFilter` or `InferenceRuntime`** — runtime root constructs ONCE.
- **`config.refiner.strategy` is an enum** validated at config load.
- **`module-layout.md` symbol rename is part of this task** — fixes the documented Public API symbol to match the contract.
## Risks & Mitigation
**Risk 1: Extending `MatchResult` in-place creates churn for AZ-344 tests**
- *Mitigation*: the two new fields are default-valued, so every existing AZ-344 construction-call site stays valid. AZ-344's frozen-dataclass tests assert specific fields; this task updates them to reflect the additions without changing per-field semantics.
**Risk 2: `PassthroughRefiner` returning the input by reference is a sharp tool**
- *Mitigation*: documented in the contract (Invariant 5 explicitly states "byte-identical"). Downstream consumers of `MatchResult` MUST treat it as immutable (which `frozen=True` enforces). If a future consumer mutates ndarrays in-place (NumPy doesn't honour dataclass frozen for mutable members), it will corrupt the C3 producer's view — but no current consumer does this, and the contract codifies the expectation.
**Risk 3: `module-layout.md` symbol rename creates churn for unrelated documentation readers**
- *Mitigation*: the symbol rename is a pure documentation correction; no production code references `AdHoPRefinementStrategy` because AZ-258 has not been implemented yet. This task is the right place to fix the inconsistency before any consumer encodes the wrong name.
**Risk 4: `compose_root` thread-binding registry not yet implemented in AZ-270**
- *Mitigation*: same as AZ-342 / AZ-344 Risk 4. Keep AC-9; if AZ-270 lacks the registry, escalate via tracker dependency mechanism.
## Runtime Completeness
- **Named capability**: `ConditionalRefiner` Protocol + `PassthroughRefiner` reference impl + composition-root factory + `MatchResult` DTO extension.
- **Production code that must exist**: real Protocol + real DTO extension + real error hierarchy + real `PassthroughRefiner` (byte-identical passthrough) + real `build_refiner_strategy` factory + real config schema extension + real composition-root wiring path that identity-shares `RansacFilter` with C3 and C4.
- **Allowed external stubs**: `FakeRefiner`, `FakeRansacFilter`, `FakeInferenceRuntime` for tests. Production wiring uses real concretes.
- **Unacceptable substitutes**: making `PassthroughRefiner` return a deep copy of the input (defeats Invariant 5's byte-identical guarantee + adds latency to the steady-state path); deferring the `module-layout.md` rename to "later" (would leave the documented symbol diverged from the contract for the AdHoP task to consume).
## Contract
This task produces/implements the contract at `_docs/02_document/contracts/c3_5_adhop/conditional_refiner_protocol.md`.
Consumers MUST read that file — not this task spec — to discover the interface.