From 213defa58e52d9fd7f8d16791e355a265d796abb Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 20:23:06 -0600 Subject: [PATCH 1/6] Give a place the geohash cell that contains its own centroid The compact index assigns one owning place per cell, chosen by placetype rank and then population. Near a national border two towns routinely share a precision-5 cell, so the more populous one takes the whole cell - including the cell holding the other town's own centre. The point then resolves to the wrong country, which is the worst answer this library can give. Rank a place that is centred in a cell ahead of the population tie-break, still behind placetype rank: a county's home cell should not outrank a locality that covers it, because the locality is the more useful label for someone standing there. When two centroids share a cell the existing comparator decides, unchanged. A cover is the set of terminal nodes of a quadtree walk, so its cells never nest and a place has at most one cell containing its centroid. The claim is therefore a prefix test against the centroid geohash, exact for every cell the comparator is asked about, and it invents no cells the cover pass did not produce. It is also computable from compact_places.latitude/longitude, so append batches rank an already-written incumbent by the same rule and the result stays independent of country order. The dominant-city rollup could still delete the protected cell afterwards, so it now keeps the home cell of a town in another country. Rollups inside one country are untouched: absorbing suburbs into a metro is a deliberate labelling choice, while moving a town across a border is never right. Measured on a 7-country slice (us,mx,nl,de,at,ch,cz; WOF pinned by lockfile, precision 5, min population 5000, one country per append batch): locality centroids resolving to a different country fall from 35 to 9 of 13,192, with 26 fixed and none newly wrong. 3,032 centroids now resolve to the place itself; 2 no longer do. Lookup rows grow by 188 (0.04%) and the database by 16 KB (0.067%). --- CHANGELOG | 9 ++ README.md | 2 + scripts/generate_boundary_index.js | 97 +++++++++++- scripts/generate_wof_boundary.sh | 3 + spec/boundary_builder_merge_spec.js | 40 ++++- spec/boundary_home_cell_spec.js | 228 ++++++++++++++++++++++++++++ 6 files changed, 370 insertions(+), 9 deletions(-) create mode 100644 spec/boundary_home_cell_spec.js diff --git a/CHANGELOG b/CHANGELOG index c6118fb..5830b5e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,14 @@ ### Unreleased +- Give every place the geohash cell that contains its own centroid, ahead of + the population tie-break, so a border town no longer loses its own centre to + the larger city across the border (for example Calexico resolving to + Mexicali, or Heerlen to Aachen). Placetype ranking is unchanged, and cells + holding two centroids are still decided by population. Disable with + `--home-cell-priority false` (`WOF_HOME_CELL_PRIORITY=0`). +- Stop the dominant-city rollup absorbing the home cell of a town in another + country. Rollups inside a country are unchanged, so a metro still reads as + one city. - Fix `--append` builds handing contested geohash cells to whichever batch ran last. Cells that already belong to a better-ranked place (by placetype, then population, then centroid distance) now keep their existing owner, so border diff --git a/README.md b/README.md index ae657c1..2279b22 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,7 @@ Builder notes: - `--region-max-precision` - `--region-sparse-max-precision` + `--region-sparse-min-area-km2` for very large sparse regions (for example geohash-3 in Amazon-like interiors) - `--promote-locality-over-region` (default `true`) prefers locality labels in shared parent cells when there is no competing locality (keeps city labels sticky against region-only outskirts) +- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. The dominant-city rollup keeps a foreign town's home cell for the same reason; rollups within one country are unaffected - Dominant-city rollup keeps broad city labels sticky in mixed city/suburb cells unless there is competing major-city pressure: - `--dominant-locality-population` (default `100000`) - `--dominant-locality-ratio` (default `3`) @@ -276,6 +277,7 @@ Useful WOF build env vars: - `WOF_REGION_SPARSE_MAX_PRECISION` sparse very-large-region precision (default `3`) - `WOF_REGION_SPARSE_MIN_AREA_KM2` area threshold for sparse region precision (default `80000`) - `WOF_PROMOTE_LOCALITY_OVER_REGION=1|0` prefer locality labels over region in shared parent cells (default `1`) +- `WOF_HOME_CELL_PRIORITY=1|0` let a place keep the cell that contains its own centroid (default `1`) - `WOF_DOMINANT_LOCALITY_POPULATION` major-locality threshold for dominant-city rollup (default `100000`) - `WOF_DOMINANT_LOCALITY_RATIO` dominant-vs-next locality population ratio (default `3`) - `WOF_PARENT_LOCALITY_MIN_SHARE` minimum child-cell share for locality parent takeover (default `0.5`) diff --git a/scripts/generate_boundary_index.js b/scripts/generate_boundary_index.js index 1cbd39a..f22ddd1 100755 --- a/scripts/generate_boundary_index.js +++ b/scripts/generate_boundary_index.js @@ -22,6 +22,12 @@ const PLACETYPE_BY_CODE = { 3: 'county' } +// Precision used to precompute the geohash of a place centroid. Geohashes are +// hierarchical, so the cell containing a centroid at any precision <= this +// value is a prefix of this string; 12 is deeper than any cover precision the +// builder can emit. +const HOME_CELL_PRECISION = 12 + function parseBool(value, defaultValue) { if (value === undefined || value === null || value === '') { return defaultValue @@ -64,6 +70,7 @@ function parseArgs(argv) { regionSparseMaxPrecision: null, regionSparseMinAreaKm2: null, promoteLocalityOverRegion: true, + homeCellPriority: true, dominantLocalityPopulation: 100000, dominantLocalityRatio: 3, parentLocalityMinShare: 0.5 @@ -128,6 +135,8 @@ function parseArgs(argv) { opts.regionSparseMinAreaKm2 = Number.isFinite(sparseAreaKm2) && sparseAreaKm2 > 0 ? sparseAreaKm2 : null } else if (arg === '--promote-locality-over-region') { opts.promoteLocalityOverRegion = parseBool(argv[++i], true) + } else if (arg === '--home-cell-priority') { + opts.homeCellPriority = parseBool(argv[++i], true) } else if (arg === '--dominant-locality-population') { var dominantPopulation = Number(argv[++i]) opts.dominantLocalityPopulation = Number.isFinite(dominantPopulation) ? dominantPopulation : opts.dominantLocalityPopulation @@ -179,6 +188,7 @@ function usage() { ' --region-sparse-max-precision Optional precision for very large region polygons (for example 3)', ' --region-sparse-min-area-km2 Area threshold to apply sparse region precision', ' --promote-locality-over-region Prefer locality over region in shared parent cells when no competing locality exists (default: true)', + ' --home-cell-priority Let a place keep the cell containing its own centroid, ahead of the population tie-break (default: true)', ' --dominant-locality-population Population threshold that marks locality as major for dominant-city rollups (default: 100000)', ' --dominant-locality-ratio Required dominant-vs-next population ratio for locality rollups (default: 3)', ' --parent-locality-min-share Minimum child-cell share (0..1) required to let a locality take over a parent cell (default: 0.5)', @@ -671,6 +681,7 @@ function normalizeFeature(feature, opts) { placetypeCode: placetypeCode(placetype), centroidLat: centroid.latitude, centroidLon: centroid.longitude, + homeGeohash: geohash.encode(centroid.latitude, centroid.longitude, HOME_CELL_PRECISION), population: population, bboxMinLat: bbox.minLat, bboxMinLon: bbox.minLon, @@ -887,13 +898,66 @@ function pointDistanceScore(latitude, longitude, targetLatitude, targetLongitude ((lon - targetLon) * (lon - targetLon) * scale) } -function comparePlacesForHash(a, b, hash, hashCenterCache) { +// A place owns the "home cell" of a geohash when its own centroid falls inside +// that cell. Cover cells are the terminal nodes of a quadtree walk and never +// nest, so a place has at most one home cell in its own cover and this prefix +// test is exact for every cell the comparator can be asked about. +function ownsHomeCell(place, hash) { + if (!place || !hash) { + return false + } + + var home = place.homeGeohash + if (typeof home !== 'string') { + return false + } + + // Builds deeper than the precomputed precision encode the centroid at the + // cell's own length instead of silently dropping the claim. + if (home.length < hash.length) { + return geohash.encode(place.centroidLat, place.centroidLon, hash.length) === hash + } + + return home.slice(0, hash.length) === hash +} + +// Two places are foreign to each other only when both country ids are known +// and differ; an unknown country is treated as "same country" so the rollup +// keeps its current behaviour on records without one. +function isForeignTo(place, other) { + if (!place || !other) { + return false + } + + var placeCountry = place.countryId + var otherCountry = other.countryId + if (!placeCountry || !otherCountry) { + return false + } + + return String(placeCountry).toUpperCase() !== String(otherCountry).toUpperCase() +} + +function comparePlacesForHash(a, b, hash, hashCenterCache, homeCellPriority) { var typeRankA = placetypeRank(a.placetype) var typeRankB = placetypeRank(b.placetype) if (typeRankA !== typeRankB) { return typeRankA - typeRankB } + // Within a placetype, a place that sits in the cell keeps it even when a + // bigger neighbour also covers it. Without this, a border town loses the + // cell holding its own centroid to the larger city on the other side and + // resolves to the wrong country. When both centroids share the cell the + // usual population ordering decides. + if (homeCellPriority) { + var homeA = ownsHomeCell(a, hash) + var homeB = ownsHomeCell(b, hash) + if (homeA !== homeB) { + return homeA ? -1 : 1 + } + } + if (a.population !== b.population) { return b.population - a.population } @@ -936,12 +1000,19 @@ function cellCandidateFromCompactRow(row) { var area = Number(row.area) if (!Number.isFinite(area) || area < 0) area = 0 + var latitude = Number(row.latitude) + var longitude = Number(row.longitude) + var homeGeohash = Number.isFinite(latitude) && Number.isFinite(longitude) + ? geohash.encode(latitude, longitude, HOME_CELL_PRECISION) + : null + return { id: Number(row.id), placetype: PLACETYPE_BY_CODE[Number(row.placetype_code)] || 'region', population: population, - centroidLat: Number(row.latitude), - centroidLon: Number(row.longitude), + centroidLat: latitude, + centroidLon: longitude, + homeGeohash: homeGeohash, area: area } } @@ -1178,9 +1249,18 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) continue } + // Minor localities are folded into the dominant city, which is what + // makes a metro read as one name. The exception is a town in another + // country: absorbing the cell it sits in would put it on the wrong side + // of a border, which no label choice can justify. + var protectedHomeCell = opts.homeCellPriority && + ownsHomeCell(descendantPlace, descendantHash) && + isForeignTo(descendantPlace, placeById[String(promoted.localityId)]) + if (promoted.suppressMinorLocalities && isCityPlacetypeCode(descendantPlace.placetypeCode) && - !isMajorLocality(descendantPlace, opts)) { + !isMajorLocality(descendantPlace, opts) && + !protectedHomeCell) { delete bestByHash[descendantHash] } } @@ -1202,7 +1282,7 @@ function buildCompactLookupRows(places, opts) { var cell = place.cover[j] var hash = cell.geohash var current = bestByHash[hash] - if (!current || comparePlacesForHash(place, current, hash, hashCenterCache) < 0) { + if (!current || comparePlacesForHash(place, current, hash, hashCenterCache, opts.homeCellPriority) < 0) { bestByHash[hash] = place } } @@ -1269,7 +1349,7 @@ function buildCompactLookupRows(places, opts) { // used within a batch and keep the better match. Cells owned by places that // are part of the current batch are always rewritten, because their rows are // deleted and replaced by this run. -async function resolveExistingCellConflicts(db, rows, placeById, batchPlaceIds) { +async function resolveExistingCellConflicts(db, rows, placeById, batchPlaceIds, homeCellPriority) { if (!rows.length) { return { rows: rows, skipped: 0 } } @@ -1320,7 +1400,7 @@ async function resolveExistingCellConflicts(db, rows, placeById, batchPlaceIds) var incumbent = cellCandidateFromCompactRow(existing) var challenger = placeById[String(row.placeId)] - if (!challenger || comparePlacesForHash(incumbent, challenger, row.geohash, hashCenterCache) <= 0) { + if (!challenger || comparePlacesForHash(incumbent, challenger, row.geohash, hashCenterCache, homeCellPriority) <= 0) { skipped += 1 return } @@ -1877,7 +1957,7 @@ async function main() { batchPlaceIds[String(place.id)] = true }) - var resolved = await resolveExistingCellConflicts(db, compactBuild.rows, compactBuild.placeById, batchPlaceIds) + var resolved = await resolveExistingCellConflicts(db, compactBuild.rows, compactBuild.placeById, batchPlaceIds, options.homeCellPriority) compactLookupRows = resolved.rows lookupRowsDeferred = resolved.skipped } @@ -1915,6 +1995,7 @@ async function main() { if (options.includeRegion) modeLabel += ' + region' console.log('Mode: ' + modeLabel) console.log('Precision: ' + options.basePrecision + ' -> ' + options.maxPrecision) + console.log('Home cell priority: ' + (options.homeCellPriority ? 'true' : 'false')) console.log('Placetype precision caps: locality=' + options.localityMaxPrecision + ', localadmin=' + options.localadminMaxPrecision + ', county=' + options.countyMaxPrecision + ', region=' + options.regionMaxPrecision) if (Number.isFinite(options.regionSparseMaxPrecision) && Number.isFinite(options.regionSparseMinAreaKm2)) { console.log('Sparse region rule: area_km2>=' + options.regionSparseMinAreaKm2 + ' => max_precision=' + options.regionSparseMaxPrecision) diff --git a/scripts/generate_wof_boundary.sh b/scripts/generate_wof_boundary.sh index b5da63b..2821a06 100755 --- a/scripts/generate_wof_boundary.sh +++ b/scripts/generate_wof_boundary.sh @@ -21,6 +21,7 @@ set -euo pipefail # WOF_REGION_SPARSE_MAX_PRECISION Sparse large-region precision (default: 3) # WOF_REGION_SPARSE_MIN_AREA_KM2 Area threshold for sparse region precision (default: 80000) # WOF_PROMOTE_LOCALITY_OVER_REGION Prefer locality labels over region in shared parent cells (default: 1) +# WOF_HOME_CELL_PRIORITY Let a place keep the cell holding its own centroid (default: 1) # WOF_DOMINANT_LOCALITY_POPULATION Major-locality threshold for dominant-city rollup (default: 100000) # WOF_DOMINANT_LOCALITY_RATIO Dominant-vs-next locality population ratio (default: 3) # WOF_PARENT_LOCALITY_MIN_SHARE Minimum child-cell share (0..1) required for locality parent takeover (default: 0.5) @@ -58,6 +59,7 @@ WOF_REGION_MAX_PRECISION="${WOF_REGION_MAX_PRECISION:-4}" WOF_REGION_SPARSE_MAX_PRECISION="${WOF_REGION_SPARSE_MAX_PRECISION:-3}" WOF_REGION_SPARSE_MIN_AREA_KM2="${WOF_REGION_SPARSE_MIN_AREA_KM2:-80000}" WOF_PROMOTE_LOCALITY_OVER_REGION="${WOF_PROMOTE_LOCALITY_OVER_REGION:-1}" +WOF_HOME_CELL_PRIORITY="${WOF_HOME_CELL_PRIORITY:-1}" WOF_DOMINANT_LOCALITY_POPULATION="${WOF_DOMINANT_LOCALITY_POPULATION:-100000}" WOF_DOMINANT_LOCALITY_RATIO="${WOF_DOMINANT_LOCALITY_RATIO:-3}" WOF_PARENT_LOCALITY_MIN_SHARE="${WOF_PARENT_LOCALITY_MIN_SHARE:-0.5}" @@ -138,6 +140,7 @@ COMMON_FLAGS=( --region-sparse-max-precision "${WOF_REGION_SPARSE_MAX_PRECISION}" --region-sparse-min-area-km2 "${WOF_REGION_SPARSE_MIN_AREA_KM2}" --promote-locality-over-region "${WOF_PROMOTE_LOCALITY_OVER_REGION}" + --home-cell-priority "${WOF_HOME_CELL_PRIORITY}" --dominant-locality-population "${WOF_DOMINANT_LOCALITY_POPULATION}" --dominant-locality-ratio "${WOF_DOMINANT_LOCALITY_RATIO}" --parent-locality-min-share "${WOF_PARENT_LOCALITY_MIN_SHARE}" diff --git a/spec/boundary_builder_merge_spec.js b/spec/boundary_builder_merge_spec.js index 6198d82..df8228a 100644 --- a/spec/boundary_builder_merge_spec.js +++ b/spec/boundary_builder_merge_spec.js @@ -215,13 +215,15 @@ describe('boundary builder append merge', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'offline-geocoder-merge-')); try { const dbPath = path.join(dir, 'legacy.sqlite'); + // Oldtown owns the contested cell but is centred outside it, so the + // population ordering is what decides this one. await createLegacyDatabase(dbPath, { id: 9001, name: 'Oldtown', countryId: 'AA', placetypeCode: 0, latitude: 0.066, - longitude: 0.286 + longitude: 0.10 }, contestedHash); const newvilleInput = writeFixture(dir, 'newville.geojson', [ @@ -252,4 +254,40 @@ describe('boundary builder append merge', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it('keeps a contested cell with the existing place that is centred in it', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'offline-geocoder-merge-')); + try { + const dbPath = path.join(dir, 'legacy.sqlite'); + // Oldtown's centroid is inside the contested cell, so the appended city + // takes the rest of the shared strip but not Oldtown's own cell. + await createLegacyDatabase(dbPath, { + id: 9001, + name: 'Oldtown', + countryId: 'AA', + placetypeCode: 0, + latitude: 0.066, + longitude: 0.286 + }, contestedHash); + + const newvilleInput = writeFixture(dir, 'newville.geojson', [ + boxFeature(2101, 'Newville', 'locality', 'BB', 750000, 0.28, 0, 0.60, 0.15) + ]); + + const result = runBuilder([ + '--database', dbPath, + '--input', newvilleInput, + '--append', + '--base-precision', '4', + '--max-precision', '5', + '--index-mode', 'compact' + ]); + + expect(result.status).toEqual(0); + expect(await lookupOwner(dbPath, contestedHash)).toEqual(9001); + expect(await lookupOwner(dbPath, smallOnlyHash)).toEqual(2101); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/spec/boundary_home_cell_spec.js b/spec/boundary_home_cell_spec.js new file mode 100644 index 0000000..0acc645 --- /dev/null +++ b/spec/boundary_home_cell_spec.js @@ -0,0 +1,228 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const sqlite3 = require('sqlite3'); +const geohash = require('../src/geohash'); + +function all(db, sql, params) { + return new Promise((resolve, reject) => { + db.all(sql, params || [], (err, rows) => (err ? reject(err) : resolve(rows || []))); + }); +} + +function close(db) { + return new Promise((resolve, reject) => { + db.close((err) => (err ? reject(err) : resolve())); + }); +} + +function runBuilder(args) { + return spawnSync('node', [ + path.join(__dirname, '..', 'scripts', 'generate_boundary_index.js') + ].concat(args), { encoding: 'utf8' }); +} + +// A rectangular place with an explicit label centroid, so a fixture can put a +// town's centre anywhere inside its own polygon (real border towns sit at the +// edge of their country, not in the middle of their bounding box). +function place(spec) { + return { + type: 'Feature', + id: spec.id, + properties: { + name: spec.name, + placetype: spec.placetype, + country_id: spec.countryId, + admin1_id: 1, + is_current: 1, + population: spec.population, + centroid_lat: spec.centroid[0], + centroid_lon: spec.centroid[1] + }, + geometry: { + type: 'Polygon', + coordinates: [[ + [spec.minLon, spec.minLat], + [spec.maxLon, spec.minLat], + [spec.maxLon, spec.maxLat], + [spec.minLon, spec.maxLat], + [spec.minLon, spec.minLat] + ]] + } + }; +} + +function writeFixture(dir, name, features) { + const filePath = path.join(dir, name); + fs.writeFileSync(filePath, JSON.stringify({ type: 'FeatureCollection', features: features })); + return filePath; +} + +// Mirror the runtime lookup in src/reverse.js: the compact index is walked by +// longest matching prefix, because a cell is only stored when it disagrees +// with the coarser cell above it. +async function lookupOwner(dbPath, hash) { + const prefixes = []; + for (let length = hash.length; length >= 1; length--) { + prefixes.push(hash.slice(0, length)); + } + + const db = new sqlite3.Database(dbPath); + try { + const rows = await all(db, ` + SELECT p.id AS id, p.name AS name, p.country_id AS country_id + FROM compact_geohash_lookup l + JOIN compact_places p ON p.id = l.place_id + WHERE l.geohash IN (${prefixes.map(() => '?').join(',')}) + ORDER BY LENGTH(l.geohash) DESC, p.placetype_code ASC, p.id ASC + LIMIT 1 + `, prefixes); + return rows.length ? rows[0] : null; + } finally { + await close(db); + } +} + +describe('boundary builder home cell ownership', () => { + // Two localities straddling a national border, sharing the cell s000q. + // BIGVILLE is 18x more populous and covers the whole shared strip; + // SMALLTOWN sits in the cell and is the only one centred there. + const BIGVILLE = { + id: 1001, + name: 'Bigville', + placetype: 'locality', + countryId: 'MX', + population: 690000, + minLon: 0, minLat: 0, maxLon: 0.30, maxLat: 0.15, + centroid: [0.07, 0.10] + }; + const SMALLTOWN = { + id: 2001, + name: 'Smalltown', + placetype: 'locality', + countryId: 'US', + population: 38000, + minLon: 0.28, minLat: 0, maxLon: 0.60, maxLat: 0.15, + centroid: [0.07, 0.29] + }; + + const homeHash = geohash.encode(SMALLTOWN.centroid[0], SMALLTOWN.centroid[1], 5); + // Also inside the shared strip, but holds neither centroid. + const sharedHash = geohash.encode(0.13, 0.29, 5); + + const commonFlags = ['--base-precision', '4', '--max-precision', '5', '--index-mode', 'compact']; + + function withTempDir(run) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'offline-geocoder-home-')); + return run(dir).finally(() => fs.rmSync(dir, { recursive: true, force: true })); + } + + it('gives a place the cell holding its own centroid over a larger neighbour', async () => { + await withTempDir(async (dir) => { + const input = writeFixture(dir, 'border.geojson', [place(BIGVILLE), place(SMALLTOWN)]); + const dbPath = path.join(dir, 'border.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + + const home = await lookupOwner(dbPath, homeHash); + expect(home.id).toEqual(SMALLTOWN.id); + expect(home.country_id).toEqual('US'); + + // The rule claims only the home cell: the rest of the shared strip + // still goes to the more populous place. + expect((await lookupOwner(dbPath, sharedHash)).id).toEqual(BIGVILLE.id); + }); + }); + + it('falls back to population when the rule is disabled', async () => { + await withTempDir(async (dir) => { + const input = writeFixture(dir, 'border.geojson', [place(BIGVILLE), place(SMALLTOWN)]); + const dbPath = path.join(dir, 'border.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--home-cell-priority', 'false' + ].concat(commonFlags)).status).toEqual(0); + + const home = await lookupOwner(dbPath, homeHash); + expect(home.id).toEqual(BIGVILLE.id); + expect(home.country_id).toEqual('MX'); + }); + }); + + it('still folds a minor locality into a dominant city inside one country', async () => { + await withTempDir(async (dir) => { + // Same geometry, one country: the metro rollup is a deliberate labelling + // choice ("this whole area reads as the big city") and stays untouched. + const input = writeFixture(dir, 'metro.geojson', [ + place(BIGVILLE), + place(Object.assign({}, SMALLTOWN, { countryId: 'MX' })) + ]); + const dbPath = path.join(dir, 'metro.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + expect((await lookupOwner(dbPath, homeHash)).id).toEqual(BIGVILLE.id); + }); + }); + + it('leaves cells shared by two centred places to the population ordering', async () => { + await withTempDir(async (dir) => { + // Both centroids now land in s000q, so neither claim is privileged. + const input = writeFixture(dir, 'twins.geojson', [ + place(Object.assign({}, BIGVILLE, { centroid: [0.075, 0.292] })), + place(Object.assign({}, SMALLTOWN, { centroid: [0.065, 0.288] })) + ]); + const dbPath = path.join(dir, 'twins.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + expect((await lookupOwner(dbPath, homeHash)).id).toEqual(BIGVILLE.id); + }); + }); + + it('keeps placetype rank ahead of the home cell claim', async () => { + await withTempDir(async (dir) => { + // The county is centred in s000q and the locality is not, but a county + // label is a worse answer than a locality label for a point inside both. + const input = writeFixture(dir, 'county.geojson', [ + place(BIGVILLE), + place({ + id: 3001, + name: 'Border County', + placetype: 'county', + countryId: 'US', + population: 0, + minLon: 0.28, minLat: 0, maxLon: 0.60, maxLat: 0.15, + centroid: [0.07, 0.29] + }) + ]); + const dbPath = path.join(dir, 'county.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--include-county', 'true' + ].concat(commonFlags)).status).toEqual(0); + + expect((await lookupOwner(dbPath, homeHash)).id).toEqual(BIGVILLE.id); + }); + }); + + it('resolves the home cell the same way in either append order', async () => { + await withTempDir(async (dir) => { + const bigInput = writeFixture(dir, 'big.geojson', [place(BIGVILLE)]); + const smallInput = writeFixture(dir, 'small.geojson', [place(SMALLTOWN)]); + + const bigFirstDb = path.join(dir, 'big-first.sqlite'); + const smallFirstDb = path.join(dir, 'small-first.sqlite'); + + expect(runBuilder(['--database', bigFirstDb, '--input', bigInput].concat(commonFlags)).status).toEqual(0); + expect(runBuilder(['--database', bigFirstDb, '--input', smallInput, '--append'].concat(commonFlags)).status).toEqual(0); + + expect(runBuilder(['--database', smallFirstDb, '--input', smallInput].concat(commonFlags)).status).toEqual(0); + expect(runBuilder(['--database', smallFirstDb, '--input', bigInput, '--append'].concat(commonFlags)).status).toEqual(0); + + for (const dbPath of [bigFirstDb, smallFirstDb]) { + expect((await lookupOwner(dbPath, homeHash)).id).toEqual(SMALLTOWN.id); + expect((await lookupOwner(dbPath, sharedHash)).id).toEqual(BIGVILLE.id); + } + }); + }); +}); From 34bb9688142df070d9687ab207347fec48da21cc Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 21:30:13 -0600 Subject: [PATCH 2/6] Enforce home-cell ownership across nested and promoted cells The home-cell rule was applied in two places that each look at a single exact hash, so it left two gaps. The rollup's cross-border protection only ran in the descendant sweep, which skips hashes no longer than the promoted parent. When a foreign town's home cell was the parent cell itself, the promotion overwrote it before anything could protect it, and the town's own centre resolved across the border. The parent is now checked with the same rule before it is replaced; the promotion is refused rather than inverted, so cells the dominant city covers itself still answer with the dominant city. The comparator only ever ranks places that emitted the same hash, because candidates are looked up by exact hash. Cover cells do not nest within one place but do across places, so a town could own the coarse cell holding its centroid while a neighbour emitted a finer cell over the same point - and the runtime's longest-prefix walk answered with the neighbour. A reconciliation pass now replays the ownership rule down the chain of cells containing each centroid, using the same comparator, so placetype rank still decides first and a cell shared by two centroids still goes by population. It only reassigns cells that already exist, so the index cannot grow; the redundant-row pass usually drops the reassigned row entirely. Measured on the 7-country slice used for this branch (us/mx/nl/de/at/ch/cz, 13,163 locality centroids, precision 4->5, min population 5,000): both gaps fire zero times and the database is byte-identical, 467,585 rows either way. All 302 nested home-cell conflicts in that data are a region or county losing to a finer locality or county cell, which placetype rank already decides correctly; none are same-rank. The six remaining wrong-country centroids (Ebbs, Au, Kreuzlingen, St. Margrethen, Freilassing, Simbach a. Inn) all share a precision-5 cell with the winner, so population legitimately decides them. The fixes are proven by red-green specs rather than by the slice. The build summary now reports both counters so a world build can show whether either rule ever fires. --- CHANGELOG | 10 ++- README.md | 2 +- scripts/generate_boundary_index.js | 120 ++++++++++++++++++++++++++--- spec/boundary_home_cell_spec.js | 118 ++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5830b5e..3647bad 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,8 +7,14 @@ holding two centroids are still decided by population. Disable with `--home-cell-priority false` (`WOF_HOME_CELL_PRIORITY=0`). - Stop the dominant-city rollup absorbing the home cell of a town in another - country. Rollups inside a country are unchanged, so a metro still reads as - one city. + country, whether that cell is a descendant of the promoted parent or the + promoted parent itself. Rollups inside a country are unchanged, so a metro + still reads as one city. +- Apply home-cell ownership across nested cover cells, not only within a single + hash. A place that owns the coarse cell holding its centroid now also keeps + the finer cells over that same point when a neighbour emits them, so the rule + holds for the longest-prefix lookup the runtime actually performs. Placetype + ranking still decides first, and no cell is added to the index. - Fix `--append` builds handing contested geohash cells to whichever batch ran last. Cells that already belong to a better-ranked place (by placetype, then population, then centroid distance) now keep their existing owner, so border diff --git a/README.md b/README.md index 2279b22..fb4b5b0 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ Builder notes: - `--region-max-precision` - `--region-sparse-max-precision` + `--region-sparse-min-area-km2` for very large sparse regions (for example geohash-3 in Amazon-like interiors) - `--promote-locality-over-region` (default `true`) prefers locality labels in shared parent cells when there is no competing locality (keeps city labels sticky against region-only outskirts) -- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. The dominant-city rollup keeps a foreign town's home cell for the same reason; rollups within one country are unaffected +- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. Because the index is walked by longest matching prefix, the claim is enforced across nested cells too: a place that owns the coarse cell holding its centroid also keeps the finer cells over that point. The dominant-city rollup keeps a foreign town's home cell for the same reason, whether that cell is the promoted parent or one of its descendants; rollups within one country are unaffected - Dominant-city rollup keeps broad city labels sticky in mixed city/suburb cells unless there is competing major-city pressure: - `--dominant-locality-population` (default `100000`) - `--dominant-locality-ratio` (default `3`) diff --git a/scripts/generate_boundary_index.js b/scripts/generate_boundary_index.js index f22ddd1..bc76ac2 100755 --- a/scripts/generate_boundary_index.js +++ b/scripts/generate_boundary_index.js @@ -1115,15 +1115,93 @@ function localityShareMeetsThreshold(localityId, group, opts) { return localityShareInParent(localityId, group) >= threshold } +// Minor localities are folded into the dominant city, which is what makes a +// metro read as one name. The exception is a town in another country: +// absorbing the cell holding its centroid would put its own centre on the +// wrong side of a border, which no label choice can justify. Intra-country +// absorption is deliberate and stays untouched. +function protectsForeignHomeCell(place, hash, dominantPlace, opts) { + if (!opts.homeCellPriority || !place) { + return false + } + + return isCityPlacetypeCode(place.placetypeCode) && + ownsHomeCell(place, hash) && + isForeignTo(place, dominantPlace) +} + +// The cell map is keyed by exact hash, so the comparator only ever ranks +// places that emitted the *same* cell. Cover cells never nest within one +// place, but they do nest across places: a town can win the coarse cell that +// holds its centroid while a neighbour emits a finer cell over the same point, +// and the runtime's longest-prefix walk then answers with the neighbour. +// Replay the ownership rule down the chain of cells containing the centroid so +// the home-cell guarantee holds at lookup time, not merely per hash. Only +// existing cells are reassigned - no new cell is created, so this cannot grow +// the index. +function reconcileNestedHomeCells(bestByHash, placeById, opts) { + if (!opts.homeCellPriority) { + return 0 + } + + var hashes = Object.keys(bestByHash) + var deepest = 0 + for (var lengthIndex = 0; lengthIndex < hashes.length; lengthIndex++) { + if (hashes[lengthIndex].length > deepest) { + deepest = hashes[lengthIndex].length + } + } + + var hashCenterCache = Object.create(null) + var reassigned = 0 + + for (var i = 0; i < hashes.length; i++) { + var hash = hashes[i] + var owner = bestByHash[hash] + if (owner === undefined) { + continue + } + + var place = placeById[String(owner)] + if (!place || !ownsHomeCell(place, hash)) { + continue + } + + for (var length = hash.length + 1; length <= deepest; length++) { + var finerHash = geohash.encode(place.centroidLat, place.centroidLon, length) + var incumbentId = bestByHash[finerHash] + if (incumbentId === undefined || Number(incumbentId) === Number(place.id)) { + continue + } + + var incumbent = placeById[String(incumbentId)] + if (!incumbent) { + continue + } + + // Reassign only when the same comparator that governs a single cell + // would hand this one over, so placetype rank still outranks the home + // cell and a cell shared by two centroids keeps its population ordering. + if (comparePlacesForHash(place, incumbent, finerHash, hashCenterCache, true) < 0) { + bestByHash[finerHash] = place.id + reassigned += 1 + } + } + } + + return reassigned +} + function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) { if (!opts.promoteLocalityOverRegion) { - return + return 0 } + var refusedPromotions = 0 var minPrecision = Number(opts.basePrecision || 1) var maxPrecision = Number(opts.maxPrecision || minPrecision) if (maxPrecision <= minPrecision) { - return + return refusedPromotions } for (var precision = maxPrecision - 1; precision >= minPrecision; precision--) { @@ -1215,6 +1293,15 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) } } + // The parent cell can itself be the home cell of a town across the + // border. The descendant sweep below only visits hashes longer than + // `precision`, so without this check the promotion would overwrite that + // town's own centroid cell and never get the chance to protect it. + if (protectsForeignHomeCell(existingPlace, parentHash, placeById[String(promotion.localityId)], opts)) { + refusedPromotions += 1 + continue + } + bestByHash[parentHash] = promotion.localityId promotedParents[parentHash] = promotion } @@ -1249,13 +1336,12 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) continue } - // Minor localities are folded into the dominant city, which is what - // makes a metro read as one name. The exception is a town in another - // country: absorbing the cell it sits in would put it on the wrong side - // of a border, which no label choice can justify. - var protectedHomeCell = opts.homeCellPriority && - ownsHomeCell(descendantPlace, descendantHash) && - isForeignTo(descendantPlace, placeById[String(promoted.localityId)]) + var protectedHomeCell = protectsForeignHomeCell( + descendantPlace, + descendantHash, + placeById[String(promoted.localityId)], + opts + ) if (promoted.suppressMinorLocalities && isCityPlacetypeCode(descendantPlace.placetypeCode) && @@ -1265,6 +1351,8 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) } } } + + return refusedPromotions } function buildCompactLookupRows(places, opts) { @@ -1295,7 +1383,11 @@ function buildCompactLookupRows(places, opts) { bestByHashId[currentHash] = bestByHash[currentHash].id } - promoteLocalityParentsByRegionCompetition(bestByHashId, placeById, opts) + // Runs before the rollup so the metro rollup keeps the last word on which + // cells a dominant city absorbs inside its own country. + var nestedHomeCellFixes = reconcileNestedHomeCells(bestByHashId, placeById, opts) + + var refusedPromotions = promoteLocalityParentsByRegionCompetition(bestByHashId, placeById, opts) var rows = Object.keys(bestByHashId).map(function(hash) { return { @@ -1338,7 +1430,9 @@ function buildCompactLookupRows(places, opts) { return { rows: compact, - placeById: placeById + placeById: placeById, + nestedHomeCellFixes: nestedHomeCellFixes, + refusedPromotions: refusedPromotions } } @@ -1996,6 +2090,10 @@ async function main() { console.log('Mode: ' + modeLabel) console.log('Precision: ' + options.basePrecision + ' -> ' + options.maxPrecision) console.log('Home cell priority: ' + (options.homeCellPriority ? 'true' : 'false')) + if (compactBuild) { + console.log('Home cells kept from a nested foreign cell: ' + (compactBuild.nestedHomeCellFixes || 0)) + console.log('Parent promotions refused over a foreign home cell: ' + (compactBuild.refusedPromotions || 0)) + } console.log('Placetype precision caps: locality=' + options.localityMaxPrecision + ', localadmin=' + options.localadminMaxPrecision + ', county=' + options.countyMaxPrecision + ', region=' + options.regionMaxPrecision) if (Number.isFinite(options.regionSparseMaxPrecision) && Number.isFinite(options.regionSparseMinAreaKm2)) { console.log('Sparse region rule: area_km2>=' + options.regionSparseMinAreaKm2 + ' => max_precision=' + options.regionSparseMaxPrecision) diff --git a/spec/boundary_home_cell_spec.js b/spec/boundary_home_cell_spec.js index 0acc645..670789d 100644 --- a/spec/boundary_home_cell_spec.js +++ b/spec/boundary_home_cell_spec.js @@ -225,4 +225,122 @@ describe('boundary builder home cell ownership', () => { } }); }); + + // The cases below turn on a place whose polygon swallows a whole + // base-precision cell, so its cover terminates at precision 4 and the cell + // holding its centre is a *coarse* one. That is what puts it out of reach + // of the two rules that only ever look at one exact hash: the rollup's + // descendant sweep (which skips hashes no longer than the parent) and the + // comparator (which only ranks places that emitted the same hash). + describe('across nested cover cells', () => { + // s000 spans lon 0..0.3515625, lat 0..0.17578125 and splits into 8x4 + // children at precision 5. + const PARENT_HASH = 's000'; + + // A rural town on the far side of the border. Its polygon contains all of + // s000, so s000 is both its cover cell and its home cell. + const HOMELAND = { + id: 4001, + name: 'Homeland', + placetype: 'locality', + countryId: 'US', + population: 40000, + minLon: -0.05, minLat: -0.05, maxLon: 0.40, maxLat: 0.22, + centroid: [0.02, 0.02] + }; + + // The city across the border: seven times the population, covering the + // eastern six of the eight child columns (24 of 32 cells). + const METROPOLIS = { + id: 4002, + name: 'Metropolis', + placetype: 'locality', + countryId: 'MX', + population: 690000, + minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, + centroid: [0.07, 0.29] + }; + + // A small neighbour in the same country as Metropolis, present only so the + // rollup takes its dominant-city branch instead of the single-locality one. + const SUBURB = { + id: 4003, + name: 'Suburb', + placetype: 'locality', + countryId: 'MX', + population: 20000, + minLon: 0.045, minLat: 0.002, maxLon: 0.086, maxLat: 0.085, + centroid: [0.04, 0.06] + }; + + const metropolisCell = geohash.encode(0.06, 0.285, 5); + + it('keeps a foreign home cell when the rollup promotes that very cell', async () => { + await withTempDir(async (dir) => { + // Homeland's centre sits in the one child column no Mexican place + // covers, so nothing finer than s000 answers for it: if the promotion + // takes s000, the town's own centre reads as Mexico. + const input = writeFixture(dir, 'promote.geojson', [ + place(HOMELAND), place(METROPOLIS), place(SUBURB) + ]); + const dbPath = path.join(dir, 'promote.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + + const homeCell = geohash.encode(HOMELAND.centroid[0], HOMELAND.centroid[1], 5); + expect(homeCell.slice(0, PARENT_HASH.length)).toEqual(PARENT_HASH); + + const owner = await lookupOwner(dbPath, homeCell); + expect(owner.id).toEqual(HOMELAND.id); + expect(owner.country_id).toEqual('US'); + + // The promotion is refused, not inverted: cells Metropolis covers + // itself still answer with Metropolis. + expect((await lookupOwner(dbPath, metropolisCell)).id).toEqual(METROPOLIS.id); + }); + }); + + it('keeps a coarse home cell against a finer cell emitted by a neighbour', async () => { + await withTempDir(async (dir) => { + // Same geometry, but Homeland's centre now sits in a child cell + // Metropolis covers. Neither place emits the other's hash, so no + // single-cell comparison ever ranks them and the runtime's + // longest-prefix walk answers with the finer foreign row. + const input = writeFixture(dir, 'nested.geojson', [ + place(Object.assign({}, HOMELAND, { centroid: [0.11, 0.11] })), + place(METROPOLIS) + ]); + const dbPath = path.join(dir, 'nested.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + + const homeCell = geohash.encode(0.11, 0.11, 5); + expect(homeCell.length).toEqual(5); + expect(homeCell.slice(0, PARENT_HASH.length)).toEqual(PARENT_HASH); + + const owner = await lookupOwner(dbPath, homeCell); + expect(owner.id).toEqual(HOMELAND.id); + expect(owner.country_id).toEqual('US'); + + // Only the cell holding the centre changes hands. + expect((await lookupOwner(dbPath, metropolisCell)).id).toEqual(METROPOLIS.id); + }); + }); + + it('leaves the nested cell alone when the rule is disabled', async () => { + await withTempDir(async (dir) => { + const input = writeFixture(dir, 'nested.geojson', [ + place(Object.assign({}, HOMELAND, { centroid: [0.11, 0.11] })), + place(METROPOLIS) + ]); + const dbPath = path.join(dir, 'nested-off.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--home-cell-priority', 'false' + ].concat(commonFlags)).status).toEqual(0); + + expect((await lookupOwner(dbPath, geohash.encode(0.11, 0.11, 5))).id).toEqual(METROPOLIS.id); + }); + }); + }); }); From 2e9c722825f301b29a38c52307acbd23f839d580 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Thu, 20 Aug 2026 02:09:10 -0600 Subject: [PATCH 3/6] Stop the rollup stripping a town of the cell holding its own centre Measuring the branch against a batched build - one country per --append invocation, the shape the world database is actually built in - turned up three border towns whose centres crossed the border there but not in a single-batch build: Douglas AZ read as Agua Prieta, and Mittelberg and Riezlern in the Austrian Kleinwalsertal read as Oberstdorf. The cause is not the append merge. In each case a *county* dominated the parent cell and the rollup's descendant sweep deleted the locality's own home cell, because isCityPlacetypeCode lumps counties in with localities. Inside one batch that is merely a worse label; across batches it removes the only row defending the cell, and the neighbouring country written earlier keeps it. The rollup now only folds a place into a better- or equally-ranked label, which is what the cell comparator does anyway - it ranks placetype before everything else. Rollups between localities, which are what make a metro read as one city, are untouched. Two smaller fixes to the nested home-cell pass added last commit: Redundancy in the compact index was judged against any matching ancestor prefix, so a row was dropped whenever some coarser cell named the same place - even when a different place owned the cell in between, which the runtime's longest-prefix walk answers with instead. It is now judged against the nearest kept ancestor. The pass also read cell ownership as it mutated it, so a place that lost its own cell to a coarser claimant never carried its claim to the cells below, and the result depended on the order places were read. Claims are now collected before any is applied; each cell is still settled by a running maximum under the comparator, so the order no longer matters. Measured on the 7-country slice (us/mx/nl/de/at/ch/cz, min population 5,000, precision 4->5), built both as one batch and as seven appends: build centroids wrong-country lookup rows single batch, before 13,163 6 467,585 single batch, after 13,163 6 470,258 seven appends, before 13,192 9 467,772 seven appends, after 13,192 6 470,426 The batched build now agrees with the single-batch one, so the border answer no longer depends on how countries are partitioned into batches. The rollup guard is strictly additive: 2,673 new rows (+0.6%, +116 KB), every one of them owned by a locality, and not one existing cell changes hands. The other two fixes are inert on this data - zero shadowed rows across all seven batches, byte-identical databases - and are proven by red-green specs instead. The six remaining wrong-country centroids all share a precision-5 cell with the winner, so population decides them; only deeper precision moves those. --- CHANGELOG | 15 +++ README.md | 1 + scripts/generate_boundary_index.js | 83 ++++++++++++----- spec/boundary_home_cell_spec.js | 141 +++++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 25 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3647bad..fc7537c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,21 @@ the finer cells over that same point when a neighbour emits them, so the rule holds for the longest-prefix lookup the runtime actually performs. Placetype ranking still decides first, and no cell is added to the index. +- Stop the dominant-city rollup suppressing a place whose placetype it does not + outrank. A county could absorb the cell holding a town's own centre, which is + a downgrade the cell comparator would never make - and because the rollup runs + per batch, it left that cell undefended in an `--append` build, so the next + country written took it and the town's centre resolved across the border + (Douglas AZ reading as Agua Prieta; Mittelberg and Riezlern, the Austrian + Kleinwalsertal, reading as Oberstdorf). Rollups between localities, which are + what make a metro read as one city, are unchanged. +- Judge compact-index redundancy against the nearest kept ancestor cell instead + of any matching prefix. A row was dropped whenever some coarser cell named the + same place, even when a different place owned the cell in between, and the + lookup then answered with that other place. +- Reconcile nested home cells from a snapshot of cell ownership, so a place that + loses its own cell to a coarser claimant still carries its claim to the cells + below it. The pass no longer depends on the order places are read. - Fix `--append` builds handing contested geohash cells to whichever batch ran last. Cells that already belong to a better-ranked place (by placetype, then population, then centroid distance) now keep their existing owner, so border diff --git a/README.md b/README.md index fb4b5b0..e4b71ee 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,7 @@ Builder notes: - Dominant-city rollup keeps broad city labels sticky in mixed city/suburb cells unless there is competing major-city pressure: - `--dominant-locality-population` (default `100000`) - `--dominant-locality-ratio` (default `3`) + - A rollup only folds a place into a better- or equally-ranked label, so a county never absorbs a locality: that would be a downgrade, and it would also strip the town of the cell holding its own centre - Parent-cell takeover guard: - `--parent-locality-min-share` (default `0.5`) requires locality ownership of at least that child-cell share before replacing a parent cell label - Excludes neighbourhood-like placetypes from default reverse output diff --git a/scripts/generate_boundary_index.js b/scripts/generate_boundary_index.js index bc76ac2..8afa680 100755 --- a/scripts/generate_boundary_index.js +++ b/scripts/generate_boundary_index.js @@ -1144,33 +1144,40 @@ function reconcileNestedHomeCells(bestByHash, placeById, opts) { return 0 } + // Collect every claim before applying any of it. A claimant can lose its + // own cell to a coarser claimant partway through the pass, and reading the + // owner map as it mutates would then drop the displaced place's claim on the + // cells below - which made the result depend on the order places were read. + // Each cell is still settled by a running maximum under the comparator, so + // applying the claims in any order gives the same map. var hashes = Object.keys(bestByHash) var deepest = 0 - for (var lengthIndex = 0; lengthIndex < hashes.length; lengthIndex++) { - if (hashes[lengthIndex].length > deepest) { - deepest = hashes[lengthIndex].length - } - } - - var hashCenterCache = Object.create(null) - var reassigned = 0 + var claims = [] for (var i = 0; i < hashes.length; i++) { var hash = hashes[i] - var owner = bestByHash[hash] - if (owner === undefined) { - continue + if (hash.length > deepest) { + deepest = hash.length } - var place = placeById[String(owner)] + var place = placeById[String(bestByHash[hash])] if (!place || !ownsHomeCell(place, hash)) { continue } - for (var length = hash.length + 1; length <= deepest; length++) { - var finerHash = geohash.encode(place.centroidLat, place.centroidLon, length) + claims.push({ place: place, precision: hash.length }) + } + + var hashCenterCache = Object.create(null) + var reassigned = 0 + + for (var claimIndex = 0; claimIndex < claims.length; claimIndex++) { + var claimant = claims[claimIndex].place + + for (var length = claims[claimIndex].precision + 1; length <= deepest; length++) { + var finerHash = geohash.encode(claimant.centroidLat, claimant.centroidLon, length) var incumbentId = bestByHash[finerHash] - if (incumbentId === undefined || Number(incumbentId) === Number(place.id)) { + if (incumbentId === undefined || Number(incumbentId) === Number(claimant.id)) { continue } @@ -1182,8 +1189,8 @@ function reconcileNestedHomeCells(bestByHash, placeById, opts) { // Reassign only when the same comparator that governs a single cell // would hand this one over, so placetype rank still outranks the home // cell and a cell shared by two centroids keeps its population ordering. - if (comparePlacesForHash(place, incumbent, finerHash, hashCenterCache, true) < 0) { - bestByHash[finerHash] = place.id + if (comparePlacesForHash(claimant, incumbent, finerHash, hashCenterCache, true) < 0) { + bestByHash[finerHash] = claimant.id reassigned += 1 } } @@ -1336,16 +1343,27 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) continue } + var dominantPlace = placeById[String(promoted.localityId)] var protectedHomeCell = protectsForeignHomeCell( descendantPlace, descendantHash, - placeById[String(promoted.localityId)], + dominantPlace, opts ) + // The rollup makes a metro read as one city, so it may only fold a place + // into a better- or equally-ranked label. A county absorbing a locality + // is a downgrade the comparator would never make, and it also strips the + // town of the cell holding its own centre - which in an append build is + // then taken by whichever neighbouring country was written first, + // because nothing is left to defend it. + var dominantOutranks = dominantPlace && + placetypeRank(dominantPlace.placetype) <= placetypeRank(descendantPlace.placetype) + if (promoted.suppressMinorLocalities && isCityPlacetypeCode(descendantPlace.placetypeCode) && !isMajorLocality(descendantPlace, opts) && + dominantOutranks && !protectedHomeCell) { delete bestByHash[descendantHash] } @@ -1407,23 +1425,36 @@ function buildCompactLookupRows(places, opts) { var compact = [] var selectedByHash = Object.create(null) + var shadowedRowsKept = 0 for (var index = 0; index < rows.length; index++) { var row = rows[index] - var redundant = false - for (var precision = 1; precision < row.geohash.length; precision++) { - var prefix = row.geohash.slice(0, precision) - if (selectedByHash[prefix] === row.placeId) { - redundant = true + // Only the *nearest* kept ancestor decides whether a row can be dropped. + // The runtime answers with the longest matching prefix, so a row is + // redundant only when the cell that would answer in its place already + // names the same place. Matching any ancestor drops rows that a nearer, + // different owner shadows, and the lookup then returns that other place. + var nearestAncestor + for (var precision = row.geohash.length - 1; precision >= 1; precision--) { + var ancestor = selectedByHash[row.geohash.slice(0, precision)] + if (ancestor !== undefined) { + nearestAncestor = ancestor break } } - if (redundant) { + if (nearestAncestor === row.placeId) { continue } + for (var farther = row.geohash.length - 1; farther >= 1; farther--) { + if (selectedByHash[row.geohash.slice(0, farther)] === row.placeId) { + shadowedRowsKept += 1 + break + } + } + selectedByHash[row.geohash] = row.placeId compact.push(row) } @@ -1432,7 +1463,8 @@ function buildCompactLookupRows(places, opts) { rows: compact, placeById: placeById, nestedHomeCellFixes: nestedHomeCellFixes, - refusedPromotions: refusedPromotions + refusedPromotions: refusedPromotions, + shadowedRowsKept: shadowedRowsKept } } @@ -2093,6 +2125,7 @@ async function main() { if (compactBuild) { console.log('Home cells kept from a nested foreign cell: ' + (compactBuild.nestedHomeCellFixes || 0)) console.log('Parent promotions refused over a foreign home cell: ' + (compactBuild.refusedPromotions || 0)) + console.log('Rows kept from a nearer shadowing owner: ' + (compactBuild.shadowedRowsKept || 0)) } console.log('Placetype precision caps: locality=' + options.localityMaxPrecision + ', localadmin=' + options.localadminMaxPrecision + ', county=' + options.countyMaxPrecision + ', region=' + options.regionMaxPrecision) if (Number.isFinite(options.regionSparseMaxPrecision) && Number.isFinite(options.regionSparseMinAreaKm2)) { diff --git a/spec/boundary_home_cell_spec.js b/spec/boundary_home_cell_spec.js index 670789d..8c6896b 100644 --- a/spec/boundary_home_cell_spec.js +++ b/spec/boundary_home_cell_spec.js @@ -205,6 +205,47 @@ describe('boundary builder home cell ownership', () => { }); }); + it('will not let a county rollup swallow a locality it outranks', async () => { + await withTempDir(async (dir) => { + // A county dominates its parent cell on population and would suppress + // every minor place under it - including the locality whose centre is + // there. That is a label downgrade the comparator would never make, + // and in an append build it leaves the cell undefended: the next + // country written takes it, and the town's centre crosses the border. + const county = { + id: 6001, + name: 'Border County', + placetype: 'county', + countryId: 'US', + population: 400000, + minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, + centroid: [0.07, 0.29] + }; + const town = { + id: 6002, + name: 'County Town', + placetype: 'locality', + countryId: 'US', + population: 20000, + minLon: 0.045, minLat: 0.002, maxLon: 0.086, maxLat: 0.085, + centroid: [0.04, 0.06] + }; + + const input = writeFixture(dir, 'county-rollup.geojson', [place(county), place(town)]); + const dbPath = path.join(dir, 'county-rollup.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--include-county', 'true' + ].concat(commonFlags)).status).toEqual(0); + + const owner = await lookupOwner(dbPath, geohash.encode(town.centroid[0], town.centroid[1], 5)); + expect(owner.id).toEqual(town.id); + + // The county still holds the cells it covers on its own. + expect((await lookupOwner(dbPath, geohash.encode(0.06, 0.285, 5))).id).toEqual(county.id); + }); + }); + it('resolves the home cell the same way in either append order', async () => { await withTempDir(async (dir) => { const bigInput = writeFixture(dir, 'big.geojson', [place(BIGVILLE)]); @@ -343,4 +384,104 @@ describe('boundary builder home cell ownership', () => { }); }); }); + + // Three precisions are needed to see these two: a claim has to be able to + // lose one level and still matter at the next, which cannot happen when the + // index only holds two. Base precision 3 gives the cells s00 > s000 > s0000. + describe('with three levels of nesting', () => { + const deepFlags = ['--base-precision', '3', '--max-precision', '5', '--index-mode', 'compact']; + + // Covers all of s00, so its cover terminates at precision 3. + const PROVINCE_TOWN = { + id: 5001, + name: 'Province Town', + placetype: 'locality', + countryId: 'US', + population: 100000, + minLon: -0.05, minLat: -0.05, maxLon: 1.45, maxLat: 1.45, + centroid: [0.02, 0.02] + }; + + // Covers all of s000, so its cover terminates at precision 4, one level + // inside Province Town. Its centre is in s000 but not in s0000. + const INNER_CITY = { + id: 5002, + name: 'Inner City', + placetype: 'locality', + countryId: 'MX', + population: 500000, + minLon: -0.02, minLat: -0.02, maxLon: 0.37, maxLat: 0.19, + centroid: [0.11, 0.11] + }; + + // Clips the corner of s0000 from outside s00, so it emits that one cell at + // precision 5 and is centred nowhere near it. + const CORNER_TOWN = { + id: 5003, + name: 'Corner Town', + placetype: 'locality', + countryId: 'MX', + population: 200000, + minLon: -0.30, minLat: -0.30, maxLon: 0.02, maxLat: 0.02, + centroid: [-0.15, -0.15] + }; + + it('keeps a reclaimed cell that a nearer owner shadows', async () => { + await withTempDir(async (dir) => { + // Province Town owns s00 and is centred in it, so it claims down the + // chain: it loses s000 to the bigger Inner City on population but wins + // s0000 from Corner Town, which is not centred there. That leaves + // s00=Province Town, s000=Inner City, s0000=Province Town - and the + // s0000 row only survives compaction if redundancy is judged against + // the nearest kept ancestor rather than any matching prefix. + const input = writeFixture(dir, 'shadow.geojson', [ + place(PROVINCE_TOWN), place(INNER_CITY), place(CORNER_TOWN) + ]); + const dbPath = path.join(dir, 'shadow.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(deepFlags)).status).toEqual(0); + + const owner = await lookupOwner(dbPath, geohash.encode(0.02, 0.02, 5)); + expect(owner.id).toEqual(PROVINCE_TOWN.id); + expect(owner.country_id).toEqual('US'); + + // Inner City still holds the level in between. + expect((await lookupOwner(dbPath, geohash.encode(0.11, 0.11, 5))).id).toEqual(INNER_CITY.id); + }); + }); + + it('propagates a displaced place\'s claim in either read order', async () => { + await withTempDir(async (dir) => { + // Province Town takes s000 from Inner City here (it is the more + // populous of the two), so Inner City's own claim on the cell holding + // its centre has to survive losing the cell it was claiming from. + const outer = Object.assign({}, PROVINCE_TOWN, { population: 900000 }); + const inner = Object.assign({}, INNER_CITY, { population: 300000 }); + const neighbour = { + id: 5004, + name: 'Neighbour City', + placetype: 'locality', + countryId: 'MX', + population: 690000, + minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, + centroid: [0.07, 0.29] + }; + + const orders = { + 'outer-first': [place(outer), place(inner), place(neighbour)], + 'inner-first': [place(neighbour), place(inner), place(outer)] + }; + + for (const [label, features] of Object.entries(orders)) { + const input = writeFixture(dir, `order-${label}.geojson`, features); + const dbPath = path.join(dir, `order-${label}.sqlite`); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(deepFlags)).status).toEqual(0); + + const owner = await lookupOwner(dbPath, geohash.encode(inner.centroid[0], inner.centroid[1], 5)); + expect(`${label}:${owner.id}`).toEqual(`${label}:${inner.id}`); + } + }); + }); + }); }); From 7521d561ac0093f149a84e3016c6e6db18f73170 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Thu, 20 Aug 2026 11:05:01 -0600 Subject: [PATCH 4/6] Scope home-cell guarantee before curation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e4b71ee..e72556f 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ Builder notes: - `--region-max-precision` - `--region-sparse-max-precision` + `--region-sparse-min-area-km2` for very large sparse regions (for example geohash-3 in Amazon-like interiors) - `--promote-locality-over-region` (default `true`) prefers locality labels in shared parent cells when there is no competing locality (keeps city labels sticky against region-only outskirts) -- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. Because the index is walked by longest matching prefix, the claim is enforced across nested cells too: a place that owns the coarse cell holding its centroid also keeps the finer cells over that point. The dominant-city rollup keeps a foreign town's home cell for the same reason, whether that cell is the promoted parent or one of its descendants; rollups within one country are unaffected +- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. Because the index is walked by longest matching prefix, the claim is enforced across nested cells too: a place that owns the coarse cell holding its centroid also keeps the finer cells over that point. The dominant-city rollup keeps a foreign town's home cell for the same reason, whether that cell is the promoted parent or one of its descendants; rollups within one country are unaffected. This is a guarantee of the generated, uncurated database; an explicitly applied [curation overlay](curation/README.md) is the final labeling layer and may deliberately reassign matching cells, including an absorbed place's home cell - Dominant-city rollup keeps broad city labels sticky in mixed city/suburb cells unless there is competing major-city pressure: - `--dominant-locality-population` (default `100000`) - `--dominant-locality-ratio` (default `3`) From 67c3b5c3d225eee0bb49ecf04d4cc85557f32438 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Thu, 20 Aug 2026 11:13:00 -0600 Subject: [PATCH 5/6] Close the rank guard at the parent and claim from every place's cover Two follow-ons to the rank guard, both settled the same way as the last round: build the 7-country slice as seven separate --append invocations, which is the topology the world database is built in, and count. The rank guard only covered the descendant sweep, so a county dominating the child cells could still replace a minor same-country locality at the promoted parent itself. A locality whose cover terminates at that parent has no descendant row for the sweep to protect, so its centroid resolved to the county - the exact downgrade the guard was added to prevent. The same rank check now runs before the parent is replaced. Nested home-cell claims were collected from the cells a place had won. When two same-rank places are centred in the same coarse cell the population tie-break gives it to one of them, and the loser's claim vanished even though it still covers the finer cells over its own centre and still outranks a third place that is not centred there. Claims now come from each place's own cover, which does not depend on what it won. Both are inert on this data: seven-append and single-batch databases are byte-identical to 2e9c722 - 0 rows changed, 0 added - and wrong-country stays at 6 batched, 6 single, 470,426 / 470,258 rows. They are proven by red-green specs, not by the slice. The build summary now reports the rollup and home-cell invariants, which is what settled the two findings not taken. Across all seven batches: foreign home cells kept below the promoted rank, 0; parent promotions refused on placetype rank, 0; promotions shadowing an ancestor home cell, 2 - both same-country and both over a lower-ranked ancestor, which is the locality-over-region rule working. On the finished index, 333 centroids sit under a longer row owned by another place; every one of them is a region or county shadowed by a better-ranked place, 0 cross-country and 0 where the shadowed place ranks same or better, in both topologies. --- CHANGELOG | 17 ++-- scripts/generate_boundary_index.js | 121 +++++++++++++++++++++++------ spec/boundary_home_cell_spec.js | 51 ++++++++++++ 3 files changed, 160 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ea5c0b5..7b1daad 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,9 +15,10 @@ the finer cells over that same point when a neighbour emits them, so the rule holds for the longest-prefix lookup the runtime actually performs. Placetype ranking still decides first, and no cell is added to the index. -- By default, stop the dominant-city rollup suppressing a place whose placetype - it does not outrank. A county could absorb the cell holding a town's own - centre, which is a downgrade the cell comparator would never make - and +- By default, stop the dominant-city rollup suppressing or replacing a place + whose placetype it does not outrank, whether that place owns a descendant cell + or the promoted parent itself. A county could absorb the cell holding a town's + own centre, which is a downgrade the cell comparator would never make - and because the rollup runs per batch, it left that cell undefended in an `--append` build, so the next country written took it and the town's centre resolved across the border (Douglas AZ reading as Agua Prieta; Mittelberg and @@ -28,9 +29,13 @@ of any matching prefix. A row was dropped whenever some coarser cell named the same place, even when a different place owned the cell in between, and the lookup then answered with that other place. -- Reconcile nested home cells from a snapshot of cell ownership, so a place that - loses its own cell to a coarser claimant still carries its claim to the cells - below it. The pass no longer depends on the order places are read. +- Reconcile nested home cells from each place's own cover rather than from the + cells it happened to win, so a place that loses the cell holding its centroid + to a co-centred neighbour still carries its claim to the finer cells over that + point. The pass no longer depends on the order places are read. +- Report the rollup and home-cell invariants in the build summary (promotions + refused, home cells reclaimed or kept, rows kept from a shadowing owner), so a + world build shows whether any of these rules ever fires. - Keep counties out of the dominant-city rollup. A parent geohash cell rolled up to the place that dominates its children on population, and `county` counted as a city-like placetype, so a metro could be labelled with its diff --git a/scripts/generate_boundary_index.js b/scripts/generate_boundary_index.js index af359b0..2f048f6 100755 --- a/scripts/generate_boundary_index.js +++ b/scripts/generate_boundary_index.js @@ -1251,20 +1251,36 @@ function reconcileNestedHomeCells(bestByHash, placeById, opts) { // applying the claims in any order gives the same map. var hashes = Object.keys(bestByHash) var deepest = 0 - var claims = [] - for (var i = 0; i < hashes.length; i++) { - var hash = hashes[i] - if (hash.length > deepest) { - deepest = hash.length + if (hashes[i].length > deepest) { + deepest = hashes[i].length } + } - var place = placeById[String(bestByHash[hash])] - if (!place || !ownsHomeCell(place, hash)) { + // Claims come from each place's own cover rather than from the cells it + // happens to have won. When two places are centred in the same coarse cell + // the population tie-break gives it to one of them, but the loser still + // covers the finer cells over its centroid and still outranks a third place + // that is not centred there - reading claimants off the owner map would drop + // that claim entirely. + var claims = [] + var placeIds = Object.keys(placeById) + + for (var placeIndex = 0; placeIndex < placeIds.length; placeIndex++) { + var place = placeById[placeIds[placeIndex]] + if (!place || !place.cover) { continue } - claims.push({ place: place, precision: hash.length }) + // Cover cells never nest within one place, so at most one of them holds + // the centroid. + for (var coverIndex = 0; coverIndex < place.cover.length; coverIndex++) { + var coverHash = place.cover[coverIndex].geohash + if (ownsHomeCell(place, coverHash)) { + claims.push({ place: place, precision: coverHash.length }) + break + } + } } var hashCenterCache = Object.create(null) @@ -1303,11 +1319,18 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) return 0 } - var refusedPromotions = 0 + var stats = { + refusedPromotions: 0, + refusedParentRankPromotions: 0, + promotionsShadowingAncestorHome: 0, + promotionsShadowingForeignAncestorHome: 0, + promotionsShadowingBetterRankedAncestorHome: 0, + foreignHomeProtectionsBelowRank: 0 + } var minPrecision = Number(opts.basePrecision || 1) var maxPrecision = Number(opts.maxPrecision || minPrecision) if (maxPrecision <= minPrecision) { - return refusedPromotions + return stats } for (var precision = maxPrecision - 1; precision >= minPrecision; precision--) { @@ -1399,15 +1422,54 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) } } + var dominantPlace = placeById[String(promotion.localityId)] + // The parent cell can itself be the home cell of a town across the // border. The descendant sweep below only visits hashes longer than // `precision`, so without this check the promotion would overwrite that // town's own centroid cell and never get the chance to protect it. - if (protectsForeignHomeCell(existingPlace, parentHash, placeById[String(promotion.localityId)], opts)) { - refusedPromotions += 1 + if (protectsForeignHomeCell(existingPlace, parentHash, dominantPlace, opts)) { + stats.refusedPromotions += 1 continue } + // The same rank rule the descendant sweep applies: a rollup may not + // replace a place with a worse-ranked label. A locality whose cover + // terminates at the parent has no descendant row for the sweep to + // protect, so without this its centroid would resolve to the county that + // dominated the children. + var parentOptedInDominantCounty = dominantPlace && + dominantPlace.placetypeCode === PLACETYPE_CODES.county && + isDominantCityPlacetypeCode(dominantPlace.placetypeCode, opts) + + if (existingPlace && dominantPlace && !parentOptedInDominantCounty && + placetypeRank(dominantPlace.placetype) > placetypeRank(existingPlace.placetype)) { + stats.refusedParentRankPromotions += 1 + continue + } + + // Diagnostic only: a promoted parent that did not exist before adds a + // longer prefix inside a coarser cell, which can shadow the centroid of + // the place that owns that coarser cell. + if (existingId === undefined) { + for (var ancestorLength = parentHash.length - 1; ancestorLength >= 1; ancestorLength--) { + var ancestorId = bestByHash[parentHash.slice(0, ancestorLength)] + if (ancestorId === undefined) continue + + var ancestorPlace = placeById[String(ancestorId)] + if (ancestorPlace && ownsHomeCell(ancestorPlace, parentHash)) { + stats.promotionsShadowingAncestorHome += 1 + if (isForeignTo(ancestorPlace, dominantPlace)) { + stats.promotionsShadowingForeignAncestorHome += 1 + } + if (placetypeRank(ancestorPlace.placetype) <= placetypeRank(dominantPlace.placetype)) { + stats.promotionsShadowingBetterRankedAncestorHome += 1 + } + } + break + } + } + bestByHash[parentHash] = promotion.localityId promotedParents[parentHash] = promotion } @@ -1442,25 +1504,32 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) continue } - var dominantPlace = placeById[String(promoted.localityId)] + var promotedPlace = placeById[String(promoted.localityId)] var protectedHomeCell = protectsForeignHomeCell( descendantPlace, descendantHash, - dominantPlace, + promotedPlace, opts ) + // Diagnostic only: the foreign-home exception keeping a row whose + // placetype ranks below the place promoted over it. + if (protectedHomeCell && promotedPlace && + placetypeRank(descendantPlace.placetype) > placetypeRank(promotedPlace.placetype)) { + stats.foreignHomeProtectionsBelowRank += 1 + } + // The rollup normally folds a place only into a better- or equally-ranked // label. A county absorbing a locality is a downgrade the comparator // would never make, so counties are excluded from the dominant role by // default. Explicitly listing `county` in --dominant-city-placetypes is // the compatibility escape hatch that restores the earlier suppression // behavior, including over lower-ranked locality descendants. - var dominantOutranks = dominantPlace && - placetypeRank(dominantPlace.placetype) <= placetypeRank(descendantPlace.placetype) - var optedInDominantCounty = dominantPlace && - dominantPlace.placetypeCode === PLACETYPE_CODES.county && - isDominantCityPlacetypeCode(dominantPlace.placetypeCode, opts) + var dominantOutranks = promotedPlace && + placetypeRank(promotedPlace.placetype) <= placetypeRank(descendantPlace.placetype) + var optedInDominantCounty = promotedPlace && + promotedPlace.placetypeCode === PLACETYPE_CODES.county && + isDominantCityPlacetypeCode(promotedPlace.placetypeCode, opts) if (promoted.suppressMinorLocalities && isCityPlacetypeCode(descendantPlace.placetypeCode) && @@ -1472,7 +1541,7 @@ function promoteLocalityParentsByRegionCompetition(bestByHash, placeById, opts) } } - return refusedPromotions + return stats } function buildCompactLookupRows(places, opts) { @@ -1507,7 +1576,7 @@ function buildCompactLookupRows(places, opts) { // cells a dominant city absorbs inside its own country. var nestedHomeCellFixes = reconcileNestedHomeCells(bestByHashId, placeById, opts) - var refusedPromotions = promoteLocalityParentsByRegionCompetition(bestByHashId, placeById, opts) + var rollupStats = promoteLocalityParentsByRegionCompetition(bestByHashId, placeById, opts) var rows = Object.keys(bestByHashId).map(function(hash) { return { @@ -1565,7 +1634,7 @@ function buildCompactLookupRows(places, opts) { rows: compact, placeById: placeById, nestedHomeCellFixes: nestedHomeCellFixes, - refusedPromotions: refusedPromotions, + rollupStats: rollupStats, shadowedRowsKept: shadowedRowsKept } } @@ -2248,7 +2317,13 @@ async function main() { console.log('Home cell priority: ' + (options.homeCellPriority ? 'true' : 'false')) if (compactBuild) { console.log('Home cells kept from a nested foreign cell: ' + (compactBuild.nestedHomeCellFixes || 0)) - console.log('Parent promotions refused over a foreign home cell: ' + (compactBuild.refusedPromotions || 0)) + var rollup = compactBuild.rollupStats || {} + console.log('Parent promotions refused over a foreign home cell: ' + (rollup.refusedPromotions || 0)) + console.log('Parent promotions refused on placetype rank: ' + (rollup.refusedParentRankPromotions || 0)) + console.log('Promotions shadowing an ancestor home cell: ' + (rollup.promotionsShadowingAncestorHome || 0) + + ' (foreign ' + (rollup.promotionsShadowingForeignAncestorHome || 0) + + ', better-ranked ' + (rollup.promotionsShadowingBetterRankedAncestorHome || 0) + ')') + console.log('Foreign home cells kept below the promoted rank: ' + (rollup.foreignHomeProtectionsBelowRank || 0)) console.log('Rows kept from a nearer shadowing owner: ' + (compactBuild.shadowedRowsKept || 0)) } console.log('Placetype precision caps: locality=' + options.localityMaxPrecision + ', localadmin=' + options.localadminMaxPrecision + ', county=' + options.countyMaxPrecision + ', region=' + options.regionMaxPrecision) diff --git a/spec/boundary_home_cell_spec.js b/spec/boundary_home_cell_spec.js index 8c6896b..8e24b8c 100644 --- a/spec/boundary_home_cell_spec.js +++ b/spec/boundary_home_cell_spec.js @@ -246,6 +246,57 @@ describe('boundary builder home cell ownership', () => { }); }); + it('will not let a county rollup take the parent cell from a locality', async () => { + await withTempDir(async (dir) => { + // Same downgrade one level up: the locality's cover terminates at the + // parent, so it owns no descendant row for the sweep to protect. Only a + // rank check on the replacement itself keeps its centre out of the + // county. + const town = { + id: 6101, + name: 'Parent Town', + placetype: 'locality', + countryId: 'US', + population: 20000, + minLon: -0.05, minLat: -0.05, maxLon: 0.40, maxLat: 0.22, + centroid: [0.02, 0.02] + }; + const county = { + id: 6102, + name: 'Wide County', + placetype: 'county', + countryId: 'US', + population: 400000, + minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, + centroid: [0.07, 0.29] + }; + // Reaches below Parent Town's southern edge so the contained-locality + // prune leaves it alone; it exists only to make the rollup pick a + // dominant place rather than take its single-locality branch. + const suburb = { + id: 6103, + name: 'Second Town', + placetype: 'locality', + countryId: 'US', + population: 10000, + minLon: 0.045, minLat: -0.08, maxLon: 0.086, maxLat: 0.085, + centroid: [0.04, 0.06] + }; + + const input = writeFixture(dir, 'parent-rank.geojson', [place(town), place(county), place(suburb)]); + const dbPath = path.join(dir, 'parent-rank.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--include-county', 'true' + ].concat(commonFlags)).status).toEqual(0); + + const owner = await lookupOwner(dbPath, geohash.encode(town.centroid[0], town.centroid[1], 5)); + expect(owner.id).toEqual(town.id); + + expect((await lookupOwner(dbPath, geohash.encode(0.06, 0.285, 5))).id).toEqual(county.id); + }); + }); + it('resolves the home cell the same way in either append order', async () => { await withTempDir(async (dir) => { const bigInput = writeFixture(dir, 'big.geojson', [place(BIGVILLE)]); From 641e0618db00b4ec95995bb9c7fb647ca5b767d8 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Thu, 20 Aug 2026 11:27:07 -0600 Subject: [PATCH 6/6] Harden home-cell ownership edge cases --- CHANGELOG | 4 +- README.md | 2 +- scripts/generate_boundary_index.js | 13 ++- spec/boundary_home_cell_spec.js | 125 ++++++++++++++++++++++++++--- 4 files changed, 130 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 7b1daad..2e53cb3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,7 +5,9 @@ the larger city across the border (for example Calexico resolving to Mexicali, or Heerlen to Aachen). Placetype ranking is unchanged, and cells holding two centroids are still decided by population. Disable with - `--home-cell-priority false` (`WOF_HOME_CELL_PRIORITY=0`). + `--home-cell-priority false` (`WOF_HOME_CELL_PRIORITY=0`). Centroid + coordinates outside the place geometry, including an exterior bounding-box + midpoint fallback, do not receive home-cell priority. - Stop the dominant-city rollup absorbing the home cell of a town in another country, whether that cell is a descendant of the promoted parent or the promoted parent itself. Rollups inside a country are unchanged, so a metro diff --git a/README.md b/README.md index d55ed95..e1dc63e 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,7 @@ Builder notes: - `--region-max-precision` - `--region-sparse-max-precision` + `--region-sparse-min-area-km2` for very large sparse regions (for example geohash-3 in Amazon-like interiors) - `--promote-locality-over-region` (default `true`) prefers locality labels in shared parent cells when there is no competing locality (keeps city labels sticky against region-only outskirts) -- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. Because the index is walked by longest matching prefix, the claim is enforced across nested cells too: a place that owns the coarse cell holding its centroid also keeps the finer cells over that point. The dominant-city rollup keeps a foreign town's home cell for the same reason, whether that cell is the promoted parent or one of its descendants; rollups within one country are unaffected. This is a guarantee of the generated, uncurated database; an explicitly applied [curation overlay](curation/README.md) is the final labeling layer and may deliberately reassign matching cells, including an absorbed place's home cell +- `--home-cell-priority` (default `true`) gives a place the cell that contains its own centroid, ahead of the population tie-break, so a border town is not swallowed by the larger city across the border. Placetype ranking still applies first, and a cell holding two centroids is still decided by population. A centroid coordinate outside the place geometry does not receive this priority; this includes bounding-box midpoint fallbacks that land outside concave, holed, or multipart geometry. Because the index is walked by longest matching prefix, the claim is enforced across nested cells too: a place that owns the coarse cell holding its centroid also keeps the finer cells over that point. The dominant-city rollup keeps a foreign town's home cell for the same reason, whether that cell is the promoted parent or one of its descendants; rollups within one country are unaffected. This is a guarantee of the generated, uncurated database; an explicitly applied [curation overlay](curation/README.md) is the final labeling layer and may deliberately reassign matching cells, including an absorbed place's home cell - Dominant-city rollup keeps broad city labels sticky in mixed city/suburb cells unless there is competing major-city pressure: - `--dominant-locality-population` (default `100000`) - `--dominant-locality-ratio` (default `3`) diff --git a/scripts/generate_boundary_index.js b/scripts/generate_boundary_index.js index 2f048f6..ce7f248 100755 --- a/scripts/generate_boundary_index.js +++ b/scripts/generate_boundary_index.js @@ -694,6 +694,17 @@ function normalizeFeature(feature, opts) { var bbox = geometry.geometryBbox(normalizedGeometry) var centroid = extractCentroid(properties, normalizedGeometry) + // Generic GeoJSON can omit an explicit centroid, in which case the bbox + // midpoint is only a storage fallback: for a concave, holed, or multipart + // shape it may not belong to the place at all. Keep the coordinates for the + // existing schema, but do not let an exterior point claim a home cell. + var homeGeohash = geometry.pointInGeometry( + normalizedGeometry, + centroid.latitude, + centroid.longitude + ) + ? geohash.encode(centroid.latitude, centroid.longitude, HOME_CELL_PRECISION) + : null var countryId = extractCountryId(properties) var name = extractName(properties, feature) @@ -730,7 +741,7 @@ function normalizeFeature(feature, opts) { placetypeCode: placetypeCode(placetype), centroidLat: centroid.latitude, centroidLon: centroid.longitude, - homeGeohash: geohash.encode(centroid.latitude, centroid.longitude, HOME_CELL_PRECISION), + homeGeohash: homeGeohash, population: population, bboxMinLat: bbox.minLat, bboxMinLon: bbox.minLon, diff --git a/spec/boundary_home_cell_spec.js b/spec/boundary_home_cell_spec.js index 8e24b8c..6226487 100644 --- a/spec/boundary_home_cell_spec.js +++ b/spec/boundary_home_cell_spec.js @@ -135,6 +135,55 @@ describe('boundary builder home cell ownership', () => { }); }); + it('does not grant home priority to a fallback bbox midpoint outside the geometry', async () => { + await withTempDir(async (dir) => { + // The C-shaped place has no explicit centroid, so its fallback is the + // bbox midpoint (0.1, 0.3), in the open notch rather than the polygon. + // Its upper arm still intersects the same geohash cell as Notch Town; + // treating that exterior fallback as a home point would let population + // hand the town's real centre to the wrong country. + const cShape = { + type: 'Feature', + id: 3001, + properties: { + name: 'C Province', + placetype: 'locality', + country_id: 'MX', + admin1_id: 1, + is_current: 1, + population: 690000 + }, + geometry: { + type: 'Polygon', + coordinates: [[ + [0, 0], [0.6, 0], [0.6, 0.08], [0.25, 0.08], + [0.25, 0.12], [0.6, 0.12], [0.6, 0.2], [0, 0.2], [0, 0] + ]] + } + }; + const notchTown = place({ + id: 3002, + name: 'Notch Town', + placetype: 'locality', + countryId: 'US', + population: 38000, + minLon: 0.27, minLat: 0.085, maxLon: 0.33, maxLat: 0.115, + centroid: [0.1, 0.3] + }); + + const input = writeFixture(dir, 'exterior-fallback.geojson', [cShape, notchTown]); + const dbPath = path.join(dir, 'exterior-fallback.sqlite'); + + expect(runBuilder([ + '--database', dbPath, '--input', input, '--dominant-locality-population', '0' + ].concat(commonFlags)).status).toEqual(0); + + const owner = await lookupOwner(dbPath, geohash.encode(0.1, 0.3, 5)); + expect(owner.id).toEqual(notchTown.id); + expect(owner.country_id).toEqual('US'); + }); + }); + it('falls back to population when the rule is disabled', async () => { await withTempDir(async (dir) => { const input = writeFixture(dir, 'border.geojson', [place(BIGVILLE), place(SMALLTOWN)]); @@ -246,12 +295,13 @@ describe('boundary builder home cell ownership', () => { }); }); - it('will not let a county rollup take the parent cell from a locality', async () => { + it('will not let a rollup take the parent cell from a place it outranks', async () => { await withTempDir(async (dir) => { - // Same downgrade one level up: the locality's cover terminates at the - // parent, so it owns no descendant row for the sweep to protect. Only a + // Same downgrade one level up. The locality's cover terminates at the + // parent, so it owns no descendant row for the sweep to protect - only a // rank check on the replacement itself keeps its centre out of the - // county. + // localadmin. A localadmin is an eligible dominant placetype by default, + // unlike a county, so this is the case the parent guard still answers. const town = { id: 6101, name: 'Parent Town', @@ -261,10 +311,10 @@ describe('boundary builder home cell ownership', () => { minLon: -0.05, minLat: -0.05, maxLon: 0.40, maxLat: 0.22, centroid: [0.02, 0.02] }; - const county = { + const municipality = { id: 6102, - name: 'Wide County', - placetype: 'county', + name: 'Wide Municipality', + placetype: 'localadmin', countryId: 'US', population: 400000, minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, @@ -272,7 +322,7 @@ describe('boundary builder home cell ownership', () => { }; // Reaches below Parent Town's southern edge so the contained-locality // prune leaves it alone; it exists only to make the rollup pick a - // dominant place rather than take its single-locality branch. + // dominant place rather than take its single-candidate branch. const suburb = { id: 6103, name: 'Second Town', @@ -283,17 +333,70 @@ describe('boundary builder home cell ownership', () => { centroid: [0.04, 0.06] }; - const input = writeFixture(dir, 'parent-rank.geojson', [place(town), place(county), place(suburb)]); + const input = writeFixture(dir, 'parent-rank.geojson', [place(town), place(municipality), place(suburb)]); const dbPath = path.join(dir, 'parent-rank.sqlite'); expect(runBuilder([ - '--database', dbPath, '--input', input, '--include-county', 'true' + '--database', dbPath, '--input', input, '--include-localadmin', 'true' ].concat(commonFlags)).status).toEqual(0); const owner = await lookupOwner(dbPath, geohash.encode(town.centroid[0], town.centroid[1], 5)); expect(owner.id).toEqual(town.id); - expect((await lookupOwner(dbPath, geohash.encode(0.06, 0.285, 5))).id).toEqual(county.id); + expect((await lookupOwner(dbPath, geohash.encode(0.06, 0.285, 5))).id).toEqual(municipality.id); + }); + }); + + it('keeps the home-cell claim of a place that lost that cell on population', async () => { + await withTempDir(async (dir) => { + // Two towns across a border both swallow the same coarse cell and are + // both centred in it, so population decides who owns it. The loser still + // covers the finer cells over its own centre, and still outranks a third + // place that is not centred there - reading claimants off the cells a + // place happens to have won drops that claim entirely. + const winner = { + id: 7001, + name: 'Bigger Twin', + placetype: 'locality', + countryId: 'MX', + population: 500000, + minLon: -0.05, minLat: -0.05, maxLon: 0.40, maxLat: 0.22, + centroid: [0.02, 0.02] + }; + const loser = { + id: 7002, + name: 'Smaller Twin', + placetype: 'locality', + countryId: 'US', + population: 300000, + minLon: -0.06, minLat: -0.06, maxLon: 0.41, maxLat: 0.23, + centroid: [0.11, 0.11] + }; + const neighbour = { + id: 7003, + name: 'Third City', + placetype: 'locality', + countryId: 'MX', + population: 690000, + minLon: 0.088, minLat: -0.05, maxLon: 0.60, maxLat: 0.22, + centroid: [0.07, 0.29] + }; + + const input = writeFixture(dir, 'twins.geojson', [place(winner), place(loser), place(neighbour)]); + const dbPath = path.join(dir, 'twins.sqlite'); + + expect(runBuilder(['--database', dbPath, '--input', input].concat(commonFlags)).status).toEqual(0); + + // The coarse cell goes to the more populous twin, as documented. + expect((await lookupOwner(dbPath, geohash.encode(winner.centroid[0], winner.centroid[1], 5))).id) + .toEqual(winner.id); + + // The loser still takes the finer cell holding its own centre. + const owner = await lookupOwner(dbPath, geohash.encode(loser.centroid[0], loser.centroid[1], 5)); + expect(owner.id).toEqual(loser.id); + expect(owner.country_id).toEqual('US'); + + expect((await lookupOwner(dbPath, geohash.encode(0.06, 0.285, 5))).id).toEqual(neighbour.id); }); });