Co-authored-by: Cursor <cursoragent@cursor.com>
14 KiB
Batch 16 Report — Refactor 03 Phase 3 (continued)
Date: 2026-05-11 Epic: AZ-350 (03-code-quality-refactoring) Status: ✅ Complete
Scope (1 task / 3 SP)
| ID | C-ID | Title | Points | Component |
|---|---|---|---|---|
| AZ-367 | C14 | Shared TileGridStitcher for region + route image generation |
3 | Common + Services.RegionProcessing + Services.RouteManagement |
Solo batch. Pure structural deduplication: the grid-placement loop and overlay primitives
that previously lived in RegionService.StitchTilesAsync and RouteProcessingService.StitchRouteTilesAsync
move to SatelliteProvider.Common.Imaging.TileGridStitcher. Behavior preserved end-to-end
(verified by byte-for-byte pixel comparison tests + smoke + integration).
Changes
Production
-
NEW
SatelliteProvider.Common/Imaging/TileGridStitcher.cs- Sealed instance class. No DI registration — callers
newit (matches theTileCsvWriterprecedent from batch 13). StitchAsync(IEnumerable<TilePlacement> tiles, int tileSizePixels, bool deduplicateByTileCoords, bool swallowTileLoadErrors, CancellationToken)— returns aStitchResultexposing the image and the min/max tile bounds (callers need the bounds for their own overlay calculations).DrawCross(Image<Rgb24> image, Point center, Rgb24 color, int armLength, int thickness = 10)— symmetric[-armLength, armLength]×[-halfThickness, halfThickness]cross. Default thickness 10 matches the route stitcher's pre-refactor literal.DrawRectangleBorder(Image<Rgb24> image, Rectangle rect, Rgb24 color, int thickness = 5)— convertsRectangle(x, y, w, h)to inclusive corners(x1, y1, x2, y2)internally so the call-site math reads naturally. Default thickness 5 matches the route stitcher's pre-refactor literal.- Companion records / enum in the same file:
TilePlacement(int TileX, int TileY, string FilePath),StitchResult(...),MissingTile(...),MissingTileReason { FileNotFound, LoadFailed }. - Defensive guards:
ArgumentNullException.ThrowIfNullontiles;ArgumentOutOfRangeExceptionfor non-positivetileSizePixels;InvalidOperationExceptionfor empty tile sequence. - Exception safety: if a tile load throws outside swallow mode, the partially-stitched image is disposed before the exception propagates (try / catch /
stitchedImage.Dispose(); throw;).
- Sealed instance class. No DI registration — callers
-
MODIFIED
SatelliteProvider.Common/SatelliteProvider.Common.csproj- Added
<PackageReference Include="SixLabors.ImageSharp" Version="3.1.11" />. - Three service projects already pinned this version; the Common csproj inherits the same pin.
- Common is Layer 1 (Foundation) in
module-layout.md. Adding the NuGet does not introduce a new project reference — the layering DAG is preserved.
- Added
-
MODIFIED
SatelliteProvider.Services.RegionProcessing/RegionService.csStitchTilesAsyncnow delegates the grid placement tonew TileGridStitcher().StitchAsync(...)withdeduplicateByTileCoords: false, swallowTileLoadErrors: false(preserves the original "throw on corrupt tile" behavior — region path historically did not try/catch aroundImage.LoadAsync).- The center-cross overlay (1-pixel thin, asymmetric
[-5, 5)arms) stays inline at the call site — it is not the same shape asDrawCrossand not duplicated anywhere else, so factoring it would be over-extraction. - Missing-tile log message preserved verbatim (
"Tile file not found: {FilePath}"), now driven fromresult.MissingTiles. using SixLabors.ImageSharp.Processing;import dropped — no more.Mutate(...)calls remain in this file.
-
MODIFIED
SatelliteProvider.Services.RouteManagement/RouteProcessingService.csStitchRouteTilesAsyncnow delegates the grid placement tonew TileGridStitcher().StitchAsync(...)withdeduplicateByTileCoords: true, swallowTileLoadErrors: true(preserves the original dedup-by-(TileX, TileY)+ sort-by-(TileY, TileX)+ per-tile try/catch behavior).- Geofence rectangle overlay loop now calls
stitcher.DrawRectangleBorder(image, new Rectangle(x1, y1, x2 - x1 + 1, y2 - y1 + 1), yellow)—Rectangle.Width/Heightare derived from the previous inclusive corner pair so the internal math (x2 = X + Width - 1) round-trips to the same values. - Route point cross overlay now calls
stitcher.DrawCross(image, new Point(pixelX, pixelY), red, 50)— default thickness 10 matches the original literal. - The two private static helpers
DrawRectangleBorderandDrawCross(54 LOC) deleted — no remaining callers. - Explicit
BackgroundColor(Color.Black)removed —new Image<Rgb24>(w, h)already initializes todefault(Rgb24) = (0,0,0) = black, so the previous explicit fill was a functional no-op (verified by the byte-for-byte stitch test). - Empty-after-filter guard added: if filename parsing produces zero usable placements, log a warning and return. The previous code would have thrown on
tileCoords.Minwith an empty sequence; the new path fails softly with the same end-user outcome (no stitched image). - Missing-tile log messages preserved verbatim, now driven from
result.MissingTilesand theMissingTileReasondiscriminant._logger.LogWarning(missing.Error, "Failed to load tile at {FilePath}, leaving black", ...)is restored for the load-failure case so the exception remains in the log line. using SixLabors.ImageSharp.Processing;import dropped — no more.Mutate(...)calls remain in this file.
Tests
- NEW
SatelliteProvider.Tests/TileGridStitcherTests.cs— 11 tests:StitchAsync_PlacesEachTileAtCorrectGridOffset_AZ367_AC1— synthetic 2×2 grid; asserts each tile lands at(TileX - minX, TileY - minY) * size.StitchAsync_DeduplicatesAndSortsWhenRequested_AZ367— three placements with one duplicate cell; asserts dedup keeps the first and sort is(TileY, TileX).StitchAsync_SwallowsLoadErrors_WhenFlagSet_AZ367— one missing file; assertsMissingTilescarriesReason = FileNotFound.StitchAsync_PropagatesLoadErrors_WhenFlagFalse_AZ367— corrupt PNG (text bytes); asserts the exception propagates whenswallowTileLoadErrors: false.StitchAsync_EmptyTiles_ThrowsInvalidOperation_AZ367— emptyIEnumerable<TilePlacement>throws.StitchAsync_NonPositiveTileSize_ThrowsArgumentOutOfRange_AZ367—tileSizePixels: 0throws.DrawCross_PaintsHorizontalAndVerticalArms_AtCenter_AZ367_AC1— sanity check on cross arm endpoints.DrawCross_ProducesIdenticalOutputToInlinedPreRefactorCross_AZ367_AC2— 200×200 image; runs both the newDrawCrossand a verbatim inlined copy of the route stitcher's pre-refactorDrawCross. Asserts every pixel is identical (40,000 pixel comparisons).DrawRectangleBorder_DrawsTopBottomLeftRightWalls_AZ367_AC1— corner pixels yellow, interior pixel untouched.DrawRectangleBorder_ProducesIdenticalOutputToInlinedPreRefactorBorder_AZ367_AC2— 100×100 image; runs both the newDrawRectangleBorderand a verbatim inlined copy of the pre-refactor border. Asserts every pixel is identical (10,000 pixel comparisons).StitchAsync_OutputMatchesInlinedPreRefactorRegionStitch_AZ367_AC2— three solid-color synthetic tiles; runs the newStitchAsyncand a verbatim inlined copy of the region stitcher's pre-refactor grid loop. Asserts every pixel is identical across the resulting 2×2 grid.
The three pixel-equality tests are the explicit guard for AC-2 (pixel-for-pixel identical output). They are seeded with the literal byte patterns of the pre-refactor implementations so any drift in DrawCross, DrawRectangleBorder, or StitchAsync will fail them.
SatelliteProvider.Tests/RouteProcessingServiceTests.cs and RegionServiceTests.cs — unchanged. The four existing ExtractTileCoordinatesFromFilename_* tests still pass; they exercise the helper that feeds TilePlacement.TileX/TileY for the route side, and that helper is untouched. The five RegionServiceTests integration scenarios (ProcessRegionAsync_*) drive end-to-end region processing including stitching and continue to pass.
Verification
- Unit tests: 123 / 123 passing (was 112 — +11 new
TileGridStitchertests; no existing test removed or modified). - Integration suite (full): container exited 0. Verified scenarios include:
- Region processing happy path (stitched image generated)
- Route processing happy path (stitched route image generated with geofence rectangles and route point crosses; 690 tiles ZIP)
- Idempotency tests (AZ-362) — still green
- Migration 012 tests (AZ-357) — still green
- CORS / 5xx-sanitization / 501 stubs / security tests — still green
- Pixel-equality evidence (unit-level, AC-2):
DrawCross_ProducesIdenticalOutputToInlinedPreRefactorCross_AZ367_AC2PASSED — 40,000 pixel comparisons all matched.DrawRectangleBorder_ProducesIdenticalOutputToInlinedPreRefactorBorder_AZ367_AC2PASSED — 10,000 pixel comparisons all matched.StitchAsync_OutputMatchesInlinedPreRefactorRegionStitch_AZ367_AC2PASSED — entire 16×16 stitched image matched the inlined pre-refactor output byte-for-byte.
Acceptance criteria coverage
| AC | Evidence |
|---|---|
AC-1 Single stitcher used by both consumers (grep for per-tile placement loop confined to TileGridStitcher) |
Both call sites refactored. The placement loop foreach (var tile in ...) { var destX = (tile.TileX - minX) * tileSizePixels; ... DrawImage } exists only in SatelliteProvider.Common/Imaging/TileGridStitcher.cs. Verified by reading the new RegionService.StitchTilesAsync and RouteProcessingService.StitchRouteTilesAsync. |
| AC-2 Output images pixel-for-pixel identical for existing test scenarios | Three byte-equality unit tests above + the integration suite (which generates real region/route stitched images and exits 0). |
| AC-3 37 unit + 5 smoke tests stay green | 123/123 unit (was 112, +11 new) + full integration suite green. |
Behavior preservation notes
- Region stitcher:
- Tile coords still derived from each tile's
(Latitude, Longitude)viaGeoUtils.WorldToTilePos. tileSizePixelsstill read fromtiles.First().TileSizePixels.- No dedup, no per-tile try/catch (matches pre-refactor — corrupt tile will still propagate).
- The center-cross overlay logic (1-pixel thin, asymmetric
[-5, 5)) is kept inline at the call site, byte-equivalent to before. - Missing-file log message unchanged:
"Tile file not found: {FilePath}".
- Tile coords still derived from each tile's
- Route stitcher:
- Tile coords still derived from
ExtractTileCoordinatesFromFilename. tileSizePixels = 256literal preserved at the call site.- Dedup by
(TileX, TileY)and sort by(TileY, TileX)preserved (passed through toStitchAsync). - Per-tile try/catch behavior preserved (
swallowTileLoadErrors: true). - Missing-file vs. load-failure log messages preserved verbatim, dispatched off
MissingTileReason. - Geofence rectangle offsets
(geoMinY - minY + 1),(geoMaxY - minY + 1),(geoMaxX - minX + 2)preserved. - Route point cross arm length 50, default thickness 10 preserved.
BackgroundColor(Color.Black)removed — proven equivalent tonew Image<Rgb24>(w, h)byStitchAsync_OutputMatchesInlinedPreRefactorRegionStitch_AZ367_AC2.
- Tile coords still derived from
Architecture / SRP impact
SatelliteProvider.Commongains anImaging/subfolder. Public API surface grows by 4 types (TileGridStitcher,TilePlacement,StitchResult,MissingTile) + 1 enum (MissingTileReason). All five are internal collaborators of the two stitcher call sites — no other component imports them.RegionService.csshrinks from 385 → 380 lines (~5 LOC net; the gain is qualitative — the duplicated grid loop is gone).RouteProcessingService.csshrinks from 727 → ~620 lines (~107 LOC net — 54 LOC ofDrawCross+DrawRectangleBorderdeleted plus ~50 LOC of the placement loop replaced by an 8-line stitcher call).- The next refactor (
AZ-364, C11 — decomposeRouteProcessingServicegod-class) will have one fewer responsibility to extract because the stitching collaborator already exists. - Module layout doc gap:
_docs/02_document/module-layout.mdCommon Public API list does not yet enumerateImaging/. Will be folded into refactor Phase 7 (documentation sync) per scope discipline (coderule.mdc).
Per-batch code review (inline)
Standalone /code-review invocation skipped — solo batch, extracted-from-existing logic, behavior preservation directly attested by three byte-equality unit tests plus integration suite.
Reduced 7-phase review:
- Spec compliance — AC-1 / AC-2 / AC-3 all satisfied (table above).
- Code quality — sealed instance class; ArgumentNullException / ArgumentOutOfRangeException / InvalidOperationException guards; exception-safe image disposal; no bare catches; companion records keep the API surface clear.
- Security — no new attack surface; no path traversal opportunity introduced; missing tiles surfaced as data (
MissingTilerecords), not via exceptions. - Performance — same O(N) complexity. One additional
ToList()materialization on the input enumerable (needed for the Count check + Min/Max). Bounded by route size (≤690 tiles in the integration smoke). - Cross-task consistency — pattern matches
TileCsvWriter(instance class, no DI, callersnewit) andRegionFailureClassifier(small focused collaborator with explicit semantics). Style matches the rest of the run (ArgumentNullException.ThrowIfNull,Arrange / Act / Asserttest layout). - Architecture — Common gains an ImageSharp NuGet but no new project references. Layering DAG preserved. The four new public types live under
SatelliteProvider.Common.Imagingnamespace, isolated from the rest of Common.
Verdict: PASS. No findings.
Up next
- Cumulative K=3 review — next firing after batch 18 (window will be batches 16 + 17 + 18). Current window batches 13–15 was already covered by
cumulative_review_batches_13-15_cycle1_report.md(PASS) from batch 15's wrap-up. - Batch 17 candidate: AZ-364 (C11 Decompose
RouteProcessingServicegod-class, 5 SP, folds AZ-360). Its dependencies — AZ-366 (Haversine consolidation, batch 12) and AZ-367 (this batch) — are both now Done. The "folds AZ-360 (C08 IServiceProvider → IRegionService)" sub-task lands inside the same decompose so a separate AZ-360 batch is not needed. - After AZ-364, Phase 3 of
03-code-quality-refactoringis complete. Phase 4 (typing / config / tooling / polish — AZ-371, AZ-370, AZ-373, AZ-374, AZ-375, AZ-376, AZ-378, AZ-379, AZ-380, AZ-372) begins.