Add dense-county precision rule for municipality-sized counties - #7
Conversation
The compact index assigns each geohash cell to exactly one place. In countries whose county placetype is a small municipality (Guatemala, Honduras, El Salvador, ...), the county precision cap of 4 produces cells of roughly 39x20 km, so the most populous municipality in each cell swallows its neighbors: they own zero cells and can never be returned by a lookup. Guatemala, for example, can only resolve to 47 of its 303 municipalities, and Antigua Guatemala resolves to Escuintla. Mirror the existing sparse-region rule (which lowers precision for very large regions) with a dense-county rule that raises it for very small counties: when both --county-dense-max-precision and --county-dense-max-area-km2 are set, county polygons whose bbox area is at or under the threshold are covered at the dense precision instead of the normal county cap. The dense precision never falls below the county cap and is clamped to the global --max-precision. Large counties are unaffected, and their cells fully inside the polygon still terminate at the base precision, so the row-count cost is concentrated where the rule fires. The rule is off by default; generate_wof_boundary.sh forwards it via WOF_COUNTY_DENSE_MAX_PRECISION / WOF_COUNTY_DENSE_MAX_AREA_KM2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72ddc6a13b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: WOF_COUNTY_MAX_PRECISION defaults to WOF_MAX_PRECISION (5), so enabling only the two dense env vars built every county at the dense precision already, making the area threshold a no-op and keeping the global row-count cost the rule exists to avoid. When both dense vars are set and WOF_COUNTY_MAX_PRECISION is not given explicitly, default the regular county cap to one below the dense precision (never below WOF_BASE_PRECISION). An explicit WOF_COUNTY_MAX_PRECISION always wins, and nothing changes for users who do not enable the rule. Verified on the Guatemala WOF archive: dense vars alone now produce county=4 with the dense rule active (2,280 rows); an explicit county cap of 5 still wins (4,234 rows).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ec1a96691
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review round 2: the derivation lived only in the shell helper, so (1) passing the two dense flags directly to generate_boundary_index.js without --county-max-precision left the county cap at the global max, reproducing the no-op footgun at the CLI layer, and (2) the shell derived the cap from the raw dense value, so a dense precision above WOF_MAX_PRECISION (which the node side clamps) could derive a cap equal to or beyond the global max. Fix both by deriving in one place, in the node script: when both dense options are set and valid and --county-max-precision was not provided, the county cap defaults to one below the dense precision after it has been clamped to --max-precision (and never below --base-precision). The shell helper now simply omits --county-max-precision when WOF_COUNTY_MAX_PRECISION is unset, so both entry points behave identically; with the rule off, the node fallback (global max) matches the helper's previous default. New spec covers the dense-flags-only CLI path (verified red-green: without the derivation the large county gains precision-5 cells). Guatemala smoke via the helper: dense vars only => county=4 with the rule active (2,280 rows); dense precision 6 with max 5 => same (clamped, cap 4); explicit cap 5 still wins and rule-off still builds county=5 (4,234 rows).
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…recision # Conflicts: # CHANGELOG
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Problem
The compact index assigns each geohash cell to exactly one winning place. In countries whose
countyplacetype is a small municipality (Guatemala, Honduras, Costa Rica, El Salvador, Colombia, Mexico's south, Thailand, ...), the county precision cap of 4 produces cells of roughly 39x20 km, and the most populous municipality in each cell swallows its neighbors: they own zero cells and can never be returned by a lookup. Measured on a Guatemala build, only 47 of 303 municipalities are reachable, and Antigua Guatemala resolves to "Escuintla". Raising the county cap to 5 globally fixes precision but multiplies county rows by ~26 in the worst case, which a world build cannot afford.The repo already has a precedent for area-keyed precision: the sparse-region rule (
--region-sparse-max-precision/--region-sparse-min-area-km2) lowers precision for very large regions. This PR adds its mirror image for counties.Change
scripts/generate_boundary_index.js: when both--county-dense-max-precisionand--county-dense-max-area-km2are set and valid, county polygons whose bbox area (bboxAreaKm2, same measure as the sparse-region rule) is at or under the threshold are covered at the dense precision. The dense precision never falls below the regular--county-max-precisionand is clamped to the global--max-precision. The rule is off unless both flags are present.scripts/generate_wof_boundary.shforwards the rule viaWOF_COUNTY_DENSE_MAX_PRECISION/WOF_COUNTY_DENSE_MAX_AREA_KM2(documented in the header; empty by default, i.e. off).Dense county rule: area_km2<=... => max_precision=...).### Unreleased.Measurements
Basket of 7 countries (GT, HN, CR, MX, CO, TH, FR) built with
scripts/generate_wof_boundary.sh,WOF_MAX_PRECISION=5, defaults otherwise. Legs: the shipped configuration (WOF_COUNTY_MAX_PRECISION=4), the upper bound (county=5), and county=4 plus the dense rule at precision 5 with area thresholds 300 / 1000 / 3000 km2. (The FR archive also carries GF/GP/MQ/RE/YT; they are included in totals.)County owners with at least one cell (and county lookup rows) per leg:
Observations:
World extrapolation
The shipped world DB (county cap 4) has 356,739 lookup rows / 21.7 MB, of which 142,185 are county cells. Bucketing its 114 county-bearing countries by average county footprint (county cells per owning county, a proxy for county size):
Extra rows per baseline county cell (cell-weighted from the basket): S = 1.53 / 7.81 / 18.05 and M = 1.06 / 4.06 / 8.84 at thresholds 300 / 1000 / 3000. The L bucket has no basket exemplar, so it is bracketed: an upper bound copying M (pretends RU/US/CN counties are as threshold-eligible as Mexican municipios - a deliberate overestimate), a floor from measured FR (~0.03-0.11), and a reasoned central estimate (0.05 / 0.35 / 1.5) from the small-county tails of the L membership (US east coast, Swedish kommuner, AR partidos). At ~49.5 bytes per row on top of 21.7 MB:
Recommendation
WOF_COUNTY_DENSE_MAX_PRECISION=5withWOF_COUNTY_DENSE_MAX_AREA_KM2=1000: the central world estimate is ~28 MB (under a ~35 MB working target), and even the deliberately pessimistic upper bound stays under a 50 MB ceiling, while recovering nearly all swallowed municipalities in the affected countries. 3000 km2 risks blowing past 50 MB if the L bucket is more eligible than expected; 300 km2 is the safe fallback (worst case 28 MB) but leaves CR, TH, CO, and MX only partially fixed. The first real world build with the flag should confirm the actual size before shipping.Tests
spec/boundary_builder_dense_county_spec.jsspawns the CLI on synthetic GeoJSON: a municipality-sized county inside one precision-4 cell (bbox ~69 km2) gets all its cells at the dense precision 5, a ~104,000 km2 county in the same build stays at the county cap 4, and with the flags absent both stay at 4 (and the summary line is absent).Expected $[0].len = 4 to equal 5.