satellite-provider/_docs/03_implementation/reviews/batch_25_cycle1_review.md

Code Review Report

Batch: 25 (cycle 1) Tasks: AZ-484 (Multi-source tile storage schema) Date: 2026-05-11 Verdict: PASS

Findings

None.

Phase Summary

Phase 1 — Context

Read AZ-484 task spec, _docs/02_document/contracts/data-access/tile-storage.md v1.0.0, the existing _docs/02_document/architecture.md Architecture Vision section, and the existing module-layout.md per-component map. Mapped the 15 changed files to AZ-484 (single-task batch).

Phase 2 — Spec Compliance

Walked every AC against code:

AC	Promise	Validating test
AC-1	Per-source coexistence on the same cell	MultiSourceInsertCoexistsUnderNewIndex_AZ484_AC1 (TEMP), NewUniqueConstraintIncludesSourceColumn_AZ484_AC1 (live schema)
AC-2	Most-recent across sources on read	MostRecentAcrossSourcesSelection_AZ484_AC2
AC-3	Same-source UPSERT collapses to one row	SameSourceUpsertReplacesPreviousRow_AZ484_AC3
AC-4	Migration backfill leaves no orphans	BackfillUpdateAssignsGoogleMapsAndCapturedAt_AZ484_AC4 (TEMP simulation of the migration UPDATE)
AC-5	Google Maps path stamps Source + CapturedAt	BuildTileEntity_SetsGoogleMapsSourceAndUtcCapturedAt_AZ484_AC5
AC-6	Existing 200 unit + 5 smoke pass unchanged	Verified via the full suite run (handed off to autodev Step 11)
AC-7	Architecture / glossary / module-layout / contract updated	Documents amended in this batch; contract Status flipped from draft to frozen

Contract verification against _docs/02_document/contracts/data-access/tile-storage.md v1.0.0:

• Shape: source VARCHAR(32) NOT NULL, captured_at TIMESTAMP NOT NULL — matches migration 013.
• 5-column unique index idx_tiles_unique_location_source — created by migration 013.
• Producer write API: InsertAsync UPSERT on the 5-column key, refreshes captured_at/updated_at/file_path/tile_x/tile_y — matches.
• Consumer read API: GetByTileCoordinatesAsync LIMIT 1 ordered by (captured_at DESC, updated_at DESC, id DESC); GetTilesByRegionAsync uses DISTINCT ON (latitude, longitude, tile_zoom, tile_size_meters) with the same tie-break tuple — matches.
• Wire format: TileSource.GoogleMaps → 'google_maps', TileSource.Uav → 'uav' enforced by TileSourceTypeHandler (necessary because the generic EnumStringTypeHandler<T> would emit 'googlemaps').
• Inv-1 / Inv-2 / Inv-5: NOT NULL columns + handler Parse throws DataException on unknown values (no silent coercion per coderule.mdc).
• Inv-3: 5-column unique index.
• Inv-4: identical tie-break tuple in GetByTileCoordinatesAsync and the inner DISTINCT ON of GetTilesByRegionAsync guarantees identical winner per cell.

Phase 3 — Code Quality

• SRP: TileSourceTypeHandler is a focused persistence concern (the bidirectional wire-format mapping); kept separate from the generic EnumStringTypeHandler<T> instead of leaking snake_case logic into the generic.
• Comments: only added where intent is non-obvious (snake_case wire-format requirement, new ORDER BY tuple, per-source UPSERT semantics, transactional migration rationale). No narration-of-code comments.
• Tests: every new test uses Arrange / Act / Assert.
• DRY: CreateTempTilesTable factored out across the three TEMP-table integration tests.

Phase 4 — Security Quick-Scan

• All SQL parameters bound (@Source, @CapturedAt, etc.) — no string interpolation of caller-supplied values.
• Migration backfill literal is 'google_maps', not user input.
• No new secrets or credentials introduced.

Phase 5 — Performance Scan

• The new DISTINCT ON in GetTilesByRegionAsync can use idx_tiles_unique_location_source for the partition prefix; no extra round-trip; slow-query log threshold preserved.
• No N+1 patterns introduced.

Phase 6 — Cross-Task Consistency

Single-task batch. Internal consistency: enum members, wire values, migration backfill literal, and test assertions all agree on 'google_maps' / 'uav'.

Phase 7 — Architecture Compliance

• Layering: TileSource enum lives in SatelliteProvider.Common.Enums (Layer 1 Foundation). DataAccess (Layer 1) and TileDownloader (Layer 3) both consume it through Common — no new cross-sibling ProjectReferences.
• Public API respect: TileSource and TileSourceTypeHandler are public; module-layout.md Common Public API list updated to include TileSource.cs.
• No new cycles.
• No duplicate symbols across components.

Baseline Delta

Not computed inline — this batch makes no structural changes that would shift the existing _docs/02_document/architecture_compliance_baseline.md deltas. The AZ-484 changes stay within the existing layering invariants confirmed in earlier baseline scans.

Verdict Logic

No Critical, High, Medium, or Low findings → PASS.