Add quota-aware resumable LocationIQ world validation sweep - #6
Conversation
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ae8efb2a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60fd9e27bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daed2e86e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9266e1481
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…se 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc1a8deace
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… script - 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 037ce23164
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e2351e531
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 959a054ec7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32458795be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…idation-sweep # Conflicts: # CHANGELOG
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16e91b2dbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b3bdecb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…uards - 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ff5a3c22d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb4f2c16e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0519f0d917
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (value === null || value === undefined || typeof value === 'boolean') return NaN | ||
| if (typeof value === 'string' && !value.trim()) return NaN | ||
| return Number(value) |
There was a problem hiding this comment.
Fresh evidence beyond the null/boolean/blank fix is that JSON arrays still reach Number(value): for example, {"lat":[],"lon":[]} is accepted as a real point at (0, 0), while [40] is accepted as 40. A malformed JSONL row can therefore be queried, cached, and included in the country report instead of being counted as skipped; require coordinates to be scalar numbers or numeric strings before coercion.
Useful? React with 👍 / 👎.
1 participant
Problem
The existing
scripts/validate_with_locationiq.jsvalidates a single database by random-sampling its own geohash cells — useful for spot checks, but it cannot answer the question the geocoder world-fix program needs answered: where in the world is the offline data worst? A world-wide comparison against LocationIQ needs deliberate sampling (top places per country), and it must survive LocationIQ's free tier: a run over thousands of points cannot finish in one day, so it has to be strictly quota-capped, rate-limited, and resumable across invocations without ever re-spending a request.Change
Two new subcommands on the existing script (the legacy random-sample mode is unchanged, including its
LOCATIONIQ_API_KEYenv var and CLI conventions):sample— builds a JSONL points file ({lat, lon, country, name, population}per line) from a GeoNames-style TSV (the cities1000 format), selecting the top-N most populous places per country (default 25). An optional--max-pointstotal cap is filled round-robin by rank so every country keeps coverage. No data file is committed to the repo.sweep— reverse geocodes every point with LocationIQ and compares against the offline geocoder (reverseMode: 'boundary'):--daily-cap); requests are also rate-limited (default 1/s,--rps). The counter is persisted before each request, so even a crash can only under-spend the cap. On HTTP 429 the run backs off and stops cleanly with a resume message.report.mdranks countries by mismatch rate (point counts, agreement %, country mismatches, 10 worst examples with coordinates and both answers);mismatches.jsonlis the machine-readable list of all mismatches.--dry-runrebuilds all of this from cache with zero network.Usage
Tests
spec/locationiq_sweep_spec.js; the HTTP function is injected, so no spec touches the network. Covered: sampler TSV parsing/top-N/round-robin cap, name normalization (diacritics, non-Latin scripts), county-vs-locality agreement, severe country mismatch, empty-answer classification, daily-cap enforcement + state persistence + resume, cached points never re-queried, UTC-day quota reset, clean 429 stop without caching the failure, dry-run with zero network calls, and report/mismatch-JSONL content.LOCATIONIQ_API_KEYwas not set in the environment.