Skip to content

Record gdalcubes read failures after open; drop items without a mask asset (#99) - #102

Merged
NewGraphEnvironment merged 7 commits into
mainfrom
99-gdalcubes-read-failures-after-open-mask
Oct 14, 2026
Merged

NewGraphEnvironment merged 7 commits into
mainfrom
99-gdalcubes-read-failures-after-open-mask

Conversation

@NewGraphEnvironment

Copy link
Copy Markdown
Owner

Summary

#87 made drift abort when gdalcubes fails to open an image. Reads that fail after the open still came back with chunk status OK. The worst is a cloud-mask (SCL) read that fails: the scene is used unmasked and cloud reaches the median. This PR does two things.

  • The cause is fixed in gdalcubes, on our fork: NewGraphEnvironment/gdalcubes#2.
    • warp_simple() ignored ChunkAndWarpImage()'s return. It now returns nullptr, and gdalcubes' existing code marks the chunk INCOMPLETE.
    • A mask that fails to open is now recorded.
    • A worker chunk file the main process can't read back is now ERROR.
    • A worker killed by a signal no longer hangs write_ncdf(). The hang was the log printer spinning on a missing log file.
    • Upstream PR drafted, not posted: planning/archive/2026-10-issue-99-read-failures-after-open/upstream-pr-draft.md.
  • drift side (Remotes: stays appelmar/gdalcubes):
    • stac_cube_items() drops STAC items that have no mask asset. gdalcubes would otherwise read them unmasked. The drop is announced, and an all-dropped listing raises c("drift_no_mask_items", "drift_no_items"). dft_stac_composite() skips that year with its own reason rather than "no scenes".
    • New fixtures delete or corrupt a band or SCL COG, corrupting the tile with the IFD intact.
    • Tests that need the fork fix are gated on a capability probe, read_failures_recorded(). On 0.7.5 they SKIP with a drift#99 reason. The probe is cross-checked, ungated, against an independent read failure, so a wrong "no" fails a test instead of skipping.
    • cube_check_chunks() abort text, the roxygen and inst/notes/gdalcubes-pc-gotchas.md now say what is recorded with and without the fix.

Correction to the issue table. In drift's read shape (apply_pixel() with no select_bands()), gdalcubes also reads SCL as a band. So a deleted SCL is already caught on 0.7.5; an ungated real-path test pins that. The case drift really missed is a corrupted SCL:

SCL drift's shape, 0.7.5 fork
deleted 9 chunks failed 9
corrupted 0 (scene unmasked) 9

Related Issues

Test plan

  • devtools::test() green on installed gdalcubes 0.7.5 (gated tests SKIP, naming drift#99)
  • cube and composite tests green on the fork build (R_LIBS=<fork build>)
  • DRIFT_TEST_ASSUME_READ_STATUS=true against 0.7.5: exactly the gated tests fail
  • Mutations go red:
    • unwiring the mask-asset drop;
    • a probe forced to FALSE;
    • select_bands() inserted in stac_cube_assemble();
    • the composite handler order swapped.
  • gdalcubes fork: test_chunk_status.R gives 10 of 32 failing on 0.7.5 and 32 of 32 passing on the fork; test_worker_failure.R times out on 0.7.5 and passes in about 1 s; full tinytest 159/159
  • lintr::lint_package() adds nothing (211 on main and branch); pkgdown::check_pkgdown() clean
  • /code-check: fork 3 rounds and drift 3 rounds, each ended by an enumeration (fork: 39-row ownership walk; drift: every new assertion and guard checked with mutations)

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN

NewGraphEnvironment and others added 7 commits October 14, 2026 05:36
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
gdalcubes uses an image whose mask band is absent UNMASKED and records the
chunk as OK, so clouds reach the median with nothing R can see.
stac_features_masked() drops such features in stac_cube_items(), before
is_pre is computed, announcing the count and ids; if none is left the
read raises drift_no_items, so a multi-year composite skips that year.

Also records the plan review, the fork fix's measurements and the
worker-hang mechanism in the PWF.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
- chunk_fixture() writes DEFLATE COGs and can delete or corrupt either the
  band or the SCL file; tif_corrupt_tile() breaks the first tile at its
  TileOffsets entry so the file opens and then fails to read.
- read_failures_recorded() asks the installed gdalcubes, at parallel 1
  and 2, whether a read after open is recorded; the tests that need it
  skip with a drift#99 reason on 0.7.5 and fail when forced on against it.
- New tests: band read failure, SCL read failure (cube and composite, in
  drift's apply_pixel read shape), and an ungated deleted-SCL test, which
  0.7.5 already records because gdalcubes reads SCL as a band there too.
- cube_check_chunks() abort text and the guard roxygen describe what is
  recorded with and without the fix; gdalcubes-pc-gotchas.md gains a #99
  section.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
- An all-dropped listing raises c("drift_no_mask_items", "drift_no_items"),
  and dft_stac_composite() skips that year as "no scene has the
  cloud-mask asset" rather than "no scenes"; the final abort points at the
  warnings instead of only suggesting more months.
- The capability probe is cross-checked, ungated and without the override,
  against an independent read failure (a corrupted SCL in drift's read
  shape), so a probe that wrongly answers FALSE fails a test instead of
  silently skipping the gated ones. The override accepts true/TRUE/1 and
  false/0 and refuses anything else.
- tif_corrupt_tile() asserts a classic TIFF with a single tile, since for
  more tiles TileOffsets points at an array.

Also records fork review rounds 2-3 in the PWF.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Code-check round 2: the pattern's trailing '.*b' matched the 'b' in
'gdalcubes' in the message's fixed text, so the id was never checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
- read_failures_recorded() no longer compares parallel 1 with 2: gdalcubes
  runs every chunk in Rscript workers at every parallel, so the comparison
  could never fire. The comment now says why the probe still answers for
  the library that does drift's reads.
- A deleted SCL is now tested through drift's real read path
  (dft_stac_composite() over the fixture), not the helper's copy of its
  shape; with select_bands() inserted in stac_cube_assemble() it fails.
- gdalcubes-pc-gotchas.md records that workers run at parallel = 1 too and
  load gdalcubes from R_LIBS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRhUJsuKABLfpBGktPoiBN
@NewGraphEnvironment
NewGraphEnvironment merged commit 97da78c into main Oct 14, 2026
1 check passed
@NewGraphEnvironment
NewGraphEnvironment deleted the 99-gdalcubes-read-failures-after-open-mask branch October 14, 2026 14:38
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