satellite-provider/_docs/03_implementation/batch_10_report.md

Batch 10 Report — Refactor 03 Phase 2 (continued)

Date: 2026-05-10 Epic: AZ-350 (03-code-quality-refactoring) Status: ✅ Complete, pushed (after batch 11 commit, riding with 09 cumulative review)

Scope (1 task / 5 SP)

ID	C-ID	Title	Points	Component
AZ-357	C06	Drop tile Version concept; latest row wins; new migration	5	Services.TileDownloader + DataAccess

Single-task batch — DB migration is higher risk and benefits from dedicated review focus.

Changes

Migration

• NEW SatelliteProvider.DataAccess/Migrations/012_DropTileVersionConstraint.sql
  • Drops idx_tiles_unique_location (5-column).
  • Dedupes by 4-column key using ROW_NUMBER() OVER (PARTITION BY ... ORDER BY updated_at DESC, id DESC) — keeps latest row per cell, deterministic tie-break by id.
  • Recreates idx_tiles_unique_location on (latitude, longitude, tile_zoom, tile_size_meters).
  • ALTER COLUMN version DROP NOT NULL and DROP DEFAULT so new rows can store NULL.
  • Column itself preserved (per coderule.mdc — no column drops without confirmation; covered by AZ-373 / C20 separately).

Production

• MODIFIED SatelliteProvider.DataAccess/Models/TileEntity.cs
  • Version changed from int → int? (matches the now-nullable column).
• MODIFIED SatelliteProvider.Common/DTO/TileMetadata.cs
  • Version changed to int? to surface the nullable column to consumers (HTTP shape preserved per the task constraint — the field is still present in JSON).
• MODIFIED SatelliteProvider.Api/Program.cs (DownloadTileResponse)
  • Version changed to int? for the same reason.
• MODIFIED SatelliteProvider.DataAccess/Repositories/TileRepository.cs
  • InsertAsync.ON CONFLICT clause: 5-col → 4-col (drops version).
  • GetByTileCoordinatesAsync: ORDER BY version DESC → ORDER BY updated_at DESC (latest row wins per AC-1).
  • GetTilesByRegionAsync: ORDER BY version DESC, latitude DESC, longitude ASC → ORDER BY latitude DESC, longitude ASC, updated_at DESC (after migration there's at most 1 row per cell so version-ordering is meaningless; updated_at is the meaningful tie-break).
  • FindExistingTileAsync left untouched — slated for deletion in AZ-376 / C23.
• MODIFIED SatelliteProvider.Services.TileDownloader/TileService.cs
  • Removed var currentVersion = DateTime.UtcNow.Year; and the .Where(t => t.Version == currentVersion) cache filter (root cause of LF-1: cache expiring on Jan 1).
  • BuildTileEntity signature: dropped the currentVersion parameter; now writes Version = null. New code never writes the deprecated year value.
  • All 3 call sites updated to drop the year argument.

Tests

• MODIFIED SatelliteProvider.Tests/TileServiceTests.cs
  • Replaced DownloadAndStoreTilesAsync_IgnoresStaleVersionCachedTiles_BT02b with DownloadAndStoreTilesAsync_TreatsCachedTileFromPriorYearAsFresh_AZ357_AC1 — same setup with a Version = Year - 1 row, but inverted assertion: the cached tile IS reused (not re-downloaded). Directly proves AC-1 (cache survives year boundary).
  • Added BuildTileEntity_DoesNotPopulateVersion_AZ357 — captures the entity passed to InsertAsync and asserts Version == null. Enforces the "new code does not write to it" constraint.

Verification

• Unit tests: 69 / 69 passing (was 68 → +2 new AZ-357 tests, −1 inverted/replaced test = net +1).
• Integration smoke + full suite: green. Container exits 0. The 20-point extended-route test ran 690 tiles end-to-end with the new schema applied to a fresh Postgres volume — exercises:
  • Insert path: writes Version = null, conflicts on the new 4-col key.
  • Read path: GetTilesByRegionAsync returns tiles ordered by updated_at DESC.
  • GetOrDownloadTileAsync cache-hit path: tile lookup uses ORDER BY updated_at DESC.

Acceptance criteria coverage

AC	Evidence
AC-1 Cache survives year boundary	Unit test TreatsCachedTileFromPriorYearAsFresh_AZ357_AC1: prior-year Version row reused; InsertAsync not called.
AC-2 Migration runs cleanly on populated tile data	New integration tests in MigrationTests.cs: DedupeSqlCollapsesDuplicatesByLatestUpdatedAt_AZ357_AC2 exercises the dedupe DELETE against a temp table with intentional 4-column duplicates (3-way duplicate, updated_at-tie broken by id, plus a unique row); NewUniqueConstraintExistsOnFourColumns_AZ357_AC2 queries pg_indexes to confirm the recreated index has the new shape and excludes version. Added in a follow-up commit before batch 11; see "AC-2 follow-up" below.
AC-3 Upsert behaves on the new key	New InsertAsync.ON CONFLICT (latitude, longitude, tile_zoom, tile_size_meters) clause; integration suite re-runs identical (lat,lon,zoom,size) inserts during the route test (690 tiles processed without unique-violation errors).
AC-4 37 unit + 5 smoke tests stay green	69 unit + 5 smoke + 3 stub-contract green.

AC-2 follow-up (closed)

The original batch left AC-2 partially covered — the migration ran against an empty DB volume on integration startup. Per user direction (option B at batch 10 review pause), a populated-duplicates test was added in a follow-up commit before batch 11:

• NEW SatelliteProvider.IntegrationTests/MigrationTests.cs with two tests:
  1. DedupeSqlCollapsesDuplicatesByLatestUpdatedAt_AZ357_AC2 — creates a session-scoped tiles_dedupe_test temp table, seeds it with intentional 4-column duplicates that have varying updated_at and id, runs the exact dedupe SQL from migration 012, and asserts only the expected rows survive (newest updated_at wins; ties broken by largest id; unique rows preserved).
  2. NewUniqueConstraintExistsOnFourColumns_AZ357_AC2 — queries pg_indexes against the live DB to assert idx_tiles_unique_location exists as a unique 4-column btree index and does not include the version column. Catches the case where a developer skips the migration or rolls back the index recreation.
• NEW dependency Npgsql 9.0.2 in SatelliteProvider.IntegrationTests.csproj for direct DB connectivity (matches the version used elsewhere in the suite).
• MODIFIED docker-compose.tests.yml — added DB_CONNECTION_STRING env var and postgres: condition: service_healthy to integration-tests.depends_on so the test container can connect directly to the same DB the API uses.
• MODIFIED SatelliteProvider.IntegrationTests/Program.cs — wired MigrationTests.RunAll() into both smoke and full suites.

Implementation note (debugging trace): first attempt used CREATE TEMP TABLE ... ON COMMIT DROP. Each Npgsql command runs in its own implicit transaction by default, so the table was dropped immediately after the CREATE committed. Removed the ON COMMIT DROP clause — temp tables are automatically dropped when the connection closes (session-scoped), which is exactly what we want.

Result: AC-2 now fully covered. Tests run on every integration suite invocation (smoke + full), green.

Behavior preservation

• HTTP shape: DownloadTileResponse still has version field. JSON output is "version": null for new tiles, "version": 2025 (or other year) for tiles inserted before this migration. Consumers parsing as int? (most JSON libraries default to nullable) are unaffected; consumers parsing as int would need to handle null. None observed in the suite.
• Cache semantics: stricter (cache survives year flip) — the intended behavior. The replaced test asserted the bug; the new test asserts the fix.

Up next

• Batch 11: AZ-362 (idempotent POST contract for caller-supplied GUIDs, 3 SP) — Api + RegionProcessing + RouteManagement. Depends on AZ-353 (done in batch 8). This will be the next-and-final batch this session unless paused.
• After batch 11, K=3 cumulative review trigger fires again (batches 10, 11, 12) — but only 2 batches new, so falls below threshold. Continue per user direction.