Closes cumulative review 46-48 F1 (Medium) + F3 (Low). Adds iso_ts_from_clock(clock) alongside iso_ts_now() in the Layer-1 helper; migrates four duplicate definitions in c2_vpr (net_vlad, ultra_vpr, _faiss_bridge) and c12_operator_orchestrator (operator_reloc_service). Output format flipped +00:00 -> Z to align with iso_ts_now() and the canonical FDR _TS fixture (FDR schema test passes unmodified). 18 helper AC tests + 186 sibling tests pass; ruff clean. Co-authored-by: Cursor <cursoragent@cursor.com>
6.3 KiB
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.pyandtests/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 checkon all 7 changed source/test files — clean.
Architectural decisions
Clockimport viaTYPE_CHECKING—iso_ts_from_clockonly needsClockfor its parameter annotation. Importing it underTYPE_CHECKINGkeeps the runtime dependency surface stdlib-only, simplifies the AC-6 lint, and avoids any risk of importing component code at module load.- Output format flip
+00:00→Z— closes cumulative-46-48 F3. The FDRtsfield is typedstrwith no schema-level format validation, and the canonical_TSfixture intest_az272_fdr_record_schema.pyalready usesZ. AC-5 is the safety net; verified by re-running the unmodified schema test suite. - Module-level callers preserved their local symbol via
as _iso_ts_from_clock— call-site shape unchanged, so the existing_emit_fdr/_publishbuilders read identically. Instance-method callers (FaissBridge, OperatorReLocService) became one-line delegations rather than imports because the call-site isself._iso_ts_from_clock()— keeping the method preserves the class's encapsulated public-ish surface in tests that mock it. - Single test file (
test_az508_iso_timestamps.py) — the helper module is one cohesive Layer-1 concern; a separatetest_az526_*.pyfile would have fragmented the AC walk across two tracker IDs without semantic benefit. The AZ-526 tests are sectioned under a# AZ-526divider.
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).