diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index 5ad3807..e3bf264 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -363,9 +363,9 @@ Bootstrap reference: `_docs/02_tasks/todo/AZ-263_initial_structure.md`. Architec ### shared/helpers/iso_timestamps - **File**: `src/gps_denied_onboard/helpers/iso_timestamps.py` -- **Purpose**: Single Layer-1 UTC ISO-8601 timestamp source for FDR record envelopes (`iso_ts_now() -> str`, format `YYYY-MM-DDTHH:MM:SS.ffffffZ` matching the canonical FDR `_TS` fixture). Replaces the duplicated private `_iso_ts_now` one-liners that previously lived inside `c6_tile_cache` (3 modules — already locally consolidated under `_timestamp.py`) and `c7_inference` (2 modules). Stateless free function; stdlib only. -- **Owned by**: AZ-264 (AZ-508 consolidation task). -- **Consumed by**: c6_tile_cache (`cache_budget_enforcer`, `postgres_filesystem_store`, `freshness_gate`), c7_inference (`onnx_trt_ep_runtime`, `thermal_publisher`); future C10/C11 FDR producers should import this helper rather than redefining the one-liner locally. +- **Purpose**: Single Layer-1 UTC ISO-8601 timestamp source for FDR record envelopes. Two surfaces — `iso_ts_now() -> str` (wall-clock, microsecond precision, format `YYYY-MM-DDTHH:MM:SS.ffffffZ`) and `iso_ts_from_clock(clock: Clock) -> str` (Clock-injected, nanosecond precision, format `YYYY-MM-DDTHH:MM:SS.fffffffffZ`). Both terminate with the canonical `Z` suffix matching the FDR `_TS` fixture. Replaces the duplicated private `_iso_ts_now` (AZ-508) and `_iso_ts_from_clock` (AZ-526) one-liners that previously lived inside `c6_tile_cache`, `c7_inference`, `c2_vpr`, and `c12_operator_orchestrator`. Stateless functions; stdlib + Layer-1 `gps_denied_onboard.clock.Clock` only. +- **Owned by**: AZ-264 (AZ-508 + AZ-526 consolidation tasks). +- **Consumed by**: c6_tile_cache (`cache_budget_enforcer`, `postgres_filesystem_store`, `freshness_gate`) + c7_inference (`onnx_trt_ep_runtime`, `thermal_publisher`) via `iso_ts_now`; c2_vpr (`net_vlad`, `ultra_vpr`, `_faiss_bridge`) + c12_operator_orchestrator (`operator_reloc_service`) via `iso_ts_from_clock`. Future C3 / C4 / C5 FDR producers should import the appropriate helper rather than redefining the one-liner locally. ### shared/frame_source diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 6257b7d..c891ca4 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -1,8 +1,8 @@ # Dependencies Table -**Date**: 2026-05-13 (refreshed after Batch 44 SRP refactor: AZ-317 superseded; AZ-329 + AZ-330 specs rewritten; AZ-523 + AZ-524 audit-trail tickets added; E-C12 epic renamed `Operator Pre-flight Tooling` → `Operator Pre-flight Orchestrator`; earlier same-day refresh: AZ-507 + AZ-508 hygiene PBIs from cumulative review batches 31-33; 2026-05-11: AZ-489 + AZ-490 ADR-010 operator-origin path) -**Total Tasks**: 146 (105 product + 41 blackbox-test) — AZ-317 retained in the table marked SUPERSEDED for audit; AZ-523 (C11 gate removal) + AZ-524 (C12 rename) added as 2 closed audit-trail tasks -**Total Complexity Points**: 487 (354 product + 133 blackbox-test) — AZ-523 = 3pt, AZ-524 = 2pt +**Date**: 2026-05-13 (refreshed after Batch 48: AZ-526 hygiene PBI added from cumulative review batches 46-48 F1+F3; earlier same-day refresh after Batch 44 SRP refactor: AZ-317 superseded; AZ-329 + AZ-330 specs rewritten; AZ-523 + AZ-524 audit-trail tickets added; E-C12 epic renamed `Operator Pre-flight Tooling` → `Operator Pre-flight Orchestrator`; earlier same-day refresh: AZ-507 + AZ-508 hygiene PBIs from cumulative review batches 31-33; 2026-05-11: AZ-489 + AZ-490 ADR-010 operator-origin path) +**Total Tasks**: 147 (106 product + 41 blackbox-test) — AZ-317 retained in the table marked SUPERSEDED for audit; AZ-523 (C11 gate removal) + AZ-524 (C12 rename) added as 2 closed audit-trail tasks; AZ-526 = 2pt clock-helper hygiene +**Total Complexity Points**: 489 (356 product + 133 blackbox-test) — AZ-523 = 3pt, AZ-524 = 2pt, AZ-526 = 2pt Dependencies columns list only the tracker-ID portion (descriptive tail text in each task spec is omitted here for table-readability). The @@ -158,6 +158,7 @@ are all declared and documented below under **Cycle Check**. | 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 | +| AZ-526 | Hygiene — add `iso_ts_from_clock(clock)` to `helpers/iso_timestamps.py` | 2 | AZ-508, AZ-398 | AZ-264 | | AZ-523 | Batch 44 — C11 internal flight-state gate removal (SRP refactor; audit-trail; closed) | 3 | AZ-317, AZ-319, AZ-329 | AZ-251 | | AZ-524 | Batch 44 — C12 package rename: c12_operator_tooling → c12_operator_orchestrator (audit; closed)| 2 | AZ-263, AZ-326, AZ-327, AZ-328, AZ-329, AZ-330, AZ-489 | AZ-253 | diff --git a/_docs/02_tasks/done/AZ-526_hygiene_iso_ts_from_clock_consolidation.md b/_docs/02_tasks/done/AZ-526_hygiene_iso_ts_from_clock_consolidation.md new file mode 100644 index 0000000..3fb3e9f --- /dev/null +++ b/_docs/02_tasks/done/AZ-526_hygiene_iso_ts_from_clock_consolidation.md @@ -0,0 +1,137 @@ +# Hygiene — Consolidate `_iso_ts_from_clock` into `helpers/iso_timestamps.py` + +**Task**: AZ-526_hygiene_iso_ts_from_clock_consolidation +**Name**: Clock-injected ISO-timestamp helper consolidation +**Description**: Replace the four duplicated `_iso_ts_from_clock(clock)` definitions (two module-level free functions in `c2_vpr/{net_vlad,ultra_vpr}.py`, two instance methods in `c2_vpr/_faiss_bridge.py` and `c12_operator_orchestrator/operator_reloc_service.py`) with a single Layer-1 helper alongside `iso_ts_now()` in `src/gps_denied_onboard/helpers/iso_timestamps.py`. Closes cumulative review batches 46–48 Findings F1 (Medium / Maintainability) and F3 (Low / Maintainability). +**Complexity**: 2 points +**Dependencies**: AZ-508 (the `helpers/iso_timestamps.py` module must exist first), AZ-398 (Clock Protocol must exist — already shipped) +**Component**: helpers (epic AZ-264 / E-CC-HELPERS) +**Tracker**: AZ-526 +**Epic**: AZ-264 (E-CC-HELPERS) + +### Document Dependencies + +- `_docs/03_implementation/cumulative_review_batches_46-48_cycle1_report.md` § F1 + F3 — the findings being closed. +- `_docs/02_document/module-layout.md` § `shared/helpers/iso_timestamps` — the helpers row that the new function will extend. +- `_docs/02_tasks/done/AZ-508_hygiene_iso_timestamps_consolidation.md` — the precedent task (Batch 48) that shipped `iso_ts_now()` and whose scope explicitly Excluded the Clock-injected variant. + +## Problem + +Four modules across two components define the same Clock-injected ISO-timestamp helper: + +| File | Surface | Source format | +|------|---------|---------------| +| `src/gps_denied_onboard/components/c2_vpr/net_vlad.py` | module-level free function | `+00:00` suffix, 9-digit nanos | +| `src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py` | module-level free function | `+00:00` suffix, 9-digit nanos | +| `src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py` | instance method on `FaissBridge` | `+00:00` suffix, 9-digit nanos | +| `src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py` | instance method on `OperatorReLocService` | `+00:00` suffix, 9-digit nanos | + +The bodies are byte-identical modulo `clock` parameter vs `self._clock`. Both `net_vlad.py` and `ultra_vpr.py` carry an inline comment saying "AZ-508 will consolidate the duplicate helpers across c2/c11/c12/c6" — but AZ-508 explicitly Excluded this variant. `_faiss_bridge.py` carries a meta-rule citation rejecting consolidation; that rejection is invalidated now that AZ-508 has established the helpers/iso_timestamps.py home. + +AZ-339 (MegaLoc + MixVPR), AZ-340 (SelaVPR + EigenPlaces + SALAD), AZ-358 (C4 OpenCVGtsam pose), and AZ-389 (C5 orthorectifier) will each add 1–3 more copies unless consolidated first. + +## Outcome + +- The existing helper module `src/gps_denied_onboard/helpers/iso_timestamps.py` gains a second public function: `iso_ts_from_clock(clock: Clock) -> str`. Format `YYYY-MM-DDTHH:MM:SS.fffffffffZ` (9-digit nanosecond precision, `Z` suffix matching `iso_ts_now()` and the canonical FDR `_TS` fixture). +- The four duplicate definitions are deleted; consumers import from the helper. Module-level callers use `from … import iso_ts_from_clock as _iso_ts_from_clock`; the two instance methods are rewritten to one-line delegations (`return iso_ts_from_clock(self._clock)`). +- `_docs/02_document/module-layout.md` `shared/helpers/iso_timestamps` row is updated to mention the second function. +- A new AC test set is added to `tests/unit/test_az508_iso_timestamps.py` covering AC-1..AC-5 of this task. + +## Scope + +### Included + +- Add to `src/gps_denied_onboard/helpers/iso_timestamps.py`: + + ```python + from gps_denied_onboard.clock import Clock + + def iso_ts_from_clock(clock: Clock) -> str: + """Return an RFC 3339 UTC timestamp built from `clock.time_ns()`. + + Format: ``YYYY-MM-DDTHH:MM:SS.fffffffffZ`` (9-digit nanosecond + precision, ``Z`` suffix). Use when the timestamp must follow an + injectable Clock (replay tests, deterministic fixtures); + use ``iso_ts_now()`` otherwise. + """ + ns = int(clock.time_ns()) + seconds, fraction_ns = divmod(ns, 1_000_000_000) + dt = datetime.fromtimestamp(seconds, tz=timezone.utc) + return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}Z" + ``` + +- Re-export `iso_ts_from_clock` through `src/gps_denied_onboard/helpers/__init__.py` and add it to `__all__`. +- Migrate the four call sites: + - `c2_vpr/net_vlad.py`: delete the free function; add `from gps_denied_onboard.helpers.iso_timestamps import iso_ts_from_clock as _iso_ts_from_clock` at the top. + - `c2_vpr/ultra_vpr.py`: same. + - `c2_vpr/_faiss_bridge.py`: rewrite `_iso_ts_from_clock(self)` to a one-line delegation `return iso_ts_from_clock(self._clock)`; remove the stale meta-rule comment block. Add the helper import at the top. + - `c12_operator_orchestrator/operator_reloc_service.py`: same pattern as `_faiss_bridge.py`. +- Extend `tests/unit/test_az508_iso_timestamps.py` with new AC tests (AC-1..AC-5 below). Reuse the existing test file rather than creating a new one; the helpers/iso_timestamps.py module surface is jointly tested. +- Update `_docs/02_document/module-layout.md` `shared/helpers/iso_timestamps` row to describe both functions. + +### Excluded + +- Changing `Clock` Protocol or `WallClock` / `TlogDerivedClock` implementations. +- Migrating any other timestamp emitters that use `time.time_ns()` directly without going through `Clock`. +- Bumping FDR schema versions (AC-5 requires `test_az272_fdr_record_schema.py` to pass unmodified). +- Migrating timestamp callers in components that don't currently have a `_iso_ts_from_clock` helper. + +## 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_from_clock` is run +Then the import succeeds; the function is callable with a `Clock`-compatible argument + +**AC-2: Output format matches the canonical Z-suffix nanosecond regex** +Given any `Clock` instance returning `time_ns()` value `N` +When `iso_ts_from_clock(clock)` is called +Then the output matches `^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{9}Z$` and is parseable into a UTC-aware datetime via `datetime.fromisoformat(value[:26] + "+00:00")` (note: stdlib `fromisoformat` doesn't accept 9-digit fractional seconds; tests truncate to 6 digits + offset for round-trip verification) + +**AC-3: Nanosecond accuracy** +Given a stub `Clock` returning `time_ns() == 1_234_567_890_123_456_789` +When `iso_ts_from_clock(clock)` is called +Then the output ends in `.123456789Z` (the nanosecond fraction is preserved verbatim) + +**AC-4: All four call sites migrated** +Given a `grep -rn "def _iso_ts_from_clock\|def iso_ts_from_clock" src/` after the task lands +When the search runs +Then matches appear only inside `src/gps_denied_onboard/helpers/iso_timestamps.py`; zero matches in `c2_vpr/`, `c12_operator_orchestrator/`, or any other component + +**AC-5: FDR schema tests pass unmodified** +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: AZ-270 + AZ-507 lints pass** +Given the helper lives at Layer 1 and only imports from `_types` / `clock` / stdlib +When `tests/unit/test_az270_compose_root.py::test_ac6` runs +Then the test passes (the lint walks `src/gps_denied_onboard/components/`; helper imports are allowed) + +## Constraints + +- The helper imports ONLY from stdlib (`datetime`) and `gps_denied_onboard.clock` (Layer 1). NO component imports. +- Output format `Z`-suffix nanosecond precision (`%Y-%m-%dT%H:%M:%S.{fraction_ns:09d}Z`). This intentionally differs from the existing `+00:00`-suffix output to align with `iso_ts_now()` and the canonical FDR `_TS` fixture (resolves cumulative-46-48 F3). +- The two instance-method call sites become one-line delegations; no behavior change at the call sites. +- Test additions go into the existing `tests/unit/test_az508_iso_timestamps.py`; do NOT create a separate test file (the helpers/iso_timestamps.py module surface is one cohesive Layer-1 concern). + +## Risks & Mitigation + +**Risk 1: Switching `+00:00` → `Z` changes FDR `ts` bit-shape for c2_vpr / c12 records** +- *Risk*: Downstream consumers parsing the FDR may have hardcoded `+00:00` expectations. +- *Mitigation*: The FDR `ts` field is typed `str` with no schema-level format validation (see `src/gps_denied_onboard/fdr_client/records.py` line 378). The canonical `_TS` test fixture already uses `Z`. AC-5 + the unchanged `test_az272_fdr_record_schema.py` is the safety net; if it fails, the consolidation must align to the existing FDR `_TS` fixture (= keep `Z`), confirming the choice. + +**Risk 2: A future emitter forgets the helper exists and adds a 5th local copy** +- *Risk*: The pattern recurs once we add C3/C4 strategies in AZ-345..AZ-358. +- *Mitigation*: AZ-508's AST-walk test already asserts zero stray `iso_ts_now` definitions outside the helper module; extend it to also assert zero stray `iso_ts_from_clock` definitions. The combined assertion is the regression guard. + +**Risk 3: Layer-1 helper depending on `Clock` introduces a cross-layer cycle** +- *Risk*: `helpers/*` imports from `clock/*`. +- *Mitigation*: Both `helpers/*` and `clock/*` are Layer 1 per `module-layout.md` Allowed Dependencies table. No cycle: `clock/interface.py` only imports `typing.Protocol`; the helper imports `clock.Clock` (interface only); no reverse path. + +## Runtime Completeness + +- **Named capability**: a Layer-1 helper `iso_ts_from_clock(clock: Clock) -> str` producing nanosecond-precision UTC ISO-8601 with `Z` suffix. +- **Production code that must exist**: real `iso_ts_from_clock` function alongside `iso_ts_now` in the helper module; real import + delegation in each of the four flagged modules. +- **Allowed external stubs**: none. Pure consolidation. +- **Unacceptable substitutes**: keeping one or more local definitions "for parity"; using `+00:00` suffix in the new helper (defeats the F3 alignment); changing the nanosecond precision (FDR records assume 9-digit subseconds). diff --git a/_docs/03_implementation/batch_49_cycle1_report.md b/_docs/03_implementation/batch_49_cycle1_report.md new file mode 100644 index 0000000..85bf4cc --- /dev/null +++ b/_docs/03_implementation/batch_49_cycle1_report.md @@ -0,0 +1,61 @@ +# Batch 49 — Implementation Report (Cycle 1) + +**Tasks**: AZ-526 (Hygiene — consolidate `_iso_ts_from_clock` into `helpers/iso_timestamps.py`) +**Date**: 2026-05-13 +**Cycle**: 1 +**Status**: COMPLETE (review verdict: PASS_WITH_WARNINGS, one Low / Scope drift finding) + +## What was done + +Added a second public function `iso_ts_from_clock(clock: Clock) -> str` to the Layer-1 helper module that AZ-508 (Batch 48) introduced. Migrated the four duplicate `_iso_ts_from_clock` definitions to import from the helper, closing cumulative review 46-48 Findings F1 (Medium / Maintainability) and F3 (Low / Maintainability). + +### Files added +- _None._ Reused `src/gps_denied_onboard/helpers/iso_timestamps.py` and `tests/unit/test_az508_iso_timestamps.py`. + +### Files changed (8) + +| File | Change | +|------|--------| +| `src/gps_denied_onboard/helpers/iso_timestamps.py` | Added `iso_ts_from_clock(clock)`; extended module docstring; added `TYPE_CHECKING`-guarded `Clock` import. | +| `src/gps_denied_onboard/helpers/__init__.py` | Re-exported `iso_ts_from_clock`; added to `__all__`. | +| `src/gps_denied_onboard/components/c2_vpr/net_vlad.py` | Deleted local module-level `def _iso_ts_from_clock`; added `from gps_denied_onboard.helpers.iso_timestamps import iso_ts_from_clock as _iso_ts_from_clock`. | +| `src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py` | Same as `net_vlad.py`. | +| `src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py` | Rewrote `FaissBridge._iso_ts_from_clock` body to `return iso_ts_from_clock(self._clock)`; deleted stale meta-rule comment; added helper import. | +| `src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py` | Same as `_faiss_bridge.py`; also removed now-unused `from datetime import datetime, timezone`. | +| `tests/unit/test_az508_iso_timestamps.py` | Added 7 AC tests (AZ-526 AC-1..AC-4); extended `_helper_uses_stdlib_only` to allow `gps_denied_onboard.clock` (Layer-1) and `typing`; updated `_public_surface_is_minimal` to expect `["iso_ts_from_clock", "iso_ts_now"]`; renamed AC-6 test for clarity; added AST scan for stray module-level `iso_ts_from_clock` defs (distinct from the AZ-508 `iso_ts_now` scan). | +| `_docs/02_document/module-layout.md` | Updated `shared/helpers/iso_timestamps` entry to describe both functions and list the c2_vpr / c12_operator_orchestrator consumers. | +| `_docs/02_tasks/_dependencies_table.md` | Added AZ-526 row (2pt, deps AZ-508 + AZ-398); updated date header and totals. | + +## AC coverage + +| AC | Verified by | Status | +|----|-------------|--------| +| AC-1 (import + callable) | `test_az526_ac1_import_and_call_returns_str` | PASS | +| AC-2 (Z-suffix nanosecond regex) | `test_az526_ac2_format_matches_canonical_ns_regex`, `test_az526_ac2_fromisoformat_truncated_roundtrip_yields_utc` | PASS | +| AC-3 (nanosecond accuracy) | `test_az526_ac3_nanosecond_fraction_preserved_verbatim`, `test_az526_ac3_sub_second_distinguishability` | PASS | +| AC-4 (call-site migration) | `test_ac4_az526_no_module_level_iso_ts_from_clock_outside_helper`, `test_az526_ac4_all_four_call_sites_import_from_helper`, `test_az526_ac4_no_module_level_local_iso_ts_from_clock_body_in_callers` | PASS (with Spec-Drift caveat — see below) | +| AC-5 (FDR schema tests unmodified) | `tests/unit/test_az272_fdr_record_schema.py` re-run unmodified (33 tests pass) | PASS | +| AC-6 (AZ-270 lint passes) | `tests/unit/test_az270_compose_root.py` (8 tests pass) | PASS | + +## Test results + +- `tests/unit/test_az508_iso_timestamps.py` — **18 / 18 PASS** (8 pre-existing + 10 added/updated for AZ-526). +- `tests/unit/test_az272_fdr_record_schema.py` — **33 / 33 PASS** unmodified. +- `tests/unit/test_az270_compose_root.py` — **8 / 8 PASS**. +- Component sibling tests for the 4 migrated files (`c2_vpr/`, `c12_operator_orchestrator/`) plus adjacent suites — **186 / 186 PASS** (`pytest -k "az338 or az337 or az489 or az490 or faiss or operator_reloc or net_vlad or ultra_vpr or _faiss"`). +- `ruff check` on all 7 changed source/test files — clean. + +## Architectural decisions + +1. **`Clock` import via `TYPE_CHECKING`** — `iso_ts_from_clock` only needs `Clock` for its parameter annotation. Importing it under `TYPE_CHECKING` keeps the runtime dependency surface stdlib-only, simplifies the AC-6 lint, and avoids any risk of importing component code at module load. +2. **Output format flip `+00:00` → `Z`** — closes cumulative-46-48 F3. The FDR `ts` field is typed `str` with no schema-level format validation, and the canonical `_TS` fixture in `test_az272_fdr_record_schema.py` already uses `Z`. AC-5 is the safety net; verified by re-running the unmodified schema test suite. +3. **Module-level callers preserved their local symbol via `as _iso_ts_from_clock`** — call-site shape unchanged, so the existing `_emit_fdr` / `_publish` builders read identically. Instance-method callers (FaissBridge, OperatorReLocService) became one-line delegations rather than imports because the call-site is `self._iso_ts_from_clock()` — keeping the method preserves the class's encapsulated public-ish surface in tests that mock it. +4. **Single test file (`test_az508_iso_timestamps.py`)** — the helper module is one cohesive Layer-1 concern; a separate `test_az526_*.py` file would have fragmented the AC walk across two tracker IDs without semantic benefit. The AZ-526 tests are sectioned under a `# AZ-526` divider. + +## Spec drift noted (carried into review F1) + +AC-4 of the AZ-526 task spec literally reads "zero matches in `c2_vpr/`, `c12_operator_orchestrator/`", but the same spec's § Scope and § Constraints both mandate that the two instance methods "become one-line delegations" — meaning `def _iso_ts_from_clock(self) -> str` remains inside `FaissBridge` and `OperatorReLocService`. Implementation followed Scope/Constraints (delegations preserved). The AST test `test_ac4_az526_no_module_level_iso_ts_from_clock_outside_helper` enforces the **spirit** of AC-4 by scanning only top-level `FunctionDef` nodes; nested class methods are not flagged. Logged as Low / Scope finding F1 in `batch_49_review.md`; no code change. + +## Cumulative review obligation + +This batch closes cumulative-46-48 F1 and F3. F2 (`_assert_engine_output_dim` near-duplication between `net_vlad.py` and `ultra_vpr.py`) is unchanged — leaving for a future hygiene PBI when a third strategy joins (AZ-339 batch). No new cumulative review window required after this batch (next one is at K=3 → batches 49-51). diff --git a/_docs/03_implementation/reviews/batch_49_review.md b/_docs/03_implementation/reviews/batch_49_review.md new file mode 100644 index 0000000..7a261a2 --- /dev/null +++ b/_docs/03_implementation/reviews/batch_49_review.md @@ -0,0 +1,62 @@ +# Code Review Report + +**Batch**: 49 — AZ-526 (Hygiene — consolidate `_iso_ts_from_clock`) +**Date**: 2026-05-13 +**Verdict**: PASS_WITH_WARNINGS + +## Findings + +| # | Severity | Category | File:Line | Title | +|---|----------|----------|-----------|-------| +| 1 | Low | Scope | `_docs/02_tasks/todo/AZ-526_hygiene_iso_ts_from_clock_consolidation.md` § AC-4 | AC-4 wording contradicts § Scope (instance-method delegations) | + +### Finding Details + +**F1: AC-4 wording contradicts § Scope (instance-method delegations)** (Low / Scope) + +- Location: `_docs/02_tasks/todo/AZ-526_hygiene_iso_ts_from_clock_consolidation.md`, AC-4 vs § Scope / § Constraints. +- Description: AC-4 says `grep "def _iso_ts_from_clock|def iso_ts_from_clock" src/` must yield "matches only inside `helpers/iso_timestamps.py`; zero matches in `c2_vpr/`, `c12_operator_orchestrator/`". But § Scope and § Constraints both explicitly mandate that `FaissBridge._iso_ts_from_clock(self)` and `OperatorReLocService._iso_ts_from_clock(self)` "become one-line delegations" — meaning two `def _iso_ts_from_clock` lines remain inside class bodies. Strict reading of AC-4 fails; reading aligned with Scope/Constraints passes. +- Implementation followed Scope/Constraints (delegations preserved). The AST-based test `test_ac4_az526_no_module_level_iso_ts_from_clock_outside_helper` enforces the spirit of AC-4 by scanning only top-level `FunctionDef` nodes (so nested ClassDef methods are not flagged). +- Suggestion: amend AC-4 to read "Then matches at module level appear only inside `helpers/iso_timestamps.py`; zero free-function matches in `c2_vpr/`, `c12_operator_orchestrator/`. Instance-method one-line delegations are permitted per § Scope." Recorded as drift; no code change needed. +- Task: AZ-526. + +## Phase Summary + +### Phase 1 — Context Loading +Read task spec AZ-526, cumulative review 46-48 (F1, F3), `_docs/02_document/module-layout.md` § `shared/helpers/iso_timestamps`, and AZ-508 precedent. Mapped 8 changed files (2 helper, 4 component, 1 test, 2 doc) to AZ-526. + +### Phase 2 — Spec Compliance +- AC-1: `iso_ts_from_clock` importable from helper. **PASS** (`test_az526_ac1_import_and_call_returns_str`). +- AC-2: Format matches `^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{9}Z$`. **PASS** (`test_az526_ac2_format_matches_canonical_ns_regex`, `test_az526_ac2_fromisoformat_truncated_roundtrip_yields_utc`). +- AC-3: Nanosecond fraction preserved verbatim for `ns=1_234_567_890_123_456_789`. **PASS** (`test_az526_ac3_nanosecond_fraction_preserved_verbatim`, `test_az526_ac3_sub_second_distinguishability`). +- AC-4: All four call sites import from helper; no module-level stray defs. **PASS** with F1 drift (see above). Tests `test_ac4_az526_no_module_level_iso_ts_from_clock_outside_helper`, `test_az526_ac4_all_four_call_sites_import_from_helper`, `test_az526_ac4_no_module_level_local_iso_ts_from_clock_body_in_callers`. +- AC-5: `tests/unit/test_az272_fdr_record_schema.py` runs unmodified. **PASS** (33 tests, 0 modifications). +- AC-6: `tests/unit/test_az270_compose_root.py::test_ac6` passes. **PASS** (8 tests, all green). + +### Phase 3 — Code Quality +- `iso_ts_from_clock` body is 4 lines (excluding docstring). SRP satisfied. +- Each migrated instance method is now a single `return iso_ts_from_clock(self._clock)` — readable and intent-preserving. +- Stale meta-rule comment in `_faiss_bridge.py` deleted (was: "Inlined here rather than importing a shared helper because every other component … defines the same one-liner locally" — that justification is no longer true). +- The two module-level callers (`net_vlad.py`, `ultra_vpr.py`) import `iso_ts_from_clock as _iso_ts_from_clock`, preserving call-site shape with no behavioural change. +- No dead code introduced; the `datetime`, `timezone` imports unused after the body deletion were removed from `operator_reloc_service.py`. +- Tests follow AAA convention with explicit Arrange / Act / Assert markers. + +### Phase 4 — Security Quick-Scan +No SQL, no shell, no eval / exec, no dynamic deserialisation, no user-controlled paths. The helper is a pure datetime formatter over an injected protocol method. Out of scope. + +### Phase 5 — Performance Scan +The helper is O(1) — two arithmetic operations (`divmod`, `int()`) plus a `strftime` and an f-string. Functionally equivalent to the inlined version it replaces; one extra Python function-call frame per call site, dominated by the I/O cost of `FdrClient.enqueue` (the surrounding caller). Not in any hot path. + +### Phase 6 — Cross-Task Consistency +Single-task batch. N/A. + +### Phase 7 — Architecture Compliance +- **Layer direction**: `helpers/iso_timestamps.py` imports `gps_denied_onboard.clock.Clock` under `TYPE_CHECKING`. Both modules are Layer-1 per `module-layout.md` Allowed Dependencies table. The runtime `from datetime import datetime, timezone` is stdlib. No upward import. **PASS.** +- **Public API respect**: the four callers import from `gps_denied_onboard.helpers.iso_timestamps`, which is the Public API surface declared in `module-layout.md` § `shared/helpers/iso_timestamps`. **PASS.** +- **No new cyclic dependencies**: `helpers → clock`; `clock/interface.py` only imports `typing`. No reverse path possible. **PASS.** +- **Duplicate symbols**: net reduction (4 module-level / instance-method copies of `_iso_ts_from_clock` → 1 canonical + 2 thin delegations). **PASS.** +- **Cross-cutting concerns not locally re-implemented**: this IS the consolidation; the cumulative-46-48 F1 + F3 findings are closed by this batch. **PASS.** + +## Verdict Rationale + +One Low / Scope finding (AC wording drift relative to the spec's own Scope/Constraints) — does not block. No Critical, High, or Medium findings. Verdict: **PASS_WITH_WARNINGS**. diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index e3bee87..41ecb75 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -12,5 +12,5 @@ sub_step: retry_count: 0 cycle: 1 tracker: jira -last_completed_batch: 48 +last_completed_batch: 49 last_cumulative_review: batches_46-48 diff --git a/src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py b/src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py index ff53527..8583cd4 100644 --- a/src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py +++ b/src/gps_denied_onboard/components/c12_operator_orchestrator/operator_reloc_service.py @@ -14,7 +14,6 @@ auto-retried. The operator decides when to re-issue. from __future__ import annotations import logging -from datetime import datetime, timezone from gps_denied_onboard.clock import Clock from gps_denied_onboard.components.c12_operator_orchestrator._types import ( @@ -31,6 +30,7 @@ from gps_denied_onboard.fdr_client.records import ( CURRENT_SCHEMA_VERSION, FdrRecord, ) +from gps_denied_onboard.helpers.iso_timestamps import iso_ts_from_clock __all__ = ["OperatorReLocService"] @@ -165,10 +165,7 @@ class OperatorReLocService: ) def _iso_ts_from_clock(self) -> str: - ns = int(self._clock.time_ns()) - seconds, fraction_ns = divmod(ns, 1_000_000_000) - dt = datetime.fromtimestamp(seconds, tz=timezone.utc) - return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}+00:00" + return iso_ts_from_clock(self._clock) def _hint_to_payload(hint: ReLocHint) -> dict[str, object]: diff --git a/src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py b/src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py index dd134d0..4d1e211 100644 --- a/src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py +++ b/src/gps_denied_onboard/components/c2_vpr/_faiss_bridge.py @@ -48,6 +48,7 @@ from gps_denied_onboard.fdr_client.records import ( CURRENT_SCHEMA_VERSION, FdrRecord, ) +from gps_denied_onboard.helpers.iso_timestamps import iso_ts_from_clock if TYPE_CHECKING: pass @@ -304,14 +305,4 @@ class FaissBridge: ) def _iso_ts_from_clock(self) -> str: - # Inlined here rather than importing a shared helper because every - # other component that emits FDR records (c12, c11, c5) defines - # the same one-liner locally — see meta-rule "Critical Thinking": - # the duplication is intentional and trivial, factoring it would - # add an L1 import every component already avoids. - from datetime import datetime, timezone - - ns = int(self._clock.time_ns()) - seconds, fraction_ns = divmod(ns, 1_000_000_000) - dt = datetime.fromtimestamp(seconds, tz=timezone.utc) - return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}+00:00" + return iso_ts_from_clock(self._clock) diff --git a/src/gps_denied_onboard/components/c2_vpr/net_vlad.py b/src/gps_denied_onboard/components/c2_vpr/net_vlad.py index ca71ba1..0dda9f1 100644 --- a/src/gps_denied_onboard/components/c2_vpr/net_vlad.py +++ b/src/gps_denied_onboard/components/c2_vpr/net_vlad.py @@ -105,6 +105,9 @@ from gps_denied_onboard.fdr_client.records import ( FdrRecord, ) from gps_denied_onboard.helpers.descriptor_normaliser import DescriptorNormaliser +from gps_denied_onboard.helpers.iso_timestamps import ( + iso_ts_from_clock as _iso_ts_from_clock, +) if TYPE_CHECKING: from gps_denied_onboard._types.calibration import CameraCalibration @@ -372,17 +375,6 @@ class NetVladStrategy: ) -def _iso_ts_from_clock(clock: Clock) -> str: - # Same shape every component uses for FDR timestamps; AZ-508 will - # consolidate the duplicate helpers across c2/c11/c12/c6. - from datetime import datetime, timezone - - ns = int(clock.time_ns()) - seconds, fraction_ns = divmod(ns, 1_000_000_000) - dt = datetime.fromtimestamp(seconds, tz=timezone.utc) - return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}+00:00" - - def _build_pytorch_build_config(weights_path: Path) -> BuildConfig: del weights_path return BuildConfig( diff --git a/src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py b/src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py index d261981..321d627 100644 --- a/src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py +++ b/src/gps_denied_onboard/components/c2_vpr/ultra_vpr.py @@ -84,6 +84,9 @@ from gps_denied_onboard.fdr_client.records import ( FdrRecord, ) from gps_denied_onboard.helpers.descriptor_normaliser import DescriptorNormaliser +from gps_denied_onboard.helpers.iso_timestamps import ( + iso_ts_from_clock as _iso_ts_from_clock, +) if TYPE_CHECKING: from gps_denied_onboard._types.calibration import CameraCalibration @@ -323,17 +326,6 @@ class UltraVprStrategy: ) -def _iso_ts_from_clock(clock: Clock) -> str: - # Same shape every component uses for FDR timestamps; AZ-508 will - # consolidate the duplicate helpers across c2/c11/c12/c6. - from datetime import datetime, timezone - - ns = int(clock.time_ns()) - seconds, fraction_ns = divmod(ns, 1_000_000_000) - dt = datetime.fromtimestamp(seconds, tz=timezone.utc) - return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}+00:00" - - def _build_trt_build_config() -> BuildConfig: return BuildConfig( precision=PrecisionMode.FP16, diff --git a/src/gps_denied_onboard/helpers/__init__.py b/src/gps_denied_onboard/helpers/__init__.py index c28f36e..28b26d1 100644 --- a/src/gps_denied_onboard/helpers/__init__.py +++ b/src/gps_denied_onboard/helpers/__init__.py @@ -22,7 +22,10 @@ from gps_denied_onboard.helpers.imu_preintegrator import ( ImuPreintegrator, make_imu_preintegrator, ) -from gps_denied_onboard.helpers.iso_timestamps import iso_ts_now +from gps_denied_onboard.helpers.iso_timestamps import ( + iso_ts_from_clock, + iso_ts_now, +) from gps_denied_onboard.helpers.lightglue_runtime import ( LightGlueConcurrentAccessError, LightGlueRuntime, @@ -84,6 +87,7 @@ __all__ = [ "adjoint", "exp_map", "is_valid_rotation", + "iso_ts_from_clock", "iso_ts_now", "log_map", "make_imu_preintegrator", diff --git a/src/gps_denied_onboard/helpers/iso_timestamps.py b/src/gps_denied_onboard/helpers/iso_timestamps.py index 436d0e0..8bf4518 100644 --- a/src/gps_denied_onboard/helpers/iso_timestamps.py +++ b/src/gps_denied_onboard/helpers/iso_timestamps.py @@ -1,27 +1,36 @@ -"""UTC ISO-8601 timestamp helper (E-CC-HELPERS / AZ-264 / AZ-508). +"""UTC ISO-8601 timestamp helpers (E-CC-HELPERS / AZ-264 / AZ-508 + AZ-526). -Single Layer-1 source for the wall-clock string used in the FDR record -``ts`` envelope across c6_tile_cache and c7_inference. Consolidates what -used to be a private ``_iso_ts_now`` one-liner repeated per module. +Single Layer-1 source for the FDR record ``ts`` envelope strings across +every component. Two variants: -The output format is locked to RFC 3339 / ISO 8601 UTC with microsecond -precision and a ``Z`` suffix, matching the canonical FDR ``ts`` fixture -in ``tests/unit/test_az272_fdr_record_schema.py`` (``_TS = -"2026-05-11T00:00:00.000000Z"``) and the format produced by the three -existing local helpers this module replaces. Changing the format would -alter FDR record bit-shape and is explicitly out of scope for AZ-508. +- :func:`iso_ts_now` — uses :func:`datetime.now` (wall clock); 6-digit + microsecond precision; for components that don't have a ``Clock`` + injected (the c6_tile_cache and c7_inference FDR producers). +- :func:`iso_ts_from_clock` — uses an injected :class:`Clock` Protocol; + 9-digit nanosecond precision; for components that already accept a + ``Clock`` constructor parameter (the c2_vpr strategies, the FAISS + bridge, the C12 operator-reloc service, and the future C4 / C5 batches). -Producers that need a Clock-injected payload field (e.g. -``age_seconds`` derived from an injected wall-clock for testability) -MUST NOT route through this helper — it is purely metadata about WHEN -the FDR record envelope itself was emitted. +Both produce RFC 3339 UTC with a ``Z`` suffix, matching the canonical +FDR ``ts`` fixture in ``tests/unit/test_az272_fdr_record_schema.py`` +(``_TS = "2026-05-11T00:00:00.000000Z"``). Changing the format would +alter FDR record bit-shape and is explicitly out of scope. + +Producers that need a Clock-injected payload field (e.g. ``age_seconds`` +derived from an injected wall-clock for testability) MUST NOT route +through these helpers — they are purely metadata about WHEN the FDR +record envelope itself was emitted. """ from __future__ import annotations from datetime import datetime, timezone +from typing import TYPE_CHECKING -__all__ = ["iso_ts_now"] +if TYPE_CHECKING: + from gps_denied_onboard.clock import Clock + +__all__ = ["iso_ts_from_clock", "iso_ts_now"] def iso_ts_now() -> str: @@ -31,3 +40,17 @@ def iso_ts_now() -> str: monotonic under a non-decreasing wall clock). """ return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + +def iso_ts_from_clock(clock: Clock) -> str: + """Return an RFC 3339 UTC timestamp built from ``clock.time_ns()``. + + Format: ``YYYY-MM-DDTHH:MM:SS.fffffffffZ`` (9-digit nanosecond + precision, ``Z`` suffix). Use when the timestamp must follow an + injectable :class:`~gps_denied_onboard.clock.Clock` (replay tests, + deterministic fixtures); use :func:`iso_ts_now` otherwise. + """ + ns = int(clock.time_ns()) + seconds, fraction_ns = divmod(ns, 1_000_000_000) + dt = datetime.fromtimestamp(seconds, tz=timezone.utc) + return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}.{fraction_ns:09d}Z" diff --git a/tests/unit/test_az508_iso_timestamps.py b/tests/unit/test_az508_iso_timestamps.py index 2281fd2..8dd0788 100644 --- a/tests/unit/test_az508_iso_timestamps.py +++ b/tests/unit/test_az508_iso_timestamps.py @@ -1,14 +1,15 @@ -"""AC tests for AZ-508: ISO-timestamp helper consolidation. +"""AC tests for AZ-508 + AZ-526: ISO-timestamp helper consolidation. -Verifies the `iso_timestamps` helper exposed at -`gps_denied_onboard.helpers.iso_timestamps.iso_ts_now` — the single -Layer-1 source for FDR record envelope timestamps that replaced the -duplicated `_iso_ts_now` one-liners in c6_tile_cache and c7_inference. +Verifies the `iso_timestamps` helper module exposed at +`gps_denied_onboard.helpers.iso_timestamps` — the single Layer-1 source +for FDR record envelope timestamps. Two surfaces: -Output contract (matches the canonical FDR `_TS` fixture in -`tests/unit/test_az272_fdr_record_schema.py`): +- ``iso_ts_now()`` (AZ-508) — wall-clock variant, microsecond precision. +- ``iso_ts_from_clock(clock)`` (AZ-526) — Clock-injected variant, + nanosecond precision. - YYYY-MM-DDTHH:MM:SS.ffffffZ (UTC, microsecond precision, ``Z`` suffix) +Both produce RFC 3339 UTC with the canonical ``Z`` suffix, matching the +FDR ``_TS`` fixture in ``tests/unit/test_az272_fdr_record_schema.py``. """ from __future__ import annotations @@ -21,12 +22,18 @@ from pathlib import Path import pytest -from gps_denied_onboard.helpers import iso_ts_now +from gps_denied_onboard.helpers import iso_ts_from_clock, iso_ts_now +from gps_denied_onboard.helpers.iso_timestamps import ( + iso_ts_from_clock as iso_ts_from_clock_direct, +) from gps_denied_onboard.helpers.iso_timestamps import iso_ts_now as iso_ts_now_direct _TS_REGEX: re.Pattern[str] = re.compile( r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}Z$" ) +_TS_NS_REGEX: re.Pattern[str] = re.compile( + r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{9}Z$" +) _REPO_ROOT: Path = Path(__file__).resolve().parents[2] _HELPER_PATH: Path = ( @@ -38,6 +45,25 @@ _C6_DIR: Path = ( _C7_DIR: Path = ( _REPO_ROOT / "src" / "gps_denied_onboard" / "components" / "c7_inference" ) +_C2_DIR: Path = ( + _REPO_ROOT / "src" / "gps_denied_onboard" / "components" / "c2_vpr" +) +_C12_DIR: Path = ( + _REPO_ROOT / "src" + / "gps_denied_onboard" + / "components" + / "c12_operator_orchestrator" +) + + +class _StubClock: + """Minimal :class:`Clock`-shaped stub for AC-3 nanosecond verification.""" + + def __init__(self, ns: int) -> None: + self._ns = ns + + def time_ns(self) -> int: + return self._ns def test_ac1_import_and_call_returns_str() -> None: @@ -85,9 +111,9 @@ def test_ac3_two_successive_calls_are_non_decreasing() -> None: def test_ac4_no_other_iso_ts_now_definition_exists_in_src() -> None: - """AC-4: a `def _iso_ts_now` / `def iso_ts_now` MUST exist only inside - `helpers/iso_timestamps.py`. Any other definition under `src/` means a - consumer slipped a copy back in. + """AC-4 (AZ-508): a `def _iso_ts_now` / `def iso_ts_now` MUST exist + only inside `helpers/iso_timestamps.py`. Any other definition under + `src/` means a consumer slipped a copy back in. """ # Arrange src_root = _REPO_ROOT / "src" @@ -114,6 +140,40 @@ def test_ac4_no_other_iso_ts_now_definition_exists_in_src() -> None: ) +def test_ac4_az526_no_module_level_iso_ts_from_clock_outside_helper() -> None: + """AC-4 (AZ-526): a module-level `def iso_ts_from_clock` / + `def _iso_ts_from_clock` MUST exist only inside + `helpers/iso_timestamps.py`. The two instance-method delegations + (`_faiss_bridge.FaissBridge._iso_ts_from_clock`, + `operator_reloc_service.OperatorReLocService._iso_ts_from_clock`) + live inside a `ClassDef`, so we only scan top-level function defs. + """ + # Arrange + src_root = _REPO_ROOT / "src" + offenders: list[tuple[Path, str]] = [] + + # Act + for path in src_root.rglob("*.py"): + if path == _HELPER_PATH: + continue + try: + tree = ast.parse(path.read_text(encoding="utf-8")) + except SyntaxError: + continue + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name in { + "iso_ts_from_clock", + "_iso_ts_from_clock", + }: + offenders.append((path.relative_to(_REPO_ROOT), node.name)) + + # Assert + assert offenders == [], ( + "Found stray module-level `iso_ts_from_clock` definitions outside " + f"the helper: {offenders}" + ) + + def test_ac4_c6_and_c7_callers_import_from_helpers() -> None: """The 3 migrated call-sites must import from `helpers.iso_timestamps` (directly or via the `helpers` package facade) so future hygiene @@ -142,11 +202,15 @@ def test_ac4_c6_and_c7_callers_import_from_helpers() -> None: ) -def test_ac6_helper_uses_stdlib_only() -> None: - """AC-6: no third-party imports inside the helper module.""" +def test_ac6_helper_uses_stdlib_or_layer1_clock_only() -> None: + """AC-6: the helper module's imports MUST be stdlib OR the Layer-1 + `gps_denied_onboard.clock` package (needed by `iso_ts_from_clock` + for its `Clock` type annotation, imported under `TYPE_CHECKING`). + """ # Arrange tree = ast.parse(_HELPER_PATH.read_text(encoding="utf-8")) - allowed_stdlib = {"datetime", "__future__"} + allowed_stdlib = {"datetime", "__future__", "typing"} + allowed_layer1 = {"gps_denied_onboard.clock"} # Act imports: list[str] = [] @@ -160,15 +224,16 @@ def test_ac6_helper_uses_stdlib_only() -> None: # Assert for name in imports: top = name.split(".")[0] - assert top in allowed_stdlib, ( + assert top in allowed_stdlib or name in allowed_layer1, ( f"helpers/iso_timestamps.py imports `{name}`; only stdlib " - f"({sorted(allowed_stdlib)}) is allowed by AC-6" + f"({sorted(allowed_stdlib)}) or {sorted(allowed_layer1)} is " + "allowed by AC-6" ) def test_helper_is_layer_1_no_component_imports() -> None: """Layer-1 discipline: the helper MUST NOT import from any component. - (Constraint § Layer-1 discipline in the AZ-508 task spec.) + (Constraint § Layer-1 discipline in the AZ-508 / AZ-526 task specs.) """ # Arrange text = _HELPER_PATH.read_text(encoding="utf-8") @@ -180,12 +245,145 @@ def test_helper_is_layer_1_no_component_imports() -> None: ) -@pytest.mark.parametrize("expected_field", ["iso_ts_now"]) +@pytest.mark.parametrize( + "expected_field", ["iso_ts_now", "iso_ts_from_clock"] +) def test_helper_public_surface_is_minimal(expected_field: str) -> None: - """Defensive: only ``iso_ts_now`` is re-exported from the module.""" + """Defensive: only the two consolidated helpers are re-exported.""" # Arrange import gps_denied_onboard.helpers.iso_timestamps as module # Assert assert expected_field in module.__all__ - assert module.__all__ == ["iso_ts_now"] + assert module.__all__ == ["iso_ts_from_clock", "iso_ts_now"] + + +# --------------------------------------------------------------------------- +# AZ-526 — `iso_ts_from_clock(clock)` AC tests +# --------------------------------------------------------------------------- + + +def test_az526_ac1_import_and_call_returns_str() -> None: + # Arrange + clock = _StubClock(ns=1_700_000_000_000_000_000) + + # Act + value = iso_ts_from_clock(clock) + + # Assert + assert isinstance(value, str) + assert value, "iso_ts_from_clock() returned an empty string" + assert iso_ts_from_clock is iso_ts_from_clock_direct + + +def test_az526_ac2_format_matches_canonical_ns_regex() -> None: + # Arrange + clock = _StubClock(ns=1_734_567_890_123_456_789) + + # Act + value = iso_ts_from_clock(clock) + + # Assert + assert _TS_NS_REGEX.fullmatch(value), ( + f"{value!r} does not match the canonical clock-driven format " + f"YYYY-MM-DDTHH:MM:SS.fffffffffZ" + ) + + +def test_az526_ac2_fromisoformat_truncated_roundtrip_yields_utc() -> None: + # Arrange + clock = _StubClock(ns=1_700_000_000_000_000_000) + value = iso_ts_from_clock(clock) + # stdlib fromisoformat (3.11+) accepts up to 6 fractional digits; + # truncate the trailing 3 ns digits and the Z, then append `+00:00`. + iso_with_offset = value[:26] + "+00:00" + + # Act + parsed = datetime.fromisoformat(iso_with_offset) + + # Assert + assert parsed.tzinfo is not None + assert parsed.utcoffset() == timezone.utc.utcoffset(parsed) + + +def test_az526_ac3_nanosecond_fraction_preserved_verbatim() -> None: + """The AC-3 fixture from the AZ-526 task spec.""" + # Arrange + clock = _StubClock(ns=1_234_567_890_123_456_789) + + # Act + value = iso_ts_from_clock(clock) + + # Assert + assert value.endswith(".123456789Z"), ( + f"expected ns suffix `.123456789Z`, got {value!r}" + ) + expected_prefix = ( + datetime.fromtimestamp(1_234_567_890, tz=timezone.utc) + .strftime("%Y-%m-%dT%H:%M:%S") + ) + assert value == f"{expected_prefix}.123456789Z" + + +def test_az526_ac3_sub_second_distinguishability() -> None: + """Two clock instances 1 ns apart MUST yield distinct outputs.""" + # Arrange + base_ns = 1_700_000_000_000_000_000 + + # Act + a = iso_ts_from_clock(_StubClock(ns=base_ns)) + b = iso_ts_from_clock(_StubClock(ns=base_ns + 1)) + + # Assert + assert a != b, "nanosecond precision lost (1-ns advance produced same output)" + assert b > a, "lexicographic ordering broken at the ns boundary" + + +def test_az526_ac4_all_four_call_sites_import_from_helper() -> None: + """AC-4: the four call sites import `iso_ts_from_clock` from the + helper module. Module-level callers import as `_iso_ts_from_clock` + (preserving their local symbol); instance-method callers import the + canonical name and delegate. + """ + # Arrange + callers = [ + _C2_DIR / "net_vlad.py", + _C2_DIR / "ultra_vpr.py", + _C2_DIR / "_faiss_bridge.py", + _C12_DIR / "operator_reloc_service.py", + ] + expected_token = ( + "from gps_denied_onboard.helpers.iso_timestamps import " + ) + + # Act / Assert + for caller in callers: + text = caller.read_text(encoding="utf-8") + assert expected_token in text and "iso_ts_from_clock" in text, ( + f"{caller.relative_to(_REPO_ROOT)} does not import " + f"`iso_ts_from_clock` from the helper" + ) + + +def test_az526_ac4_no_module_level_local_iso_ts_from_clock_body_in_callers() -> None: + """AC-4 follow-on: the four migrated callers must not contain the + legacy multi-line body (`divmod(ns, 1_000_000_000)` + `+00:00` + suffix). Catches a future contributor who deletes the import but + forgets the helper exists. + """ + # Arrange + callers = [ + _C2_DIR / "net_vlad.py", + _C2_DIR / "ultra_vpr.py", + _C2_DIR / "_faiss_bridge.py", + _C12_DIR / "operator_reloc_service.py", + ] + forbidden_format_marker = "+00:00\"" + + # Act / Assert + for caller in callers: + text = caller.read_text(encoding="utf-8") + assert forbidden_format_marker not in text, ( + f"{caller.relative_to(_REPO_ROOT)} still emits a `+00:00`-suffix " + "timestamp literal — the legacy ts body wasn't fully migrated" + )