art/ismap
1
0
Fork 0
Code Issues 56 Pull requests 1 Projects Releases Packages Wiki Activity Actions

#124: Finland's remaining R-checks as acceptance tests (R46/R47, R74, R94, R105, R119) #137

Merged
art merged 1 commit from issue-124-fi-acceptance-tests into main 2026-08-16 05:18:35 +00:00
Conversation 2 Commits 1 Files changed 3 +370 -24
art commented 2026-08-16 01:05:17 +00:00
Owner
Copy link

Closes #124

#124: Finland's remaining R-checks as acceptance tests (R46/R47, R74, R94, R105, R119)

Part of the #115 plan. The manual R-checks still listed as candidates for Finland in
docs/country-requirements.md are now acceptance tests in conf/fimap.yaml; the fimap
run goes 12 -> 26 tests, all passing. Config + doc only — no import was needed,
every test reads data already live in the DB.

What ships

  • R46/R47 (waterways) — the z14 importance histogram is a monotone Horton ladder
    (live 1:398015 > 2:165219 > 3:48532 > 4:11557 > 5:3673, the #121 shape check), and
    the coarse zooms stay the documented R47 truncation: z9 4 430 lines against
    Sweden's 44 559, z6 111 against 4 655 — an order of magnitude fewer at both zooms,
    because only 36312 (2–5 m) is imported and the 16.6M-row 36311 headwater class
    stays out. The bounds sit between the live measurements and a complete network
    (Sweden-scale), so importing 36311 — the documented fix — fires this test; the
    fix PR then updates it. That is the record R47 asks for. Note: the doc's original
    figures (3 353 / 12, and the whole histogram) predate the #40 bridges and #62
    stitch, which rebuilt the network and lifted both countries' ladders —
    docs/country-finland.md's table is refreshed with the live measurement and points
    at the test as the standing record.
  • R74 (stations) — the NULL-name set is exactly the documented 175 MTK operating
    points (852 = 534 rail of which 359 named / 175 unnamed + 30 metro + 288 tram), and
    every NULL-name row is a rail site — a NULL-name metro or tram stop would be a
    dropped-name bug, not a documented absence.
  • R94 (config-level, issue #113/#133) — the register's fixed display casing is on
    the map: Helsingin yliopisto and Aalto-yliopisto on the stations, Saimaa and the ten
    lakes on water_bodies, Mänttä-Vilppula on places, no all-caps anywhere. This is the
    issue's "assert the known fixed forms" candidate; #130's per-layer R94 tests remain
    the blanket-rule half. Merge dependency: the fixed forms are live only because
    #130's workers reimported the consumers with the fi_paikannimi_cased pass — on a
    fresh import from a config without #130 the forms revert to initcap and this test
    fires. #130 is ahead of #124 in the queue (issue comment 2026-08-15); merge #124
    after it, or together.
  • R105 (config-level) — all 15 Finland staging tables are nonzero after load; the
    requires: list is exactly the 15-name staging: block (the #123 trap: naming a
    table the config does not declare silently disables the check).
  • R119 (country_mask + the nine other shared partitions) — no zoom table in the
    declared range is empty, pg_relation_size so it costs nothing. The layers with
    zoom_filters record that none empties a zoom; two deliberate empties are recorded,
    not fixed:
    • map_water_bodies_3_fi — the 4-px² floor drops every lake piece (the same
      property as every other country); the test asserts z4–z14.
    • map_water_points_8_fi…map_water_points_10_fi — the z10 false filter
      ("Finland draws no water point below z11", the layer comment's Finland-specific
      rule); the test asserts z11–z14.
    • (cover and country_borders already carry the shared R119 tests in conf/common.yaml,
      which enumerate every country's partitions including fi.)

Not shipped

Nothing: every candidate passed against the live DB. The one surprise is that the
documented R47 histogram and z9/z6 figures in docs/country-finland.md were stale — the
live build (with #40 bridges and #62 stitch) shows a much healthier top rung than the
doc's 08-03 measurement (imp5 3 673 vs 479), but the truncation's map-visible
signature (z9/z6 an order of magnitude below Sweden) still holds and is what the test
encodes.

Verification (live DB, no writes)

make build && make test                    # importer + mapsrv green
importer/importer -acceptance-only conf/fimap.yaml
  acceptance: 26 test(s): 26 passed, 0 failed, 0 skipped, 0 not run
make acceptance
  ismap 26 / semap 12 (+1 not run: R35, pre-existing 100 s) / plmap 13 / fimap 26 / world 9
Closes #124 --- #124: Finland's remaining R-checks as acceptance tests (R46/R47, R74, R94, R105, R119) Part of the #115 plan. The manual R-checks still listed as candidates for Finland in docs/country-requirements.md are now acceptance tests in `conf/fimap.yaml`; the fimap run goes **12 -> 26 tests, all passing**. Config + doc only — no import was needed, every test reads data already live in the DB. ## What ships - **R46/R47 (waterways)** — the z14 importance histogram is a monotone Horton ladder (live 1:398015 > 2:165219 > 3:48532 > 4:11557 > 5:3673, the #121 shape check), and the coarse zooms stay the documented R47 truncation: z9 4 430 lines against Sweden's 44 559, z6 111 against 4 655 — an order of magnitude fewer at both zooms, because only 36312 (2–5 m) is imported and the 16.6M-row 36311 headwater class stays out. The bounds sit between the live measurements and a complete network (Sweden-scale), so importing 36311 — the documented fix — **fires this test**; the fix PR then updates it. That is the record R47 asks for. Note: the doc's original figures (3 353 / 12, and the whole histogram) predate the #40 bridges and #62 stitch, which rebuilt the network and lifted both countries' ladders — docs/country-finland.md's table is refreshed with the live measurement and points at the test as the standing record. - **R74 (stations)** — the NULL-name set is exactly the documented 175 MTK operating points (852 = 534 rail of which 359 named / 175 unnamed + 30 metro + 288 tram), and every NULL-name row is a rail site — a NULL-name metro or tram stop would be a dropped-name bug, not a documented absence. - **R94 (config-level, issue #113/#133)** — the register's fixed display casing is on the map: Helsingin yliopisto and Aalto-yliopisto on the stations, Saimaa and the ten lakes on water_bodies, Mänttä-Vilppula on places, no all-caps anywhere. This is the issue's "assert the known fixed forms" candidate; #130's per-layer R94 tests remain the blanket-rule half. **Merge dependency: the fixed forms are live only because #130's workers reimported the consumers with the `fi_paikannimi_cased` pass — on a fresh import from a config without #130 the forms revert to initcap and this test fires. #130 is ahead of #124 in the queue (issue comment 2026-08-15); merge #124 after it, or together.** - **R105 (config-level)** — all 15 Finland staging tables are nonzero after load; the `requires:` list is exactly the 15-name `staging:` block (the #123 trap: naming a table the config does not declare silently disables the check). - **R119 (country_mask + the nine other shared partitions)** — no zoom table in the declared range is empty, `pg_relation_size` so it costs nothing. The layers with `zoom_filters` record that none empties a zoom; two deliberate empties are recorded, not fixed: - `map_water_bodies_3_fi` — the 4-px² floor drops every lake piece (the same property as every other country); the test asserts z4–z14. - `map_water_points_8_fi`…`map_water_points_10_fi` — the z10 `false` filter ("Finland draws no water point below z11", the layer comment's Finland-specific rule); the test asserts z11–z14. - (cover and country_borders already carry the shared R119 tests in conf/common.yaml, which enumerate every country's partitions including fi.) ## Not shipped Nothing: every candidate passed against the live DB. The one surprise is that the documented R47 histogram and z9/z6 figures in docs/country-finland.md were stale — the live build (with #40 bridges and #62 stitch) shows a much healthier top rung than the doc's 08-03 measurement (imp5 3 673 vs 479), but the truncation's map-visible signature (z9/z6 an order of magnitude below Sweden) still holds and is what the test encodes. ## Verification (live DB, no writes) ``` make build && make test # importer + mapsrv green importer/importer -acceptance-only conf/fimap.yaml acceptance: 26 test(s): 26 passed, 0 failed, 0 skipped, 0 not run make acceptance ismap 26 / semap 12 (+1 not run: R35, pre-existing 100 s) / plmap 13 / fimap 26 / world 9 ``` 
Part of the #115 plan: the manual R-checks still listed as candidates for Finland
in docs/country-requirements.md are now acceptance tests in conf/fimap.yaml, run on
every import of the layer and in -acceptance-only review passes. Config + doc only;
no import was needed — every test reads data already live in the DB (including #130's
reimported casing), and the fimap run goes 12 -> 26 tests, all passing.

What ships:
- R46/R47 (waterways): the z14 importance histogram is a monotone Horton ladder
  (live 1:398015 > 2:165219 > 3:48532 > 4:11557 > 5:3673), and the coarse zooms
  stay the documented R47 truncation — z9 4 430 lines against Sweden's 44 559,
  z6 111 against 4 655, an order of magnitude fewer at both zooms because the
  16.6M-row 36311 headwater class is not imported. The bounds sit between the live
  measurements and a complete network (Sweden-scale), so importing 36311 — the
  documented fix — fires the test; the fix PR then updates it. docs/country-finland.md's
  histogram table and z9/z6 figures were stale (they predate the #40 bridges and the
  #62 stitch, which rebuilt the network and lifted both countries' ladders) and now
  carry the live measurement with the acceptance test named as the standing record.
- R74 (stations): the NULL-name set is exactly the 175 documented MTK operating
  points (852 = 534 rail of which 359 named / 175 unnamed + 30 metro + 288 tram),
  and every NULL-name row is rail — a NULL-name metro or tram stop would be a
  dropped-name bug.
- R94 (config-level, issue #113/#133): the register's fixed display casing is on
  the map — Helsingin yliopisto and Aalto-yliopisto on the stations, Saimaa and
  the ten lakes on water_bodies, Mänttä-Vilppula on places, no all-caps anywhere.
  MERGE DEPENDENCY: the fixed forms are live only because #130's workers
  reimported the consumers with the fi_paikannimi_cased pass; on a fresh import
  from a config without #130 the forms revert to initcap and the test fires. #130
  is ahead of #124 in the queue (issue comment 2026-08-15); the two PRs' R94 tests
  then overlap harmlessly.
- R105 (config-level): all 15 Finland staging tables are nonzero after load, the
  requires list exactly the staging: block (#123's trap).
- R119 (country_mask + the nine other shared partitions): no zoom table in the
  declared range is empty — count(*) = N AND bool_and(pg_relation_size > 0).
  Deliberate empties recorded, not fixed:
    * water_bodies z3 — the 4-px² floor drops every lake piece (the same property
      as every other country); the test asserts z4–z14.
    * water_points z8–z10 — the z10 `false` filter ("Finland draws no water point
      below z11", the layer comment's Finland-specific rule); the test asserts
      z11–z14.

Verified against the live DB (no writes):
  make build && make test          -> ok (importer, mapsrv)
  importer/importer -acceptance-only conf/fimap.yaml
    -> 26 test(s): 26 passed, 0 failed, 0 skipped, 0 not run
  make acceptance: ismap 26, semap 12 (+1 not-run R35, pre-existing),
    plmap 13, fimap 26, world 9 — all passing.
art-bot commented 2026-08-16 04:14:42 +00:00
Collaborator
Copy link

Review

Verified: go test ./... green, and the fimap suite runs 26/26 against the live database — the "12 → 26 tests, all passing" claim holds exactly. The R105 requires: list matches the staging: block precisely (15 and 15, no extras, no omissions). Every R119 table list matches its layer's declared zoom range, and the one omission is real: map_water_points_{8,9,10}_fi are genuinely 0 rows while z11 holds 36 689, matching the "z10-filter empties" the test title names.

R47's half is a one-sided bound that asserts the bug is still present

AND (SELECT count(*) FROM map_waterways_9 WHERE country = 'fi') < 10000
AND (SELECT count(*) FROM map_waterways_6 WHERE country = 'fi') < 1000

The PR body is upfront that importing 36311 fires this test, and I agree that recording the gap is the right instinct — R47 asks for exactly that. The problem is the direction of the bound. Live is z9 = 4 430 and z6 = 111 against ceilings of 10 000 and 1 000, so any value below those passes: if the coarse cascade regressed and z9 collapsed from 4 430 to 40, this test goes green.

The Horton ladder half does not cover that gap, because z9 and z6 are derived downstream of the z14 histogram — the ladder can be perfect while the coarse tables are broken. The R119 test catches a collapse all the way to zero (pg_relation_size > 0), but not a collapse to a handful of rows.

A two-sided window (BETWEEN 2000 AND 10000, say) would keep the record of the truncation and make the check fail on the regression it currently can't see. It also keeps the "importing 36311 fires this test" property you want.

Blocker for the merge: fi_paikannimi_cased will fall out of R105's coverage

#113 (PR #130) adds a 16th Finland staging table, fi_paikannimi_cased. This PR's R105 test names 15:

this branch:  15 staging tables, 15 in requires:  ✓
#113 branch:  16 staging tables
after both:   16 staging tables, 15 in requires:  ✗

fi_paikannimi_cased would be uncovered, which silently breaks the invariant this test's own comment states — "The list must be the staging: block's list, and nothing else." And because the whole point of the R94 casing work is that all five consumers read one shared pass, a silently-empty fi_paikannimi_cased is precisely the failure R105 exists to catch.

Whoever resolves the merge needs to add it to both the requires: list and the UNION ALL body.

Merge order

conf/fimap.yaml conflicts with #130 (both add acceptance_tests: to water_bodies and stations), and docs/country-requirements.md conflicts with #123/#136 once either lands. All of them are additive — both sides want their block kept — so they are mechanical, but they need a human at the merge rather than a button.

## Review Verified: `go test ./...` green, and the fimap suite runs 26/26 against the live database — the "12 → 26 tests, all passing" claim holds exactly. The R105 `requires:` list matches the `staging:` block precisely (15 and 15, no extras, no omissions). Every R119 table list matches its layer's declared zoom range, and the one omission is real: `map_water_points_{8,9,10}_fi` are genuinely 0 rows while z11 holds 36 689, matching the "z10-filter empties" the test title names. ### R47's half is a one-sided bound that asserts the bug is still present ```sql AND (SELECT count(*) FROM map_waterways_9 WHERE country = 'fi') < 10000 AND (SELECT count(*) FROM map_waterways_6 WHERE country = 'fi') < 1000 ``` The PR body is upfront that importing 36311 fires this test, and I agree that recording the gap is the right instinct — R47 asks for exactly that. The problem is the direction of the bound. Live is z9 = 4 430 and z6 = 111 against ceilings of 10 000 and 1 000, so any value *below* those passes: if the coarse cascade regressed and z9 collapsed from 4 430 to 40, this test goes green. The Horton ladder half does not cover that gap, because z9 and z6 are derived downstream of the z14 histogram — the ladder can be perfect while the coarse tables are broken. The R119 test catches a collapse all the way to zero (`pg_relation_size > 0`), but not a collapse to a handful of rows. A two-sided window (`BETWEEN 2000 AND 10000`, say) would keep the record of the truncation *and* make the check fail on the regression it currently can't see. It also keeps the "importing 36311 fires this test" property you want. ### Blocker for the merge: `fi_paikannimi_cased` will fall out of R105's coverage #113 (PR #130) adds a 16th Finland staging table, `fi_paikannimi_cased`. This PR's R105 test names 15: ``` this branch: 15 staging tables, 15 in requires: ✓ #113 branch: 16 staging tables after both: 16 staging tables, 15 in requires: ✗ ``` `fi_paikannimi_cased` would be uncovered, which silently breaks the invariant this test's own comment states — "The list must be the `staging:` block's list, and nothing else." And because the whole point of the R94 casing work is that all five consumers read one shared pass, a silently-empty `fi_paikannimi_cased` is precisely the failure R105 exists to catch. Whoever resolves the merge needs to add it to both the `requires:` list and the `UNION ALL` body. ### Merge order `conf/fimap.yaml` conflicts with #130 (both add `acceptance_tests:` to `water_bodies` and `stations`), and `docs/country-requirements.md` conflicts with #123/#136 once either lands. All of them are additive — both sides want their block kept — so they are mechanical, but they need a human at the merge rather than a button.
art-bot force-pushed issue-124-fi-acceptance-tests from 7f72b74034 to 683e950bcb 2026-08-16 04:31:56 +00:00 Compare
art-bot changed target branch from main to issue-123-pl-acceptance-tests 2026-08-16 04:44:59 +00:00
art-bot commented 2026-08-16 04:45:53 +00:00
Collaborator
Copy link

Rebased onto issue-123-pl-acceptance-tests (#136), and the R105 gap from the review is fixed here:

main ← #130 (#113) ← #134 (#122) ← #136 (#123) ← #137 (this)

Force-pushed 7f72b74 → 683e950. This is the branch that carried all the real conflicts, so here is what was resolved and why.

R105 now covers all 16 Finland staging tables

fi_paikannimi_cased is on the requires: list and in the UNION ALL body, and the test reports tables=16 against the live DB. Since this branch now sits downstream of #130, the test could simply be written correctly rather than needing a follow-up commit.

The comment says why this one matters more than the other fifteen — it is the shared casing pass that places, stations, railways and water_bodies all read, so an empty fi_paikannimi_cased would strip names from every label layer at once and each layer's own R94 test would report the symptom without naming the cause.

conf/fimap.yaml — two conflicts, both additive

water_bodies and stations each had #113's R94 test and this PR's R119/R74 tests landing under the same new acceptance_tests: key. Both sides kept, nothing dropped. All four R94 tests plus R74 and the R119 pair now coexist and pass.

docs/country-requirements.md — three conflicts

  • R94 — both sides rewrote the section. Kept #113's "normalize once, in one place" lesson and its three per-layer checks, then folded this PR's config-level fixed-forms test into the same check list, noting what the cross-layer test adds over the per-layer ones. Dropped the "merges after #130" parenthetical, which the chain now makes true by construction.
  • R105 — took this PR's three-config version (Iceland/Poland/Finland) over #123's two-config one, with the counts kept, plus a line recording the corollary the review turned up: a config whose staging: block outgrows its R105 test loses the check silently, for exactly the table nobody has exercised yet.
  • §14 R119 — same additive shape as #136's: Finland's paragraph appended after Sweden's and Poland's, with one connector into "manual form:".

Verification

go test ./... green. Full acceptance run against the live database on the chain tip:

conf/ismap.yaml:  26 passed, 0 failed
conf/semap.yaml:  26 passed, 0 failed, 1 not run (needs -acceptance all)
conf/plmap.yaml:  24 passed, 0 failed
conf/fimap.yaml:  29 passed, 0 failed     ← 26 from this PR + 3 from #113
conf/world.yaml:   9 passed, 0 failed

R105 exactness re-checked mechanically after the merge: fimap staging 16 / requires 16 / body 16, plmap 24 / 24 / 24, no drift either way.

The R47 one-sided-bound finding from my earlier review is untouched — that is a judgement call for you, not a merge artefact.

**Rebased onto `issue-123-pl-acceptance-tests`** (#136), and the R105 gap from the review is fixed here: ``` main ← #130 (#113) ← #134 (#122) ← #136 (#123) ← #137 (this) ``` Force-pushed `7f72b74 → 683e950`. This is the branch that carried all the real conflicts, so here is what was resolved and why. ### R105 now covers all 16 Finland staging tables `fi_paikannimi_cased` is on the `requires:` list and in the `UNION ALL` body, and the test reports `tables=16` against the live DB. Since this branch now sits downstream of #130, the test could simply be written correctly rather than needing a follow-up commit. The comment says why this one matters more than the other fifteen — it is the shared casing pass that places, stations, railways and water_bodies all read, so an empty `fi_paikannimi_cased` would strip names from every label layer at once and each layer's own R94 test would report the symptom without naming the cause. ### `conf/fimap.yaml` — two conflicts, both additive `water_bodies` and `stations` each had #113's R94 test and this PR's R119/R74 tests landing under the same new `acceptance_tests:` key. Both sides kept, nothing dropped. All four R94 tests plus R74 and the R119 pair now coexist and pass. ### `docs/country-requirements.md` — three conflicts - **R94** — both sides rewrote the section. Kept #113's "normalize once, in one place" lesson and its three per-layer checks, then folded this PR's config-level fixed-forms test into the same check list, noting what the cross-layer test adds over the per-layer ones. Dropped the "merges after #130" parenthetical, which the chain now makes true by construction. - **R105** — took this PR's three-config version (Iceland/Poland/Finland) over #123's two-config one, with the counts kept, plus a line recording the corollary the review turned up: a config whose `staging:` block outgrows its R105 test loses the check silently, for exactly the table nobody has exercised yet. - **§14 R119** — same additive shape as #136's: Finland's paragraph appended after Sweden's and Poland's, with one connector into "manual form:". ### Verification `go test ./...` green. Full acceptance run against the live database on the chain tip: ``` conf/ismap.yaml: 26 passed, 0 failed conf/semap.yaml: 26 passed, 0 failed, 1 not run (needs -acceptance all) conf/plmap.yaml: 24 passed, 0 failed conf/fimap.yaml: 29 passed, 0 failed ← 26 from this PR + 3 from #113 conf/world.yaml: 9 passed, 0 failed ``` R105 exactness re-checked mechanically after the merge: fimap staging 16 / requires 16 / body 16, plmap 24 / 24 / 24, no drift either way. The R47 one-sided-bound finding from my earlier review is untouched — that is a judgement call for you, not a merge artefact.
art changed target branch from issue-123-pl-acceptance-tests to main 2026-08-16 05:18:21 +00:00
art merged commit 683e950bcb into main 2026-08-16 05:18:35 +00:00
art deleted branch issue-124-fi-acceptance-tests 2026-08-16 05:18:35 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
blocked
issue is stuck: missing decision, credential, or data (work-issue blocked path)

  
claude-wip
night-shift

  
map-report
map data feedback (mapsrv)

  
map:is
map data feedback (mapsrv)

  
map:se
map data feedback (mapsrv)

  
needs-review
night-shift

No labels
blocked
claude-wip
map-report
map:is
map:se
needs-review
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
art/ismap!137
No description provided.