Files
Oleksandr Bezdieniezhnykh 10d31b4c1c [AZ-367] Refactor C14: extract shared TileGridStitcher
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 02:55:25 +03:00

14 KiB
Raw Permalink Blame History

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 new it (matches the TileCsvWriter precedent from batch 13).
    • StitchAsync(IEnumerable<TilePlacement> tiles, int tileSizePixels, bool deduplicateByTileCoords, bool swallowTileLoadErrors, CancellationToken) — returns a StitchResult exposing 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) — converts Rectangle(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.ThrowIfNull on tiles; ArgumentOutOfRangeException for non-positive tileSizePixels; InvalidOperationException for 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;).
  • 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.
  • MODIFIED SatelliteProvider.Services.RegionProcessing/RegionService.cs

    • StitchTilesAsync now delegates the grid placement to new TileGridStitcher().StitchAsync(...) with deduplicateByTileCoords: false, swallowTileLoadErrors: false (preserves the original "throw on corrupt tile" behavior — region path historically did not try/catch around Image.LoadAsync).
    • The center-cross overlay (1-pixel thin, asymmetric [-5, 5) arms) stays inline at the call site — it is not the same shape as DrawCross and not duplicated anywhere else, so factoring it would be over-extraction.
    • Missing-tile log message preserved verbatim ("Tile file not found: {FilePath}"), now driven from result.MissingTiles.
    • using SixLabors.ImageSharp.Processing; import dropped — no more .Mutate(...) calls remain in this file.
  • MODIFIED SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs

    • StitchRouteTilesAsync now delegates the grid placement to new TileGridStitcher().StitchAsync(...) with deduplicateByTileCoords: 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 / Height are 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 DrawRectangleBorder and DrawCross (54 LOC) deleted — no remaining callers.
    • Explicit BackgroundColor(Color.Black) removed — new Image<Rgb24>(w, h) already initializes to default(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.Min with 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.MissingTiles and the MissingTileReason discriminant. _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; asserts MissingTiles carries Reason = FileNotFound.
    • StitchAsync_PropagatesLoadErrors_WhenFlagFalse_AZ367 — corrupt PNG (text bytes); asserts the exception propagates when swallowTileLoadErrors: false.
    • StitchAsync_EmptyTiles_ThrowsInvalidOperation_AZ367 — empty IEnumerable<TilePlacement> throws.
    • StitchAsync_NonPositiveTileSize_ThrowsArgumentOutOfRange_AZ367tileSizePixels: 0 throws.
    • DrawCross_PaintsHorizontalAndVerticalArms_AtCenter_AZ367_AC1 — sanity check on cross arm endpoints.
    • DrawCross_ProducesIdenticalOutputToInlinedPreRefactorCross_AZ367_AC2 — 200×200 image; runs both the new DrawCross and a verbatim inlined copy of the route stitcher's pre-refactor DrawCross. 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 new DrawRectangleBorder and 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 new StitchAsync and 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.csunchanged. 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 TileGridStitcher tests; 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_AC2 PASSED — 40,000 pixel comparisons all matched.
    • DrawRectangleBorder_ProducesIdenticalOutputToInlinedPreRefactorBorder_AZ367_AC2 PASSED — 10,000 pixel comparisons all matched.
    • StitchAsync_OutputMatchesInlinedPreRefactorRegionStitch_AZ367_AC2 PASSED — 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) via GeoUtils.WorldToTilePos.
    • tileSizePixels still read from tiles.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}".
  • Route stitcher:
    • Tile coords still derived from ExtractTileCoordinatesFromFilename.
    • tileSizePixels = 256 literal preserved at the call site.
    • Dedup by (TileX, TileY) and sort by (TileY, TileX) preserved (passed through to StitchAsync).
    • 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 to new Image<Rgb24>(w, h) by StitchAsync_OutputMatchesInlinedPreRefactorRegionStitch_AZ367_AC2.

Architecture / SRP impact

  • SatelliteProvider.Common gains an Imaging/ 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.cs shrinks from 385 → 380 lines (~5 LOC net; the gain is qualitative — the duplicated grid loop is gone).
  • RouteProcessingService.cs shrinks from 727 → ~620 lines (~107 LOC net — 54 LOC of DrawCross + DrawRectangleBorder deleted plus ~50 LOC of the placement loop replaced by an 8-line stitcher call).
  • The next refactor (AZ-364, C11 — decompose RouteProcessingService god-class) will have one fewer responsibility to extract because the stitching collaborator already exists.
  • Module layout doc gap: _docs/02_document/module-layout.md Common Public API list does not yet enumerate Imaging/. 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 (MissingTile records), 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, callers new it) and RegionFailureClassifier (small focused collaborator with explicit semantics). Style matches the rest of the run (ArgumentNullException.ThrowIfNull, Arrange / Act / Assert test layout).
  • Architecture — Common gains an ImageSharp NuGet but no new project references. Layering DAG preserved. The four new public types live under SatelliteProvider.Common.Imaging namespace, 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 1315 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 RouteProcessingService god-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-refactoring is 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.