No reviewers
Labels
No labels
blocked
claude-wip
map-report
map:is
map:se
needs-review
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
art/ismap!189
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "gr-roads-rail-waterways"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #179
Closes #182
Greece transport + water layers: roads (TEN-T), railways + stations, waterways — #179 #180 #182
What this does
Implements #179 (GAIAOSE rail + stations), #180 (TEN-T roads) and #182 (waterways
district gap) as one branch, because the three share one importer change and
one country config batch.
Importer (commit 1, reviewable on its own):
importer/geojson.go— a GeoJSON staging loader (all six geometry types,properties as text columns). The two Greek WFS services serve GeoJSON in
EPSG:2100 with no crs member, so
source_sridis a config decision.importer/staging.go— PolyLine (+Z/M) support in the shapefile loader; itread Point and Polygon only, and TEN-T roads are a PolyLine shapefile.
importer/arcgis.go— thearcgis:provider: a paged ArcGIS RESTFeatureServer query (resultOffset pages, concatenated into one GeoJSON file
per order). Anonymous like
downloads:; existing files are reused,--refetchforces a re-query. Wired into the config merge, the import fetchphase, and
fetch/list+ their usage docs. This is the "fetch shape theimporter does not have yet" that #179 recorded as known gap 3.
Greece layers (commit 2):
waterways(#182) — floods_rev1el##_hntw× 10 +el##_hntz× 4.The #182 "are the missing districts published elsewhere?" check has a real
finding: the four districts without a dense hntw network (EL06/07/11/14) DO
publish the named main-river
hntztier, in the same workspace, coincidingwith hntw within ~100 m where both exist. So the layer fills the four
districts instead of leaving them empty, which is the exact hole #182 exists
to prevent. Names: hntz carries
name_el; hntw joins the nearest namedriver water body (
gr_rwb,wb_name_el— the catalogue's promised R92 join;92 of its 1 415 lines carry no name, and testing that only after the nearest
one is picked throws away the named river just behind it).
Strahler over the union, rooted at the coast (
gr_coastderived staging —MakeValid-then-Union, because the plain union fails with the GEOS
side-location conflict the #14 landmine warned about), bridged through the
73 lakes (
gr_lwb).roads(#180) — the TEN-T tier: Core → motorway, Core-upgraded → trunk,Comprehensive (± to be upgraded) → primary. The four kept classes are a
whitelist, so New Construction and anything unrecognised are dropped rather
than falling through to
primary. Options 2/3 quickcheck: a data.europa.eu sweep (2026-08-21) found no national ΔΜΕΟ file —
only municipal/regional clips (Syros, Patras, Rethymno, Crete, the ΔΟΥ
network) — and the region geoservers are not uniformly public (Crete's is
the verified exception), so 13-region assembly is 13 schema reconciliations
on endpoints that mostly need discovering first, and the national-file ask
cannot be implemented today. Option 1 is the layer; 2/3 stay the documented
upgrade path. The shared R25 test now exempts gr with a written note
(TEN-T has no level column at all — not a mapping gap). The exemption is
written as
country = 'gr' AND levelled = 0, so it expires by itself the dayGreece has any level at all instead of standing over an unmapped column.
railways+stations(#179) — GAIAOSE Railnet through the new provider.rail_class from the source's own M/B taxonomy (main/branch; no siding in the
source — Finland's mirror case), disused (in_use=0) → siding so abandoned
track draws only from z11, electrified NULL (the source has no such
attribute; a 0/1 would lie about a partly-electrified network). Stations:
436 named (7 of 443 rows are unnamed debris, dropped), importance cut from
GAIAOSE's own 0/1/3/4 code onto the shared 1–3 ladder (0 → 3 = the three
unranked termini; 3/4 → 2; 1 → 1 = Προαστιακός halts), names as delivered
(ALL CAPS Greek — initcap strips the tonos accents).
conf/mapsrv.yaml
maps:entry).Review fixes (commits 3 and 4, from the review of this PR):
importer/sharedtree.go— the data tree is shared between two accounts, andevery file this branch fetched landed 0600, so an import from the other
account died on a permission denied. Not a Greece bug: all six providers used
os.CreateTemp, which hardcodes 0600 with no way to widen it, andos.MkdirAll(dir, 0o755), whose mode the umask masks further. One place nowcreates files under
data/and chmods after creating — additive,setgid-preserving, tested under
umask 0077.a single-part PolyLine came out as
LINESTRING((x y,x y))(not WKT — TEN-T isall multipart, so it never fired); the GeoJSON loader took its column order
from a Go map, i.e. a different staging schema on every load; and the
arcgis:provider asserted types on decoded JSON (a panic on a servicewithout
maxRecordCount, or on ArcGIS's HTTP-200 error envelope), paged atthe service's
maxRecordCountwhen the layer may cap harder, and couldnot terminate against a service that ignores
resultOffset.geojson_test.go,arcgis_test.go,sharedtree_test.go—which the project's own precedent (
gml_test.go,clms_test.go) called forand which catch every defect above.
(see above), and R11/R182 asserted staging-table counts that no damage to the
layer can disturb — it compares the partition against the sources now.
name, so the 7 whitespace-only GAIAOSE rows put empty labels on 38 rail
segments; filtering inside the subquery fixes both halves. Same shape in the
waterways name join.
lines);
fetch-grfetched 2 of Greece's 14 sources and now runsimporter fetch conf/gr(which coversdownloads:too);base.yamlstillcalled roads and rail documented absences; two comments described a district
stamp that nothing does;
docs/country-gr.mdhad thetransitrow twice.The one open question (unchanged, #179)
GAIAOSE asserts no licence on its portal item (R2). The layers are built and
verified; the go-live call on GAIAOSE as the source waits on a written answer
from GAIAOSE, and the CC-BY-3.0 geodata.gov.gr copy is the documented fallback.
Verification (live database)
importer config-check conf/gr— OK, 6 layers / 39 tests.Full gr import (repeatedly during development; final state):
per-zoom row counts — waterways 16 @ z3 → 10 581 @ z14, roads 18 @ z5 → 529 @
z14, railways 59 @ z3 → 9 603 @ z12, stations 3 @ z9 → 436 @ z14; no NULL
importance/class anywhere; 5 875 named waterway segments, 0 blank names, and 0
blank-named rail segments.
Acceptance (
--acceptance all):The new tests: R11/R182 district coverage (all 14 districts reach the partition
— 10 581 layer rows against 10 581 source rows, with per-district counts in the
diagnostic), R46/R47 Horton histogram (1:6373 > 2:2543 > 3:1195 > 4:380 > 5:90),
R46 shared contract, R119 ×4, R137 roads partition vocabulary and a second
R137 asking the source whether its five-value class vocabulary still holds,
R137 rail band/class mix, R75 station ladder, R105 (22 staging tables), plus the
untouched mask/cover suite.
Repo-wide
make acceptance-all: is 38/38, se 45/45, fi 49/49, gr 39/39,world 28/28. pl 45/46 — the one failure is the pre-existing marginal
timeout on "layer buildings (pl) / R120: every Poland source part becomes a
classified footprint" (10 s, open issue "Poland buildings R120 acceptance test
is a marginal timeout on a healthy query"), unrelated to this branch: it
queries pl_bubd/map_buildings, which nothing here touches (6 s standalone, ~10 s
under load).
make testandmake check-queriespass;importer tiles --check conf/grpasses decoded tiles for all four new layers (z3/z14, z5/z14, z3/z12, z9/z14).
Real-browser renders (scripts/cdp-map-check.mjs, real time, no virtual time),
re-measured after the review fixes. The viewports are written down this time so
the numbers can be reproduced; the count is distinct rendered features per
source layer, so it is not directly comparable with the first pass's figures:
#z/lat/lon6/38.8/22.59/39.5/22.611/37.98/23.738/35.24/24.8010/40.64/22.94Screenshot review confirms the layers and no broken geometry: at z6 the TEN-T
network reads as Egnatia plus the Athens–Thessaloniki corridor over the cover
with Crete present, and at z11 Athens all nine surviving station dots carry a
legible Greek label with its tonos intact (Σ.ΣΤΑΘΜΟΣ ΠΕΛΟΠΟΝΝΗΣΟΥ, Σ.ΣΤΑΘΜΟΣ
ΡΟΥΦ, ΑΓΙΟΙ ΑΝΑΡΓΥΡΟΙ, …) — which is also the blank-label fix visible on the
map. One console error on the Greece page is worth naming so it is
not mistaken for a defect:
GET /api/issues?map=grreturns400 unknown map "gr"because the running mapsrv predates this branch'sconf/mapsrv.yamlchange —
is/se/pl/fiall return 200 on the same instance, andgrjoins them once mapsrv is restarted. (The other,
404 /favicon.ico, is on everymap page.)
What is deliberately not here
water_bodies(its sources gr_rwb/gr_lwb are staged and read by waterways;the layer itself is the next Greece item — known gap 3 in docs/country-gr.md).
layer here is the national rail set; #179's R69 half is separate).
data/ is shared between the art and claude accounts through the shared-data group, and a source file one account fetches has to stay readable and writable by the other. It was not: all 18 files this branch adds landed 0600, and the next import from the other account died on a permission denied that reads like a fetch bug. Not a Greece problem — every provider had the same two lines, os.MkdirAll(dir, 0o755) os.CreateTemp(dir, ".<prefix>-*.tmp") and os.CreateTemp hardcodes 0600 with no way to ask for anything else: a umask can only narrow it. MkdirAll is the same trap one level up, where the mode argument IS masked, so 0775 lands 0755 and the other account cannot even create a sibling file. The Makefile has carried a `umask 002 &&` in front of one curl for exactly this reason, which fixes one file and no others. So importer/sharedtree.go becomes the only place that creates a file under data/, and it chmods after creating rather than hoping: additive (never removes a permission anyone opened up on purpose), setgid-preserving (the shared tree needs it to keep new files in the group), and applied to the temp file before a byte is streamed, so a failure costs nothing and is loud. All six providers, the zip extractor and the fetchmeta writer go through it. The tests run under umask 0077 — the one that would strip every group bit — because a mode that only holds under a friendly umask is the bug, not the fix. Then the three defects in this branch's new code that its own sources happened not to trigger: - esriPolyLineToEWKT wrapped writeRing's parentheses a second time, so a single-part PolyLine came out as LINESTRING((x y,x y)) — not WKT at all; PostGIS answers "parse error - invalid geometry" and finalizeStaging's cast takes the whole staging table down. TEN-T never tripped it because all five of its features are multipart (3 to 317 parts). One part per feature is the ordinary line shapefile. Degenerate one-point parts are dropped now too, rather than emitted as an invalid ring. - The GeoJSON loader read its column order out of a Go map, which randomises iteration: the staging schema came out in a different order on every load, against a doc comment promising first-appearance order. buildLoad fixes the ingest schema from the FIRST file of a glob and reuses it for the rest, so the obvious refactor of the 14 near-identical floods staging entries into one glob: would have COPYied file 2..n's values under file 1's random column names. The order now comes off the raw JSON as a token scan. Also: property strings are NUL-sanitised (a \u0000 escape is legal JSON and fatal to a COPY — the sanitizeDBFText case, now shared as sanitizeCopyText), two keys differing only in case are reported instead of colliding into one column, and empty or one-point coordinate arrays are named as errors instead of becoming LINESTRING() and POLYGON(). - The arcgis: provider asserted types on decoded JSON — svc["maxRecordCount"] .(float64) panics on a service that does not advertise it, and on the {"error":{...}} envelope ArcGIS returns under HTTP 200 for a token failure. It also paged at the SERVICE's maxRecordCount when the LAYER may cap harder (service 2000, layer 1000 reads one short page, calls it the end of the layer and writes half a network that ensureArcGIS then caches forever), and used len(features) < page as its only stop condition, which never becomes true on a service that ignores resultOffset — an infinite loop that grows until the process dies. exceededTransferLimit decides now, with the short-page test as fallback, an explicit refusal when the layer says supportsPagination false, and a page cap behind both. `importer fetch` grows a downloads provider, because it claimed to fetch "every provider the config declares" while skipping the static downloads: half — which is how a make fetch-* target ends up topping it up by hand and falling behind a config that grew a source. geojson_test.go, arcgis_test.go, sharedtree_test.go and the PolyLine cases are new; the project already tests a loader (gml_test.go) and a provider (clms_test.go), and every defect above is one those tests would have caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UGAZTzU6FGJSQKj437rr3LReview fixes to the Greece batch. Every one of them is a case where the config said something the SQL did not do. THE TESTS THAT COULD NOT FAIL. R25's Greece exemption was `OR country = 'gr'` — permanent, while its own comment claimed the test "is what notices an unmapped column in a partition that is supposed to have one". It would not have: when the ΔΜΕΟ network replaces TEN-T and its level column goes unmapped, a permanent exemption stays green over exactly that bug. `OR (country = 'gr' AND levelled = 0)` expires by itself — the day Greece has any level at all, the test fails and the exemption gets deleted. R11/R182 was named "the waterways layer covers all 14 river basin districts" and asserted that 14 staging tables are nonzero — which is character-for-character what R105 already asserts over the same 14 tables, and which no amount of damage to the layer can disturb. Delete a UNION branch from the query, point two branches at the same table: every staging table is exactly as full as before and the test passes. It now compares the partition against the sources: at z14 nothing is dropped or merged, so the row count is exactly ST_Dump's yield across the 14 tables (10 581), and a missing branch shows up as a shortfall the size of the district. The diagnostic prints per-district counts. Roads had the same shape one level down. `WHERE ten_t_en <> 'Comprehensive Network New Construction'` with `ELSE 'primary'` closing the CASE fails OPEN: any change to that string — DBF padding, a re-wording — and unbuilt motorway is drawn as a national road, while R137's "exactly three classes" still holds, because three classes are still three classes. The filter is a whitelist of the four kept classes now, so an unrecognised value is dropped rather than guessed at, and a new test asks the SOURCE whether its five-value vocabulary is still the five values the mapping was written against — the only place that question can be answered. Stations' importance CASE lost its ELSE for the same reason: a fifth GAIAOSE code lands NULL and is caught, instead of being filed under 2 with the 369 regular stations. THE 38 BLANK LABELS. The railways name join picked the nearest station and only then discarded a blank name, so the 7 whitespace-only GAIAOSE station rows — the same 7 the stations layer drops outright — put an empty-string label on 38 rail segments, and took the named station just behind them out of the running. Filtering inside the subquery fixes both halves: 0 blank names now. The waterways name join had it too, against 92 blank-named river water bodies; with the filter the layer names 5 875 segments instead of 5 827. TEN COPIES OF ONE JOIN. The waterways query repeated the same eight-line LATERAL nearest-name subquery once per district, 120 of the file's 236 lines. Any change to that join had to be made ten times and could be made to nine — and the district test, as it stood, could not see the difference. One join over the union of the ten tables; the file is 188 lines. Also: the R119/R180 diagnostics printed as `lon_min=&{2023 -2 2}` because round(x::numeric, 2) needs the ::float8 the other configs all use — a failure diagnostic nobody can read is not one. `fetch-gr` fetched the mask GML and CORINE and none of the twelve sources this branch added, so it goes through `importer fetch conf/gr`, which now covers downloads: too. base.yaml still listed roads and rail as documented absences with "no official, non-OSM, permissive national geometry for them", next to the two layer files that disprove it. downloads.yaml and staging.yaml both described a district code "stamped by the waterways layer query's UNION branches" that nothing stamps and 03-waterways.yaml says cannot exist. docs/country-gr.md had the transit row twice and a config path broken across a line. Verified: 39/39 gr acceptance (was 38 — the source-vocabulary test is new), is 38/38, se 45/45, fi 49/49, world 28/28, pl 45/46 (the pre-existing marginal buildings timeout, unrelated). check-queries clean on all six configs, make test green, tiles --check 0 failures. Re-imported waterways/roads/railways/ stations: 10 581 waterway rows at z14 across all 14 districts, 0 blank rail names, 529 roads, 436 stations, no NULL importance. The re-import also reloaded the staging tables through the fixed GeoJSON loader — gr_gaiaose_stations' columns now match the source file's key order exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UGAZTzU6FGJSQKj437rr3L