Add curation overlay mechanism with first entry for Guatemala - #5
Conversation
Generic build rules should fix everything data can fix; curation is a last resort for judgment calls the data cannot know. Overlay files live in curation/ (one JSON file per country) so the community can propose changes via PR, with every entry carrying a rationale and probe coordinates. scripts/apply_curation.js (npm run curate) validates files and place ids before writing, applies merges in a single transaction, and is idempotent by construction. A merge relabels compact_geohash_lookup cells of absorbed places to the target place, but only at geohash length >= minPrecision, so coarse fallback cells keep their original owner. Absorbed places stay in compact_places for forward lookups and old readers. --dry-run previews per-entry counts; --verify runs each entry's probes through the boundary reverse lookup, with --skip-unresolvable to defer probes whose expected place owns no cells yet. First entry (curation/gt.json): in Guatemala City's metro the Carretera a El Salvador corridor (Santa Catarina Pinula, Fraijanes) functions as part of the city, and the municipality of Guatemala carries the bare name "Guatemala", which reads as a country-level label; all three are absorbed into the Guatemala City label at precision >= 5. Mixco and Villa Nueva keep their own identities. The probes describe the intended end state once the world database is rebuilt with county boundaries at precision 5; on the current shipped database the merge is a validated no-op.
![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: e13cde83d5
ℹ️ 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 on the curation overlay and the question of how conflicting community curations are handled: - Validate globally across all loaded curation files before any write. A place id absorbed by two entries is rejected whether the targets conflict or agree (duplicate absorption invites drift when one copy is later edited), and an id that is any entry's merge target may not be absorbed by another entry (merge chains would make the result depend on application order and break idempotence). After validation every lookup row is touched by at most one entry, so application order is deterministic and irrelevant. - Run --verify probes inside the apply transaction on the same connection and roll the overlay back if any probe fails, so a nonzero exit reliably means the database is unchanged. - Derive the reverse lookup's precision range from the geohash lengths present in the database instead of the library defaults, so verification works on databases built outside the 4..7 range. - Base --skip-unresolvable on the entry's inputs, not just the expected label: a failing probe is deferred when the expected place or one of the entry's absorbed places owns no cells yet. This makes the documented ship-before-rebuild workflow actually work when the merge target already owns cells elsewhere, while genuine mismatches on a fully built database still fail even with the flag. - Document the contribution and conflict policy in curation/README.md: one country per file, probes are permanent regression guards, conflicts are rejected as a set, schema compatibility is governed by COMPATIBILITY.md (curation only relabels lookup rows), and the maintainer arbitrates judgment disputes.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e27167fded
ℹ️ 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 the second Codex review round on the curation overlay: - Count only cells an entry can actually relabel when deciding whether a merge source is resolvable: a place owning cells solely below the entry's minPrecision still leaves the merge a no-op for that source, so its failing probes are now deferred under --skip-unresolvable. - Scope the expected-place resolvability check to the entry's country so a homonymous place elsewhere in the world cannot make an expected name look resolvable; warning and hint messages now name the country and the precision floor. - Validate probe lat/lon types before use: null, false, and empty strings coerce to coordinate 0 under Number(), which would let a malformed guard probe run at an unintended location. - Reword the curation README's compatibility reference: COMPATIBILITY.md is introduced by open PR #4, so point at that PR instead of a relative link that does not resolve on this branch.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ab57f8928
ℹ️ 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 the third Codex review round on the curation overlay: - A missing merge source no longer defers every failing probe in its entry: the missing-source reason now applies only to probes expecting the merge target's name. Guard probes expecting any other name fail and roll the transaction back even under --skip-unresolvable, so permanent regression guards keep their teeth. - Expected-place ownership is now evaluated against the post-apply transaction state instead of a pre-apply snapshot: when the pending merge itself grants the target cells, a failing probe expecting the target exposes an incomplete absorb set and is treated as a genuine failure rather than an unresolvable probe. Missing sources are still snapshotted pre-apply, since the apply drains them. - Referenced place ids are validated against the file's declared country: an id that exists but belongs to another country is rejected before anything is written, enforcing the one-country-per-file policy against typo'd WOF ids. The Guatemala file references only GT places and remains valid.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba1bc1883f
ℹ️ 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 the fourth Codex review round on the curation overlay: - Record what each apply drained in a curation_journal bookkeeping table inside the curated database (per target and absorbed place, cumulative relabeled-cell counts, written inside the transaction so a rolled-back apply leaves no trace). "Source owns no relabelable cells" is ambiguous between a build that never had them and an overlay that already consumed them; the journal disambiguates, so re-verification of an already-curated database stays strict and --skip-unresolvable cannot mask later regressions. The apply now updates per source (equivalent to the previous IN-list update, since conflict validation makes sources disjoint) to record the counts. - Enforce that a curation file's name matches its declared country (gt.json must declare GT), closing the hole where applying a file by name could modify a different country than the name implies. - Normalize country id case in the expected-place ownership query with UPPER(), consistently with place validation, so databases storing lowercase country codes cannot turn a genuine mismatch into a deferrable probe.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02c6f6dd4d
ℹ️ 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 the fifth Codex review round on the curation overlay: - Invalidate drain evidence when the compact tables are rebuilt. The journal survived the boundary generator's replace mode, so stale evidence from a previous database generation kept verification strict where it should defer. The apply now maintains an inert marker trigger on compact_geohash_lookup: SQLite drops triggers with their table, so a replace-mode rebuild kills the marker, and journal present + marker gone means the evidence describes a previous generation - the next apply clears it and starts fresh. In-place edits (including the cell corruption verification must stay strict about) never drop the table, which is what a content fingerprint could not distinguish. Solved entirely from the apply tool's side. - Enforce the guard-probe requirement at validation time: every entry must have at least one probe whose expected label differs from the merge target's name, so a same-country typo absorbing an unrelated municipality can no longer pass verification with positive probes only. gt.json already satisfies this via its Mixco and Antigua guards.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62e85d643b
ℹ️ 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 the sixth Codex review round on the curation overlay: - Record every drained cell with its original owner in a curation_journal_cells table, and add --revert, which restores each journaled cell that is still owned by its merge target back to its original owner and clears the journal. This resolves the P1 append conflict from the apply tool's side: the generator's append mode removes a place's old cells by current ownership, which curation has rewritten, so appending onto a curated database would strand cells on the merge target forever. The supported lifecycle for an append-mode refresh is now revert, refresh, re-apply, and is documented; appending without reverting remains unsupported. - Make drain evidence precision-aware: a source counts as previously drained only if journaled cells of length >= the entry's current minPrecision exist, so evidence from an earlier precision-5 revision no longer vouches for a precision-6 revision the database never had cells for. - Require a positive probe in addition to the guard: an entry whose probes are all guards never exercises the intended relabeling, so --verify could commit a merge that changed the wrong cells or none. - Verify positive probes against the merge target's place id, not just its name, so a same-named place in the same country cannot mask an incomplete merge.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c8aa4568b
ℹ️ 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 the seventh Codex review round on the curation overlay: - Make apply a reconciliation to the entry as written: journaled cells below the entry's current minPrecision (drained by an earlier, broader revision) are returned to their original owners in the same apply, so raising minPrecision from 5 to 6 no longer leaves the precision-5 cells silently stuck on the merge target. Cells something else has since overwritten are left alone and their journal records retired; reconciliation is idempotent and reported per entry (dry-run shows the would-be returns too). - Require positive probes to resolve from a lookup cell: verification now runs with reverseDebug and a positive probe only passes when the result came via geohash_lookup, so the nearest-centroid fallback returning the merge target itself for an uncovered coordinate can no longer mask incomplete source coverage. Guard probes still assert the label a user would see, however resolved.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a02c142d7e
ℹ️ 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 the eighth Codex review round on the curation overlay. The per-cell journal now serves as the single definition of "this entry's territory", which simplifies the verification rules rather than adding more special cases: - A failing positive probe is deferred only when it sits on no cell this entry has drained AND a source is unavailable - probes over territory the entry demonstrably absorbed stay strict, so a missing future source can no longer excuse a regression over a present one. - When an entry relabeled cells, at least one passing positive probe must have resolved from those cells; positives sitting only on target-native territory (or cells another entry relabeled) prove nothing about the entry's own effect and now fail verification with a message asking for a probe on the absorbed territory. - Reconciliation now also covers structural revisions: journaled cells whose source is no longer absorbed by any current entry for that target are returned to their original owners, grouped per target so that several entries sharing a target stay safe. Deleting an entire entry remains the one change apply cannot see; documented as --revert then re-apply. - Every probe's expected label must name a place that exists in the entry's country, validated before anything is written: a cell-less place is a deferrable data gap, but a label naming no place is a typo, and deferring it would commit the overlay without an effective guard.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df33289d3
ℹ️ 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".
Data correction, validated against a freshly built world database with county boundaries at precision 5: - Replace the synthesized km-17 probe, which sat on a cell genuinely owned by Villa Canales (a municipality that keeps its own identity). Every gt.json coordinate is now empirically chosen from cells the merge actually drains: Santa Catarina Pinula's two cells and Fraijanes' one, the latter previously having no probe coverage at all. The full entry passes strict --verify with no skips, relabeling 3 cells. - Record in the rationale that the municipality of Guatemala owns no precision-5 cells, because Guatemala City's locality polygon already covers its municipal territory; it stays in absorb as future-proofing. Also address the ninth Codex review round, folding two findings into one reconciliation rule rather than adding cases: - Reconcile any journaled (target, source) pair the loaded files no longer declare, scoped to the countries in the run and performed before the merges are applied. This subsumes dropping a source and now also covers retargeting one to a different `into`, whose cells were parked on the old target and had to be released first. - Require exactly one file per country across all loaded files, so applying one file can never look like another's entries were removed. - Judge a positive probe by the lookup row it actually matched: passing through a cell this entry drained and still owns proves the effect, while a failure on a drained cell stays strict even after that cell is handed elsewhere. A lost curated cell can no longer be masked by a coarser target-owned cell covering the same point. - Preview journal restorations in --dry-run output.
Data correction: Guatemala probes verified against a world buildThe probe coordinates in What the real data says. The merge relabels 3 cells. The absorbed places own exactly:
What changed. The km-17 probe is replaced by two empirically chosen coordinates — one in Santa Catarina Pinula's second cell, one in Fraijanes' cell — so every absorbed municipality that owns territory is now covered by a positive probe. Strict verification (no Re-running against the already-curated database relabels 0 cells and still verifies strictly, exercising the journal on real data. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c98af68ccf
ℹ️ 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".
# Conflicts: # CHANGELOG
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Address the tenth Codex review round. All three findings are the same mistake in different places - reading the database at a moment that no longer describes what the transaction will do - so the fix is to move the measurements rather than add rules: - Compute source availability inside the transaction, after orphan reconciliation and before the merges drain the sources. A retargeted source looks unavailable beforehand only because its cells are still parked on the old target; the stale verdict could excuse an unrelated failing positive probe under --skip-unresolvable. - Count cells a retargeted entry will reclaim in its dry-run total. Reconciliation hands those cells back to the source and the entry immediately drains them, so they belong in the entry's own count; they are no longer double-reported as releases. - Count only orphaned cells the old target still owns. A cell that was deleted or taken over by a third place merely has its journal record retired, which the preview now reflects.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 466e0b8584
ℹ️ 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 the eleventh Codex review round. A positive probe can land on a cell still owned by an absorbed place, which only happens below the entry's minPrecision. Two different situations look identical in the data, and the owning source tells them apart: - that source has relabelable cells elsewhere, so the build already produces this entry's precision and the probe was simply placed on ground the merge deliberately leaves alone. It can never pass, so deferring it hid a broken curation file: it is now always reported, naming the cell, its precision, and the two ways out. - that source has no relabelable cells at all, so the build has not reached this precision here. That is the documented ship-ahead case and stays deferrable. Distinguishing the two matters: keeping only the strict reading would have broken three established deferral scenarios (a raised minPrecision the build has never produced, a replace-mode rebuild that lost the fine cells, and a source owning only coarse cells), all of which the suite caught. The existing missing-source set already encodes the distinction, so no new query or rule was needed.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 859a6a7290
ℹ️ 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".
| ? await probeTerritory(db, entry, probe, boundary) | ||
| : { onDrainedCell: false, throughDrainedCell: false } | ||
|
|
||
| if (actual === probe.expect && idMatches && pathMatches) { |
There was a problem hiding this comment.
When a guard coordinate has no matching compact geohash row, reverse lookup can return the expected place via regional_centroid_fallback; because pathMatches is automatically true for non-positive probes, this condition records the guard as passing. A mistakenly absorbed municipality can therefore have its actual lookup cells relabeled while a guard near its centroid continues returning its name through fallback, defeating the mandatory guard's purpose. Require guard probes, like positive probes, to resolve via geohash_lookup so they exercise ownership that the overlay could change.
Useful? React with 👍 / 👎.
1 participant
Problem
Some labeling problems cannot be solved by generic build rules, because the source data has no way to express them. The first such case: in Guatemala City's metro, the Carretera a El Salvador corridor is considered part of the city by the people who live there, yet administratively it belongs to the municipalities Santa Catarina Pinula and Fraijanes. On top of that, the municipality of Guatemala carries the bare WOF name "Guatemala", which renders like a country-only label in apps. No population threshold, precision cap, or rollup rule can know any of this — it is a judgment call.
At the same time, such judgment calls must not leak: a genuinely middle-of-nowhere point that only matches a coarse fallback cell should keep reading country-ish rather than claiming a city, and neighboring municipalities with their own identities (Mixco, Villa Nueva) must keep their names.
Change
curation/— a new directory of hand-maintained overlay files, one JSON file per country, applied on top of a generated compact boundary database.curation/README.mddocuments the philosophy (curation is a last resort — prefer generalizing the build rules), the file format, and how to apply and verify. Community changes are welcome via PR; every entry must carry arationaleandprobes, including guard probes that pin down where the curation must not reach.scripts/apply_curation.js(npm run curate) — validates every file and every place id before touching anything (malformed JSON, unknown fields, missing ids, and an id appearing as bothintoandabsorball abort with a clear message and nonzero exit), then applies each merge in a single transaction:Coarse cells below
minPrecisionkeep their original owner. Absorbed places stay incompact_places(forward lookups and old readers may reference them). The operation is idempotent by construction.--dry-runreports per-entry affected-row counts without writing;--verifyruns each entry's probes through the library's boundary reverse lookup and fails nonzero on mismatch, with--skip-unresolvableto defer probes whose expected place owns no cells yet.curation/gt.json— the first entry: Guatemala City (421169087) absorbs Santa Catarina Pinula (1108695621), Fraijanes (421185999), and the bare-named municipality of Guatemala (421191461) at precision >= 5 only. Its probes describe the intended end state once the world database is rebuilt with county boundaries indexed at precision 5; on the currently shipped database Santa Catarina Pinula owns no cells, so the merge is a validated no-op there — the mechanism still validates and applies cleanly.Format example
{ "country": "GT", "entries": [ { "op": "merge", "into": 421169087, "absorb": [421191461, 1108695621, 421185999], "minPrecision": 5, "rationale": "The Carretera a El Salvador corridor functions as part of Guatemala City...", "probes": [ { "lat": 14.5330, "lon": -90.4350, "expect": "Guatemala City", "note": "km 18 Carretera a El Salvador" }, { "lat": 14.6339, "lon": -90.6064, "expect": "Mixco", "note": "guard: Mixco stays itself" } ] } ] }Tests
spec/curation_spec.jsbuilds a synthetic compact-v2 database (a locality, two absorbed counties, a bystander county, and a place with no cells) with lookup rows at precisions 4 and 5, and asserts:minPrecisionguard — verified red/green by temporarily removing theLENGTH(geohash) >=clause and watching this spec fail)compact_placesrows are untouched--dry-runwrites nothingintoand absorbed, malformed JSON, badminPrecision, unknown ops, and typo'd field names--verifypasses matching probes, fails nonzero on mismatch, and--skip-unresolvableskips (with a warning) probes whose expected place owns no cellscuration/gt.jsonis structurally validFull suite: 44 specs, 0 failures (31 baseline + 13 new).