From dad1d39844b68cdb0074ba19e31d90ea34c813e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 13:46:18 +0000 Subject: [PATCH 1/6] osm bake: plan + measured results for the Brandenburg region bake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Berlin bake this session recovered a map that had drawn grey for a day, and the four failure modes it exposed are each invisible until they bite. This plan turns that run into a repeatable procedure with a gate per failure, and records what Brandenburg actually measured against it. The prediction was deliberately falsifiable, and it was falsified — in the direction the plan itself flagged as the live risk. Rows, slab bytes and books all landed UNDER the linear extrapolation from Berlin (0.82-0.87x) while chains landed 15% OVER: rural ways are longer with more nodes each, so row count scales sub-linearly against PBF bytes while per-way chain data scales super-linearly. A single "close enough" scalar would have hidden that the two quantities move in OPPOSITE directions. Consequence recorded for the next region: size the slab from rows-per-PBF-byte, size .chains from way geometry, and do not treat them as interchangeable. Measured: 7,330,219 rows, 3,753,072,128 B slab (3.50 GiB), digest 7bb1db0cc8794f79, 127.6 s, peak RSS 2.34 GiB, VERDICT PARITY with 6,732,666 tags exact / 0 bad and 590,437 junction rows / 0 bad. Memory was the flagged risk and did not bite. The tooling the plan references (fetch_pbf / validate_bake / publish_bake + README) is published to the bake bucket under q2/bakes/tools/, because the Berlin validation and publish steps were run INLINE and left nothing reproducible behind. The scripts are the steps that actually ran. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- .../plans/2026-08-13-brandenburg-bake.md | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 claude-notes/plans/2026-08-13-brandenburg-bake.md diff --git a/claude-notes/plans/2026-08-13-brandenburg-bake.md b/claude-notes/plans/2026-08-13-brandenburg-bake.md new file mode 100644 index 000000000..402151801 --- /dev/null +++ b/claude-notes/plans/2026-08-13-brandenburg-bake.md @@ -0,0 +1,187 @@ +# Brandenburg bake — plan + +**Status:** ready to execute · **PR: HOLD** (operator: open only after the next PR lands) +**Executor:** Sonnet (mechanical; every step below is a measured command, not a judgement call) + +## Overview + +Bake `brandenburg-latest.osm.pbf` into the V3 slab + sidecars, validate it the +same way the Berlin bake was validated on 2026-08-13, and publish it to +`q2/bakes/brandenburg-v1/`. Serving it is then `OSM_BAKE_REGION=brandenburg` +and a restart — **no code change**: `bake` is region-agnostic and the sidecars +resolve by extension from the slab's own stem. + +This plan exists because the Berlin bake this session exposed four failure +modes that are invisible until they bite. Each has a gate below. + +## Preconditions + +- [ ] **Disk.** Outputs are ~4.7 GB (extrapolated, see Sizing). Free space + first: `cargo clean` in `/home/user/q2` reclaims ~5.5 GB and is not + needed for the bake — the baker lives in `openstreetmap-website-rs`, + which is already built. Verify `df -h /` shows **≥ 8 GB** before starting. +- [ ] **Baker built:** `openstreetmap-website-rs/target/release/{bake,parity}`. + If absent: `cargo build --release --bin bake --bin parity` in that repo + (~31 s cold; it does *not* pull lance/datafusion). +- [ ] **S3 env present:** `AWS_S3_BUCKET_NAME`, `AWS_ACCESS_KEY_ID`, + `AWS_SECRET_ACCESS_KEY`, `AWS_ENDPOINT_URL`. Never print their values. + +## Sizing — extrapolated from Berlin, NOT measured + +Berlin measured this session: 98,840,376 B PBF → 2,766,291 rows → +1,416,340,992 B slab, 62.7 MB books, 63.9 MB chains, **38.9 s**. + +Brandenburg's PBF is 298,424,907 B = **3.02× Berlin**, so a linear read gives: + +| | extrapolated | +|---|---| +| rows | ~8.4 M | +| slab | ~4.3 GB | +| books | ~190 MB | +| chains | ~193 MB | +| bake wall time | ~2 min | + +**Treat these as predictions to be falsified, not facts.** Record the real +numbers in the Results section. Two reasons they may be wrong in either +direction: Brandenburg is rural, so its ways are longer with more nodes each +(rows could scale sub-linearly against PBF bytes while chains scale +super-linearly); and PBF size tracks features, not rows. + +**Memory is the under-considered risk.** `read_features_with_chains` holds a +`coords: HashMap` over *every* node — Berlin indexed 7.87 M, so +Brandenburg is ~24 M entries plus `way_chains`. Expect multiple GB of RSS. If +the bake is OOM-killed, that is the cause; it is not a disk problem and +retrying will not help. + +## Work items + +### 1. Reassemble the input +- [ ] Download `OSM/bb.part.00` … `bb.part.03` from the bucket. +- [ ] `cat bb.part.00 bb.part.01 bb.part.02 bb.part.03 > brandenburg-latest.osm.pbf` + (plain byte split — name order restores it exactly). +- [ ] **Gate:** `sha256sum -c` against + `0662b67825091986d45b8df070d1df43fa8048548e8defe680ec8df61fe1038c` + must print `OK`. Do not proceed on failure. +- [ ] Delete the four parts once verified (saves ~298 MB while baking). + +> Why from S3 and not a fresh download: an OSM `*-latest.osm.pbf` always +> serves *today's* snapshot, so the pinned bytes are **not re-downloadable**. +> This is the exact input the overflow-split rung was measured from. + +### 2. Bake +- [ ] `bake brandenburg-latest.osm.pbf brandenburg` + — output stem `brandenburg` (no extension) so the sidecars land as + `brandenburg.books` / `brandenburg.chains`, which is what + `with_extension()` resolves to on the serving side. Upload the stem file + under the key `brandenburg.soa`. +- [ ] Record the reported `slab digest`, `ROWS`, `bytes`, and timing. + +### 3. Validate — every gate is two-sided + +- [ ] **Codebook format.** `brandenburg.books[0..8] == b"OSMCBK\0\x03"`. + *This is the exact failure that took Berlin's map down for a day*: the + deployed codebook was `\x02` after `openstreetmap-website-rs@3142a8d` + bumped the format, so it was refused at the magic check before the + digest check ever ran, and every shape fell back to `ShapeClass::Other` + (grey map, HTTP 200, no error anywhere). +- [ ] **Digest agreement.** `books[24..32] == chains[8..16]` and both equal the + digest `bake` reported. Books/chains headers: books is + `magic(8) rows(8) slots(8) slab(8)`, chains is `magic(8) slab(8)`. +- [ ] **Row count.** `books.rows == slab_bytes / 512`, and `slab_bytes % 512 == 0`. +- [ ] **Parity.** `parity brandenburg-latest.osm.pbf brandenburg brandenburg.books` + must print `VERDICT PARITY` with **`tags exact … bad 0`** and + `NOT recovered 0`. This is the gate that actually proves the codebook + resolves; the header checks only prove it is readable. + +### 4. Publish +- [ ] Archive anything already at `q2/bakes/brandenburg-v1/` to + `q2/bakes/archive/brandenburg-v1-/` by **server-side copy** + (`s3.copy`, no download). Copy — do not delete — so there is never a + window without a bake. +- [ ] Generate `SHA256SUMS` in `sha256sum` format, naming the slab + `brandenburg.soa` (not the local stem). +- [ ] Upload **artifacts first, `SHA256SUMS` LAST.** `fetch_sums` gates every + artifact: new files under old sums fail `download_verified`, which + "leaves no file behind" — i.e. a total outage, silently. +- [ ] Verify from the bucket: re-read the headers by ranged GET, confirm sizes + match local and `SHA256SUMS` matches what was uploaded. + +### 5. Roll out (operator) +- [ ] Set `OSM_BAKE_REGION=brandenburg` and restart cockpit-server. +- [ ] **Gate:** `GET /api/osm/health` → `books.loaded: true` and + `styling: "ok: …"`. If false, the `styling` line names which of the four + causes it is — do not guess. +- [ ] **Gate:** a tile's class histogram shows real classes, not + `{"other": N}`. + +## Risks and the traps already paid for + +1. **`OSM_SLAB_PATH` short-circuits hydration.** `ensure_slab_local` returns + early if that env var is set and the file exists — before any S3 or + checksum logic. If it is set in the deploy, a restart will reuse the stale + region's slab forever. Unset it, or clear the volume. +2. **A restart is mandatory.** `open_books()` caches in a `OnceLock` and + hydration runs at boot; uploading changes nothing until the process + restarts. +3. **Cold-boot delay.** Hydration blocks the listener bind. Berlin's 1.42 GB + took long enough to serve 502s; ~4.7 GB will take proportionally longer. + Expect a visible outage window on the first boot after the switch — this is + the hydrator working, not failing. +4. **Both regions cannot be served at once.** `OSM_BAKE_REGION` is a single + value. Serving Brandenburg replaces Berlin; it does not add to it. + +## Results — executed 2026-08-13 + +| | predicted | measured | ratio | +|---|---|---|---| +| rows | ~8.4 M | **7,330,219** | 0.87× | +| slab bytes | ~4.3 GB | **3,753,072,128** (3.50 GiB) | 0.87× | +| books | ~190 MB | **155,979,619** | 0.82× | +| chains | ~193 MB | **221,252,224** | **1.15×** | +| slab digest | — | `7bb1db0cc8794f79` | — | +| bake wall time | ~2 min | **127.6 s** | 1.06× | +| parity verdict | `PARITY` | **`VERDICT PARITY`** | ✓ | +| peak RSS | unknown | **2.34 GiB** | — | + +Parity detail: `tags exact 6,732,666 bad 0`, `NOT recovered 0`, `unknown to +source 0`, `kind mismatches 0`, node/derived position `bad 0`, junction rows +`590,437 ok / 0 bad`. + +### The prediction was falsified, in the direction this plan flagged + +Rows, slab bytes and books all landed **under** the linear extrapolation +(0.82–0.87×) while chains landed **15% over** — the one metric that moved the +opposite way. That is exactly the Sizing section's stated hypothesis: rural +ways are longer with more nodes each, so **row count scales sub-linearly** +against PBF bytes while **per-way chain data scales super-linearly**. Reading +one number as "close enough" would have hidden that the two quantities move in +opposite directions; only the paired prediction exposes it. + +Practical consequence for the next region: size the slab from a rows-per-PBF- +byte ratio, but size `.chains` from way *geometry*, not from PBF bytes. The two +are not interchangeable and a single scalar cannot carry both. + +**Memory was the flagged risk and it did not bite:** 2.34 GiB peak against a +worry of "multiple GB", with ~24 M nodes indexed. No OOM. + +### Notes from the run + +- Reassembly sha256 gate passed before any bake work began. +- `q2/bakes/brandenburg-v1/` was empty, so the archive-copy step was a + correct no-op. `berlin-v1/` was listed for visibility only — never copied, + modified or deleted, and confirmed intact afterwards. +- Upload order held: three artifacts, then `SHA256SUMS`. +- All four objects were re-verified by ranged GET **from the bucket** (magic + bytes, digest agreement, sizes, sums content) rather than by comparing to + what was uploaded — the distinction that catches a bad transfer. +- Rollout (step 5) deliberately untouched; it is the operator's call. +- The run used download+`cat`, not the streaming fetch: the parts were already + verified and deleted before that guidance landed. **Next bake should use + `fetch_pbf.py`** — peak overhead ~8 MB instead of holding the input twice. + +## References + +- Berlin bake + the codebook-format outage: q2 PR #127, this session. +- Reassembly + provenance: `OSM/REASSEMBLE.md` in the bake bucket. +- Region is config, not code: `osm_slab_hydrate::{bake_region, artifacts, + default_prefix}`. From bcab12c5954cd04699550c588e743e618f0701e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 10:40:18 +0000 Subject: [PATCH 2/6] osm lance: degrade instead of panicking past Arrow's i32 array ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brandenburg (7,330,219 rows) crash-looped the service at startup: FixedSizeBinaryArray error: value size 512 * length 7330219 exceeds maximum valid offset of 2147483647 `FixedSizeBinaryArray` holds the whole row column in ONE flat Buffer, and Arrow's classic (non-Large) array format bounds that buffer to i32::MAX bytes. At our 512-byte stride that is a hard ceiling of 4,194,303 rows — not a tunable. Berlin's 2,766,291 rows sat under it, so this never fired before; Brandenburg is 1.75x over. The crash was avoidable and contradicted this module's own design. Every other failure here returns None, `locate_row_column`'s doc says plainly "None on any doubt — this path is a pure optimization ... never a hard requirement", and main.rs's caller already has a safe None arm ("the raw .soa slab keeps serving the map"). But the array was built with `FixedSizeBinaryArray::new`, which is `try_new(..).unwrap()`, so this one failure mode panicked past a fallback everything else already used. Fix: check the row-count bound BEFORE construction and return None, and switch `new` -> `try_new` as defense-in-depth so no other Arrow validation failure can panic either. Oversized regions simply skip the Lance mmap-offset optimization and serve from the raw .soa slab — slower, fully correct, already the proven path. Splitting into multiple Arrow batches was considered and rejected: the read path requires the row column to be ONE contiguous run in ONE data file (`locate_row_column` checks 1 and 4 — the tail anchor exists BECAUSE a fragmented layout is unsafe to address by raw offset). That is a real design change, not an incident fix. Three regression tests, including one that reproduces the exact rows=7_330_219 from the incident against a tiny synthetic buffer (the guard runs before the bytes are used, so no multi-GB fixture is needed), plus a two-sided test proving ordinary row counts still build a valid array — a guard that fired unconditionally would pass the first test for the wrong reason. Verified: cargo check -p cockpit-server. The full test-binary link could not be completed here — this crate's lance/datafusion/arrow closure exhausted container disk twice mid-build (two separate critical-disk incidents, ~13 GiB target/). Shipping regardless because the change's worst case is the fallback it already selects, and production is crash-looping now. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- crates/cockpit-server/src/osm_lance.rs | 110 ++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 2 deletions(-) diff --git a/crates/cockpit-server/src/osm_lance.rs b/crates/cockpit-server/src/osm_lance.rs index eb3cb21b5..bdecbf9ed 100644 --- a/crates/cockpit-server/src/osm_lance.rs +++ b/crates/cockpit-server/src/osm_lance.rs @@ -281,16 +281,65 @@ async fn write_lance( ]); let schema = Arc::new(Schema::new_with_metadata(vec![field], schema_meta)); + // `FixedSizeBinaryArray` has no offsets buffer — the whole column is one + // flat `Buffer` addressed as `stride * index` — but Arrow still bounds + // that buffer's byte length to `i32::MAX` (the classic, non-Large Arrow + // array contract). This is a hard ceiling on ROW COUNT, not a tunable: + // for our 512-byte stride that's 4,194,303 rows. Brandenburg's 7,330,219 + // rows is the first bake to cross it — `FixedSizeBinaryArray::new` + // panics inside Arrow's own validation, and unlike every OTHER failure + // in this function, a `.unwrap()` panic here previously took the whole + // process down at startup rather than degrading. That contradicted this + // module's own stated design: `locate_row_column`'s doc says plainly + // "`None` on any doubt — this path is a pure optimization... never a + // hard requirement", and `main.rs`'s caller already has a fully safe + // `None` arm — "the raw .soa slab keeps serving the map" — for exactly + // this situation. So the fix is to reach that existing, proven fallback + // instead of panicking past it: check the bound BEFORE construction and + // return `None`, same as every `Err(e) => { error!(...); return None }` + // arm elsewhere in this function. + // + // Splitting into multiple Arrow batches was considered and rejected: the + // read path's mmap+offset serving requires the row column to be ONE + // contiguous byte run in ONE data file (`locate_row_column`'s own doc, + // check 1 and check 4 — the tail-anchor verification exists BECAUSE a + // fragmented layout is unsafe to address by raw offset). Multiple + // batches risk landing non-contiguously on Lance's own disk layout, + // which the tail anchor would then catch — but only after paying a + // real design/verification cost this incident's urgency doesn't afford. + // Skipping the optimization for oversized regions is strictly safer and + // already fully supported. + let max_rows_per_array = usize::try_from(i32::MAX).unwrap_or(usize::MAX) / NODE_ROW_STRIDE; + if rows > max_rows_per_array { + tracing::warn!( + rows, max_rows_per_array, stride = NODE_ROW_STRIDE, + "osm lance: row count exceeds FixedSizeBinaryArray's i32 byte-length ceiling \ + (Arrow's classic-array limit, not tunable); skipping the Lance mmap-offset \ + optimization — the raw .soa slab keeps serving the map unaffected" + ); + return None; + } + // Zero-copy import: `Buffer::from_vec` ADOPTS this allocation rather // than copying it — asserted by pointer identity, not merely trusted // (same discipline `soa_to_lance` uses; a 1+ GiB slab makes a silent // copy here a real cost, not a rounding error). let before = bytes.as_ptr(); - let array = FixedSizeBinaryArray::new( + let array = match FixedSizeBinaryArray::try_new( NODE_ROW_STRIDE as i32, arrow::buffer::Buffer::from_vec(bytes), None, - ); + ) { + Ok(a) => a, + Err(e) => { + // Defense-in-depth below the row-count bound checked above: ANY + // other Arrow validation failure degrades the same way, never a + // panic. `try_new`, never `new` — `new` is `try_new(..).unwrap()` + // and this function exists specifically to stop doing that. + tracing::error!(error = %e, "osm lance: building the row array failed"); + return None; + } + }; debug_assert_eq!( array.value_data().as_ptr(), before, @@ -489,6 +538,63 @@ mod tests { assert!(reopen_if_warm(&dest, 10, "deadbeef").await.is_none()); } + /// The exact boundary this incident crossed: Brandenburg's real + /// `rows=7_330_219` PANICKED the process at startup + /// (`arrow-array-58.3.0/src/array/fixed_size_binary_array.rs:106`, + /// `value size 512 * length 7330219 exceeds maximum valid offset of + /// 2147483647`) instead of returning `None`, even though the caller in + /// `main.rs` already has a fully safe `None` arm ("the raw .soa slab + /// keeps serving the map"). This pins the row-count math independently + /// of ever needing a multi-gigabyte test fixture: the guard runs BEFORE + /// `bytes` is used to build the array, so a short, mismatched-length + /// buffer exercises exactly the same code path the real 3.75 GB slab + /// would have hit, at zero fixture cost. + #[tokio::test] + async fn write_lance_degrades_instead_of_panicking_past_the_arrow_i32_ceiling() { + let dir = tempfile::tempdir().expect("tempdir"); + let dest = dir.path().join("brandenburg.lance"); + let slab_path = dir.path().join("brandenburg.soa"); + std::fs::write(&slab_path, [0u8; NODE_ROW_STRIDE]).expect("write stub slab"); + + let real_incident_rows = 7_330_219usize; + let got = write_lance(&dest, vec![0u8; 4], real_incident_rows, "deadbeef", &slab_path).await; + assert!( + got.is_none(), + "must return None, not panic, at the exact row count that crashed production" + ); + assert!(!dest.exists(), "a declined write must leave no partial dataset behind"); + } + + /// The two-sided half of the boundary: this module exists to let VALID + /// bakes through, not merely to reject oversized ones. A guard that + /// fires unconditionally would pass the test above for the wrong + /// reason — this proves rows comfortably under the ceiling still reach + /// (and clear) Arrow's own array construction. + #[test] + fn a_row_count_under_the_ceiling_builds_a_valid_array() { + let rows = 1_000usize; // Berlin-scale, nowhere near the 4.19M cap + let bytes = vec![0u8; rows * NODE_ROW_STRIDE]; + let array = FixedSizeBinaryArray::try_new( + NODE_ROW_STRIDE as i32, + arrow::buffer::Buffer::from_vec(bytes), + None, + ); + assert!(array.is_ok(), "an ordinary row count must not be rejected"); + assert_eq!(array.unwrap().len(), rows); + } + + /// The ceiling itself, computed the same way the guard computes it — + /// independent confirmation of the 4,194,303 figure cited in the fix's + /// doc comment, so a future stride change can't silently drift the two + /// out of sync. + #[test] + fn the_row_ceiling_matches_i32_max_over_the_stride() { + let max_rows_per_array = usize::try_from(i32::MAX).unwrap() / NODE_ROW_STRIDE; + assert_eq!(max_rows_per_array, 4_194_303); + assert!(7_330_219 > max_rows_per_array, "Brandenburg's real row count must exceed it"); + assert!(2_766_291 < max_rows_per_array, "Berlin's real row count must clear it"); + } + /// The falsifier for [`search_head_for_probe`]: a real hit, surrounded /// by bytes that must NOT be mistaken for it. Distinct leading/trailing /// padding — same byte would let a false match at the wrong offset From bd7e08ea000347abfcffd66b2f68ac23ed3d4219 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 10:51:53 +0000 Subject: [PATCH 3/6] osm lance: check the row ceiling before the slab is read, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review (Codex P2, CodeRabbit Major) caught that bcab12c fixed the panic but positioned the guard wrong. `rows` is known in `ensure_lance_local` at line 124; the guard sat ~165 lines downstream in `write_lance`, so an oversized region reached it only AFTER: 1. `remove_stale_dataset` deleted a perfectly usable dataset, and 2. `std::fs::read` allocated and read the whole 3.75 GB slab, before declining on a decision that needed only a row count. That traded a panic-loop for an OOM risk plus a guaranteed-useless multi-gigabyte read on every boot, forever — and destroyed a working dataset on the way. It stopped the crash, but "degrade cheaply" was the claim and it didn't. Moved the check to immediately after `rows` is computed, and removed the downstream duplicate rather than keeping the same arithmetic in two places where it can drift. `try_new` stays as the backstop for other Arrow validation failures. Extracted `arrow_max_rows_per_array()` / `row_count_fits_arrow_array()` so production and tests share ONE predicate. The previous ceiling test re-derived `i32::MAX / STRIDE` and asserted it equalled 4,194,303, which proves the arithmetic agrees with itself and nothing about the shipped guard — deleting the guard entirely would have left it green. It now calls the real predicate at exactly 4,194,303 (accept) and 4,194,304 (reject), plus both real regions on their measured sides. The Brandenburg regression test was also structurally unable to catch this defect: it called `write_lance` directly, bypassing the guard whose position was wrong, and passed `vec![0u8; 4]` against a 512-byte stride — so it could return `None` via the array-construction error path even with the row guard absent. It now drives `ensure_lance_local` (the real entry point) with a sparse file at Brandenburg's exact byte length (3.75 GB apparent, ~zero real disk) and asserts a pre-existing dataset SURVIVES, which is the destructive half a row-count-only assertion is blind to. Also fixes the publication plan (CodeRabbit Major, independent finding). It said "artifacts first, SHA256SUMS last", reasoning that sums-last minimises the bad window — but the step's own note already recorded why that is not enough: `fetch_sums` gates every artifact, and `download_verified` "leaves no file behind" on mismatch, so for the whole upload the live prefix serves new artifacts under an old manifest and any boot in that window finds no slab at all. Ordering shrinks the window; only staging removes it. Rewritten to stage under an immutable dated prefix, verify THAT prefix from the bucket, then cut over atomically via `OSM_SLAB_S3_PREFIX`, retaining the previous prefix for rollback. Verified: cargo check -p cockpit-server was in flight at commit time and the full test link remains unavailable here (this crate's lance/datafusion/arrow closure exhausted container disk twice). CI cannot cover it either: test-suite.yml and hub-client-e2e.yml both carry `if: github.repository == 'quarto-dev/q2'`, so every job skips on this fork. Shipping regardless because the currently-deployed commit is actively deleting datasets and reading 3.75 GB per boot, and these changes only reduce what it does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- .../plans/2026-08-13-brandenburg-bake.md | 42 +++-- crates/cockpit-server/src/osm_lance.rs | 168 +++++++++++++----- 2 files changed, 150 insertions(+), 60 deletions(-) diff --git a/claude-notes/plans/2026-08-13-brandenburg-bake.md b/claude-notes/plans/2026-08-13-brandenburg-bake.md index 402151801..90fbcf923 100644 --- a/claude-notes/plans/2026-08-13-brandenburg-bake.md +++ b/claude-notes/plans/2026-08-13-brandenburg-bake.md @@ -93,18 +93,36 @@ retrying will not help. `NOT recovered 0`. This is the gate that actually proves the codebook resolves; the header checks only prove it is readable. -### 4. Publish -- [ ] Archive anything already at `q2/bakes/brandenburg-v1/` to - `q2/bakes/archive/brandenburg-v1-/` by **server-side copy** - (`s3.copy`, no download). Copy — do not delete — so there is never a - window without a bake. -- [ ] Generate `SHA256SUMS` in `sha256sum` format, naming the slab - `brandenburg.soa` (not the local stem). -- [ ] Upload **artifacts first, `SHA256SUMS` LAST.** `fetch_sums` gates every - artifact: new files under old sums fail `download_verified`, which - "leaves no file behind" — i.e. a total outage, silently. -- [ ] Verify from the bucket: re-read the headers by ranged GET, confirm sizes - match local and `SHA256SUMS` matches what was uploaded. +### 4. Publish — stage, verify, then cut over + +**Never upload in place over a live prefix.** An earlier version of this plan +said "artifacts first, `SHA256SUMS` last", reasoning that sums-last minimises +the bad window. It does not close it. For the whole upload the live prefix +holds new artifacts under the *old* manifest, and `fetch_sums` gates every +artifact against it: `download_verified` rejects each mismatch and **leaves no +file behind**. Any boot in that window therefore finds no slab at all — a +total outage, silently, from a step whose own note already said so. Ordering +makes the window smaller; only staging removes it. + +- [ ] Upload the three artifacts **and** `SHA256SUMS` to an immutable, dated + staging prefix — `q2/bakes/brandenburg-v1-/`. Nothing + reads it yet, so a partial upload here is inert rather than an outage. +- [ ] Verify the STAGED prefix from the bucket, not from local state: ranged + GET the books/chains headers, confirm the magic, confirm both digests + agree, confirm sizes, and confirm `SHA256SUMS` content matches. (This + catches a bad transfer; comparing to what you uploaded does not.) +- [ ] **Cut over only after the staged prefix verifies**, by pointing the + runtime at it (`OSM_SLAB_S3_PREFIX`) — one atomic change, not a + multi-object mutation. There is no moment where a reader sees a + half-published prefix. +- [ ] **Keep the previous prefix** for rollback; it stays complete and + self-consistent throughout, so reverting is the same one-value change. + +> If a same-prefix publish is ever genuinely unavoidable, the ONLY safe order +> is: delete `SHA256SUMS` first (readers then fail closed on a missing +> manifest and keep their warm volume), upload artifacts, upload sums last. +> Staging is still preferable — it never makes the live prefix unreadable at +> all. ### 5. Roll out (operator) - [ ] Set `OSM_BAKE_REGION=brandenburg` and restart cockpit-server. diff --git a/crates/cockpit-server/src/osm_lance.rs b/crates/cockpit-server/src/osm_lance.rs index bdecbf9ed..d472b168e 100644 --- a/crates/cockpit-server/src/osm_lance.rs +++ b/crates/cockpit-server/src/osm_lance.rs @@ -125,6 +125,35 @@ pub async fn ensure_lance_local(slab_path: &Path) -> Option { let digest_hex = format!("{:016x}", osm_soa_bake::codebook::hash_slab(&map)); drop(map); + // The Arrow ceiling is decided by `rows` ALONE, so decide it here — the + // earliest point `rows` exists — rather than downstream in `write_lance`. + // + // `FixedSizeBinaryArray` holds the whole row column in one flat `Buffer`, + // and Arrow's classic (non-`Large`) array format bounds that buffer to + // `i32::MAX` bytes. At our 512-byte stride that is a hard ceiling of + // 4,194,303 rows — not a tunable. Berlin's 2,766,291 rows sit under it; + // Brandenburg's 7,330,219 are 1.75x over, which panicked startup in a + // crash-loop until this guard existed. + // + // Position matters as much as the check. Placed downstream, this guard + // was reached only AFTER `remove_stale_dataset` had deleted a perfectly + // good existing dataset and after `std::fs::read` had allocated the whole + // 3.75 GB slab — every boot, forever, to reach a decision that needed + // only a row count. That trades a panic for an OOM risk plus a + // guaranteed-useless multi-gigabyte read, which is not a fix. Deciding + // before both is what makes the fallback actually cheap. + if !row_count_fits_arrow_array(rows) { + tracing::warn!( + rows, + max_rows_per_array = arrow_max_rows_per_array(), + stride = NODE_ROW_STRIDE, + "osm lance: row count exceeds FixedSizeBinaryArray's i32 byte-length ceiling \ + (Arrow's classic-array limit, not tunable); skipping the Lance mmap-offset \ + optimization without reading the slab — the raw .soa slab keeps serving the map" + ); + return None; + } + if let Some(warm) = reopen_if_warm(&dest, rows, &digest_hex).await { tracing::info!( path = %dest.display(), rows, @@ -173,6 +202,31 @@ pub async fn ensure_lance_local(slab_path: &Path) -> Option { write_lance(&dest, bytes, rows, &digest_hex, slab_path).await } +/// Most rows one `FixedSizeBinaryArray` can hold at [`NODE_ROW_STRIDE`]. +/// +/// The array has no offsets buffer — the whole column is one flat `Buffer` +/// addressed as `stride * index` — but Arrow's classic (non-`Large`) format +/// still bounds that buffer's byte length to `i32::MAX`. At our 512-byte +/// stride that is **4,194,303 rows**, and it is a property of the format, not +/// a tunable. +#[must_use] +fn arrow_max_rows_per_array() -> usize { + usize::try_from(i32::MAX).unwrap_or(usize::MAX) / NODE_ROW_STRIDE +} + +/// Whether `rows` can be built as a single Arrow row array. +/// +/// Extracted so the production guard and its tests are the SAME predicate. +/// The first version of this fix inlined the comparison and tested it by +/// re-deriving the formula in the test — which proves the arithmetic agrees +/// with itself, not that the shipped code rejects the row count that crashed +/// production. A test that cannot fail when the guard is deleted is not a +/// test of the guard. +#[must_use] +fn row_count_fits_arrow_array(rows: usize) -> bool { + rows <= arrow_max_rows_per_array() +} + /// Remove a stale dataset directory in full, so the follow-up write starts /// from nothing and `data/` ends up holding EXACTLY the one fragment file /// the write produces. Split out for the falsifier below; guarded at the @@ -281,23 +335,11 @@ async fn write_lance( ]); let schema = Arc::new(Schema::new_with_metadata(vec![field], schema_meta)); - // `FixedSizeBinaryArray` has no offsets buffer — the whole column is one - // flat `Buffer` addressed as `stride * index` — but Arrow still bounds - // that buffer's byte length to `i32::MAX` (the classic, non-Large Arrow - // array contract). This is a hard ceiling on ROW COUNT, not a tunable: - // for our 512-byte stride that's 4,194,303 rows. Brandenburg's 7,330,219 - // rows is the first bake to cross it — `FixedSizeBinaryArray::new` - // panics inside Arrow's own validation, and unlike every OTHER failure - // in this function, a `.unwrap()` panic here previously took the whole - // process down at startup rather than degrading. That contradicted this - // module's own stated design: `locate_row_column`'s doc says plainly - // "`None` on any doubt — this path is a pure optimization... never a - // hard requirement", and `main.rs`'s caller already has a fully safe - // `None` arm — "the raw .soa slab keeps serving the map" — for exactly - // this situation. So the fix is to reach that existing, proven fallback - // instead of panicking past it: check the bound BEFORE construction and - // return `None`, same as every `Err(e) => { error!(...); return None }` - // arm elsewhere in this function. + // The Arrow i32 row ceiling is enforced by `ensure_lance_local` BEFORE + // the slab is read, so an oversized region never reaches this function. + // The check is not repeated here: two copies of the same arithmetic in + // two places is how they drift apart. `try_new` below is the backstop + // that keeps this function honest regardless. // // Splitting into multiple Arrow batches was considered and rejected: the // read path's mmap+offset serving requires the row column to be ONE @@ -309,16 +351,6 @@ async fn write_lance( // real design/verification cost this incident's urgency doesn't afford. // Skipping the optimization for oversized regions is strictly safer and // already fully supported. - let max_rows_per_array = usize::try_from(i32::MAX).unwrap_or(usize::MAX) / NODE_ROW_STRIDE; - if rows > max_rows_per_array { - tracing::warn!( - rows, max_rows_per_array, stride = NODE_ROW_STRIDE, - "osm lance: row count exceeds FixedSizeBinaryArray's i32 byte-length ceiling \ - (Arrow's classic-array limit, not tunable); skipping the Lance mmap-offset \ - optimization — the raw .soa slab keeps serving the map unaffected" - ); - return None; - } // Zero-copy import: `Buffer::from_vec` ADOPTS this allocation rather // than copying it — asserted by pointer identity, not merely trusted @@ -544,25 +576,48 @@ mod tests { /// `value size 512 * length 7330219 exceeds maximum valid offset of /// 2147483647`) instead of returning `None`, even though the caller in /// `main.rs` already has a fully safe `None` arm ("the raw .soa slab - /// keeps serving the map"). This pins the row-count math independently - /// of ever needing a multi-gigabyte test fixture: the guard runs BEFORE - /// `bytes` is used to build the array, so a short, mismatched-length - /// buffer exercises exactly the same code path the real 3.75 GB slab - /// would have hit, at zero fixture cost. + /// keeps serving the map"). + /// + /// Drives `ensure_lance_local` — the real entry point — NOT `write_lance`. + /// The first version of this test called `write_lance` directly, which + /// would now pass while proving nothing: the guard that matters lives + /// upstream, and a test that skips it cannot catch the guard being + /// mispositioned (which is exactly the defect review caught). + /// + /// Uses a real oversized slab file rather than a stub, because the + /// property under test is that the ceiling is decided from the row count + /// ALONE. A sparse file makes 3.75 GB of apparent length cost no real + /// disk, so this stays cheap while remaining a genuine end-to-end check. #[tokio::test] - async fn write_lance_degrades_instead_of_panicking_past_the_arrow_i32_ceiling() { + async fn oversized_slab_declines_without_reading_or_destroying_anything() { let dir = tempfile::tempdir().expect("tempdir"); - let dest = dir.path().join("brandenburg.lance"); let slab_path = dir.path().join("brandenburg.soa"); - std::fs::write(&slab_path, [0u8; NODE_ROW_STRIDE]).expect("write stub slab"); - let real_incident_rows = 7_330_219usize; - let got = write_lance(&dest, vec![0u8; 4], real_incident_rows, "deadbeef", &slab_path).await; + // Sparse file: Brandenburg's exact byte length, ~no blocks allocated. + let real_incident_rows = 7_330_219u64; + let len = real_incident_rows * NODE_ROW_STRIDE as u64; + let f = std::fs::File::create(&slab_path).expect("create slab"); + f.set_len(len).expect("set sparse length"); + drop(f); + + // A pre-existing dataset dir that MUST survive: the mispositioned + // guard deleted this before declining, which is the destructive half + // of the defect and is invisible to a row-count-only assertion. + let dest = slab_path.with_extension("lance"); + std::fs::create_dir_all(dest.join("data")).expect("mkdir dataset"); + std::fs::write(dest.join("data").join("keepme.lance"), b"pre-existing") + .expect("write sentinel"); + + let got = ensure_lance_local(&slab_path).await; assert!( got.is_none(), - "must return None, not panic, at the exact row count that crashed production" + "must return None, not panic, at the row count that crashed production" + ); + assert!( + dest.join("data").join("keepme.lance").exists(), + "declining must not delete an existing dataset — the guard has to run \ + before remove_stale_dataset, not after it" ); - assert!(!dest.exists(), "a declined write must leave no partial dataset behind"); } /// The two-sided half of the boundary: this module exists to let VALID @@ -583,16 +638,33 @@ mod tests { assert_eq!(array.unwrap().len(), rows); } - /// The ceiling itself, computed the same way the guard computes it — - /// independent confirmation of the 4,194,303 figure cited in the fix's - /// doc comment, so a future stride change can't silently drift the two - /// out of sync. + /// Both sides of the ceiling, through the PRODUCTION predicate rather + /// than a re-derivation of its formula. + /// + /// The earlier version of this test recomputed `i32::MAX / STRIDE` and + /// asserted the result equalled 4,194,303 — which proves the arithmetic + /// agrees with itself and nothing about the shipped guard. Deleting the + /// guard entirely would have left it green. This calls + /// `row_count_fits_arrow_array`, so it fails if the real predicate ever + /// stops rejecting oversized counts, and it costs no allocation. #[test] - fn the_row_ceiling_matches_i32_max_over_the_stride() { - let max_rows_per_array = usize::try_from(i32::MAX).unwrap() / NODE_ROW_STRIDE; - assert_eq!(max_rows_per_array, 4_194_303); - assert!(7_330_219 > max_rows_per_array, "Brandenburg's real row count must exceed it"); - assert!(2_766_291 < max_rows_per_array, "Berlin's real row count must clear it"); + fn the_row_ceiling_predicate_accepts_the_last_row_and_rejects_the_first_over() { + let max = arrow_max_rows_per_array(); + assert_eq!(max, 4_194_303, "512-byte stride under Arrow's i32 byte ceiling"); + + assert!(row_count_fits_arrow_array(max), "the last fitting row count must be accepted"); + assert!( + !row_count_fits_arrow_array(max + 1), + "one row past the ceiling must be rejected — this is the exact boundary, \ + and an off-by-one here is a panic in production" + ); + + // The two real regions, on the sides the incident put them. + assert!(row_count_fits_arrow_array(2_766_291), "Berlin must still take the fast path"); + assert!( + !row_count_fits_arrow_array(7_330_219), + "Brandenburg's real row count is what crashed production" + ); } /// The falsifier for [`search_head_for_probe`]: a real hit, surrounded From 32a97afb2b940f886144e509c0446a2a6e84c657 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 11:09:17 +0000 Subject: [PATCH 4/6] osm bake: record the Arrow row-ceiling crash in the Brandenburg plan The plan's rollout section promised a restart and two health gates. What actually happened on `OSM_BAKE_REGION=brandenburg` was a startup crash-loop, and nothing in the document would have predicted it or explained it afterwards. The cause is a format constant, not a bake defect: `FixedSizeBinaryArray` holds the whole row column in one flat `Buffer`, Arrow bounds that buffer to `i32::MAX` bytes, and at a 512-byte stride that caps a region at 4,194,303 rows. Berlin's 2,766,291 fit; Brandenburg's 7,330,219 are 1.75x over. Records the ceiling with both regions measured against it, the fix (PR #129), and the two consequences an operator needs and could not otherwise know: Brandenburg serves correctly but WITHOUT the mmap+offset fast path, and any future region past ~4.19M rows will do the same until multi-batch Lance writes land. The test for the next region is one comparison against a number `bake` already reports. Also points the Publish section at the staged `publish_bake.py` now in the bucket, so the tooling and the plan no longer disagree about how to publish. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- .../plans/2026-08-13-brandenburg-bake.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/claude-notes/plans/2026-08-13-brandenburg-bake.md b/claude-notes/plans/2026-08-13-brandenburg-bake.md index 90fbcf923..7b6d2fe1e 100644 --- a/claude-notes/plans/2026-08-13-brandenburg-bake.md +++ b/claude-notes/plans/2026-08-13-brandenburg-bake.md @@ -118,6 +118,14 @@ makes the window smaller; only staging removes it. - [ ] **Keep the previous prefix** for rollback; it stays complete and self-consistent throughout, so reverting is the same one-value change. +> **Tooling.** `q2/bakes/tools/publish_bake.py` in the bake bucket now +> implements exactly this: it stages to `q2/bakes/-v1-/`, +> refuses to reuse a stamp, verifies the staged prefix **by reading it back +> from the bucket**, and prints the one `OSM_SLAB_S3_PREFIX=…` value to cut +> over. The previous in-place version is kept at +> `q2/bakes/tools/archive/publish_bake.in-place.py` — it is the script that +> produced the hazard described above, retained for provenance, not for use. + > If a same-prefix publish is ever genuinely unavoidable, the ONLY safe order > is: delete `SHA256SUMS` first (readers then fail closed on a missing > manifest and keep their warm volume), upload artifacts, upload sums last. @@ -147,6 +155,49 @@ makes the window smaller; only staging removes it. the hydrator working, not failing. 4. **Both regions cannot be served at once.** `OSM_BAKE_REGION` is a single value. Serving Brandenburg replaces Berlin; it does not add to it. +5. **Brandenburg is past Arrow's row ceiling, so it serves from the raw + `.soa`, not from Lance.** This bit during rollout — see below. + +## The rollout crash — Arrow's i32 array ceiling (measured 2026-08-14) + +Setting `OSM_BAKE_REGION=brandenburg` put the service in a **startup +crash-loop**. Not a bake defect: every artifact verified, and the panic was +downstream of hydration in the Lance conversion. + +`osm_lance` converts the slab into a one-column Lance dataset so tiles can be +served by mmap+offset. That column is a `FixedSizeBinaryArray`, which holds +every row in ONE flat `Buffer`, and Arrow's classic (non-`Large`) array format +bounds a buffer to `i32::MAX` bytes. At our 512-byte stride that is a hard +ceiling: + +| | rows | vs ceiling | +|---|---|---| +| ceiling (`i32::MAX / 512`) | 4,194,303 | — | +| Berlin | 2,766,291 | 0.66× — fits | +| **Brandenburg** | **7,330,219** | **1.75× — panics** | + +It is a **format constant, not a tunable**. Arrow's own validation panicked, +and the `.unwrap()` took the process down at boot — contradicting this module's +own design, where the Lance path is explicitly "a pure optimization... never a +hard requirement" and the caller already has a safe `None` arm. + +Fixed in q2 PR #129: the row count is checked immediately after it is +computed — before a stale dataset is removed and before the 3.75 GB +`std::fs::read` — and an oversized region returns `None` to the **existing** +raw-`.soa` fallback. Position was as load-bearing as the check: two reviewers +independently caught that a downstream guard would delete a usable dataset and +read gigabytes to reach a decision that needs only a row count. + +**Operator consequence:** Brandenburg serves correctly but **without the +mmap+offset fast path**. That is a performance difference, not a correctness +one. Restoring it needs multi-batch Lance writes whose row column stays ONE +contiguous run in ONE data file — `locate_row_column`'s checks 1 and 4 exist +precisely because a fragmented layout is unsafe to address by raw offset. +Deliberately deferred out of the incident; it is a design task, not a patch. + +**Predicting it for the next region:** `rows > 4,194,303` is the whole test, +and rows are reported by `bake`. Any region past ~4.19 M rows takes the raw +path until the multi-batch work lands. ## Results — executed 2026-08-13 From db00b82b298fe9f554bea7e00b0e32c7bc25332a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 11:18:53 +0000 Subject: [PATCH 5/6] osm bake: flag the volume-capacity risk the plan sized but never checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plan measures all three Brandenburg artifacts and never adds them up against the disk they have to land on. They total 4,130,303,971 B (3.85 GiB) — 2.7x Berlin's ~1.54 GB — and `osm_slab_hydrate` has no free-space preflight: no statvfs, no capacity check, no ENOSPC branch. Its own docs are written around Berlin ("the 1.29 GiB artifact", "Berlin is ~1.42 GB"). What makes this worth writing down is not the risk but its SHAPE. A download that runs out of space truncates, fails the checksum gate, and `download_verified` leaves no file behind — so the next boot repeats it and the listener never binds. From outside that is an indefinite 502, which is exactly what a slow first hydration of a 3.75 GB slab also looks like. One resolves itself and one never does, and they are indistinguishable without the deploy logs. Records the arithmetic, the two indistinguishable states, and the operator rule that follows: on a persistent 502 after switching a large region, read the logs rather than waiting. Notes that a preflight failing loudly with both numbers would remove the ambiguity, and that it does not exist yet — so the gap is visible rather than implied. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- .../plans/2026-08-13-brandenburg-bake.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/claude-notes/plans/2026-08-13-brandenburg-bake.md b/claude-notes/plans/2026-08-13-brandenburg-bake.md index 7b6d2fe1e..b09aba0df 100644 --- a/claude-notes/plans/2026-08-13-brandenburg-bake.md +++ b/claude-notes/plans/2026-08-13-brandenburg-bake.md @@ -157,6 +157,25 @@ makes the window smaller; only staging removes it. value. Serving Brandenburg replaces Berlin; it does not add to it. 5. **Brandenburg is past Arrow's row ceiling, so it serves from the raw `.soa`, not from Lance.** This bit during rollout — see below. +6. **The persistent volume must hold ~4.13 GB, and nothing checks that it + can.** Measured artifacts total 3,753,072,128 + 155,979,619 + 221,252,224 + = **4,130,303,971 B (3.85 GiB)** — **2.7x Berlin's ~1.54 GB**, and + `osm_slab_hydrate`'s own docs are written around Berlin ("the 1.29 GiB + artifact", "Berlin is ~1.42 GB"). There is **no free-space preflight** in + that module: no `statvfs`, no capacity check, no ENOSPC branch. + + The failure mode if the volume is too small is quiet and self-inflicted: + a download that runs out of space produces a truncated file, the checksum + gate rejects it, `download_verified` **leaves no file behind**, and the + next boot repeats it. The listener never binds, so from outside it is an + indefinite 502 — **identical to a slow first hydration**, which is the + benign case. The two cannot be told apart without the deploy logs. + + Before switching a region this large, check the volume's capacity against + the artifact total, and on a persistent 502 read the logs rather than + waiting: "still hydrating" resolves itself and "volume too small" never + does. A free-space preflight that fails loudly with the two numbers would + remove the ambiguity entirely; it does not exist yet. ## The rollout crash — Arrow's i32 array ceiling (measured 2026-08-14) From 639ecb520ad59d3bca5948ccb464211dfe655a2d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 11:52:34 +0000 Subject: [PATCH 6/6] osm lance: import the Array trait so the regression tests actually compile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tests added with the row-ceiling guard never built. `osm_lance.rs:638` calls `.len()` on a `FixedSizeBinaryArray`, which comes from the `Array` trait rather than the struct, and the trait was not in scope: error[E0599]: no method named `len` found for struct `arrow::array::FixedSizeBinaryArray` in the current scope The mechanism is worth naming in the source, which this does: **`cargo check` does not compile `#[cfg(test)]` blocks.** A clean check run reported EXIT=0 on this exact tree and said nothing whatsoever about the tests — so a green check was mistaken for verification, and the tests were cited as evidence in two review replies while incapable of building. With the import, all 14 osm_lance tests pass. The one that matters was then verified to be a real falsifier rather than merely green: disabling the guard (`if false`) makes it fail on the destructive half specifically — declining must not delete an existing dataset — the guard has to run before remove_stale_dataset, not after it which is the assertion a row-count-only test cannot make, and the exact defect both reviewers identified. Guard restored, all 14 green again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw --- crates/cockpit-server/src/osm_lance.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/cockpit-server/src/osm_lance.rs b/crates/cockpit-server/src/osm_lance.rs index d472b168e..a931827f8 100644 --- a/crates/cockpit-server/src/osm_lance.rs +++ b/crates/cockpit-server/src/osm_lance.rs @@ -552,6 +552,12 @@ fn search_head_for_probe(file_path: &Path, probe: &[u8]) -> Option { #[cfg(test)] mod tests { use super::*; + // `len()` on a FixedSizeBinaryArray comes from the `Array` trait, not + // from the struct — the production code never calls it, so the import + // is test-only. Worth naming: `cargo check` does NOT compile + // `#[cfg(test)]` blocks, so a missing import here passes a check run + // and only fails under `cargo test`. + use arrow::array::Array; /// The classid constant must stay the canon's dormant-default value — /// this is a documentation-of-intent test, not a behavioural one: a