mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-22 13:21:13 +00:00
[AZ-777] Phase 1: wire e2e-runner to real satellite-provider + C11 contract adapt
Adapt C11 HttpTileDownloader to the AZ-505 v1.0.0 tile-inventory
contract (POST /api/satellite/tiles/inventory + GET /tiles/{z}/{x}/{y})
and wire the Jetson e2e harness against the real parent-suite
satellite-provider service. Closes Phase 1 of 5 for AZ-777; STOP
gate before Phase 2 (Derkachi catalog seed).
C11 changes:
- _LIST_PATH / _GET_PATH replaced with _INVENTORY_PATH + _TILES_PATH.
- _do_enumerate enumerates bbox tile coords client-side and posts
chunked inventory requests (5000-entry cap per the contract).
- _download_one_tile parses tile_id_str into (z,x,y) and fetches
the slippy-map URL.
- Common GET / POST retry+auth ladder consolidated into _send_request.
- New module helpers: _enumerate_bbox_tile_coords,
_tile_center_latlon, _tile_size_meters_at, _format_tile_id_str,
_parse_tile_id_str, _chunk_iter.
- _DEFAULT_ESTIMATED_TILE_BYTES (50 KiB) replaces the inventory-side
estimatedBytes field the v1.0.0 contract dropped.
Tests:
- 14/14 unit tests in tests/unit/c11_tile_manager/test_tile_downloader.py
rewritten for the new POST inventory + slippy-map GET handler.
_StubTileWriter rekeyed by call-index (the downloader now derives
lat/lon from the slippy-map coord, so fixtures can't fabricate
arbitrary positions).
- New Tier-2 smoke at tests/e2e/satellite_provider/test_smoke.py:
validates inventory POST schema + drives HttpTileDownloader against
the real service. Gated by RUN_REPLAY_E2E=1 + tier2.
Compose / env:
- e2e-runner SATELLITE_PROVIDER_URL switched from mock-sat:5100 to
https://satellite-provider:8080; TLS_INSECURE + Bearer JWT env +
depends_on satellite-provider added.
- .env.test.example documents SATELLITE_PROVIDER_API_KEY + dev TLS
bypass security note.
- scripts/mint_dev_jwt.py mints HS256 dev JWTs from env / .env.test.
- pyjwt added to dev extras.
Tracker hygiene:
- AZ-777 row in _dependencies_table.md bumped 5pt -> 8pt to match
the 2026-05-21 override decision log.
Code review: PASS_WITH_WARNINGS (3 medium/low findings, all deferred
to later AZ-777 phases) -- see batch_104_review.md. Batch report at
batch_104_cycle3_report.md.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
@@ -2,7 +2,7 @@
|
||||
|
||||
**Date**: 2026-05-21 (cycle-3 Step 9 New Task — added AZ-776 (3pt open-loop ESKF composition profile via `c4_pose.enabled` flag, no deps, epic AZ-602) + AZ-777 (5pt Derkachi C6 reference tile cache + FAISS descriptor index from OSM/CARTO basemap, depends on AZ-776, epic AZ-602). Both unblock the 7 currently-`@xfail`-masked Derkachi e2e tests on Jetson; AZ-776 unblocks 5 (AC-1, AC-2, AC-5, AC-6 realtime, AC-6 asap), AZ-777 unblocks the remaining 2 (AC-3 + AZ-699 real-flight verdict). Earlier 2026-05-19 (refreshed late-morning after 11:27 Jetson Tier-2 e2e run for AZ-618 — surfaced a NEW gap: replay-mode `Config` lacks `c6_tile_cache` block, so `build_pre_constructed → _build_c6_descriptor_index → _c6_config` raises `KeyError` for AC-1/2/5/6. Follow-up filed as AZ-687 (2pt) under E-AZ-602 with guard at the bootstrap layer (NOT silent fallback in `_c6_config`). Earlier same-day mid-day after AZ-618 split: per the spec author's own Sizing-note recommendation + user-rule cap on PBI complexity, AZ-618 was split into 6 subtasks AZ-619..AZ-624 in Jira (subtasks of AZ-618; epic AZ-602 stays grandparent). AZ-618 retained at 0pt as the umbrella tracker; aggregate actionable work is 16pt across the subtasks (vs. AZ-618's original 5pt filing — author's "likely a true 8" caveat was understated due to c5_isam2_graph_handle ordering + GPU builder unknowns). Earlier same-day refresh at start of Step-7 rewind for AZ-618 — Step-11 Jetson tier-2 e2e gate identified missing internal product implementation: `runtime_root.main()` does not build the airborne `pre_constructed` infrastructure dict before `compose_root()`; AZ-618 = 5pt cross-cutting follow-up to AZ-591, lives under E-AZ-602; all 12 dep tasks are in `done/`. Earlier 2026-05-16 (cycle-1 completeness-gate post-mortem): AZ-589 + AZ-590 closed Won't Fix — were wrong abstraction (OKVIS v1 `ThreadedKFVio` API doesn't exist in OKVIS2 upstream; VINS-Mono `cpp/vins_mono/upstream/` submodule never existed; the actual production gap is the empty central `_STRATEGY_REGISTRY` affecting EVERY component with a strategy-selecting config field, not just c1_vio); replaced by AZ-591 (cross-cutting compose_root per-binary bootstrap, todo/, 5pt) + AZ-592 (AZ-332 Tier-2 validation bundle, backlog/, 5pt placeholder) + AZ-593 (AZ-333 Tier-2 validation bundle, backlog/, 5pt placeholder); AZ-332 + AZ-333 re-classified in gate report from FAIL to BLOCKED-on-Tier-2 per the original tasks' Implementation Notes deferral handles; earlier same-day after end of cycle-1 gate: AZ-589 + AZ-590 created (now closed); earlier same-day after end of Batch 64: AZ-558 implementation closed — `MavlinkTransport` seam now routes every C8 outbound MAVLink byte; AZ-401 AC-9 + AZ-404 AC-4b unskipped together; encoder helpers extracted to `_outbound_mavlink_payloads.py`; live-mode `compose_root` injection deferred to whichever future batch registers AP/iNav strategies in an airborne binary; earlier 2026-05-14: refreshed at start of Batch 63: AZ-559 closed Won't Fix — gap was illusory; `TileSource.ONBOARD_INGEST` + `TileMetadata.quality_metadata` + `write_tile`'s `FreshnessRejectionError` already cover the AZ-389 mid-flight ingest semantic without any new API; AZ-389 dep restored to AZ-303; earlier same-day after Batch 61: AZ-558 follow-up added — routes C8 outbound encoder bytes through `MavlinkTransport` seam; closes AZ-401 AC-9 deferred during batch 61 due to encoder-side routing not being in the AZ-401 task envelope; earlier same-day after cumulative review batches 52-54: AZ-528 hygiene PBI added for c1_vio strategy facade orchestration-spine 3-way duplication (Medium); earlier same-day after Batch 53: AZ-333 VINS-Mono landed — first c1_vio strategy after the AZ-332 OKVIS2 production-default; consolidation hygiene for the strategy-facade duplication deferred to a post-AZ-334 PBI; earlier same-day after Batch 51: AZ-527 hygiene PBI added from cumulative review batches 49-51 F1; 2026-05-13: AZ-526 hygiene PBI added from cumulative review batches 46-48 F1+F3; 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**: 165 (124 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; AZ-527 = 2pt c2 engine-dim helper hygiene; AZ-528 = 3pt c1_vio facade-spine hygiene; AZ-558 = 3pt MavlinkTransport routing follow-up; AZ-559 closed Won't Fix; AZ-589 + AZ-590 closed Won't Fix (kept in table as 0pt audit-trail rows); AZ-591 = 5pt cross-cutting compose_root bootstrap (todo/); AZ-592 = 5pt OKVIS2 Tier-2 placeholder (backlog/); AZ-593 = 5pt VINS-Mono Tier-2 placeholder (backlog/); AZ-618 = 0pt umbrella (split into AZ-619..AZ-624 on 2026-05-19); AZ-619..AZ-624 = 6 subtasks of AZ-618 covering Phase A..F of the airborne `pre_constructed` assembly, summing to 16pt actionable work; AZ-687 = 2pt replay-mode guard follow-up surfaced by AZ-618 Tier-2 run on 2026-05-19
|
||||
**Total Complexity Points**: 543 (410 product + 133 blackbox-test) — +3pt AZ-776 + 5pt AZ-777 added 2026-05-21 — AZ-523 = 3pt, AZ-524 = 2pt, AZ-526 = 2pt, AZ-527 = 2pt, AZ-528 = 3pt, AZ-558 = 3pt, AZ-589 + AZ-590 retained at 5pt each but closed Won't Fix (treated as 0 effective pts going forward), AZ-591 = 5pt, AZ-592 = 5pt placeholder, AZ-593 = 5pt placeholder, AZ-618 = 0pt umbrella post-split, AZ-619 = 2pt, AZ-620 = 3pt, AZ-621 = 3pt, AZ-622 = 3pt, AZ-623 = 3pt, AZ-624 = 2pt, AZ-687 = 2pt
|
||||
**Total Complexity Points**: 546 (413 product + 133 blackbox-test) — +3pt AZ-776 + 8pt AZ-777 (5→8 override 2026-05-21 cycle-3 batch 104; see `_docs/_process_leftovers/2026-05-21_az777_complexity_override.md` for rationale + the spec refresh that pulled e2e-runner wiring + C11 contract adapt + Derkachi catalog seed + fixture replacement + un-xfail into one ticket) — AZ-523 = 3pt, AZ-524 = 2pt, AZ-526 = 2pt, AZ-527 = 2pt, AZ-528 = 3pt, AZ-558 = 3pt, AZ-589 + AZ-590 retained at 5pt each but closed Won't Fix (treated as 0 effective pts going forward), AZ-591 = 5pt, AZ-592 = 5pt placeholder, AZ-593 = 5pt placeholder, AZ-618 = 0pt umbrella post-split, AZ-619 = 2pt, AZ-620 = 3pt, AZ-621 = 3pt, AZ-622 = 3pt, AZ-623 = 3pt, AZ-624 = 2pt, AZ-687 = 2pt
|
||||
|
||||
Dependencies columns list only the tracker-ID portion (descriptive tail
|
||||
text in each task spec is omitted here for table-readability). The
|
||||
@@ -184,7 +184,7 @@ are all declared and documented below under **Cycle Check**.
|
||||
| AZ-701 | T5: HTTP Replay API service (POST tlog+video, return GPS fixes + map) | 5 | AZ-699, AZ-700 | AZ-696 |
|
||||
| AZ-702 | T6: Topotek KHP20S30 camera calibration (factory-sheet approximation) | 1 | None | AZ-696 |
|
||||
| AZ-776 | Open-loop ESKF composition profile (c4_pose.enabled flag) | 3 | None | AZ-602 |
|
||||
| AZ-777 | Derkachi C6 reference tile cache + FAISS descriptor index (OSM/CARTO) | 5 | AZ-776 | AZ-602 |
|
||||
| AZ-777 | Derkachi e2e: wire EXISTING parent-suite satellite-provider into operator pre-flight fixture | 8 (override) | AZ-776 | AZ-602 |
|
||||
|
||||
## Notes
|
||||
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
# Batch 104 — Cycle 3 — AZ-777 Phase 1
|
||||
|
||||
**Date**: 2026-05-21
|
||||
**Tasks**: AZ-777 Phase 1 (e2e-runner wire + C11 contract adapt + smoke test).
|
||||
**Story points**: 8 (explicit override; see decision log).
|
||||
**Jira status**: AZ-777 → still `In Progress` — Phase 1 of 5 done; STOP gate before Phase 2.
|
||||
|
||||
## What shipped
|
||||
|
||||
The Jetson e2e harness now consumes the **real** parent-suite
|
||||
`satellite-provider` .NET service over its compose-DNS name +
|
||||
self-signed dev TLS cert + Bearer JWT auth. C11's
|
||||
`HttpTileDownloader` has been adapted to the AZ-505 v1.0.0
|
||||
`tile-inventory.md` contract — bulk POST inventory lookup keyed by
|
||||
slippy-map (z,x,y) coords, plus per-tile GET via
|
||||
`/tiles/{z}/{x}/{y}`. A Tier-2 smoke test exercises the wire
|
||||
end-to-end against the running service.
|
||||
|
||||
This batch closes the first of AZ-777's five explicit STOP-gated
|
||||
phases. Phases 2–5 remain on the to-do queue:
|
||||
|
||||
- Phase 2 — Derkachi tile catalog seed via
|
||||
`POST /api/satellite/request` (CC-BY basemap source, license
|
||||
attribution baked in).
|
||||
- Phase 3 — replace the placeholder `operator_pre_flight_setup`
|
||||
fixture with a real C10 + C11 driver that yields a
|
||||
`PopulatedC6Cache`.
|
||||
- Phase 4 — un-xfail the Tier-2 Derkachi AC-3 + AZ-699 verdict
|
||||
tests.
|
||||
- Phase 5 — extend the replay-protocol / architecture / Derkachi
|
||||
README docs.
|
||||
|
||||
## Files changed
|
||||
|
||||
Production (1):
|
||||
|
||||
- `src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py`
|
||||
|
||||
Tests (3):
|
||||
|
||||
- `tests/unit/c11_tile_manager/test_tile_downloader.py` (rewritten;
|
||||
14 AC tests; all PASS)
|
||||
- `tests/e2e/satellite_provider/__init__.py` (new)
|
||||
- `tests/e2e/satellite_provider/test_smoke.py` (new; 2 tier2 tests)
|
||||
|
||||
Compose / env (2):
|
||||
|
||||
- `docker-compose.test.jetson.yml`
|
||||
- `.env.test.example`
|
||||
|
||||
Tooling (2):
|
||||
|
||||
- `scripts/mint_dev_jwt.py` (new)
|
||||
- `pyproject.toml` (added `pyjwt>=2.8,<3.0` to dev extras)
|
||||
|
||||
Tracker docs (3):
|
||||
|
||||
- `_docs/02_tasks/_dependencies_table.md` (AZ-777 5→8pt)
|
||||
- `_docs/03_implementation/reviews/batch_104_review.md` (new)
|
||||
- `_docs/03_implementation/batch_104_cycle3_report.md` (this file)
|
||||
|
||||
## AC coverage
|
||||
|
||||
| AC | Phase 1 portion satisfied? | Evidence |
|
||||
|----|----------------------------|----------|
|
||||
| AC-1 (compose lints; depends_on satellite-provider) | ✅ | `docker compose -f docker-compose.test.jetson.yml config` exits 0 with the new env block. |
|
||||
| AC-2 unit (`_do_enumerate` POST inventory + `_download_one_tile` slippy-map GET) | ✅ | `tests/unit/c11_tile_manager/test_tile_downloader.py` 14/14 PASS. |
|
||||
| AC-2 live (Bearer-authenticated round-trip vs. running service) | ⏸ | `tests/e2e/satellite_provider/test_smoke.py` is in place; runs next time the Jetson harness fires. |
|
||||
| AC-3..6 | ⏳ | Out of scope (Phases 2–5). |
|
||||
|
||||
## Test run results
|
||||
|
||||
```
|
||||
$ python -m pytest tests/unit/c11_tile_manager/ -v --tb=short
|
||||
============================== 58 passed in 3.99s ==============================
|
||||
|
||||
$ python -m pytest tests/unit/runtime_root/ tests/unit/c11_tile_manager/ -v --tb=short
|
||||
============================= 113 passed in 3.68s ==============================
|
||||
|
||||
$ python -m pytest tests/e2e/satellite_provider/test_smoke.py -v --tb=short
|
||||
============================== 2 skipped in 0.68s ==============================
|
||||
(skip reason: AZ-777 satellite-provider smoke gated by RUN_REPLAY_E2E=1)
|
||||
```
|
||||
|
||||
Suite-wide test run is deferred to the end of the AZ-777
|
||||
implementation phase per the iterative-skill exception in
|
||||
`.cursor/rules/coderule.mdc` — Phase 1 is a batch, not the end of
|
||||
implementation. The two test trees that depend on the modified code
|
||||
(`tests/unit/c11_tile_manager/` and `tests/unit/runtime_root/`) are
|
||||
green.
|
||||
|
||||
## Code review
|
||||
|
||||
See `_docs/03_implementation/reviews/batch_104_review.md` —
|
||||
**verdict: PASS_WITH_WARNINGS**. Three findings (1 Medium
|
||||
Architecture, 1 Medium Maintainability, 1 Low Maintainability); all
|
||||
deferred to later AZ-777 phases or future tuning with clear
|
||||
ownership. No Critical or High findings.
|
||||
|
||||
## Risks acknowledged on this batch
|
||||
|
||||
- **TLS_INSECURE not in production code path yet** — only the smoke
|
||||
test honours `SATELLITE_PROVIDER_TLS_INSECURE`. Phase 3 (the real
|
||||
`operator_pre_flight_setup` fixture) is the first production-ish
|
||||
consumer of `HttpTileDownloader`; it MUST plumb the flag through.
|
||||
Flagged as F1 in the batch review.
|
||||
- **`_DEFAULT_ESTIMATED_TILE_BYTES = 50 KiB`** — conservative for
|
||||
CARTO Voyager basemap; may under-reserve for UAV-uploaded tiles.
|
||||
Acceptable for Phase 1; revisit in Phase 5. Flagged as F2.
|
||||
- **Smoke test passes when catalog is empty** — by design;
|
||||
exercises the wire pre-Phase-2 and tightens automatically once
|
||||
Phase 2 seeds tiles. Flagged as F3.
|
||||
|
||||
## STOP gate
|
||||
|
||||
This batch closes Phase 1 of AZ-777's 5-phase plan. The next phase
|
||||
(Derkachi tile catalog seed) needs operator alignment on the
|
||||
imagery source (CARTO Voyager Basemap proposed in the spec) and on
|
||||
the bbox / zoom-range envelope. Pause for user decision before
|
||||
Phase 2.
|
||||
@@ -0,0 +1,241 @@
|
||||
# Code Review Report
|
||||
|
||||
**Batch**: AZ-777 Phase 1 (e2e-runner wire + C11 contract adapt + smoke test)
|
||||
**Date**: 2026-05-21
|
||||
**Verdict**: PASS_WITH_WARNINGS
|
||||
|
||||
## Scope
|
||||
|
||||
AZ-777 is an 8-pt task with 5 explicit STOP-gated phases. This batch
|
||||
delivers **Phase 1 only** — the e2e-runner wiring to the existing
|
||||
parent-suite satellite-provider service + the C11 `HttpTileDownloader`
|
||||
contract adaptation to the AZ-505 v1.0.0 `tile-inventory.md` API +
|
||||
the Tier-2 smoke test that validates the wire.
|
||||
|
||||
Phases 2–5 (catalog seed via `POST /api/satellite/request`, real
|
||||
`operator_pre_flight_setup` fixture, un-xfail Tier-2 tests, docs)
|
||||
are out of scope for this batch and are gated behind a STOP per the
|
||||
task spec's Risk-5 mitigation.
|
||||
|
||||
## Files changed
|
||||
|
||||
Production (1):
|
||||
|
||||
- `src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py`
|
||||
— `_LIST_PATH` / `_GET_PATH` replaced with `_INVENTORY_PATH`
|
||||
(`POST /api/satellite/tiles/inventory`) + `_TILES_PATH`
|
||||
(`GET /tiles/{z}/{x}/{y}`); `_do_enumerate` rewritten to enumerate
|
||||
bbox tile coords client-side and POST chunked inventory requests;
|
||||
`_download_one_tile` re-routes to slippy-map URL; common retry /
|
||||
auth logic refactored into `_send_request`; new module helpers:
|
||||
`_enumerate_bbox_tile_coords`, `_tile_center_latlon`,
|
||||
`_tile_size_meters_at`, `_format_tile_id_str`, `_parse_tile_id_str`,
|
||||
`_chunk_iter`; new constants `_DEFAULT_ESTIMATED_TILE_BYTES`
|
||||
(50 KiB, conservative tile-size estimate since inventory no longer
|
||||
returns content-length hints), `_INVENTORY_MAX_ENTRIES_PER_REQUEST`,
|
||||
`_EARTH_EQUATORIAL_CIRCUMFERENCE_M`, `_TILE_SIZE_PIXELS`.
|
||||
|
||||
Tests (2):
|
||||
|
||||
- `tests/unit/c11_tile_manager/test_tile_downloader.py` — all 14
|
||||
AC tests rewritten to drive `_make_inventory_handler` (POST
|
||||
inventory + GET tile) instead of the old GET-list handler;
|
||||
`_StubTileWriter` rekeyed by call-index instead of by
|
||||
`(z,lat,lon)` strings (the downloader now derives lat/lon from
|
||||
the slippy-map coord, so fixtures cannot fabricate arbitrary
|
||||
lat/lons); `_DEFAULT_ESTIMATED_TILE_BYTES` constant mirrored.
|
||||
All 14 tests PASS.
|
||||
- `tests/e2e/satellite_provider/test_smoke.py` (new) — two Tier-2
|
||||
smoke tests: (i) raw `POST /api/satellite/tiles/inventory` for a
|
||||
1-tile Derkachi-bbox query, asserts the documented response
|
||||
schema; (ii) drives the adapted `HttpTileDownloader` against the
|
||||
real service with an in-memory C6 stub (Phase-3 fixture will
|
||||
replace it with real C6).
|
||||
- `tests/e2e/satellite_provider/__init__.py` (new).
|
||||
|
||||
Compose / env (2):
|
||||
|
||||
- `docker-compose.test.jetson.yml` — e2e-runner env block:
|
||||
`SATELLITE_PROVIDER_URL` switched from `http://mock-sat:5100` to
|
||||
`https://satellite-provider:8080`; `SATELLITE_PROVIDER_TLS_INSECURE=1`
|
||||
added (dev-only); `SATELLITE_PROVIDER_API_KEY` sourced from
|
||||
`.env.test`; `JWT_*` forwarded for in-container fallback minting;
|
||||
`depends_on: { satellite-provider: { condition: service_healthy } }`
|
||||
added.
|
||||
- `.env.test.example` — new `SATELLITE_PROVIDER_API_KEY` variable
|
||||
with documentation + dev TLS bypass security note.
|
||||
|
||||
Tooling (2):
|
||||
|
||||
- `scripts/mint_dev_jwt.py` (new) — HS256 dev-JWT mint helper.
|
||||
Reads JWT secret / iss / aud from env or `.env.test`; emits a
|
||||
signed JWT to stdout. Convenience for dev workflows; production
|
||||
retrieves tokens from the admin API.
|
||||
- `pyproject.toml` — added `pyjwt>=2.8,<3.0` to `[dev]` extras.
|
||||
|
||||
Tracker docs (1):
|
||||
|
||||
- `_docs/02_tasks/_dependencies_table.md` — AZ-777 row bumped from
|
||||
5pt to 8pt (matches the 2026-05-21 decision-log override in
|
||||
`_docs/_process_leftovers/2026-05-21_az777_complexity_override.md`).
|
||||
|
||||
## Phase 2 — Spec Compliance
|
||||
|
||||
| AC | Status | Notes |
|
||||
|----|--------|-------|
|
||||
| AC-1 (`docker compose config` exits 0 with `depends_on satellite-provider`) | ✅ Verified | Compose lint passes locally with the new env block. |
|
||||
| AC-2 unit half (`_do_enumerate` POST inventory + `_download_one_tile` slippy-map GET against stubbed responses) | ✅ Verified | 14/14 unit tests PASS against the new contract. |
|
||||
| AC-2 live half (Bearer-authenticated round-trip against the running service) | ⏸ Deferred to Tier-2 Jetson run | Smoke test gated by `RUN_REPLAY_E2E=1` + `tier2`; auto-skips on dev macOS. |
|
||||
| AC-3..6 | ⏳ Out of scope (Phases 2–5) | Phase 1 → 2 STOP gate. |
|
||||
|
||||
No spec gaps within Phase 1. AC-2's live validation runs the next
|
||||
time the Jetson harness fires; the test code is in place.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| 1 | Medium | Architecture | `src/gps_denied_onboard/runtime_root/c11_factory.py` | TLS_INSECURE flag not plumbed through production composition root |
|
||||
| 2 | Medium | Maintainability | `src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py:84` | `_DEFAULT_ESTIMATED_TILE_BYTES` is a project-wide guess, not configurable |
|
||||
| 3 | Low | Maintainability | `tests/e2e/satellite_provider/test_smoke.py` | Smoke test passes when catalog is empty (Phase-2 dependency) |
|
||||
|
||||
### Finding Details
|
||||
|
||||
**F1: TLS_INSECURE flag not plumbed through production composition root**
|
||||
(Medium / Architecture)
|
||||
|
||||
- Location: `src/gps_denied_onboard/runtime_root/c11_factory.py`
|
||||
- Description: `build_tile_downloader` takes a caller-owned
|
||||
`httpx.Client`, so the operator binary that wires C11 is the
|
||||
layer that must honour `SATELLITE_PROVIDER_TLS_INSECURE`. No
|
||||
production caller exists today — `build_tile_downloader` only
|
||||
has the Tier-2 smoke test as a live consumer. Phase 3 of AZ-777
|
||||
introduces the `operator_pre_flight_setup` fixture that will be
|
||||
the first live caller; the TLS_INSECURE handling will land there.
|
||||
- Suggestion: When Phase 3 ships, the new caller must read
|
||||
`SATELLITE_PROVIDER_TLS_INSECURE` and pass the right `verify=`
|
||||
to `httpx.Client(...)` — mirror the approach used in
|
||||
`tests/e2e/satellite_provider/test_smoke.py::_make_http_client`.
|
||||
Also consider a `WARNING` log line at startup whenever the
|
||||
insecure flag is active so the operator can audit it.
|
||||
- Task: AZ-777 Phase 3 (deferred)
|
||||
|
||||
**F2: `_DEFAULT_ESTIMATED_TILE_BYTES` is a project-wide guess**
|
||||
(Medium / Maintainability)
|
||||
|
||||
- Location: `src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py:84`
|
||||
- Description: The AZ-505 v1.0.0 inventory contract dropped the
|
||||
per-entry `estimatedBytes` field, so the AZ-308 budget pre-check
|
||||
reserves a constant 50 KiB per `present=true` tile. 50 KiB is
|
||||
conservative for typical CARTO Voyager tiles (8-30 KiB) but
|
||||
under-reserves for high-detail UAV uploads (30-80 KiB). The
|
||||
budget can over-reserve safely; under-reserving fails the
|
||||
AZ-308 contract.
|
||||
- Suggestion: Either (a) add the constant to `C11Config` so
|
||||
operators can tune it per imagery source, or (b) file a
|
||||
parent-suite ticket to restore `estimatedBytes` in the inventory
|
||||
response. For Phase 1 the constant is acceptable; revisit in
|
||||
Phase 5 docs.
|
||||
- Task: AZ-777 Phase 5 / future config refactor
|
||||
|
||||
**F3: Smoke test passes when catalog is empty** (Low / Maintainability)
|
||||
|
||||
- Location: `tests/e2e/satellite_provider/test_smoke.py:test_smoke_c11_download_via_http_pipeline`
|
||||
- Description: The C11 pipeline smoke asserts SUCCESS with
|
||||
`tiles_downloaded == len(write_calls)`. Pre-Phase-2 the catalog
|
||||
is empty → every entry comes back `present=false` → the test
|
||||
passes with zero downloads, which proves the wire works but
|
||||
does NOT prove tiles actually land in C6. The conditional
|
||||
`if report.tiles_downloaded > 0` block tightens the assertion
|
||||
once the catalog is seeded.
|
||||
- Suggestion: Accepted by design for Phase 1; Phase 2's catalog
|
||||
seed automatically turns this from "wire works" into "tiles
|
||||
land" without test changes.
|
||||
- Task: AZ-777 Phase 2
|
||||
|
||||
## Phase 3 — Code Quality
|
||||
|
||||
- **SOLID**: `_send_request` consolidates GET / POST retry + auth
|
||||
in one place instead of two near-duplicates; methods stay small
|
||||
(`_send_get` / `_send_post` are 5-line shims over the common
|
||||
path). Slippy-map helpers are module-level pure functions —
|
||||
they don't reach for `self` and don't depend on `httpx`, so
|
||||
the unit tests can reuse them directly.
|
||||
- **Error handling**: every failure path raises a typed C11 error
|
||||
(`SatelliteProviderError`, `RateLimitedError`,
|
||||
`CacheBudgetExceededError`); no bare `except`s; no silently
|
||||
swallowed errors. The Retry-After parser handles both seconds-
|
||||
and HTTP-date forms; OOB values clamp to 0 instead of
|
||||
propagating garbage.
|
||||
- **Naming**: `_inventory_path` / `_tiles_path` / `_tile_id_str` /
|
||||
`_parse_tile_id_str` etc. all read directly against the AZ-505
|
||||
contract; no surprises.
|
||||
- **Complexity**: `_send_request` is the longest method at ~80 LOC
|
||||
but it's a linear retry ladder; cyclomatic complexity is
|
||||
bounded by the four response branches (transport-error / 401-3
|
||||
/ 429 / 5xx / 200). `_do_enumerate` is 14 LOC.
|
||||
- **Test quality**: every AC test arranges a specific contract
|
||||
scenario (POST inventory + GET tile) and asserts both the
|
||||
downloader's report counts AND the C6 stub's call records.
|
||||
Tests do NOT just "assert no exception".
|
||||
|
||||
## Phase 4 — Security Quick-Scan
|
||||
|
||||
- No SQL strings touched.
|
||||
- JWT mint helper uses PyJWT's `jwt.encode` with HS256; no
|
||||
hand-rolled crypto; secrets come from env, never hardcoded.
|
||||
- `.env.test.example`'s `SATELLITE_PROVIDER_API_KEY` placeholder
|
||||
is `PASTE-MINTED-JWT-HERE` — the smoke test treats that exact
|
||||
string as "unset" and skips, so a developer accidentally
|
||||
committing the placeholder cannot get false confidence.
|
||||
- `Authorization` header redacted in error logs as
|
||||
`Bearer ***` per AZ-316 AC-11; the AC-11 test verifies the
|
||||
real API key never appears in any log record.
|
||||
- `SATELLITE_PROVIDER_TLS_INSECURE` is opt-in via env var;
|
||||
default is verify=True. The dev-only nature is documented in
|
||||
the compose comment, in `.env.test.example`, and (when Phase 3
|
||||
lands) will be logged at startup.
|
||||
|
||||
## Phase 5 — Performance Scan
|
||||
|
||||
- Inventory POST chunks at 5000 entries per the contract cap; one
|
||||
POST per up-to-5000-tile bbox.
|
||||
- Backoff schedule unchanged (`_DEFAULT_BACKOFF_SCHEDULE_S =
|
||||
(1, 2, 4, 8)`); session retry budget enforced.
|
||||
- `test_nfr_throughput_1000_tiles_under_budget` passes in <1 s
|
||||
locally (budget is 10 s) — no O(n²) bookkeeping regression.
|
||||
- No N+1 patterns; no blocking I/O in async paths (whole module
|
||||
is sync).
|
||||
|
||||
## Phase 6 — Cross-Task Consistency
|
||||
|
||||
Single task in this batch (AZ-777 Phase 1). N/A.
|
||||
|
||||
## Phase 7 — Architecture Compliance
|
||||
|
||||
- **Layer direction**: C11 still does not import C6 directly; the
|
||||
Protocol cuts (`_TileWriterLike`, `_BudgetEnforcerLike`) stay
|
||||
in `tile_downloader.py`. The composition root
|
||||
(`runtime_root/c11_factory.py::_C6DownloadAdapter`) remains the
|
||||
single bridge.
|
||||
- **Public API respect**: no cross-component imports added.
|
||||
- **No new cyclic deps**: no new module-level imports between
|
||||
components.
|
||||
- **Architecture principle #5** (`satellite-provider` owns OSM /
|
||||
CARTO tile network I/O; the onboard companion is read-only via
|
||||
C11 during pre-flight): this batch is the first time C11 is
|
||||
actually wired to consume that contract — the principle is
|
||||
honoured for the first time end-to-end.
|
||||
- **ADR compliance**: ADR-004 (event-driven cross-component
|
||||
comms): C11 → satellite-provider is HTTP, which is explicitly
|
||||
scoped out of ADR-004 (the ADR governs intra-onboard comms,
|
||||
not external-service calls). No drift. No new ADR required —
|
||||
the task spec explicitly states this is execution of existing
|
||||
decisions.
|
||||
|
||||
## Verdict justification
|
||||
|
||||
PASS_WITH_WARNINGS — three Medium / Low findings, no Critical or
|
||||
High. The Medium findings are deferred to later AZ-777 phases or
|
||||
to future tuning, with clear ownership; no blocking gap in Phase 1
|
||||
itself.
|
||||
@@ -4,12 +4,12 @@
|
||||
flow: existing-code
|
||||
step: 10
|
||||
name: Implement
|
||||
status: paused
|
||||
status: in_progress
|
||||
sub_step:
|
||||
phase: 7
|
||||
name: batch-loop
|
||||
detail: "batch 104 cycle3: AZ-777 spec re-refined against codebase reality + Jira synced; Phase 1 ready (see canonical spec + 2026-05-21 decision-log addendum)"
|
||||
detail: "AZ-777 Phase 1 done; STOP gate before Phase 2 (catalog seed)"
|
||||
retry_count: 0
|
||||
cycle: 3
|
||||
tracker: jira
|
||||
last_completed_batch: 103
|
||||
last_completed_batch: 104
|
||||
|
||||
Reference in New Issue
Block a user