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

stations: T and J badges for metro and commuter rail (issue #25) #50

Merged
art merged 2 commits from night/issue-25-station-icons into main 2026-08-04 08:51:34 +00:00
Conversation 2 Commits 2 Files changed 13 +346 -41
art commented 2026-08-04 08:26:46 +00:00
Owner
Copy link

Implements #25 for Stockholm (metro T + commuter J badges); the issue stays open for the wider ask — icons for any local transit in any city we map — tracked by #24 (Warszawa) and follow-ups.

What this does

Sweden's stations layer now knows the Stockholm pendeltåg, and the style draws badge icons instead of plain dots: the standard T for tunnelbana stations and a matching J for commuter-rail stations, from z11 up.

Data: how the J got its stations

  • NJDB (the rail-traffic-locations source) cannot tell pendeltåg from mainline stations — nothing in the file says which services stop where.
  • The GTFS Sweden 3 feed can: SL's pendeltåg (lines 40/41/43/48) is route_type 106 under agency 505000000000000001. That route_type is shared with a dozen other operators (Östgötapendeln, Pågatåg, Krösatåg…), so readGTFSStops gained a per-route_type agency filter (agencies: {106: [SL]}). 900 (tram) is deliberately left unfiltered — Västtrafik/Östgötatrafiken/Skånetrafiken trams ride the same code and their stops belong on the map.
  • The stations layer became a single query over two staging tables (NJDB now loads into pg like Poland's feeds): each of the 55 pendeltåg stops claims its nearest NJDB station (≤ 500 m) as commuter. The station keeps its survey-grade NJDB position, name and importance; NJDB rows keep mode NULL everywhere else.
  • Why the nearest-claim and not a radius: the two feeds' positions differ by up to ~220 m, and a 500 m radius around each stop also tagged the neighbouring twin (Uppsala C's freight yard at 92 m, Södertälje syd's second platform, Stockholm C vs City). One-claim-per-stop tags exactly 55.
  • transit_mode gained commuter, and enumTypesSQL now emits ALTER TYPE … ADD VALUE IF NOT EXISTS so a live database built by an older config migrates instead of needing a rebuild.

Style: the icons

  • station_icons (z11+) draws sprite ["get","mode"] — the sprite names equal the enum labels, so metro→rail-metro (Maki, CC0) and commuter→a hand-drawn J in the same box style (commuter-j.svg).
  • station_dots keeps tram + heavy-rail dots at all zooms; metro/commuter show as dots below z11 and badges above.
  • The icons are placement-exempt (icon-allow-overlap + icon-ignore-placement): a badge is a point mark and must not vanish because a road name sits near it. Without the exemption the icon collided with its own label in the neighbouring symbol layer and silently never drew — caught by driving Chrome over CDP, where the icons layer rendered 0 features until the exemption was added.

Verification

  • Live DB: enum {metro,tram,commuter}; z14 1595 NULL / 309 tram / 100 metro / 55 commuter, total 2059 — unchanged, so no station was lost or doubled.
  • z12 Stockholm tile decodes with metro/commuter/tram modes present; zoom filters behave (tram drops at z11, importance≥3 at z9).
  • Chrome over CDP (real time, per docs/performance.md): z13 Stockholm renders 23 metro/commuter badges + 37 labels, z14 T-Centralen 10 badges; zero console errors; style validates clean with gl-style-validate.
  • Screenshots: /tmp/stockholm_z13.png, /tmp/stockholm_z14_tcentralen.png (from this session).
Implements #25 for Stockholm (metro T + commuter J badges); the issue stays open for the wider ask — icons for any local transit in any city we map — tracked by #24 (Warszawa) and follow-ups. --- ## What this does Sweden's stations layer now knows the Stockholm pendeltåg, and the style draws badge icons instead of plain dots: the standard **T** for tunnelbana stations and a matching **J** for commuter-rail stations, from z11 up. ### Data: how the J got its stations - NJDB (the rail-traffic-locations source) cannot tell pendeltåg from mainline stations — nothing in the file says which services stop where. - The GTFS Sweden 3 feed can: SL's pendeltåg (lines 40/41/43/48) is route_type 106 under agency `505000000000000001`. That route_type is shared with a dozen other operators (Östgötapendeln, Pågatåg, Krösatåg…), so `readGTFSStops` gained a **per-route_type agency filter** (`agencies: {106: [SL]}`). 900 (tram) is deliberately left unfiltered — Västtrafik/Östgötatrafiken/Skånetrafiken trams ride the same code and their stops belong on the map. - The stations layer became a single query over two staging tables (NJDB now loads into pg like Poland's feeds): each of the 55 pendeltåg stops **claims its nearest NJDB station** (≤ 500 m) as `commuter`. The station keeps its survey-grade NJDB position, name and importance; NJDB rows keep mode NULL everywhere else. - Why the nearest-claim and not a radius: the two feeds' positions differ by up to ~220 m, and a 500 m radius around each stop also tagged the neighbouring twin (Uppsala C's freight yard at 92 m, Södertälje syd's second platform, Stockholm C vs City). One-claim-per-stop tags exactly 55. - `transit_mode` gained `commuter`, and `enumTypesSQL` now emits `ALTER TYPE … ADD VALUE IF NOT EXISTS` so a live database built by an older config migrates instead of needing a rebuild. ### Style: the icons - `station_icons` (z11+) draws sprite `["get","mode"]` — the sprite names equal the enum labels, so metro→rail-metro (Maki, CC0) and commuter→a hand-drawn J in the same box style (`commuter-j.svg`). - `station_dots` keeps tram + heavy-rail dots at all zooms; metro/commuter show as dots below z11 and badges above. - The icons are placement-exempt (`icon-allow-overlap` + `icon-ignore-placement`): a badge is a point mark and must not vanish because a road name sits near it. Without the exemption the icon collided with its own label in the neighbouring symbol layer and silently never drew — caught by driving Chrome over CDP, where the icons layer rendered 0 features until the exemption was added. ## Verification - Live DB: enum `{metro,tram,commuter}`; z14 `1595 NULL / 309 tram / 100 metro / 55 commuter`, total 2059 — unchanged, so no station was lost or doubled. - z12 Stockholm tile decodes with metro/commuter/tram modes present; zoom filters behave (tram drops at z11, importance≥3 at z9). - Chrome over CDP (real time, per docs/performance.md): z13 Stockholm renders 23 metro/commuter badges + 37 labels, z14 T-Centralen 10 badges; zero console errors; style validates clean with gl-style-validate. - Screenshots: /tmp/stockholm_z13.png, /tmp/stockholm_z14_tcentralen.png (from this session).
Sweden's stations layer now tags the Stockholm pendeltåg and the style draws
badge icons instead of plain dots from z11.

Data — how the J got its stations:
- NJDB cannot tell pendeltåg from mainline stations; the GTFS feed can:
  SL's pendeltåg (40/41/43/48) is route_type 106 under agency
  505000000000000001. readGTFSStops gained a per-route_type agency filter
  (agencies: {106: [SL]}) because 106 is shared with a dozen other
  operators and 900 (tram) is shared with Västtrafik/Östgötatrafiken/
  Skånetrafiken, whose stops must stay.
- The stations layer became one query over two staging tables (NJDB now
  loads into pg like Poland's feeds): each of the 55 pendeltåg stops claims
  its nearest NJDB station (<=500 m) as commuter, keeping the survey-grade
  NJDB position, name and importance. One-claim-per-stop, not a radius: the
  feeds' positions differ by up to ~220 m, and a radius also tagged the
  neighbouring twin (Uppsala C's freight yard at 92 m).
- transit_mode gained commuter, and enumTypesSQL now emits ALTER TYPE ...
  ADD VALUE IF NOT EXISTS so live DBs migrate instead of needing a rebuild.

Style:
- station_icons (z11+) draws sprite [get, mode]; sprite names equal the
  enum labels: metro -> rail-metro (Maki, CC0), commuter -> a hand-drawn J
  in the same box style (commuter-j.svg).
- station_dots keeps tram + heavy-rail dots at all zooms; metro/commuter
  show as dots below z11 and badges above.
- Icons are placement-exempt: a badge must not vanish because a road name
  sits near it. Without the exemption the icon collided with its own label
  in the neighbouring symbol layer and silently never drew (found by
  driving Chrome over CDP: 0 rendered icons until it was added).

Verified against the live DB: enum {metro,tram,commuter}; z14 1595 NULL /
309 tram / 100 metro / 55 commuter, total 2059 unchanged (nothing lost or
doubled); Stockholm z12-z14 tiles carry the modes; Chrome-over-CDP shows
23 badges at z13 and 10 at z14 T-Centralen with zero console errors.
art-bot commented 2026-08-04 08:31:19 +00:00
Collaborator
Copy link

Reviewed at 629121a, against the live database and a running server rather than by reading alone.

The feature works end to end. What I verified:

  • Sprites generate — /sprite.json carries metro and commuter at 15×15. tram is absent by design: the station_icons filter passes only metro/commuter and tram keeps the dot treatment.
  • Tiles carry the data — /tiles/map/12/2253/1204 (Stockholm) decodes to stations with mode metro 20, tram 13, commuter 4, null 9.
  • importance is preserved, not flattened — commuter stations come out 25 at importance 2 and 30 at 3, so a pendeltåg stop replacing a z9 mainline station does not drop to z11, exactly as the config comment promises.
  • No duplicate output on today's data — 0 duplicate (name, geom), 0 stations sharing a point.
  • style.json valid, 64 layers, and the station_dots → icon handover at z11 reads correctly.

1. Two commuter stops can claim the same NJDB station, duplicating it

In conf/semap.yaml, the claimed CTE:

SELECT DISTINCT ON (c.geom) r.id AS r_id
FROM commuter c CROSS JOIN LATERAL (... ORDER BY c.geom <-> r.geom LIMIT 1) r

DISTINCT ON (c.geom) guarantees each stop claims one station. Nothing guarantees each station is claimed by at most one stop — and LEFT JOIN claimed cl ON cl.r_id = r.id then emits that station twice.

(The DISTINCT ON is also close to vacuous as written: the LATERAL already returns at most one row per stop, so it only does anything when two stops share a geometry.)

It does not bite today — 55 stops resolve to 55 distinct stations — but it is one data change away. Stockholms central already has two commuter stops within 500 m: Stockholm Centralstation at 435 m and Stockholm City at 383 m. The double-claim is avoided only because Stockholm City's nearest is its own NJDB station at 173 m.

The layer comment reasons carefully about one stop matching many stations — it names Uppsala C's freight yard, Södertälje syd's second platform, Stockholm C vs City — but not many stops matching one station, which is the direction that multiplies rows.

Since claimed is used purely as an existence test, the fix is one word:

SELECT DISTINCT r.id AS r_id

Same 55 rows on current data, immune to the case above.

2. The enum-migration comment gives a false reason

importer/main.go says "PG 12+ allows the new value to be used later in the same transaction, so a fresh import just works on a migrated database."

Tested against the live server (PG 18.4):

BEGIN; DO $$ BEGIN ALTER TYPE rv_enum ADD VALUE IF NOT EXISTS 'c'; END $$; SELECT 'c'::rv_enum;
ERROR:  unsafe use of new value "c" of enum type rv_enum
HINT:  New enum values must be committed before they can be used.

The code is correct — the DDL runs as its own dest.Exec at main.go:172, well before any layer insert, so it commits first. But the comment documents why it is safe and the stated reason is the opposite of true. Anyone later folding that Exec into a transaction with the data load would trust it and get a hard failure on exactly the migration path this was added for.

Suggested rewording: the ALTER runs in its own statement, well before any layer uses the label, because PG will not let a new enum value be used in the transaction that added it.

Worth noting the DO block wrapper itself is fine on PG 18 — that was the part I expected to be a problem and it is not.

3. The agency filter fails silently, and has no test

importer/ has six test files and none touches GTFS. The new agencies: filter decides which operator's commuter stops appear, and its only guard is:

if len(routeMode) == 0 { return error }

routeMode accumulates across all wanted route types. So if agency 505000000000000001 stops matching — feed change, ID change, a routes.txt with no agency_id column — route_type 106 contributes nothing, 401 and 900 still fill the map, no error fires, and commuter stations quietly vanish. Given the ID is an opaque 18-digit literal in the config, I would make the emptiness check per route type rather than over the union, and add one test over a small synthetic feed.

Smaller notes

  • mapsrv/icons/commuter-j.svg has no stated license. The README documents provenance ("drawn to match rail-metro — Maki has no J") but every other icon there is CC0-from-Maki, so a hand-drawn addition in that list is ambiguous about its terms.
  • Issue #25 asks for more than this delivers — "any local transit in any city we map, if they have standard icons". This does Stockholm's T and J; Gothenburg, Norrköping and Malmö trams stay dots. Reasonable scope, but #25 should probably stay open rather than auto-close on merge.
  • Ignore any TestDumpGlyphPNG failure — it writes to /tmp and only fails in my sandbox, on main as well as here.

Recommendation

1 and 2 are small enough to fix on the branch now — one word and one comment. 3 is better as a follow-up issue than a blocker.

Reviewed at `629121a`, against the live database and a running server rather than by reading alone. **The feature works end to end.** What I verified: - **Sprites generate** — `/sprite.json` carries `metro` and `commuter` at 15×15. `tram` is absent by design: the `station_icons` filter passes only metro/commuter and tram keeps the dot treatment. - **Tiles carry the data** — `/tiles/map/12/2253/1204` (Stockholm) decodes to `stations` with mode metro 20, tram 13, commuter 4, null 9. - **`importance` is preserved, not flattened** — commuter stations come out 25 at importance 2 and 30 at 3, so a pendeltåg stop replacing a z9 mainline station does not drop to z11, exactly as the config comment promises. - **No duplicate output on today's data** — 0 duplicate `(name, geom)`, 0 stations sharing a point. - **`style.json` valid**, 64 layers, and the `station_dots` → icon handover at z11 reads correctly. --- ## 1. Two commuter stops can claim the same NJDB station, duplicating it In `conf/semap.yaml`, the `claimed` CTE: ```sql SELECT DISTINCT ON (c.geom) r.id AS r_id FROM commuter c CROSS JOIN LATERAL (... ORDER BY c.geom <-> r.geom LIMIT 1) r ``` `DISTINCT ON (c.geom)` guarantees each *stop* claims one station. Nothing guarantees each *station* is claimed by at most one stop — and `LEFT JOIN claimed cl ON cl.r_id = r.id` then emits that station twice. (The `DISTINCT ON` is also close to vacuous as written: the LATERAL already returns at most one row per stop, so it only does anything when two stops share a geometry.) It does not bite today — 55 stops resolve to 55 distinct stations — but it is one data change away. **Stockholms central already has two commuter stops within 500 m**: Stockholm Centralstation at 435 m and Stockholm City at 383 m. The double-claim is avoided only because Stockholm City's nearest is its own NJDB station at 173 m. The layer comment reasons carefully about one stop matching many stations — it names Uppsala C's freight yard, Södertälje syd's second platform, Stockholm C vs City — but not many stops matching one station, which is the direction that multiplies rows. Since `claimed` is used purely as an existence test, the fix is one word: ```sql SELECT DISTINCT r.id AS r_id ``` Same 55 rows on current data, immune to the case above. ## 2. The enum-migration comment gives a false reason `importer/main.go` says *"PG 12+ allows the new value to be used later in the same transaction, so a fresh import just works on a migrated database."* Tested against the live server (PG 18.4): ``` BEGIN; DO $$ BEGIN ALTER TYPE rv_enum ADD VALUE IF NOT EXISTS 'c'; END $$; SELECT 'c'::rv_enum; ERROR: unsafe use of new value "c" of enum type rv_enum HINT: New enum values must be committed before they can be used. ``` **The code is correct** — the DDL runs as its own `dest.Exec` at `main.go:172`, well before any layer insert, so it commits first. But the comment documents *why* it is safe and the stated reason is the opposite of true. Anyone later folding that Exec into a transaction with the data load would trust it and get a hard failure on exactly the migration path this was added for. Suggested rewording: the ALTER runs in its own statement, well before any layer uses the label, because PG will not let a new enum value be used in the transaction that added it. Worth noting the `DO` block wrapper itself is fine on PG 18 — that was the part I expected to be a problem and it is not. ## 3. The agency filter fails silently, and has no test `importer/` has six test files and none touches GTFS. The new `agencies:` filter decides which operator's commuter stops appear, and its only guard is: ```go if len(routeMode) == 0 { return error } ``` `routeMode` accumulates across *all* wanted route types. So if agency `505000000000000001` stops matching — feed change, ID change, a `routes.txt` with no `agency_id` column — route_type 106 contributes nothing, 401 and 900 still fill the map, no error fires, and commuter stations quietly vanish. Given the ID is an opaque 18-digit literal in the config, I would make the emptiness check per route type rather than over the union, and add one test over a small synthetic feed. ## Smaller notes - **`mapsrv/icons/commuter-j.svg` has no stated license.** The README documents provenance ("drawn to match rail-metro — Maki has no J") but every other icon there is CC0-from-Maki, so a hand-drawn addition in that list is ambiguous about its terms. - **Issue #25 asks for more than this delivers** — "any local transit in any city we map, if they have standard icons". This does Stockholm's T and J; Gothenburg, Norrköping and Malmö trams stay dots. Reasonable scope, but #25 should probably stay open rather than auto-close on merge. - Ignore any `TestDumpGlyphPNG` failure — it writes to `/tmp` and only fails in my sandbox, on `main` as well as here. ## Recommendation **1** and **2** are small enough to fix on the branch now — one word and one comment. **3** is better as a follow-up issue than a blocker.
art force-pushed night/issue-25-station-icons from 629121aed8 to 211133fc2e 2026-08-04 08:48:57 +00:00 Compare
art commented 2026-08-04 08:49:15 +00:00
Author
Owner
Copy link

Review addressed — all three findings fixed on the branch (211133f, pushed; force-with-lease because the branch was rebased onto main-with-#49 before the review):

  1. Duplicate claims — claimed now uses SELECT DISTINCT r.id instead of DISTINCT ON (c.geom). You were right on both counts: the LATERAL already returns one row per stop (so DISTINCT ON was vacuous), and two stops can pick the same station — Stockholm Centralstation and Stockholm City are both within 500 m of the NJDB Stockholm C point. Re-imported: still exactly 55 commuter / 1595 NULL / 309 tram / 100 metro, and 0 duplicate (name, geom), matching your measurement.

  2. Enum comment — rewritten. The ALTER runs as its own statement before any insert, which is the load-bearing fact; PG refuses to use a new enum value in the transaction that added it. Thank you for actually testing that against 18.4 — I'd stated the opposite.

  3. Agency filter silence — fixed on the branch rather than filed: a route_type narrowed by an agencies: filter must match at least one route or the load errors (route_type 106 with agencies [...] matched no routes), so a feed change or a mistyped id can no longer quietly drop a whole mode. Also added the first GTFS tests (importer/gtfs_test.go, synthetic feed): agency filter, per-route-type mismatch, no-routes error, and the metro-wins parent collapse.

Smaller notes:

  • commuter-j.svg now states CC0-1.0 (SVG comment + icons README), matching the rest of the set.
  • #25 auto-close — PR description no longer says Closes #25; the issue stays open for the wider 'any city' ask, with #24 as the next instance.

Verified: go test green (importer incl. new tests, mapsrv), make build, stations layer re-imported against the live DB.

Review addressed — all three findings fixed on the branch (211133f, pushed; force-with-lease because the branch was rebased onto main-with-#49 before the review): 1. **Duplicate claims** — `claimed` now uses `SELECT DISTINCT r.id` instead of `DISTINCT ON (c.geom)`. You were right on both counts: the LATERAL already returns one row per stop (so DISTINCT ON was vacuous), and two stops can pick the same station — Stockholm Centralstation and Stockholm City are both within 500 m of the NJDB Stockholm C point. Re-imported: still exactly 55 commuter / 1595 NULL / 309 tram / 100 metro, and 0 duplicate (name, geom), matching your measurement. 2. **Enum comment** — rewritten. The ALTER runs as its own statement before any insert, which is the load-bearing fact; PG refuses to *use* a new enum value in the transaction that added it. Thank you for actually testing that against 18.4 — I'd stated the opposite. 3. **Agency filter silence** — fixed on the branch rather than filed: a route_type narrowed by an `agencies:` filter must match at least one route or the load errors (`route_type 106 with agencies [...] matched no routes`), so a feed change or a mistyped id can no longer quietly drop a whole mode. Also added the first GTFS tests (`importer/gtfs_test.go`, synthetic feed): agency filter, per-route-type mismatch, no-routes error, and the metro-wins parent collapse. Smaller notes: - **commuter-j.svg** now states CC0-1.0 (SVG comment + icons README), matching the rest of the set. - **#25 auto-close** — PR description no longer says `Closes #25`; the issue stays open for the wider 'any city' ask, with #24 as the next instance. Verified: `go test` green (importer incl. new tests, mapsrv), `make build`, stations layer re-imported against the live DB.
art merged commit 211133fc2e into main 2026-08-04 08:51:34 +00:00
art deleted branch night/issue-25-station-icons 2026-08-04 08:51:34 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
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
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!50
No description provided.