From 5d6d6c01a1c0e6c94b428aac1e4436ac4d686404 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:00:47 -0700 Subject: [PATCH 1/6] Initialize PWF baseline for #25 --- planning/active/findings.md | 114 +++++++++++++++++++++++++++++++++++ planning/active/progress.md | 8 +++ planning/active/task_plan.md | 32 ++++++++++ 3 files changed, 154 insertions(+) create mode 100644 planning/active/findings.md create mode 100644 planning/active/progress.md create mode 100644 planning/active/task_plan.md diff --git a/planning/active/findings.md b/planning/active/findings.md new file mode 100644 index 0000000..60c4c70 --- /dev/null +++ b/planning/active/findings.md @@ -0,0 +1,114 @@ +# Findings — dft_stac_fetch cache key omits AOI (#25) + +## Issue context + +**Repo:** NewGraphEnvironment/drift · **Severity:** high (silent wrong data, no error) · **Version seen:** 0.2.2 + +### Summary + +`dft_stac_fetch()` caches fetched rasters at `file.path(cache_source_dir, paste0(yr, ".nc"))` +(`R/dft_stac_fetch.R:103`) — keyed only by **source** and **year**, with **no AOI component**. Any +two calls with the same `source`/`year` but different `aoi` collide: the second call finds the +first call's NetCDF, skips the fetch (when `force = FALSE`, the default), and returns the **first +AOI's raster masked to the second AOI**. No warning, no error — just wrong data. + +### Evidence (real occurrence) + +Running two BC watershed areas through a floodplain/LULC pipeline that calls +`dft_stac_fetch(source = "io-lulc", years = c(2017, 2020, 2023))`: + +1. Area A (Neexdzii, a reach of the Bulkley) ran first → populated + `~/Library/Caches/drift/io-lulc/{2017,2020,2023}.nc` with Neexdzii's extent. Correct output. +2. Area B (MORR / Morice, ~80 km west, larger) ran second → `dft_stac_fetch` found the cache files + and returned **Neexdzii's** rasters, masked to the MORR floodplain. + +Cache extent vs. AOIs (EPSG:32609, metres): + +| | E min–max | N min–max | +|---|---|---| +| cache `io-lulc/*.nc` | 645443–696463 | 6000758–6056578 | +| **Neexdzii** fp bbox | 645444–696461 | 6000762–6056573 | ← cache == Area A | +| **MORR** fp bbox | 566715–651331 | 5948369–6035818 | ← what Area B should have gotten | + +Result: MORR's land cover was classified over only the ~3% where the Neexdzii cached extent +overlaps the MORR floodplain (near the shared Bulkley/Morice confluence); "tree loss" came out +22 ha of Bulkley-valley agricultural transitions instead of the true MORR figure. + +### Secondary bug: `force = TRUE` cannot overwrite + +`force = TRUE` routes to the fetch branch and calls `gdalcubes::write_ncdf(cube, cache_file)` +without removing the existing file first. When the cache file exists, `write_ncdf` errors: + +``` +Error: File already exists, please change the output filename or set overwrite = TRUE +``` + +So `force = TRUE` cannot be used to bypass a stale/colliding cache — the user must manually delete +the file (or call `dft_cache_clear()`). + +### Fix (from issue) + +1. **Put the AOI in the cache key.** Hash the AOI (bbox + geometry) into the filename. Preserves + caching for repeat runs of the *same* AOI while eliminating cross-AOI collisions. (Also fold + `res`, `crs`, `aggregation` into the key, since they change the output too.) +2. **Fix `force = TRUE`** to overwrite instead of erroring. +3. **Defensive check (optional):** on a cache hit, verify the cached raster's extent covers the + requested AOI bbox; if not, re-fetch. + +### Minimal repro + +```r +library(drift) +a <- sf::st_as_sf(sf::st_sfc(sf::st_buffer(sf::st_point(c(-126.75, 54.41)), 0.1), crs = 4326)) +b <- sf::st_as_sf(sf::st_sfc(sf::st_buffer(sf::st_point(c(-127.75, 54.05)), 0.1), crs = 4326)) # ~65 km west +ra <- dft_stac_fetch(a, source = "io-lulc", years = 2020) # fetches +rb <- dft_stac_fetch(b, source = "io-lulc", years = 2020) # returns a's cached raster, masked to b -> mostly NA +# terra::ext(rb[["2020"]]) matches a, not b +``` + +## Plan-mode exploration (2026-07-06) + +### Code facts + +- Only place the `.nc` filename is constructed: `R/dft_stac_fetch.R:103`. Written at :126, + read at :107/:127. No other code, test, vignette, or data-raw script assumes the pattern. +- `dft_cache_clear()` / `dft_cache_info()` (`R/dft_cache.R`) are filename-agnostic + (`list.files(recursive = TRUE)` / `unlink(recursive = TRUE)`) — unaffected by a filename change. + `dft_cache_clear(source=)` assumes only the per-source subdirectory, which is kept. +- Fetch-affecting params NOT in the current key: `aoi`, `res`, `crs`, `dt`, `aggregation`, + `resampling`, `stac_url`, `collection`, `asset`. All must enter the hash. `sign_fn` doesn't + affect pixels; `source` remains the directory. +- `rlang` and `sf` are already in Imports; `digest` is not a dependency and isn't needed — + `rlang::hash()` (XXH128) works. No hashing exists anywhere in the package yet. +- Existing tests never exercise the network fetch path (only `auto_utm_epsg` and the + missing-gdalcubes error). Cache-key helper is unit-testable fully offline via `drift:::`. + +### Design decisions (validated against installed sf 1.1.0 / rlang 1.2.0 / gdalcubes 0.7.3) + +- **Hash WKB (`sf::st_as_binary(sf::st_geometry(x), endian = "little")`), not the sfc object.** + sfc carries a PROJ-generated CRS WKT that drifts across PROJ versions → spurious cache misses. + WKB is coordinates + geometry type only; CRS enters the key separately as `target_crs`. + Also immune to sf attribute columns (verified: sf-with-attributes and bare sfc hash identically + via WKB). +- **`as.numeric(res)`** — `10L` vs `10` serialize differently under `rlang::hash()`; identical + fetches would get different keys without coercion. +- **Hash post-resolution `stac_url`/`collection`/`asset`** (after the `%||%` config resolution), + never the raw possibly-NULL args — otherwise `dft_stac_fetch(aoi)` and an explicit-but-identical + call hash differently. Bonus: also fixes a latent collision where a custom collection with + default `source = "io-lulc"` landed in the io-lulc dir keyed only by year. +- **Year stays out of the hash** — key computed once before the per-year `lapply`; filename + `_.nc` groups all years of one call under a shared readable suffix. +- **`write_ncdf(..., overwrite = TRUE)` over bare `unlink()`** — gdalcubes 0.7.3 signature is + `write_ncdf(x, fname, overwrite = FALSE, ...)`. Bare `unlink()` fails *silently* on Windows when + a prior SpatRaster holds a GDAL handle on the file, reproducing the original confusing error. +- **Extent check (issue's optional item 3) skipped** — user confirmed 2026-07-06. gdalcubes' + `cube_view` only ever *enlarges* extents to fit the pixel grid, so a containment check with + one-pixel tolerance would validate nothing; post-hash-fix, legacy `.nc` files can never + match the new pattern anyway. +- **Old-format cache files become dead weight** — correct behavior; do NOT auto-delete (can't + attribute them to an AOI). NEWS notes existing caches refetch and `dft_cache_clear()` reclaims + space. +- **POSIX silent-swap caveat** — with `force = TRUE`, a SpatRaster returned by an earlier call and + backed by the same cache file may lazily reopen and see the new content. Newly reachable (the + old behavior errored first), but benign under the hash key: the overwritten file corresponds to + the identical parameter set. Documented in `@param force` rather than engineered around. diff --git a/planning/active/progress.md b/planning/active/progress.md new file mode 100644 index 0000000..4a0fade --- /dev/null +++ b/planning/active/progress.md @@ -0,0 +1,8 @@ +# Progress — dft_stac_fetch cache key omits AOI (#25) + +## Session 2026-07-06 + +- Plan-mode exploration — phases approved by user (extent check explicitly skipped) +- Created branch `25-dft-stac-fetch-cache-key-omits-aoi-secon` off main +- Scaffolded PWF baseline from issue #25 with approved phases +- Next: start Phase 1 diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md new file mode 100644 index 0000000..c6a1264 --- /dev/null +++ b/planning/active/task_plan.md @@ -0,0 +1,32 @@ +# Task: dft_stac_fetch cache key omits AOI -> second area silently gets first area's raster (#25) + +`dft_stac_fetch()` caches fetched rasters at `file.path(cache_source_dir, paste0(yr, ".nc"))` +(`R/dft_stac_fetch.R:103`) — keyed only by **source** and **year**, with **no AOI component**. Any +two calls with the same `source`/`year` but different `aoi` collide: the second call finds the +first call's NetCDF, skips the fetch (when `force = FALSE`, the default), and returns the **first +AOI's raster masked to the second AOI**. No warning, no error — just wrong data. + +## Phase 1: Cache key includes AOI + fetch parameters + +- [ ] Add internal `stac_cache_key()` helper in `R/dft_stac_fetch.R` — WKB geometry + `as.numeric(res)` + `target_crs` + `dt` + `aggregation` + `resampling` + post-resolution `stac_url`/`collection`/`asset`, `rlang::hash()`, 12-char prefix +- [ ] Compute key once after AOI/CRS resolution; cache filename becomes `_.nc` (`R/dft_stac_fetch.R:103`) +- [ ] Unit tests in `tests/testthat/test-dft_stac_fetch.R` (all local, no network): determinism; shifted geometry → different key; different `res`/`crs`/`collection`/`asset`/`stac_url` → different keys; `res = 10` vs `10L` → same key; sf-with-attributes vs bare sfc → same key; key matches `^[0-9a-f]{12}$` + +## Phase 2: force = TRUE overwrites cleanly + +- [ ] `gdalcubes::write_ncdf(cube, cache_file, overwrite = TRUE)` (`R/dft_stac_fetch.R:126`) +- [ ] Update `@param force` roxygen — overwrites the cached file; note that a SpatRaster returned earlier and backed by the same file may silently see new content on POSIX + +## Phase 3: Docs + release + +- [ ] Roxygen note in `dft_stac_fetch` docs: cache entries keyed by AOI geometry + fetch parameters; `devtools::document()` +- [ ] NEWS.md entry for 0.2.3: bug + fix, existing caches will refetch, `dft_cache_clear()` reclaims space +- [ ] `lintr::lint_package()` clean + full `devtools::test()` pass +- [ ] Version bump to 0.2.3 in DESCRIPTION as final commit + +## Validation + +- [ ] Tests pass +- [ ] `/code-check` clean on each commit +- [ ] PWF checkboxes match landed work +- [ ] `/planning-archive` on completion From 9e2816a9e5cbcaea4cce25ef80237255928a35e5 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:06:17 -0700 Subject: [PATCH 2/6] Key STAC cache files by AOI and fetch parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache filename was .nc under a per-source directory — no AOI component. Two calls with the same source/year but different AOIs collided: the second silently got the first AOI's raster masked to its own extent. Add internal stac_cache_key(): rlang::hash() over the AOI geometry as WKB plus res (coerced to double), target CRS, dt, aggregation, resampling, and post-resolution stac_url/collection/asset. Filename becomes _.nc; year stays out of the hash so all years of one call share a readable suffix. Existing caches simply refetch; dft_cache_clear() reclaims the old files. Relates to #25 Co-Authored-By: Claude Fable 5 --- R/dft_stac_fetch.R | 29 +++++++++++++++- planning/active/progress.md | 4 ++- planning/active/task_plan.md | 6 ++-- tests/testthat/test-dft_stac_fetch.R | 52 ++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 5 deletions(-) diff --git a/R/dft_stac_fetch.R b/R/dft_stac_fetch.R index 58223ac..dfdaccf 100644 --- a/R/dft_stac_fetch.R +++ b/R/dft_stac_fetch.R @@ -97,10 +97,14 @@ dft_stac_fetch <- function(aoi, source_label <- if (!is.null(source)) source else "custom" cache_source_dir <- file.path(cache_base, source_label) dir.create(cache_source_dir, recursive = TRUE, showWarnings = FALSE) + cache_key <- stac_cache_key( + aoi_target, res, target_crs, dt, aggregation, resampling, + stac_url, collection, asset + ) # Fetch per year result <- lapply(years, function(yr) { - cache_file <- file.path(cache_source_dir, paste0(yr, ".nc")) + cache_file <- file.path(cache_source_dir, paste0(yr, "_", cache_key, ".nc")) if (!force && file.exists(cache_file)) { message(" ", yr, ": cached") @@ -136,6 +140,29 @@ dft_stac_fetch <- function(aoi, } +#' Cache key for one STAC fetch parameter set +#' +#' Hashes everything that changes the written raster except year, which stays +#' as the readable filename prefix (all years of one call share a key). The +#' geometry is hashed as WKB so sf attribute columns and PROJ-version CRS +#' representation differences can't change the key; the CRS enters separately +#' as `target_crs`. `res` is coerced to double so `10L` and `10` key alike. +#' Callers must pass post-resolution `stac_url`/`collection`/`asset`, never +#' the raw possibly-NULL arguments. +#' @noRd +stac_cache_key <- function(aoi_target, res, target_crs, dt, aggregation, + resampling, stac_url, collection, asset) { + geom_wkb <- sf::st_as_binary(sf::st_geometry(aoi_target), endian = "little") + substr( + rlang::hash(list( + geom_wkb, as.numeric(res), target_crs, dt, aggregation, + resampling, stac_url, collection, asset + )), + 1, 12 + ) +} + + #' Auto-detect UTM EPSG code from sf geometry #' @noRd auto_utm_epsg <- function(x) { diff --git a/planning/active/progress.md b/planning/active/progress.md index 4a0fade..538181b 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -5,4 +5,6 @@ - Plan-mode exploration — phases approved by user (extent check explicitly skipped) - Created branch `25-dft-stac-fetch-cache-key-omits-aoi-secon` off main - Scaffolded PWF baseline from issue #25 with approved phases -- Next: start Phase 1 +- Phase 1 complete: `stac_cache_key()` helper + `_.nc` filenames + 5 local key tests + (suite: 192 pass, 0 fail; lint clean) +- Next: Phase 2 (force overwrite) diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index c6a1264..b994081 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -8,9 +8,9 @@ AOI's raster masked to the second AOI**. No warning, no error — just wrong dat ## Phase 1: Cache key includes AOI + fetch parameters -- [ ] Add internal `stac_cache_key()` helper in `R/dft_stac_fetch.R` — WKB geometry + `as.numeric(res)` + `target_crs` + `dt` + `aggregation` + `resampling` + post-resolution `stac_url`/`collection`/`asset`, `rlang::hash()`, 12-char prefix -- [ ] Compute key once after AOI/CRS resolution; cache filename becomes `_.nc` (`R/dft_stac_fetch.R:103`) -- [ ] Unit tests in `tests/testthat/test-dft_stac_fetch.R` (all local, no network): determinism; shifted geometry → different key; different `res`/`crs`/`collection`/`asset`/`stac_url` → different keys; `res = 10` vs `10L` → same key; sf-with-attributes vs bare sfc → same key; key matches `^[0-9a-f]{12}$` +- [x] Add internal `stac_cache_key()` helper in `R/dft_stac_fetch.R` — WKB geometry + `as.numeric(res)` + `target_crs` + `dt` + `aggregation` + `resampling` + post-resolution `stac_url`/`collection`/`asset`, `rlang::hash()`, 12-char prefix +- [x] Compute key once after AOI/CRS resolution; cache filename becomes `_.nc` (`R/dft_stac_fetch.R:103`) +- [x] Unit tests in `tests/testthat/test-dft_stac_fetch.R` (all local, no network): determinism; shifted geometry → different key; different `res`/`crs`/`collection`/`asset`/`stac_url` → different keys; `res = 10` vs `10L` → same key; sf-with-attributes vs bare sfc → same key; key matches `^[0-9a-f]{12}$` ## Phase 2: force = TRUE overwrites cleanly diff --git a/tests/testthat/test-dft_stac_fetch.R b/tests/testthat/test-dft_stac_fetch.R index 75d8596..f0fbe2b 100644 --- a/tests/testthat/test-dft_stac_fetch.R +++ b/tests/testthat/test-dft_stac_fetch.R @@ -28,3 +28,55 @@ test_that("dft_stac_fetch requires gdalcubes", { ) expect_error(dft_stac_fetch(aoi), "gdalcubes") }) + +# helpers for stac_cache_key tests: a unit-square polygon (optionally shifted) +# and a key call with fixed defaults so each test varies one input at a time +square_aoi <- function(dx = 0) { + sf::st_sfc( + sf::st_polygon(list(rbind( + c(0 + dx, 0), c(1 + dx, 0), c(1 + dx, 1), c(0 + dx, 1), c(0 + dx, 0) + ))), + crs = 32609 + ) +} + +cache_key <- function(aoi = square_aoi(), res = 10, target_crs = "EPSG:32609", + dt = "P1Y", aggregation = "first", resampling = "near", + stac_url = "https://example.com/stac", + collection = "test-collection", asset = "data") { + drift:::stac_cache_key(aoi, res, target_crs, dt, aggregation, resampling, + stac_url, collection, asset) +} + +test_that("stac_cache_key is deterministic and 12-char hex", { + k1 <- cache_key(square_aoi()) + k2 <- cache_key(square_aoi()) + expect_equal(k1, k2) + expect_match(k1, "^[0-9a-f]{12}$") +}) + +test_that("stac_cache_key changes when the AOI geometry changes", { + expect_false(cache_key(square_aoi()) == cache_key(square_aoi(dx = 0.5))) +}) + +test_that("stac_cache_key changes with each fetch-affecting parameter", { + base <- cache_key() + expect_false(cache_key(res = 20) == base) + expect_false(cache_key(target_crs = "EPSG:32610") == base) + expect_false(cache_key(dt = "P2Y") == base) + expect_false(cache_key(aggregation = "median") == base) + expect_false(cache_key(resampling = "bilinear") == base) + expect_false(cache_key(stac_url = "https://other.com/stac") == base) + expect_false(cache_key(collection = "other-collection") == base) + expect_false(cache_key(asset = "other-asset") == base) +}) + +test_that("stac_cache_key treats integer and double res alike", { + expect_equal(cache_key(res = 10L), cache_key(res = 10)) +}) + +test_that("stac_cache_key ignores sf attribute columns", { + bare <- square_aoi() + with_attrs <- sf::st_sf(name = "a", area = 1.5, geometry = bare) + expect_equal(cache_key(with_attrs), cache_key(bare)) +}) From 352aec9eac477cb74ff23c985cce742c1f2a52eb Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:08:27 -0700 Subject: [PATCH 3/6] Let force = TRUE overwrite the cached NetCDF force = TRUE routed to the fetch branch but write_ncdf errored on the existing file ("File already exists"), so force could never bypass a stale cache without a manual unlink. Pass overwrite = TRUE instead; unlike a bare unlink() this still fails loudly if the file is locked (Windows). Document in @param force that a previously returned raster backed by the same file may pick up the rewritten contents. Fixes #25 Co-Authored-By: Claude Fable 5 --- R/dft_stac_fetch.R | 7 +++++-- planning/active/progress.md | 3 ++- planning/active/task_plan.md | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/R/dft_stac_fetch.R b/R/dft_stac_fetch.R index dfdaccf..1f2a1e1 100644 --- a/R/dft_stac_fetch.R +++ b/R/dft_stac_fetch.R @@ -24,7 +24,10 @@ #' for categorical data). #' @param cache_dir Character. Cache directory path. When `NULL`, uses #' [dft_cache_path()]. -#' @param force Logical. Re-fetch even if cached (default `FALSE`). +#' @param force Logical. Re-fetch even if cached, overwriting the cached file +#' (default `FALSE`). A raster returned by an earlier call with the same +#' parameters is backed by that file and may silently pick up the rewritten +#' contents. #' @param sign_fn A signing function for STAC assets. Default is #' [rstac::sign_planetary_computer()]. #' @@ -127,7 +130,7 @@ dft_stac_fetch <- function(aoi, resampling = resampling ) cube <- gdalcubes::raster_cube(col, v) - gdalcubes::write_ncdf(cube, cache_file) + gdalcubes::write_ncdf(cube, cache_file, overwrite = TRUE) r <- terra::rast(cache_file) } diff --git a/planning/active/progress.md b/planning/active/progress.md index 538181b..ca6812e 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -7,4 +7,5 @@ - Scaffolded PWF baseline from issue #25 with approved phases - Phase 1 complete: `stac_cache_key()` helper + `_.nc` filenames + 5 local key tests (suite: 192 pass, 0 fail; lint clean) -- Next: Phase 2 (force overwrite) +- Phase 2 complete: `write_ncdf(..., overwrite = TRUE)` + `@param force` doc caveat +- Next: Phase 3 (docs + release) diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index b994081..3759307 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -14,8 +14,8 @@ AOI's raster masked to the second AOI**. No warning, no error — just wrong dat ## Phase 2: force = TRUE overwrites cleanly -- [ ] `gdalcubes::write_ncdf(cube, cache_file, overwrite = TRUE)` (`R/dft_stac_fetch.R:126`) -- [ ] Update `@param force` roxygen — overwrites the cached file; note that a SpatRaster returned earlier and backed by the same file may silently see new content on POSIX +- [x] `gdalcubes::write_ncdf(cube, cache_file, overwrite = TRUE)` (`R/dft_stac_fetch.R:126`) +- [x] Update `@param force` roxygen — overwrites the cached file; note that a SpatRaster returned earlier and backed by the same file may silently see new content on POSIX ## Phase 3: Docs + release From fc7c4e253fb6bf5fba6f23a9c3ad31f923b6e0e2 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:14:49 -0700 Subject: [PATCH 4/6] Document cache keying and note migration in NEWS Relates to #25 Co-Authored-By: Claude Fable 5 --- NEWS.md | 5 +++++ R/dft_stac_fetch.R | 7 +++++++ man/dft_stac_fetch.Rd | 13 ++++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 8cc09ec..28edc82 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# drift 0.2.3 + +- Fix silent cross-AOI cache collision in `dft_stac_fetch()` (#25). Cache files were keyed by source + year only, so fetching a second AOI with the same source/year silently returned the first AOI's raster masked to the second AOI's extent. Cache filenames now include a hash of the AOI geometry and all fetch-affecting parameters (`res`, `crs`, `dt`, `aggregation`, `resampling`, `stac_url`, `collection`, `asset`). Existing caches re-fetch on first use after upgrading; `dft_cache_clear()` reclaims the orphaned old-format files. +- `force = TRUE` now overwrites the cached file instead of erroring with "File already exists" (#25). + # drift 0.2.2 - Startup quote pool expanded to 113. Adds 52 domain-expert quotes from 11 voices across floodplain/river process (David Montgomery, Ellen Wohl), Indigenous stewardship (Robin Wall Kimmerer, Kyle Whyte, Nancy Turner, Jeannette Armstrong), ecosystem valuation (Kai Chan), Canadian public voices (David Suzuki, Wade Davis), and legacy conservation (Aldo Leopold, Wendell Berry). diff --git a/R/dft_stac_fetch.R b/R/dft_stac_fetch.R index 1f2a1e1..e72e202 100644 --- a/R/dft_stac_fetch.R +++ b/R/dft_stac_fetch.R @@ -5,6 +5,13 @@ #' collection hosting single-band classified rasters (IO LULC, ESA WorldCover, #' custom COGs). #' +#' Fetched rasters are cached under [dft_cache_path()] as +#' `/_.nc`, where `key` is a hash of the AOI geometry and +#' every fetch parameter that affects the output (`res`, `crs`, `dt`, +#' `aggregation`, `resampling`, `stac_url`, `collection`, `asset`). Repeat +#' calls with the same AOI and parameters reuse the cache; changing any of +#' them re-fetches. +#' #' @param aoi An `sf` polygon defining the area of interest. #' @param source Character. A known source name passed to [dft_stac_config()]. #' Ignored when `stac_url`, `collection`, and `asset` are all provided. diff --git a/man/dft_stac_fetch.Rd b/man/dft_stac_fetch.Rd index dee7af7..372f9b7 100644 --- a/man/dft_stac_fetch.Rd +++ b/man/dft_stac_fetch.Rd @@ -53,7 +53,10 @@ for categorical data).} \item{cache_dir}{Character. Cache directory path. When \code{NULL}, uses \code{\link[=dft_cache_path]{dft_cache_path()}}.} -\item{force}{Logical. Re-fetch even if cached (default \code{FALSE}).} +\item{force}{Logical. Re-fetch even if cached, overwriting the cached file +(default \code{FALSE}). A raster returned by an earlier call with the same +parameters is backed by that file and may silently pick up the rewritten +contents.} \item{sign_fn}{A signing function for STAC assets. Default is \code{\link[rstac:items_sign_planetary_computer]{rstac::sign_planetary_computer()}}.} @@ -69,3 +72,11 @@ per-year rasters cropped and masked to the AOI. Works with any STAC collection hosting single-band classified rasters (IO LULC, ESA WorldCover, custom COGs). } +\details{ +Fetched rasters are cached under \code{\link[=dft_cache_path]{dft_cache_path()}} as +\verb{/_.nc}, where \code{key} is a hash of the AOI geometry and +every fetch parameter that affects the output (\code{res}, \code{crs}, \code{dt}, +\code{aggregation}, \code{resampling}, \code{stac_url}, \code{collection}, \code{asset}). Repeat +calls with the same AOI and parameters reuse the cache; changing any of +them re-fetches. +} From b09c0fbbed29423381e39cf85759e4bf31ae2ca8 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:14:49 -0700 Subject: [PATCH 5/6] Release v0.2.3 Co-Authored-By: Claude Fable 5 --- DESCRIPTION | 4 ++-- planning/active/progress.md | 6 +++++- planning/active/task_plan.md | 14 +++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7c1251a..1afb974 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: drift Title: Detecting Riparian and Inland Floodplain Transitions -Version: 0.2.2 -Date: 2026-04-14 +Version: 0.2.3 +Date: 2026-07-06 Authors@R: c( person("Allan", "Irvine", , "al@newgraphenvironment.com", role = c("aut", "cre"), comment = c(ORCID = "0000-0002-3495-2128")), diff --git a/planning/active/progress.md b/planning/active/progress.md index ca6812e..0e84092 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -8,4 +8,8 @@ - Phase 1 complete: `stac_cache_key()` helper + `_.nc` filenames + 5 local key tests (suite: 192 pass, 0 fail; lint clean) - Phase 2 complete: `write_ncdf(..., overwrite = TRUE)` + `@param force` doc caveat -- Next: Phase 3 (docs + release) +- Phase 3 complete: cache-keying note in roxygen, NEWS 0.2.3, version bump +- E2E verified against live Planetary Computer STAC: two AOIs ~64 km apart produced two + distinct cache files (`2020_622235623d95.nc`, `2020_d631fe72d838.nc`), each raster matched + its own AOI extent, re-run hit the cache, `force = TRUE` overwrote without error +- Next: /planning-archive + /gh-pr-push diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 3759307..5dd1412 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -19,14 +19,14 @@ AOI's raster masked to the second AOI**. No warning, no error — just wrong dat ## Phase 3: Docs + release -- [ ] Roxygen note in `dft_stac_fetch` docs: cache entries keyed by AOI geometry + fetch parameters; `devtools::document()` -- [ ] NEWS.md entry for 0.2.3: bug + fix, existing caches will refetch, `dft_cache_clear()` reclaims space -- [ ] `lintr::lint_package()` clean + full `devtools::test()` pass -- [ ] Version bump to 0.2.3 in DESCRIPTION as final commit +- [x] Roxygen note in `dft_stac_fetch` docs: cache entries keyed by AOI geometry + fetch parameters; `devtools::document()` +- [x] NEWS.md entry for 0.2.3: bug + fix, existing caches will refetch, `dft_cache_clear()` reclaims space +- [x] `lintr::lint_package()` clean + full `devtools::test()` pass (one pre-existing vignette lint, untouched by this branch) +- [x] Version bump to 0.2.3 in DESCRIPTION as final commit ## Validation -- [ ] Tests pass -- [ ] `/code-check` clean on each commit -- [ ] PWF checkboxes match landed work +- [x] Tests pass +- [x] `/code-check` clean on each commit +- [x] PWF checkboxes match landed work - [ ] `/planning-archive` on completion From b9e8d80ad279750ad02837ba38b30dfa74c2ba90 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 6 Jul 2026 17:15:42 -0700 Subject: [PATCH 6/6] Archive planning files for issue #25 --- .../2026-07-issue-25-cache-key-aoi/README.md | 21 +++++++++++++++++++ .../findings.md | 0 .../progress.md | 0 .../task_plan.md | 2 +- 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 planning/archive/2026-07-issue-25-cache-key-aoi/README.md rename planning/{active => archive/2026-07-issue-25-cache-key-aoi}/findings.md (100%) rename planning/{active => archive/2026-07-issue-25-cache-key-aoi}/progress.md (100%) rename planning/{active => archive/2026-07-issue-25-cache-key-aoi}/task_plan.md (98%) diff --git a/planning/archive/2026-07-issue-25-cache-key-aoi/README.md b/planning/archive/2026-07-issue-25-cache-key-aoi/README.md new file mode 100644 index 0000000..9d7e691 --- /dev/null +++ b/planning/archive/2026-07-issue-25-cache-key-aoi/README.md @@ -0,0 +1,21 @@ +# Issue #25 — dft_stac_fetch cache key omits AOI + +## Outcome + +Fixed a silent wrong-data bug: `dft_stac_fetch()` cached NetCDFs as `/.nc`, so a +second AOI with the same source/year silently received the first AOI's raster masked to its own +extent (real occurrence: MORR got Neexdzii's rasters). Added internal `stac_cache_key()` — +`rlang::hash()` over the AOI geometry as WKB plus every fetch-affecting parameter (`res` coerced +to double, target CRS, `dt`, `aggregation`, `resampling`, post-resolution +`stac_url`/`collection`/`asset`) — giving filenames `_.nc`; also made `force = TRUE` +overwrite via `write_ncdf(..., overwrite = TRUE)` instead of erroring. Key learnings: hash sf +geometry as WKB, not the sfc object (PROJ-version CRS WKT drift causes spurious misses, and sf +attribute columns would leak into the key); coerce numerics before hashing (`10L` vs `10` hash +differently under `rlang::hash()`); hash post-default-resolution values, never possibly-NULL +args; and skip extent-containment checks on cache hits — gdalcubes only ever enlarges extents, +so the check validates nothing. Verified end-to-end against live Planetary Computer STAC (two +AOIs → two distinct cache files, correct extents, cache hit, force overwrite). Released as +v0.2.3. + +Closed by: commits 9e2816a / 352aec9 / fc7c4e2 / b09c0fb, PR pending (branch +`25-dft-stac-fetch-cache-key-omits-aoi-secon`) diff --git a/planning/active/findings.md b/planning/archive/2026-07-issue-25-cache-key-aoi/findings.md similarity index 100% rename from planning/active/findings.md rename to planning/archive/2026-07-issue-25-cache-key-aoi/findings.md diff --git a/planning/active/progress.md b/planning/archive/2026-07-issue-25-cache-key-aoi/progress.md similarity index 100% rename from planning/active/progress.md rename to planning/archive/2026-07-issue-25-cache-key-aoi/progress.md diff --git a/planning/active/task_plan.md b/planning/archive/2026-07-issue-25-cache-key-aoi/task_plan.md similarity index 98% rename from planning/active/task_plan.md rename to planning/archive/2026-07-issue-25-cache-key-aoi/task_plan.md index 5dd1412..0b83eea 100644 --- a/planning/active/task_plan.md +++ b/planning/archive/2026-07-issue-25-cache-key-aoi/task_plan.md @@ -29,4 +29,4 @@ AOI's raster masked to the second AOI**. No warning, no error — just wrong dat - [x] Tests pass - [x] `/code-check` clean on each commit - [x] PWF checkboxes match landed work -- [ ] `/planning-archive` on completion +- [x] `/planning-archive` on completion