Summary
Robustness/doc-polish suggestions from the Deep pre-push review of #92/#93 (PR#94,
Round 2 — approved, these were the non-blocking suggestions). Captured here so they
aren't lost.
Items
-
Silent spill loss. restoreSpilledSamples is silent on a missing/corrupt spill
while persistBackscatterTile plain-overwrites the tile — so an out-of-band spill
deletion (e.g. a /tmp reaper on a multi-day import) would silently drop the
pre-eviction intensity. Detect a missing/short spill on reload and WARN (or fail
loudly) rather than silently continuing with only post-reload beams.
-
Wrong atomicity comment. A comment claims tile_io::saveTile does an atomic
temp-then-rename; verified false — marine_tiled_raster_store/src/tile_io.cpp:132
writes directly to the final path via GDAL Create (no temp+rename). Fix the
comment. (The non-atomicity itself is the pre-existing uma#189 concern; low-impact
for a re-runnable offline pass, but the comment is actively inaccurate.)
-
"Lossless" claim scope. The bolded "backscatter is lossless under eviction"
reads unconditionally; the real guarantee holds for a consistent re-survey — a
depth-disambiguation divergence between visits could diverge. Scope the wording.
-
Registry write timing. Registries are written only at finalize; a mid-pass
crash leaves evicted tiles with a source_index but no registry.json. Likely
acceptable (single re-runnable pass) but undocumented — note it at the eviction
site, or periodically flush the registry.
-
Targeted single-tile reload. reloadEvictedTile's loadWindow(sw,ne) over the
tile's own corners relies on loadWindow corner-inclusivity to reload exactly that
tile (correct per source). A targeted single-tile reload routine would be clearer
and less coupled to loadWindow's inclusivity semantics.
Follow-up to #92 / #93 (PR#94).
Authored-By: Claude Code Agent
Model: Claude Opus 4.8 (1M context)
Summary
Robustness/doc-polish suggestions from the Deep pre-push review of #92/#93 (PR#94,
Round 2 — approved, these were the non-blocking suggestions). Captured here so they
aren't lost.
Items
Silent spill loss.
restoreSpilledSamplesis silent on a missing/corrupt spillwhile
persistBackscatterTileplain-overwrites the tile — so an out-of-band spilldeletion (e.g. a
/tmpreaper on a multi-day import) would silently drop thepre-eviction intensity. Detect a missing/short spill on reload and WARN (or fail
loudly) rather than silently continuing with only post-reload beams.
Wrong atomicity comment. A comment claims
tile_io::saveTiledoes an atomictemp-then-rename; verified false —
marine_tiled_raster_store/src/tile_io.cpp:132writes directly to the final path via GDAL
Create(no temp+rename). Fix thecomment. (The non-atomicity itself is the pre-existing uma#189 concern; low-impact
for a re-runnable offline pass, but the comment is actively inaccurate.)
"Lossless" claim scope. The bolded "backscatter is lossless under eviction"
reads unconditionally; the real guarantee holds for a consistent re-survey — a
depth-disambiguation divergence between visits could diverge. Scope the wording.
Registry write timing. Registries are written only at
finalize; a mid-passcrash leaves evicted tiles with a
source_indexbut noregistry.json. Likelyacceptable (single re-runnable pass) but undocumented — note it at the eviction
site, or periodically flush the registry.
Targeted single-tile reload.
reloadEvictedTile'sloadWindow(sw,ne)over thetile's own corners relies on
loadWindowcorner-inclusivity to reload exactly thattile (correct per source). A targeted single-tile reload routine would be clearer
and less coupled to
loadWindow's inclusivity semantics.Follow-up to #92 / #93 (PR#94).
Authored-By:
Claude Code AgentModel:
Claude Opus 4.8 (1M context)