From 0ae8efb2a3ebc2c2e467608e7a6f638bff511999 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Tue, 18 Aug 2026 08:55:32 -0600 Subject: [PATCH 01/12] Add quota-aware resumable LocationIQ world validation sweep Extend scripts/validate_with_locationiq.js with two subcommands while keeping the legacy random-sample validator intact: - sample: builds a JSONL points file from a GeoNames-style TSV (cities1000 format), selecting the top-N most populous places per country (default 25), with an optional total cap filled round-robin by rank so every country keeps coverage. - sweep: reverse geocodes every point with LocationIQ and compares the answer against the offline geocoder. Every response is cached as JSONL keyed by coordinates rounded to four decimals, so re-running the same command never re-queries a cached point. A persisted state file records the UTC date and request count, enforcing a daily cap (default 4500) across invocations on the same UTC day, alongside a requests-per-second limit (default 1). On HTTP 429 the run backs off and stops cleanly with a resume message. Outputs: a Markdown report ranking countries by mismatch rate (country mismatch is severe; name comparison is case/diacritics-insensitive and counts a match against any LocationIQ locality/county/state field as agreement) with the 10 worst examples, plus a machine-readable mismatch JSONL. --dry-run evaluates the cache only, with no network. Name normalization now preserves non-Latin scripts so world-wide comparisons work outside Latin alphabets. A new spec file covers the sampler, comparison rules, cap enforcement and persistence, cache skipping, UTC-day reset, 429 handling, dry-run and report generation, all with an injected HTTP function so tests never touch the network. --- CHANGELOG | 4 + README.md | 30 + scripts/validate_with_locationiq.js | 882 +++++++++++++++++++++++++++- spec/locationiq_sweep_spec.js | 420 +++++++++++++ 4 files changed, 1328 insertions(+), 8 deletions(-) create mode 100644 spec/locationiq_sweep_spec.js diff --git a/CHANGELOG b/CHANGELOG index 56126d9..18a2a40 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +### Unreleased + +- Add `sample` and `sweep` subcommands to `scripts/validate_with_locationiq.js`: a quota-aware, resumable world validation sweep against LocationIQ (JSONL response cache keyed by rounded coordinates, persisted per-UTC-day request cap, per-country Markdown mismatch report plus machine-readable mismatch JSONL, `--dry-run` for cache-only evaluation) and a GeoNames-TSV sampler that picks the top-N most populous places per country. Name comparison now preserves non-Latin scripts. + ### 1.0.0 - 2018-08-02 - Add bug fix for reverse lookup in rural/remote areas (@cosorio94) diff --git a/README.md b/README.md index ae657c1..1adba99 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,36 @@ It creates/updates: Cache DB path is automatic (default behavior): `tmp/locationiq-validation-.sqlite`. +### World Validation Sweep (LocationIQ) + +The same script also runs a quota-aware, resumable sweep over a world-wide +points file and ranks countries by mismatch rate, so data work can be aimed at +the worst areas first: + +```bash +# 1. Build a points file from a GeoNames-style TSV (e.g. cities1000.txt): +# the top 25 most populous places per country. Not committed to the repo. +node scripts/validate_with_locationiq.js sample \ + --geonames tmp/cities1000.txt --per-country 25 + +# 2. Run the sweep. Stays inside LocationIQ's free tier by default: +# max 4500 requests per UTC day (persisted across invocations) at 1 req/s. +LOCATIONIQ_API_KEY=... node scripts/validate_with_locationiq.js sweep \ + --points tmp/locationiq-sweep/points.jsonl --database tmp/world.sqlite + +# 3. Rebuild the report from cache only, no network: +node scripts/validate_with_locationiq.js sweep \ + --points tmp/locationiq-sweep/points.jsonl --database tmp/world.sqlite --dry-run +``` + +Every LocationIQ response is cached as JSONL keyed by coordinates rounded to +four decimals, so re-running the same command never re-queries a cached point — +if a run stops at the daily cap or on HTTP 429, just run it again later to +resume. Outputs land under `--workdir` (default `tmp/locationiq-sweep/`): +`report.md` (per-country point counts, agreement %, country mismatches, worst +examples) and `mismatches.jsonl` (machine-readable list of all mismatches). +See `sweep --help` and `sample --help` for all options. + ## License This library is licensed under [the MIT license](https://github.com/lucaspiller/offline-geocoder/blob/master/LICENSE). diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 414fbdf..45bad9e 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -11,9 +11,14 @@ const geohash = require('../src/geohash') function usage() { return [ - 'Usage: node scripts/validate_with_locationiq.js --database [options]', + 'Usage:', + ' node scripts/validate_with_locationiq.js [legacy options] Random-sample validation of one database (options below)', + ' node scripts/validate_with_locationiq.js sample [options] Build a world points file from a GeoNames-style TSV', + ' node scripts/validate_with_locationiq.js sweep [options] Quota-aware, resumable world validation sweep', '', - 'Options:', + 'Run `sample --help` or `sweep --help` for the subcommand options.', + '', + 'Legacy options:', ' --database Geocoder SQLite database to validate (required)', ' --api-key LocationIQ API key (or env LOCATIONIQ_API_KEY)', ' --samples Number of sample points to evaluate (default: 200)', @@ -403,7 +408,7 @@ function normalizeName(value) { .normalize('NFKD') .replace(/[\u0300-\u036f]/g, '') .toLowerCase() - .replace(/[^a-z0-9]+/g, ' ') + .replace(/[^\p{L}\p{N}]+/gu, ' ') .trim() .replace(/\s+/g, ' ') } @@ -576,7 +581,7 @@ function fetchJson(endpointUrl, timeoutMs) { }) } -function buildLocationIqUrl(endpoint, apiKey, latitude, longitude) { +function buildLocationIqUrl(endpoint, apiKey, latitude, longitude, acceptLanguage) { var url = new URL(endpoint) url.searchParams.set('key', apiKey) url.searchParams.set('lat', String(latitude)) @@ -584,6 +589,9 @@ function buildLocationIqUrl(endpoint, apiKey, latitude, longitude) { url.searchParams.set('format', 'json') url.searchParams.set('normalizecity', '1') url.searchParams.set('addressdetails', '1') + if (acceptLanguage) { + url.searchParams.set('accept-language', acceptLanguage) + } return url.toString() } @@ -891,7 +899,865 @@ async function main() { } } -main().catch(function(err) { - console.error(err.message || err) - process.exit(1) -}) +// --------------------------------------------------------------------------- +// World validation sweep (subcommands: sample, sweep) +// +// `sample` turns a GeoNames-style TSV (cities1000 format) into a JSONL points +// file with the top-N most populous places per country. `sweep` reverse +// geocodes every point with LocationIQ (JSONL response cache, persisted UTC +// daily request cap) and compares the answers against the offline geocoder, +// ranking countries by mismatch rate in a Markdown report. +// --------------------------------------------------------------------------- + +var SWEEP_CACHE_DECIMALS = 4 +var SWEEP_TIMEOUT_MS = 20000 +var SWEEP_DEFAULT_WORKDIR = 'tmp/locationiq-sweep' +var SWEEP_SEVERITY = { + country_mismatch: 3, + offline_empty: 2, + name_mismatch: 1 +} + +var LIQ_LOCALITY_KEYS = ['city', 'town', 'village', 'municipality', 'hamlet', 'borough', 'suburb', 'city_district', 'district', 'quarter', 'neighbourhood'] +var LIQ_COUNTY_KEYS = ['county', 'state_district'] +var LIQ_STATE_KEYS = ['state', 'region', 'province'] + +function sampleUsage() { + return [ + 'Usage: node scripts/validate_with_locationiq.js sample --geonames [options]', + '', + 'Builds a JSONL points file ({lat, lon, country, name, population} per line)', + 'from a GeoNames-style TSV (the cities1000 format: tab separated, feature', + 'class in column 7, country code in column 9, population in column 15).', + 'Rows whose feature class is not P are skipped. No data file is committed to', + 'the repository; download e.g. cities1000.zip from download.geonames.org.', + '', + 'Options:', + ' --geonames GeoNames-style TSV input (required)', + ' --out Output JSONL points file (default: ' + SWEEP_DEFAULT_WORKDIR + '/points.jsonl)', + ' --per-country Places per country, most populous first (default: 25)', + ' --max-points Optional total cap, filled round-robin by rank so every country keeps coverage', + ' --help, -h Show this help message' + ].join('\n') +} + +function sweepUsage() { + return [ + 'Usage: node scripts/validate_with_locationiq.js sweep --points --database [options]', + '', + 'Reverse geocodes every point with LocationIQ and compares the answer to the', + 'offline geocoder. Every LocationIQ response is cached (JSONL, keyed by', + 'coordinates rounded to ' + SWEEP_CACHE_DECIMALS + ' decimals); cached points are never re-queried, so', + 're-running the same command resumes where the previous run stopped. A state', + 'file records the UTC date and request count, so multiple runs on the same', + 'UTC day share one daily cap. On HTTP 429 the run backs off and stops', + 'cleanly. Designed for LocationIQ\'s free tier; run one sweep at a time.', + '', + 'Options:', + ' --points JSONL points file (required; see the sample subcommand)', + ' --database Offline geocoder SQLite database (required)', + ' --workdir Directory for cache/state/report files (default: ' + SWEEP_DEFAULT_WORKDIR + ')', + ' --cache LocationIQ response cache (default: /cache.jsonl)', + ' --state Daily quota state file (default: /quota.json)', + ' --report Markdown report output (default: /report.md)', + ' --mismatches Mismatch JSONL output (default: /mismatches.jsonl)', + ' --api-key LocationIQ API key (or env LOCATIONIQ_API_KEY)', + ' --daily-cap Max LocationIQ requests per UTC day, all runs combined (default: 4500)', + ' --rps Max LocationIQ requests per second (default: 1)', + ' --max-requests Optional per-run request limit (useful for smoke tests)', + ' --endpoint LocationIQ reverse endpoint (default: https://us1.locationiq.com/v1/reverse)', + ' --accept-language Accept-language sent to LocationIQ (default: en; empty to disable)', + ' --reverse-mode centroid|boundary (default: boundary)', + ' --base-precision Boundary lookup base precision (default: 4)', + ' --max-precision Boundary lookup max precision (default: 7)', + ' --dry-run No network: evaluate cached responses only and rebuild the report', + ' --help, -h Show this help message', + '', + 'Example:', + ' node scripts/validate_with_locationiq.js sample --geonames tmp/cities1000.txt --per-country 25', + ' LOCATIONIQ_API_KEY=... node scripts/validate_with_locationiq.js sweep \\', + ' --points ' + SWEEP_DEFAULT_WORKDIR + '/points.jsonl --database tmp/world.sqlite', + ' node scripts/validate_with_locationiq.js sweep \\', + ' --points ' + SWEEP_DEFAULT_WORKDIR + '/points.jsonl --database tmp/world.sqlite --dry-run' + ].join('\n') +} + +function utcDateString(date) { + return date.toISOString().slice(0, 10) +} + +function sweepCoordKey(latitude, longitude) { + return Number(latitude).toFixed(SWEEP_CACHE_DECIMALS) + ',' + Number(longitude).toFixed(SWEEP_CACHE_DECIMALS) +} + +// --- sample: GeoNames TSV -> points JSONL ---------------------------------- + +function parseGeonamesLine(line) { + if (!line || !line.trim()) return null + + var cols = line.split('\t') + if (cols.length < 15) return null + + var latitude = Number(cols[4]) + var longitude = Number(cols[5]) + var featureClass = String(cols[6] || '').trim() + var country = String(cols[8] || '').trim().toUpperCase() + var population = Number(cols[14]) + + if (!Number.isFinite(latitude) || latitude < -90 || latitude > 90) return null + if (!Number.isFinite(longitude) || longitude < -180 || longitude > 180) return null + if (featureClass !== 'P') return null + if (!/^[A-Z]{2}$/.test(country)) return null + + return { + geonameid: Number(cols[0]) || 0, + name: String(cols[1] || '').trim(), + lat: latitude, + lon: longitude, + country: country, + population: Number.isFinite(population) && population > 0 ? Math.trunc(population) : 0 + } +} + +function selectTopPlaces(rows, perCountry, maxPoints) { + var byCountry = Object.create(null) + for (var i = 0; i < rows.length; i++) { + var row = rows[i] + if (!byCountry[row.country]) byCountry[row.country] = [] + byCountry[row.country].push(row) + } + + var countries = Object.keys(byCountry).sort() + var total = 0 + for (var c = 0; c < countries.length; c++) { + var list = byCountry[countries[c]] + list.sort(function(a, b) { + if (b.population !== a.population) return b.population - a.population + return a.geonameid - b.geonameid + }) + byCountry[countries[c]] = list.slice(0, perCountry) + total += byCountry[countries[c]].length + } + + var picked = [] + if (maxPoints && total > maxPoints) { + // Fill round-robin by rank so a total cap trims depth per country instead + // of dropping whole countries. + for (var rank = 0; rank < perCountry && picked.length < maxPoints; rank++) { + for (var j = 0; j < countries.length && picked.length < maxPoints; j++) { + var ranked = byCountry[countries[j]] + if (rank < ranked.length) picked.push(ranked[rank]) + } + } + } else { + for (var k = 0; k < countries.length; k++) { + picked = picked.concat(byCountry[countries[k]]) + } + } + + picked.sort(function(a, b) { + if (a.country !== b.country) return a.country < b.country ? -1 : 1 + if (b.population !== a.population) return b.population - a.population + return a.geonameid - b.geonameid + }) + + return picked.map(function(place) { + return { + lat: place.lat, + lon: place.lon, + country: place.country, + name: place.name, + population: place.population + } + }) +} + +function buildSamplePoints(tsvText, perCountry, maxPoints) { + var lines = String(tsvText).split(/\r?\n/) + var rows = [] + var skipped = 0 + for (var i = 0; i < lines.length; i++) { + if (!lines[i].trim()) continue + var row = parseGeonamesLine(lines[i]) + if (row) rows.push(row) + else skipped += 1 + } + return { + points: selectTopPlaces(rows, perCountry, maxPoints), + parsed: rows.length, + skipped: skipped + } +} + +function parseSampleArgs(argv) { + var opts = { + geonames: null, + out: path.resolve(SWEEP_DEFAULT_WORKDIR, 'points.jsonl'), + perCountry: 25, + maxPoints: null, + help: false + } + + for (var i = 0; i < argv.length; i++) { + var arg = argv[i] + + if (arg === '--geonames') { + opts.geonames = path.resolve(argv[++i]) + } else if (arg === '--out') { + opts.out = path.resolve(argv[++i]) + } else if (arg === '--per-country') { + opts.perCountry = Math.max(1, Math.trunc(Number(argv[++i]))) + } else if (arg === '--max-points') { + var maxPoints = Number(argv[++i]) + opts.maxPoints = Number.isFinite(maxPoints) && maxPoints > 0 ? Math.trunc(maxPoints) : null + } else if (arg === '--help' || arg === '-h') { + opts.help = true + } else { + throw new Error('Unknown sample argument: ' + arg) + } + } + + return opts +} + +async function sampleMain(argv) { + var opts = parseSampleArgs(argv) + if (opts.help) { + console.log(sampleUsage()) + return + } + + if (!opts.geonames) { + throw new Error('Missing required --geonames (see `sample --help`)') + } + if (!fs.existsSync(opts.geonames)) { + throw new Error('GeoNames TSV not found: ' + opts.geonames) + } + if (!Number.isFinite(opts.perCountry) || opts.perCountry <= 0) { + throw new Error('--per-country must be > 0') + } + + var result = buildSamplePoints(fs.readFileSync(opts.geonames, 'utf8'), opts.perCountry, opts.maxPoints) + if (!result.points.length) { + throw new Error('No usable rows found in ' + opts.geonames) + } + + var countries = Object.create(null) + var lines = [] + for (var i = 0; i < result.points.length; i++) { + countries[result.points[i].country] = true + lines.push(JSON.stringify(result.points[i])) + } + + fs.mkdirSync(path.dirname(opts.out), { recursive: true }) + fs.writeFileSync(opts.out, lines.join('\n') + '\n', 'utf8') + + console.log('Parsed rows: ' + result.parsed + ' (skipped ' + result.skipped + ')') + console.log('Points written: ' + result.points.length + ' across ' + Object.keys(countries).length + ' countries') + console.log('Points file: ' + opts.out) +} + +// --- sweep: cache, quota state, comparison, report ------------------------- + +function loadPointsFile(pointsPath) { + var lines = fs.readFileSync(pointsPath, 'utf8').split(/\r?\n/) + var points = [] + var seen = Object.create(null) + var skipped = 0 + var duplicates = 0 + + for (var i = 0; i < lines.length; i++) { + if (!lines[i].trim()) continue + + var row + try { + row = JSON.parse(lines[i]) + } catch (err) { + skipped += 1 + continue + } + + var lat = Number(row.lat !== undefined ? row.lat : row.latitude) + var lon = Number(row.lon !== undefined ? row.lon : row.longitude) + if (!Number.isFinite(lat) || lat < -90 || lat > 90 || !Number.isFinite(lon) || lon < -180 || lon > 180) { + skipped += 1 + continue + } + + var key = sweepCoordKey(lat, lon) + if (seen[key]) { + duplicates += 1 + continue + } + seen[key] = true + + points.push({ + key: key, + lat: lat, + lon: lon, + country: row.country ? String(row.country).toUpperCase() : '', + name: row.name ? String(row.name) : '', + population: Number(row.population) || 0 + }) + } + + return { points: points, skipped: skipped, duplicates: duplicates } +} + +function loadSweepCache(cachePath) { + var map = Object.create(null) + if (!fs.existsSync(cachePath)) return map + + var lines = fs.readFileSync(cachePath, 'utf8').split(/\r?\n/) + for (var i = 0; i < lines.length; i++) { + if (!lines[i].trim()) continue + try { + var entry = JSON.parse(lines[i]) + if (entry && entry.key) map[entry.key] = entry + } catch (err) { + // Ignore torn/corrupt lines (e.g. from an interrupted write); the point + // will simply be fetched again. + } + } + return map +} + +function appendSweepCache(cachePath, entry) { + fs.appendFileSync(cachePath, JSON.stringify(entry) + '\n', 'utf8') +} + +function loadQuotaState(statePath, todayUtc) { + try { + var raw = JSON.parse(fs.readFileSync(statePath, 'utf8')) + if (raw && raw.date === todayUtc && Number.isFinite(Number(raw.count))) { + return { date: todayUtc, count: Math.max(0, Math.trunc(Number(raw.count))) } + } + } catch (err) { + // Missing or unreadable state resets to zero for today. + } + return { date: todayUtc, count: 0 } +} + +function saveQuotaState(statePath, state) { + fs.mkdirSync(path.dirname(statePath), { recursive: true }) + fs.writeFileSync(statePath, JSON.stringify({ date: state.date, count: state.count }) + '\n', 'utf8') +} + +function findLiqNameMatch(normalizedOfflineName, address, displayName) { + if (!normalizedOfflineName) return null + + var groups = [LIQ_LOCALITY_KEYS, LIQ_COUNTY_KEYS, LIQ_STATE_KEYS] + for (var g = 0; g < groups.length; g++) { + var hit = matchAddressValue(normalizedOfflineName, address, groups[g]) + if (hit) return hit.key + } + + if (displayNameMatch(normalizedOfflineName, displayName)) { + return 'display_name' + } + + return null +} + +function comparePoint(point, offlineResult, cacheEntry) { + var body = cacheEntry && cacheEntry.body && typeof cacheEntry.body === 'object' ? cacheEntry.body : null + var address = body && body.address && typeof body.address === 'object' ? body.address : null + var liqOk = Boolean(cacheEntry && Number(cacheEntry.status) === 200 && address && !body.error) + + var offlineName = offlineResult && offlineResult.name ? String(offlineResult.name) : '' + var offlineCountry = offlineResult && offlineResult.country && offlineResult.country.id + ? String(offlineResult.country.id).toUpperCase() + : '' + + var record = { + key: point.key, + lat: point.lat, + lon: point.lon, + country: point.country || '', + sample_name: point.name || '', + offline_name: offlineName, + offline_country: offlineCountry, + liq_name: '', + liq_country: '', + liq_display_name: '', + verdict: '', + match_via: '' + } + + if (liqOk) { + record.liq_name = extractLocationIqLocality(address) + record.liq_country = address.country_code ? String(address.country_code).toUpperCase() : '' + record.liq_display_name = body.display_name ? String(body.display_name) : '' + } + if (!record.country) { + record.country = record.liq_country || offlineCountry || '??' + } + + if (!liqOk) { + record.verdict = offlineName ? 'liq_empty' : 'both_empty' + } else if (!offlineName) { + record.verdict = 'offline_empty' + } else if (offlineCountry && record.liq_country && offlineCountry !== record.liq_country) { + record.verdict = 'country_mismatch' + } else { + var via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) + if (via) { + record.verdict = 'agree' + record.match_via = via + } else { + record.verdict = 'name_mismatch' + } + } + + return record +} + +function mdEscape(value) { + return String(value === undefined || value === null ? '' : value).replace(/\|/g, '\\|') +} + +function formatAnswer(name, country, fallback) { + if (name) return name + (country ? ' (' + country + ')' : '') + if (fallback) return fallback.length > 60 ? fallback.slice(0, 57) + '...' : fallback + return '(no result)' +} + +function buildSweepReport(params) { + var records = params.records || [] + var perCountry = Object.create(null) + var totals = { agree: 0, country_mismatch: 0, name_mismatch: 0, offline_empty: 0, liq_empty: 0, both_empty: 0 } + + for (var i = 0; i < records.length; i++) { + var record = records[i] + var bucket = perCountry[record.country] + if (!bucket) { + bucket = perCountry[record.country] = { + country: record.country, + evaluated: 0, + agree: 0, + country_mismatch: 0, + name_mismatch: 0, + offline_empty: 0, + liq_empty: 0, + both_empty: 0 + } + } + bucket.evaluated += 1 + if (bucket[record.verdict] !== undefined) bucket[record.verdict] += 1 + if (totals[record.verdict] !== undefined) totals[record.verdict] += 1 + } + + var countryRows = Object.keys(perCountry).map(function(code) { + var bucket = perCountry[code] + bucket.verifiable = bucket.agree + bucket.country_mismatch + bucket.name_mismatch + bucket.offline_empty + bucket.mismatchRate = bucket.verifiable > 0 ? (bucket.verifiable - bucket.agree) / bucket.verifiable : 0 + return bucket + }) + countryRows.sort(function(a, b) { + if (b.mismatchRate !== a.mismatchRate) return b.mismatchRate - a.mismatchRate + if (b.verifiable !== a.verifiable) return b.verifiable - a.verifiable + return a.country < b.country ? -1 : (a.country > b.country ? 1 : 0) + }) + + var verifiable = totals.agree + totals.country_mismatch + totals.name_mismatch + totals.offline_empty + var agreementPct = verifiable > 0 ? (totals.agree * 100) / verifiable : 0 + + var worst = records + .filter(function(record) { return SWEEP_SEVERITY[record.verdict] }) + .sort(function(a, b) { + var severity = (SWEEP_SEVERITY[b.verdict] || 0) - (SWEEP_SEVERITY[a.verdict] || 0) + if (severity !== 0) return severity + if (a.country !== b.country) return a.country < b.country ? -1 : 1 + return a.key < b.key ? -1 : 1 + }) + .slice(0, 10) + + var lines = [] + lines.push('# LocationIQ world validation sweep') + lines.push('') + lines.push('- Generated: ' + params.generatedAt) + lines.push('- Offline database: `' + params.databaseLabel + '`') + lines.push('- Points file: `' + params.pointsPath + '` (' + params.totalPoints + ' points; ' + + records.length + ' evaluated, ' + params.unfetched + ' awaiting fetch)') + lines.push('- Verifiable points: ' + verifiable + ' — agreement ' + totals.agree + '/' + verifiable + + ' (' + agreementPct.toFixed(1) + '%)') + lines.push('- Mismatches: ' + (verifiable - totals.agree) + ' (country ' + totals.country_mismatch + + ', name ' + totals.name_mismatch + ', offline empty ' + totals.offline_empty + ')') + lines.push('- Unverifiable: ' + (totals.liq_empty + totals.both_empty) + + ' (LocationIQ empty ' + totals.liq_empty + ', both empty ' + totals.both_empty + ')') + lines.push('- Requests used on ' + params.quota.date + ' (UTC): ' + params.quota.count + '/' + params.dailyCap) + if (params.stopReason) { + lines.push('- Run stopped early: ' + params.stopReason + (params.stopDetail ? ' (' + params.stopDetail + ')' : '')) + } + lines.push('') + lines.push('## Countries ranked by mismatch rate (worst first)') + lines.push('') + lines.push('| Country | Points | Verifiable | Agreement | Country mismatch | Name mismatch | Offline empty | LIQ empty |') + lines.push('| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: |') + for (var c = 0; c < countryRows.length; c++) { + var row = countryRows[c] + var pct = row.verifiable > 0 ? ((row.agree * 100) / row.verifiable).toFixed(1) + '%' : '-' + lines.push('| ' + mdEscape(row.country) + ' | ' + row.evaluated + ' | ' + row.verifiable + ' | ' + pct + + ' | ' + row.country_mismatch + ' | ' + row.name_mismatch + ' | ' + row.offline_empty + ' | ' + row.liq_empty + ' |') + } + lines.push('') + lines.push('## Worst examples') + lines.push('') + if (worst.length) { + lines.push('| Lat | Lon | Sampled place | Offline answer | LocationIQ answer | Verdict |') + lines.push('| ---: | ---: | --- | --- | --- | --- |') + for (var w = 0; w < worst.length; w++) { + var bad = worst[w] + lines.push('| ' + bad.lat + ' | ' + bad.lon + + ' | ' + mdEscape(formatAnswer(bad.sample_name, bad.country)) + + ' | ' + mdEscape(formatAnswer(bad.offline_name, bad.offline_country)) + + ' | ' + mdEscape(formatAnswer(bad.liq_name, bad.liq_country, bad.liq_display_name)) + + ' | ' + bad.verdict + ' |') + } + } else { + lines.push('No mismatches recorded.') + } + lines.push('') + lines.push('## Verdicts') + lines.push('') + lines.push('- `agree`: countries match and the offline name matches a LocationIQ locality/county/state field (or a display-name segment) after case/diacritics-insensitive normalization') + lines.push('- `country_mismatch` (severe): the two geocoders disagree on the country') + lines.push('- `name_mismatch`: countries match but no LocationIQ field matches the offline name') + lines.push('- `offline_empty`: LocationIQ answered but the offline geocoder returned nothing') + lines.push('- `liq_empty` / `both_empty`: LocationIQ had no answer — excluded from agreement figures') + lines.push('') + return lines.join('\n') +} + +function writeSweepMismatches(mismatchesPath, records) { + var lines = [] + for (var i = 0; i < records.length; i++) { + if (SWEEP_SEVERITY[records[i].verdict]) { + lines.push(JSON.stringify(records[i])) + } + } + fs.mkdirSync(path.dirname(mismatchesPath), { recursive: true }) + fs.writeFileSync(mismatchesPath, lines.length ? lines.join('\n') + '\n' : '', 'utf8') + return lines.length +} + +async function runSweep(opts, deps) { + var log = (deps && deps.log) || console.log + var sleepImpl = (deps && deps.sleep) || sleep + var nowImpl = (deps && deps.now) || function() { return new Date() } + + if (!deps || typeof deps.fetchJson !== 'function') throw new Error('runSweep requires deps.fetchJson') + if (typeof deps.reverse !== 'function') throw new Error('runSweep requires deps.reverse') + + var loaded = loadPointsFile(opts.pointsPath) + var points = loaded.points + if (!points.length) { + throw new Error('No usable points in ' + opts.pointsPath) + } + if (loaded.skipped || loaded.duplicates) { + log('Points file: skipped ' + loaded.skipped + ' invalid and ' + loaded.duplicates + ' duplicate rows') + } + + fs.mkdirSync(path.dirname(opts.cachePath), { recursive: true }) + var cache = loadSweepCache(opts.cachePath) + var todayUtc = utcDateString(nowImpl()) + var state = loadQuotaState(opts.statePath, todayUtc) + + var stopReason = null + var stopDetail = '' + var requestsThisRun = 0 + var delayMs = Math.ceil(1000 / (opts.rps > 0 ? opts.rps : 1)) + var lastRequestAt = 0 + + if (!opts.dryRun) { + for (var i = 0; i < points.length; i++) { + var point = points[i] + if (cache[point.key]) continue + + if (state.count >= opts.dailyCap) { + stopReason = 'daily_cap' + break + } + if (opts.maxRequests !== null && opts.maxRequests !== undefined && requestsThisRun >= opts.maxRequests) { + stopReason = 'max_requests' + break + } + + var waitMs = lastRequestAt + delayMs - Date.now() + if (waitMs > 0) await sleepImpl(waitMs) + + // Count the attempt before it happens so a crash mid-request can only + // over-count, never let a later run exceed the cap. + state.count += 1 + saveQuotaState(opts.statePath, state) + requestsThisRun += 1 + lastRequestAt = Date.now() + + var url = buildLocationIqUrl(opts.endpoint, opts.apiKey, point.lat, point.lon, opts.acceptLanguage) + var response + try { + response = await deps.fetchJson(url, SWEEP_TIMEOUT_MS) + } catch (err) { + stopReason = 'fetch_error' + stopDetail = String(err && err.message ? err.message : err) + break + } + + var status = Number(response && response.status) + if (status === 200 || status === 400 || status === 404) { + // 200 is an answer; 404 is LocationIQ's definitive "unable to geocode" + // (e.g. open ocean) and 400 a definitive rejection of the coordinates — + // all cacheable so they are never asked again. + var entry = { + key: point.key, + lat: point.lat, + lon: point.lon, + status: status, + body: response.json, + fetched_at: new Date().toISOString() + } + appendSweepCache(opts.cachePath, entry) + cache[point.key] = entry + } else if (status === 401 || status === 403) { + stopReason = 'auth_error' + stopDetail = 'HTTP ' + status + break + } else if (status === 429) { + stopReason = 'rate_limited' + stopDetail = 'HTTP 429' + break + } else { + stopReason = 'server_error' + stopDetail = 'HTTP ' + status + break + } + } + } + + var records = [] + var verdictCounts = Object.create(null) + var unfetched = 0 + for (var j = 0; j < points.length; j++) { + var entryForPoint = cache[points[j].key] + if (!entryForPoint) { + unfetched += 1 + continue + } + var offlineResult = await deps.reverse(points[j].lat, points[j].lon) + var record = comparePoint(points[j], offlineResult || null, entryForPoint) + verdictCounts[record.verdict] = (verdictCounts[record.verdict] || 0) + 1 + records.push(record) + } + + var report = buildSweepReport({ + generatedAt: new Date().toISOString(), + databaseLabel: opts.databaseLabel, + pointsPath: opts.pointsPath, + totalPoints: points.length, + unfetched: unfetched, + records: records, + quota: state, + dailyCap: opts.dailyCap, + stopReason: stopReason, + stopDetail: stopDetail + }) + fs.mkdirSync(path.dirname(opts.reportPath), { recursive: true }) + fs.writeFileSync(opts.reportPath, report, 'utf8') + var mismatchCount = writeSweepMismatches(opts.mismatchesPath, records) + + log('Points: ' + points.length + ' total, ' + records.length + ' evaluated, ' + unfetched + ' awaiting fetch') + log('LocationIQ requests this run: ' + requestsThisRun + ' (today ' + state.date + ' UTC: ' + state.count + '/' + opts.dailyCap + ')') + log('Report: ' + opts.reportPath) + log('Mismatches: ' + opts.mismatchesPath + ' (' + mismatchCount + ' rows)') + + if (stopReason === 'daily_cap') { + log('Daily request cap reached (' + state.count + '/' + opts.dailyCap + ' for ' + state.date + ' UTC). ' + + unfetched + ' points still unfetched. Re-run the same command after the next UTC day starts to resume; cached points are never re-queried.') + } else if (stopReason === 'rate_limited') { + log('LocationIQ returned HTTP 429 (rate limited). Backing off and stopping this run cleanly; all fetched responses are cached. Wait for the quota window to reset, then re-run the same command to resume.') + } else if (stopReason === 'auth_error') { + log('LocationIQ rejected the API key (' + stopDetail + '). Check LOCATIONIQ_API_KEY / --api-key, then re-run to resume.') + } else if (stopReason === 'fetch_error' || stopReason === 'server_error') { + log('Request failed (' + stopDetail + '). Stopping this run cleanly; re-run the same command to resume from the cache.') + } else if (stopReason === 'max_requests') { + log('Per-run request limit reached (--max-requests ' + opts.maxRequests + '). Re-run to continue.') + } else if (opts.dryRun && unfetched > 0) { + log('Dry run: ' + unfetched + ' points have no cached response yet; run without --dry-run to fetch them.') + } else if (unfetched === 0) { + log('All points have cached LocationIQ responses.') + } + + return { + totalPoints: points.length, + evaluated: records.length, + unfetched: unfetched, + requestsThisRun: requestsThisRun, + quota: { date: state.date, count: state.count }, + stopReason: stopReason, + stopDetail: stopDetail, + verdictCounts: verdictCounts, + mismatchCount: mismatchCount + } +} + +function parseSweepArgs(argv) { + var opts = { + pointsPath: null, + database: null, + workdir: path.resolve(SWEEP_DEFAULT_WORKDIR), + cachePath: null, + statePath: null, + reportPath: null, + mismatchesPath: null, + apiKey: process.env.LOCATIONIQ_API_KEY || '', + endpoint: 'https://us1.locationiq.com/v1/reverse', + acceptLanguage: 'en', + dailyCap: 4500, + rps: 1, + maxRequests: null, + reverseMode: 'boundary', + basePrecision: 4, + maxPrecision: 7, + dryRun: false, + help: false + } + + for (var i = 0; i < argv.length; i++) { + var arg = argv[i] + + if (arg === '--points') { + opts.pointsPath = path.resolve(argv[++i]) + } else if (arg === '--database' || arg === '-d') { + opts.database = path.resolve(argv[++i]) + } else if (arg === '--workdir') { + opts.workdir = path.resolve(argv[++i]) + } else if (arg === '--cache') { + opts.cachePath = path.resolve(argv[++i]) + } else if (arg === '--state') { + opts.statePath = path.resolve(argv[++i]) + } else if (arg === '--report') { + opts.reportPath = path.resolve(argv[++i]) + } else if (arg === '--mismatches') { + opts.mismatchesPath = path.resolve(argv[++i]) + } else if (arg === '--api-key') { + opts.apiKey = String(argv[++i] || '') + } else if (arg === '--endpoint') { + opts.endpoint = String(argv[++i] || opts.endpoint) + } else if (arg === '--accept-language') { + opts.acceptLanguage = String(argv[++i] || '') + } else if (arg === '--daily-cap') { + opts.dailyCap = Math.max(0, Math.trunc(Number(argv[++i]))) + } else if (arg === '--rps') { + opts.rps = Math.max(0.1, Number(argv[++i])) + } else if (arg === '--max-requests') { + var maxRequests = Number(argv[++i]) + opts.maxRequests = Number.isFinite(maxRequests) && maxRequests >= 0 ? Math.trunc(maxRequests) : null + } else if (arg === '--reverse-mode') { + opts.reverseMode = String(argv[++i] || 'boundary').toLowerCase() + } else if (arg === '--base-precision') { + opts.basePrecision = Math.max(1, Math.trunc(Number(argv[++i]))) + } else if (arg === '--max-precision') { + opts.maxPrecision = Math.max(opts.basePrecision, Math.trunc(Number(argv[++i]))) + } else if (arg === '--dry-run') { + opts.dryRun = true + } else if (arg === '--help' || arg === '-h') { + opts.help = true + } else { + throw new Error('Unknown sweep argument: ' + arg) + } + } + + if (!opts.cachePath) opts.cachePath = path.join(opts.workdir, 'cache.jsonl') + if (!opts.statePath) opts.statePath = path.join(opts.workdir, 'quota.json') + if (!opts.reportPath) opts.reportPath = path.join(opts.workdir, 'report.md') + if (!opts.mismatchesPath) opts.mismatchesPath = path.join(opts.workdir, 'mismatches.jsonl') + + return opts +} + +async function sweepMain(argv) { + var opts = parseSweepArgs(argv) + if (opts.help) { + console.log(sweepUsage()) + return + } + + if (!opts.pointsPath) { + throw new Error('Missing required --points (see `sweep --help`; the sample subcommand builds one)') + } + if (!fs.existsSync(opts.pointsPath)) { + throw new Error('Points file not found: ' + opts.pointsPath) + } + if (!opts.database) { + throw new Error('Missing required --database') + } + if (!fs.existsSync(opts.database)) { + throw new Error('Database not found: ' + opts.database) + } + if (!opts.dryRun && !opts.apiKey) { + throw new Error('Missing LocationIQ API key (--api-key or LOCATIONIQ_API_KEY); use --dry-run to evaluate the cache without network') + } + + fs.mkdirSync(opts.workdir, { recursive: true }) + + var geocoder = createGeocoder({ + database: opts.database, + reverseMode: opts.reverseMode === 'centroid' ? 'centroid' : 'boundary', + boundary: { + basePrecision: opts.basePrecision, + maxPrecision: opts.maxPrecision + } + }) + + try { + opts.databaseLabel = opts.database + await runSweep(opts, { + fetchJson: fetchJson, + reverse: function(latitude, longitude) { + return geocoder.reverse(latitude, longitude) + }, + sleep: sleep, + log: console.log + }) + } finally { + if (geocoder && geocoder.db && typeof geocoder.db.close === 'function') { + await new Promise(function(resolve) { + geocoder.db.close(function() { resolve() }) + }) + } + } +} + +function dispatch() { + var argv = process.argv.slice(2) + if (argv[0] === 'sample') return sampleMain(argv.slice(1)) + if (argv[0] === 'sweep') return sweepMain(argv.slice(1)) + if (argv.length && argv[0].charAt(0) !== '-') { + return Promise.reject(new Error('Unknown subcommand: ' + argv[0] + ' (expected sample, sweep or legacy options; see --help)')) + } + return main() +} + +module.exports = { + normalizeName: normalizeName, + namesMatch: namesMatch, + extractLocationIqLocality: extractLocationIqLocality, + buildLocationIqUrl: buildLocationIqUrl, + parseGeonamesLine: parseGeonamesLine, + selectTopPlaces: selectTopPlaces, + buildSamplePoints: buildSamplePoints, + sweepCoordKey: sweepCoordKey, + utcDateString: utcDateString, + loadPointsFile: loadPointsFile, + loadQuotaState: loadQuotaState, + comparePoint: comparePoint, + buildSweepReport: buildSweepReport, + runSweep: runSweep +} + +if (require.main === module) { + dispatch().catch(function(err) { + console.error(err.message || err) + process.exit(1) + }) +} diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js new file mode 100644 index 0000000..4b17404 --- /dev/null +++ b/spec/locationiq_sweep_spec.js @@ -0,0 +1,420 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const liq = require('../scripts/validate_with_locationiq'); + +const WORLD_POINTS = [ + { lat: 13.6929, lon: -89.2182, country: 'SV', name: 'San Salvador', population: 525990 }, + { lat: 13.4767, lon: -89.3072, country: 'SV', name: 'Nuevo Cuscatlan', population: 7000 }, + { lat: 14.6349, lon: -90.5069, country: 'GT', name: 'Guatemala City', population: 994938 }, + { lat: 48.1374, lon: 11.5755, country: 'DE', name: 'Munich', population: 1260391 }, + { lat: 55.7522, lon: 37.6156, country: 'RU', name: 'Moscow', population: 10381222 } +]; + +function makeTmpDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'offline-geocoder-liq-sweep-')); +} + +function geonamesRow(id, name, lat, lon, featureClass, country, population) { + const cols = new Array(19).fill(''); + cols[0] = String(id); + cols[1] = name; + cols[2] = name; + cols[4] = String(lat); + cols[5] = String(lon); + cols[6] = featureClass; + cols[7] = 'PPL'; + cols[8] = country; + cols[14] = String(population); + return cols.join('\t'); +} + +function writePointsFile(dir, points) { + const file = path.join(dir, 'points.jsonl'); + fs.writeFileSync(file, points.map((p) => JSON.stringify(p)).join('\n') + '\n'); + return file; +} + +function sweepOpts(dir, overrides) { + return Object.assign({ + pointsPath: path.join(dir, 'points.jsonl'), + cachePath: path.join(dir, 'cache.jsonl'), + statePath: path.join(dir, 'quota.json'), + reportPath: path.join(dir, 'report.md'), + mismatchesPath: path.join(dir, 'mismatches.jsonl'), + databaseLabel: 'fixture.sqlite', + apiKey: 'test-key', + endpoint: 'https://liq.invalid/v1/reverse', + acceptLanguage: 'en', + dailyCap: 4500, + rps: 1000, + maxRequests: null, + dryRun: false + }, overrides || {}); +} + +function okResponse(address, displayName) { + return { status: 200, json: { display_name: displayName || '', address } }; +} + +function makeDeps(fetchHandler, reverseHandler) { + const deps = { + calls: [], + fetchJson: async (url) => { + deps.calls.push(url); + return fetchHandler(url, deps.calls.length); + }, + reverse: async (lat, lon) => (reverseHandler + ? reverseHandler(lat, lon) + : { name: 'Testville', country: { id: 'US', name: 'United States' } }), + sleep: async () => {}, + log: () => {} + }; + return deps; +} + +function readState(dir) { + return JSON.parse(fs.readFileSync(path.join(dir, 'quota.json'), 'utf8')); +} + +function cacheLines(dir) { + const file = path.join(dir, 'cache.jsonl'); + if (!fs.existsSync(file)) return []; + return fs.readFileSync(file, 'utf8').split('\n').filter((line) => line.trim()); +} + +function point(overrides) { + return Object.assign({ + key: '13.4767,-89.3072', + lat: 13.4767, + lon: -89.3072, + country: 'SV', + name: 'Sample Place' + }, overrides || {}); +} + +describe('locationiq sweep', () => { + describe('sample generator', () => { + it('selects the top places per country by population and emits point fields', () => { + const tsv = [ + geonamesRow(1, 'Smallville', 40.1, -100.1, 'P', 'US', 100), + geonamesRow(2, 'Bigville', 40.2, -100.2, 'P', 'US', 500), + geonamesRow(3, 'Midville', 40.3, -100.3, 'P', 'US', 300), + geonamesRow(4, 'Pueblo Grande', 20.1, -99.1, 'P', 'MX', 400), + geonamesRow(5, 'Pueblo Chico', 20.2, -99.2, 'P', 'MX', 50) + ].join('\n'); + + const result = liq.buildSamplePoints(tsv, 2, null); + + expect(result.parsed).toEqual(5); + expect(result.skipped).toEqual(0); + expect(result.points.map((p) => p.name)).toEqual([ + 'Pueblo Grande', 'Pueblo Chico', 'Bigville', 'Midville' + ]); + expect(result.points[0]).toEqual({ + lat: 20.1, lon: -99.1, country: 'MX', name: 'Pueblo Grande', population: 400 + }); + }); + + it('applies a total cap round-robin by rank so every country keeps coverage', () => { + const tsv = [ + geonamesRow(1, 'US One', 40.1, -100.1, 'P', 'US', 500), + geonamesRow(2, 'US Two', 40.2, -100.2, 'P', 'US', 400), + geonamesRow(3, 'MX One', 20.1, -99.1, 'P', 'MX', 300), + geonamesRow(4, 'MX Two', 20.2, -99.2, 'P', 'MX', 200) + ].join('\n'); + + const result = liq.buildSamplePoints(tsv, 2, 3); + + expect(result.points.length).toEqual(3); + const byCountry = result.points.reduce((acc, p) => { + acc[p.country] = (acc[p.country] || 0) + 1; + return acc; + }, {}); + expect(byCountry.US).toEqual(1); + expect(byCountry.MX).toEqual(2); + expect(result.points.some((p) => p.name === 'US One')).toBeTrue(); + expect(result.points.some((p) => p.name === 'US Two')).toBeFalse(); + }); + + it('skips non-P feature classes and malformed rows', () => { + const tsv = [ + geonamesRow(1, 'Cityville', 40.1, -100.1, 'P', 'US', 100), + geonamesRow(2, 'Some Region', 41.0, -101.0, 'A', 'US', 99999), + geonamesRow(3, 'Bad Latitude', 999, -100.0, 'P', 'US', 100), + 'too\tfew\tcolumns' + ].join('\n'); + + const result = liq.buildSamplePoints(tsv, 25, null); + + expect(result.parsed).toEqual(1); + expect(result.skipped).toEqual(3); + expect(result.points.map((p) => p.name)).toEqual(['Cityville']); + }); + }); + + describe('name comparison', () => { + it('normalizes case, diacritics and non-Latin scripts', () => { + expect(liq.normalizeName('Kilómetro 18')).toEqual('kilometro 18'); + expect(liq.normalizeName('MÜNCHEN')).toEqual('munchen'); + expect(liq.normalizeName('São Paulo')).toEqual('sao paulo'); + expect(liq.normalizeName('Москва')).toEqual('москва'); + }); + + it('agrees when the offline name matches a locality field after normalization', () => { + const record = liq.comparePoint( + point({ name: 'Kilómetro 18' }), + { name: 'Kilómetro 18', country: { id: 'SV' } }, + { status: 200, body: { display_name: 'Kilometro 18, El Salvador', address: { village: 'Kilometro 18', country_code: 'sv' } } } + ); + + expect(record.verdict).toEqual('agree'); + expect(record.match_via).toEqual('village'); + }); + + it('agrees when the offline name matches the county or state instead of the locality', () => { + const countyRecord = liq.comparePoint( + point(), + { name: 'Sacatepéquez', country: { id: 'GT' } }, + { status: 200, body: { display_name: '', address: { city: 'Antigua Guatemala', county: 'Sacatepequez', country_code: 'gt' } } } + ); + expect(countyRecord.verdict).toEqual('agree'); + expect(countyRecord.match_via).toEqual('county'); + + const stateRecord = liq.comparePoint( + point(), + { name: 'La Libertad', country: { id: 'SV' } }, + { status: 200, body: { display_name: '', address: { city: 'Zaragoza', state: 'La Libertad', country_code: 'sv' } } } + ); + expect(stateRecord.verdict).toEqual('agree'); + expect(stateRecord.match_via).toEqual('state'); + }); + + it('flags country mismatches as severe even when the names agree', () => { + const record = liq.comparePoint( + point({ country: 'GT' }), + { name: 'Tecún Umán', country: { id: 'GT' } }, + { status: 200, body: { display_name: '', address: { city: 'Tecun Uman', country_code: 'mx' } } } + ); + + expect(record.verdict).toEqual('country_mismatch'); + }); + + it('classifies name mismatches and empty answers', () => { + const nameMismatch = liq.comparePoint( + point(), + { name: 'Zaragoza', country: { id: 'SV' } }, + { status: 200, body: { display_name: 'Nuevo Cuscatlan, La Libertad, El Salvador', address: { city: 'Nuevo Cuscatlan', country_code: 'sv' } } } + ); + expect(nameMismatch.verdict).toEqual('name_mismatch'); + + const offlineEmpty = liq.comparePoint( + point(), + null, + { status: 200, body: { display_name: '', address: { city: 'Somewhere', country_code: 'sv' } } } + ); + expect(offlineEmpty.verdict).toEqual('offline_empty'); + + const liqEmpty = liq.comparePoint( + point(), + { name: 'Zaragoza', country: { id: 'SV' } }, + { status: 404, body: { error: 'Unable to geocode' } } + ); + expect(liqEmpty.verdict).toEqual('liq_empty'); + + const bothEmpty = liq.comparePoint( + point(), + null, + { status: 404, body: { error: 'Unable to geocode' } } + ); + expect(bothEmpty.verdict).toEqual('both_empty'); + }); + }); + + describe('runSweep', () => { + it('stops at the daily cap, persists quota state, and resumes when the cap allows', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const respond = () => okResponse({ city: 'Testville', country_code: 'us' }, 'Testville'); + + const first = makeDeps(respond); + const summary = await liq.runSweep(sweepOpts(dir, { dailyCap: 3 }), first); + expect(first.calls.length).toEqual(3); + expect(first.calls[0]).toContain('key=test-key'); + expect(first.calls[0]).toContain('accept-language=en'); + expect(summary.stopReason).toEqual('daily_cap'); + expect(summary.requestsThisRun).toEqual(3); + expect(cacheLines(dir).length).toEqual(3); + + const state = readState(dir); + expect(state.date).toEqual(liq.utcDateString(new Date())); + expect(state.count).toEqual(3); + + // Same UTC day, same cap: a new invocation must not spend any requests. + const second = makeDeps(respond); + const summary2 = await liq.runSweep(sweepOpts(dir, { dailyCap: 3 }), second); + expect(second.calls.length).toEqual(0); + expect(summary2.stopReason).toEqual('daily_cap'); + expect(readState(dir).count).toEqual(3); + + // A raised cap resumes, fetching only the uncached points. + const third = makeDeps(respond); + const summary3 = await liq.runSweep(sweepOpts(dir, { dailyCap: 10 }), third); + expect(third.calls.length).toEqual(2); + expect(summary3.stopReason).toBeNull(); + expect(summary3.evaluated).toEqual(5); + expect(readState(dir).count).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('never re-queries points that already have a cached response', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const cached = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: 'San Salvador, El Salvador', address: { city: 'San Salvador', country_code: 'sv' } } + }; + fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(4); + expect(deps.calls.some((url) => url.includes('lat=13.6929'))).toBeFalse(); + expect(summary.evaluated).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('resets the persisted quota count on a new UTC day', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ date: '2000-01-01', count: 4500 }) + '\n'); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(5); + expect(summary.stopReason).toBeNull(); + const state = readState(dir); + expect(state.date).toEqual(liq.utcDateString(new Date())); + expect(state.count).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('backs off and stops cleanly on HTTP 429 without caching the failure', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps((url, callNumber) => { + if (callNumber === 1) return okResponse({ city: 'Testville', country_code: 'us' }); + return { status: 429, json: { error: 'Rate Limited Second' } }; + }); + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(2); + expect(summary.stopReason).toEqual('rate_limited'); + expect(cacheLines(dir).length).toEqual(1); + expect(summary.evaluated).toEqual(1); + // The failed attempt still counts against the persisted quota. + expect(readState(dir).count).toEqual(2); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('evaluates the cache without any network calls in dry-run mode', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const cached = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } + }; + fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + + const deps = makeDeps(() => { + throw new Error('dry-run must not touch the network'); + }); + const summary = await liq.runSweep(sweepOpts(dir, { dryRun: true, apiKey: '' }), deps); + + expect(deps.calls.length).toEqual(0); + expect(summary.evaluated).toEqual(1); + expect(summary.unfetched).toEqual(4); + expect(fs.existsSync(path.join(dir, 'report.md'))).toBeTrue(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('writes the per-country Markdown report and the mismatch JSONL', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, [ + { lat: 13.6929, lon: -89.2182, country: 'SV', name: 'San Salvador' }, + { lat: 13.4767, lon: -89.3072, country: 'SV', name: 'Nuevo Cuscatlan' }, + { lat: 14.6349, lon: -90.5069, country: 'GT', name: 'Guatemala City' } + ]); + + const responses = { + '13.6929': okResponse({ city: 'San Salvador', country_code: 'sv' }, 'San Salvador, El Salvador'), + '13.4767': okResponse({ city: 'Nuevo Cuscatlan', country_code: 'sv' }, 'Nuevo Cuscatlan, El Salvador'), + '14.6349': okResponse({ city: 'Guatemala City', country_code: 'gt' }, 'Guatemala City, Guatemala') + }; + const offline = { + '13.6929': { name: 'San Salvador', country: { id: 'SV' } }, + '13.4767': { name: 'Zaragoza', country: { id: 'SV' } }, + '14.6349': { name: 'Tapachula', country: { id: 'MX' } } + }; + const deps = makeDeps( + (url) => responses[new URL(url).searchParams.get('lat')], + (lat) => offline[String(lat)] + ); + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(summary.evaluated).toEqual(3); + expect(summary.verdictCounts).toEqual({ agree: 1, name_mismatch: 1, country_mismatch: 1 }); + expect(summary.mismatchCount).toEqual(2); + + const report = fs.readFileSync(path.join(dir, 'report.md'), 'utf8'); + expect(report).toContain('- Verifiable points: 3 — agreement 1/3 (33.3%)'); + expect(report).toContain('| GT | 1 | 1 | 0.0% | 1 | 0 |'); + expect(report).toContain('| SV | 2 | 2 | 50.0% | 0 | 1 |'); + // GT (100% mismatch) must rank above SV (50% mismatch). + expect(report.indexOf('| GT |')).toBeLessThan(report.indexOf('| SV |')); + expect(report).toContain('## Worst examples'); + expect(report).toContain('country_mismatch'); + expect(report).toContain('14.6349'); + + const mismatches = fs.readFileSync(path.join(dir, 'mismatches.jsonl'), 'utf8') + .split('\n') + .filter((line) => line.trim()) + .map((line) => JSON.parse(line)); + expect(mismatches.length).toEqual(2); + expect(mismatches.map((m) => m.verdict).sort()).toEqual(['country_mismatch', 'name_mismatch']); + const severe = mismatches.find((m) => m.verdict === 'country_mismatch'); + expect(severe.offline_name).toEqual('Tapachula'); + expect(severe.liq_name).toEqual('Guatemala City'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + }); +}); From 60fd9e27bcfef8d6e4ee9cee676df19319dfc5fb Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Tue, 18 Aug 2026 16:19:01 -0600 Subject: [PATCH 02/12] Harden quota safety and Unicode handling per review Address Codex review findings on the LocationIQ sweep: - Fail closed when an existing quota state file is unreadable instead of resetting today's count to zero, and write the state atomically (temp file + rename) so an interrupted save cannot truncate it. - Reject non-numeric --daily-cap/--rps/--max-requests/--per-country/ --max-points/precision values instead of letting NaN silently disable the daily cap or the rate limit; runSweep also validates its quota options directly as defense in depth. - Roll the quota state over when the UTC day changes mid-run so post-midnight requests are attributed to the new day and a later invocation cannot spend nearly twice the cap. - Preserve essential Unicode marks (e.g. Devanagari vowel signs) during name normalization while still stripping optional vocalization (Latin diacritics, Arabic harakat, Hebrew niqqud). - Repair a torn cache tail before appending so a new entry is never glued onto a truncated fragment and refetched on every run. - Load sqlite3 lazily so the TSV-only sample command works without the optional peer dependency installed. Six new regression specs cover the fixes; 51 specs green. --- scripts/validate_with_locationiq.js | 127 ++++++++++++++++++---- spec/locationiq_sweep_spec.js | 162 ++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 19 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 45bad9e..61af5ca 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -4,7 +4,6 @@ const fs = require('fs') const path = require('path') const https = require('https') -const sqlite3 = require('sqlite3') const createGeocoder = require('../src/index') const geohash = require('../src/geohash') @@ -55,6 +54,14 @@ function parseBool(value, defaultValue) { return defaultValue } +function requireNumericArg(flag, raw) { + var value = Number(raw) + if (raw === undefined || raw === null || String(raw).trim() === '' || !Number.isFinite(value)) { + throw new Error(flag + ' requires a numeric value, got: ' + (raw === undefined ? '(missing)' : raw)) + } + return value +} + function parseArgs(argv) { var opts = { database: null, @@ -134,6 +141,9 @@ function mulberry32(seed) { } function dbOpen(dbPath) { + // Lazy so TSV-only commands (sample) work when the optional sqlite3 peer + // dependency is not installed. + var sqlite3 = require('sqlite3') return new sqlite3.Database(dbPath) } @@ -406,9 +416,14 @@ function normalizeName(value) { if (!value) return '' return String(value) .normalize('NFKD') - .replace(/[\u0300-\u036f]/g, '') + // Strip only marks that are optional spelling variants: Latin combining + // diacritics (U+0300-U+036F), Hebrew niqqud/cantillation (U+0591-U+05C7) + // and Arabic harakat (U+064B-U+065F, U+0670). Marks in other scripts, + // e.g. Devanagari vowel signs, are essential letters and are preserved + // by \p{M} below. + .replace(/[\u0300-\u036f\u0591-\u05c7\u064b-\u065f\u0670]/g, '') .toLowerCase() - .replace(/[^\p{L}\p{N}]+/gu, ' ') + .replace(/[^\p{L}\p{M}\p{N}]+/gu, ' ') .trim() .replace(/\s+/g, ' ') } @@ -1106,10 +1121,13 @@ function parseSampleArgs(argv) { } else if (arg === '--out') { opts.out = path.resolve(argv[++i]) } else if (arg === '--per-country') { - opts.perCountry = Math.max(1, Math.trunc(Number(argv[++i]))) + opts.perCountry = Math.max(1, Math.trunc(requireNumericArg('--per-country', argv[++i]))) } else if (arg === '--max-points') { - var maxPoints = Number(argv[++i]) - opts.maxPoints = Number.isFinite(maxPoints) && maxPoints > 0 ? Math.trunc(maxPoints) : null + var maxPoints = Math.trunc(requireNumericArg('--max-points', argv[++i])) + if (maxPoints <= 0) { + throw new Error('--max-points must be > 0, got: ' + maxPoints) + } + opts.maxPoints = maxPoints } else if (arg === '--help' || arg === '-h') { opts.help = true } else { @@ -1223,24 +1241,68 @@ function loadSweepCache(cachePath) { } function appendSweepCache(cachePath, entry) { - fs.appendFileSync(cachePath, JSON.stringify(entry) + '\n', 'utf8') + // If an interrupted append left a torn final line without a newline, + // appending directly would glue the new entry onto the fragment, making + // both unreadable and re-spending a request on that point every run. + // Start a fresh line whenever the file does not already end with one. + var prefix = '' + try { + var stat = fs.statSync(cachePath) + if (stat.size > 0) { + var fd = fs.openSync(cachePath, 'r') + var lastByte = Buffer.alloc(1) + try { + fs.readSync(fd, lastByte, 0, 1, stat.size - 1) + } finally { + fs.closeSync(fd) + } + if (lastByte.toString('utf8') !== '\n') prefix = '\n' + } + } catch (err) { + // Missing file: the append below creates it. + } + fs.appendFileSync(cachePath, prefix + JSON.stringify(entry) + '\n', 'utf8') } function loadQuotaState(statePath, todayUtc) { + var raw try { - var raw = JSON.parse(fs.readFileSync(statePath, 'utf8')) - if (raw && raw.date === todayUtc && Number.isFinite(Number(raw.count))) { - return { date: todayUtc, count: Math.max(0, Math.trunc(Number(raw.count))) } + raw = fs.readFileSync(statePath, 'utf8') + } catch (err) { + if (err && err.code === 'ENOENT') { + return { date: todayUtc, count: 0 } } + throw err + } + + var parsed = null + try { + parsed = JSON.parse(raw) } catch (err) { - // Missing or unreadable state resets to zero for today. + parsed = null + } + + // Fail closed: an existing-but-unreadable state file must not silently + // reset today's count to zero, or a corrupted file would allow a fresh + // full daily cap of requests. + if (!parsed || typeof parsed.date !== 'string' || !Number.isFinite(Number(parsed.count))) { + throw new Error('Quota state file ' + statePath + ' exists but is unreadable; refusing to guess the request count. ' + + 'Inspect it and, only if you are sure no requests were made today (UTC), delete it to reset.') } - return { date: todayUtc, count: 0 } + + if (parsed.date !== todayUtc) { + return { date: todayUtc, count: 0 } + } + return { date: todayUtc, count: Math.max(0, Math.trunc(Number(parsed.count))) } } function saveQuotaState(statePath, state) { + // Write-then-rename so an interrupted save can never leave a truncated + // state file behind (which loadQuotaState would refuse to read). fs.mkdirSync(path.dirname(statePath), { recursive: true }) - fs.writeFileSync(statePath, JSON.stringify({ date: state.date, count: state.count }) + '\n', 'utf8') + var tmpPath = statePath + '.tmp' + fs.writeFileSync(tmpPath, JSON.stringify({ date: state.date, count: state.count }) + '\n', 'utf8') + fs.renameSync(tmpPath, statePath) } function findLiqNameMatch(normalizedOfflineName, address, displayName) { @@ -1449,6 +1511,19 @@ async function runSweep(opts, deps) { if (!deps || typeof deps.fetchJson !== 'function') throw new Error('runSweep requires deps.fetchJson') if (typeof deps.reverse !== 'function') throw new Error('runSweep requires deps.reverse') + // Guard the quota knobs: a NaN cap would make every comparison false and + // silently disable the daily limit. + if (!Number.isFinite(opts.dailyCap) || opts.dailyCap < 0) { + throw new Error('dailyCap must be a finite number >= 0, got: ' + opts.dailyCap) + } + if (!Number.isFinite(opts.rps) || opts.rps <= 0) { + throw new Error('rps must be a finite number > 0, got: ' + opts.rps) + } + if (opts.maxRequests !== null && opts.maxRequests !== undefined && + (!Number.isFinite(opts.maxRequests) || opts.maxRequests < 0)) { + throw new Error('maxRequests must be a finite number >= 0 when set, got: ' + opts.maxRequests) + } + var loaded = loadPointsFile(opts.pointsPath) var points = loaded.points if (!points.length) { @@ -1474,6 +1549,16 @@ async function runSweep(opts, deps) { var point = points[i] if (cache[point.key]) continue + // A long run can cross midnight UTC: roll the state over so requests + // are attributed to the day they are actually made in, otherwise a + // later invocation would reset the stale date and allow nearly twice + // the cap within the new UTC day. + var attemptDate = utcDateString(nowImpl()) + if (attemptDate !== state.date) { + state = { date: attemptDate, count: 0 } + saveQuotaState(opts.statePath, state) + } + if (state.count >= opts.dailyCap) { stopReason = 'daily_cap' break @@ -1646,18 +1731,19 @@ function parseSweepArgs(argv) { } else if (arg === '--accept-language') { opts.acceptLanguage = String(argv[++i] || '') } else if (arg === '--daily-cap') { - opts.dailyCap = Math.max(0, Math.trunc(Number(argv[++i]))) + // Reject non-numeric values outright: a NaN here would silently + // disable the daily quota instead of enforcing it. + opts.dailyCap = Math.max(0, Math.trunc(requireNumericArg('--daily-cap', argv[++i]))) } else if (arg === '--rps') { - opts.rps = Math.max(0.1, Number(argv[++i])) + opts.rps = Math.max(0.1, requireNumericArg('--rps', argv[++i])) } else if (arg === '--max-requests') { - var maxRequests = Number(argv[++i]) - opts.maxRequests = Number.isFinite(maxRequests) && maxRequests >= 0 ? Math.trunc(maxRequests) : null + opts.maxRequests = Math.max(0, Math.trunc(requireNumericArg('--max-requests', argv[++i]))) } else if (arg === '--reverse-mode') { opts.reverseMode = String(argv[++i] || 'boundary').toLowerCase() } else if (arg === '--base-precision') { - opts.basePrecision = Math.max(1, Math.trunc(Number(argv[++i]))) + opts.basePrecision = Math.max(1, Math.trunc(requireNumericArg('--base-precision', argv[++i]))) } else if (arg === '--max-precision') { - opts.maxPrecision = Math.max(opts.basePrecision, Math.trunc(Number(argv[++i]))) + opts.maxPrecision = Math.max(opts.basePrecision, Math.trunc(requireNumericArg('--max-precision', argv[++i]))) } else if (arg === '--dry-run') { opts.dryRun = true } else if (arg === '--help' || arg === '-h') { @@ -1746,10 +1832,13 @@ module.exports = { parseGeonamesLine: parseGeonamesLine, selectTopPlaces: selectTopPlaces, buildSamplePoints: buildSamplePoints, + parseSampleArgs: parseSampleArgs, + sampleMain: sampleMain, sweepCoordKey: sweepCoordKey, utcDateString: utcDateString, loadPointsFile: loadPointsFile, loadQuotaState: loadQuotaState, + parseSweepArgs: parseSweepArgs, comparePoint: comparePoint, buildSweepReport: buildSweepReport, runSweep: runSweep diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 4b17404..8c0fa8f 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -1,6 +1,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); +const { spawnSync } = require('child_process'); const liq = require('../scripts/validate_with_locationiq'); @@ -162,6 +163,17 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('Москва')).toEqual('москва'); }); + it('preserves essential Unicode marks while stripping optional vocalization', () => { + // Devanagari vowel signs are combining marks but essential: stripping + // them would collapse different names into false agreements. + expect(liq.normalizeName('किला')).toEqual('किला'); + expect(liq.normalizeName('किला')).not.toEqual(liq.normalizeName('कुल')); + // Arabic harakat and Hebrew niqqud are optional vocalization: the same + // name with and without them must compare equal. + expect(liq.normalizeName('مُحَمَّد')).toEqual(liq.normalizeName('محمد')); + expect(liq.normalizeName('יְרוּשָׁלַיִם')).toEqual(liq.normalizeName('ירושלים')); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -416,5 +428,155 @@ describe('locationiq sweep', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it('fails closed without spending requests when the quota state file is unreadable', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + // A truncated write (e.g. the process died mid-save) leaves invalid + // JSON behind. Resetting to zero here would allow a fresh full cap. + fs.writeFileSync(path.join(dir, 'quota.json'), '{"date":"2026-'); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/quota state/i); + expect(deps.calls.length).toEqual(0); + + // Same for structurally invalid but parseable content. + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ foo: 1 })); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/quota state/i); + expect(deps.calls.length).toEqual(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rejects invalid quota options instead of running uncapped', async () => { + expect(() => liq.parseSweepArgs(['--daily-cap', 'lots'])).toThrowError(/--daily-cap/); + expect(() => liq.parseSweepArgs(['--daily-cap'])).toThrowError(/--daily-cap/); + expect(() => liq.parseSweepArgs(['--rps', 'fast'])).toThrowError(/--rps/); + expect(() => liq.parseSweepArgs(['--max-requests', 'many'])).toThrowError(/--max-requests/); + + // Defense in depth: runSweep itself refuses a NaN cap before fetching. + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + await expectAsync(liq.runSweep(sweepOpts(dir, { dailyCap: NaN }), deps)).toBeRejectedWithError(/dailyCap/); + expect(deps.calls.length).toEqual(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rolls quota state over when the UTC day changes mid-run', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + // now() is called once at startup, then once per attempt: the first + // two attempts happen before midnight UTC, the last three after. + let nowCalls = 0; + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + deps.now = () => { + nowCalls += 1; + return nowCalls <= 3 + ? new Date('2026-01-01T23:59:00Z') + : new Date('2026-01-02T00:00:30Z'); + }; + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(5); + expect(summary.stopReason).toBeNull(); + // Requests made after midnight are attributed to the new UTC day, so + // a later invocation cannot reset a stale date and double the cap. + const state = readState(dir); + expect(state.date).toEqual('2026-01-02'); + expect(state.count).toEqual(3); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('repairs a torn cache tail so new entries are never glued onto a fragment', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const seeded = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: '', address: { city: 'San Salvador', country_code: 'sv' } } + }; + // Valid entry followed by a torn final line with no newline, as an + // interrupted append would leave it. + fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(seeded) + '\n' + '{"key":"13.4767'); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(4); + expect(summary.evaluated).toEqual(5); + const parseable = cacheLines(dir).filter((line) => { + try { + JSON.parse(line); + return true; + } catch (err) { + return false; + } + }); + expect(parseable.length).toEqual(5); + + // The repaired cache must fully satisfy a second run: zero requests. + const again = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary2 = await liq.runSweep(sweepOpts(dir), again); + expect(again.calls.length).toEqual(0); + expect(summary2.evaluated).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + }); + + describe('optional sqlite3 dependency', () => { + it('runs the TSV-only sample command without sqlite3 installed', () => { + const dir = makeTmpDir(); + try { + const tsvPath = path.join(dir, 'cities.tsv'); + const outPath = path.join(dir, 'points.jsonl'); + fs.writeFileSync(tsvPath, geonamesRow(1, 'Testville', 40.0, -100.0, 'P', 'US', 1000) + '\n'); + + const script = path.join(__dirname, '..', 'scripts', 'validate_with_locationiq.js'); + const shim = [ + "const Module = require('module');", + 'const originalLoad = Module._load;', + 'Module._load = function(request, ...rest) {', + " if (request === 'sqlite3') {", + ' const err = new Error("Cannot find module \'sqlite3\'");', + " err.code = 'MODULE_NOT_FOUND';", + ' throw err;', + ' }', + ' return originalLoad.call(this, request, ...rest);', + '};', + 'const liq = require(process.env.LIQ_SCRIPT);', + "liq.sampleMain(['--geonames', process.env.LIQ_TSV, '--out', process.env.LIQ_OUT])", + " .then(() => console.log('SAMPLE_OK'))", + ' .catch((err) => { console.error(err.message); process.exit(1); });' + ].join('\n'); + + const result = spawnSync('node', ['-e', shim], { + encoding: 'utf8', + env: Object.assign({}, process.env, { LIQ_SCRIPT: script, LIQ_TSV: tsvPath, LIQ_OUT: outPath }) + }); + + expect(result.status).toEqual(0); + expect(result.stdout).toContain('SAMPLE_OK'); + const written = fs.readFileSync(outPath, 'utf8').trim().split('\n').map((line) => JSON.parse(line)); + expect(written.length).toEqual(1); + expect(written[0].name).toEqual('Testville'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); }); From daed2e86e598735d39e30d29592b7694c6cf812e Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Tue, 18 Aug 2026 21:53:40 -0600 Subject: [PATCH 03/12] Address round-2 review: normalization, quota edges, sampling fairness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip U+0300-036F combining marks only from Latin base letters: NFKD decomposes Cyrillic й/ё/ї into base + mark from that same block, so unconditional stripping collapsed distinct letters into false agreements. - Re-derive the UTC day after the rate-limit wait, so a sleep that crosses midnight attributes the request to the day it is actually made in and the cap cannot be double-spent. - Classify points where either geocoder omits its country code as a new unverifiable country_unknown verdict (reported separately, excluded from agreement) instead of letting a name-only match count as agreement. - Let --dry-run proceed with a warning and an "unknown" quota line when quota.json is unreadable: it makes no requests and leaves the damaged file untouched for inspection. Network runs still fail closed. - Shuffle the sampler's round-robin country order deterministically so --max-points below the country count drops an unbiased subset instead of the alphabet tail; sample warns how many countries were left out. Five new/extended regression specs; 56 specs green. --- scripts/validate_with_locationiq.js | 139 +++++++++++++++++++++------- spec/locationiq_sweep_spec.js | 134 ++++++++++++++++++++++++++- 2 files changed, 234 insertions(+), 39 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 61af5ca..f5fc3c6 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -414,14 +414,32 @@ async function ensureSamplePoints(sourceDb, cacheDb, lookupTable, targetCount, s function normalizeName(value) { if (!value) return '' - return String(value) - .normalize('NFKD') - // Strip only marks that are optional spelling variants: Latin combining - // diacritics (U+0300-U+036F), Hebrew niqqud/cantillation (U+0591-U+05C7) - // and Arabic harakat (U+064B-U+065F, U+0670). Marks in other scripts, - // e.g. Devanagari vowel signs, are essential letters and are preserved - // by \p{M} below. - .replace(/[\u0300-\u036f\u0591-\u05c7\u064b-\u065f\u0670]/g, '') + + // Strip only marks that are optional spelling variants, and only where + // they are optional. Combining diacritics (U+0300-U+036F) are dropped + // solely when they modify a Latin base letter (e vs \u00e9): the same block + // spells essential letters in other scripts (Cyrillic \u0438 + breve = \u0439), so + // stripping them unconditionally would collapse distinct names. Hebrew + // niqqud/cantillation (U+0591-U+05C7) and Arabic harakat (U+064B-U+065F, + // U+0670) are optional vocalization regardless of position. All other + // marks (e.g. Devanagari vowel signs) are preserved via \p{M} below. + var decomposed = String(value).normalize('NFKD') + var kept = '' + var lastBaseIsLatin = false + for (var ch of decomposed) { + var code = ch.codePointAt(0) + if (code >= 0x0300 && code <= 0x036f) { + if (!lastBaseIsLatin) kept += ch + continue + } + if ((code >= 0x0591 && code <= 0x05c7) || (code >= 0x064b && code <= 0x065f) || code === 0x0670) { + continue + } + kept += ch + lastBaseIsLatin = (code >= 0x41 && code <= 0x5a) || (code >= 0x61 && code <= 0x7a) + } + + return kept .toLowerCase() .replace(/[^\p{L}\p{M}\p{N}]+/gu, ' ') .trim() @@ -927,6 +945,10 @@ async function main() { var SWEEP_CACHE_DECIMALS = 4 var SWEEP_TIMEOUT_MS = 20000 var SWEEP_DEFAULT_WORKDIR = 'tmp/locationiq-sweep' +// Fixed seed for the sampler's country-order shuffle: stable across runs, but +// not alphabetical, so a small --max-points does not bias the world sample +// toward alphabetically-early country codes. +var SWEEP_SAMPLE_SHUFFLE_SEED = 1729 var SWEEP_SEVERITY = { country_mismatch: 3, offline_empty: 2, @@ -951,7 +973,9 @@ function sampleUsage() { ' --geonames GeoNames-style TSV input (required)', ' --out Output JSONL points file (default: ' + SWEEP_DEFAULT_WORKDIR + '/points.jsonl)', ' --per-country Places per country, most populous first (default: 25)', - ' --max-points Optional total cap, filled round-robin by rank so every country keeps coverage', + ' --max-points Optional total cap, filled round-robin by rank over a deterministically', + ' shuffled country order; a cap below the country count leaves some', + ' countries out (an unbiased subset — a warning reports how many)', ' --help, -h Show this help message' ].join('\n') } @@ -1057,10 +1081,14 @@ function selectTopPlaces(rows, perCountry, maxPoints) { var picked = [] if (maxPoints && total > maxPoints) { // Fill round-robin by rank so a total cap trims depth per country instead - // of dropping whole countries. + // of dropping whole countries. The per-rank country order is shuffled + // deterministically: with an alphabetical order, a cap smaller than the + // country count would always drop the same alphabetically-late countries + // and systematically bias the world report. + var order = deterministicShuffle(countries, SWEEP_SAMPLE_SHUFFLE_SEED) for (var rank = 0; rank < perCountry && picked.length < maxPoints; rank++) { - for (var j = 0; j < countries.length && picked.length < maxPoints; j++) { - var ranked = byCountry[countries[j]] + for (var j = 0; j < order.length && picked.length < maxPoints; j++) { + var ranked = byCountry[order[j]] if (rank < ranked.length) picked.push(ranked[rank]) } } @@ -1097,10 +1125,16 @@ function buildSamplePoints(tsvText, perCountry, maxPoints) { if (row) rows.push(row) else skipped += 1 } + var countriesTotal = Object.create(null) + for (var j = 0; j < rows.length; j++) { + countriesTotal[rows[j].country] = true + } + return { points: selectTopPlaces(rows, perCountry, maxPoints), parsed: rows.length, - skipped: skipped + skipped: skipped, + countriesTotal: Object.keys(countriesTotal).length } } @@ -1170,8 +1204,14 @@ async function sampleMain(argv) { fs.mkdirSync(path.dirname(opts.out), { recursive: true }) fs.writeFileSync(opts.out, lines.join('\n') + '\n', 'utf8') + var coveredCountries = Object.keys(countries).length console.log('Parsed rows: ' + result.parsed + ' (skipped ' + result.skipped + ')') - console.log('Points written: ' + result.points.length + ' across ' + Object.keys(countries).length + ' countries') + console.log('Points written: ' + result.points.length + ' across ' + coveredCountries + ' countries') + if (coveredCountries < result.countriesTotal) { + console.log('Warning: --max-points ' + opts.maxPoints + ' is below the country count; only ' + + coveredCountries + ' of ' + result.countriesTotal + ' countries are included ' + + '(an unbiased deterministic subset). Increase --max-points for full world coverage.') + } console.log('Points file: ' + opts.out) } @@ -1359,7 +1399,13 @@ function comparePoint(point, offlineResult, cacheEntry) { record.verdict = offlineName ? 'liq_empty' : 'both_empty' } else if (!offlineName) { record.verdict = 'offline_empty' - } else if (offlineCountry && record.liq_country && offlineCountry !== record.liq_country) { + } else if (!offlineCountry || !record.liq_country) { + // One side lacks a country code, so the severe check cannot run and the + // point must not inflate agreement either: classify it as unverifiable. + // Any name match is still recorded in match_via for context. + record.match_via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) || '' + record.verdict = 'country_unknown' + } else if (offlineCountry !== record.liq_country) { record.verdict = 'country_mismatch' } else { var via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) @@ -1387,7 +1433,7 @@ function formatAnswer(name, country, fallback) { function buildSweepReport(params) { var records = params.records || [] var perCountry = Object.create(null) - var totals = { agree: 0, country_mismatch: 0, name_mismatch: 0, offline_empty: 0, liq_empty: 0, both_empty: 0 } + var totals = { agree: 0, country_mismatch: 0, name_mismatch: 0, offline_empty: 0, liq_empty: 0, both_empty: 0, country_unknown: 0 } for (var i = 0; i < records.length; i++) { var record = records[i] @@ -1401,7 +1447,8 @@ function buildSweepReport(params) { name_mismatch: 0, offline_empty: 0, liq_empty: 0, - both_empty: 0 + both_empty: 0, + country_unknown: 0 } } bucket.evaluated += 1 @@ -1445,22 +1492,27 @@ function buildSweepReport(params) { ' (' + agreementPct.toFixed(1) + '%)') lines.push('- Mismatches: ' + (verifiable - totals.agree) + ' (country ' + totals.country_mismatch + ', name ' + totals.name_mismatch + ', offline empty ' + totals.offline_empty + ')') - lines.push('- Unverifiable: ' + (totals.liq_empty + totals.both_empty) + - ' (LocationIQ empty ' + totals.liq_empty + ', both empty ' + totals.both_empty + ')') - lines.push('- Requests used on ' + params.quota.date + ' (UTC): ' + params.quota.count + '/' + params.dailyCap) + lines.push('- Unverifiable: ' + (totals.liq_empty + totals.both_empty + totals.country_unknown) + + ' (LocationIQ empty ' + totals.liq_empty + ', both empty ' + totals.both_empty + + ', country unknown ' + totals.country_unknown + ')') + var quotaCount = params.quota && params.quota.count !== null && params.quota.count !== undefined && + Number.isFinite(Number(params.quota.count)) ? Number(params.quota.count) : null + lines.push('- Requests used on ' + params.quota.date + ' (UTC): ' + + (quotaCount === null ? 'unknown (quota state unreadable)' : quotaCount + '/' + params.dailyCap)) if (params.stopReason) { lines.push('- Run stopped early: ' + params.stopReason + (params.stopDetail ? ' (' + params.stopDetail + ')' : '')) } lines.push('') lines.push('## Countries ranked by mismatch rate (worst first)') lines.push('') - lines.push('| Country | Points | Verifiable | Agreement | Country mismatch | Name mismatch | Offline empty | LIQ empty |') - lines.push('| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: |') + lines.push('| Country | Points | Verifiable | Agreement | Country mismatch | Name mismatch | Offline empty | LIQ empty | Country unknown |') + lines.push('| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |') for (var c = 0; c < countryRows.length; c++) { var row = countryRows[c] var pct = row.verifiable > 0 ? ((row.agree * 100) / row.verifiable).toFixed(1) + '%' : '-' lines.push('| ' + mdEscape(row.country) + ' | ' + row.evaluated + ' | ' + row.verifiable + ' | ' + pct + - ' | ' + row.country_mismatch + ' | ' + row.name_mismatch + ' | ' + row.offline_empty + ' | ' + row.liq_empty + ' |') + ' | ' + row.country_mismatch + ' | ' + row.name_mismatch + ' | ' + row.offline_empty + ' | ' + row.liq_empty + + ' | ' + row.country_unknown + ' |') } lines.push('') lines.push('## Worst examples') @@ -1487,6 +1539,7 @@ function buildSweepReport(params) { lines.push('- `name_mismatch`: countries match but no LocationIQ field matches the offline name') lines.push('- `offline_empty`: LocationIQ answered but the offline geocoder returned nothing') lines.push('- `liq_empty` / `both_empty`: LocationIQ had no answer — excluded from agreement figures') + lines.push('- `country_unknown`: one side answered without a country code, so the comparison cannot be verified — excluded from agreement figures (a name match, if any, is noted in `match_via`)') lines.push('') return lines.join('\n') } @@ -1536,7 +1589,18 @@ async function runSweep(opts, deps) { fs.mkdirSync(path.dirname(opts.cachePath), { recursive: true }) var cache = loadSweepCache(opts.cachePath) var todayUtc = utcDateString(nowImpl()) - var state = loadQuotaState(opts.statePath, todayUtc) + var state + try { + state = loadQuotaState(opts.statePath, todayUtc) + } catch (err) { + if (!opts.dryRun) throw err + // A dry run makes no requests, so an unreadable quota file must not + // block inspecting the cache. Leave the file untouched for inspection + // and report the day's usage as unknown. + state = { date: todayUtc, count: null } + log('Warning: ' + String(err && err.message ? err.message : err)) + log('Continuing anyway: --dry-run makes no requests and leaves the quota state untouched.') + } var stopReason = null var stopDetail = '' @@ -1549,10 +1613,19 @@ async function runSweep(opts, deps) { var point = points[i] if (cache[point.key]) continue - // A long run can cross midnight UTC: roll the state over so requests - // are attributed to the day they are actually made in, otherwise a - // later invocation would reset the stale date and allow nearly twice - // the cap within the new UTC day. + if (opts.maxRequests !== null && opts.maxRequests !== undefined && requestsThisRun >= opts.maxRequests) { + stopReason = 'max_requests' + break + } + + var waitMs = lastRequestAt + delayMs - Date.now() + if (waitMs > 0) await sleepImpl(waitMs) + + // A long run can cross midnight UTC — including during the rate-limit + // wait just above, so the day is derived only after it. Roll the state + // over so the request counts against the day it is actually made in; + // otherwise a later invocation would reset the stale date and allow + // nearly twice the cap within the new UTC day. var attemptDate = utcDateString(nowImpl()) if (attemptDate !== state.date) { state = { date: attemptDate, count: 0 } @@ -1563,13 +1636,6 @@ async function runSweep(opts, deps) { stopReason = 'daily_cap' break } - if (opts.maxRequests !== null && opts.maxRequests !== undefined && requestsThisRun >= opts.maxRequests) { - stopReason = 'max_requests' - break - } - - var waitMs = lastRequestAt + delayMs - Date.now() - if (waitMs > 0) await sleepImpl(waitMs) // Count the attempt before it happens so a crash mid-request can only // over-count, never let a later run exceed the cap. @@ -1651,7 +1717,8 @@ async function runSweep(opts, deps) { var mismatchCount = writeSweepMismatches(opts.mismatchesPath, records) log('Points: ' + points.length + ' total, ' + records.length + ' evaluated, ' + unfetched + ' awaiting fetch') - log('LocationIQ requests this run: ' + requestsThisRun + ' (today ' + state.date + ' UTC: ' + state.count + '/' + opts.dailyCap + ')') + log('LocationIQ requests this run: ' + requestsThisRun + ' (today ' + state.date + ' UTC: ' + + (state.count === null ? 'unknown' : state.count + '/' + opts.dailyCap) + ')') log('Report: ' + opts.reportPath) log('Mismatches: ' + opts.mismatchesPath + ' (' + mismatchCount + ' rows)') diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 8c0fa8f..c31faa5 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -133,10 +133,31 @@ describe('locationiq sweep', () => { acc[p.country] = (acc[p.country] || 0) + 1; return acc; }, {}); - expect(byCountry.US).toEqual(1); - expect(byCountry.MX).toEqual(2); + // With a cap of 3 over 2 countries, both keep their top place and + // exactly one (shuffle-determined) country gets its second. + expect(byCountry.US).toBeGreaterThanOrEqual(1); + expect(byCountry.MX).toBeGreaterThanOrEqual(1); + expect(byCountry.US + byCountry.MX).toEqual(3); expect(result.points.some((p) => p.name === 'US One')).toBeTrue(); - expect(result.points.some((p) => p.name === 'US Two')).toBeFalse(); + expect(result.points.some((p) => p.name === 'MX One')).toBeTrue(); + }); + + it('spreads a cap below the country count over a deterministic non-alphabetical subset', () => { + const codes = ['AA', 'AB', 'AC', 'AD', 'AE', 'AF', 'AG', 'AH', 'AI', 'AJ']; + const tsv = codes + .map((code, i) => geonamesRow(i + 1, code + ' City', i, i, 'P', code, 100)) + .join('\n'); + + const first = liq.buildSamplePoints(tsv, 1, 5); + expect(first.points.length).toEqual(5); + expect(first.countriesTotal).toEqual(10); + const selected = first.points.map((p) => p.country); + // An alphabetical fill would always drop the same alphabet-late + // countries; the shuffled order must not equal the alphabetical prefix. + expect(selected).not.toEqual(['AA', 'AB', 'AC', 'AD', 'AE']); + // ...but it must be deterministic so re-generated points files match. + const second = liq.buildSamplePoints(tsv, 1, 5); + expect(second.points.map((p) => p.country)).toEqual(selected); }); it('skips non-P feature classes and malformed rows', () => { @@ -174,6 +195,19 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('יְרוּשָׁלַיִם')).toEqual(liq.normalizeName('ירושלים')); }); + it('strips combining diacritics only from Latin bases, keeping Cyrillic letters distinct', () => { + // NFKD decomposes these into base + U+0300-block marks; stripping the + // mark unconditionally would collapse distinct letters (й→и, ё→е, ї→і). + expect(liq.normalizeName('й')).not.toEqual(liq.normalizeName('и')); + expect(liq.normalizeName('ё')).not.toEqual(liq.normalizeName('е')); + expect(liq.normalizeName('ї')).not.toEqual(liq.normalizeName('і')); + // Precomposed and decomposed spellings of the same letter still agree. + expect(liq.normalizeName('й')).toEqual(liq.normalizeName('й')); + expect(liq.normalizeName('Йошкар-Ола')).toEqual(liq.normalizeName('йошкар ола')); + // Latin diacritics still fold. + expect(liq.normalizeName('São Paulo')).toEqual('sao paulo'); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -242,6 +276,42 @@ describe('locationiq sweep', () => { ); expect(bothEmpty.verdict).toEqual('both_empty'); }); + + it('marks points country_unknown when either side lacks a country code', () => { + // LocationIQ answered with a matching locality but no country_code: + // the severe check cannot run, and the name match alone must not be + // counted as agreement. + const missingLiq = liq.comparePoint( + point(), + { name: 'Zaragoza', country: { id: 'SV' } }, + { status: 200, body: { display_name: 'Zaragoza', address: { city: 'Zaragoza' } } } + ); + expect(missingLiq.verdict).toEqual('country_unknown'); + expect(missingLiq.match_via).toEqual('city'); + + const missingOffline = liq.comparePoint( + point(), + { name: 'Zaragoza' }, + { status: 200, body: { display_name: '', address: { city: 'Zaragoza', country_code: 'sv' } } } + ); + expect(missingOffline.verdict).toEqual('country_unknown'); + + // The report treats these as unverifiable, not as agreement. + const report = liq.buildSweepReport({ + generatedAt: 'now', + databaseLabel: 'db', + pointsPath: 'points.jsonl', + totalPoints: 1, + unfetched: 0, + records: [missingLiq], + quota: { date: '2026-08-18', count: 0 }, + dailyCap: 4500, + stopReason: null, + stopDetail: '' + }); + expect(report).toContain('- Verifiable points: 0 — agreement 0/0 (0.0%)'); + expect(report).toContain('country unknown 1'); + }); }); describe('runSweep', () => { @@ -536,6 +606,64 @@ describe('locationiq sweep', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it('attributes a request to the new UTC day when the rate-limit wait crosses midnight', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS.slice(0, 2)); + // The clock only advances past midnight during the rate-limit sleep + // before the second request. + let clock = new Date('2026-01-01T23:59:59Z'); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + deps.now = () => clock; + deps.sleep = async () => { clock = new Date('2026-01-02T00:00:01Z'); }; + + const summary = await liq.runSweep(sweepOpts(dir, { rps: 1, dailyCap: 1 }), deps); + + // One request on each UTC day is within a cap of 1 per day; recording + // the post-midnight request against yesterday would either block it + // or let a later run double-spend the new day. + expect(deps.calls.length).toEqual(2); + expect(summary.stopReason).toBeNull(); + const state = readState(dir); + expect(state.date).toEqual('2026-01-02'); + expect(state.count).toEqual(1); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('warns instead of failing when dry-run finds an unreadable quota state file', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const tornState = '{"date":"2026-'; + fs.writeFileSync(path.join(dir, 'quota.json'), tornState); + const cached = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } + }; + fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + + const deps = makeDeps(() => { + throw new Error('dry-run must not touch the network'); + }); + const summary = await liq.runSweep(sweepOpts(dir, { dryRun: true, apiKey: '' }), deps); + + expect(deps.calls.length).toEqual(0); + expect(summary.evaluated).toEqual(1); + expect(summary.quota.count).toBeNull(); + const report = fs.readFileSync(path.join(dir, 'report.md'), 'utf8'); + expect(report).toContain('unknown (quota state unreadable)'); + // The damaged file is left untouched for inspection. + expect(fs.readFileSync(path.join(dir, 'quota.json'), 'utf8')).toEqual(tornState); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe('optional sqlite3 dependency', () => { From b9266e148190c140522ae8b18abe35703367c833 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Tue, 18 Aug 2026 22:05:54 -0600 Subject: [PATCH 04/12] Address round-3 review: hamza marks, cache config stamp, mode validation - Restrict the always-stripped Arabic range to the true harakat (U+064B-U+0652, plus U+0670 superscript alef). Maddah and hamza marks (U+0653-U+0655) are preserved: NFKD decomposes precomposed hamza letters into base + one of these marks, so stripping them collapsed distinct names into false agreements. - Stamp the response cache with the request-shaping options it was built with (endpoint, accept-language) and reject network runs whose options differ, instead of silently evaluating stale responses. Dry runs only evaluate the cache as-is and are exempt. - Validate --reverse-mode against the two documented values instead of silently mapping typos to boundary and measuring the wrong lookup algorithm. Three new regression specs; 59 specs green. --- scripts/validate_with_locationiq.js | 62 +++++++++++++++++++-- spec/locationiq_sweep_spec.js | 85 +++++++++++++++++++++++++---- 2 files changed, 131 insertions(+), 16 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index f5fc3c6..1b69807 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -420,8 +420,11 @@ function normalizeName(value) { // solely when they modify a Latin base letter (e vs \u00e9): the same block // spells essential letters in other scripts (Cyrillic \u0438 + breve = \u0439), so // stripping them unconditionally would collapse distinct names. Hebrew - // niqqud/cantillation (U+0591-U+05C7) and Arabic harakat (U+064B-U+065F, - // U+0670) are optional vocalization regardless of position. All other + // niqqud/cantillation (U+0591-U+05C7) and the Arabic harakat proper + // (U+064B-U+0652 plus U+0670 superscript alef) are optional vocalization + // regardless of position. Maddah and hamza marks (U+0653-U+0655) are NOT + // stripped: NFKD decomposes alef/waw/yeh-hamza letters into base + one of + // these marks, so removing them would collapse distinct letters. All other // marks (e.g. Devanagari vowel signs) are preserved via \p{M} below. var decomposed = String(value).normalize('NFKD') var kept = '' @@ -432,7 +435,7 @@ function normalizeName(value) { if (!lastBaseIsLatin) kept += ch continue } - if ((code >= 0x0591 && code <= 0x05c7) || (code >= 0x064b && code <= 0x065f) || code === 0x0670) { + if ((code >= 0x0591 && code <= 0x05c7) || (code >= 0x064b && code <= 0x0652) || code === 0x0670) { continue } kept += ch @@ -991,6 +994,9 @@ function sweepUsage() { 'file records the UTC date and request count, so multiple runs on the same', 'UTC day share one daily cap. On HTTP 429 the run backs off and stops', 'cleanly. Designed for LocationIQ\'s free tier; run one sweep at a time.', + 'The cache records the endpoint and accept-language it was built with, and', + 'network runs with different values are rejected — use a separate --workdir', + 'per configuration (--dry-run only evaluates the cache and is exempt).', '', 'Options:', ' --points JSONL points file (required; see the sample subcommand)', @@ -1304,6 +1310,41 @@ function appendSweepCache(cachePath, entry) { fs.appendFileSync(cachePath, prefix + JSON.stringify(entry) + '\n', 'utf8') } +function readSweepCacheMeta(cachePath) { + if (!fs.existsSync(cachePath)) return null + + var lines = fs.readFileSync(cachePath, 'utf8').split(/\r?\n/) + for (var i = 0; i < lines.length; i++) { + if (!lines[i].trim()) continue + try { + var entry = JSON.parse(lines[i]) + if (entry && entry.meta && typeof entry.meta === 'object') return entry.meta + } catch (err) { + // Torn lines are handled by loadSweepCache; skip them here too. + } + } + return null +} + +function ensureSweepCacheConfig(cachePath, config) { + // Cached responses depend on the request-shaping options, so the cache is + // stamped with them and a run using different options is rejected loudly: + // silently evaluating stale responses (or silently refetching everything) + // would corrupt the report or re-spend quota. + var meta = readSweepCacheMeta(cachePath) + if (!meta) { + appendSweepCache(cachePath, { meta: config }) + return + } + if (meta.endpoint !== config.endpoint || meta.acceptLanguage !== config.acceptLanguage) { + throw new Error('Cache ' + cachePath + ' was built with endpoint=' + meta.endpoint + + ' accept-language=' + (meta.acceptLanguage || '(none)') + + ', but this run uses endpoint=' + config.endpoint + + ' accept-language=' + (config.acceptLanguage || '(none)') + + '. Responses are not comparable across these settings: use a separate --workdir or --cache, or delete the cache to refetch.') + } +} + function loadQuotaState(statePath, todayUtc) { var raw try { @@ -1587,6 +1628,11 @@ async function runSweep(opts, deps) { } fs.mkdirSync(path.dirname(opts.cachePath), { recursive: true }) + if (!opts.dryRun) { + // Dry runs only evaluate what is already cached, so the request-shaping + // options are irrelevant there; network runs must match the cache. + ensureSweepCacheConfig(opts.cachePath, { endpoint: opts.endpoint, acceptLanguage: opts.acceptLanguage }) + } var cache = loadSweepCache(opts.cachePath) var todayUtc = utcDateString(nowImpl()) var state @@ -1806,7 +1852,13 @@ function parseSweepArgs(argv) { } else if (arg === '--max-requests') { opts.maxRequests = Math.max(0, Math.trunc(requireNumericArg('--max-requests', argv[++i]))) } else if (arg === '--reverse-mode') { - opts.reverseMode = String(argv[++i] || 'boundary').toLowerCase() + // The mode defines which lookup algorithm the report measures, so a + // typo must not silently reconfigure the experiment. + var reverseMode = String(argv[++i] || '').toLowerCase() + if (reverseMode !== 'centroid' && reverseMode !== 'boundary') { + throw new Error('--reverse-mode must be centroid or boundary, got: ' + (argv[i] === undefined ? '(missing)' : argv[i])) + } + opts.reverseMode = reverseMode } else if (arg === '--base-precision') { opts.basePrecision = Math.max(1, Math.trunc(requireNumericArg('--base-precision', argv[++i]))) } else if (arg === '--max-precision') { @@ -1855,7 +1907,7 @@ async function sweepMain(argv) { var geocoder = createGeocoder({ database: opts.database, - reverseMode: opts.reverseMode === 'centroid' ? 'centroid' : 'boundary', + reverseMode: opts.reverseMode, boundary: { basePrecision: opts.basePrecision, maxPrecision: opts.maxPrecision diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index c31faa5..4f6be72 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -85,6 +85,18 @@ function cacheLines(dir) { return fs.readFileSync(file, 'utf8').split('\n').filter((line) => line.trim()); } +function cacheEntries(dir) { + return cacheLines(dir) + .map((line) => { + try { + return JSON.parse(line); + } catch (err) { + return null; + } + }) + .filter((entry) => entry && entry.key); +} + function point(overrides) { return Object.assign({ key: '13.4767,-89.3072', @@ -208,6 +220,18 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('São Paulo')).toEqual('sao paulo'); }); + it('keeps Arabic hamza and maddah distinctions while stripping harakat', () => { + // NFKD decomposes أ/إ/آ into alef + U+0653-0655; those marks are + // orthographically essential, so distinct names must stay distinct. + expect(liq.normalizeName('أمل')).not.toEqual(liq.normalizeName('امل')); + expect(liq.normalizeName('إربد')).not.toEqual(liq.normalizeName('اربد')); + expect(liq.normalizeName('آزاد')).not.toEqual(liq.normalizeName('ازاد')); + // Precomposed hamza letters equal their decomposed spellings. + expect(liq.normalizeName('أ')).toEqual(liq.normalizeName('أ')); + // True harakat remain optional vocalization. + expect(liq.normalizeName('مُحَمَّد')).toEqual(liq.normalizeName('محمد')); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -328,7 +352,7 @@ describe('locationiq sweep', () => { expect(first.calls[0]).toContain('accept-language=en'); expect(summary.stopReason).toEqual('daily_cap'); expect(summary.requestsThisRun).toEqual(3); - expect(cacheLines(dir).length).toEqual(3); + expect(cacheEntries(dir).length).toEqual(3); const state = readState(dir); expect(state.date).toEqual(liq.utcDateString(new Date())); @@ -409,7 +433,7 @@ describe('locationiq sweep', () => { expect(deps.calls.length).toEqual(2); expect(summary.stopReason).toEqual('rate_limited'); - expect(cacheLines(dir).length).toEqual(1); + expect(cacheEntries(dir).length).toEqual(1); expect(summary.evaluated).toEqual(1); // The failed attempt still counts against the persisted quota. expect(readState(dir).count).toEqual(2); @@ -538,6 +562,13 @@ describe('locationiq sweep', () => { } }); + it('rejects unsupported --reverse-mode values instead of silently mapping them', () => { + expect(() => liq.parseSweepArgs(['--reverse-mode', 'centriod'])).toThrowError(/--reverse-mode/); + expect(() => liq.parseSweepArgs(['--reverse-mode'])).toThrowError(/--reverse-mode/); + expect(liq.parseSweepArgs(['--reverse-mode', 'centroid']).reverseMode).toEqual('centroid'); + expect(liq.parseSweepArgs(['--reverse-mode', 'Boundary']).reverseMode).toEqual('boundary'); + }); + it('rolls quota state over when the UTC day changes mid-run', async () => { const dir = makeTmpDir(); try { @@ -587,15 +618,7 @@ describe('locationiq sweep', () => { expect(deps.calls.length).toEqual(4); expect(summary.evaluated).toEqual(5); - const parseable = cacheLines(dir).filter((line) => { - try { - JSON.parse(line); - return true; - } catch (err) { - return false; - } - }); - expect(parseable.length).toEqual(5); + expect(cacheEntries(dir).length).toEqual(5); // The repaired cache must fully satisfy a second run: zero requests. const again = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); @@ -664,6 +687,46 @@ describe('locationiq sweep', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it('rejects a network run against a cache built with different request options', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const respond = () => okResponse({ city: 'Testville', country_code: 'us' }); + + const first = makeDeps(respond); + await liq.runSweep(sweepOpts(dir), first); + expect(first.calls.length).toEqual(5); + + // A different accept-language must not silently reuse the cache. + const second = makeDeps(respond); + await expectAsync(liq.runSweep(sweepOpts(dir, { acceptLanguage: 'fr' }), second)) + .toBeRejectedWithError(/accept-language/); + expect(second.calls.length).toEqual(0); + + // Neither may a different endpoint. + const third = makeDeps(respond); + await expectAsync(liq.runSweep(sweepOpts(dir, { endpoint: 'https://eu1.liq.invalid/v1/reverse' }), third)) + .toBeRejectedWithError(/endpoint/); + expect(third.calls.length).toEqual(0); + + // Matching options keep resuming from the cache. + const fourth = makeDeps(respond); + const summary = await liq.runSweep(sweepOpts(dir), fourth); + expect(fourth.calls.length).toEqual(0); + expect(summary.evaluated).toEqual(5); + + // Dry runs only evaluate the cache as-is, so options are exempt. + const fifth = makeDeps(() => { + throw new Error('dry-run must not touch the network'); + }); + const drySummary = await liq.runSweep(sweepOpts(dir, { acceptLanguage: 'fr', dryRun: true, apiKey: '' }), fifth); + expect(fifth.calls.length).toEqual(0); + expect(drySummary.evaluated).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe('optional sqlite3 dependency', () => { From fc1a8deace2df590c1cad2f41ac73490f24334c4 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Tue, 18 Aug 2026 22:19:46 -0600 Subject: [PATCH 05/12] Address round-4 review: maqaf, token matching, shared cap, 400s, sparse answers - Restrict the stripped Hebrew ranges to actual combining marks; punctuation in the same block (maqaf, paseq, sof pasuq, nun hafukha) now reaches the separator normalization, so hyphenated and spaced spellings of the same name compare equal instead of mismatching. - namesMatch containment now respects token boundaries: a short name that is merely a substring of an unrelated word ("Ham" in "Hamme") no longer counts as agreement, while whole-token qualifier matches ("Salvador" in "San Salvador") still do. - The daily quota state default moved outside the workdir (tmp/locationiq-quota.json): per-configuration workdirs share one cap for the same API key, so following the cache-separation guidance cannot multiply the daily allowance. - HTTP 400 is treated as a configuration error: the run stops immediately without caching, instead of burning a request per point and permanently polluting the cache. 404 stays cacheable, as it is LocationIQ's coordinate-specific "unable to geocode" answer. - HTTP 200 answers with no locality/county/state name are classified as a new unverifiable liq_name_missing verdict instead of inflating name-mismatch rates; the severe country check still runs first. Five new regression specs; 64 specs green. --- README.md | 4 +- scripts/validate_with_locationiq.js | 106 ++++++++++++++++++++----- spec/locationiq_sweep_spec.js | 117 ++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 1adba99..7ecf5cb 100644 --- a/README.md +++ b/README.md @@ -341,7 +341,9 @@ node scripts/validate_with_locationiq.js sweep \ Every LocationIQ response is cached as JSONL keyed by coordinates rounded to four decimals, so re-running the same command never re-queries a cached point — if a run stops at the daily cap or on HTTP 429, just run it again later to -resume. Outputs land under `--workdir` (default `tmp/locationiq-sweep/`): +resume. The daily-cap state lives outside the workdir (default +`tmp/locationiq-quota.json`), so multiple sweep configurations share one cap +for the same API key. Outputs land under `--workdir` (default `tmp/locationiq-sweep/`): `report.md` (per-country point counts, agreement %, country mismatches, worst examples) and `mismatches.jsonl` (machine-readable list of all mismatches). See `sweep --help` and `sample --help` for all options. diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 1b69807..0f8009b 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -435,7 +435,15 @@ function normalizeName(value) { if (!lastBaseIsLatin) kept += ch continue } - if ((code >= 0x0591 && code <= 0x05c7) || (code >= 0x064b && code <= 0x0652) || code === 0x0670) { + // Hebrew: strip only the actual combining marks (cantillation, niqqud, + // rafe, shin/sin dots, upper/lower dots, qamats qatan). Punctuation in + // the same block — maqaf U+05BE, paseq U+05C0, sof pasuq U+05C3, nun + // hafukha U+05C6 — must survive to the separator normalization below, + // or hyphenated names would glue together into a different word. + var isHebrewOptionalMark = (code >= 0x0591 && code <= 0x05bd) || code === 0x05bf || + code === 0x05c1 || code === 0x05c2 || code === 0x05c4 || code === 0x05c5 || code === 0x05c7 + var isArabicHarakat = (code >= 0x064b && code <= 0x0652) || code === 0x0670 + if (isHebrewOptionalMark || isArabicHarakat) { continue } kept += ch @@ -449,11 +457,33 @@ function normalizeName(value) { .replace(/\s+/g, ' ') } +function tokensContain(container, contained) { + var containerTokens = container.split(' ') + var containedTokens = contained.split(' ') + if (!containedTokens.length || containedTokens.length > containerTokens.length) { + return false + } + for (var start = 0; start + containedTokens.length <= containerTokens.length; start++) { + var matched = true + for (var i = 0; i < containedTokens.length; i++) { + if (containerTokens[start + i] !== containedTokens[i]) { + matched = false + break + } + } + if (matched) return true + } + return false +} + function namesMatch(left, right) { if (!left || !right) return false if (left === right) return true - if (left.indexOf(right) !== -1 || right.indexOf(left) !== -1) return true - return false + // Containment must respect token boundaries: a short name that is merely + // a substring of an unrelated word ("ham" inside "hamme") is not + // agreement, while "salvador" inside "san salvador" still matches as a + // whole-token qualifier relationship. + return tokensContain(left, right) || tokensContain(right, left) } function extractLocationIqLocality(address) { @@ -948,6 +978,10 @@ async function main() { var SWEEP_CACHE_DECIMALS = 4 var SWEEP_TIMEOUT_MS = 20000 var SWEEP_DEFAULT_WORKDIR = 'tmp/locationiq-sweep' +// The quota state deliberately lives OUTSIDE the workdir: separate workdirs +// per endpoint/language configuration must still share one daily cap, +// because they all spend requests against the same API key. +var SWEEP_DEFAULT_STATE_PATH = 'tmp/locationiq-quota.json' // Fixed seed for the sampler's country-order shuffle: stable across runs, but // not alphabetical, so a small --max-points does not bias the world sample // toward alphabetically-early country codes. @@ -1001,9 +1035,11 @@ function sweepUsage() { 'Options:', ' --points JSONL points file (required; see the sample subcommand)', ' --database Offline geocoder SQLite database (required)', - ' --workdir Directory for cache/state/report files (default: ' + SWEEP_DEFAULT_WORKDIR + ')', + ' --workdir Directory for cache/report files (default: ' + SWEEP_DEFAULT_WORKDIR + ')', ' --cache LocationIQ response cache (default: /cache.jsonl)', - ' --state Daily quota state file (default: /quota.json)', + ' --state Daily quota state file (default: ' + SWEEP_DEFAULT_STATE_PATH + ').', + ' Deliberately outside the workdir: all sweep configurations', + ' share one daily cap because they spend the same API key.', ' --report Markdown report output (default: /report.md)', ' --mismatches Mismatch JSONL output (default: /mismatches.jsonl)', ' --api-key LocationIQ API key (or env LOCATIONIQ_API_KEY)', @@ -1386,12 +1422,13 @@ function saveQuotaState(statePath, state) { fs.renameSync(tmpPath, statePath) } +var LIQ_NAME_KEY_GROUPS = [LIQ_LOCALITY_KEYS, LIQ_COUNTY_KEYS, LIQ_STATE_KEYS] + function findLiqNameMatch(normalizedOfflineName, address, displayName) { if (!normalizedOfflineName) return null - var groups = [LIQ_LOCALITY_KEYS, LIQ_COUNTY_KEYS, LIQ_STATE_KEYS] - for (var g = 0; g < groups.length; g++) { - var hit = matchAddressValue(normalizedOfflineName, address, groups[g]) + for (var g = 0; g < LIQ_NAME_KEY_GROUPS.length; g++) { + var hit = matchAddressValue(normalizedOfflineName, address, LIQ_NAME_KEY_GROUPS[g]) if (hit) return hit.key } @@ -1402,6 +1439,17 @@ function findLiqNameMatch(normalizedOfflineName, address, displayName) { return null } +function liqHasComparableName(address) { + if (!address || typeof address !== 'object') return false + for (var g = 0; g < LIQ_NAME_KEY_GROUPS.length; g++) { + var keys = LIQ_NAME_KEY_GROUPS[g] + for (var i = 0; i < keys.length; i++) { + if (address[keys[i]]) return true + } + } + return false +} + function comparePoint(point, offlineResult, cacheEntry) { var body = cacheEntry && cacheEntry.body && typeof cacheEntry.body === 'object' ? cacheEntry.body : null var address = body && body.address && typeof body.address === 'object' ? body.address : null @@ -1448,6 +1496,11 @@ function comparePoint(point, offlineResult, cacheEntry) { record.verdict = 'country_unknown' } else if (offlineCountry !== record.liq_country) { record.verdict = 'country_mismatch' + } else if (!liqHasComparableName(address)) { + // Countries agree, but LocationIQ supplied no locality/county/state name + // to compare against (common for sparse rural responses): unverifiable, + // not a name mismatch. + record.verdict = 'liq_name_missing' } else { var via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) if (via) { @@ -1474,7 +1527,7 @@ function formatAnswer(name, country, fallback) { function buildSweepReport(params) { var records = params.records || [] var perCountry = Object.create(null) - var totals = { agree: 0, country_mismatch: 0, name_mismatch: 0, offline_empty: 0, liq_empty: 0, both_empty: 0, country_unknown: 0 } + var totals = { agree: 0, country_mismatch: 0, name_mismatch: 0, offline_empty: 0, liq_empty: 0, both_empty: 0, country_unknown: 0, liq_name_missing: 0 } for (var i = 0; i < records.length; i++) { var record = records[i] @@ -1489,7 +1542,8 @@ function buildSweepReport(params) { offline_empty: 0, liq_empty: 0, both_empty: 0, - country_unknown: 0 + country_unknown: 0, + liq_name_missing: 0 } } bucket.evaluated += 1 @@ -1533,8 +1587,9 @@ function buildSweepReport(params) { ' (' + agreementPct.toFixed(1) + '%)') lines.push('- Mismatches: ' + (verifiable - totals.agree) + ' (country ' + totals.country_mismatch + ', name ' + totals.name_mismatch + ', offline empty ' + totals.offline_empty + ')') - lines.push('- Unverifiable: ' + (totals.liq_empty + totals.both_empty + totals.country_unknown) + - ' (LocationIQ empty ' + totals.liq_empty + ', both empty ' + totals.both_empty + + lines.push('- Unverifiable: ' + (totals.liq_empty + totals.both_empty + totals.country_unknown + totals.liq_name_missing) + + ' (LocationIQ empty ' + totals.liq_empty + ', no name ' + totals.liq_name_missing + + ', both empty ' + totals.both_empty + ', country unknown ' + totals.country_unknown + ')') var quotaCount = params.quota && params.quota.count !== null && params.quota.count !== undefined && Number.isFinite(Number(params.quota.count)) ? Number(params.quota.count) : null @@ -1546,14 +1601,14 @@ function buildSweepReport(params) { lines.push('') lines.push('## Countries ranked by mismatch rate (worst first)') lines.push('') - lines.push('| Country | Points | Verifiable | Agreement | Country mismatch | Name mismatch | Offline empty | LIQ empty | Country unknown |') - lines.push('| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |') + lines.push('| Country | Points | Verifiable | Agreement | Country mismatch | Name mismatch | Offline empty | LIQ empty | LIQ no name | Country unknown |') + lines.push('| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |') for (var c = 0; c < countryRows.length; c++) { var row = countryRows[c] var pct = row.verifiable > 0 ? ((row.agree * 100) / row.verifiable).toFixed(1) + '%' : '-' lines.push('| ' + mdEscape(row.country) + ' | ' + row.evaluated + ' | ' + row.verifiable + ' | ' + pct + ' | ' + row.country_mismatch + ' | ' + row.name_mismatch + ' | ' + row.offline_empty + ' | ' + row.liq_empty + - ' | ' + row.country_unknown + ' |') + ' | ' + row.liq_name_missing + ' | ' + row.country_unknown + ' |') } lines.push('') lines.push('## Worst examples') @@ -1581,6 +1636,7 @@ function buildSweepReport(params) { lines.push('- `offline_empty`: LocationIQ answered but the offline geocoder returned nothing') lines.push('- `liq_empty` / `both_empty`: LocationIQ had no answer — excluded from agreement figures') lines.push('- `country_unknown`: one side answered without a country code, so the comparison cannot be verified — excluded from agreement figures (a name match, if any, is noted in `match_via`)') + lines.push('- `liq_name_missing`: countries agree but LocationIQ returned no locality/county/state name to compare against — excluded from agreement figures') lines.push('') return lines.join('\n') } @@ -1701,10 +1757,10 @@ async function runSweep(opts, deps) { } var status = Number(response && response.status) - if (status === 200 || status === 400 || status === 404) { - // 200 is an answer; 404 is LocationIQ's definitive "unable to geocode" - // (e.g. open ocean) and 400 a definitive rejection of the coordinates — - // all cacheable so they are never asked again. + if (status === 200 || status === 404) { + // 200 is an answer and 404 is LocationIQ's definitive, coordinate- + // specific "unable to geocode" (e.g. open ocean) — both cacheable so + // they are never asked again. var entry = { key: point.key, lat: point.lat, @@ -1715,6 +1771,14 @@ async function runSweep(opts, deps) { } appendSweepCache(opts.cachePath, entry) cache[point.key] = entry + } else if (status === 400) { + // A rejected request shape is a configuration problem, not a fact + // about the coordinates (the points loader already validates ranges). + // Caching it or continuing would burn the daily allowance on a + // systematically broken request: stop, and cache nothing. + stopReason = 'bad_request' + stopDetail = 'HTTP 400' + break } else if (status === 401 || status === 403) { stopReason = 'auth_error' stopDetail = 'HTTP ' + status @@ -1773,6 +1837,8 @@ async function runSweep(opts, deps) { unfetched + ' points still unfetched. Re-run the same command after the next UTC day starts to resume; cached points are never re-queried.') } else if (stopReason === 'rate_limited') { log('LocationIQ returned HTTP 429 (rate limited). Backing off and stopping this run cleanly; all fetched responses are cached. Wait for the quota window to reset, then re-run the same command to resume.') + } else if (stopReason === 'bad_request') { + log('LocationIQ rejected the request shape (HTTP 400). This is a configuration problem (endpoint or parameters), so the response was not cached and the run stopped before spending more quota. Fix the configuration, then re-run to resume.') } else if (stopReason === 'auth_error') { log('LocationIQ rejected the API key (' + stopDetail + '). Check LOCATIONIQ_API_KEY / --api-key, then re-run to resume.') } else if (stopReason === 'fetch_error' || stopReason === 'server_error') { @@ -1873,7 +1939,7 @@ function parseSweepArgs(argv) { } if (!opts.cachePath) opts.cachePath = path.join(opts.workdir, 'cache.jsonl') - if (!opts.statePath) opts.statePath = path.join(opts.workdir, 'quota.json') + if (!opts.statePath) opts.statePath = path.resolve(SWEEP_DEFAULT_STATE_PATH) if (!opts.reportPath) opts.reportPath = path.join(opts.workdir, 'report.md') if (!opts.mismatchesPath) opts.mismatchesPath = path.join(opts.workdir, 'mismatches.jsonl') diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 4f6be72..7157d34 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -232,6 +232,63 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('مُحَمَّد')).toEqual(liq.normalizeName('محمد')); }); + it('treats Hebrew maqaf as a separator instead of stripping it', () => { + // U+05BE MAQAF is punctuation, not a mark: stripping it glues the words + // together, so hyphenated and spaced spellings would falsely mismatch. + expect(liq.normalizeName('בית־שמש')).toEqual(liq.normalizeName('בית שמש')); + expect(liq.normalizeName('בית־שמש')).toEqual('בית שמש'); + // Niqqud and cantillation are still stripped. + expect(liq.normalizeName('יְרוּשָׁלַיִם')).toEqual(liq.normalizeName('ירושלים')); + }); + + it('requires token boundaries for partial name matches', () => { + expect(liq.namesMatch('ham', 'hamme')).toBeFalse(); + expect(liq.namesMatch('salvador', 'san salvador')).toBeTrue(); + expect(liq.namesMatch('new york', 'york')).toBeTrue(); + + // Offline "Ham" against LocationIQ "Hamme" is a real disagreement, not + // agreement via substring. + const record = liq.comparePoint( + point({ name: 'Ham' }), + { name: 'Ham', country: { id: 'BE' } }, + { status: 200, body: { display_name: 'Hamme, Belgium', address: { city: 'Hamme', country_code: 'be' } } } + ); + expect(record.verdict).toEqual('name_mismatch'); + }); + + it('treats a country-only LocationIQ answer as unverifiable, not a name mismatch', () => { + const record = liq.comparePoint( + point({ country: 'FR' }), + { name: 'Petite Ville', country: { id: 'FR' } }, + { status: 200, body: { display_name: 'France', address: { country_code: 'fr' } } } + ); + expect(record.verdict).toEqual('liq_name_missing'); + + // The severe country check still runs before the name-field check. + const mismatch = liq.comparePoint( + point({ country: 'FR' }), + { name: 'Petite Ville', country: { id: 'DE' } }, + { status: 200, body: { display_name: 'France', address: { country_code: 'fr' } } } + ); + expect(mismatch.verdict).toEqual('country_mismatch'); + + // Excluded from the verifiable denominator in the report. + const report = liq.buildSweepReport({ + generatedAt: 'now', + databaseLabel: 'db', + pointsPath: 'points.jsonl', + totalPoints: 1, + unfetched: 0, + records: [record], + quota: { date: '2026-08-19', count: 0 }, + dailyCap: 4500, + stopReason: null, + stopDetail: '' + }); + expect(report).toContain('- Verifiable points: 0 — agreement 0/0 (0.0%)'); + expect(report).toContain('no name 1'); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -727,6 +784,66 @@ describe('locationiq sweep', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it('shares one daily cap across configuration workdirs', async () => { + // The default state path is independent of --workdir, so per-config + // workdirs cannot each start a fresh quota. + const parsed = liq.parseSweepArgs(['--workdir', path.join(os.tmpdir(), 'liq-alt')]); + expect(parsed.statePath).toEqual(path.resolve('tmp/locationiq-quota.json')); + expect(parsed.cachePath).toEqual(path.join(path.resolve(os.tmpdir(), 'liq-alt'), 'cache.jsonl')); + expect(liq.parseSweepArgs(['--state', path.join(os.tmpdir(), 'q.json')]).statePath) + .toEqual(path.resolve(os.tmpdir(), 'q.json')); + + // Two workdirs (separate caches/configs) sharing one state file must + // also share one daily cap. + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const perConfig = (name, extra) => sweepOpts(dir, Object.assign({ + cachePath: path.join(dir, name, 'cache.jsonl'), + reportPath: path.join(dir, name, 'report.md'), + mismatchesPath: path.join(dir, name, 'mismatches.jsonl'), + statePath: path.join(dir, 'quota.json'), + dailyCap: 3 + }, extra)); + + const first = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const s1 = await liq.runSweep(perConfig('en', {}), first); + expect(first.calls.length).toEqual(3); + expect(s1.stopReason).toEqual('daily_cap'); + + const second = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const s2 = await liq.runSweep(perConfig('fr', { acceptLanguage: 'fr' }), second); + expect(second.calls.length).toEqual(0); + expect(s2.stopReason).toEqual('daily_cap'); + expect(readState(dir).count).toEqual(3); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('stops without caching when the endpoint rejects the request shape with HTTP 400', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps((url, callNumber) => { + if (callNumber === 1) return okResponse({ city: 'Testville', country_code: 'us' }); + return { status: 400, json: { error: 'Invalid request' } }; + }); + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + // A systemic 400 must stop immediately, not burn a request per point. + expect(deps.calls.length).toEqual(2); + expect(summary.stopReason).toEqual('bad_request'); + // The rejection is not cached, so a fixed configuration can retry it. + expect(cacheEntries(dir).length).toEqual(1); + // The attempt still counts against the persisted quota. + expect(readState(dir).count).toEqual(2); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe('optional sqlite3 dependency', () => { From 037ce23164b1f566d51b377972d5068a5960b765 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 09:44:27 -0600 Subject: [PATCH 06/12] Address round-5 review: display-name fallback, flag swallowing, Latin script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Attempt the name match before classifying a response as liq_name_missing, so a display-name-only match (accepted as agreement everywhere else) counts as agreement even when the address block carries no name fields. - Value-taking string options now reject a following option token as their value: `--accept-language --dry-run` previously swallowed the dry-run flag and could turn a cache-only command into a real network run. The documented empty accept-language value still works. - Latin-base detection for diacritic stripping now uses the Unicode script property instead of [A-Za-z], so accented forms of non-ASCII Latin letters (NFKD turns ǿ into ø + U+0301) fold correctly. Three new regression specs; 67 specs green. --- scripts/validate_with_locationiq.js | 55 +++++++++++++++++++---------- spec/locationiq_sweep_spec.js | 32 +++++++++++++++++ 2 files changed, 69 insertions(+), 18 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 0f8009b..90b056b 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -62,6 +62,16 @@ function requireNumericArg(flag, raw) { return value } +function requireValueArg(flag, raw) { + // A following option token means the value was accidentally omitted; + // consuming it would silently change behavior (e.g. --accept-language + // swallowing --dry-run turns a cache-only command into a network run). + if (raw === undefined || (typeof raw === 'string' && raw.slice(0, 2) === '--')) { + throw new Error(flag + ' requires a value, got: ' + (raw === undefined ? '(missing)' : raw)) + } + return String(raw) +} + function parseArgs(argv) { var opts = { database: null, @@ -412,6 +422,8 @@ async function ensureSamplePoints(sourceDb, cacheDb, lookupTable, targetCount, s } } +var LATIN_BASE_RE = /\p{Script=Latin}/u + function normalizeName(value) { if (!value) return '' @@ -447,7 +459,10 @@ function normalizeName(value) { continue } kept += ch - lastBaseIsLatin = (code >= 0x41 && code <= 0x5a) || (code >= 0x61 && code <= 0x7a) + // Script-based, not ASCII-based: letters like ø or đ are Latin bases + // that never decompose themselves, yet their accented forms do (ǿ is + // ø + U+0301 under NFKD) and must fold the same way as e/é. + lastBaseIsLatin = LATIN_BASE_RE.test(ch) } return kept @@ -1193,9 +1208,9 @@ function parseSampleArgs(argv) { var arg = argv[i] if (arg === '--geonames') { - opts.geonames = path.resolve(argv[++i]) + opts.geonames = path.resolve(requireValueArg('--geonames', argv[++i])) } else if (arg === '--out') { - opts.out = path.resolve(argv[++i]) + opts.out = path.resolve(requireValueArg('--out', argv[++i])) } else if (arg === '--per-country') { opts.perCountry = Math.max(1, Math.trunc(requireNumericArg('--per-country', argv[++i]))) } else if (arg === '--max-points') { @@ -1496,16 +1511,18 @@ function comparePoint(point, offlineResult, cacheEntry) { record.verdict = 'country_unknown' } else if (offlineCountry !== record.liq_country) { record.verdict = 'country_mismatch' - } else if (!liqHasComparableName(address)) { - // Countries agree, but LocationIQ supplied no locality/county/state name - // to compare against (common for sparse rural responses): unverifiable, - // not a name mismatch. - record.verdict = 'liq_name_missing' } else { + // Attempt the match first: display_name segments count as agreement even + // when the address block carries no name fields at all. var via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) if (via) { record.verdict = 'agree' record.match_via = via + } else if (!liqHasComparableName(address)) { + // Countries agree, but LocationIQ supplied no locality/county/state + // name to compare against (common for sparse rural responses): + // unverifiable, not a name mismatch. + record.verdict = 'liq_name_missing' } else { record.verdict = 'name_mismatch' } @@ -1890,25 +1907,27 @@ function parseSweepArgs(argv) { var arg = argv[i] if (arg === '--points') { - opts.pointsPath = path.resolve(argv[++i]) + opts.pointsPath = path.resolve(requireValueArg('--points', argv[++i])) } else if (arg === '--database' || arg === '-d') { - opts.database = path.resolve(argv[++i]) + opts.database = path.resolve(requireValueArg('--database', argv[++i])) } else if (arg === '--workdir') { - opts.workdir = path.resolve(argv[++i]) + opts.workdir = path.resolve(requireValueArg('--workdir', argv[++i])) } else if (arg === '--cache') { - opts.cachePath = path.resolve(argv[++i]) + opts.cachePath = path.resolve(requireValueArg('--cache', argv[++i])) } else if (arg === '--state') { - opts.statePath = path.resolve(argv[++i]) + opts.statePath = path.resolve(requireValueArg('--state', argv[++i])) } else if (arg === '--report') { - opts.reportPath = path.resolve(argv[++i]) + opts.reportPath = path.resolve(requireValueArg('--report', argv[++i])) } else if (arg === '--mismatches') { - opts.mismatchesPath = path.resolve(argv[++i]) + opts.mismatchesPath = path.resolve(requireValueArg('--mismatches', argv[++i])) } else if (arg === '--api-key') { - opts.apiKey = String(argv[++i] || '') + opts.apiKey = requireValueArg('--api-key', argv[++i]) } else if (arg === '--endpoint') { - opts.endpoint = String(argv[++i] || opts.endpoint) + opts.endpoint = requireValueArg('--endpoint', argv[++i]) } else if (arg === '--accept-language') { - opts.acceptLanguage = String(argv[++i] || '') + // '' is a documented value (disables the header), but a following + // option token means the value was omitted. + opts.acceptLanguage = requireValueArg('--accept-language', argv[++i]) } else if (arg === '--daily-cap') { // Reject non-numeric values outright: a NaN here would silently // disable the daily quota instead of enforcing it. diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 7157d34..5a15d4c 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -289,6 +289,27 @@ describe('locationiq sweep', () => { expect(report).toContain('no name 1'); }); + it('accepts a display-name-only match before falling back to liq_name_missing', () => { + // The address block has no name fields, but the display_name segment + // matches: that is agreement, not an unverifiable answer. + const record = liq.comparePoint( + point({ country: 'US' }), + { name: 'Testville', country: { id: 'US' } }, + { status: 200, body: { display_name: 'Testville, United States', address: { country_code: 'us' } } } + ); + expect(record.verdict).toEqual('agree'); + expect(record.match_via).toEqual('display_name'); + }); + + it('folds diacritics on non-ASCII Latin bases', () => { + // NFKD decomposes ǿ into ø + U+0301; ø is Latin but not ASCII, so the + // base test must use the Unicode script property, not [a-z]. + expect(liq.normalizeName('ǿ')).toEqual(liq.normalizeName('ø')); + expect(liq.normalizeName('Ǿrsted')).toEqual(liq.normalizeName('Ørsted')); + // Non-Latin bases still keep their marks. + expect(liq.normalizeName('й')).not.toEqual(liq.normalizeName('и')); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -619,6 +640,17 @@ describe('locationiq sweep', () => { } }); + it('rejects option flags consumed as values for value-taking options', () => { + // Swallowing the next flag can silently flip network behavior: + // `--accept-language --dry-run` must not eat the dry-run flag. + expect(() => liq.parseSweepArgs(['--accept-language', '--dry-run'])).toThrowError(/--accept-language/); + expect(() => liq.parseSweepArgs(['--endpoint'])).toThrowError(/--endpoint/); + expect(() => liq.parseSweepArgs(['--api-key', '--points', 'x.jsonl'])).toThrowError(/--api-key/); + expect(() => liq.parseSampleArgs(['--geonames', '--out'])).toThrowError(/--geonames/); + // The documented empty value still works. + expect(liq.parseSweepArgs(['--accept-language', '']).acceptLanguage).toEqual(''); + }); + it('rejects unsupported --reverse-mode values instead of silently mapping them', () => { expect(() => liq.parseSweepArgs(['--reverse-mode', 'centriod'])).toThrowError(/--reverse-mode/); expect(() => liq.parseSweepArgs(['--reverse-mode'])).toThrowError(/--reverse-mode/); From 3e2351e531b2ce04458cf47484ff963dff4d4c2d Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 09:57:23 -0600 Subject: [PATCH 07/12] Address round-6 review: type-strict state, order-free precision, input coercion - Quota state validation requires count to actually be a number: Number(null)/Number(true)/Number('') are all finite, so a damaged {"count": null} file would otherwise reset the day's tally and re-open the cap. Structurally damaged state now fails closed. - Precision bounds are validated after the whole argument list is parsed: --max-precision 5 --base-precision 6 now errors in either order instead of silently producing an inconsistent pair that src/index would replace with its own default. A bare --base-precision above the default raises the default maximum. - GeoNames rows with blank lat/lon columns are rejected instead of coercing to (0, 0) and displacing valid top-N places. - Points-file rows that are valid JSON but not objects (null, numbers) are counted as skipped instead of crashing the sweep; null/boolean/blank coordinate values are rejected the same way. Four new regression specs; 71 specs green. --- scripts/validate_with_locationiq.js | 49 +++++++++++++++++++---- spec/locationiq_sweep_spec.js | 62 +++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 90b056b..7cc84d7 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -1094,8 +1094,13 @@ function parseGeonamesLine(line) { var cols = line.split('\t') if (cols.length < 15) return null - var latitude = Number(cols[4]) - var longitude = Number(cols[5]) + // Blank columns must not coerce to coordinate (0, 0): Number('') is 0, + // which would pass the range checks below. + var latRaw = String(cols[4] || '').trim() + var lonRaw = String(cols[5] || '').trim() + if (!latRaw || !lonRaw) return null + var latitude = Number(latRaw) + var longitude = Number(lonRaw) var featureClass = String(cols[6] || '').trim() var country = String(cols[8] || '').trim().toUpperCase() var population = Number(cols[14]) @@ -1274,6 +1279,15 @@ async function sampleMain(argv) { // --- sweep: cache, quota state, comparison, report ------------------------- +function coerceCoord(value) { + // Number(null), Number(true) and Number('') are all finite (0/1/0), so a + // missing or blank coordinate would otherwise pass the range checks as a + // real point at (0, 0). + if (value === null || value === undefined || typeof value === 'boolean') return NaN + if (typeof value === 'string' && !value.trim()) return NaN + return Number(value) +} + function loadPointsFile(pointsPath) { var lines = fs.readFileSync(pointsPath, 'utf8').split(/\r?\n/) var points = [] @@ -1292,8 +1306,15 @@ function loadPointsFile(pointsPath) { continue } - var lat = Number(row.lat !== undefined ? row.lat : row.latitude) - var lon = Number(row.lon !== undefined ? row.lon : row.longitude) + // Valid JSON is not necessarily a point: `null` (or any non-object) + // must count as a skipped row, not crash the whole sweep. + if (!row || typeof row !== 'object') { + skipped += 1 + continue + } + + var lat = coerceCoord(row.lat !== undefined ? row.lat : row.latitude) + var lon = coerceCoord(row.lon !== undefined ? row.lon : row.longitude) if (!Number.isFinite(lat) || lat < -90 || lat > 90 || !Number.isFinite(lon) || lon < -180 || lon > 180) { skipped += 1 continue @@ -1417,7 +1438,10 @@ function loadQuotaState(statePath, todayUtc) { // Fail closed: an existing-but-unreadable state file must not silently // reset today's count to zero, or a corrupted file would allow a fresh // full daily cap of requests. - if (!parsed || typeof parsed.date !== 'string' || !Number.isFinite(Number(parsed.count))) { + // The count must BE a number, not merely coerce to one: Number(null), + // Number(true) and Number('') are all finite, so a structurally damaged + // state file would otherwise reset the day's count and re-open the cap. + if (!parsed || typeof parsed.date !== 'string' || typeof parsed.count !== 'number' || !Number.isFinite(parsed.count)) { throw new Error('Quota state file ' + statePath + ' exists but is unreadable; refusing to guess the request count. ' + 'Inspect it and, only if you are sure no requests were made today (UTC), delete it to reset.') } @@ -1425,7 +1449,7 @@ function loadQuotaState(statePath, todayUtc) { if (parsed.date !== todayUtc) { return { date: todayUtc, count: 0 } } - return { date: todayUtc, count: Math.max(0, Math.trunc(Number(parsed.count))) } + return { date: todayUtc, count: Math.max(0, Math.trunc(parsed.count)) } } function saveQuotaState(statePath, state) { @@ -1902,6 +1926,7 @@ function parseSweepArgs(argv) { dryRun: false, help: false } + var maxPrecisionSet = false for (var i = 0; i < argv.length; i++) { var arg = argv[i] @@ -1947,7 +1972,8 @@ function parseSweepArgs(argv) { } else if (arg === '--base-precision') { opts.basePrecision = Math.max(1, Math.trunc(requireNumericArg('--base-precision', argv[++i]))) } else if (arg === '--max-precision') { - opts.maxPrecision = Math.max(opts.basePrecision, Math.trunc(requireNumericArg('--max-precision', argv[++i]))) + opts.maxPrecision = Math.max(1, Math.trunc(requireNumericArg('--max-precision', argv[++i]))) + maxPrecisionSet = true } else if (arg === '--dry-run') { opts.dryRun = true } else if (arg === '--help' || arg === '-h') { @@ -1957,6 +1983,15 @@ function parseSweepArgs(argv) { } } + // Cross-field constraints are checked only after the whole argument list + // is parsed, so option order cannot change the experiment configuration. + if (!maxPrecisionSet && opts.maxPrecision < opts.basePrecision) { + opts.maxPrecision = opts.basePrecision + } + if (opts.maxPrecision < opts.basePrecision) { + throw new Error('--max-precision (' + opts.maxPrecision + ') must be >= --base-precision (' + opts.basePrecision + ')') + } + if (!opts.cachePath) opts.cachePath = path.join(opts.workdir, 'cache.jsonl') if (!opts.statePath) opts.statePath = path.resolve(SWEEP_DEFAULT_STATE_PATH) if (!opts.reportPath) opts.reportPath = path.join(opts.workdir, 'report.md') diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 5a15d4c..301100b 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -186,6 +186,39 @@ describe('locationiq sweep', () => { expect(result.skipped).toEqual(3); expect(result.points.map((p) => p.name)).toEqual(['Cityville']); }); + + it('rejects GeoNames rows with blank coordinate columns', () => { + // Number('') is 0, so a blank column would otherwise pass the range + // checks as coordinate (0, 0) and displace a valid place from the top-N. + const blankLat = geonamesRow(1, 'Ghost Town', '', -100.0, 'P', 'US', 999999); + const valid = geonamesRow(2, 'Realville', 40.0, -100.0, 'P', 'US', 100); + + const result = liq.buildSamplePoints([blankLat, valid].join('\n'), 1, null); + + expect(result.parsed).toEqual(1); + expect(result.skipped).toEqual(1); + expect(result.points.map((p) => p.name)).toEqual(['Realville']); + }); + + it('skips valid-JSON non-object rows in the points file instead of aborting', () => { + const dir = makeTmpDir(); + try { + const file = path.join(dir, 'points.jsonl'); + fs.writeFileSync(file, [ + JSON.stringify({ lat: 1, lon: 2, country: 'US' }), + 'null', + '5', + JSON.stringify({ lat: null, lon: null, country: 'US' }) + ].join('\n') + '\n'); + + const loaded = liq.loadPointsFile(file); + + expect(loaded.points.length).toEqual(1); + expect(loaded.skipped).toEqual(3); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe('name comparison', () => { @@ -622,6 +655,35 @@ describe('locationiq sweep', () => { } }); + it('fails closed when the quota count is present but not a number', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + // Number(null), Number(true) and Number('') are all finite, so a + // coercion-based check would load these as a fresh count of 0/1. + for (const badCount of [null, true, '']) { + fs.writeFileSync( + path.join(dir, 'quota.json'), + JSON.stringify({ date: liq.utcDateString(new Date()), count: badCount }) + ); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/quota state/i); + } + expect(deps.calls.length).toEqual(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('validates precision bounds after all arguments are parsed', () => { + // Option order must not change the experiment configuration. + expect(() => liq.parseSweepArgs(['--max-precision', '5', '--base-precision', '6'])).toThrowError(/--max-precision/); + expect(() => liq.parseSweepArgs(['--base-precision', '6', '--max-precision', '5'])).toThrowError(/--max-precision/); + // The default maximum rises with an explicit base; explicit pairs hold. + expect(liq.parseSweepArgs(['--base-precision', '8']).maxPrecision).toEqual(8); + expect(liq.parseSweepArgs(['--base-precision', '5', '--max-precision', '5']).maxPrecision).toEqual(5); + }); + it('rejects invalid quota options instead of running uncapped', async () => { expect(() => liq.parseSweepArgs(['--daily-cap', 'lots'])).toThrowError(/--daily-cap/); expect(() => liq.parseSweepArgs(['--daily-cap'])).toThrowError(/--daily-cap/); From 959a054ec7338fbaea7c66437db1f47098b0e99e Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 15:14:29 -0600 Subject: [PATCH 08/12] Address round-7 review: quota integrity, route-level 404s, cross-run pacing - Reject negative and fractional persisted quota counts as damaged state instead of clamping them to zero, which would have handed out another full daily cap. - Distinguish route-level 404s from coordinate-level ones: only a body carrying LocationIQ's per-coordinate "unable to geocode" error shape is cached as a genuine miss. A 404 for the route itself (mistyped --endpoint, proxy, API change) now stops the run without caching, like the 400 case, rather than consuming the whole daily cap and permanently marking every point liq_empty. - Reject documented short flags (-h, -d, -i) as option values too, so `--accept-language -h` errors instead of starting a network sweep. - Preflight the offline database with a representative reverse lookup before the first request, so a wrong-schema --database fails for free instead of after the daily allowance is spent. - Persist the last-attempt timestamp with the quota state and seed the pacing clock from it, so a sweep resumed in a new process still honors --rps for its first request. The count still resets per UTC day; the pacing timestamp does not. Five new regression specs; 76 specs green. --- scripts/validate_with_locationiq.js | 90 +++++++++++++++++----- spec/locationiq_sweep_spec.js | 114 +++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 19 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 7cc84d7..5a43456 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -62,11 +62,16 @@ function requireNumericArg(flag, raw) { return value } +var SHORT_OPTION_TOKENS = { '-h': true, '-d': true, '-i': true } + function requireValueArg(flag, raw) { // A following option token means the value was accidentally omitted; // consuming it would silently change behavior (e.g. --accept-language - // swallowing --dry-run turns a cache-only command into a network run). - if (raw === undefined || (typeof raw === 'string' && raw.slice(0, 2) === '--')) { + // swallowing --dry-run turns a cache-only command into a network run, and + // swallowing -h starts a sweep instead of printing help). + var isOptionToken = typeof raw === 'string' && + (raw.slice(0, 2) === '--' || SHORT_OPTION_TOKENS[raw] === true) + if (raw === undefined || isOptionToken) { throw new Error(flag + ' requires a value, got: ' + (raw === undefined ? '(missing)' : raw)) } return String(raw) @@ -1279,6 +1284,15 @@ async function sampleMain(argv) { // --- sweep: cache, quota state, comparison, report ------------------------- +function isCoordinateLevelNotFound(body) { + // LocationIQ answers an un-geocodable coordinate with a JSON error body + // ({"error": "Unable to geocode"}). A 404 without that shape means the + // route itself is missing, which is a configuration problem. + if (!body || typeof body !== 'object') return false + if (typeof body.error !== 'string') return false + return /unable to geocode|no results? found/i.test(body.error) +} + function coerceCoord(value) { // Number(null), Number(true) and Number('') are all finite (0/1/0), so a // missing or blank coordinate would otherwise pass the range checks as a @@ -1437,19 +1451,26 @@ function loadQuotaState(statePath, todayUtc) { // Fail closed: an existing-but-unreadable state file must not silently // reset today's count to zero, or a corrupted file would allow a fresh - // full daily cap of requests. - // The count must BE a number, not merely coerce to one: Number(null), - // Number(true) and Number('') are all finite, so a structurally damaged - // state file would otherwise reset the day's count and re-open the cap. - if (!parsed || typeof parsed.date !== 'string' || typeof parsed.count !== 'number' || !Number.isFinite(parsed.count)) { - throw new Error('Quota state file ' + statePath + ' exists but is unreadable; refusing to guess the request count. ' + + // full daily cap of requests. The count must BE a nonnegative integer, + // not merely coerce to one: Number(null)/Number(true)/Number('') are all + // finite, and clamping a negative count would equally re-open the cap. + var countIsValid = Boolean(parsed) && typeof parsed.count === 'number' && + Number.isInteger(parsed.count) && parsed.count >= 0 + if (!parsed || typeof parsed.date !== 'string' || !countIsValid) { + throw new Error('Quota state file ' + statePath + ' exists but is damaged; refusing to guess the request count. ' + 'Inspect it and, only if you are sure no requests were made today (UTC), delete it to reset.') } + // The last-attempt time is advisory (it only delays the next request), so + // an absent or unusable value degrades to "no wait" rather than failing. + var lastRequestAt = typeof parsed.lastRequestAt === 'number' && Number.isFinite(parsed.lastRequestAt) && parsed.lastRequestAt > 0 + ? parsed.lastRequestAt + : 0 + if (parsed.date !== todayUtc) { - return { date: todayUtc, count: 0 } + return { date: todayUtc, count: 0, lastRequestAt: lastRequestAt } } - return { date: todayUtc, count: Math.max(0, Math.trunc(parsed.count)) } + return { date: todayUtc, count: parsed.count, lastRequestAt: lastRequestAt } } function saveQuotaState(statePath, state) { @@ -1457,7 +1478,9 @@ function saveQuotaState(statePath, state) { // state file behind (which loadQuotaState would refuse to read). fs.mkdirSync(path.dirname(statePath), { recursive: true }) var tmpPath = statePath + '.tmp' - fs.writeFileSync(tmpPath, JSON.stringify({ date: state.date, count: state.count }) + '\n', 'utf8') + var payload = { date: state.date, count: state.count } + if (state.lastRequestAt) payload.lastRequestAt = state.lastRequestAt + fs.writeFileSync(tmpPath, JSON.stringify(payload) + '\n', 'utf8') fs.renameSync(tmpPath, statePath) } @@ -1724,6 +1747,19 @@ async function runSweep(opts, deps) { log('Points file: skipped ' + loaded.skipped + ' invalid and ' + loaded.duplicates + ' duplicate rows') } + if (!opts.dryRun) { + // Preflight the offline database before spending a single request: a + // wrong-schema or unreadable database would otherwise only surface at + // report time, after the network loop had consumed the daily cap. + try { + await deps.reverse(points[0].lat, points[0].lon) + } catch (err) { + throw new Error('Offline geocoder lookup failed before any request was made (' + + String(err && err.message ? err.message : err) + + '). Check --database points at a geocoder database built for --reverse-mode ' + (opts.reverseMode || 'boundary') + '.') + } + } + fs.mkdirSync(path.dirname(opts.cachePath), { recursive: true }) if (!opts.dryRun) { // Dry runs only evaluate what is already cached, so the request-shaping @@ -1749,7 +1785,9 @@ async function runSweep(opts, deps) { var stopDetail = '' var requestsThisRun = 0 var delayMs = Math.ceil(1000 / (opts.rps > 0 ? opts.rps : 1)) - var lastRequestAt = 0 + // Seeded from the persisted state so a sweep resumed immediately in a new + // process still honors the configured rate limit for its first request. + var lastRequestAt = state.lastRequestAt || 0 if (!opts.dryRun) { for (var i = 0; i < points.length; i++) { @@ -1771,7 +1809,8 @@ async function runSweep(opts, deps) { // nearly twice the cap within the new UTC day. var attemptDate = utcDateString(nowImpl()) if (attemptDate !== state.date) { - state = { date: attemptDate, count: 0 } + // The count resets per UTC day; the pacing timestamp does not. + state = { date: attemptDate, count: 0, lastRequestAt: state.lastRequestAt || 0 } saveQuotaState(opts.statePath, state) } @@ -1781,11 +1820,13 @@ async function runSweep(opts, deps) { } // Count the attempt before it happens so a crash mid-request can only - // over-count, never let a later run exceed the cap. + // over-count, never let a later run exceed the cap. The attempt time + // is persisted with it so the next process paces from this request. + lastRequestAt = Date.now() state.count += 1 + state.lastRequestAt = lastRequestAt saveQuotaState(opts.statePath, state) requestsThisRun += 1 - lastRequestAt = Date.now() var url = buildLocationIqUrl(opts.endpoint, opts.apiKey, point.lat, point.lon, opts.acceptLanguage) var response @@ -1798,10 +1839,21 @@ async function runSweep(opts, deps) { } var status = Number(response && response.status) + if (status === 404 && !isCoordinateLevelNotFound(response.json)) { + // A 404 that does not carry LocationIQ's per-coordinate "unable to + // geocode" shape is the ROUTE being missing (mistyped --endpoint, a + // proxy, an API change), not a fact about this point. Treat it like + // the 400 case: stop immediately and cache nothing, instead of + // spending the whole daily cap and poisoning the cache. + stopReason = 'bad_endpoint' + stopDetail = 'HTTP 404 without a coordinate-level error body' + break + } + if (status === 200 || status === 404) { - // 200 is an answer and 404 is LocationIQ's definitive, coordinate- - // specific "unable to geocode" (e.g. open ocean) — both cacheable so - // they are never asked again. + // 200 is an answer and a coordinate-level 404 is LocationIQ's + // definitive "unable to geocode" (e.g. open ocean) — both cacheable + // so they are never asked again. var entry = { key: point.key, lat: point.lat, @@ -1878,6 +1930,8 @@ async function runSweep(opts, deps) { unfetched + ' points still unfetched. Re-run the same command after the next UTC day starts to resume; cached points are never re-queried.') } else if (stopReason === 'rate_limited') { log('LocationIQ returned HTTP 429 (rate limited). Backing off and stopping this run cleanly; all fetched responses are cached. Wait for the quota window to reset, then re-run the same command to resume.') + } else if (stopReason === 'bad_endpoint') { + log('LocationIQ returned HTTP 404 for the route itself, not for the coordinate (' + stopDetail + '). This usually means --endpoint is wrong or a proxy intercepted the request, so the response was not cached and the run stopped before spending more quota. Fix the endpoint, then re-run to resume.') } else if (stopReason === 'bad_request') { log('LocationIQ rejected the request shape (HTTP 400). This is a configuration problem (endpoint or parameters), so the response was not cached and the run stopped before spending more quota. Fix the configuration, then re-run to resume.') } else if (stopReason === 'auth_error') { diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 301100b..4f9e509 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -675,6 +675,112 @@ describe('locationiq sweep', () => { } }); + it('fails closed on a negative or fractional persisted quota count', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + // Clamping these to zero would hand out another full daily cap. + for (const badCount of [-1, -4500, 2.5]) { + fs.writeFileSync( + path.join(dir, 'quota.json'), + JSON.stringify({ date: liq.utcDateString(new Date()), count: badCount }) + ); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/quota state/i); + } + expect(deps.calls.length).toEqual(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('stops without caching when a 404 rejects the route rather than the coordinate', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps((url, callNumber) => { + if (callNumber === 1) return okResponse({ city: 'Testville', country_code: 'us' }); + // A route-level 404 (mistyped endpoint, proxy) carries no + // coordinate-level error shape. + return { status: 404, json: { message: 'Not Found' } }; + }); + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(2); + expect(summary.stopReason).toEqual('bad_endpoint'); + expect(cacheEntries(dir).length).toEqual(1); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('still caches a genuine coordinate-level 404 as an unverifiable answer', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => ({ status: 404, json: { error: 'Unable to geocode' } })); + + const summary = await liq.runSweep(sweepOpts(dir), deps); + + expect(deps.calls.length).toEqual(5); + expect(summary.stopReason).toBeNull(); + expect(cacheEntries(dir).length).toEqual(5); + expect(summary.verdictCounts.liq_empty).toEqual(5); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('preflights the offline database before spending any request', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps( + () => okResponse({ city: 'Testville', country_code: 'us' }), + () => { + throw new Error('SQLITE_ERROR: no such table: compact_places'); + } + ); + + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/no such table/); + // A wrong-schema database must not cost a single request. + expect(deps.calls.length).toEqual(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('honors the rate limit across resumed invocations', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const opts = () => sweepOpts(dir, { rps: 1, maxRequests: 1 }); + + const first = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const waits = []; + first.sleep = async (ms) => { waits.push(ms); }; + await liq.runSweep(opts(), first); + expect(first.calls.length).toEqual(1); + const persisted = readState(dir); + expect(persisted.lastRequestAt).toBeGreaterThan(0); + + // A new process resumes immediately: its first request must still be + // paced against the previous invocation's last request. + const second = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const secondWaits = []; + second.sleep = async (ms) => { secondWaits.push(ms); }; + await liq.runSweep(opts(), second); + + expect(second.calls.length).toEqual(1); + expect(secondWaits.length).toEqual(1); + expect(secondWaits[0]).toBeGreaterThan(0); + expect(secondWaits[0]).toBeLessThanOrEqual(1000); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it('validates precision bounds after all arguments are parsed', () => { // Option order must not change the experiment configuration. expect(() => liq.parseSweepArgs(['--max-precision', '5', '--base-precision', '6'])).toThrowError(/--max-precision/); @@ -709,8 +815,14 @@ describe('locationiq sweep', () => { expect(() => liq.parseSweepArgs(['--endpoint'])).toThrowError(/--endpoint/); expect(() => liq.parseSweepArgs(['--api-key', '--points', 'x.jsonl'])).toThrowError(/--api-key/); expect(() => liq.parseSampleArgs(['--geonames', '--out'])).toThrowError(/--geonames/); - // The documented empty value still works. + // Documented short flags are option tokens too: swallowing -h would + // start a network sweep instead of printing help. + expect(() => liq.parseSweepArgs(['--accept-language', '-h'])).toThrowError(/--accept-language/); + expect(() => liq.parseSweepArgs(['--endpoint', '-d'])).toThrowError(/--endpoint/); + // The documented empty value still works, and values that merely look + // dash-ish are accepted. expect(liq.parseSweepArgs(['--accept-language', '']).acceptLanguage).toEqual(''); + expect(liq.parseSweepArgs(['--api-key', '-secret-']).apiKey).toEqual('-secret-'); }); it('rejects unsupported --reverse-mode values instead of silently mapping them', () => { From 32458795be661cdc39c23508fbd825d7a4c3da7d Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 15:27:54 -0600 Subject: [PATCH 09/12] Validate the persisted quota date before treating it as an earlier day A damaged but string-valued date, such as {"date":"","count":4500}, passed validation and then failed the same-day comparison, resetting the count to zero and permitting another full daily cap. The stored date must now be a canonical UTC date (YYYY-MM-DD that round-trips), so malformed strings and impossible dates like 2026-02-30 fail closed as damaged state. A genuine earlier date still rolls the count over. One new regression spec; 77 specs green. --- scripts/validate_with_locationiq.js | 14 +++++++++++++- spec/locationiq_sweep_spec.js | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 5a43456..f12aa42 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -1087,6 +1087,16 @@ function utcDateString(date) { return date.toISOString().slice(0, 10) } +function isCanonicalUtcDateString(value) { + // Must be YYYY-MM-DD *and* a real calendar date: the round-trip rejects + // both malformed strings and impossible dates like 2026-02-30, either of + // which would otherwise read as "an earlier day" and reset the count. + if (typeof value !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(value)) return false + var parsedMs = Date.parse(value + 'T00:00:00Z') + if (!Number.isFinite(parsedMs)) return false + return utcDateString(new Date(parsedMs)) === value +} + function sweepCoordKey(latitude, longitude) { return Number(latitude).toFixed(SWEEP_CACHE_DECIMALS) + ',' + Number(longitude).toFixed(SWEEP_CACHE_DECIMALS) } @@ -1454,9 +1464,11 @@ function loadQuotaState(statePath, todayUtc) { // full daily cap of requests. The count must BE a nonnegative integer, // not merely coerce to one: Number(null)/Number(true)/Number('') are all // finite, and clamping a negative count would equally re-open the cap. + // The date must be a real canonical UTC date for the same reason: any + // other string would look like "an earlier day" and reset the count. var countIsValid = Boolean(parsed) && typeof parsed.count === 'number' && Number.isInteger(parsed.count) && parsed.count >= 0 - if (!parsed || typeof parsed.date !== 'string' || !countIsValid) { + if (!parsed || !isCanonicalUtcDateString(parsed.date) || !countIsValid) { throw new Error('Quota state file ' + statePath + ' exists but is damaged; refusing to guess the request count. ' + 'Inspect it and, only if you are sure no requests were made today (UTC), delete it to reset.') } diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 4f9e509..9036b7a 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -694,6 +694,30 @@ describe('locationiq sweep', () => { } }); + it('fails closed on a damaged persisted quota date', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + // Any string that is not a real canonical UTC date would otherwise + // read as "an earlier day" and reset a fully spent count to zero. + for (const badDate of ['', 'yesterday', '2026-8-19', '2026-02-30', '20260819']) { + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ date: badDate, count: 4500 })); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/quota state/i); + } + expect(deps.calls.length).toEqual(0); + + // A genuine earlier day still rolls the count over. + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ date: '2000-01-01', count: 4500 })); + const summary = await liq.runSweep(sweepOpts(dir), deps); + expect(deps.calls.length).toEqual(5); + expect(summary.stopReason).toBeNull(); + expect(readState(dir).date).toEqual(liq.utcDateString(new Date())); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it('stops without caching when a 404 rejects the route rather than the coordinate', async () => { const dir = makeTmpDir(); try { From a8b3bdecb91e2118cfb84af11ed99a58027bbe3c Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 17:06:24 -0600 Subject: [PATCH 10/12] Address review: clock-skew quota safety, stream aborts, unstamped caches - Fail closed when the persisted quota date lies in the future. Those requests were counted by LocationIQ against the real (earlier) day, so resetting after a backward clock correction would hand out a second full cap. Genuinely earlier dates still roll over. - Clamp a future lastRequestAt to now: a backward clock correction would otherwise make the pacing wait cover the whole clock offset and stall the sweep for that long before its first request. - Handle response-stream failures in fetchJson. An aborted chunked response previously left the promise unsettled forever (the request-level error listener does not fire once a response has begun, and no 'end' arrives), hanging the sweep; it now rejects and produces the documented resumable fetch_error stop. fetchJson also honors http:// URLs so it can be pointed at a local mock endpoint. - Reject a populated cache that carries no configuration record instead of stamping it with the current settings and reusing responses of unknown provenance; only an empty cache is adopted. Documented in the sweep usage text. Six new specs, including loopback-only fetchJson coverage (127.0.0.1, no external host); 88 specs green. --- scripts/validate_with_locationiq.js | 73 +++++++++-- spec/locationiq_sweep_spec.js | 180 +++++++++++++++++++++++++++- 2 files changed, 241 insertions(+), 12 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index f12aa42..17ede2a 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -3,6 +3,7 @@ const fs = require('fs') const path = require('path') +const http = require('http') const https = require('https') const createGeocoder = require('../src/index') @@ -646,21 +647,48 @@ function buildVerdict(localityMatch, countryMatch, localName, liqLocality) { function fetchJson(endpointUrl, timeoutMs) { return new Promise(function(resolve, reject) { - var req = https.get(endpointUrl, function(response) { + var settled = false + function fail(err) { + if (settled) return + settled = true + reject(err instanceof Error ? err : new Error(String(err))) + } + function succeed(value) { + if (settled) return + settled = true + resolve(value) + } + + // http is supported so a sweep can be pointed at a local mock endpoint; + // real LocationIQ traffic stays on https. + var transport = String(endpointUrl).slice(0, 6) === 'http:/' ? http : https + + var req = transport.get(endpointUrl, function(response) { var chunks = [] response.on('data', function(chunk) { chunks.push(chunk) }) + // A stream reset or abort AFTER headers arrive is not reported by the + // request-level error listener: without these the process would die on + // an unhandled stream error instead of producing a resumable stop. + response.on('error', fail) + response.on('aborted', function() { + fail(new Error('Response stream aborted before completion')) + }) response.on('end', function() { + if (!response.complete) { + fail(new Error('Response stream ended before the full body was received')) + return + } var body = Buffer.concat(chunks).toString('utf8') try { var parsed = JSON.parse(body) - resolve({ status: response.statusCode || 0, json: parsed, raw: body }) + succeed({ status: response.statusCode || 0, json: parsed, raw: body }) } catch (err) { - reject(new Error('Invalid JSON response (' + (response.statusCode || 0) + '): ' + body.slice(0, 200))) + fail(new Error('Invalid JSON response (' + (response.statusCode || 0) + '): ' + body.slice(0, 200))) } }) }) - req.on('error', reject) + req.on('error', fail) req.setTimeout(timeoutMs, function() { req.destroy(new Error('Request timed out after ' + timeoutMs + 'ms')) }) @@ -1050,7 +1078,10 @@ function sweepUsage() { 'cleanly. Designed for LocationIQ\'s free tier; run one sweep at a time.', 'The cache records the endpoint and accept-language it was built with, and', 'network runs with different values are rejected — use a separate --workdir', - 'per configuration (--dry-run only evaluates the cache and is exempt).', + 'per configuration (--dry-run only evaluates the cache and is exempt). A', + 'cache holding responses but no such record (e.g. from an older version of', + 'this script) is rejected rather than adopted, since the settings that', + 'produced those responses are unknown; only an empty cache is stamped.', '', 'Options:', ' --points JSONL points file (required; see the sample subcommand)', @@ -1429,6 +1460,17 @@ function ensureSweepCacheConfig(cachePath, config) { // would corrupt the report or re-spend quota. var meta = readSweepCacheMeta(cachePath) if (!meta) { + // Only a new or empty cache may be stamped. A populated but unstamped + // cache (e.g. written by an earlier version of this script) has an + // unknown provenance: adopting it would bless responses that may have + // been fetched under a different endpoint or language. + var existing = loadSweepCache(cachePath) + if (Object.keys(existing).length > 0) { + throw new Error('Cache ' + cachePath + ' holds responses but no configuration record, so the endpoint and ' + + 'accept-language that produced them are unknown. Point --cache/--workdir at a new location, or delete ' + + 'the cache to refetch under the current settings (endpoint=' + config.endpoint + + ' accept-language=' + (config.acceptLanguage || '(none)') + ').') + } appendSweepCache(cachePath, { meta: config }) return } @@ -1475,9 +1517,23 @@ function loadQuotaState(statePath, todayUtc) { // The last-attempt time is advisory (it only delays the next request), so // an absent or unusable value degrades to "no wait" rather than failing. - var lastRequestAt = typeof parsed.lastRequestAt === 'number' && Number.isFinite(parsed.lastRequestAt) && parsed.lastRequestAt > 0 - ? parsed.lastRequestAt - : 0 + // A future timestamp (backward clock correction) is clamped to now, or the + // computed wait would stall the run for the whole clock offset. + var nowMs = Date.now() + var lastRequestAt = 0 + if (typeof parsed.lastRequestAt === 'number' && Number.isFinite(parsed.lastRequestAt) && parsed.lastRequestAt > 0) { + lastRequestAt = Math.min(parsed.lastRequestAt, nowMs) + } + + if (parsed.date > todayUtc) { + // Requests made while the host clock ran ahead were counted by + // LocationIQ against the real (earlier) day, so resetting here would + // hand out a second full cap. Both fields are ISO YYYY-MM-DD, which + // compares lexicographically. + throw new Error('Quota state file ' + statePath + ' records a future date (' + parsed.date + + ', today is ' + todayUtc + ' UTC); refusing to reset the request count. Check the host clock, then ' + + 'delete the file only if you are sure no requests were made today (UTC).') + } if (parsed.date !== todayUtc) { return { date: todayUtc, count: 0, lastRequestAt: lastRequestAt } @@ -2130,6 +2186,7 @@ function dispatch() { } module.exports = { + fetchJson: fetchJson, normalizeName: normalizeName, namesMatch: namesMatch, extractLocationIqLocality: extractLocationIqLocality, diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 9036b7a..8f66b94 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -1,5 +1,6 @@ const fs = require('fs'); const os = require('os'); +const http = require('http'); const path = require('path'); const { spawnSync } = require('child_process'); @@ -97,6 +98,21 @@ function cacheEntries(dir) { .filter((entry) => entry && entry.key); } +// A cache written by a normal sweep always carries a configuration stamp on +// its first line; fixtures must look the same or they are (correctly) +// rejected as being of unknown provenance. +const CACHE_META_LINE = JSON.stringify({ + meta: { endpoint: 'https://liq.invalid/v1/reverse', acceptLanguage: 'en' } +}); + +function writeCacheFile(dir, body, options) { + const withMeta = !options || options.stamped !== false; + const file = (options && options.file) || path.join(dir, 'cache.jsonl'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, (withMeta ? CACHE_META_LINE + '\n' : '') + body); + return file; +} + function point(overrides) { return Object.assign({ key: '13.4767,-89.3072', @@ -499,7 +515,7 @@ describe('locationiq sweep', () => { status: 200, body: { display_name: 'San Salvador, El Salvador', address: { city: 'San Salvador', country_code: 'sv' } } }; - fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + writeCacheFile(dir, JSON.stringify(cached) + '\n'); const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); const summary = await liq.runSweep(sweepOpts(dir), deps); @@ -564,7 +580,7 @@ describe('locationiq sweep', () => { status: 200, body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } }; - fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + writeCacheFile(dir, JSON.stringify(cached) + '\n'); const deps = makeDeps(() => { throw new Error('dry-run must not touch the network'); @@ -694,6 +710,81 @@ describe('locationiq sweep', () => { } }); + it('fails closed on a future persisted quota date', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + // Requests made while the clock ran ahead were counted by LocationIQ + // against the real day, so resetting here would grant a second cap. + const tomorrow = liq.utcDateString(new Date(Date.now() + 24 * 3600 * 1000)); + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ date: tomorrow, count: 4500 })); + + await expectAsync(liq.runSweep(sweepOpts(dir), deps)).toBeRejectedWithError(/future date/i); + expect(deps.calls.length).toEqual(0); + // The evidence is left in place rather than overwritten. + expect(readState(dir).count).toEqual(4500); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('clamps a future pacing timestamp instead of stalling the run', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + // A backward clock correction can leave lastRequestAt an hour ahead; + // pacing must never wait longer than the configured interval. + fs.writeFileSync(path.join(dir, 'quota.json'), JSON.stringify({ + date: liq.utcDateString(new Date()), + count: 0, + lastRequestAt: Date.now() + 3600 * 1000 + })); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const waits = []; + deps.sleep = async (ms) => { waits.push(ms); }; + + const summary = await liq.runSweep(sweepOpts(dir, { rps: 1, maxRequests: 1 }), deps); + + expect(summary.requestsThisRun).toEqual(1); + waits.forEach((ms) => expect(ms).toBeLessThanOrEqual(1000)); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rejects a populated cache that carries no configuration record', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const legacy = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } + }; + // A cache from an earlier implementation: responses of unknown + // provenance must not be adopted and stamped as if they were ours. + writeCacheFile(dir, JSON.stringify(legacy) + '\n', { stamped: false }); + + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + await expectAsync(liq.runSweep(sweepOpts(dir), deps)) + .toBeRejectedWithError(/no configuration record/i); + expect(deps.calls.length).toEqual(0); + + // An empty cache file is still adopted and stamped normally. + writeCacheFile(dir, '', { stamped: false }); + const fresh = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary = await liq.runSweep(sweepOpts(dir), fresh); + expect(fresh.calls.length).toEqual(5); + expect(summary.stopReason).toBeNull(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it('fails closed on a damaged persisted quota date', async () => { const dir = makeTmpDir(); try { @@ -898,7 +989,7 @@ describe('locationiq sweep', () => { }; // Valid entry followed by a torn final line with no newline, as an // interrupted append would leave it. - fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(seeded) + '\n' + '{"key":"13.4767'); + writeCacheFile(dir, JSON.stringify(seeded) + '\n' + '{"key":"13.4767'); const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); const summary = await liq.runSweep(sweepOpts(dir), deps); @@ -956,7 +1047,7 @@ describe('locationiq sweep', () => { status: 200, body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } }; - fs.writeFileSync(path.join(dir, 'cache.jsonl'), JSON.stringify(cached) + '\n'); + writeCacheFile(dir, JSON.stringify(cached) + '\n'); const deps = makeDeps(() => { throw new Error('dry-run must not touch the network'); @@ -1117,4 +1208,85 @@ describe('locationiq sweep', () => { } }); }); + + describe('fetchJson', () => { + // These use a loopback server on 127.0.0.1 only: no external host is + // contacted, and no LocationIQ request is ever made. + function withServer(handler, run) { + return new Promise((resolve, reject) => { + const server = http.createServer(handler); + server.listen(0, '127.0.0.1', async () => { + const url = 'http://127.0.0.1:' + server.address().port + '/reverse'; + try { + resolve(await run(url)); + } catch (err) { + reject(err); + } finally { + server.close(); + } + }); + server.on('error', reject); + }); + } + + it('rejects rather than hanging when a chunked response is aborted mid-stream', async () => { + // Without response-stream handlers this promise never settles: the + // request-level 'error' listener does not fire once a response has + // begun, and no 'end' arrives on an aborted chunked stream — so the + // sweep would stall forever instead of stopping resumably. + await withServer( + (req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json', 'Transfer-Encoding': 'chunked' }); + res.write(JSON.stringify({ address: { city: 'Truncated', country_code: 'us' } })); + setTimeout(() => res.socket.destroy(), 20); + }, + async (url) => { + const outcome = await Promise.race([ + liq.fetchJson(url, 5000).then(() => 'resolved', (err) => 'rejected: ' + err.message), + new Promise((resolve) => setTimeout(() => resolve('hung'), 2000)) + ]); + expect(outcome).toMatch(/^rejected: /); + } + ); + }); + + it('rejects when the connection drops mid-body', async () => { + await withServer( + (req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json', 'Content-Length': '64' }); + res.write('{"address":'); + res.socket.destroy(); + }, + async (url) => { + await expectAsync(liq.fetchJson(url, 5000)).toBeRejectedWithError(Error); + } + ); + }); + + it('resolves a complete JSON response', async () => { + await withServer( + (req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ address: { city: 'Testville', country_code: 'us' } })); + }, + async (url) => { + const response = await liq.fetchJson(url, 5000); + expect(response.status).toEqual(200); + expect(response.json.address.city).toEqual('Testville'); + } + ); + }); + + it('rejects a non-JSON body rather than resolving garbage', async () => { + await withServer( + (req, res) => { + res.writeHead(502, { 'Content-Type': 'text/html' }); + res.end('bad gateway'); + }, + async (url) => { + await expectAsync(liq.fetchJson(url, 5000)).toBeRejectedWithError(/Invalid JSON response \(502\)/); + } + ); + }); + }); }); From 5ff5a3c22d5aed60485d9d444417d6c63402732d Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 18:53:38 -0600 Subject: [PATCH 11/12] Address review: backward clocks, sparse answers, endpoint and cache guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Stop the run when the host clock moves backward across midnight mid-sweep instead of resetting the daily count, which would grant a second full cap for requests LocationIQ already counted. This is the mirror of the fail-closed rule for a persisted future date; only a date that advances rolls the count over. - Classify an empty offline answer against a country-only LocationIQ response as liq_name_missing rather than offline_empty: nothing was supplied to have matched, so it is unverifiable, not a failure. A country mismatch now outranks every name-level verdict. - Validate --endpoint at parse time (valid http(s) URL) and build each request URL before the quota count is persisted, so a malformed endpoint cannot record a request that was never made, bypass the report, or leave the cache stamped with an unusable value. - Fold Latin letters NFKD leaves undecomposed (ł, đ, ø, þ, ß, æ, œ and friends), so native-versus-ASCII spellings such as Łódź/Lodz, Tromsø/Tromso and Đà Nẵng/Da Nang compare equal as the documented diacritics-insensitive comparison promises. - Apply the unstamped-cache rejection to dry runs too: provenance affects a rebuilt report as much as a fetched one. Dry runs still skip the comparison against the current request options, and no longer stamp or write to the cache at all. Five new regression specs; 93 specs green. --- scripts/validate_with_locationiq.js | 100 +++++++++++++++++++---- spec/locationiq_sweep_spec.js | 121 ++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 14 deletions(-) diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index 17ede2a..ca75bf5 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -430,6 +430,32 @@ async function ensureSamplePoints(sourceDb, cacheDb, lookupTable, targetCount, s var LATIN_BASE_RE = /\p{Script=Latin}/u +// Latin letters that NFKD does not decompose: their diacritic or ligature is +// part of the letter itself. LocationIQ and the offline database often differ +// exactly here (Łódź/Lodz, Tromsø/Tromso, Đà Nẵng/Da Nang), so fold them to +// the ASCII spelling the other side is likely to use. +var LATIN_FOLD_MAP = { + 'ł': 'l', 'ŀ': 'l', + 'đ': 'd', 'ð': 'd', + 'ø': 'o', 'œ': 'oe', + 'æ': 'ae', + 'þ': 'th', + 'ß': 'ss', + 'ħ': 'h', + 'ŧ': 't', + 'ı': 'i', + 'ĸ': 'k' +} + +function foldLatinLetters(value) { + var out = '' + for (var ch of value) { + var mapped = LATIN_FOLD_MAP[ch] + out += mapped === undefined ? ch : mapped + } + return out +} + function normalizeName(value) { if (!value) return '' @@ -471,8 +497,7 @@ function normalizeName(value) { lastBaseIsLatin = LATIN_BASE_RE.test(ch) } - return kept - .toLowerCase() + return foldLatinLetters(kept.toLowerCase()) .replace(/[^\p{L}\p{M}\p{N}]+/gu, ' ') .trim() .replace(/\s+/g, ' ') @@ -1453,7 +1478,8 @@ function readSweepCacheMeta(cachePath) { return null } -function ensureSweepCacheConfig(cachePath, config) { +function ensureSweepCacheConfig(cachePath, config, options) { + var dryRun = Boolean(options && options.dryRun) // Cached responses depend on the request-shaping options, so the cache is // stamped with them and a run using different options is rejected loudly: // silently evaluating stale responses (or silently refetching everything) @@ -1471,9 +1497,13 @@ function ensureSweepCacheConfig(cachePath, config) { 'the cache to refetch under the current settings (endpoint=' + config.endpoint + ' accept-language=' + (config.acceptLanguage || '(none)') + ').') } - appendSweepCache(cachePath, { meta: config }) + // A dry run evaluates without fetching, so it has nothing to stamp. + if (!dryRun) appendSweepCache(cachePath, { meta: config }) return } + // Only a fetching run must agree with the cache's settings: a dry run + // re-reports whatever the cache holds, whatever options it was given. + if (dryRun) return if (meta.endpoint !== config.endpoint || meta.acceptLanguage !== config.acceptLanguage) { throw new Error('Cache ' + cachePath + ' was built with endpoint=' + meta.endpoint + ' accept-language=' + (meta.acceptLanguage || '(none)') + @@ -1616,16 +1646,20 @@ function comparePoint(point, offlineResult, cacheEntry) { if (!liqOk) { record.verdict = offlineName ? 'liq_empty' : 'both_empty' + } else if (offlineCountry && record.liq_country && offlineCountry !== record.liq_country) { + // The severe check outranks every name-level classification. + record.verdict = 'country_mismatch' } else if (!offlineName) { - record.verdict = 'offline_empty' + // An empty offline answer is only a failure if LocationIQ actually + // supplied a name to have matched; a country-only response gives + // nothing to verify against either way. + record.verdict = liqHasComparableName(address) ? 'offline_empty' : 'liq_name_missing' } else if (!offlineCountry || !record.liq_country) { // One side lacks a country code, so the severe check cannot run and the // point must not inflate agreement either: classify it as unverifiable. // Any name match is still recorded in match_via for context. record.match_via = findLiqNameMatch(normalizeName(offlineName), address, record.liq_display_name) || '' record.verdict = 'country_unknown' - } else if (offlineCountry !== record.liq_country) { - record.verdict = 'country_mismatch' } else { // Attempt the match first: display_name segments count as agreement even // when the address block carries no name fields at all. @@ -1829,11 +1863,15 @@ async function runSweep(opts, deps) { } fs.mkdirSync(path.dirname(opts.cachePath), { recursive: true }) - if (!opts.dryRun) { - // Dry runs only evaluate what is already cached, so the request-shaping - // options are irrelevant there; network runs must match the cache. - ensureSweepCacheConfig(opts.cachePath, { endpoint: opts.endpoint, acceptLanguage: opts.acceptLanguage }) - } + // Provenance is checked in both modes — responses of unknown endpoint or + // language corrupt a report just as much when it is rebuilt offline. Only + // the comparison against the current request options is skipped for a dry + // run, which re-reports the cache rather than extending it. + ensureSweepCacheConfig( + opts.cachePath, + { endpoint: opts.endpoint, acceptLanguage: opts.acceptLanguage }, + { dryRun: opts.dryRun } + ) var cache = loadSweepCache(opts.cachePath) var todayUtc = utcDateString(nowImpl()) var state @@ -1876,6 +1914,15 @@ async function runSweep(opts, deps) { // otherwise a later invocation would reset the stale date and allow // nearly twice the cap within the new UTC day. var attemptDate = utcDateString(nowImpl()) + if (attemptDate < state.date) { + // The clock moved backward across midnight mid-run. Resetting here + // would grant a second full cap for requests LocationIQ already + // counted, so stop instead — the mirror of the fail-closed rule for + // a persisted future date. + stopReason = 'clock_backward' + stopDetail = 'state date ' + state.date + ' is later than the current UTC date ' + attemptDate + break + } if (attemptDate !== state.date) { // The count resets per UTC day; the pacing timestamp does not. state = { date: attemptDate, count: 0, lastRequestAt: state.lastRequestAt || 0 } @@ -1887,6 +1934,18 @@ async function runSweep(opts, deps) { break } + // Build the URL before spending anything: a malformed endpoint would + // otherwise throw after the count was persisted, recording a request + // that was never made and bypassing the report entirely. + var url + try { + url = buildLocationIqUrl(opts.endpoint, opts.apiKey, point.lat, point.lon, opts.acceptLanguage) + } catch (err) { + stopReason = 'bad_request' + stopDetail = 'could not build request URL: ' + String(err && err.message ? err.message : err) + break + } + // Count the attempt before it happens so a crash mid-request can only // over-count, never let a later run exceed the cap. The attempt time // is persisted with it so the next process paces from this request. @@ -1896,7 +1955,6 @@ async function runSweep(opts, deps) { saveQuotaState(opts.statePath, state) requestsThisRun += 1 - var url = buildLocationIqUrl(opts.endpoint, opts.apiKey, point.lat, point.lon, opts.acceptLanguage) var response try { response = await deps.fetchJson(url, SWEEP_TIMEOUT_MS) @@ -1998,6 +2056,8 @@ async function runSweep(opts, deps) { unfetched + ' points still unfetched. Re-run the same command after the next UTC day starts to resume; cached points are never re-queried.') } else if (stopReason === 'rate_limited') { log('LocationIQ returned HTTP 429 (rate limited). Backing off and stopping this run cleanly; all fetched responses are cached. Wait for the quota window to reset, then re-run the same command to resume.') + } else if (stopReason === 'clock_backward') { + log('The host clock moved backward across a UTC day boundary mid-run (' + stopDetail + '). Stopping instead of resetting the daily count, which would allow a second full cap for requests LocationIQ already counted. Fix the clock, then re-run to resume.') } else if (stopReason === 'bad_endpoint') { log('LocationIQ returned HTTP 404 for the route itself, not for the coordinate (' + stopDetail + '). This usually means --endpoint is wrong or a proxy intercepted the request, so the response was not cached and the run stopped before spending more quota. Fix the endpoint, then re-run to resume.') } else if (stopReason === 'bad_request') { @@ -2070,7 +2130,19 @@ function parseSweepArgs(argv) { } else if (arg === '--api-key') { opts.apiKey = requireValueArg('--api-key', argv[++i]) } else if (arg === '--endpoint') { - opts.endpoint = requireValueArg('--endpoint', argv[++i]) + // Fail here rather than mid-run: an unusable endpoint would otherwise + // only surface once the cache had been stamped with it. + var endpoint = requireValueArg('--endpoint', argv[++i]) + var endpointUrl + try { + endpointUrl = new URL(endpoint) + } catch (err) { + throw new Error('--endpoint must be a valid URL, got: ' + endpoint) + } + if (endpointUrl.protocol !== 'https:' && endpointUrl.protocol !== 'http:') { + throw new Error('--endpoint must be an http(s) URL, got: ' + endpoint) + } + opts.endpoint = endpoint } else if (arg === '--accept-language') { // '' is a documented value (disables the header), but a following // option token means the value was omitted. diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index 8f66b94..b15d453 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -359,6 +359,42 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('й')).not.toEqual(liq.normalizeName('и')); }); + it('folds Latin letters that NFKD leaves undecomposed', () => { + // These carry the diacritic inside the letter, so decomposition alone + // never reaches the ASCII spelling the other geocoder likely uses. + expect(liq.normalizeName('Łódź')).toEqual(liq.normalizeName('Lodz')); + expect(liq.normalizeName('Tromsø')).toEqual(liq.normalizeName('Tromso')); + expect(liq.normalizeName('Đà Nẵng')).toEqual(liq.normalizeName('Da Nang')); + expect(liq.normalizeName('Þingholt')).toEqual(liq.normalizeName('Thingholt')); + expect(liq.normalizeName('Großenhain')).toEqual(liq.normalizeName('Grossenhain')); + + const record = liq.comparePoint( + point({ country: 'PL' }), + { name: 'Łódź', country: { id: 'PL' } }, + { status: 200, body: { display_name: '', address: { city: 'Lodz', country_code: 'pl' } } } + ); + expect(record.verdict).toEqual('agree'); + }); + + it('classifies an empty offline answer against a country-only response as unverifiable', () => { + // LocationIQ supplied no name to have matched, so this is not an + // offline failure and must not count as a verifiable mismatch. + const sparse = liq.comparePoint( + point({ country: 'US' }), + null, + { status: 200, body: { display_name: 'United States', address: { country_code: 'us' } } } + ); + expect(sparse.verdict).toEqual('liq_name_missing'); + + // With a real LocationIQ name, an empty offline answer is still a miss. + const genuine = liq.comparePoint( + point({ country: 'US' }), + null, + { status: 200, body: { display_name: '', address: { city: 'Somewhere', country_code: 'us' } } } + ); + expect(genuine.verdict).toEqual('offline_empty'); + }); + it('agrees when the offline name matches a locality field after normalization', () => { const record = liq.comparePoint( point({ name: 'Kilómetro 18' }), @@ -710,6 +746,91 @@ describe('locationiq sweep', () => { } }); + it('stops instead of resetting when the clock moves backward across midnight mid-run', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + // Two requests land on 2026-01-02, then the clock is corrected back + // to 2026-01-01. Resetting there would grant a second full cap. + let nowCalls = 0; + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + deps.now = () => { + nowCalls += 1; + return nowCalls <= 3 + ? new Date('2026-01-02T00:10:00Z') + : new Date('2026-01-01T23:50:00Z'); + }; + + const summary = await liq.runSweep(sweepOpts(dir, { dailyCap: 10 }), deps); + + expect(deps.calls.length).toEqual(2); + expect(summary.stopReason).toEqual('clock_backward'); + const state = readState(dir); + expect(state.date).toEqual('2026-01-02'); + expect(state.count).toEqual(2); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rejects a malformed endpoint before any quota is spent', async () => { + // Parse-level guard: the cache is never stamped with an unusable value. + expect(() => liq.parseSweepArgs(['--endpoint', 'not-a-url'])).toThrowError(/--endpoint/); + expect(() => liq.parseSweepArgs(['--endpoint', 'ftp://example.invalid/reverse'])).toThrowError(/--endpoint/); + expect(liq.parseSweepArgs(['--endpoint', 'https://eu1.locationiq.com/v1/reverse']).endpoint) + .toEqual('https://eu1.locationiq.com/v1/reverse'); + + // Defense in depth: if an unusable endpoint reaches runSweep anyway, + // the URL is built before the count is persisted. + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const summary = await liq.runSweep(sweepOpts(dir, { endpoint: 'not-a-url' }), deps); + + expect(deps.calls.length).toEqual(0); + expect(summary.stopReason).toEqual('bad_request'); + // No request was made, so nothing may be counted against the quota — + // the state file is not even created. + expect(summary.quota.count).toEqual(0); + expect(fs.existsSync(path.join(dir, 'quota.json'))).toBeFalse(); + // The report is still written rather than the run throwing. + expect(fs.existsSync(path.join(dir, 'report.md'))).toBeTrue(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rejects a populated unstamped cache during dry runs too', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const legacy = { + key: liq.sweepCoordKey(WORLD_POINTS[0].lat, WORLD_POINTS[0].lon), + lat: WORLD_POINTS[0].lat, + lon: WORLD_POINTS[0].lon, + status: 200, + body: { display_name: '', address: { city: 'Testville', country_code: 'us' } } + }; + writeCacheFile(dir, JSON.stringify(legacy) + '\n', { stamped: false }); + + const deps = makeDeps(() => { + throw new Error('dry-run must not touch the network'); + }); + // Provenance matters for a rebuilt report just as much as for a fetch. + await expectAsync(liq.runSweep(sweepOpts(dir, { dryRun: true, apiKey: '' }), deps)) + .toBeRejectedWithError(/no configuration record/i); + + // A dry run over an absent cache still works and writes nothing. + fs.rmSync(path.join(dir, 'cache.jsonl')); + const summary = await liq.runSweep(sweepOpts(dir, { dryRun: true, apiKey: '' }), deps); + expect(summary.evaluated).toEqual(0); + expect(fs.existsSync(path.join(dir, 'cache.jsonl'))).toBeFalse(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it('fails closed on a future persisted quota date', async () => { const dir = makeTmpDir(); try { From fb4f2c16e2f056ba885da66187e78a9a2850e893 Mon Sep 17 00:00:00 2001 From: Sebastian Schloesser Date: Wed, 19 Aug 2026 20:06:57 -0600 Subject: [PATCH 12/12] Address review: Greek tonos, in-process clock rollback, key-scoped quota MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip the Greek tonos when it sits on a Greek base, so Αθήνα and ΑΘΗΝΑ compare equal as the documented diacritics-insensitive comparison promises. The dialytika is preserved (it distinguishes letters rather than marking stress), as are marks on other non-Latin bases. - Cap the per-iteration pacing wait at the configured interval. A clock correction after this process issued a request left lastRequestAt in the future, and the raw difference would have slept for the whole offset, stalling the run and delaying the clock_backward check. - Scope the default quota state per API key via a non-reversible fingerprint. LocationIQ meters each key separately, so one key's spent cap no longer stops another key's sweep; configurations using the same key still share one tally, and an explicit --state still forces sharing across keys. - Bound geohash precision to the format maximum of 12. The reverse lookup encodes one hash per precision level, so an accidental extra digit would hang the run doing meaningless work. Four new regression specs; 97 specs green. --- README.md | 6 ++- scripts/validate_with_locationiq.js | 56 ++++++++++++++++++++--- spec/locationiq_sweep_spec.js | 71 +++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 7ecf5cb..5d959d4 100644 --- a/README.md +++ b/README.md @@ -342,8 +342,10 @@ Every LocationIQ response is cached as JSONL keyed by coordinates rounded to four decimals, so re-running the same command never re-queries a cached point — if a run stops at the daily cap or on HTTP 429, just run it again later to resume. The daily-cap state lives outside the workdir (default -`tmp/locationiq-quota.json`), so multiple sweep configurations share one cap -for the same API key. Outputs land under `--workdir` (default `tmp/locationiq-sweep/`): +`tmp/locationiq-quota.json`, suffixed with a non-reversible fingerprint of the +API key), so every sweep configuration using one key shares a single cap, while +separate keys — which LocationIQ meters separately — keep their own tallies. +Outputs land under `--workdir` (default `tmp/locationiq-sweep/`): `report.md` (per-country point counts, agreement %, country mismatches, worst examples) and `mismatches.jsonl` (machine-readable list of all mismatches). See `sweep --help` and `sample --help` for all options. diff --git a/scripts/validate_with_locationiq.js b/scripts/validate_with_locationiq.js index ca75bf5..f89d93c 100644 --- a/scripts/validate_with_locationiq.js +++ b/scripts/validate_with_locationiq.js @@ -3,6 +3,7 @@ const fs = require('fs') const path = require('path') +const crypto = require('crypto') const http = require('http') const https = require('https') @@ -429,6 +430,7 @@ async function ensureSamplePoints(sourceDb, cacheDb, lookupTable, targetCount, s } var LATIN_BASE_RE = /\p{Script=Latin}/u +var GREEK_BASE_RE = /\p{Script=Greek}/u // Latin letters that NFKD does not decompose: their diacritic or ligature is // part of the letter itself. LocationIQ and the offline database often differ @@ -473,10 +475,18 @@ function normalizeName(value) { var decomposed = String(value).normalize('NFKD') var kept = '' var lastBaseIsLatin = false + var lastBaseIsGreek = false for (var ch of decomposed) { var code = ch.codePointAt(0) if (code >= 0x0300 && code <= 0x036f) { - if (!lastBaseIsLatin) kept += ch + // Strip when the base is Latin (é → e), and for the Greek tonos on a + // Greek base (ή → η): it marks stress and is routinely dropped in + // uppercase or accent-stripped place data, so Αθήνα must equal ΑΘΗΝΑ. + // Every other mark on a non-Latin base is part of the letter (Cyrillic + // и + breve = й), including the Greek dialytika, which distinguishes + // letters rather than marking stress. + var isGreekTonos = code === 0x0301 && lastBaseIsGreek + if (!lastBaseIsLatin && !isGreekTonos) kept += ch continue } // Hebrew: strip only the actual combining marks (cantillation, niqqud, @@ -495,6 +505,7 @@ function normalizeName(value) { // that never decompose themselves, yet their accented forms do (ǿ is // ø + U+0301 under NFKD) and must fold the same way as e/é. lastBaseIsLatin = LATIN_BASE_RE.test(ch) + lastBaseIsGreek = GREEK_BASE_RE.test(ch) } return foldLatinLetters(kept.toLowerCase()) @@ -1055,6 +1066,8 @@ var SWEEP_DEFAULT_WORKDIR = 'tmp/locationiq-sweep' // per endpoint/language configuration must still share one daily cap, // because they all spend requests against the same API key. var SWEEP_DEFAULT_STATE_PATH = 'tmp/locationiq-quota.json' +// A base32 geohash carries no useful precision past 12 characters. +var MAX_GEOHASH_PRECISION = 12 // Fixed seed for the sampler's country-order shuffle: stable across runs, but // not alphabetical, so a small --max-points does not bias the world sample // toward alphabetically-early country codes. @@ -1113,9 +1126,12 @@ function sweepUsage() { ' --database Offline geocoder SQLite database (required)', ' --workdir Directory for cache/report files (default: ' + SWEEP_DEFAULT_WORKDIR + ')', ' --cache LocationIQ response cache (default: /cache.jsonl)', - ' --state Daily quota state file (default: ' + SWEEP_DEFAULT_STATE_PATH + ').', - ' Deliberately outside the workdir: all sweep configurations', - ' share one daily cap because they spend the same API key.', + ' --state Daily quota state file (default: ' + SWEEP_DEFAULT_STATE_PATH + ', suffixed', + ' with a non-reversible fingerprint of the API key).', + ' Deliberately outside the workdir, so every endpoint/language', + ' configuration using the same key shares one daily cap; keys are', + ' metered separately by LocationIQ, so each gets its own tally.', + ' Pass this explicitly to force several keys to share one cap.', ' --report Markdown report output (default: /report.md)', ' --mismatches Mismatch JSONL output (default: /mismatches.jsonl)', ' --api-key LocationIQ API key (or env LOCATIONIQ_API_KEY)', @@ -1143,6 +1159,23 @@ function utcDateString(date) { return date.toISOString().slice(0, 10) } +function apiKeyFingerprint(apiKey) { + // Non-reversible and never logged in full: only used to keep one + // credential's tally separate from another's. + return crypto.createHash('sha256').update(String(apiKey), 'utf8').digest('hex').slice(0, 12) +} + +function defaultStatePath(apiKey) { + // LocationIQ meters each key separately, so the default state is scoped + // per credential: sharing one file across keys would stop a fresh key's + // sweep at another key's spent cap. Scoping stays independent of the + // workdir, so endpoint/language configurations on the SAME key still + // share one tally. Pass --state explicitly to force sharing. + if (!apiKey) return path.resolve(SWEEP_DEFAULT_STATE_PATH) + var parsed = path.parse(path.resolve(SWEEP_DEFAULT_STATE_PATH)) + return path.join(parsed.dir, parsed.name + '-' + apiKeyFingerprint(apiKey) + parsed.ext) +} + function isCanonicalUtcDateString(value) { // Must be YYYY-MM-DD *and* a real calendar date: the round-trip rejects // both malformed strings and impossible dates like 2026-02-30, either of @@ -1905,7 +1938,11 @@ async function runSweep(opts, deps) { break } - var waitMs = lastRequestAt + delayMs - Date.now() + // Cap the wait at the configured interval: if the clock moved backward + // after this process issued a request, lastRequestAt sits ahead of now + // and the raw difference would sleep for the entire offset — long + // enough to stall the run and to delay the clock_backward check below. + var waitMs = Math.min(delayMs, lastRequestAt + delayMs - Date.now()) if (waitMs > 0) await sleepImpl(waitMs) // A long run can cross midnight UTC — including during the rate-limit @@ -2185,9 +2222,16 @@ function parseSweepArgs(argv) { if (opts.maxPrecision < opts.basePrecision) { throw new Error('--max-precision (' + opts.maxPrecision + ') must be >= --base-precision (' + opts.basePrecision + ')') } + // A base32 geohash saturates at 12 characters (sub-millimetre), and the + // reverse lookup encodes one hash per precision level, so an accidental + // extra digit would otherwise hang the run doing meaningless work. + if (opts.basePrecision > MAX_GEOHASH_PRECISION || opts.maxPrecision > MAX_GEOHASH_PRECISION) { + throw new Error('geohash precision must be between 1 and ' + MAX_GEOHASH_PRECISION + + ', got --base-precision ' + opts.basePrecision + ' --max-precision ' + opts.maxPrecision) + } if (!opts.cachePath) opts.cachePath = path.join(opts.workdir, 'cache.jsonl') - if (!opts.statePath) opts.statePath = path.resolve(SWEEP_DEFAULT_STATE_PATH) + if (!opts.statePath) opts.statePath = defaultStatePath(opts.apiKey) if (!opts.reportPath) opts.reportPath = path.join(opts.workdir, 'report.md') if (!opts.mismatchesPath) opts.mismatchesPath = path.join(opts.workdir, 'mismatches.jsonl') diff --git a/spec/locationiq_sweep_spec.js b/spec/locationiq_sweep_spec.js index b15d453..806d72d 100644 --- a/spec/locationiq_sweep_spec.js +++ b/spec/locationiq_sweep_spec.js @@ -359,6 +359,18 @@ describe('locationiq sweep', () => { expect(liq.normalizeName('й')).not.toEqual(liq.normalizeName('и')); }); + it('treats the Greek tonos as optional while keeping the dialytika', () => { + // Uppercase and accent-stripped place data routinely drop the stress + // accent, so these spellings must compare equal. + expect(liq.normalizeName('Αθήνα')).toEqual(liq.normalizeName('ΑΘΗΝΑ')); + expect(liq.normalizeName('Αθήνα')).toEqual(liq.normalizeName('Αθηνα')); + expect(liq.normalizeName('Θεσσαλονίκη')).toEqual(liq.normalizeName('ΘΕΣΣΑΛΟΝΙΚΗ')); + // The dialytika distinguishes letters rather than marking stress. + expect(liq.normalizeName('Μαϊάμι')).not.toEqual(liq.normalizeName('Μαιαμι')); + // Marks on other non-Latin bases are still preserved. + expect(liq.normalizeName('й')).not.toEqual(liq.normalizeName('и')); + }); + it('folds Latin letters that NFKD leaves undecomposed', () => { // These carry the diacritic inside the letter, so decomposition alone // never reaches the ASCII spelling the other geocoder likely uses. @@ -773,6 +785,65 @@ describe('locationiq sweep', () => { } }); + it('caps the pacing wait when the clock rolls back mid-process', async () => { + const dir = makeTmpDir(); + try { + writePointsFile(dir, WORLD_POINTS); + const deps = makeDeps(() => okResponse({ city: 'Testville', country_code: 'us' })); + const waits = []; + deps.sleep = async (ms) => { waits.push(ms); }; + // The clock is an hour ahead while the first request is recorded, + // then corrected backward. lastRequestAt is now an hour in the + // future, so the raw difference would sleep for that whole offset; + // the wait must stay bounded by the configured interval instead. + const realNow = Date.now; + let calls = 0; + spyOn(Date, 'now').and.callFake(() => { + calls += 1; + const base = realNow.call(Date); + return calls <= 2 ? base + 3600 * 1000 : base; + }); + + const summary = await liq.runSweep(sweepOpts(dir, { rps: 1, dailyCap: 3 }), deps); + + expect(summary.requestsThisRun).toBeGreaterThan(1); + expect(waits.length).toBeGreaterThan(0); + waits.forEach((ms) => expect(ms).toBeLessThanOrEqual(1000)); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('scopes the default quota state per API key', () => { + // LocationIQ meters each key separately, so one key's spent cap must + // not stop another key's sweep. + const a = liq.parseSweepArgs(['--api-key', 'key-a']).statePath; + const b = liq.parseSweepArgs(['--api-key', 'key-b']).statePath; + expect(a).not.toEqual(b); + // The key itself never appears in the path. + expect(a).not.toContain('key-a'); + + // The same key shares one tally across workdirs and languages. + const sameKeyElsewhere = liq.parseSweepArgs([ + '--api-key', 'key-a', '--workdir', path.join(os.tmpdir(), 'liq-fr'), '--accept-language', 'fr' + ]).statePath; + expect(sameKeyElsewhere).toEqual(a); + + // An explicit --state still forces sharing across keys. + const shared = path.join(os.tmpdir(), 'shared-quota.json'); + expect(liq.parseSweepArgs(['--api-key', 'key-a', '--state', shared]).statePath) + .toEqual(liq.parseSweepArgs(['--api-key', 'key-b', '--state', shared]).statePath); + }); + + it('rejects geohash precisions beyond the format maximum', () => { + // reverseHashes encodes one geohash per precision level, so an + // accidental extra digit would hang the run doing meaningless work. + expect(() => liq.parseSweepArgs(['--max-precision', '100000'])).toThrowError(/precision must be between 1 and 12/); + expect(() => liq.parseSweepArgs(['--base-precision', '50'])).toThrowError(/precision must be between 1 and 12/); + expect(liq.parseSweepArgs(['--base-precision', '4', '--max-precision', '7']).maxPrecision).toEqual(7); + expect(liq.parseSweepArgs(['--base-precision', '12', '--max-precision', '12']).maxPrecision).toEqual(12); + }); + it('rejects a malformed endpoint before any quota is spent', async () => { // Parse-level guard: the cache is never stamped with an unusable value. expect(() => liq.parseSweepArgs(['--endpoint', 'not-a-url'])).toThrowError(/--endpoint/);