geotiff: pin open_geotiff(max_cloud_bytes=...) dispatcher silent drop (#1974)

[brendancol]

Summary

Test-coverage sweep pass 16 on the geotiff module. Closes a Cat 4 HIGH parameter-coverage gap on the open_geotiff dispatcher's max_cloud_bytes kwarg.

max_cloud_bytes was added in #1928 as an eager fsspec cloud-budget guard and re-ordered into the canonical reader signature by #1957. open_geotiff accepts the kwarg in its signature but only forwards it to _read_to_array on the eager non-VRT branch (xrspatial/geotiff/__init__.py:431). The GPU branch at line 410, the dask branch at line 422, and the VRT branch at line 362 never reference the kwarg, so:

open_geotiff(p, max_cloud_bytes=8, gpu=True)   # silently drops
open_geotiff(p, max_cloud_bytes=8, chunks=4)   # silently drops
open_geotiff(vrt, max_cloud_bytes=8)           # silently drops

Same class of dispatcher-silently-drops-backend-kwarg bug fixed by #1561, #1605, #1685, and #1810 for other backend-only kwargs. The two sibling kwargs on_gpu_failure (line 339) and missing_sources (line 355) already raise ValueError when used on a path where they do not apply.

Filed issue #1974 for the dispatcher fix. This sweep is test-only.

Tests

xrspatial/geotiff/tests/test_max_cloud_bytes_dispatcher_silent_drop_2026_05_15.py (11 tests, all behaving as designed locally):

• 4 xfail(strict=True) pinning the fix surface (gpu, dask, vrt, dask+gpu). Flip to xpass (= test failure under strict=True) when the dispatcher fix lands.
• 3 passing pins on the current silent-drop behaviour so the fix is visible as a diff.
• 4 positive pins that the eager local + file-like paths accept the kwarg (the docstring no-op contract).

Test plan

• All 11 tests pass / xfail as designed on the local host (pytest xrspatial/geotiff/tests/test_max_cloud_bytes_dispatcher_silent_drop_2026_05_15.py -v -> 7 passed, 4 xfailed).
• CI to confirm parity (the dask+gpu xfail skips when cupy / CUDA is unavailable).

[brendancol]

PR Review

What looks good

• The xfail-strict + counter-pin pattern is the right shape for a test-only sweep PR. The four xfail(strict=True) cases pin the fix target (they flip from xpass → fail when the dispatcher gains the ValueError), and the TestCurrentSilentDropPins siblings make the current buggy behaviour visible as a test diff at fix time. Reads cleanly and matches the precedent set by geotiff: backend-specific entry points silently drop kwargs of their dispatchers #1561 / geotiff: open_geotiff(gpu=True) silently drops window= and band= kwargs #1605 / geotiff: open_geotiff silently drops overview_level when routing to VRT reader #1685 / geotiff: open_geotiff silently drops missing_sources when routing to read_vrt #1810.
• The eager positive pins (TestEagerLocalPathAcceptsMaxCloudBytes, TestEagerFileLikeAcceptsMaxCloudBytes) lock in the documented no-op contract for local and file-like sources separately from the dispatch-rejection contract. Useful regression net regardless of how geotiff: open_geotiff silently drops max_cloud_bytes on gpu / chunks / VRT paths #1974 is resolved.
• Module docstring explicitly enumerates the two acceptable fix surfaces (ValueError guard vs. thread the budget through every backend), which keeps the test's "after" expectations precise without over-prescribing.
• Tests run as designed locally: 7 passed, 4 xfailed.

Suggestions

• The cupy availability check is duplicated verbatim in test_dispatcher_dask_gpu_path_rejects_max_cloud_bytes (lines 209–217) and TestCurrentSilentDropPins.test_gpu_path_silently_accepts_today (lines 241–249). Worth a shared module-level fixture (or pytest.importorskip plus a cupy.cuda.is_available() skip) — twelve lines of boilerplate becomes one.
• pytest.raises(ValueError, match=r"max_cloud_bytes") requires the fix author's error message to mention max_cloud_bytes literally. The sibling on_gpu_failure / missing_sources errors do follow that convention, so this is consistent — but worth flagging in geotiff: open_geotiff silently drops max_cloud_bytes on gpu / chunks / VRT paths #1974 so the fix author doesn't ship a message like "this kwarg only applies on the eager path" and trip the xpass.
• _build_local_tif writes via the eager path, which after rebase routes through the writer's CRS validator (geotiff: reject bool and unresolvable EPSG in writer crs= (#1971) #1978). attrs={'crs': 4326} is fine, but if a future commit makes that attrs entry stricter (e.g. requires crs_wkt instead), this fixture quietly stops producing test inputs. Low risk, but the helper depends on a chunk of the write API that has been churning.

Nits

• TestCurrentSilentDropPins will need wholesale removal when geotiff: open_geotiff silently drops max_cloud_bytes on gpu / chunks / VRT paths #1974 lands; the docstring already says so. Worth a one-line FIXME or # remove with #1974 comment at class level so a future cleanup author can grep for it.
• test_local_file_max_cloud_bytes_small_is_noop passes max_cloud_bytes=8. The intent (8 bytes — far below the file size — proves local files skip the budget) reads from context but is not stated. A one-line comment like # 8 bytes is far below the file size; local files skip the budget would help a reader who lands here without geotiff: eager fsspec cloud read pulls full object before dim guard #1928 context.

Checklist

• Algorithm matches reference/paper — not applicable (test-coverage sweep)
• All implemented backends covered — the four backend dispatch arms each have a xfail pin
• NaN handling — not applicable
• Edge cases covered — local vs. file-like vs. dask vs. gpu vs. vrt vs. dask+gpu
• Dask chunk boundaries — not applicable
• No premature materialisation in test fixtures
• No benchmark required (test-only PR)
• Docstrings present and accurate — module and class docstrings carry the fix-visibility contract