Build the frame before st_sf() sees it, so the reporting columns survive a tibble

[NewGraphEnvironment]

What

fly_footprint() silently dropped footprint_basis, footprint_terrain, height_agl and dem_coverage whenever its input carried 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 — lost the whole reporting surface 0.4.0 (#30) and 0.5.0 (#9) added.

Nothing errored. Geometry and every downstream number were correct; only the audit trail was gone, which is why two releases shipped with it.

Cause

sf::st_sf() builds its attribute frame with a branch that 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. A general R trap rather than a fly quirk. The fix assigns the four columns onto the attribute frame first, so they are inside x[[1]] where they survive, and the caller keeps the class they passed in.

Why the suite could not see it

Every fixture in the package reads back as plain sf, data.frame — the bundled GeoPackage, and both synthesized fixtures, which are built by st_sf() on plain vectors. No case added along the existing axis could have found this. Seventh instance of the pattern in inst/notes/terrain-correction.md.

So the tests sweep the input-class axis instead — plain / tibble / grouped / bcdc_sf — asserting identical names, identical values and identical geometry across shapes. The tibble shape is read honestly via st_read(as_tibble = TRUE) rather than by overwriting class(), and the premise is asserted inline so a future sf change fails by naming the real cause.

Two further defects the reviews surfaced

Both reproduced before acting, both now guarded:

• A colliding column was never overwritten. Where the input already carried footprint_basis, the old code appended a duplicate footprint_basis.1 and left the caller value under the documented name — so footprints$footprint_basis != "unknown_format", the filter the docs prescribe, read the caller's column while fly's own answer sat unread beside it. This hit plain sf callers too, who were never affected by the tibble bug.
• Zero-row input returned logical columns. ifelse(logical(0), ...) is logical(0), so an empty result would not bind_rows() onto a populated one — exactly the per-AOI ledger this reporting surface exists for (stac_airphoto_bc#16).

Corrections to my own claims

Two things I wrote and the reviews falsified:

• @return said "the input class is preserved". The class set survives; the order does not — st_transform() (not st_sf()) moves sf to the front, so bcdc_sf, sf, ... returns sf, bcdc_sf, .... An identical(class(out), class(in)) assertion would have passed on the three shapes I had and failed on the one caller the issue is about.
• A NEWS line claimed the overwrite change made fly_footprint(fly_footprint(x)) idempotent. It does not: a plain 20-row sf comes back with 100 rows, silently, because st_coordinates() on polygons yields 5 rows per feature. Pre-existing and unchanged, but the sentence invited the round trip. Cut.

Verification

• Suite 225 → 338, FAIL 0 | WARN 0 | SKIP 0
• R CMD check Status: OK (the only gate on the Title rules — this repo has no R-CMD-check workflow)
• Every new guard proven to fail against the real prior code from 8585fd5 — not a reconstruction — patching both asNamespace("fly") and the attached binding, with a proof-of-patch value printed before asserting
• lintr unchanged; NAMESPACE byte-identical at 9 exports
• End to end against the live catalogue, the case no fixture can reach:

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
documented filter: 1254 of 1405        # 151 frames excluded that were invisible before

Also

Widens the DESCRIPTION Title and Description, which described roughly half the package — they predated fly_fetch(), fly_georef() and fly_bearing().

Released as v0.5.1. Not tagged on the branch — /gh-pr-merge tags after merge, and a tag on a branch tip that review may amend moves silently.

Follow-up worth filing

The st_sf() trailing-column behaviour belongs in soul's conventions/code-check.md as the general rule — a constructor taking ... may discard trailing arguments depending on the class of the first one; pass a fully-built frame instead — with a sweep across the other NGE packages.

Closes #35
Closes #31

🤖 Generated with Claude Code

https://claude.ai/code/session_01GBKqedyBysV7hB4DuL98ZR