[AZ-350] Cumulative K=3 review for batches 19-21: PASS_WITH_WARNINGS

F1 (Low/Maintainability): module-layout.md docs stale on DataAccess
project reference after AZ-370; tracked for refactor Phase 7.
F2 (Low/Maintainability): redundant builder.Services.AddHttpClient()
in Program.cs after AZ-374; deferred per batch 21 design note.
No Critical/High findings; auto-chain to next batch (AZ-372).

Co-authored-by: Cursor <cursoragent@cursor.com>

_docs/03_implementation/cumulative_review_batches_19-21_cycle1_report.md

@@ -0,0 +1,231 @@
+# Cumulative Code Review — Batches 19-21 (cycle 1)
+
+**Window**: Batches 19 (AZ-370), 20 (AZ-373), 21 (AZ-374)
+**Trigger**: K=3 cumulative review fired after batch 21 (`/implement` Step 14.5)
+**Date**: 2026-05-11
+**Mode**: cumulative (all 7 phases)
+**Verdict**: PASS_WITH_WARNINGS — 2 Low findings
+
+## 1. Scope (cumulative diff vs. previous cumulative review at batch 18)
+
+| Batch | Task(s) | Component(s) | Net LOC |
+|-------|---------|--------------|---------|
+| 19 | AZ-370 (C17 — status / point-type string→enum + DB Dapper handler + AC RT2 update) | Common.Enums + Common.DTO + DataAccess.{Models,Repositories,TypeHandlers} + Services.RegionProcessing + Services.RouteManagement + Api + Tests | +~330 / -~50 |
+| 20 | AZ-373 (C20 — drop `MapsVersion` from new writes; option (a) selected) | Services.TileDownloader.TileService + Common.DTO + Api.Program | +6 / -14 |
+| 21 | AZ-374 (C21 — typed/named `HttpClient` for Google Maps + co-locate registration in component extension) | Services.TileDownloader.{GoogleMapsDownloaderV2, ServiceCollectionExtensions} + Tests | +95 / -22 |
+
+Net cumulative: **+431 / -86** across **29 files**.
+
+**Files in cumulative window** (production):
+
+- **NEW**:
+  - `SatelliteProvider.Common/Enums/RegionStatus.cs` (b19) — `Queued, Processing, Completed, Failed`.
+  - `SatelliteProvider.Common/Enums/RoutePointType.cs` (b19) — `Start, End, Action, Intermediate`.
+  - `SatelliteProvider.DataAccess/TypeHandlers/EnumStringTypeHandler.cs` (b19) — generic `SqlMapper.TypeHandler<T>` + idempotent `DapperEnumTypeHandlers.RegisterAll()` startup registrar.
+
+- **RENAMED** (git-tracked):
+  - `SatelliteProvider.Common/DTO/RegionStatus.cs` → `SatelliteProvider.Common/DTO/RegionStatusResponse.cs` (b19) — DTO renamed to free the `RegionStatus` identifier for the new enum; integration tests already used `RegionStatusResponse` for the same wire shape.
+
+- **MODIFIED** (touched by ≥1 batch in window):
+  - `SatelliteProvider.DataAccess/SatelliteProvider.DataAccess.csproj` (b19: new `<ProjectReference>` to Common — see F1).
+  - `SatelliteProvider.DataAccess/Models/{RegionEntity, RoutePointEntity}.cs` (b19: typed Status/PointType properties).
+  - `SatelliteProvider.DataAccess/Repositories/{IRegionRepository, RegionRepository}.cs` (b19: `GetByStatusAsync` parameter typed).
+  - `SatelliteProvider.Common/Interfaces/IRegionService.cs` (b19: return type now `RegionStatusResponse`).
+  - `SatelliteProvider.Common/DTO/{DownloadTileResponse, TileMetadata, RoutePointDto}.cs` (b19/b20: enum typing on RoutePointDto; `MapsVersion` removed from DownloadTileResponse + TileMetadata).
+  - `SatelliteProvider.Services.RegionProcessing/RegionService.cs` (b19: five string literals → enum members).
+  - `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs` (b19: three region-status comparisons → enum members).
+  - `SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs` (b19: four point-type assignments → enum members).
+  - `SatelliteProvider.Services.TileDownloader/TileService.cs` (b19: `MapToMetadata` enum-aware; b20: `MapsVersion = null`).
+  - `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` (b21: `IHttpClient` → `IHttpClientFactory`; 3 `CreateClient(name)` call sites; 3 `internal const`s).
+  - `SatelliteProvider.Services.TileDownloader/TileDownloaderServiceCollectionExtensions.cs` (b21: `AddHttpClient(name, ...)` registration).
+  - `SatelliteProvider.Api/Program.cs` (b19: `DapperEnumTypeHandlers.RegisterAll()` + `JsonStringEnumConverter(CamelCase)`; b20: drop `MapsVersion` projection in `GetTileByLatLon`).
+
+- **DELETED**: none (file rename via git for `RegionStatus.cs` → `RegionStatusResponse.cs`).
+
+**Test deltas in window**:
+
+| Batch | New tests | Notes |
+|-------|-----------|-------|
+| 19 | +30 | `EnumStringTypeHandlerTests` (28: theories per enum member ×8 lowercase write + ×9 case-insensitive read + 11 rejection/idempotency) + `AcceptanceCriteriaRT2Tests` (1: runtime markdown assertion for AC-3) + retyped expectations in 4 existing test files. |
+| 20 | +3 | `TileServiceTests`: `BuildTileEntity_DoesNotPopulateMapsVersion_AZ373_AC1`, `DownloadTileResponse_DoesNotExposeMapsVersion_AZ373_AC2`, `TileMetadata_DoesNotExposeMapsVersion_AZ373_AC1`. |
+| 21 | +1 | `TileDownloaderRegistrationTests.AddTileDownloader_RegistersNamedGoogleMapsTilesHttpClient_AZ374_AC1` — `ServiceCollection` → `BuildServiceProvider` → `IHttpClientFactory.CreateClient("GoogleMapsTiles")` → asserts User-Agent contains `Mozilla/5.0` and `Timeout == 100s`. |
+
+End-of-window unit count: **175** (was 141 before batch 19; net +34). Smoke: 5/5.
+
+## 2. Phase 1 — Context Loading
+
+Re-read:
+
+- AZ-370, AZ-373, AZ-374 task specs (`_docs/02_tasks/done/`)
+- `_docs/02_document/module-layout.md` (component boundaries — see F1)
+- `_docs/02_document/architecture.md` `## Architecture Vision` and current state
+- `_docs/02_document/architecture_compliance_baseline.md` (5 baseline findings, all `Resolved`; F5 specifically tracked `DataAccess ↔ Common` and is structurally re-visited by this window — see F1 + §8 + §9)
+- `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C17, C20, C21 entries)
+
+All three tasks sit within epic AZ-350. Common theme across the window: **strengthen typing at component seams (string → enum; nullable contract intent; named `HttpClient`) without changing observable wire formats.** The three concerns are orthogonal (DB+JSON enums, DTO field removal, DI registration) — no shared symbol, no shared file save except for `Program.cs` and `TileService.cs` where the changes are sequential and non-overlapping.
+
+## 3. Phase 2 — Spec Compliance (cumulative)
+
+| Task | ACs | Verification |
+|------|-----|--------------|
+| AZ-370 (C17) | AC-1 (compare/write sites use enums) / AC-2 (DB round-trip preserves values) / AC-3 (acceptance_criteria.md RT2 lists all 4 point types) / AC-4 (tests stay green) | 8 lowercase-write theories + 9 case-insensitive-read theories + 11 rejection/idempotency facts in `EnumStringTypeHandlerTests`; runtime markdown assertion in `AcceptanceCriteriaRT2Tests`; production sites exercised via existing `RegionServiceTests` / `RouteServiceTests` / `RoutePointGraphBuilderTests` / `RouteResponseMapperTests` updated to compare against enum members. Smoke validates the end-to-end DB round-trip and JSON wire format. Verified in batch 19. |
+| AZ-373 (C20) | AC-1 (semantics explicit — option a: no new writes) / AC-2 (HTTP shape decision recorded — field removed) / AC-3 (tests stay green) | `BuildTileEntity_DoesNotPopulateMapsVersion_AZ373_AC1` captures persisted entity; two reflection tests assert the property is absent from both DTOs. Decision and rationale recorded inside the batch report. Verified in batch 20. |
+| AZ-374 (C21) | AC-1 (single registration with User-Agent + Timeout) / AC-2 (downloader uses `CreateClient("GoogleMapsTiles")` everywhere) / AC-3 (tests stay green) | `AddTileDownloader_RegistersNamedGoogleMapsTilesHttpClient_AZ374_AC1` directly exercises the named registration end-to-end. AC-2 covered structurally (the only `CreateClient(` occurrences in `GoogleMapsDownloaderV2.cs` reference `GoogleMapsTilesClientName` — confirmed by grep, see §7) and behaviorally by smoke (session-token + single-tile + batch-tile paths all use the named client). Verified in batch 21. |
+
+No spec-gap detected on any of the three tasks.
+
+## 4. Phase 3 — Code Quality (cumulative)
+
+- **SOLID**: SRP improvements continue. Enum + type handler split (b19) places each concern in its own file (enum in Common foundation, handler in DataAccess infrastructure, registration in startup). Named HttpClient registration (b21) moves component-internal HTTP concerns (`User-Agent`, timeout, client name) **into** the `AddTileDownloader()` extension method instead of leaking them into `Program.cs` — a deliberate SRP improvement over the task spec's stated location and explained in the batch 21 design note.
+- **Error handling**: `EnumStringTypeHandler<T>.Parse` rejects null/DBNull/empty/undefined-enum-value with descriptive `DataException` messages. `RegisterAll()` uses `Interlocked.Exchange` for idempotency. No bare `catch` introduced. No empty exception handlers. `swallowTileLoadErrors`-style flags untouched in this window.
+- **Naming**: every new identifier is intent-revealing — `RegionStatus`, `RoutePointType`, `RegionStatusResponse`, `EnumStringTypeHandler<T>`, `DapperEnumTypeHandlers`, `RegisterAll`, `GoogleMapsTilesClientName`, `DefaultHttpClientTimeoutSeconds`, `UserAgent`. The DTO rename (`RegionStatus` → `RegionStatusResponse`) follows the rationale recorded in the batch 19 report (genuine response object: Id + Status + paths + counters + timestamps). C# naming convention is applied uniformly — `UserAgent` (PascalCase) replaces the prior `USER_AGENT` (SCREAMING_CASE), consistent with the rest of the codebase.
+- **Complexity**: every new method is ≤ 40 LOC. `EnumStringTypeHandler<T>.Parse` is 18 LOC; `SetValue` is 4 LOC. `RegisterAll` is 9 LOC. The new test methods in `EnumStringTypeHandlerTests` are theories with 4 inline cases each — readable and bounded.
+- **DRY**: positive net DRY across the window. The string `"completed"`/`"failed"`/`"queued"`/`"processing"` literals were scattered across 5 production sites; now they are 4 enum members + a single Dapper handler. The string `"start"`/`"end"`/`"action"`/`"intermediate"` literals were scattered across 2 production sites; now 4 enum members. The `User-Agent` string and timeout value lived inside `GoogleMapsDownloaderV2` and were applied per-call via `DefaultRequestHeaders.UserAgent.ParseAdd(...)`; now set once at registration time, used at every `CreateClient(name)` resolution. The named-client name itself is a single `internal const` referenced from both the consumer and the extension method.
+- **Test quality**: all new tests use xUnit + FluentAssertions + the `// Arrange / // Act / // Assert` comment block. Behavior-preservation evidence is direct (param `DbType` + lowercase value assertion for AC-1; `Enum.TryParse` round-trip for AC-2; reflection check for AC-2/AC-1 on DTOs; `IHttpClientFactory` resolution for AC-1 on b21). No "no error thrown"-only tests.
+- **Dead code**: nothing left dangling. The pre-rename `RegionStatus` DTO is gone (renamed file). The per-call `DefaultRequestHeaders.UserAgent.ParseAdd(...)` lines (2 sites in b21) are gone. The `MapsVersion` write in `BuildTileEntity` and the `MapsVersion = tile.MapsVersion` projection in `GetTileByLatLon` are gone. The pre-refactor string-typed `Status`/`PointType` are gone from production code (a repo-wide grep — see §7 — finds only test fixtures that intentionally probe the type handler).
+- **Static-vs-instance**: `MapToMetadata` in `TileService` remains `static` because it is a pure projection over the entity argument and reads no instance state. `EnumStringTypeHandler<T>` is non-static because it implements `SqlMapper.TypeHandler<T>`. `DapperEnumTypeHandlers.RegisterAll` is `static` and uses an `Interlocked.Exchange` flag — fits `coderule.mdc`'s "pure self-contained" guidance for static methods.
+
+No code-quality finding.
+
+## 5. Phase 4 — Security Quick-Scan (cumulative)
+
+- No new SQL building, no new string-interpolated queries, no `Process.Start` / `eval` / native invocation introduced in the window.
+- No new credentials or secrets. The `GoogleMapsTiles` client name, the `Mozilla/5.0...` User-Agent, and the 100s timeout are non-secret operational constants.
+- No new user-controlled input reaches `EnumStringTypeHandler<T>.Parse`. The only callers are Dapper marshalling code reading `string`/`null` values from PostgreSQL columns the application itself wrote — and the handler still rejects undefined values with a descriptive `DataException`, so a poisoned DB row surfaces a controlled error rather than a silent map.
+- `JsonStringEnumConverter(JsonNamingPolicy.CamelCase)` accepts case-insensitively on read and emits camelCase on write. The enum members are all single-word lower-case strings (`queued`, `processing`, `completed`, `failed`, `start`, `end`, `action`, `intermediate`) — no upper-case identifier slipping through, no whitespace, no JSON injection surface.
+- `MapsVersion` removal (b20) **reduces** the surface — one less server-side field exposed on every `/api/satellite/tiles/latlon` response. No security regression; arguably a small positive (removes a leaky implementation detail that previously suggested an internal versioning scheme to clients).
+- Named `HttpClient` registration sets headers and timeout at one place. Per-call call sites can no longer accidentally drop the User-Agent. AZ-353 sanitized 5xx envelope still applies to all new code paths — no batch in the window introduces a `Results.Problem(detail: ex.Message, ...)` regression.
+
+No security finding.
+
+## 6. Phase 5 — Performance Scan (cumulative)
+
+- **Enum typing (b19)**: Dapper enum marshalling adds `Enum.TryParse` + `Enum.IsDefined` per row read on `Status` / `PointType` columns. Net cost is a single-digit-microsecond branch per row; on the hot read paths (`GetByStatusAsync` returning a small set of pending regions; `GetByRouteIdAsync` returning a route's point list) this is below noise. The `JsonStringEnumConverter` lookup cost is amortized inside `System.Text.Json`'s converter cache (registered once at startup). No measurable regression.
+- **`RegisterAll` idempotency (b19)**: `Interlocked.Exchange` is called once at app start. No per-request cost.
+- **`MapsVersion = null` (b20)**: one fewer string allocation per persisted tile. Strictly positive.
+- **Named `HttpClient` (b21)**: `IHttpClientFactory` pools `HttpMessageHandler` instances per name. The previous per-call header parse (`DefaultRequestHeaders.UserAgent.ParseAdd(USER_AGENT)`) is replaced by a one-time configuration at registration time — saves the parse cost on every `CreateClient()` resolution. Strictly positive perf direction.
+- No new I/O, no new DB calls, no new HTTP calls introduced in the window.
+
+No performance regression. Slight positive direction on b20 (one fewer allocation) and b21 (one fewer header parse per request).
+
+## 7. Phase 6 — Cross-Task Consistency (cumulative — main focus of K=3 review)
+
+Style & pattern consistency across the three batches:
+
+- **Class shape**: every new type follows the existing conventions. `EnumStringTypeHandler<T>` is `public class : SqlMapper.TypeHandler<T> where T : struct, Enum` — a single explicit instance with no fields and two methods. `DapperEnumTypeHandlers` is a `public static class` with a `_registered` flag — matches the existing `CorsConfigurationValidator` (introduced by AZ-354 in epic AZ-350 Phase 2) and `DapperTypeHandlers`-style registrars commonly seen in .NET DI bootstrapping. The new enums are bare `public enum` with no `[Flags]` or explicit values — consistent with idiomatic .NET enum-as-DB-value pattern.
+- **Configuration injection convention**: untouched in this window. b21 uses three `internal const`s on `GoogleMapsDownloaderV2` instead of new `IOptions<T>` keys because the values are not operational tunables (User-Agent is product-internal; client name is structural; 100s default is `HttpClient`'s implicit default preserved). This is a deliberate exception, recorded in the batch 21 design note, and does not contradict the b18 (AZ-371) magic-numbers-to-config pattern, which targets *tunable* operational levers.
+- **Test convention**: every new test file uses xUnit + FluentAssertions + the `// Arrange / // Act / // Assert` comment block. The `AZ370_AC1` / `AZ373_AC2` / `AZ374_AC1` suffix convention (introduced by AZ-371 in batch 18 and carried by AZ-353/AZ-362) is applied uniformly. No drift.
+- **Error-handling convention**: every new public method that takes a reference parameter throws `DataException` (Dapper conventions) or `ArgumentException`/`ArgumentNullException` (BCL conventions) with descriptive messages. Consistent with the prior window.
+- **DI-registration convention**: per-component `*ServiceCollectionExtensions.cs` registrations. Batch 19 adds `DapperEnumTypeHandlers.RegisterAll()` to `Program.cs` (one-time startup side effect — not a service registration, so correctly placed at the entry point rather than inside an extension method). Batch 21 adds `AddHttpClient(name, ...)` inside `AddTileDownloader()`. This is consistent: ambient startup state goes into `Program.cs`; component DI goes into the extension.
+- **Symbol drift**: searched all new public/internal type names across the codebase.
+  - `RegionStatus` — single `enum` definition under `SatelliteProvider.Common.Enums`. The prior `class RegionStatus` (DTO) was renamed to `RegionStatusResponse` in the same batch. Grep confirms no remaining identifier collision.
+  - `RoutePointType` — single `enum` definition under `SatelliteProvider.Common.Enums`. No duplicate.
+  - `EnumStringTypeHandler` — single generic class under `SatelliteProvider.DataAccess.TypeHandlers`. No duplicate.
+  - `DapperEnumTypeHandlers` — single static class. No duplicate.
+  - `GoogleMapsTilesClientName` — single `internal const` on `GoogleMapsDownloaderV2`. Referenced by the same-assembly extension method and by source-level grep test discipline. The test in `SatelliteProvider.Tests` uses the literal string `"GoogleMapsTiles"` because the const is `internal` and `SatelliteProvider.Tests` has no `InternalsVisibleTo` on `SatelliteProvider.Services.TileDownloader`. This is acceptable (no other realistic option without expanding the public API surface), but the literal does need to stay in sync if the constant is renamed — recorded as an info note, not a finding.
+  - `UserAgent` (internal const) — single occurrence; renamed from `USER_AGENT` in the same batch.
+  - `MapsVersion` — single property on `TileEntity` (DataAccess; retained because the column still exists). Removed from `DownloadTileResponse`, `TileMetadata`, and the integration test wire shape. No drift between DB and DTO.
+- **Interface drift**: `IRegionService` changed (`Task<RegionStatusResponse>` instead of `Task<RegionStatus>` — the former DTO, now the renamed `RegionStatusResponse`). All callers updated in the same batch. No partial migration. `ITileService`, `IRouteService`, `IRegionRepository`, `IRouteRepository`, `ITileRepository`, `ISatelliteDownloader` — unchanged in signatures.
+- **Constructor-fan-out coordination**: batch 21 changed `GoogleMapsDownloaderV2`'s `HttpClient` ctor parameter to `IHttpClientFactory`. The only DI registration site (`AddTileDownloader()`) was updated in the same batch. The b18 (AZ-371) IOptions<T> fan-out from the previous window is untouched. No drift.
+- **Wire-format consistency**: the b19 `JsonStringEnumConverter(CamelCase)` policy emits `"queued"` / `"start"` etc. — byte-identical to the pre-refactor string literals — verified by passing the existing integration smoke tests that still parse `Status` as `string`. The b20 wire-format change (`mapsVersion` removed) is intentional, documented in the batch report, and validated by reflection tests asserting the property does not exist on `DownloadTileResponse`/`TileMetadata`.
+
+One **F1** finding emerges from this phase: see §10.
+
+## 8. Phase 7 — Architecture Compliance
+
+Checks applied to the cumulative window:
+
+1. **Layer direction**: every new file lives in the layer it should:
+   - `RegionStatus.cs` + `RoutePointType.cs` (b19) → `Common/Enums/` (Layer 1 — Foundation). Common has no upstream imports (`Imports from: (none)` per `module-layout.md` row 36). Correct.
+   - `EnumStringTypeHandler.cs` (b19) → `DataAccess/TypeHandlers/` (Layer 1 — Foundation). The handler imports `Common.Enums`, requiring a new `<ProjectReference>` from DataAccess to Common. Same-layer dependency between two Layer 1 components.
+   - `GoogleMapsDownloaderV2.cs` + `TileDownloaderServiceCollectionExtensions.cs` (b21) → `Services.TileDownloader/` (Layer 3 — Application). The named-client registration adds an internal coupling between the extension method and the downloader (`internal const` shared between them in the same assembly). Correct — does not leak across components.
+   - `Program.cs` changes (b19/b20) → `Api/` (Layer 4). Correct.
+
+2. **Same-layer dependency** (DataAccess → Common, NEW in b19): this is the structural shift of the window.
+   - **Direction**: DataAccess → Common is the **correct** direction. Common has zero upstream dependencies (verified — `Common.csproj` lists no `<ProjectReference>`). Reversing this (Common → DataAccess) would break the foundation invariant and create a cycle.
+   - **Justification**: the new enums (`RegionStatus`, `RoutePointType`) are domain concepts shared across persistence (DataAccess entities), service layer (regions/routes), and the JSON wire (DTOs). Their natural home is `Common.Enums`. DataAccess needs them for both Dapper marshalling (the type handler in `DataAccess.TypeHandlers`) and for the entity property types (`RegionEntity.Status: RegionStatus`).
+   - **Doc state**: `_docs/02_document/module-layout.md` line 54 (`DataAccess: Imports from: (none — fully self-contained, no project references)`) and line 152 (`DataAccess has no ProjectReference to Common: confirmed by inspecting SatelliteProvider.DataAccess.csproj`) and the layer table at line 143 are now stale. This is documentation drift introduced by b19. See **F1** in §10. The refactor skill's Phase 7 (Documentation) will refresh the layout; this finding is a cumulative-review tripwire to make sure it does.
+
+3. **No new cyclic dependencies**: the module DAG is acyclic.
+   - Common ← (no upstream).
+   - DataAccess → Common (NEW in b19; one-way edge, no return path).
+   - TileDownloader → Common, DataAccess. ✅
+   - RegionProcessing → Common, DataAccess. ✅
+   - RouteManagement → Common, DataAccess. ✅
+   - Api → Common, DataAccess, all three Services. ✅
+   - **No sibling references between RouteManagement / RegionProcessing / TileDownloader.** Verified by reading the three `.csproj` files.
+
+4. **Duplicate symbols across components**:
+   - `RegionStatus` — single `enum` in `Common.Enums`. The prior `class RegionStatus` (DTO) has been renamed to `RegionStatusResponse` (same file, git-tracked rename). Grep confirms a single definition site for each.
+   - `RoutePointType` — single `enum` in `Common.Enums`. No duplicate.
+   - `EnumStringTypeHandler<T>`, `DapperEnumTypeHandlers` — single definitions in `DataAccess.TypeHandlers`. No duplicates.
+   - `GoogleMapsTilesClientName`, `UserAgent`, `DefaultHttpClientTimeoutSeconds` — `internal const`s on `GoogleMapsDownloaderV2`; referenced only by the same-assembly extension method. No duplicates.
+
+5. **Cross-cutting concerns not locally re-implemented**:
+   - Enum DB marshalling → single generic `EnumStringTypeHandler<T>` in `DataAccess.TypeHandlers` (not duplicated per-enum). Future enums register one more line in `RegisterAll()`. Correctly hoisted.
+   - Enum JSON marshalling → single `JsonStringEnumConverter(JsonNamingPolicy.CamelCase)` in `ConfigureHttpJsonOptions`. Correctly centralized.
+   - Outbound HTTP configuration (User-Agent, timeout) → single named-client registration in `AddTileDownloader()`. Correctly hoisted from per-call site code into DI.
+
+6. **Public API growth**:
+   - `Common`: +2 enum types (RegionStatus, RoutePointType). Non-breaking addition.
+   - `Common.DTO.RegionStatusResponse`: renamed from `RegionStatus`. Wire-shape compatible (Id + Status + paths + counters + timestamps unchanged); the integration tests already used the new name.
+   - `Common.DTO.{DownloadTileResponse, TileMetadata}`: `MapsVersion` property removed. **Single observable JSON wire change in the window** — `mapsVersion` field no longer appears in `/api/satellite/tiles/latlon` responses. Intentional per AZ-373 AC-2.
+   - `DataAccess`: +1 generic class, +1 static class. New project reference to Common.
+   - `IRegionService`: return-type identifier renamed (`RegionStatus` DTO → `RegionStatusResponse`). The underlying wire shape is unchanged. All callers updated in the same batch.
+
+7. **DI registration coherence**:
+   - `Program.cs` line 26: `DapperEnumTypeHandlers.RegisterAll()` called once before DI builder construction — guarantees the handlers are registered before any repository ever marshals a row.
+   - `AddTileDownloader()`: now also registers a named `HttpClient` for Google Maps. The bare `builder.Services.AddHttpClient()` on `Program.cs:36` is now **structurally redundant** because the named registration also wires up `IHttpClientFactory`. See **F2** in §10. The batch 21 report explicitly acknowledges this as deferred.
+
+## 9. Baseline Delta (vs. `_docs/02_document/architecture_compliance_baseline.md`)
+
+| Bucket | Count | Notes |
+|--------|-------|-------|
+| Carried over | 0 | All 5 baseline findings (F1–F5) were resolved by epics AZ-309 + AZ-315 prior to this run. The baseline file already reflects this. |
+| Resolved | 0 | None to resolve in this window — baseline already clean. |
+| Newly introduced | 0 | This window introduces no Architecture-category Critical/High/Medium findings. Note: baseline F5 specifically tracked the `DataAccess ↔ Common` relationship and recorded "DataAccess has no ProjectReference to Common" as the resolved state. Batch 19 deliberately changes that state (DataAccess now DOES reference Common, by design, to share the enums). This is **not** a regression of F5 — F5 was a documentation bug, not a structural rule. The current state is structurally valid (Layer 1 → Layer 1, acyclic, justified). The documentation must catch up. Captured as **F1** below. |
+
+## 10. Findings
+
+| # | Severity | Category | File:Line | Title |
+|---|----------|----------|-----------|-------|
+| F1 | Low | Maintainability | `_docs/02_document/module-layout.md:54,69,143,152` | DataAccess project-reference docs stale after AZ-370 |
+| F2 | Low | Maintainability | `SatelliteProvider.Api/Program.cs:36` | Redundant `builder.Services.AddHttpClient()` after AZ-374 |
+
+### Finding Details
+
+**F1: DataAccess project-reference docs stale after AZ-370** (Low / Maintainability)
+- Location: `_docs/02_document/module-layout.md` lines 54 (`Imports from: (none — fully self-contained, no project references)`), 69 / 84 / 98 (component rows referencing the prior state), 143 (`1. Foundation | Common, DataAccess | Common: (none); DataAccess: (none)`), and 152 (`DataAccess has no ProjectReference to Common: confirmed by inspecting SatelliteProvider.DataAccess.csproj`).
+- Description: Batch 19 (AZ-370) added `<ProjectReference Include="..\SatelliteProvider.Common\SatelliteProvider.Common.csproj" />` to `SatelliteProvider.DataAccess.csproj` so the DataAccess models (`RegionEntity.Status`, `RoutePointEntity.PointType`) and the new Dapper handler can reference `Common.Enums.{RegionStatus, RoutePointType}`. The dependency direction is correct (Layer 1 → Layer 1 with no cycle; Common has no upstream), but the layout doc still describes the pre-AZ-370 state.
+- Impact: Docs disagree with code. Future readers (or a re-run of `code-review/SKILL.md` in baseline mode) will report a contradiction. Tooling that derives an import graph from the layout doc will undercount edges by one.
+- Suggestion: Update `module-layout.md` row 38 (`DataAccess`) `Imports from:` to `Common`; update row 143 (`1. Foundation`) `Allowed Dependencies` column to `Common: (none); DataAccess: Common`; remove the stale claim at line 152. This is the refactor skill's Phase 7 (Documentation) responsibility — listed here so Phase 7 does not miss it.
+- Task: AZ-370 (b19).
+
+**F2: Redundant `builder.Services.AddHttpClient()` after AZ-374** (Low / Maintainability)
+- Location: `SatelliteProvider.Api/Program.cs:36`.
+- Description: After AZ-374, `AddTileDownloader()` calls `services.AddHttpClient(GoogleMapsTilesClientName, ...)`, which transitively registers `IHttpClientFactory`. The bare `builder.Services.AddHttpClient()` call on line 36 is now structurally redundant (both registrations register the same `IHttpClientFactory` infrastructure; the named overload also configures defaults). The batch 21 report explicitly recorded this as "kept untouched; removing it is outside this task's scope".
+- Impact: One redundant line. No runtime behavior difference (the second registration is idempotent for the factory itself). Readers may assume `Program.cs` is the source of HTTP client configuration; the actual configuration lives in `AddTileDownloader()`.
+- Suggestion: Remove `builder.Services.AddHttpClient();` from `Program.cs:36` in the next adjacent-hygiene sweep. Alternative: leave it and add a one-line comment that it is intentionally defensive in case other components ever need an unnamed `HttpClient` (defer choice to the user).
+- Task: AZ-374 (b21) — out-of-scope by design.
+
+Both findings are Low. Verdict logic: only Low/Medium → **PASS_WITH_WARNINGS**. Per `/implement` Step 14.5 gate semantics, the loop continues to the next batch.
+
+## 11. Test posture at end of window
+
+- **Unit tests**: 175 / 175 passing (was 141 before batch 19; net +34 across the window — EnumStringTypeHandlerTests×28 + AcceptanceCriteriaRT2Tests×1 + TileServiceTests {3 AZ-373 + 1 AZ-374} + retyped expectations in 4 existing files).
+- **Integration smoke**: 5 / 5 scenarios passing; container exits 0. Idempotency tests (AZ-362), Migration 012 tests (AZ-357), CORS / 5xx-sanitization / 501-stub / security tests, region/route processing happy paths, tile-download-by-lat/lon — all green.
+- **No skipped tests, no flaky tests** in this window.
+
+## 12. Action
+
+Auto-chain to next batch per `/implement` Step 14. K=3 counter resets; next cumulative review fires after batch 24. Phase 4 continues with the remaining 7 tasks in `todo/`:
+
+- `AZ-372` — C19 dotnet format + analyzers + Coverlet (3 SP)
+- `AZ-375` — C22 O(N) existing-tile lookup (2 SP, needs AZ-371 ✓)
+- `AZ-376` — C23 delete unused `FindExistingTileAsync` (1 SP)
+- `AZ-377` — C24 consolidate Earth-geometry constants (2 SP, needs AZ-371 ✓)
+- `AZ-378` — C25 repo `_logger` fields (1 SP)
+- `AZ-379` — C26 repo SELECT column-list constants (2 SP)
+- `AZ-380` — C27 delete `CalculatePolygonDiagonalDistance` (1 SP)
+
+F1 + F2 are tracked for refactor Phase 7 (Documentation) — no immediate action required mid-loop.