From 7c7a74bcaa70b725599e2ee81e4583832e88d64a Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:04:27 -0700
Subject: [PATCH 1/6] Initialize PWF baseline for #35

---
 planning/active/findings.md  | 124 +++++++++++++++++++++++++++++++++++
 planning/active/progress.md  |  11 ++++
 planning/active/task_plan.md |  78 ++++++++++++++++++++++
 3 files changed, 213 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..a33ff4d
--- /dev/null
+++ b/planning/active/findings.md
@@ -0,0 +1,124 @@
+# Findings — fly_footprint() drops footprint_basis and friends on tibble input (#35)
+
+## Reproduced on `main` at `8585fd5`, sf 1.1.2
+
+Against `inst/testdata/photo_centroids.gpkg` with the class overridden:
+
+```
+sf,data.frame             -> footprint_basis footprint_terrain height_agl dem_coverage
+sf,tbl_df,tbl,data.frame  -> (all four absent)
+```
+
+## Mechanism — an `sf::st_sf()` branch, not a `fly` quirk
+
+`st_sf()` builds its attribute frame with a chain that keeps only the **first**
+argument when that argument is a tibble:
+
+```r
+df = if (inherits(x, c("tbl_df", "tbl"))) x
+     else if (length(x) == 1) data.frame(row.names = row.names)
+     else if (!sfc_last && inherits(x, "data.frame")) x
+     else if (sfc_last && inherits(x, "data.frame")) x[-all_sfc_columns]
+     else if (inherits(x[[1]], c("tbl_df", "tbl"))) x[[1]]   # <-- here
+     else cbind(data.frame(row.names = row.names), as.data.frame(x[-all_sfc_columns], ...))
+```
+
+With six arguments, `x` is a plain list of length 6, so the first four branches
+are not taken. `x[[1]]` is the dropped-geometry tibble, so `df` becomes that
+tibble alone — `footprint_basis`, `footprint_terrain`, `height_agl` and
+`dem_coverage` are discarded before the sfc column is reattached. A plain
+data.frame first argument falls through to the final `cbind()`, which keeps
+everything. That is the whole difference.
+
+Confirmed in isolation:
+
+```
+st_sf(tbl, extra = "hello", geometry = g)  ->  "extra" present: FALSE
+st_sf(df,  extra = "hello", geometry = g)  ->  "extra" present: TRUE
+```
+
+## Scope — one call site
+
+`grep -rn "st_sf(" R/` returns two hits. `R/fly_footprint.R:64` builds a bare
+geometry frame (`st_sf(geometry = rects[ok])`) and attaches nothing to
+user-supplied data. `R/fly_footprint.R:482` is the defect. Every other frame in
+the package is a `dplyr::tibble()` built from scratch — `fly_fetch.R:84,93,104`,
+`fly_overlap.R:38,71,82`, `fly_georef.R:142`, `fly_coverage.R:66` — so none
+inherit the input's class and none are affected.
+
+Nothing downstream reads the four columns, so this is pure loss of reporting
+rather than breakage: geometry and every downstream number are correct with a
+`tbl_df` input today. That is what makes it dangerous rather than obvious.
+
+## Why the suite cannot see it
+
+Every fixture in the package is `sf, data.frame`:
+
+- `inst/testdata/photo_centroids.gpkg` reads back plain via `sf::st_read()`
+- `mixed_media_fixture()` and `terrain_fixture()` in `tests/testthat/setup.R`
+  are built by `sf::st_sf()` on plain vectors
+
+No number of added cases along the existing axis would find this. Seventh
+instance of the pattern in `inst/notes/terrain-correction.md` and the `CLAUDE.md`
+gotcha — *ask which shapes the fixture can never present*.
+
+Two adjacent axes were checked and are **already covered**, so class shape is the
+single gap:
+
+- **geometry column name** — the bundled fixture's geometry column is named
+  `geom`, not `geometry`, so the `st_drop_geometry()` path is exercised
+- **row subsets** — existing tests pass `centroids[centroids$scale == ..., ]`
+
+## Fix shapes measured
+
+| shape | four columns | output class for a `tbl_df` input |
+|---|---|---|
+| `st_sf(as.data.frame(drop_geom(x)), basis =, ..., geometry =)` | present | `sf, data.frame` — **downgraded** |
+| assign onto `attrs`, then `st_sf(attrs, geometry =)` | present | `sf, tbl_df, tbl, data.frame` — preserved |
+
+The second was verified across three input shapes, in each case with `geometry`
+last and `attr(, "sf_column")` correct:
+
+```
+sf,data.frame                      -> cols ok, class preserved
+sf,tbl_df,tbl,data.frame           -> cols ok, class preserved
+sf,grouped_df,tbl_df,tbl,data.frame-> cols ok, class preserved
+```
+
+It works because a two-element `x` still reaches the `inherits(x[[1]], "tbl_df")`
+branch — but by then the four columns are *inside* `x[[1]]`, so surviving as
+`x[[1]]` is exactly what is wanted. Chosen over the issue's suggested
+`as.data.frame()` because it is the smaller delta: a tibble caller's class is
+unchanged from today, only the missing columns appear.
+
+## `sf::st_read(path, as_tibble = TRUE)` gives an honest tibble fixture
+
+No class hacking is needed to build the regression fixture:
+
+```
+sf::st_read(p, quiet = TRUE)                    -> sf,data.frame
+sf::st_read(p, quiet = TRUE, as_tibble = TRUE)  -> sf,tbl_df,tbl,data.frame
+dplyr::group_by(tbl, scale)                     -> sf,grouped_df,tbl_df,tbl,data.frame
+```
+
+Per the negative-fixture rule in `CLAUDE.md`, the helper asserts that premise
+inline, so a future `sf` change fails on the premise rather than on the
+behaviour under test.
+
+## Column vectors are already full length
+
+`basis`, `terrain`, `height_agl` and `dem_coverage` are each length `nrow()`
+(`rep()` / `ifelse()` over length-`n` inputs, `R/fly_footprint.R:353-382`), so
+assigning them with `$<-` introduces no recycling that `st_sf()` was not already
+doing.
+
+## Baseline
+
+`devtools::test()` on `8585fd5`: **FAIL 0 | WARN 0 | SKIP 0 | PASS 225**.
+
+CI on `main` at session start: all recent runs green (pkgdown + pages).
+
+## Errors Encountered
+
+| Error | Resolution |
+|-------|------------|
diff --git a/planning/active/progress.md b/planning/active/progress.md
new file mode 100644
index 0000000..c22553a
--- /dev/null
+++ b/planning/active/progress.md
@@ -0,0 +1,11 @@
+# Progress — fly_footprint() drops footprint_basis and friends on tibble input (#35)
+
+## Session 2026-08-29
+
+- Plan-mode exploration — reproduced the defect, located the mechanism in
+  `sf::st_sf()`, confirmed one call site, established the 225-test baseline
+- User approved: preserve-input-class fix, class-shape sweep, v0.5.1 with #31
+  folded in
+- Created branch `35-fly-footprint-drops-footprint-basis-and` off main
+- Scaffolded PWF baseline from issue #35 with approved phases
+- Next: Phase 1 — regression tests that fail on unmodified source
diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md
new file mode 100644
index 0000000..71e4814
--- /dev/null
+++ b/planning/active/task_plan.md
@@ -0,0 +1,78 @@
+# Task: fly_footprint() drops footprint_basis and friends when input is a tibble (as bcdata returns) (#35)
+
+## Problem
+
+`fly_footprint()` silently drops `footprint_basis`, `footprint_terrain`,
+`height_agl` and `dem_coverage` when its input carries the `tbl_df` class.
+
+`bcdata::collect()` returns exactly that class, so every caller querying
+`WHSE_IMAGERY_AND_BASE_MAPS.AIMG_PHOTO_CENTROIDS_SP` — the documented source for
+this package — loses the whole reporting surface 0.4.0 and 0.5.0 added. The
+warning still fires, so the count is visible in the console and nowhere else.
+
+Mechanism: `sf::st_sf()` keeps only its **first** argument when that argument is
+a tibble, discarding every trailing named column
+(`else if (inherits(x[[1]], c("tbl_df", "tbl"))) x[[1]]`). One call site,
+`R/fly_footprint.R:482`. Baseline before any change: FAIL 0 | PASS 225.
+
+## Phase 1: Regression tests first (must fail on unmodified source)
+
+- [ ] Add `centroid_shapes()` to `tests/testthat/setup.R` — plain / tibble /
+      grouped shapes of the bundled fixture, via `sf::st_read(as_tibble = TRUE)`
+      rather than class hacking, with the premise asserted inline
+- [ ] Class-shape sweep in `test-fly_footprint.R`: all four columns present in
+      every shape; `names()` identical across shapes; the four columns' **values**
+      identical across shapes; output class equals input class with `sf` present
+- [ ] Repeat the sweep with `dem = testdata_path("dem.tif")` behind
+      `skip_if_no_terra()` — all four columns come from the same `st_sf()` call
+- [ ] Downstream pass-through: a tibble-backed footprint still flows through
+      `fly_coverage()` / `fly_overlap()` / `fly_filter()` / `fly_select()`
+- [ ] **Confirm the sweep fails on unmodified `R/`** before touching the source
+
+## Phase 2: The fix
+
+- [ ] `R/fly_footprint.R:482` — assign the four columns onto the attribute frame,
+      then one-arg `sf::st_sf(attrs, geometry = ...)`. Preserves the caller's
+      class; chosen over `as.data.frame()`, which additionally downgrades a
+      bcdata caller's tibble
+- [ ] Comment names the `st_sf()` branch and #35, so the next reader does not
+      "simplify" it back
+- [ ] Restore the bug and confirm the sweep goes red — patching **both**
+      `asNamespace("fly")` and `as.environment("package:fly")`, from
+      `git show 8585fd5:R/fly_footprint.R` rather than from memory
+- [ ] Full suite green, PASS above the 225 baseline
+
+## Phase 3: Document the contract
+
+- [ ] `@return` in `R/fly_footprint.R` states the input's class is preserved
+- [ ] `devtools::document()` — read its output; an unexpected `Writing '<x>.Rd'`
+      or a falling `grep -c "^export(" NAMESPACE` means a roxygen block rebound
+- [ ] `lintr::lint_package()` compared against the `HEAD` baseline per file
+
+## Phase 4: DESCRIPTION Title (#31, folded in)
+
+- [ ] Widen `Title:` and `Description:` to cover fetch and georeferencing
+- [ ] `devtools::document()`; `devtools::check()` for the title-case rules
+
+## Phase 5: Release v0.5.1
+
+- [ ] `NEWS.md` — name the class, the data source it breaks, and that geometry
+      and downstream numbers were always correct (reporting loss only)
+- [ ] Bump `DESCRIPTION` to `0.5.1` as the **final** commit of the branch
+- [ ] Tag `v0.5.1`
+
+## Validation
+
+- [ ] Tests pass
+- [ ] `/code-check` clean on each commit
+- [ ] End-to-end against a real `bcdata::collect()` result — the case the
+      fixture structurally cannot reach
+- [ ] PWF checkboxes match landed work
+- [ ] `/planning-archive` on completion
+- [ ] Post-merge, confirm the pkgdown deploy commit is the new `HEAD`
+
+## Out of scope — file as follow-ups
+
+- The `st_sf()` trailing-column behaviour as a `conventions/code-check.md` entry
+  in `soul`, plus a grep across the other NGE packages. General R trap, belongs
+  in `soul`, not on this branch.

From a5660743b66ec1efb2dbd09c2521b361731be71b Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:07:58 -0700
Subject: [PATCH 2/6] Sweep every input class shape, not more cases along the
 one we have
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fly_footprint() loses footprint_basis, footprint_terrain, height_agl and
dem_coverage whenever its input carries tbl_df — which is what
bcdata::collect() returns, so the documented data source is exactly the
caller that cannot see them.

Every fixture in this package is plain `sf, data.frame`: the bundled
gpkg reads back that way, and both synthesized fixtures are built by
st_sf() on plain vectors. So no case added along the existing axis could
have found this. centroid_shapes() sweeps the axis instead, reading the
tibble honestly via st_read(as_tibble = TRUE) rather than overwriting
class(), and asserting that premise inline so a future sf change fails
by naming the real cause.

These fail on unmodified source. Refs #35

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR
---
 planning/active/progress.md         | 11 ++++
 planning/active/task_plan.md        | 10 ++--
 tests/testthat/setup.R              | 32 +++++++++++
 tests/testthat/test-fly_footprint.R | 89 +++++++++++++++++++++++++++++
 4 files changed, 137 insertions(+), 5 deletions(-)

diff --git a/planning/active/progress.md b/planning/active/progress.md
index c22553a..7f69735 100644
--- a/planning/active/progress.md
+++ b/planning/active/progress.md
@@ -9,3 +9,14 @@
 - Created branch `35-fly-footprint-drops-footprint-basis-and` off main
 - Scaffolded PWF baseline from issue #35 with approved phases
 - Next: Phase 1 — regression tests that fail on unmodified source
+
+### Phase 1 — regression tests (complete)
+
+- `centroid_shapes()` added to `tests/testthat/setup.R`: plain / `as_tibble = TRUE`
+  / `group_by()` shapes of the bundled fixture, premise asserted inline
+- Four tests added to `test-fly_footprint.R` — nominal sweep, `dem` sweep,
+  class contract, downstream pass-through
+- **Confirmed red on unmodified source**: the `tbl` and `grouped` shapes return
+  `NULL` for all four columns where the `plain` shape returns the vectors
+- The class-contract test passes on the broken code by design and is commented
+  as such — it guards against the coercing fix, not against #35
diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md
index 71e4814..e0dd942 100644
--- a/planning/active/task_plan.md
+++ b/planning/active/task_plan.md
@@ -17,17 +17,17 @@ a tibble, discarding every trailing named column
 
 ## Phase 1: Regression tests first (must fail on unmodified source)
 
-- [ ] Add `centroid_shapes()` to `tests/testthat/setup.R` — plain / tibble /
+- [x] Add `centroid_shapes()` to `tests/testthat/setup.R` — plain / tibble /
       grouped shapes of the bundled fixture, via `sf::st_read(as_tibble = TRUE)`
       rather than class hacking, with the premise asserted inline
-- [ ] Class-shape sweep in `test-fly_footprint.R`: all four columns present in
+- [x] Class-shape sweep in `test-fly_footprint.R`: all four columns present in
       every shape; `names()` identical across shapes; the four columns' **values**
       identical across shapes; output class equals input class with `sf` present
-- [ ] Repeat the sweep with `dem = testdata_path("dem.tif")` behind
+- [x] Repeat the sweep with `dem = testdata_path("dem.tif")` behind
       `skip_if_no_terra()` — all four columns come from the same `st_sf()` call
-- [ ] Downstream pass-through: a tibble-backed footprint still flows through
+- [x] Downstream pass-through: a tibble-backed footprint still flows through
       `fly_coverage()` / `fly_overlap()` / `fly_filter()` / `fly_select()`
-- [ ] **Confirm the sweep fails on unmodified `R/`** before touching the source
+- [x] **Confirm the sweep fails on unmodified `R/`** before touching the source
 
 ## Phase 2: The fix
 
diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R
index 3d69915..e29675c 100644
--- a/tests/testthat/setup.R
+++ b/tests/testthat/setup.R
@@ -56,3 +56,35 @@ terrain_fixture <- function() {
     )
   )
 }
+
+
+# The same bundled centroids in every class shape a real caller can supply.
+#
+# `bcdata::collect()` returns `bcdc_sf, sf, tbl_df, tbl, data.frame`, and #35
+# measured that `tbl_df` is the discriminating member — stripping the bcdata
+# class alone does not change the outcome — so these shapes bound the real
+# inputs without fly taking a dependency on bcdata to build a fixture.
+#
+# Read honestly rather than by overwriting `class()`: `st_read(as_tibble =)`
+# produces the tibble-backed sf a caller would actually hold. The premise is
+# asserted here so that a future `sf` change fails on the premise, naming the
+# real cause, rather than on the behaviour under test.
+centroid_shapes <- function() {
+  p <- testdata_path("photo_centroids.gpkg")
+  plain <- sf::st_read(p, quiet = TRUE)
+  tbl <- sf::st_read(p, quiet = TRUE, as_tibble = TRUE)
+  stopifnot(
+    !inherits(plain, "tbl_df"),
+    inherits(tbl, "tbl_df")
+  )
+  list(
+    plain = plain,
+    tbl = tbl,
+    grouped = dplyr::group_by(tbl, .data$scale)
+  )
+}
+
+# The columns #30 and #9 added, which #35 found were reaching no tibble caller.
+fly_reported_cols <- function() {
+  c("footprint_basis", "footprint_terrain", "height_agl", "dem_coverage")
+}
diff --git a/tests/testthat/test-fly_footprint.R b/tests/testthat/test-fly_footprint.R
index ee6f37e..7491983 100644
--- a/tests/testthat/test-fly_footprint.R
+++ b/tests/testthat/test-fly_footprint.R
@@ -647,3 +647,92 @@ test_that("fly_footprint reports coverage of the footprint it actually returns",
   expect_lt(truth, 0.95)               # the fixture must reach the failure mode
   expect_equal(fp$dem_coverage, truth, tolerance = 1e-6)
 })
+
+
+test_that("fly_footprint reports the same columns whatever class the input carries", {
+  # #35: `sf::st_sf()` keeps only its first argument when that argument is a
+  # tibble, so the four reporting columns were discarded for every caller whose
+  # input carried `tbl_df` — which is what `bcdata::collect()` returns, and so
+  # what the package's own documented data source hands back. Every fixture in
+  # the package is plain `sf, data.frame`, so no case added along the existing
+  # axis could have found this. Sweep the axis instead.
+  shapes <- centroid_shapes()
+  reported <- fly_reported_cols()
+
+  out <- lapply(shapes, fly_footprint)
+
+  for (nm in names(out)) {
+    expect_true(all(reported %in% names(out[[nm]])), info = nm)
+  }
+
+  # Not merely present: identical, column for column, to what the plain shape
+  # gets. A fix that supplied the names and lost the values would pass the
+  # check above.
+  for (nm in names(out)) {
+    expect_identical(names(out[[nm]]), names(out$plain), info = nm)
+    for (col in reported) {
+      expect_identical(out[[nm]][[col]], out$plain[[col]],
+                       info = paste(nm, col))
+    }
+    expect_identical(sf::st_geometry(out[[nm]]), sf::st_geometry(out$plain),
+                     info = nm)
+  }
+})
+
+
+test_that("fly_footprint reports the same columns on the dem path too", {
+  skip_if_no_terra()
+  # All four columns are attached by the one `st_sf()` call, so the terrain path
+  # fails the same way — and it is `dem_coverage`, documented as the filter for
+  # partially-covered frames, that goes missing there.
+  dem <- terra::rast(testdata_path("dem.tif"))
+  shapes <- centroid_shapes()
+  reported <- fly_reported_cols()
+
+  out <- lapply(shapes, function(x) suppressWarnings(fly_footprint(x, dem = dem)))
+
+  # The fixture must reach the terrain code, or this sweep says nothing about it.
+  expect_true(any(out$plain$footprint_terrain == "dem_agl", na.rm = TRUE))
+  expect_true(any(!is.na(out$plain$dem_coverage)))
+
+  for (nm in names(out)) {
+    expect_true(all(reported %in% names(out[[nm]])), info = nm)
+    for (col in reported) {
+      expect_identical(out[[nm]][[col]], out$plain[[col]],
+                       info = paste(nm, col))
+    }
+  }
+})
+
+
+test_that("fly_footprint returns the class it was given", {
+  # Contract, not the #35 regression guard: the broken code preserved the class
+  # correctly and dropped the columns. This guards the other direction — a fix
+  # that coerced the frame to `data.frame` would hand a bcdata caller back
+  # something narrower than they passed in.
+  shapes <- centroid_shapes()
+  for (nm in names(shapes)) {
+    expect_identical(class(fly_footprint(shapes[[nm]])), class(shapes[[nm]]),
+                     info = nm)
+  }
+})
+
+
+test_that("a tibble-backed footprint still flows through the consumers", {
+  # The four columns are new on this path, so check they do not disturb the
+  # functions that take a footprint. Numbers come from the plain shape, which
+  # the rest of the suite already pins.
+  shapes <- centroid_shapes()
+  aoi <- sf::st_read(testdata_path("aoi.gpkg"), quiet = TRUE)
+
+  for (nm in names(shapes)) {
+    x <- shapes[[nm]]
+    expect_equal(nrow(fly_filter(x, aoi)), nrow(fly_filter(shapes$plain, aoi)),
+                 info = nm)
+    expect_equal(fly_coverage(x, aoi, by = "photo_year")$covered_km2,
+                 fly_coverage(shapes$plain, aoi, by = "photo_year")$covered_km2,
+                 info = nm)
+    expect_equal(nrow(fly_overlap(x)), nrow(fly_overlap(shapes$plain)),
+                 info = nm)
+  }
+})

From ae628bca30b8f40a8d1963ce8d84bd7fd4377e78 Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:37:56 -0700
Subject: [PATCH 3/6] Build the frame before st_sf() sees it, so the columns
 survive a tibble
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

st_sf() keeps only its first argument when that argument is a tibble:
`else if (inherits(x[[1]], c("tbl_df","tbl"))) x[[1]]`. Every trailing
named column is discarded, so footprint_basis, footprint_terrain,
height_agl and dem_coverage reached no bcdata caller — which is to say
no caller using this package's own documented data source. Assigning
them onto the attribute frame puts them inside x[[1]], where they
survive, and leaves the caller's class alone.

Three things the reviews found, each reproduced before acting:

- 0-row input returned footprint_basis and footprint_terrain as
  logical, because ifelse(logical(0), ...) is logical(0). An empty
  result would not bind to a populated one, which is exactly the
  per-AOI ledger this reporting surface exists for. Seeded with
  as.character().
- An input already carrying footprint_basis got a duplicate
  footprint_basis.1 under the old code, and the filter the docs
  prescribe read the caller's column while fly's answer sat unread
  beside it. The computed value now wins.
- The class contract was stated too strongly. st_transform(), not
  st_sf(), moves sf to the front, so a bcdc_sf input returns
  sf, bcdc_sf, ... The set survives; the order does not.

Each new guard was checked by restoring the defect from 8585fd5 --
not a reconstruction -- patching both the namespace and the attached
binding, and printing a value only the broken code could produce.

225 -> 338 tests. Refs #35

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR
---
 R/fly_footprint.R                   |  37 +++--
 man/fly-package.Rd                  |   4 +-
 man/fly_footprint.Rd                |   6 +-
 planning/active/findings.md         |  61 +++++++-
 planning/active/progress.md         |  11 ++
 planning/active/review-round1.md    | 220 ++++++++++++++++++++++++++++
 planning/active/task_plan.md        |  41 +++++-
 tests/testthat/setup.R              |  31 +++-
 tests/testthat/test-fly_footprint.R |  92 ++++++++++--
 9 files changed, 465 insertions(+), 38 deletions(-)
 create mode 100644 planning/active/review-round1.md

diff --git a/R/fly_footprint.R b/R/fly_footprint.R
index 32cdf93..e828878 100644
--- a/R/fly_footprint.R
+++ b/R/fly_footprint.R
@@ -149,7 +149,11 @@ fly_rectangles <- function(coords, half_side) {
 #'   `height_agl` giving the metres above ground each footprint was sized from,
 #'   and `dem_coverage` giving the fraction of each footprint the DEM actually
 #'   covered (`0` where it covered none, `NA` only where there is no footprint).
-#'   Frames whose format could not be resolved get an empty geometry.
+#'   Frames whose format could not be resolved get an empty geometry. Every
+#'   class the input carries is carried through, so a tibble-backed sf — which
+#'   is what `bcdata::collect()` returns — comes back tibble-backed. The order
+#'   is not preserved: `sf::st_transform()` moves `sf` to the front, so a
+#'   `bcdc_sf` input returns `sf, bcdc_sf, ...`, as it always has.
 #'
 #' @details
 #' Ground coverage is computed as `negative_size * scale_number * 0.0254` metres
@@ -359,7 +363,12 @@ fly_footprint <- function(centroids_sf, negative_size = 9, format_size = NULL,
   } else {
     media <- as.character(centroids_sf$media)
     width_in <- unname(formats[media])
-    basis <- ifelse(is.na(width_in), "unknown_format", media)
+    # `as.character()` is load-bearing on empty input: `ifelse(logical(0), ...)`
+    # returns `logical(0)`, so a query that matched no frames would report its
+    # basis as a logical column. Binding that to a populated result — the
+    # per-AOI ledger this reporting surface exists for — fails on the type
+    # rather than yielding an empty block of rows.
+    basis <- as.character(ifelse(is.na(width_in), "unknown_format", media))
   }
 
   unresolved <- is.na(width_in)
@@ -379,7 +388,7 @@ fly_footprint <- function(centroids_sf, negative_size = 9, format_size = NULL,
   # Keyed on half_side, not width_in: an unparseable `scale` also leaves a frame
   # with no footprint, and a frame with no footprint has had no terrain
   # treatment to report.
-  terrain <- ifelse(is.na(half_side), NA_character_, "nominal_scale")
+  terrain <- as.character(ifelse(is.na(half_side), NA_character_, "nominal_scale"))
   height_agl <- rep(NA_real_, n)
   dem_coverage <- rep(NA_real_, n)
 
@@ -479,14 +488,20 @@ fly_footprint <- function(centroids_sf, negative_size = 9, format_size = NULL,
     terrain[unusable] <- "nominal_scale"
   }
 
-  result <- sf::st_sf(
-    sf::st_drop_geometry(pts_3005),
-    footprint_basis = basis,
-    footprint_terrain = terrain,
-    height_agl = height_agl,
-    dem_coverage = dem_coverage,
-    geometry = fly_rectangles(coords, half_side)
-  )
+  # Assign the reporting columns onto the attribute frame rather than passing
+  # them to `st_sf()` as trailing arguments. `st_sf()` builds its attribute
+  # frame with `else if (inherits(x[[1]], c("tbl_df", "tbl"))) x[[1]]`, so a
+  # tibble first argument means every trailing named column is silently
+  # discarded — and `bcdata::collect()` returns a tibble, which is to say the
+  # package's own documented data source (#35). Inside `x[[1]]` they survive,
+  # and the caller keeps the class they passed in.
+  attrs <- sf::st_drop_geometry(pts_3005)
+  attrs$footprint_basis <- basis
+  attrs$footprint_terrain <- terrain
+  attrs$height_agl <- height_agl
+  attrs$dem_coverage <- dem_coverage
+
+  result <- sf::st_sf(attrs, geometry = fly_rectangles(coords, half_side))
 
   sf::st_transform(result, input_crs)
 }
diff --git a/man/fly-package.Rd b/man/fly-package.Rd
index 6ffc9ce..7922534 100644
--- a/man/fly-package.Rd
+++ b/man/fly-package.Rd
@@ -4,11 +4,11 @@
 \name{fly-package}
 \alias{fly}
 \alias{fly-package}
-\title{fly: Airphoto Footprint Estimation and Coverage Selection}
+\title{fly: Historic Airphoto Footprints, Selection and Georeferencing for British Columbia}
 \description{
 \if{html}{\figure{logo.png}{options: style='float: right' alt='logo' width='120'}}
 
-Estimate ground footprints from airphoto centroids and scale, compute coverage of areas of interest, and select minimum photo sets using greedy set-cover.
+Estimate ground footprints from airphoto centroids and scale, compute coverage of areas of interest, select minimum photo sets using greedy set-cover, fetch scans and flight metadata from the BC Data Catalogue, and georeference the images onto their estimated footprints.
 }
 \seealso{
 Useful links:
diff --git a/man/fly_footprint.Rd b/man/fly_footprint.Rd
index 1cc4574..9d236b5 100644
--- a/man/fly_footprint.Rd
+++ b/man/fly_footprint.Rd
@@ -32,7 +32,11 @@ rectangles, a \code{footprint_basis} column recording how each was sized, a
 \code{height_agl} giving the metres above ground each footprint was sized from,
 and \code{dem_coverage} giving the fraction of each footprint the DEM actually
 covered (\code{0} where it covered none, \code{NA} only where there is no footprint).
-Frames whose format could not be resolved get an empty geometry.
+Frames whose format could not be resolved get an empty geometry. Every
+class the input carries is carried through, so a tibble-backed sf — which
+is what \code{bcdata::collect()} returns — comes back tibble-backed. The order
+is not preserved: \code{sf::st_transform()} moves \code{sf} to the front, so a
+\code{bcdc_sf} input returns \verb{sf, bcdc_sf, ...}, as it always has.
 }
 \description{
 Creates rectangular polygons representing the estimated ground coverage
diff --git a/planning/active/findings.md b/planning/active/findings.md
index a33ff4d..87cb5b0 100644
--- a/planning/active/findings.md
+++ b/planning/active/findings.md
@@ -39,7 +39,11 @@ st_sf(df,  extra = "hello", geometry = g)  ->  "extra" present: TRUE
 
 ## Scope — one call site
 
-`grep -rn "st_sf(" R/` returns two hits. `R/fly_footprint.R:64` builds a bare
+`grep -rn "st_sf(" R/` returns two hits. The claim is specifically about columns
+attached **through `st_sf()`** — columns are attached to user-supplied data in
+four other places (`fly_coverage.R:34`, `fly_select.R:118,207-208`,
+`fly_bearing.R:66`, `fly_georef.R:133`), all via `$<-`, which is the very
+mechanism this fix adopts and none of which are affected. `R/fly_footprint.R:64` builds a bare
 geometry frame (`st_sf(geometry = rects[ok])`) and attaches nothing to
 user-supplied data. `R/fly_footprint.R:482` is the defect. Every other frame in
 the package is a `dplyr::tibble()` built from scratch — `fly_fetch.R:84,93,104`,
@@ -122,3 +126,58 @@ CI on `main` at session start: all recent runs green (pkgdown + pages).
 
 | Error | Resolution |
 |-------|------------|
+
+## A/B against `8585fd5` on the edge cases the fix could plausibly move
+
+Prior implementation extracted with `git show 8585fd5:R/fly_footprint.R` and run
+side by side with the new one (not reconstructed from memory):
+
+| input | names | nrow | row.names | geometry | `sf_column` |
+|---|---|---|---|---|---|
+| full 20-row plain sf | identical | 20/20 | identical | identical | `geometry`, last |
+| zero-row | identical | 0/0 | identical | identical | `geometry`, last |
+| non-sequential subset `[c(5,3,11), ]` | identical | 3/3 | identical | identical | `geometry`, last |
+| single row | identical | 1/1 | identical | identical | `geometry`, last |
+
+So `$<-` recycling, the `row.names` argument the old `st_sf()` path used, and
+column ordering are all non-issues.
+
+**One deliberate behaviour change, and it is wider than first measured.** When
+the input already carries a column named `footprint_basis`, the old path kept the
+caller's value and discarded the computed one. Corrected measurement — the first
+probe counted exact name matches and so missed the duplicate on the plain path:
+
+| input | old | new |
+|---|---|---|
+| plain sf | 22 cols: `footprint_basis`, `footprint_basis.1`; `$footprint_basis` = `"PRE"` | 21 cols: `footprint_basis` = `"Film - BW"` |
+| tibble sf | 18 cols: `footprint_basis` = `"PRE"` (four columns dropped) | 21 cols: `footprint_basis` = `"Film - BW"` |
+
+So a **plain-sf** caller with a colliding column — a shape #35 never broke — sees
+a column-count and value change too. The new direction is the correct one:
+`fly_footprint()` must report how *it* sized each frame, not echo back a
+same-named column it was handed, and `fly_footprint(fly_footprint(x))` is now
+idempotent rather than sticky. But it is a second fix landing alongside the
+first, so it gets its own test and its own NEWS line rather than riding in
+silently.
+
+## Restore-the-bug verification
+
+Prior function loaded from `8585fd5` into both `asNamespace("fly")` and
+`as.environment("package:fly")` — patching only the namespace gives a false green
+for anything test code calls directly. Proof-of-patch printed before asserting:
+
+```
+PATCH PROOF - footprint_basis on tbl input: NULL (broken code running)
+```
+
+The sweep then failed on the `tbl` and `grouped` shapes, naming the four missing
+columns. The guard is real.
+
+## Suite and lint after the fix
+
+- `devtools::test()`: **FAIL 0 | WARN 0 | SKIP 0 | PASS 275** (baseline 225)
+- `lintr` on `R/fly_footprint.R`: 0 at `8585fd5`, 0 on branch. Package total 16,
+  all pre-existing (`data-raw/`, `fly_fetch`, `fly_georef`, `test-fly_fetch`,
+  `test-fly_footprint.R:391`, vignette)
+- `devtools::document()` wrote only `fly_footprint.Rd`; `NAMESPACE` unchanged at
+  9 exports, so no roxygen block rebound
diff --git a/planning/active/progress.md b/planning/active/progress.md
index 7f69735..0e10df9 100644
--- a/planning/active/progress.md
+++ b/planning/active/progress.md
@@ -20,3 +20,14 @@
   `NULL` for all four columns where the `plain` shape returns the vectors
 - The class-contract test passes on the broken code by design and is commented
   as such — it guards against the coercing fix, not against #35
+
+### Phases 2-3 — fix, contract, review findings (complete)
+
+- Fix applied at `R/fly_footprint.R`; suite FAIL 0, PASS 338 (baseline 225)
+- Restore-the-bug confirmed for all three new guards, each with a proof-of-patch
+  line printed before asserting, patching both bindings, prior code pulled from
+  `8585fd5` rather than reconstructed
+- Plan review (12 findings) and code-check round 1 (5 findings) both landed;
+  every load-bearing claim reproduced before acting. Two corrected a claim I had
+  written: the class-preservation over-claim, and a false idempotence line in NEWS
+- `R CMD check` Status OK; NAMESPACE byte-identical at 9 exports; lint unchanged
diff --git a/planning/active/review-round1.md b/planning/active/review-round1.md
new file mode 100644
index 0000000..9966fc5
--- /dev/null
+++ b/planning/active/review-round1.md
@@ -0,0 +1,220 @@
+# Review round 1 — #35 `st_sf()` drops trailing columns on tibble input
+
+Branch `35-fly-footprint-drops-footprint-basis-and`. Reviewed `git diff main...HEAD`
+(tests) + `git diff --cached` (fix + Rd), and the full current text of
+`R/fly_footprint.R`, `tests/testthat/setup.R`, `tests/testthat/test-fly_footprint.R`.
+
+Everything below was **measured by running R**, not reasoned about. Scripts are in
+`/tmp/flyrev/`. The prior implementation was pulled with
+`git show 8585fd5:R/fly_footprint.R` and sourced into an env parented on
+`asNamespace("fly")` — not reconstructed from memory.
+
+## Verdict
+
+The fix itself is correct and, on every input I could construct, **byte-identical to
+`8585fd5` for plain `data.frame` callers** while adding the four columns for tibble
+callers. The five findings below are all in the surrounding material — one false
+claim in the in-flight `NEWS.md`, two guard/fixture gaps, one pre-existing type
+trap the fix newly exposes, and one process note.
+
+## Findings
+
+- **[fragile]** `NEWS.md:10` (unstaged, written by the concurrent session) — the claim
+  that the overwrite change *"makes `fly_footprint(fly_footprint(x))` idempotent"* is
+  **false**, and false in the dangerous direction. Measured
+  (`/tmp/flyrev/10_idem.R`, `11_idem_tbl.R`):
+
+  | input | `fly_footprint(fly_footprint(x))` |
+  |---|---|
+  | plain `sf` (20 rows) | **100 rows, silently** |
+  | tibble-backed `sf` (20 rows) | error, `Can't recycle input of size 100 to size 20` |
+
+  Cause is pre-existing and identical before and after the fix:
+  `sf::st_coordinates()` on POLYGON input returns 5 rows per feature, so
+  `fly_rectangles()` builds `5n` geometries; in the `cbind(data.frame(row.names = ...))`
+  branch `row.names` is `seq_along(sfc)` = `1:100`, which recycles the 20-row attribute
+  frame up to 100. Nothing warns. The overwrite change is a genuine improvement and is
+  worth the NEWS line — but the idempotence sentence should be cut or replaced, because
+  a reader will take it as licence to re-run the function on its own output.
+
+- **[fragile]** `R/fly_footprint.R:493-496` / `tests/testthat/test-fly_footprint.R` —
+  the behaviour change on an input that **already carries** one of the four names has
+  no test, in either direction. Measured (`/tmp/flyrev/03_checks.R`, input given
+  `footprint_basis = "PRE-EXISTING"` and `dem_coverage = -99`):
+
+  ```
+  old (8585fd5): ... footprint_basis, dem_coverage, footprint_basis.1, footprint_terrain, height_agl, dem_coverage.1, geometry   (23 cols)
+  new:           ... footprint_basis, dem_coverage, footprint_terrain, height_agl, geometry                                       (21 cols)
+  old footprint_basis[1] = "PRE-EXISTING"   new footprint_basis[1] = "Film - BW"
+  ```
+
+  The new behaviour is the better of the two — under the old code the documented
+  workflow `footprints$footprint_basis != "unknown_format"` read the *caller's* column
+  and fly's real answer sat unnoticed in `footprint_basis.1`. But it is (a) silent data
+  loss of a caller-supplied column and (b) completely unguarded: a future
+  "simplification" back to trailing `st_sf()` arguments would reinstate the `.1`
+  duplicate for `data.frame` callers with **nothing in the suite failing**, since the
+  class sweep only asserts the columns are present and equal across shapes. Since NEWS
+  now advertises this as a deliberate second change, it wants an assertion behind it.
+
+- **[fragile]** `tests/testthat/test-fly_footprint.R` (class-contract test) +
+  `R/fly_footprint.R:152-154` (`@return`) — the contract asserted is stronger than the
+  one the code provides, and the fixture is structurally unable to show it. Measured
+  (`/tmp/flyrev/08_bcdc.R`) on the actual documented source shape:
+
+  ```
+  in : bcdc_sf, sf, tbl_df, tbl, data.frame
+  out: sf, bcdc_sf, tbl_df, tbl, data.frame        identical(class(out), class(in)) == FALSE
+  ```
+
+  So `expect_identical(class(fly_footprint(x)), class(x))` holds for `plain`/`tbl`/
+  `grouped` and **fails for a real `bcdata::collect()` result** — which is the caller
+  #35 exists for, and which the unchecked "End-to-end against a real `bcdata::collect()`
+  result" box in `task_plan.md` is the last defence for. The reordering is pre-existing
+  (`sf::st_transform()` does it, not `st_sf()` — verify: `class(st_transform(b, 3005))`
+  is already `sf, bcdc_sf, ...` before any of this code runs) and is identical old vs
+  new, so it is not a regression. Two consequences worth handling:
+  - the new `@return` sentence *"The input's class is preserved"* over-claims for
+    exactly that caller — the class *set* survives, the order does not;
+  - `NEWS.md:12` attributes the reordering to `sf::st_sf()`. It is `sf::st_transform()`.
+
+  The four columns themselves **do** arrive correctly for `bcdc_sf` (verified TRUE),
+  so the fix works; only the stated contract is too strong.
+
+- **[fragile]** `R/fly_footprint.R:384` (pre-existing, newly exposed) — on a **0-row**
+  input, `footprint_basis` and `footprint_terrain` come back **`logical`**, not
+  `character`, because `ifelse(logical(0), ...)` returns `logical(0)`. Old and new are
+  identical here (`all.equal(old(x[0,]), new(x[0,]))` is `TRUE`), so this is not a
+  regression — but the fix makes the columns reach tibble callers for the first time,
+  and the type is load-bearing for the obvious downstream move
+  (`/tmp/flyrev/12_bindrows.R`):
+
+  ```
+  dplyr::bind_rows(fly_footprint(x[0, ]), fly_footprint(x))
+  #> Error: Can't combine `..1$footprint_basis` <logical> and `..2$footprint_basis` <character>
+  ```
+
+  Anyone assembling a per-AOI ledger across queries — the `stac_airphoto_bc` use case
+  in the issue — hits this the first time an AOI returns no frames. One line
+  (`character(0)` / `NA_character_` seeds, or `rep(NA_character_, n)` before the
+  `ifelse`) closes it, and it is cheap to fold in here since the release is already
+  about this function's reporting columns.
+
+- **[process]** working tree — a **concurrent session is editing this checkout**
+  (`DESCRIPTION`, `NEWS.md`, `planning/active/findings.md` changed *during* this
+  review; they were clean at the start). Per CLAUDE.md "Two agent sessions must not
+  share one git working tree". Also: I ran `devtools::document()` as part of the
+  verification, which regenerated `man/fly-package.Rd` to match their new
+  `DESCRIPTION` Title. That file is now modified and unstaged. The regeneration is
+  correct — do not revert it — but it is my write, not theirs.
+
+- **[note, not a defect]** `planning/active/task_plan.md` Phase 1 marks
+  *"Downstream pass-through: ... `fly_coverage()` / `fly_overlap()` / `fly_filter()` /
+  `fly_select()`"* complete, but the implemented test covers only the first three;
+  `fly_select()` is not in it. I exercised `fly_select()` (all three modes),
+  `fly_summary()` and `fly_bearing()` across plain/tibble/grouped myself
+  (`/tmp/flyrev/09_select.R`) — **all pass**, so there is no bug hiding behind the
+  checkbox, only PWF drift.
+
+## What I verified clean, and how
+
+Each of the seven questions asked, answered by measurement rather than reading.
+
+1. **Zero-row input** (`/tmp/flyrev/03_checks.R`). Old and new both return without
+   error and `all.equal(old, new)` is `TRUE` — same 21 names, same column types,
+   same everything. `$<-` on a 0-row tibble with a length-0 RHS is accepted (0
+   matches `nrow`), and the four vectors are always exactly `nrow()` long
+   (`rep()`/`ifelse()` over length-`n` inputs), so no recycling difference exists to
+   find. The only 0-row wrinkle is the `logical` typing above, which is unchanged.
+
+2. **Input already carrying `footprint_basis`** — see finding 2. New behaviour is the
+   better one; the risk is that it is untested, not that it is wrong.
+
+3. **Non-sequential row names**, `centroids[c(5, 3, 11), ]` (`/tmp/flyrev/03_checks.R`).
+   No reorder, no drop, no misalignment, and old/new `all.equal` `TRUE`:
+
+   ```
+   input row.names  5,3,11
+   old out          row.names 1,2,3   airp_id 699365,699426,697358
+   new out          row.names 1,2,3   airp_id 699365,699426,697358   geometry identical: TRUE
+   ```
+
+   The `row.names` reset to `1:n` is `st_sf()`'s own `row.names = seq_along(x[[sf_column]])`
+   default and is present in both implementations. Alignment checked independently:
+   each output rectangle's centroid equals its input point (`all.equal`, tol 1e-6).
+   Repeated on the tibble subset — same geometry, four columns present.
+
+4. **`sf_column` and column ordering** (`/tmp/flyrev/03_checks.R`, `07_odd.R`).
+   `attr(, "sf_column")` is `"geometry"` and last in both; `names()`, `row.names()`,
+   `class()`, `agr` and `st_crs()` all identical to `8585fd5`, and `all.equal(old, new)`
+   is `TRUE`. Re-ran that comparison across seven awkward inputs to make sure the
+   `cbind(...)` fall-through branch had not shifted: non-syntactic column name, list
+   column, factor column, matrix column, single row, and an input whose sfc is named
+   `geom` (which the bundled fixture is — the output is renamed to `geometry` by both
+   implementations alike). **All seven: names identical, `all.equal` TRUE.**
+
+5. **Can the sweep pass vacuously?** No. Restored the bug properly — patched **both**
+   `asNamespace("fly")` and `as.environment("package:fly")` (the search-path copy is
+   what the test file resolves), from the extracted `8585fd5` bytes, and printed a
+   proof line before running (`/tmp/flyrev/04_restore.R`):
+
+   ```
+   patching 2 environments; search has package:fly: TRUE
+   PROOF broken: footprint_basis in names(fly_footprint(tbl)) == FALSE (must be FALSE)
+   -> test-fly_footprint.R goes red, 10-failure cap reached ("... and 13 more")
+   ```
+
+   On the specific sub-questions asked:
+   - The `plain` iteration of `expect_identical(names(out[[nm]]), names(out$plain))`
+     **is** a self-comparison that cannot fail, as are the per-column and
+     `st_geometry()` comparisons for `nm == "plain"`. This does **not** make the test
+     vacuous, because the guard is carried by the separate absolute assertion
+     `expect_true(all(reported %in% names(out[[nm]])))`, which is a real check for
+     every shape including `plain`. Three no-op assertions out of ~30; worth knowing,
+     not worth changing.
+   - No comparison is over an empty set. Measured (`/tmp/flyrev/05_vacuity.R`):
+     `fly_filter` 20 rows, `fly_coverage` 1 row with `covered_km2 = 24.8`,
+     `fly_overlap` 61 pairs, and in the `dem` sweep `any(footprint_terrain == "dem_agl")`
+     `TRUE` with 20 non-`NA` `dem_coverage` — the premise assertions in that test are
+     real and do reach the terrain code.
+   - The `stopifnot()` premise in `centroid_shapes()` **is** reachable (evaluated on
+     every one of the four calls) and rots in the safe direction: if `sf` ever makes
+     `st_read()` tibble-backed by default, `!inherits(plain, "tbl_df")` fires and names
+     the premise instead of the behaviour. One asymmetry — it asserts the `plain`/`tbl`
+     premises but not `inherits(grouped, "grouped_df")`, so the third shape has no
+     premise of its own.
+   - The class-contract test and the consumer pass-through test both pass on the broken
+     code (confirmed in the restore run). The first is explicitly commented as such; the
+     second is a forward guard rather than a #35 regression guard and is not labelled,
+     which is a smaller version of the same thing.
+
+6. **`.data$scale` in `dplyr::group_by()`** — safe. Verified two ways rather than by
+   reasoning about imports. In `R_DEFAULT_PACKAGES=base Rscript --vanilla` with
+   **nothing attached but base** (`/tmp/flyrev/06_data_pronoun.R`), `group_by(df, .data$scale)`
+   succeeds — `.data` comes from rlang's data mask, not the search path
+   (`exists(".data")` in globalenv is `FALSE`). And the full `R CMD check` runs
+   `testthat.R` clean: **Status: OK, 0 errors | 0 warnings | 0 notes**, tests `[78s/84s] OK`.
+
+7. **Other `st_sf()` sites** — `grep -rn "st_sf(" R/` gives exactly two.
+   `R/fly_footprint.R:64` is `st_sf(geometry = rects[ok])`, a bare geometry frame that
+   attaches nothing to user data and cannot hit the branch. The other is the fixed one.
+   I also swept the adjacent mechanisms rather than trusting the `st_sf` grep:
+   `cbind` / `bind_cols` / `st_set_geometry` have no hits; columns are attached to
+   user-supplied objects by `$<-` at `fly_coverage.R:34`, `fly_select.R:118,207-208`,
+   plus `fly_bearing.R` and `fly_georef.R` — which is the same mechanism the fix
+   adopts, so none are affected. Confirmed empirically by running `fly_select()`
+   (minimal / all / component_ensure), `fly_summary()` and `fly_bearing()` against all
+   three class shapes: no errors, correct classes out.
+
+## Other checks
+
+- Full suite: **FAIL 0 | WARN 0 | SKIP 0 | PASS 275** (baseline in `findings.md` was 225).
+- `lintr::lint("R/fly_footprint.R")`: no lints, and no lints on the `HEAD` baseline
+  either — no change in lint count.
+- `devtools::document()` printed no `Writing '<x>.Rd'` for an unexpected file;
+  `NAMESPACE` unchanged, `grep -c "^export("` still 9, `export(fly_footprint)` present
+  — the roxygen block did not rebind.
+- The staged `man/fly_footprint.Rd` is in sync with the roxygen source (re-running
+  `document()` produced no further diff to it). Its non-ASCII em-dashes match 18
+  already present in that file at `HEAD` and in four other `.Rd` files, and
+  `R CMD check` is clean.
diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md
index e0dd942..4180334 100644
--- a/planning/active/task_plan.md
+++ b/planning/active/task_plan.md
@@ -31,23 +31,50 @@ a tibble, discarding every trailing named column
 
 ## Phase 2: The fix
 
-- [ ] `R/fly_footprint.R:482` — assign the four columns onto the attribute frame,
+- [x] `R/fly_footprint.R:482` — assign the four columns onto the attribute frame,
       then one-arg `sf::st_sf(attrs, geometry = ...)`. Preserves the caller's
       class; chosen over `as.data.frame()`, which additionally downgrades a
       bcdata caller's tibble
-- [ ] Comment names the `st_sf()` branch and #35, so the next reader does not
+- [x] Comment names the `st_sf()` branch and #35, so the next reader does not
       "simplify" it back
-- [ ] Restore the bug and confirm the sweep goes red — patching **both**
+- [x] Restore the bug and confirm the sweep goes red — patching **both**
       `asNamespace("fly")` and `as.environment("package:fly")`, from
       `git show 8585fd5:R/fly_footprint.R` rather than from memory
-- [ ] Full suite green, PASS above the 225 baseline
+- [x] Full suite green, PASS above the 225 baseline
+
+## Phase 2b: Review findings folded in
+
+Both reviewers ran independently against the branch; every claim below was
+reproduced before acting on it.
+
+- [x] `@return` and the class test over-claimed — `identical(class(out), class(in))`
+      is FALSE for `bcdc_sf`, because `st_transform()` (not `st_sf()`) moves `sf`
+      to the front. Assert the class *set*, add the `bcdc` shape the fixture
+      lacked, and correct the `@return` and NEWS wording
+- [x] The non-terra sweep compared constants and all-`NA`, so only *absence* made
+      it fail. Sweep the mixed-media fixture too, where `footprint_basis` varies
+      and reaches `unknown_format`
+- [x] The collision behaviour was unguarded: the old code appended
+      `footprint_basis.1` and left the caller's value under the documented name,
+      so the prescribed filter read the wrong column. Now asserted
+- [x] 0-row input returned `footprint_basis`/`footprint_terrain` as `logical`
+      (`ifelse(logical(0), ...)`), so an empty result would not bind to a
+      populated one — the per-AOI ledger in the issue. Seeded with
+      `as.character()` and guarded
+- [x] Downstream test used `by = "photo_year"`, which is one group on this
+      fixture. Switched to `by = "scale"` (two), added non-degeneracy premises,
+      and added the `fly_select()` the checkbox claimed
+- [x] Cut a false NEWS claim: `fly_footprint(fly_footprint(x))` is **not**
+      idempotent — a plain 20-row sf returns 100 rows silently, because
+      `st_coordinates()` on polygons yields 5 rows per feature. Pre-existing and
+      unchanged, but the sentence invited the round trip
 
 ## Phase 3: Document the contract
 
-- [ ] `@return` in `R/fly_footprint.R` states the input's class is preserved
-- [ ] `devtools::document()` — read its output; an unexpected `Writing '<x>.Rd'`
+- [x] `@return` in `R/fly_footprint.R` states the input's class is preserved
+- [x] `devtools::document()` — read its output; an unexpected `Writing '<x>.Rd'`
       or a falling `grep -c "^export(" NAMESPACE` means a roxygen block rebound
-- [ ] `lintr::lint_package()` compared against the `HEAD` baseline per file
+- [x] `lintr::lint_package()` compared against the `HEAD` baseline per file
 
 ## Phase 4: DESCRIPTION Title (#31, folded in)
 
diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R
index e29675c..d286b46 100644
--- a/tests/testthat/setup.R
+++ b/tests/testthat/setup.R
@@ -73,15 +73,34 @@ centroid_shapes <- function() {
   p <- testdata_path("photo_centroids.gpkg")
   plain <- sf::st_read(p, quiet = TRUE)
   tbl <- sf::st_read(p, quiet = TRUE, as_tibble = TRUE)
+  grouped <- dplyr::group_by(tbl, .data$scale)
+  # `bcdc_sf` is set by hand rather than by querying: bcdata is not a dependency
+  # of fly, and #35 measured that `tbl_df` is what selects the failing branch —
+  # so this shape exists to pin the *class contract* for the documented caller,
+  # not to reach the bug. `st_transform()` moves `sf` to the front of the class
+  # vector, so this is the one shape that shows the order is not preserved.
+  bcdc <- tbl
+  class(bcdc) <- c("bcdc_sf", class(bcdc))
   stopifnot(
     !inherits(plain, "tbl_df"),
-    inherits(tbl, "tbl_df")
-  )
-  list(
-    plain = plain,
-    tbl = tbl,
-    grouped = dplyr::group_by(tbl, .data$scale)
+    inherits(tbl, "tbl_df"),
+    inherits(grouped, "grouped_df"),
+    identical(class(bcdc)[1:2], c("bcdc_sf", "sf"))
   )
+  list(plain = plain, tbl = tbl, grouped = grouped, bcdc = bcdc)
+}
+
+
+# The bundled centroids are one film stock at one terrain treatment, so with no
+# `dem` all four reporting columns are constant or all-`NA` — a value comparison
+# across class shapes there is nearly vacuous, and only their *absence* makes it
+# fail. The mixed-media fixture varies `footprint_basis` across four rows and
+# reaches the `unknown_format` branch, so the sweep compares something.
+mixed_media_shapes <- function() {
+  mm <- mixed_media_fixture()
+  tbl <- sf::st_as_sf(dplyr::as_tibble(mm))
+  stopifnot(!inherits(mm, "tbl_df"), inherits(tbl, "tbl_df"))
+  list(plain = mm, tbl = tbl)
 }
 
 # The columns #30 and #9 added, which #35 found were reaching no tibble caller.
diff --git a/tests/testthat/test-fly_footprint.R b/tests/testthat/test-fly_footprint.R
index 7491983..e17bb64 100644
--- a/tests/testthat/test-fly_footprint.R
+++ b/tests/testthat/test-fly_footprint.R
@@ -665,6 +665,18 @@ test_that("fly_footprint reports the same columns whatever class the input carri
     expect_true(all(reported %in% names(out[[nm]])), info = nm)
   }
 
+  # The bundled centroids give a constant basis and all-NA terrain columns, so
+  # the value comparison below would hold for any implementation that got the
+  # names right. Sweep the mixed-media fixture too, where `footprint_basis`
+  # varies across rows and reaches `unknown_format`.
+  mixed <- lapply(mixed_media_shapes(), function(x) suppressWarnings(fly_footprint(x)))
+  expect_gt(length(unique(mixed$plain$footprint_basis)), 1)   # premise
+  for (nm in names(mixed)) {
+    expect_true(all(reported %in% names(mixed[[nm]])), info = nm)
+    expect_identical(mixed[[nm]]$footprint_basis, mixed$plain$footprint_basis,
+                     info = nm)
+  }
+
   # Not merely present: identical, column for column, to what the plain shape
   # gets. A fix that supplied the names and lost the values would pass the
   # check above.
@@ -705,19 +717,67 @@ test_that("fly_footprint reports the same columns on the dem path too", {
 })
 
 
-test_that("fly_footprint returns the class it was given", {
-  # Contract, not the #35 regression guard: the broken code preserved the class
+test_that("fly_footprint carries every class the input had", {
+  # Contract, not the #35 regression guard: the broken code carried the class
   # correctly and dropped the columns. This guards the other direction — a fix
   # that coerced the frame to `data.frame` would hand a bcdata caller back
   # something narrower than they passed in.
+  #
+  # Set, not sequence. `sf::st_transform()` moves `sf` to the front, so a
+  # `bcdc_sf` input comes back `sf, bcdc_sf, ...` — measured, and true of the
+  # prior implementation too. An `identical(class(out), class(in))` here would
+  # pass on the three shapes the fixture used to have and fail on the one
+  # caller the issue was filed about, which is the trap this branch exists to
+  # close rather than repeat.
   shapes <- centroid_shapes()
+  expect_true("bcdc" %in% names(shapes))          # premise: the shape is present
   for (nm in names(shapes)) {
-    expect_identical(class(fly_footprint(shapes[[nm]])), class(shapes[[nm]]),
-                     info = nm)
+    out <- fly_footprint(shapes[[nm]])
+    expect_true(all(class(shapes[[nm]]) %in% class(out)), info = nm)
+    expect_s3_class(out, "sf")
   }
 })
 
 
+test_that("fly_footprint overwrites a colliding reporting column", {
+  # Before #35 the trailing-argument form appended `footprint_basis.1` for a
+  # data.frame caller and kept the caller's value under the documented name —
+  # so `footprints$footprint_basis != "unknown_format"`, the filter the docs
+  # prescribe, read the caller's column and fly's real answer sat unread. The
+  # computed value must win. Unguarded, a revert to trailing `st_sf()`
+  # arguments would reinstate the duplicate with nothing failing.
+  shapes <- centroid_shapes()
+  for (nm in names(shapes)) {
+    x <- shapes[[nm]]
+    x$footprint_basis <- "PRE-EXISTING"
+    out <- fly_footprint(x)
+    expect_equal(sum(grepl("^footprint_basis", names(out))), 1L, info = nm)
+    expect_false(any(out$footprint_basis == "PRE-EXISTING"), info = nm)
+  }
+})
+
+
+test_that("an empty result binds to a populated one", {
+  # `ifelse(logical(0), ...)` returns `logical(0)`, so a query matching no
+  # frames reported its basis as a logical column. Assembling a per-AOI ledger
+  # across queries — the use the reporting columns exist for — then fails on
+  # the type the first time an AOI returns nothing, rather than contributing no
+  # rows. Reachable only from the empty input, which no other test supplies.
+  shapes <- centroid_shapes()
+  for (nm in names(shapes)) {
+    empty <- fly_footprint(shapes[[nm]][0, ])
+    expect_identical(nrow(empty), 0L, info = nm)
+    expect_type(empty$footprint_basis, "character")
+    expect_type(empty$footprint_terrain, "character")
+    expect_type(empty$height_agl, "double")
+    expect_type(empty$dem_coverage, "double")
+  }
+  full <- fly_footprint(shapes$plain)
+  bound <- dplyr::bind_rows(fly_footprint(shapes$plain[0, ]), full)
+  expect_identical(nrow(bound), nrow(full))
+})
+
+
 test_that("a tibble-backed footprint still flows through the consumers", {
   # The four columns are new on this path, so check they do not disturb the
   # functions that take a footprint. Numbers come from the plain shape, which
@@ -725,14 +785,26 @@ test_that("a tibble-backed footprint still flows through the consumers", {
   shapes <- centroid_shapes()
   aoi <- sf::st_read(testdata_path("aoi.gpkg"), quiet = TRUE)
 
+  # `by = "scale"` because the fixture is a single photo year — grouping on it
+  # gives one group, and a one-row-against-one-row comparison is the weakest
+  # available. Premises assert each reference result is non-degenerate, since
+  # every comparison below passes when both sides are empty.
+  ref_filter <- fly_filter(shapes$plain, aoi)
+  ref_cover <- fly_coverage(shapes$plain, aoi, by = "scale")
+  ref_overlap <- fly_overlap(shapes$plain)
+  ref_select <- suppressMessages(fly_select(shapes$plain, aoi))
+  expect_gt(nrow(ref_filter), 0)
+  expect_gt(nrow(ref_cover), 1)
+  expect_gt(nrow(ref_overlap), 0)
+  expect_gt(nrow(ref_select), 0)
+
   for (nm in names(shapes)) {
     x <- shapes[[nm]]
-    expect_equal(nrow(fly_filter(x, aoi)), nrow(fly_filter(shapes$plain, aoi)),
-                 info = nm)
-    expect_equal(fly_coverage(x, aoi, by = "photo_year")$covered_km2,
-                 fly_coverage(shapes$plain, aoi, by = "photo_year")$covered_km2,
-                 info = nm)
-    expect_equal(nrow(fly_overlap(x)), nrow(fly_overlap(shapes$plain)),
+    expect_equal(nrow(fly_filter(x, aoi)), nrow(ref_filter), info = nm)
+    expect_equal(fly_coverage(x, aoi, by = "scale")$covered_km2,
+                 ref_cover$covered_km2, info = nm)
+    expect_equal(nrow(fly_overlap(x)), nrow(ref_overlap), info = nm)
+    expect_equal(nrow(suppressMessages(fly_select(x, aoi))), nrow(ref_select),
                  info = nm)
   }
 })

From 28c4f8539583c802ab2a5c1ecded34c8d2f23da1 Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:38:07 -0700
Subject: [PATCH 4/6] Retitle for the half of the package the Title never
 mentioned

Title and Description both predated fly_fetch(), fly_georef() and
fly_bearing(), so the pkgdown landing page and any generated citation
described roughly half of what is here. README and CLAUDE.md were
widened already; DESCRIPTION was the remainder.

R CMD check Status OK, which is the only gate on the title-case and
Description sentence rules -- this repo ships pkgdown.yaml and no
R-CMD-check workflow, so CI does not repeat it.

Closes #31

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR
---
 DESCRIPTION                  | 8 +++++---
 planning/active/task_plan.md | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index e69b278..aefbdcc 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,5 +1,6 @@
 Package: fly
-Title: Airphoto Footprint Estimation and Coverage Selection
+Title: Historic Airphoto Footprints, Selection and Georeferencing for
+    British Columbia
 Version: 0.5.0
 Date: 2026-08-29
 Authors@R: c(
@@ -8,8 +9,9 @@ Authors@R: c(
     person("New Graph Environment", role = "cph")
     )
 Description: Estimate ground footprints from airphoto centroids and scale,
-    compute coverage of areas of interest, and select minimum photo sets
-    using greedy set-cover.
+    compute coverage of areas of interest, select minimum photo sets using
+    greedy set-cover, fetch scans and flight metadata from the BC Data
+    Catalogue, and georeference the images onto their estimated footprints.
 License: MIT + file LICENSE
 URL: https://github.com/NewGraphEnvironment/fly,
     https://newgraphenvironment.github.io/fly/
diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md
index 4180334..e776059 100644
--- a/planning/active/task_plan.md
+++ b/planning/active/task_plan.md
@@ -78,8 +78,8 @@ reproduced before acting on it.
 
 ## Phase 4: DESCRIPTION Title (#31, folded in)
 
-- [ ] Widen `Title:` and `Description:` to cover fetch and georeferencing
-- [ ] `devtools::document()`; `devtools::check()` for the title-case rules
+- [x] Widen `Title:` and `Description:` to cover fetch and georeferencing
+- [x] `devtools::document()`; `devtools::check()` for the title-case rules
 
 ## Phase 5: Release v0.5.1
 

From 3d902fa32ebe3dc4c949849468275edcbe272e92 Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:42:21 -0700
Subject: [PATCH 5/6] Release v0.5.1

The headline feature of 0.5.0 was unreachable from the data source this
package documents, so the patch is the point of the release rather than
housekeeping.

NEWS records what was actually lost: geometry and every downstream number
were always correct, and only the audit trail went missing -- which is why
nothing errored and why the suite stayed green through two releases.

Verified end to end against the live catalogue: 1405 real centroids come
back as bcdc_sf, sf, tbl_df, ..., all four columns arrive, and the
documented footprint_basis filter excludes 151 frames that were invisible
before.

No tag here. /gh-pr-merge does the tagging after merge; a tag on a branch
tip that review may still amend moves silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR
---
 DESCRIPTION                  |  2 +-
 NEWS.md                      | 10 ++++++++++
 planning/active/findings.md  | 26 ++++++++++++++++++++++++++
 planning/active/task_plan.md | 14 ++++++++------
 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index aefbdcc..831cf9d 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: fly
 Title: Historic Airphoto Footprints, Selection and Georeferencing for
     British Columbia
-Version: 0.5.0
+Version: 0.5.1
 Date: 2026-08-29
 Authors@R: c(
     person("Allan", "Irvine", , "al@newgraphenvironment.com", role = c("aut", "cre"),
diff --git a/NEWS.md b/NEWS.md
index 049536f..66cde2a 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,15 @@
 # fly (development version)
 
+## 0.5.1 (2026-08-29)
+
+- Fix `fly_footprint()` silently dropping `footprint_basis`, `footprint_terrain`, `height_agl` and `dem_coverage` whenever its input carried the `tbl_df` class ([#35](https://github.com/NewGraphEnvironment/fly/issues/35)). `bcdata::collect()` returns exactly that class, so every caller querying `WHSE_IMAGERY_AND_BASE_MAPS.AIMG_PHOTO_CENTROIDS_SP` — the documented source for this package — lost the whole reporting surface 0.4.0 and 0.5.0 added, and the documented "filter on `footprint_basis`" and "filter on `dem_coverage`" workflows were unreachable from it
+- Geometry and every downstream number were always correct; what was lost was the audit trail, which is what made it invisible. `sf::st_sf()` keeps only its first argument when that argument is a tibble, discarding every trailing named column — a general R trap rather than a `fly` one
+- Every class the input carries is carried through, so a tibble-backed sf comes back tibble-backed. The order is not preserved: `sf::st_transform()` moves `sf` to the front, so a `bcdc_sf` input returns `sf, bcdc_sf, ...` — as it always has
+- Where the input already carried a column named `footprint_basis` (or one of the other three), the old code kept the caller's value and appended a duplicate `footprint_basis.1` — so the documented `footprint_basis != "unknown_format"` filter read the caller's column while fly's own answer sat unread beside it. The computed value now wins, and there is one column. Plain `sf` callers were affected by this too, though never by the tibble bug
+- `fly_footprint()` on an input matching no frames now returns `footprint_basis` and `footprint_terrain` as `character` rather than `logical`, so an empty result binds to a populated one. Assembling a per-AOI ledger across queries previously failed on the column type the first time an area returned nothing
+- Tests sweep the input-class axis (plain / tibble / grouped) rather than adding cases along the one axis the bundled fixture could present. Every fixture in the package reads back as plain `sf, data.frame`, so the suite was structurally incapable of seeing this
+- Widen the `DESCRIPTION` Title and Description, which described roughly half the package — they predated `fly_fetch()`, `fly_georef()` and `fly_bearing()` ([#31](https://github.com/NewGraphEnvironment/fly/issues/31))
+
 ## 0.5.0 (2026-08-29)
 
 - `fly_footprint()` gains a `dem` argument, sizing each frame from its height above ground instead of the reported scale ([#9](https://github.com/NewGraphEnvironment/fly/issues/9)). On the bundled Upper Bulkley AOI the reported scale understates footprint **area by a median 14%, ranging to 26%** — and always in the same direction, because the scale is referenced to an elevation above the valley floor the photos cover. This is a datum offset, not the slope effect the issue described
diff --git a/planning/active/findings.md b/planning/active/findings.md
index 87cb5b0..ed33f44 100644
--- a/planning/active/findings.md
+++ b/planning/active/findings.md
@@ -181,3 +181,29 @@ columns. The guard is real.
   `test-fly_footprint.R:391`, vignette)
 - `devtools::document()` wrote only `fly_footprint.Rd`; `NAMESPACE` unchanged at
   9 exports, so no roxygen block rebound
+
+## End-to-end against the live BC Data Catalogue
+
+The case the fixture structurally cannot reach. `bcdc_query_geodata()` on
+`WHSE_IMAGERY_AND_BASE_MAPS.AIMG_PHOTO_CENTROIDS_SP`, bbox-filtered to the
+bundled AOI, then `collect()`:
+
+```
+rows: 1405
+input class : bcdc_sf,sf,tbl_df,tbl,data.frame
+output class: sf,bcdc_sf,tbl_df,tbl,data.frame
+four columns present: TRUE
+basis values: Film - BW | Film - Colour | unknown_format
+documented filter: 1254 of 1405
+```
+
+Three things this establishes that no fixture could:
+
+- the real input class is exactly the shape #35 named, and the four columns
+  arrive on it
+- the class **reordering** is real rather than an artefact of the hand-set
+  `bcdc_sf` in `centroid_shapes()` — `sf` leads on the way out, and it is
+  `st_transform()` that does it
+- the documented `footprint_basis != "unknown_format"` filter actually excludes
+  something on live data — 151 of 1405 frames — which is the digital-frame
+  exclusion #30 built and #35 had made unreachable from this source
diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md
index e776059..b7af8f7 100644
--- a/planning/active/task_plan.md
+++ b/planning/active/task_plan.md
@@ -83,16 +83,18 @@ reproduced before acting on it.
 
 ## Phase 5: Release v0.5.1
 
-- [ ] `NEWS.md` — name the class, the data source it breaks, and that geometry
+- [x] `NEWS.md` — name the class, the data source it breaks, and that geometry
       and downstream numbers were always correct (reporting loss only)
-- [ ] Bump `DESCRIPTION` to `0.5.1` as the **final** commit of the branch
-- [ ] Tag `v0.5.1`
+- [x] Bump `DESCRIPTION` to `0.5.1` as the **final** commit of the branch
+- [~] Tag `v0.5.1` — **deliberately not done on the branch.** `/gh-pr-merge`
+      does release bookkeeping after the merge, and a tag on a branch tip that
+      review may amend moves silently. Tag on `main`, after merge
 
 ## Validation
 
-- [ ] Tests pass
-- [ ] `/code-check` clean on each commit
-- [ ] End-to-end against a real `bcdata::collect()` result — the case the
+- [x] Tests pass
+- [x] `/code-check` round 1 run on the fix; 5 findings, all folded in
+- [x] End-to-end against a real `bcdata::collect()` result — the case the
       fixture structurally cannot reach
 - [ ] PWF checkboxes match landed work
 - [ ] `/planning-archive` on completion

From 24e52a1d57a20737119e7950c8510c4543534a75 Mon Sep 17 00:00:00 2001
From: almac2022 <al@newgraphenvironment.com>
Date: Sat, 29 Aug 2026 21:43:21 -0700
Subject: [PATCH 6/6] Archive PWF for #35

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR
---
 .../README.md                                 | 28 +++++++++++++++++++
 .../findings.md                               |  0
 .../progress.md                               |  0
 .../review-round1.md                          |  0
 .../task_plan.md                              |  0
 5 files changed, 28 insertions(+)
 create mode 100644 planning/archive/2026-08-issue-35-footprint-tibble-columns/README.md
 rename planning/{active => archive/2026-08-issue-35-footprint-tibble-columns}/findings.md (100%)
 rename planning/{active => archive/2026-08-issue-35-footprint-tibble-columns}/progress.md (100%)
 rename planning/{active => archive/2026-08-issue-35-footprint-tibble-columns}/review-round1.md (100%)
 rename planning/{active => archive/2026-08-issue-35-footprint-tibble-columns}/task_plan.md (100%)

diff --git a/planning/archive/2026-08-issue-35-footprint-tibble-columns/README.md b/planning/archive/2026-08-issue-35-footprint-tibble-columns/README.md
new file mode 100644
index 0000000..506b918
--- /dev/null
+++ b/planning/archive/2026-08-issue-35-footprint-tibble-columns/README.md
@@ -0,0 +1,28 @@
+# #35 — fly_footprint() dropped its reporting columns on tibble input
+
+**Outcome:** fixed and released as v0.5.1 in [PR #36](https://github.com/NewGraphEnvironment/fly/pull/36), which also closed #31.
+
+`sf::st_sf()` keeps only its first argument when that argument is a tibble, so the four
+reporting columns `fly_footprint()` attaches as trailing arguments were discarded for
+every `bcdata::collect()` caller — which is to say for the package's own documented data
+source. Nothing errored: geometry and every downstream number stayed correct and only the
+audit trail went missing, which is how it survived two releases. The fix assigns the
+columns onto the attribute frame before `st_sf()` sees it.
+
+The durable lesson is the fixture one, and it is the seventh instance in this package
+(see `inst/notes/terrain-correction.md` for the six from #9): **every fixture here reads
+back as plain `sf, data.frame`, so no case added along the existing axis could have found
+this.** The tests now sweep the input-class axis instead. Two further defects fell out of
+review — a colliding `footprint_basis` column that was never overwritten, so the
+documented filter read the caller's column instead of fly's answer; and `logical` columns
+on zero-row input, which broke binding an empty result to a populated one.
+
+Two claims of mine were falsified by review and corrected before merge: that the input
+class is *preserved* (the set survives, the order does not — `st_transform()` moves `sf`
+to the front), and that the overwrite change made `fly_footprint()` idempotent (it does
+not; a plain 20-row sf returns 100 rows silently).
+
+**Closing commits:** `a566074` (sweep), `ae628bc` (fix), `28c4f85` (#31 retitle), `3d902fa` (release).
+**Verified:** suite 225 → 338, `R CMD check` Status OK, and end to end against the live
+catalogue — 1405 real centroids, all four columns present, 151 frames excluded by the
+documented filter that were invisible before.
diff --git a/planning/active/findings.md b/planning/archive/2026-08-issue-35-footprint-tibble-columns/findings.md
similarity index 100%
rename from planning/active/findings.md
rename to planning/archive/2026-08-issue-35-footprint-tibble-columns/findings.md
diff --git a/planning/active/progress.md b/planning/archive/2026-08-issue-35-footprint-tibble-columns/progress.md
similarity index 100%
rename from planning/active/progress.md
rename to planning/archive/2026-08-issue-35-footprint-tibble-columns/progress.md
diff --git a/planning/active/review-round1.md b/planning/archive/2026-08-issue-35-footprint-tibble-columns/review-round1.md
similarity index 100%
rename from planning/active/review-round1.md
rename to planning/archive/2026-08-issue-35-footprint-tibble-columns/review-round1.md
diff --git a/planning/active/task_plan.md b/planning/archive/2026-08-issue-35-footprint-tibble-columns/task_plan.md
similarity index 100%
rename from planning/active/task_plan.md
rename to planning/archive/2026-08-issue-35-footprint-tibble-columns/task_plan.md