[AZ-507] [AZ-508] Onboard hygiene PBIs from batches 31-33 review

Open two ~2-point hygiene PBIs surfaced by
_docs/03_implementation/cumulative_review_batches_31-33_cycle1_report.md:

- AZ-507 (parent AZ-246 / E-CC-CONF) — align module-layout.md
  cross-component import rules with the AZ-270 lint test. Resolves the
  doc-vs-lint contradiction surfaced on AZ-321 by tightening the doc
  (option (a) from the review) + hoisting EngineBuildError /
  CalibrationCacheError to _types/inference_errors.py.

- AZ-508 (parent AZ-264 / E-CC-HELPERS) — consolidate 5 identical
  _iso_ts_now() one-liners across c6_tile_cache + c7_inference into a
  single Layer-1 helper at helpers/iso_timestamps.py.

Dependencies table headers bumped: 142 -> 144 tasks, 478 -> 482 points
(product 345 -> 349). State file's pause-point detail cleared; next
sub_step is the implement skill's Step 3 (compute next batch) for
batch 34.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-13 01:27:04 +03:00
parent 692bbdb7a0
commit 08e657d433
4 changed files with 298 additions and 4 deletions
+17 -3
View File
@@ -1,8 +1,8 @@
# Dependencies Table
**Date**: 2026-05-11 (refreshed after AZ-489 + AZ-490 onboarding for ADR-010 operator-origin path)
**Total Tasks**: 142 (101 product + 41 blackbox-test)
**Total Complexity Points**: 478 (345 product + 133 blackbox-test)
**Date**: 2026-05-13 (refreshed after AZ-507 + AZ-508 hygiene-PBI onboarding from cumulative review batches 31-33; previously 2026-05-11 for AZ-489 + AZ-490 ADR-010 operator-origin path)
**Total Tasks**: 144 (103 product + 41 blackbox-test)
**Total Complexity Points**: 482 (349 product + 133 blackbox-test)
Dependencies columns list only the tracker-ID portion (descriptive tail
text in each task spec is omitted here for table-readability). The
@@ -156,6 +156,8 @@ are all declared and documented below under **Cycle Check**.
| AZ-446 | CSV reporter refinements — trend-line + acceptance-band annotations + Monte Carlo CI | 2 | AZ-406, AZ-445 | AZ-262 |
| AZ-489 | C12 FlightsApiClient — fetch Flight from suite flights service + offline JSON fallback | 3 | AZ-263, AZ-269, AZ-266, AZ-279, AZ-280 | AZ-253 |
| AZ-490 | C5 set_takeoff_origin entrypoint — accept operator origin from C10 Manifest | 3 | AZ-263, AZ-269, AZ-266, AZ-272, AZ-273, AZ-279, AZ-381, AZ-383, AZ-384, AZ-385, AZ-386 | AZ-260 |
| AZ-507 | Hygiene — align module-layout.md cross-component import rules with AZ-270 lint | 2 | AZ-263, AZ-270, AZ-321 | AZ-246 |
| AZ-508 | Hygiene — consolidate `_iso_ts_now` helpers into `helpers/iso_timestamps.py` | 2 | AZ-263 | AZ-264 |
## Notes
@@ -211,6 +213,18 @@ are all declared and documented below under **Cycle Check**.
- **All E-BBT tasks depend on AZ-406 (test infrastructure)**; this is
by design — AZ-406 is the foundation every blackbox test depends on
(analogous to AZ-263 for the product side).
- **Hygiene PBIs from cumulative review batches 31-33** (added
2026-05-13):
- **AZ-507** (E-CC-CONF / AZ-246) — module-layout.md ↔ AZ-270 lint
alignment. Depends on AZ-263 (structure), AZ-270 (the lint test
+ composition root), and AZ-321 (the engine_compiler whose
`except Exception` is narrowed). All deps are backward and
completed; AZ-507 can land in any future batch. NOT a gate on
AZ-322 / AZ-337 (the cumulative review F2 notes hygiene defers).
- **AZ-508** (E-CC-HELPERS / AZ-264) — `_iso_ts_now` helper
consolidation. Depends on AZ-263 only. Stateless stdlib helper;
no cross-component or runtime dependencies. NOT a gate on any
future product task; can ship in a hygiene-only batch.
## Coverage Verification (Implementation Mode)
@@ -0,0 +1,127 @@
# Hygiene — Align module-layout.md cross-component import rules with AZ-270 lint
**Task**: AZ-507_hygiene_module_layout_az270_alignment
**Name**: Module-layout ↔ AZ-270 lint alignment
**Description**: Resolve the architecture contradiction surfaced by cumulative review batches 3133 (Finding F1, Medium / Architecture): `_docs/02_document/module-layout.md` allows c10 → c7 imports via `(Public API)` while the AZ-270 lint (`test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies`) forbids ANY cross-component import. Apply option (a) from the review: tighten the doc to match the lint. Rewrite every `module-layout.md` *"Imports from"* line that names `components.X (Public API)` to instead point at `_types`. Hoist `EngineBuildError` + `CalibrationCacheError` to `_types/inference_errors.py` so consumers catch a typed envelope instead of widening to `except Exception`.
**Complexity**: 2 points
**Dependencies**: AZ-263_initial_structure, AZ-270 (Composition Root + AZ-270 lint)
**Component**: cross-cutting (architecture compliance — Configuration & Composition Root epic E-CC-CONF)
**Tracker**: AZ-507
**Epic**: AZ-246 (E-CC-CONF)
### Document Dependencies
- `_docs/02_document/module-layout.md` — the document being edited. Every line tagged `(Public API)` for a cross-component import must be replaced or removed.
- `_docs/03_implementation/cumulative_review_batches_31-33_cycle1_report.md` § F1 — the finding being closed.
- `_docs/02_document/architecture.md` — confirm the `_types`-anchored DTO pattern is the canonical cross-component contract surface.
## Problem
After AZ-321 (batch 33), `_docs/02_document/module-layout.md` and `tests/unit/test_az270_compose_root.py::test_ac6` describe contradictory rules:
- The doc tells implementers c10/c11/c12/c2/etc. MAY import from `components.X (Public API)`.
- The lint walks every `.py` under `src/gps_denied_onboard/components/` and FAILS the build on any `from gps_denied_onboard.components.X import …` where X is not the importer's own component — regardless of `Public API` status.
The lint is strictly stricter than the doc. The first task that took the doc at its word (AZ-321 `engine_compiler.py`) failed the lint immediately and had to pivot to a consumer-side `CompileEngineCallable` Protocol cut + broad `except Exception`. The pivot was correct, but the next c10/c11/c12 task to need C7 Public API symbols will hit the same wall. The contradiction must be resolved at the source.
## Outcome
- `_docs/02_document/module-layout.md` no longer references `components.X (Public API)` in any "Imports from" line. Every such reference is replaced by `_types` (DTOs already live there) or removed if the import is dead.
- A new shim module `src/gps_denied_onboard/_types/inference_errors.py` re-exports `EngineBuildError` and `CalibrationCacheError` from `c7_inference` internals (the canonical definitions stay in c7_inference; the shim is a Layer-0 typed envelope).
- `c10_provisioning/engine_compiler.py::_compile_one` narrows its `except Exception` catch to the union of `EngineBuildError | CalibrationCacheError` imported from `_types/inference_errors.py`. All other catch-all behavior is preserved (unknown exceptions still bubble up).
- The AZ-270 lint test (`tests/unit/test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies`) continues to pass without modification.
- The existing c7_inference + c10_provisioning unit test suites continue to pass without modification.
## Scope
### Included
- Edits to `_docs/02_document/module-layout.md` for c10_provisioning + any other component whose "Imports from" line names `components.X (Public API)`.
- Create `src/gps_denied_onboard/_types/inference_errors.py` with `EngineBuildError` and `CalibrationCacheError` re-exports.
- Narrow `c10_provisioning/engine_compiler.py::_compile_one`'s `except Exception` catch to the typed envelope.
- Add a doc-level note in `_docs/02_document/architecture.md` (one paragraph under the existing cross-component contract section) codifying the rule: cross-component imports go through `_types`, never through `components.X (Public API)`.
### Excluded
- Option (b) from the cumulative review (loosen the lint to read a Public API allowlist). Rejected: adds parser fragility to the lint.
- Refactoring any concrete behavior. This is documentation + a typed-error shim + one narrowed catch.
- Renaming public symbols (`EngineBuildError` and `CalibrationCacheError` keep their c7_inference canonical names; the shim re-exports under the same names).
- The F3 finding ("Consumer-Side Protocol Cuts" architecture-doc section). That is a separate hygiene PBI if/when promoted.
- Touching any consumer other than `engine_compiler.py`. There is currently only one consumer of the c7 error types (c10_provisioning).
## Acceptance Criteria
**AC-1: module-layout.md has no `components.X (Public API)` imports**
Given the AZ-321 module-layout edits documenting c10's cross-component imports
When the doc is re-read after this task
Then no "Imports from" line names `components.X (Public API)`; every former such reference points at `_types` or has been removed as dead
**AC-2: `_types/inference_errors.py` re-exports the c7 typed error envelope**
Given the new shim `src/gps_denied_onboard/_types/inference_errors.py`
When a consumer imports `from gps_denied_onboard._types.inference_errors import EngineBuildError, CalibrationCacheError`
Then both symbols resolve and are identical (same class objects) to the canonical c7_inference definitions
**AC-3: engine_compiler narrows its catch**
Given `c10_provisioning/engine_compiler.py::_compile_one` previously had `except Exception`
When the task lands
Then the catch is `except (EngineBuildError, CalibrationCacheError)` (or equivalent typed union); unknown exceptions propagate; all existing `engine_compiler` unit tests pass without test modification
**AC-4: AZ-270 lint still passes**
Given the rewritten doc + new shim + narrowed catch
When `pytest tests/unit/test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies` runs
Then the test passes (no new cross-component imports introduced)
**AC-5: Architecture doc codifies the rule**
Given `_docs/02_document/architecture.md` has the existing cross-component contract section
When this task lands
Then a new one-paragraph note under that section says: cross-component imports go through `_types/*.py` (DTOs + typed error envelopes), never through `components.X (Public API)`; the only exception is `runtime_root/*` (composition root)
**AC-6: No behavior change**
Given the c7_inference + c10_provisioning unit test suites
When the suites run after this task
Then every previously passing test still passes; every previously skipped test is still skipped for the same reason; no new failures or warnings
## Non-Functional Requirements
**Compatibility**
- No new third-party dependencies.
- No bump to numpy / opencv / pytest / mypy pins.
**Reliability**
- The `_types/inference_errors.py` shim is import-only; it does not introduce module-load side effects.
## Unit Tests
| AC Ref | What to Test | Required Outcome |
|--------|-------------|------------------|
| AC-2 | `import` from `_types.inference_errors` resolves both symbols | Identity check `is` matches c7 canonical classes |
| AC-3 | `engine_compiler._compile_one` propagates non-typed exceptions | `RuntimeError("test")` bubbles up; `EngineBuildError("test")` is caught |
| AC-4 | Run AZ-270 lint test | Pass |
| AC-6 | Full c7_inference + c10_provisioning suite | All previously-passing tests still pass |
## Constraints
- The canonical class definitions for `EngineBuildError` and `CalibrationCacheError` STAY in c7_inference. The shim is purely a re-export at the `_types` Layer-0 surface — moving the canonical definitions would change the c7 surface and risk breaking the c7 internal call sites.
- `_types/*.py` must not import from `gps_denied_onboard.components.*`. The shim achieves this via a local re-export idiom: `from gps_denied_onboard.components.c7_inference._errors import EngineBuildError, CalibrationCacheError`. The `_types` surface itself remains Layer 0; the *import direction* points downward into a component, which the AZ-270 lint already allows for `_types`-internal modules (verified — the lint only walks `components/`, not `_types/`).
- No public API surface changes. All current consumers that already catch `Exception` continue to work.
## Risks & Mitigation
**Risk 1: A consumer relies on the broad `Exception` catch to swallow non-EngineBuildError errors**
- *Risk*: Tightening the catch in `engine_compiler._compile_one` exposes a previously-suppressed error.
- *Mitigation*: Audit the c7_inference internals before the change. Currently only `EngineBuildError` and `CalibrationCacheError` are documented in c7's public surface; everything else is a programmer error. AC-3's unit test covers the propagation contract.
**Risk 2: `_types/inference_errors.py` re-export introduces an import cycle**
- *Risk*: c7_inference somehow imports back from `_types/inference_errors`, creating `c7_inference → _types/inference_errors → c7_inference._errors → c7_inference`.
- *Mitigation*: c7_inference defines its errors in `_errors.py` (or similar); it doesn't need to import from `_types`. Verify no such backward import exists before landing. Add an explicit AZ-270-style test if a back-edge appears.
**Risk 3: Architecture doc note becomes stale if `_types` is reorganized**
- *Risk*: A future refactor moves DTOs out of `_types`; the note still says "go through `_types`".
- *Mitigation*: Keep the note short and rule-shaped ("cross-component contract surface lives in the project's Layer-0 typed envelope"). The exact path is incidental to the rule.
## Runtime Completeness
- **Named capability**: architecture-compliance documentation that matches the AZ-270 enforcement gate; small typed-error envelope shim; one narrowed catch.
- **Production code that must exist**: real `_types/inference_errors.py` shim + real narrowed `except (EngineBuildError, CalibrationCacheError)` in `engine_compiler._compile_one`.
- **Allowed external stubs**: none. This task is pure refactor + docs.
- **Unacceptable substitutes**: a `# type: ignore` over the broad catch (defeats the typed envelope intent); deleting the c7 error classes and moving them wholesale into `_types` (changes c7's surface and risks regression in tests that import directly from c7); leaving the broad `except Exception` (defeats the entire reason F1 was opened).
@@ -0,0 +1,153 @@
# Hygiene — Consolidate `_iso_ts_now` helpers into `helpers/iso_timestamps.py`
**Task**: AZ-508_hygiene_iso_timestamps_consolidation
**Name**: ISO-timestamp helper consolidation
**Description**: Replace five identical private `_iso_ts_now()` one-liners across c6_tile_cache + c7_inference with a single Layer-1 helper `src/gps_denied_onboard/helpers/iso_timestamps.py` exposing `iso_ts_now() -> str`. Closes cumulative review batches 3133 Finding F2 (Low / Maintainability) and the earlier 2830 cumulative review Finding F3 (the 3 c6 copies).
**Complexity**: 2 points
**Dependencies**: AZ-263_initial_structure, AZ-266_log_module
**Component**: helpers (epic AZ-264 / E-CC-HELPERS)
**Tracker**: AZ-508
**Epic**: AZ-264 (E-CC-HELPERS)
### Document Dependencies
- `_docs/02_document/common-helpers/` — the canonical home for shared utilities. This task adds a 9th entry. No new doc file required (the helper is mechanical enough to be self-documenting); the module-layout.md helpers table gains a row.
- `_docs/03_implementation/cumulative_review_batches_31-33_cycle1_report.md` § F2 — the finding being closed.
- `_docs/03_implementation/cumulative_review_batches_28-30_cycle1_report.md` § F3 — the earlier c6 instances also closed here.
## Problem
Five modules across two components each define a private one-line UTC-ISO timestamp helper:
- `src/gps_denied_onboard/components/c7_inference/tensorrt_runtime.py` (AZ-298)
- `src/gps_denied_onboard/components/c7_inference/onnx_trt_ep_runtime.py` (AZ-299)
- `src/gps_denied_onboard/components/c6_tile_cache/postgres_filesystem_store.py` (AZ-305)
- `src/gps_denied_onboard/components/c6_tile_cache/freshness_gate.py` (AZ-307)
- `src/gps_denied_onboard/components/c6_tile_cache/cache_budget_enforcer.py` (AZ-308)
Each returns `datetime.now(timezone.utc).isoformat(timespec="microseconds")` (or the equivalent UTC-with-microseconds string) for FDR record timestamps. The function is mechanical, deterministic, and not component-specific. Every additional C7/C6/C10 task that emits FDR records is at risk of adding the 6th, 7th, … copy unless a Layer-1 helper exists.
## Outcome
- A new helper module `src/gps_denied_onboard/helpers/iso_timestamps.py` exposing a single public function `iso_ts_now() -> str` (no class wrapper; helpers prefer free functions when stateless).
- The five existing component modules import `iso_ts_now` from the helper and delete their local `_iso_ts_now()` definitions. Call sites keep working without renaming (the local symbol can re-alias the import).
- A new unit test `tests/unit/helpers/test_iso_timestamps.py` covering format, timezone, and monotonicity contracts.
- A row added to `_docs/02_document/module-layout.md`'s helpers section (or the `_docs/02_document/common-helpers/` index) listing the new helper alongside the existing 8.
## Scope
### Included
- Create `src/gps_denied_onboard/helpers/iso_timestamps.py`:
```python
from datetime import datetime, timezone
def iso_ts_now() -> str:
return datetime.now(timezone.utc).isoformat(timespec="microseconds")
```
The `timespec="microseconds"` argument is what produces the `.ffffff` suffix that the FDR record format already relies on. Re-confirm by reading the existing `_iso_ts_now()` definitions before final write.
- Replace the five local `_iso_ts_now()` definitions with imports from the helper. Preferred form:
```python
from gps_denied_onboard.helpers.iso_timestamps import iso_ts_now as _iso_ts_now
```
This keeps the call-site symbol names unchanged, so no other line of each component module needs editing.
- Add `tests/unit/helpers/test_iso_timestamps.py` covering:
- AC-1: format regex match (`^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}\+00:00$`)
- AC-2: returned string parses back via `datetime.fromisoformat(...)` to a UTC-aware datetime
- AC-3: two successive calls produce monotonically non-decreasing strings (string comparison is correct for ISO-8601 UTC microsecond format)
- Add the helper to the `_docs/02_document/module-layout.md` helpers row table (or the `common-helpers/` index, whichever is the canonical inventory).
### Excluded
- Any change to `flight_clock`, `wall_clock`, or other adapter-style time sources. Those are domain-specific clocks; this helper is a low-level UTC-ISO formatter.
- Bumping FDR schema versions. The output format is bit-identical to the current local definitions; no schema change.
- Migrating timestamp call sites outside the five flagged files. Any future emitter should use the helper from the start; existing non-flagged emitters are out of scope.
- Creating a `Clock` Protocol or replacing `datetime.now(timezone.utc)` with an injectable clock. That is a larger architectural change (testability) not in scope here.
- Touching FDR record schema tests (`tests/unit/test_az272_fdr_record_schema.py`). They should continue to pass without modification.
## Acceptance Criteria
**AC-1: Helper exists at the canonical path**
Given a fresh checkout
When `from gps_denied_onboard.helpers.iso_timestamps import iso_ts_now` is run
Then the import succeeds; the function is callable; the returned value is a `str`
**AC-2: Output format is byte-identical to the previous local helpers**
Given the existing FDR record format that the five local helpers produced
When `iso_ts_now()` is called
Then the output matches `^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}\+00:00$` and is parseable by `datetime.fromisoformat(...)` into a UTC-aware datetime
**AC-3: Monotonicity under normal clock**
Given two `iso_ts_now()` calls separated by at least 1 microsecond
When the returned strings are compared lexicographically
Then the second is `>=` the first (string comparison is valid for fixed-width ISO-8601 UTC microsecond format)
**AC-4: All five call sites migrated**
Given a `grep -rn "def _iso_ts_now" src/` after the task lands
When the search runs
Then zero matches are found in `c6_tile_cache/` or `c7_inference/` (or anywhere under `src/`); the helper is the only definition
**AC-5: FDR record schema tests still pass without modification**
Given the existing `tests/unit/test_az272_fdr_record_schema.py` suite
When the suite runs after this task
Then every previously-passing test still passes; no test file is modified
**AC-6: No new third-party dependencies**
Given `pyproject.toml`
When the task lands
Then the dependency set is unchanged (only stdlib `datetime` + `timezone` are used)
**AC-7: AZ-270 lint still passes**
Given the helpers module lives at Layer 1 (`src/gps_denied_onboard/helpers/`)
When `tests/unit/test_az270_compose_root.py::test_ac6` runs
Then the test passes (the lint only walks `src/gps_denied_onboard/components/`, so a helper import is allowed)
## Non-Functional Requirements
**Performance**
- `datetime.now(timezone.utc).isoformat(timespec="microseconds")` is the same call the existing local helpers make; this task adds zero runtime cost.
**Compatibility**
- Python stdlib only. No new third-party dependencies.
**Reliability**
- Deterministic format string; no platform-specific formatting branches.
## Unit Tests
| AC Ref | What to Test | Required Outcome |
|--------|-------------|------------------|
| AC-1 | Import + call | Returns str |
| AC-2 | Regex match + `fromisoformat` round-trip | Both pass |
| AC-3 | Two successive calls with `time.sleep(0.000002)` between | `second >= first` |
| AC-4 | Grep gate at PR-CI level (or covered by an existing import-discipline test) | Zero matches outside the helper module |
| AC-5 | Run `tests/unit/test_az272_fdr_record_schema.py` after the migration | All tests pass without modification |
| AC-7 | Run AZ-270 lint test | Pass |
## Constraints
- The helper is a pure free function. No class wrapping, no instance state.
- The helper MUST NOT import from `gps_denied_onboard.components.*` (Layer 1 discipline).
- Call sites in c6_tile_cache + c7_inference MAY keep the local `_iso_ts_now` alias for minimal diff (`from … import iso_ts_now as _iso_ts_now`). This is preferred over editing every call site.
- The output format is locked at `isoformat(timespec="microseconds")` — switching to seconds or nanoseconds would change FDR record bit-shape and is out of scope.
## Risks & Mitigation
**Risk 1: One of the five local helpers actually differs subtly (e.g., omits microseconds, uses `'Z'` suffix)**
- *Risk*: Migrating naively changes the FDR record format for some kinds.
- *Mitigation*: Before final write, read each of the five existing definitions and diff their output format. If any differ, document the discrepancy in this task's report and either (a) align the divergent caller to the canonical format (FDR records standardize on `+00:00` per existing `test_az272_fdr_record_schema.py` fixtures), or (b) split into two helpers if a real domain difference exists. AC-5's unaltered schema test suite is the safety net.
**Risk 2: Future emitter forgets the helper exists and adds a 6th local copy**
- *Risk*: The pattern recurs.
- *Mitigation*: Add a one-line note in `_docs/02_document/common-helpers/` index referencing this helper, and in module-layout.md's helpers table. Code review of future C7/C6/C10/C11 batches should reject local timestamp helpers.
**Risk 3: A circular import (helpers → components)**
- *Risk*: The helpers/ package somehow back-imports from components.
- *Mitigation*: The helper only uses stdlib `datetime` + `timezone`. No risk of cycle.
## Runtime Completeness
- **Named capability**: a single Layer-1 UTC-ISO timestamp helper that replaces five duplicated private one-liners.
- **Production code that must exist**: real `iso_ts_now()` function at the documented path; real import + delete in each of the five flagged modules.
- **Allowed external stubs**: none. This task is pure consolidation.
- **Unacceptable substitutes**: keeping one or more local `_iso_ts_now()` "for parity" (defeats the consolidation); using `datetime.utcnow()` (deprecated, returns naive datetime — would break the timezone-aware contract); changing the format string (would invalidate FDR records).
+1 -1
View File
@@ -8,7 +8,7 @@ status: in_progress
sub_step:
phase: 3
name: compute-next-batch
detail: "open hygiene PBI first (cumulative_review_batches_31-33 F1+F2: _iso_ts_now consolidation + module-layout doc-vs-AZ-270-lint alignment, ~2 pts) then start batch 34"
detail: ""
retry_count: 0
cycle: 1
tracker: jira