Skip to content

feat: per-task tile merge report (added/merged/replaced counts + added-tile list) - #266

Open
shimoncohen wants to merge 18 commits into
masterfrom
feat/tile-merge-report
Open

shimoncohen wants to merge 18 commits into
masterfrom
feat/tile-merge-report

Conversation

@shimoncohen

Copy link
Copy Markdown
Collaborator

Summary

  • Adds a per-task merge report: counts of added / merged / replaced / skipped tiles, plus the exact z/x/y list of added tiles.
  • Emitted two ways: a structured log line (counts + percentages, no list) and a JSON artifact written to FS or S3 (by REPORT:sink config) at the path from the job's AdditionalParams.ReportOutputPath (absent → log line only).
  • Classification: added = tile didn't exist in target before merge; merged = existed + target blended with source; replaced = existed + opaque source covered it; skipped = no tile produced / no source data.

How it works

  • TileMerger.MergeTiles gains an out MergeStats overload (TargetUsed, AnySourceUsed); old signature kept (CLI/tests untouched).
  • TaskExecutor computes existedBefore via an exact-coord TileExists (defensive Coord copy — Data.TileExists mutates its arg) and classifies each tile into a MergeReport, then finalizes → logs → ReportWriter.WriteReport (best-effort).
  • ReportWriter resolves the S3 client lazily so FS-only deployments without AWS creds still boot.

Spec / plan

  • Spec: docs/superpowers/specs/2026-09-15-tile-merge-report-design.md
  • Plan: docs/superpowers/plans/2026-09-15-tile-merge-report.md

Follow-up

  • MAPCO-11688 — expose these counts as Prometheus metrics + dashboard (incl. percentage-of-total) and audit/reorganize existing metrics/dashboards.

Test Plan

  • dotnet build GpkgMerger.sln — 0 errors
  • dotnet test --filter TestCategory=unit — 1155 pass, 0 fail (+12 new: MergeStats, MergeReport, ReportWriter FS/S3/no-op, TaskExecutor added/merged/replaced/skipped)
  • Manual: run a merge with ReportOutputPath set (FS and S3) and confirm artifact contents + log line

🤖 Generated with Claude Code

shimoncohen and others added 15 commits September 15, 2026 14:58
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tile list

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…askExecutor

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@shimoncohen

Copy link
Copy Markdown
Collaborator Author

Open question — report-write failure handling

Currently ReportWriter failure is best-effort: TaskExecutor logs the error and the task still succeeds (TaskExecutor.cs, try/catch around WriteReport). Should a failure to write the report artifact instead fail/reject the task?

Best-effort is the proposed default, but if the report becomes a downstream dependency, silent loss may be wrong. Decide before merge.

@shimoncohen shimoncohen self-assigned this Sep 15, 2026
shimoncohen and others added 2 commits September 15, 2026 17:43
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the added/merged/replaced/skipped decision out of TaskExecutor into
MergeReport.RecordOutcome; make the Record* counters private. Classification
permutations now unit-tested directly on MergeReport; TaskExecutorTest keeps one
end-to-end wiring test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant