#144: tile tool cleanup — the #142 review's non-blocking items #145

Merged

art merged 2 commits from issue-144-tile-tool-cleanup into main

2026-08-16 22:48:38 +00:00

art commented

2026-08-16 22:46:36 +00:00

Owner

Closes #144

#144: tile tool cleanup — the #142 review's non-blocking items

All eight items from the #142 review (comment 1643), each confirmed still open
in main's importer/tiles.go before this branch.

The items

--size-layer <name> — comma-separated, like import --layer. Filters
the layers runTileSize processes and composes with --size-zooms (verified:
tiles --size --size-layer cover --size-zooms 3-5 conf/semap.yaml runs one
layer over the window in ~1.5 s). Unknown names error before any DB work,
mirroring selectedLayers (--size-layer bogus → exit 1).
candidateTiles LIMIT — the per-feature pass is capped at 200 000 rows,
the same evidence budget probeTile already sampled, via one shared const
(sampleRows). The weight ranking is a heuristic; the cap is documented in
code and in docs/importer-cli.md. Before: tiles --size conf/semap.yaml
walked Sweden's 5.4M-row z13 cover partition whole. After: a full cover pass
(z3–z13) is ~12 s.
Failed fetch is no longer a 0-byte tile — sizeOneZoom counts fetch
errors (fetch_errors in the JSON; -- N candidate fetch(es) failed; not ranked in the table) instead of appending measured{0,0} and letting the
top-N sort discard it silently. The --check MISSING pattern, applied to
--size.
Bench JSON — the committed file was 475,904 B / 26,654 lines and
make tile-bench added one per run. Fix chosen: commit combo summaries,
keep per-tile detail local. New --bench-summary flag omits the per-tile
tiles arrays (combo summaries keep total/avg bytes and the latency
percentiles); make tile-bench passes it; the oversized committed file is
removed and replaced by a fresh summary-form record (12,391 B for both
funcs — back at the repo's 1–2 kB record convention). The per-tile
z/x/y/bytes/ms rows remain available from a direct
importer tiles --bench conf/ismap.yaml > file.json run (475,916 B, 81
tiles × 20 combos × 2 funcs). Justification: the committed record's job is
the weight/latency trend per combo; one run's per-tile dump is analysis
material that should not churn the repo on every bench run.
parseIntStrict is now strict — strconv.Atoi after trimming the
surrounding whitespace Sscanf's %d tolerated. --size-zooms 5x errors
(exit 1, invalid syntax) instead of silently meaning 5.
checkTileAtZoom uses tileTarget.tableAtZoom — the inline re-derivation
of the same prefix is gone; one code path for the probe-table name.
Static layers are deliberately skipped — tileTargets skips
l.Static != nil with a comment: a static layer is one table, no cascade,
no zxy_ function (swapSQL skips mvtFunc for it), so neither --check
nor --size applies, and a naive zoom-suffixed probe table would not exist
and report a silent EMPTY. Latent (no config declares one today) but now
impossible to hit silently. Unit-tested (TestTileTargetsSkipsStatic).
Smaller — decodeMVTTile's error path is real: mvtFields returns
errors for malformed varints (previously an unterminated varint looped
forever: uvarint returned 0,0 and the walk never advanced), payload spans
past the end of the buffer, and unknown wire types — the three call sites'
checks now do real work. probeTile no longer computes an n its format
string never referenced. The Sweden whole-country bench combo is
sweden-whole-country, matching the <country>-whole-country convention.

Verification

make build, go vet ./importer, make test — clean; new unit tests:
parseIntStrict (trailing garbage, whitespace, negatives), parseTileSizeZooms
(5x/5-/-5/9-3/0-25 errors), parseTileSizeLayers (empty → all,
unknown → error, duplicates), tableAtZoom, static-layer skip,
decoder malformed-input errors (4 cases + truncated layer), bench summary
omitempty.
Live DB (read-only), all against the running database:
- importer tiles --check conf/ismap.yaml — all 34 results PASS (unchanged
  from before the branch; R121 semantics untouched).
- importer tiles --size --size-layer cover conf/semap.yaml — one layer,
  ~12 s (was minutes).
- importer tiles --size conf/ismap.yaml --size-zooms 5-6 — full pass over
  the window in ~1.9 s, empty layers reported as (no candidates).
- importer tiles --bench conf/ismap.yaml vs
  --bench --bench-summary conf/ismap.yaml — 475,916 B (per-tile detail)
  vs 12,391 B (summaries only); both parse.
- importer tiles --size --size-zooms 5x conf/ismap.yaml and
  --size-layer bogus conf/ismap.yaml — exit 1 with a clear message.
- importer help tiles documents --size-layer and --bench-summary.

No acceptance tests were added or changed: every fix here is CLI parsing,
filtering or reporting — behavior covered by the Go unit tests above. None of
it is a SQL verdict against the database (the R121 check's semantics are
untouched, and it is already an acceptance test), so the SQL acceptance-test
mechanism has nothing to gain here.

Closes #144 --- # #144: tile tool cleanup — the #142 review's non-blocking items All eight items from the #142 review (comment 1643), each confirmed still open in main's `importer/tiles.go` before this branch. ## The items 1. **`--size-layer <name>`** — comma-separated, like `import --layer`. Filters the layers `runTileSize` processes and composes with `--size-zooms` (verified: `tiles --size --size-layer cover --size-zooms 3-5 conf/semap.yaml` runs one layer over the window in ~1.5 s). Unknown names error before any DB work, mirroring `selectedLayers` (`--size-layer bogus` → exit 1). 2. **candidateTiles LIMIT** — the per-feature pass is capped at 200 000 rows, the same evidence budget `probeTile` already sampled, via one shared const (`sampleRows`). The weight ranking is a heuristic; the cap is documented in code and in `docs/importer-cli.md`. Before: `tiles --size conf/semap.yaml` walked Sweden's 5.4M-row z13 cover partition whole. After: a full cover pass (z3–z13) is ~12 s. 3. **Failed fetch is no longer a 0-byte tile** — `sizeOneZoom` counts fetch errors (`fetch_errors` in the JSON; `-- N candidate fetch(es) failed; not ranked` in the table) instead of appending `measured{0,0}` and letting the top-N sort discard it silently. The `--check` MISSING pattern, applied to `--size`. 4. **Bench JSON** — the committed file was 475,904 B / 26,654 lines and `make tile-bench` added one per run. Fix chosen: **commit combo summaries, keep per-tile detail local**. New `--bench-summary` flag omits the per-tile `tiles` arrays (combo summaries keep total/avg bytes and the latency percentiles); `make tile-bench` passes it; the oversized committed file is removed and replaced by a fresh summary-form record (12,391 B for both funcs — back at the repo's 1–2 kB record convention). The per-tile z/x/y/bytes/ms rows remain available from a direct `importer tiles --bench conf/ismap.yaml > file.json` run (475,916 B, 81 tiles × 20 combos × 2 funcs). Justification: the committed record's job is the weight/latency trend per combo; one run's per-tile dump is analysis material that should not churn the repo on every bench run. 5. **parseIntStrict is now strict** — `strconv.Atoi` after trimming the surrounding whitespace Sscanf's `%d` tolerated. `--size-zooms 5x` errors (exit 1, `invalid syntax`) instead of silently meaning 5. 6. **checkTileAtZoom uses `tileTarget.tableAtZoom`** — the inline re-derivation of the same prefix is gone; one code path for the probe-table name. 7. **Static layers are deliberately skipped** — `tileTargets` skips `l.Static != nil` with a comment: a static layer is one table, no cascade, no `zxy_` function (`swapSQL` skips `mvtFunc` for it), so neither `--check` nor `--size` applies, and a naive zoom-suffixed probe table would not exist and report a silent EMPTY. Latent (no config declares one today) but now impossible to hit silently. Unit-tested (`TestTileTargetsSkipsStatic`). 8. **Smaller** — `decodeMVTTile`'s error path is real: `mvtFields` returns errors for malformed varints (previously an unterminated varint looped forever: `uvarint` returned 0,0 and the walk never advanced), payload spans past the end of the buffer, and unknown wire types — the three call sites' checks now do real work. `probeTile` no longer computes an `n` its format string never referenced. The Sweden whole-country bench combo is `sweden-whole-country`, matching the `<country>-whole-country` convention. ## Verification - `make build`, `go vet ./importer`, `make test` — clean; new unit tests: `parseIntStrict` (trailing garbage, whitespace, negatives), `parseTileSizeZooms` (`5x`/`5-`/`-5`/`9-3`/`0-25` errors), `parseTileSizeLayers` (empty → all, unknown → error, duplicates), `tableAtZoom`, static-layer skip, decoder malformed-input errors (4 cases + truncated layer), bench summary omitempty. - Live DB (read-only), all against the running database: - `importer tiles --check conf/ismap.yaml` — all 34 results PASS (unchanged from before the branch; R121 semantics untouched). - `importer tiles --size --size-layer cover conf/semap.yaml` — one layer, ~12 s (was minutes). - `importer tiles --size conf/ismap.yaml --size-zooms 5-6` — full pass over the window in ~1.9 s, empty layers reported as `(no candidates)`. - `importer tiles --bench conf/ismap.yaml` vs `--bench --bench-summary conf/ismap.yaml` — 475,916 B (per-tile detail) vs 12,391 B (summaries only); both parse. - `importer tiles --size --size-zooms 5x conf/ismap.yaml` and `--size-layer bogus conf/ismap.yaml` — exit 1 with a clear message. - `importer help tiles` documents `--size-layer` and `--bench-summary`. No acceptance tests were added or changed: every fix here is CLI parsing, filtering or reporting — behavior covered by the Go unit tests above. None of it is a SQL verdict against the database (the R121 check's semantics are untouched, and it is already an acceptance test), so the SQL acceptance-test mechanism has nothing to gain here.

art added 2 commits

2026-08-16 22:46:36 +00:00

#144 : tile tool cleanup — the #142 review's non-blocking items ae411f28c4

All eight items from the #142 review (comment 1643), confirmed still open in
main's importer/tiles.go:

1. --size-layer <name> (comma-separated, like import --layer): restricts the
   size pass to named layers and composes with --size-zooms. Unknown names are
   an error before any DB work, mirroring selectedLayers.
2. candidateTiles now LIMITs its per-feature pass to 200 000 rows (the same
   evidence budget probeTile samples, one shared const): --size on semap's
   z13 cover partition walked 5.4M rows before; a single cover pass is now
   seconds. The weight ranking is a heuristic, so the cap is documented, not
   hidden.
3. A failed candidate fetch is no longer recorded as measured{0,0} and silently
   discarded by the top-N sort: sizeZoom counts fetch errors (fetch_errors in
   the JSON, a "-- N candidate fetch(es) failed; not ranked" line in the table).
   A broken function now looks like an error, not a plausible short list.
4. bench/results: committed JSON was 475,904 B / 26,654 lines and make
   tile-bench added one per run. Fix chosen: commit combo summaries, keep
   per-tile detail local. New --bench-summary flag omits the per-tile tiles
   arrays; make tile-bench passes it (12,391 B for both funcs, back at the
   repo's 1-2 kB record convention); the oversized committed file is removed;
   per-tile z/x/y/bytes/ms stays available from a direct --bench run.
5. parseIntStrict now rejects trailing garbage (strconv.Atoi after trimming
   surrounding whitespace): --size-zooms 5x errors instead of silently meaning 5.
6. checkTileAtZoom uses tileTarget.tableAtZoom instead of re-deriving the same
   prefix inline.
7. Static layers are now deliberately skipped in tileTargets with a comment:
   they have one table, no cascade and no zxy_ function (swapSQL skips
   mvtFunc), so a naive zoom-suffixed probe table would not exist and the layer
   would report a silent EMPTY. No config declares one today; the next one
   cannot silently EMPTY.
8. Smaller: decodeMVTTile's error path is now real — mvtFields returns errors
   for malformed varints (which previously looped forever), payload spans past
   the end of the buffer, and unknown wire types, so the three call sites'
   checks do real work; probeTile no longer computes an n it never formats;
   the Sweden whole-country bench combo is labelled sweden-whole-country to
   match the <country>-whole-country convention.

R121 check semantics unchanged: the check ran green against the live DB before
and after (conf/ismap.yaml, all 12 layers PASS). Smoke: tiles --size
--size-layer cover conf/semap.yaml (one layer, ~12 s vs minutes), tiles --size
conf/ismap.yaml --size-zooms 5-6, tiles --bench vs --bench --bench-summary
JSON sizes (475,916 B vs 12,391 B), --size-zooms 5x and --size-layer bogus exit
1. Unit tests added: parseIntStrict, parseTileSizeZooms, parseTileSizeLayers,
tableAtZoom, static-layer skip, decoder malformed-input errors, bench summary
omitempty. No acceptance tests added — the changed behavior is CLI parsing,
filtering and reporting, all covered by Go unit tests; nothing here is a SQL
verdict against the database.

#144 : commit the first summary-form tile benchmark record 6c264633e9

make tile-bench --bench-summary now writes 12,391 B (both funcs) instead of a
475 kB per-tile dump; this is the first record in the new form, replacing the
removed 2026-08-16_478f205_tiles.json. The per-tile detail for this run remains
available from a direct `importer tiles --bench` invocation.

art referenced this pull request

2026-08-16 22:46:39 +00:00

tile tool cleanup: the #142 review's non-blocking items #144

art merged commit 6c264633e9 into main

2026-08-16 22:48:38 +00:00

art deleted branch issue-144-tile-tool-cleanup

2026-08-16 22:48:38 +00:00