Batch 23 of refactor 03-code-quality-refactoring (4 tasks, 5 SP):
- AZ-376 (C23): Delete unused FindExistingTileAsync from
ITileRepository / TileRepository. No callers; method also took the
obsolete `version` arg removed by C06/AZ-357.
- AZ-378 (C25): Repository _logger discipline.
TileRepository.GetTilesByRegionAsync now emits LogWarning when the
query exceeds SlowQueryThresholdMs (500 ms). RegionRepository and
RouteRepository drop the unused ILogger<TRepo> field, parameter, and
using; Program.cs DI registrations updated.
- AZ-379 (C26): Extract `private const string ColumnList` per repo
(TileRepository, RegionRepository, RouteRepository); SELECTs use
$@"SELECT {ColumnList} FROM ..." (C# 10+ const interpolation).
INSERT/UPDATE/DELETE unchanged; route_points single-site SELECT left
inline.
- AZ-380 (C27): Delete dead alias GeoUtils.CalculatePolygonDiagonalDistance.
Tests: +9 new (RepositoryRefactorTests x8, GeoUtilsRefactorTests x1)
covering each AC via reflection / file-content assertions; pattern
mirrors ToolingConfigurationTests (b22) and AcceptanceCriteriaRT2Tests
(b19). Unit suite 181 -> 190, all green. dotnet format clean.
Code review: PASS_WITH_WARNINGS (3 Low findings, all informational or
out-of-scope for this batch). See
_docs/03_implementation/reviews/batch_23_review.md.
Cumulative review counter 2/3; next K=3 review fires after batch 24.
Co-authored-by: Cursor <cursoragent@cursor.com>
7.0 KiB
Batch Report
Batch: 23
Tasks: AZ-376 (C23 — delete FindExistingTileAsync), AZ-378 (C25 — repo _logger fields cleanup), AZ-379 (C26 — repo ColumnList constants), AZ-380 (C27 — delete CalculatePolygonDiagonalDistance)
Date: 2026-05-11
Run: 03-code-quality-refactoring
Cycle: 1
Task Results
| Task | Status | Files Modified | Tests | AC Coverage | Issues |
|---|---|---|---|---|---|
| AZ-376_refactor_delete_findexistingtile | Done | 2 (ITileRepository.cs, TileRepository.cs) |
1 new + build | 3/3 | None |
| AZ-378_refactor_repo_logger_fields | Done | 4 (TileRepository.cs, RegionRepository.cs, RouteRepository.cs, Program.cs) |
3 new | 3/3 | None |
| AZ-379_refactor_repo_select_columnlist | Done | 3 (TileRepository.cs, RegionRepository.cs, RouteRepository.cs) |
4 new | 3/3 | None blocking |
| AZ-380_refactor_delete_polygon_diagonal | Done | 1 (GeoUtils.cs) |
1 new | 3/3 | None |
Total: 6 source files modified + 2 new test files (RepositoryRefactorTests.cs, GeoUtilsRefactorTests.cs) + 9 new test cases.
Changes
AZ-376 — delete unused FindExistingTileAsync
SatelliteProvider.DataAccess/Repositories/ITileRepository.cs: removed declaration on line 9 (no callers per grep before deletion).SatelliteProvider.DataAccess/Repositories/TileRepository.cs: removed implementation (lines 51-76).- Removed dependency on the obsolete
versionargument that C06/AZ-357 was retiring.
AZ-378 — repo _logger fields cleanup (recommended split)
TileRepository.cs:- Added
private const int SlowQueryThresholdMs = 500(named const, easy to tune later). GetTilesByRegionAsyncwraps the Dapper call inStopwatch.StartNew()/Stop()and emits_logger.LogWarning(...)if elapsed > threshold. Includes structured fields (ElapsedMs,ThresholdMs,Latitude,Longitude,SizeMeters,ZoomLevel).- Added
using System.Diagnostics.
- Added
RegionRepository.cs: removed_loggerfield, removedILogger<RegionRepository>constructor parameter, removedusing Microsoft.Extensions.Logging.RouteRepository.cs: same pattern.SatelliteProvider.Api/Program.cs:33-34: DI registrations forIRegionRepositoryandIRouteRepositorynow construct without theILogger<TRepo>arg.
AZ-379 — extract repo ColumnList constants
TileRepository.cs:private const string ColumnList = @"id, tile_zoom as TileZoom, ..."interpolated intoGetByIdAsync,GetByTileCoordinatesAsync,GetTilesByRegionAsyncviaconst string sql = $@"SELECT {ColumnList} FROM ...". INSERT/UPDATE/DELETE statements unchanged (out of scope per spec).RegionRepository.cs: same pattern; constant coversGetByIdAsync+GetByStatusAsync.RouteRepository.cs: same pattern; constant covers the tworoutes-table SELECTs (GetByIdAsync+GetRoutesWithPendingMapsAsync).GetRoutePointsAsync(singleroute_pointsSELECT) left inline — a second const would not reduce duplication.- C# 10+ const-interpolated strings supported by net8.0 target; compile-time guarantee that the column list is identical across SELECTs.
AZ-380 — delete dead alias CalculatePolygonDiagonalDistance
SatelliteProvider.Common/Utils/GeoUtils.cs:129-132: removed (no callers per grep).
Tests added
SatelliteProvider.Tests/RepositoryRefactorTests.cs (8 tests):
TileRepository_DoesNotExposeFindExistingTileAsync_AZ376_AC1(reflection)TileRepository_KeepsAndUsesLogger_AZ378_AC1(file content)RegionRepository_HasNoUnusedLoggerParameter_AZ378_AC2(reflection)RouteRepository_HasNoUnusedLoggerParameter_AZ378_AC2(reflection)TileRepository_DefinesColumnListConstantOnce_AZ379_AC1(file content)RegionRepository_DefinesColumnListConstantOnce_AZ379_AC1RouteRepository_DefinesColumnListConstantOnce_AZ379_AC1RepositoryColumnLists_ContainExpectedColumns_AZ379_AC2(per-repo column-token assertions)
SatelliteProvider.Tests/GeoUtilsRefactorTests.cs (1 test):
GeoUtils_DoesNotExposeCalculatePolygonDiagonalDistance_AZ380_AC1(reflection)
Same file-content / reflection assertion pattern established by ToolingConfigurationTests (AZ-372 b22) and AcceptanceCriteriaRT2Tests (AZ-370 b19).
AC Test Coverage
| AC | Covered by |
|---|---|
| AZ-376 AC-1 (method gone) | TileRepository_DoesNotExposeFindExistingTileAsync_AZ376_AC1 |
| AZ-376 AC-2 (build) | Successful Docker dotnet test Release build |
| AZ-376 AC-3 (tests green) | 190/190 unit run |
| AZ-378 AC-1 (kept logger used) | TileRepository_KeepsAndUsesLogger_AZ378_AC1 |
| AZ-378 AC-2 (unused loggers removed) | *_HasNoUnusedLoggerParameter_AZ378_AC2 (×2) |
| AZ-378 AC-3 (tests green) | 190/190 unit run |
| AZ-379 AC-1 (one ColumnList per repo) | *_DefinesColumnListConstantOnce_AZ379_AC1 (×3) |
| AZ-379 AC-2 (SQL byte-identical) | RepositoryColumnLists_ContainExpectedColumns_AZ379_AC2 (column tokens) + structural guarantee from compile-time interpolation; smoke run handed off to test-run skill |
| AZ-379 AC-3 (tests green) | 190/190 unit run |
| AZ-380 AC-1 (method gone) | GeoUtils_DoesNotExposeCalculatePolygonDiagonalDistance_AZ380_AC1 |
| AZ-380 AC-2 (build) | Successful Docker build |
| AZ-380 AC-3 (tests green) | 190/190 unit run |
Stale-count note carried over from b22: each task spec phrases AC-3 as "37 unit + 5 smoke". Pre-/document figure; current count is 190 unit. Spirit ("all tests green") satisfied.
Test Run
| Suite | Result | Count |
|---|---|---|
Unit (SatelliteProvider.Tests) |
All passed | 190 (was 181; +9 new tests across 2 new files) |
dotnet format whitespace --verify-no-changes |
Clean | — |
| Smoke integration (Docker) | Handed off to test-run skill | — |
Code Review Verdict: PASS_WITH_WARNINGS
Three Low findings (_docs/03_implementation/reviews/batch_23_review.md):
- F1 (Low / Maintainability):
route_pointscolumn list left inline (single use). Spec is explicit ("one ColumnList per repository"). No action. - F2 (Low / Spec-Gap): Earth-related literals still in
GetTilesByRegionAsync— explicitly the target of AZ-377 (batch 24). - F3 (Low / Spec-Gap): Stale "37 unit + 5 smoke" count in task ACs — same as b22 F1, defer to refactor Phase 7 doc sweep.
Auto-Fix Attempts: 0
Stuck Agents: None
Cumulative review counter
This is batch 2 since the last cumulative review (cumulative_review_batches_19-21_cycle1_report.md). Counter at 2/3; next cumulative review fires after batch 24 (covers batches 22-24).
Next Batch
Phase 4 continues with the remaining 2 tasks in todo/ after this batch:
AZ-375— C22 O(N) existing-tile lookup HashSet (2 SP, dep AZ-371 ✓)AZ-377— C24 consolidate Earth-geometry constants + 111000 + TileSizePixels (2 SP, dep AZ-371 ✓)
Batch 24 candidate: both tasks together (~4 SP, both touch TileDownloader / Common / DataAccess). They form the last refactor batch; the K=3 cumulative review fires immediately after. Then Phase 4 of refactor wraps up and we move to Phase 5 (Test Sync), Phase 6 (Verification), Phase 7 (Documentation), then FINAL_report.md and Step 9 (New Task).