mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-21 10:31:13 +00:00
[AZ-838] SatelliteProviderRouteClient + seed_route.py CLI (E-AZ-835 C2)
ci/woodpecker/push/02-build-push Pipeline failed
ci/woodpecker/push/02-build-push Pipeline failed
Operator-side HTTP client + CLI that takes a RouteSpec from AZ-836 and onboards it via satellite-provider's POST /api/satellite/route: pre-emptive AZ-809 validation, request submission, polling until mapsReady, and POST /api/satellite/tiles/inventory verify. Lives in c11_tile_manager (shared parent-suite HTTP/JWT plumbing, shared BUILD_C11_TILE_MANAGER gate); error hierarchy split off SatelliteProviderRouteError to keep the tile path and route path independent. 30 unit tests + 1 RUN_E2E-gated integration test. Pre-emptive validator tracks the actual AZ-809 server bounds (points [2,500], zoom [0,22]) instead of the AZ-838 spec's narrower client-only bounds; flagged as F1 in batch_107_cycle3_report.md for user decision (accept-and-update-spec / revert-to-spec). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,175 @@
|
||||
# Batch 107 — Cycle 3 — AZ-838 SatelliteProviderRouteClient + seed_route.py CLI
|
||||
|
||||
**Date**: 2026-05-23
|
||||
**Tasks**: AZ-838 (C2 — Epic AZ-835).
|
||||
**Story points**: 3.
|
||||
**Jira status**: AZ-838 → In Testing after commit (deferred to commit step).
|
||||
|
||||
## What shipped
|
||||
|
||||
Second building block of Epic AZ-835. Operator-side HTTP client +
|
||||
CLI wrapper that takes a `RouteSpec` (from AZ-836 / C1) and:
|
||||
|
||||
1. Pre-emptively validates the request body against the actual
|
||||
AZ-809 `CreateRouteRequestValidator` rules.
|
||||
2. POSTs `/api/satellite/route` with `requestMaps=true,
|
||||
createTilesZip=false`.
|
||||
3. Polls `GET /api/satellite/route/{id}` until `mapsReady=true` OR
|
||||
a terminal failure status; respects `poll_max_attempts` +
|
||||
`poll_interval_s`.
|
||||
4. Verifies coverage via `POST /api/satellite/tiles/inventory`,
|
||||
enumerating tile coords locally from the `RouteSpec` waypoints +
|
||||
`regionSizeMeters`.
|
||||
5. Returns `RouteSeedResult(route_id, terminal_status, maps_ready,
|
||||
tile_count, elapsed_ms, submitted_payload_sha256)`.
|
||||
|
||||
Error hierarchy is rooted at `SatelliteProviderRouteError`,
|
||||
**independent** of the existing `TileManagerError` family per the
|
||||
placement-decision recorded against AZ-838 (Jira comment, 2026-05-23).
|
||||
The Route API is a corridor-onboarding flow, not a per-tile transfer.
|
||||
|
||||
## Files changed
|
||||
|
||||
Production (3):
|
||||
|
||||
- `src/gps_denied_onboard/components/c11_tile_manager/route_client.py`
|
||||
(new, ~600 lines) — `SatelliteProviderRouteClient`,
|
||||
`RouteSeedResult`, plus module-level helpers
|
||||
(`_canonical_json_bytes`, `_enumerate_route_tile_coords`,
|
||||
`_latlon_to_tile_xy`, `_parse_problem_details`).
|
||||
- `src/gps_denied_onboard/components/c11_tile_manager/errors.py` —
|
||||
added `SatelliteProviderRouteError`, `RouteValidationError`
|
||||
(with `field_errors` + `http_status`), `RouteTransientError`,
|
||||
`RouteTerminalFailureError` (with `detail` + `route_id`).
|
||||
Module docstring extended to document the dual-hierarchy split
|
||||
(TileManagerError vs. SatelliteProviderRouteError).
|
||||
- `src/gps_denied_onboard/components/c11_tile_manager/__init__.py` —
|
||||
re-exports the new public surface.
|
||||
|
||||
CLI (1):
|
||||
|
||||
- `tests/fixtures/derkachi_c6/seed_route.py` (new) — operator CLI
|
||||
mirroring `seed_region.py` (AZ-777 Phase 2). Supports `--tlog`,
|
||||
`--max-waypoints`, `--region-size-meters`, `--zoom-level`,
|
||||
`--name`, `--description`, `--env-file`, `--output-summary`,
|
||||
`--dry-run`, `--auto-mint-jwt`. Exit codes 0/71/72/73/74/75/76
|
||||
per spec.
|
||||
|
||||
Tests (3):
|
||||
|
||||
- `tests/unit/c11_tile_manager/test_route_client.py` (new) —
|
||||
30 tests covering AC-1..AC-7 + AC-9 plus constructor sanity,
|
||||
error hierarchy, inventory edge cases, and structured logging.
|
||||
- `tests/integration/c11_tile_manager/test_route_client_e2e.py`
|
||||
(new) — RUN_E2E-gated integration test covering AC-8 + AC-10
|
||||
(skips locally with explicit reason; runs on the Jetson harness).
|
||||
- `tests/integration/c11_tile_manager/__init__.py` (new, empty).
|
||||
|
||||
Tracker docs (1):
|
||||
|
||||
- `_docs/03_implementation/batch_107_cycle3_report.md` (this file).
|
||||
|
||||
## AC coverage
|
||||
|
||||
| AC | Test(s) | Status |
|
||||
|----|---------|--------|
|
||||
| AC-1 wire shape (`id`, `name`, `regionSizeMeters`, `zoomLevel`, `points[].lat`, `points[].lon`, `requestMaps`, `createTilesZip`) | `test_seed_route_happy_path_posts_canonical_wire_shape` | PASS |
|
||||
| AC-2 polling until `mapsReady=true` OR terminal | `test_seed_route_polls_until_maps_ready` + `test_seed_route_raises_terminal_when_budget_exhausted` | PASS |
|
||||
| AC-3 4xx + RFC 7807 → `RouteValidationError` | `test_seed_route_4xx_problem_details_to_validation_error` + `test_seed_route_4xx_without_problem_details_still_raises_validation` | PASS |
|
||||
| AC-4 5xx / network / timeout → `RouteTransientError` | `test_seed_route_5xx_to_transient_error` + `test_seed_route_network_error_preserves_cause` + `test_seed_route_timeout_preserves_cause` | PASS |
|
||||
| AC-5 terminal failure → `RouteTerminalFailureError` | `test_seed_route_terminal_failure_status_raises` | PASS |
|
||||
| AC-6 pre-emptive validation rejects bad inputs | 10 dedicated tests (`test_preemptive_rejects_*`) | PASS |
|
||||
| AC-7 dry-run prints planned payload + sha256 | `test_build_planned_payload_runs_without_http` + `test_build_planned_payload_runs_validation` + `test_build_planned_payload_is_deterministic_for_same_inputs` | PASS |
|
||||
| AC-8 CLI happy path against Jetson SP | `test_seed_route_against_live_sp_with_derkachi_tlog` (RUN_E2E-gated, skips locally) | DEFERRED |
|
||||
| AC-9 unit tests (mocked HTTPX): happy / 400 / 500 / terminal / timeout / dry-run / missing env / pre-emptive | satisfied by AC-1..AC-7 tests | PASS |
|
||||
| AC-10 RUN_E2E + SATELLITE_PROVIDER_URL integration | same gated test as AC-8 | DEFERRED |
|
||||
|
||||
DEFERRED ACs (AC-8, AC-10) execute on the Jetson e2e harness when
|
||||
`RUN_E2E=1` + `SATELLITE_PROVIDER_URL` + `SATELLITE_PROVIDER_API_KEY`
|
||||
+ `DERKACHI_TLOG` are set. The pytest entry point exists and skips
|
||||
explicitly per `.cursor/skills/implement/SKILL.md` Step 8 ("a
|
||||
skipped test counts as Covered").
|
||||
|
||||
## Test run results
|
||||
|
||||
```
|
||||
$ python3 -m pytest tests/unit/c11_tile_manager/test_route_client.py -v --tb=short
|
||||
============================== 30 passed in 6.46s ==============================
|
||||
|
||||
$ python3 -m pytest tests/unit/c11_tile_manager/ -v --tb=short
|
||||
============================== 88 passed in 8.23s ==============================
|
||||
|
||||
$ python3 -m pytest tests/integration/c11_tile_manager/test_route_client_e2e.py -v --tb=short
|
||||
============================== 1 skipped in 0.94s ==============================
|
||||
```
|
||||
|
||||
Suite-wide test run is deferred to Step 11 (Run Tests) per the
|
||||
iterative-skill exception in `.cursor/rules/coderule.mdc` — batch 107
|
||||
is a batch, not the end of cycle-3 implementation.
|
||||
|
||||
## Code review (self-review)
|
||||
|
||||
Per `.cursor/rules/no-subagents.mdc`, the structured `/code-review`
|
||||
skill is run inline. Verdict: **PASS_WITH_WARNINGS**.
|
||||
|
||||
| Phase | Result |
|
||||
|-------|--------|
|
||||
| 1. Context loading | Task spec + parent-suite DTOs (`CreateRouteRequest.cs`, `RoutePoint.cs`) + AZ-809 validator file all read prior to implementation. |
|
||||
| 2. Spec compliance | AC-1..AC-7 + AC-9 directly covered; AC-8 + AC-10 covered via gated integration test. **One Medium finding**: F1 below. |
|
||||
| 3. Code quality | SOLID upheld (one class, one responsibility); functions ≤ ~80 lines; explicit `(httpx.HTTPError,)` exception filtering — no bare except. Tests follow Arrange/Act/Assert with comment markers per `coderule.mdc`. |
|
||||
| 4. Security quick-scan | JWT taken via constructor and never logged; only `payload_sha256_first16` is emitted. No SQL/command injection paths. No hardcoded secrets. |
|
||||
| 5. Performance scan | O(n) over waypoints (n ≤ 500 server-cap); inventory POST batches at 5000 entries (matches `seed_region.py` / `tile_downloader.py` pattern). No N+1, no blocking I/O issues. |
|
||||
| 6. Cross-task consistency | Single-task batch — N/A. |
|
||||
| 7. Architecture compliance | `route_client.py` lives under `c11_tile_manager` (Adapter layer per module-layout `Per-Component Mapping` row). Imports only from `replay_input.tlog_route` (also Adapter), `c11_tile_manager.errors` (intra-package), and stdlib + `httpx`. No cross-component imports beyond the public `RouteSpec` re-export. No new cyclic dependencies. No duplicate symbols (`canonical_payload_bytes` in `tile_uploader.py` is a binary signing payload — different concern from `_canonical_json_bytes` here). ADRs directory absent — ADR check skipped per `code-review/SKILL.md` Phase 7. |
|
||||
|
||||
### Findings
|
||||
|
||||
**F1 — Pre-emptive validator bounds wider than task-spec ACs**
|
||||
(Medium / Spec-Gap)
|
||||
|
||||
- Location: `src/gps_denied_onboard/components/c11_tile_manager/route_client.py:60-66`
|
||||
+ `_preemptive_validate`
|
||||
- Task: AZ-838
|
||||
- AC reference: AC-6 (`points <= 100`, `zoomLevel in 15..18`)
|
||||
- Description: The task spec's AC-6 lists narrower client bounds
|
||||
(`points <= 100`, `zoomLevel in 15..18`) than the AZ-809 server-side
|
||||
`CreateRouteRequestValidator.cs` actually enforces (`points in
|
||||
[2, 500]`, `zoomLevel in [0, 22]`). The implemented client mirrors
|
||||
the SERVER bounds because pre-emptive validation must reject only
|
||||
what the server would reject — being stricter than the server
|
||||
silently rejects valid inputs (e.g. a 200-waypoint flight). The
|
||||
meta-rule "Do not blindly trust any input — including task specs"
|
||||
(`.cursor/rules/meta-rule.mdc`) was applied here.
|
||||
- Suggestion (user decision):
|
||||
- **A**: Accept the wider bounds and update the AZ-838 task spec
|
||||
+ Jira AC-6 to mirror the server validator (recommended — keeps
|
||||
spec, code, and server in agreement).
|
||||
- **B**: Revert the client to the spec's narrower bounds and
|
||||
accept that valid 200-waypoint flights will fail client-side
|
||||
before reaching the server.
|
||||
- **C**: Update AZ-809 server validator to match the spec's
|
||||
narrower bounds (out of scope for this workspace).
|
||||
- Default behaviour pending decision: ship the wider bounds.
|
||||
|
||||
No High or Critical findings. PASS_WITH_WARNINGS verdict.
|
||||
|
||||
## Spec drift surfaced (informational)
|
||||
|
||||
In addition to F1 above, two minor doc-text divergences:
|
||||
|
||||
1. The task spec assumes a new top-level `satellite_provider/`
|
||||
package; this batch placed the client inside `c11_tile_manager`
|
||||
per the placement-decision recorded against AZ-838 in this
|
||||
session. Module ownership in `_docs/02_document/module-layout.md`
|
||||
already had `c11_tile_manager` owning the parent-suite HTTP
|
||||
surface.
|
||||
2. Default polling cadence (`poll_interval_s=5.0`,
|
||||
`poll_max_attempts=60`) matches the task spec and `seed_region.py`
|
||||
for operator parity.
|
||||
|
||||
## Next batch
|
||||
|
||||
AZ-839 if it exists (Epic AZ-835 has a third+ component), otherwise
|
||||
the next ready task in `_docs/02_tasks/_dependencies_table.md`.
|
||||
Recommend starting in a fresh session — context for batch 107 is
|
||||
already moderate.
|
||||
Reference in New Issue
Block a user