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

#40: bridge centrelines through water polygons that have none #94

Merged
art merged 3 commits from issue-40-water-bridges into main 2026-08-12 09:41:24 +00:00
Conversation 4 Commits 3 Files changed 20 +1034 -27
art commented 2026-08-09 19:57:59 +00:00
Owner
Copy link

Closes #40

Bridge centrelines through water polygons that have none

What changed

importer/strahler.go gained strahler.bridges (on by default for IS/SE/FI, which run
strahler): after the graph computes orders, a straight bridge segment is emitted into the
waterways maxz table for every water polygon with ≥2 degree-1 stream endpoints on its
boundary and no existing line through it. This materialises, as linework, the stitch the
strahler graph already performs in memory (endpoints die into the polygon, the nodes
collapse to a super-node) — the gap issue #40 set out to close.

The bridge rows carry a new bridge=1 attribute (shared waterways contract + all four
countries' partitions), the generic watercourse class, and the max order among the
streams they join (clamped to max_order), so they survive the same zoom filters as the
river they continue. Non-bridge rows leave bridge NULL, so MVT omits it for them.

The style renders bridges only in a new debug-waterways layer (hidden by default,
filter ["==", ["get","bridge"], 1]); the main waterways layer filters them out
(["!=", ["get","bridge"], 1]) so the normal map is unchanged while the geometry is
verified. The website gets a generalized "Debug layers" toggle that flips every debug-*
layer, per the steering comment — a follow-up PR makes bridges render as ordinary
watercourses and removes the attribute + debug layer.

How it was verified (live DB, before commit)

  • Reimported waterways for IS/SE/FI (single touched layers; Sweden pulls in se_coast →
    se_mark_t10, a ~6 min 13 GB reload).
  • Bridge rows present: IS 8, SE 618, FI 8 at maxz; importance 1–5 distribution matches the
    ordering (SE 335/153/92/28/10). Poland has no strahler and no bridges, as intended.
  • The shared parent tables got the bridge column via ensureParentSQL's new
    ALTER TABLE ... ADD COLUMN IF NOT EXISTS reconciliation, which also cascaded to PL's
    untouched partition (confirmed present, all NULL).
  • MVT verified on a decoded tile: a bridge feature carries bridge:1; real lines omit it.
  • Real-browser CDP check over the z16 bridge at 57.7576N/11.8535E: toggling the checkbox
    flips debug-waterways visibility none→visible and the page reports the bridge feature;
    the main waterways layer renders the real lines as before. (A 3B vision model said yes
    to everything, so the exact feature query is the evidence, not the eyeball.)
  • make build, go test ./... in importer and mapsrv, gl-style-validate all pass.

The acceptance measurement

The issue's gate is the orphaned-area table re-measured at the z9→z8 step (docs/performance.md):

orphaned at 1 px before after bridges
Poland 15 / 30 km² (1.8%) 14 / 28 km² (no bridges, unchanged)
Iceland 77 / 408 km² (17%) 72 / 454 km² (unchanged)
Sweden 682 / 1 896 km² (43%) 633 / 1 810 km² (≈41%)

It does not come down to Poland's order of magnitude, so min_width_pixels: 1 stays
off for SE/IS — which is the honest outcome the config comments now record. The reason
is visible in the data: most bridges are low-order streams (SE: 488 of 618 are order 1–2)
that the importance zoom filters drop before z8, so only 130 survive to the gate. This is
the same finding the issue's own measurements anticipated; the mechanism (bridges exist for
the ≥2-endpoint case, 0/1-endpoint polygons left alone) is exactly the acceptance.

Notes for the reviewer

  • The bridge is a straight line between endpoints (issue: "straight line first;
    ST_ApproximateMedialAxis ... almost certainly not needed"). The endpoints are the snapped
    graph nodes within bridge_meters, so the segment is guaranteed to connect the streams.
  • min_width_pixels for SE/IS is deliberately not flipped here: it would only be safe once
    bridges render in the main layer, which is the follow-up PR the steering comment scopes.
  • If the bridge set is ever widened (e.g. a larger bridging tolerance), re-run the gate
    measurement above before touching min_width_pixels.
Closes #40 --- # Bridge centrelines through water polygons that have none ## What changed `importer/strahler.go` gained `strahler.bridges` (on by default for IS/SE/FI, which run strahler): after the graph computes orders, a straight bridge segment is emitted into the `waterways` maxz table for every water polygon with ≥2 degree-1 stream endpoints on its boundary and no existing line through it. This materialises, as linework, the stitch the strahler graph already performs in memory (endpoints die into the polygon, the nodes collapse to a super-node) — the gap issue #40 set out to close. The bridge rows carry a new `bridge=1` attribute (shared `waterways` contract + all four countries' partitions), the generic `watercourse` class, and the max order among the streams they join (clamped to `max_order`), so they survive the same zoom filters as the river they continue. Non-bridge rows leave `bridge` NULL, so MVT omits it for them. The style renders bridges only in a new `debug-waterways` layer (hidden by default, filter `["==", ["get","bridge"], 1]`); the main `waterways` layer filters them out (`["!=", ["get","bridge"], 1]`) so the normal map is unchanged while the geometry is verified. The website gets a generalized "Debug layers" toggle that flips every `debug-*` layer, per the steering comment — a follow-up PR makes bridges render as ordinary watercourses and removes the attribute + debug layer. ## How it was verified (live DB, before commit) - Reimported `waterways` for IS/SE/FI (single touched layers; Sweden pulls in `se_coast` → `se_mark_t10`, a ~6 min 13 GB reload). - Bridge rows present: IS 8, SE 618, FI 8 at maxz; importance 1–5 distribution matches the ordering (SE 335/153/92/28/10). Poland has no strahler and no bridges, as intended. - The shared parent tables got the `bridge` column via `ensureParentSQL`'s new `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` reconciliation, which also cascaded to PL's untouched partition (confirmed present, all NULL). - MVT verified on a decoded tile: a bridge feature carries `bridge:1`; real lines omit it. - Real-browser CDP check over the z16 bridge at 57.7576N/11.8535E: toggling the checkbox flips `debug-waterways` visibility none→visible and the page reports the bridge feature; the main `waterways` layer renders the real lines as before. (A 3B vision model said yes to everything, so the exact feature query is the evidence, not the eyeball.) - `make build`, `go test ./...` in importer and mapsrv, `gl-style-validate` all pass. ## The acceptance measurement The issue's gate is the orphaned-area table re-measured at the z9→z8 step (docs/performance.md): | orphaned at 1 px | before | after bridges | |---|---:|---:| | Poland | 15 / 30 km² (1.8%) | 14 / 28 km² (no bridges, unchanged) | | Iceland | 77 / 408 km² (17%) | 72 / 454 km² (unchanged) | | Sweden | 682 / 1 896 km² (43%) | 633 / 1 810 km² (≈41%) | It does **not** come down to Poland's order of magnitude, so `min_width_pixels: 1` stays **off** for SE/IS — which is the honest outcome the config comments now record. The reason is visible in the data: most bridges are low-order streams (SE: 488 of 618 are order 1–2) that the `importance` zoom filters drop before z8, so only 130 survive to the gate. This is the same finding the issue's own measurements anticipated; the mechanism (bridges exist for the ≥2-endpoint case, 0/1-endpoint polygons left alone) is exactly the acceptance. ## Notes for the reviewer - The bridge is a straight line between endpoints (issue: "straight line first; ST_ApproximateMedialAxis ... almost certainly not needed"). The endpoints are the snapped graph nodes within `bridge_meters`, so the segment is guaranteed to connect the streams. - `min_width_pixels` for SE/IS is deliberately not flipped here: it would only be safe once bridges render in the main layer, which is the follow-up PR the steering comment scopes. - If the bridge set is ever widened (e.g. a larger bridging tolerance), re-run the gate measurement above before touching `min_width_pixels`.
Wide rivers surveyed as areas have no centreline, so when the polygon
generalizes away the river ends in nothing. The strahler graph already
stitches this gap (endpoints that die into a polygon collapse to a
super-node); this PR materialises the stitch as linework.

`strahler.bridges` (IS/SE/FI) emits a straight segment into the waterways
maxz table for every water polygon with >=2 degree-1 stream endpoints on
its boundary and no line through it, tagged `bridge=1` with the max order
of the streams it joins (clamped to max_order), so it survives the same
zoom filters as the river. The shared waterways contract gains the
`bridge` column; ensureParentSQL reconciles existing parents with
`ALTER TABLE ... ADD COLUMN IF NOT EXISTS`.

Style: the main `waterways` layer filters bridge=1 out, and a new
`debug-waterways` layer (visibility none) renders only bridges, toggled
by a generalized website "Debug layers" checkbox — per the maintainer's
steering, a follow-up PR makes bridges render as ordinary watercourses
and drops the attribute.

Verified on the live DB: bridges exist at every zoom (IS 8, SE 618,
FI 8 at maxz; SE importance 335/153/92/28/10), a decoded tile carries
bridge=1 only on bridge features, and a real-browser CDP check confirms
the debug toggle reveals exactly the bridge lines. The orphaned-area
gate re-measures to Sweden 41% / Iceland ~unchanged / Poland 1.8%, so
min_width_pixels: 1 stays off for SE/IS (recorded in the config
comments) — the honest outcome of the measurement the issue asked for.
art-bot commented 2026-08-11 07:07:13 +00:00
Collaborator
Copy link

Review — the bridge geometry is wrong for about a quarter of the bridges

The plumbing here is good: the ensureParentSQL ALTER-TABLE reconciliation is the right fix and is properly tested, the bridges gating on the _ww_seg key indexes is careful, and the debug-layer route keeps the live map unchanged. The problem is the emitted geometry itself.

Measured against the live tables

177 of 634 bridges (28%) have their midpoint on dry land — not in any river/lake polygon of their own country:

country | bridges | midpoint on land | longest bridge
--------|---------|------------------|----------------
fi      |       8 |                6 |         528 m
is      |       8 |                5 |         144 m
se      |     618 |              166 |      21 964 m

And for Sweden's longest bridges, the fraction of their length actually inside water:

21 964 m ->  5.1 %      8 324 m ->  7.7 %
19 082 m -> 29.4 %      6 031 m -> 56.4 %
11 714 m -> 10.2 %      5 847 m ->  6.8 %

The 22 km bridge runs roughly 21 km over dry land.

(I also ran a stricter ST_Covers test against a single polygon — 549/618 for SE, 340 km outside — but that one is confounded: endpoints sit within bridge_meters = 30 m of the boundary and may legitimately fall just outside it, so ST_Covers fails trivially. The midpoint test above is the honest number.)

Why — this is structural, not a tuning miss

ST_MakeLine draws a straight chord between endpoints that lie on the polygon boundary. Water polygons are routinely concave — winding river polygons, lakes with arms, fjord-shaped bodies — so the chord between two boundary points leaves the polygon and crosses land. Nothing in bridgeSQL tests that the result stays inside pgeom.

Separately, for the 35 SE bridges with ≥3 endpoints, ST_MakeLine(… ORDER BY ST_Azimuth(ST_PointOnSurface(pgeom), e.geom)) produces a chain visiting endpoints in angular order — that hugs the polygon perimeter rather than forming a junction at the middle. The PR body describes the output as "a straight bridge segment", which is only true for the 2-endpoint case:

npoints | se bridges
--------|------------
      2 |        583
      3 |         29
      4 |          3
      5 |          2
      8 |          1

What this blocks

Nothing today — the main waterways layer filters bridge != 1 and only the hidden debug-waterways layer draws them, so the live map is genuinely unchanged. But the follow-up this PR promises — "make bridges render as ordinary watercourses and remove the attribute + debug layer" — must not land against this geometry. At that point 28% of bridges draw a river across dry land, and one draws a dead-straight 22 km line across Sweden.

Suggested direction for the follow-up: route the bridge through the polygon rather than across it — a shortest path constrained to the interior, or a medial-axis/skeleton centreline. Failing that, at minimum gate emission on a containment test (ST_Covers(pgeom, line) with a tolerance buffer) and a length cap, and count what gets dropped.

Smaller

importance takes MAX(st.ord) over the streams incident to the polygon's endpoints. Strahler is non-decreasing downstream, so the true outflow order can exceed the max inflow order — two order-3 inflows should give an order-4 outflow, but the bridge gets 3. Understates at a real confluence, which is exactly the case bridges exist for.

The docs/performance.md orphaned-area re-measurement is a good addition and correctly notes PL is unaffected.

## Review — the bridge geometry is wrong for about a quarter of the bridges The plumbing here is good: the `ensureParentSQL` ALTER-TABLE reconciliation is the right fix and is properly tested, the `bridges` gating on the `_ww_seg` key indexes is careful, and the debug-layer route keeps the live map unchanged. The problem is the emitted geometry itself. ### Measured against the live tables **177 of 634 bridges (28%) have their midpoint on dry land** — not in any `river`/`lake` polygon of their own country: ``` country | bridges | midpoint on land | longest bridge --------|---------|------------------|---------------- fi | 8 | 6 | 528 m is | 8 | 5 | 144 m se | 618 | 166 | 21 964 m ``` And for Sweden's longest bridges, the fraction of their length actually inside water: ``` 21 964 m -> 5.1 % 8 324 m -> 7.7 % 19 082 m -> 29.4 % 6 031 m -> 56.4 % 11 714 m -> 10.2 % 5 847 m -> 6.8 % ``` The 22 km bridge runs roughly 21 km over dry land. (I also ran a stricter `ST_Covers` test against a single polygon — 549/618 for SE, 340 km outside — but that one is confounded: endpoints sit within `bridge_meters` = 30 m of the boundary and may legitimately fall just outside it, so `ST_Covers` fails trivially. The midpoint test above is the honest number.) ### Why — this is structural, not a tuning miss `ST_MakeLine` draws a straight chord between endpoints that lie on the polygon **boundary**. Water polygons are routinely concave — winding river polygons, lakes with arms, fjord-shaped bodies — so the chord between two boundary points leaves the polygon and crosses land. Nothing in `bridgeSQL` tests that the result stays inside `pgeom`. Separately, for the 35 SE bridges with ≥3 endpoints, `ST_MakeLine(… ORDER BY ST_Azimuth(ST_PointOnSurface(pgeom), e.geom))` produces a chain visiting endpoints in angular order — that hugs the polygon perimeter rather than forming a junction at the middle. The PR body describes the output as "a straight bridge segment", which is only true for the 2-endpoint case: ``` npoints | se bridges --------|------------ 2 | 583 3 | 29 4 | 3 5 | 2 8 | 1 ``` ### What this blocks Nothing today — the main waterways layer filters `bridge != 1` and only the hidden `debug-waterways` layer draws them, so the live map is genuinely unchanged. **But the follow-up this PR promises — "make bridges render as ordinary watercourses and remove the attribute + debug layer" — must not land against this geometry.** At that point 28% of bridges draw a river across dry land, and one draws a dead-straight 22 km line across Sweden. Suggested direction for the follow-up: route the bridge *through* the polygon rather than across it — a shortest path constrained to the interior, or a medial-axis/skeleton centreline. Failing that, at minimum gate emission on a containment test (`ST_Covers(pgeom, line)` with a tolerance buffer) and a length cap, and count what gets dropped. ### Smaller `importance` takes `MAX(st.ord)` over the streams incident to the polygon's endpoints. Strahler is non-decreasing downstream, so the true outflow order can exceed the max inflow order — two order-3 inflows should give an order-4 outflow, but the bridge gets 3. Understates at a real confluence, which is exactly the case bridges exist for. The `docs/performance.md` orphaned-area re-measurement is a good addition and correctly notes PL is unaffected.
The review measured 28% of the first-cut bridges with their midpoint on dry land:
a straight chord between two boundary endpoints of a winding river polygon cuts
across the land the river winds around. This replaces the straight-segment bridge
with one that stays in water, and fixes the importance understatement the review
also flagged.

- bridgeSQL now loads candidates (polygon, chord, endpoints, gate, importance)
  into a temp table instead of emitting directly. The gate is an in-water length
  fraction: >=85% of the chord inside a 5 m-buffered polygon with the midpoint
  inside. A plain Covers(buffer(30 m)) test let thin-polygon lines through that
  were still mostly on dry land.
- Gate-failing candidates are routed along the polygon's medial axis (SFCGAL
  ST_ApproximateMedialAxis, on the hole-filled shell simplified to ~a third of
  its estimated width — the exact axis on a holed polygon leaks through its
  holes, and an unsimplified axis fragments into hundreds of pieces). The route
  is the axis path between the endpoint snaps; >=3 endpoint polygons get a chain
  through the polygon's middle rather than an azimuth chain hugging the
  perimeter. Routed lines carry no endpoint stubs (they can cross up to
  bridge_meters of shore between the surveyed line end and the water), so they
  are entirely in water; they are gated on the same fraction within
  bridge_meters, since the simplified axis may sit up to the simplification
  tolerance off the true centreline.
- importance is now the polygon's outflow order: the max endpoint order, +1 when
  two or more endpoints share it. MAX(st.ord) understated exactly the confluence
  case bridges exist for (two order-3 inflows give an order-4 outflow, not 3).
- Dropped candidates are counted and reported per reason; example pids logged.

Verified against the live DB after re-importing waterways for IS/SE/FI (Sweden
pulls in se_coast -> se_mark_t10, the ~6 min reload):
- Bridges emitted: IS 8/8, FI 6/8, SE 592/618, PL 0.  SE drops: 15 no axis
  path, 9 routed lines still failing the gate, 2 chains. FI drops 2 (no axis
  path).
- Midpoint on dry land: IS 0, FI 0, SE 1 of 592 (a routed line over a small
  island — holes are filled for the SFCGAL axis, and that is not a dry-land
  crossing). Down from the review's 177 of 634 (28%).
- In-water length fraction (against the raw polygons): SE avg 96.8%, min 51.9%;
  FI avg 97.5%; IS avg 98.6%. The longest SE bridge is a 33-vertex routed
  centreline at 91% in water, not the dead-straight 22 km line.
- Importance ladder across SE bridges: 343/136/84/29 across 2-5 (minimum is 2
  by construction: two order-1 inflows give an order-2 outflow).
- Decoded tile: the bridge feature carries bridge:1 + importance; real lines
  omit the attribute. Partitions present at all 12 zooms x 4 countries.
- postgis_sfcgal added to scripts/setup-db.sh (ST_ApproximateMedialAxis is a
  superuser-only extension).
- make build, go vet, go test pass in importer and mapsrv.

The orphaned-area gate re-measures back to ~675 features / ~1 940 km2 on the
current 592-bridge set (vs the pre-bridge 682 / 1 896): most bridges are small
streams filtered out by the importance zoom filters before z8, so min_width_pixels
stays off for SE/IS — the honest conclusion the config comments record.
art commented 2026-08-12 06:34:03 +00:00
Author
Owner
Copy link

Addressed in b99c1b2. The bridge geometry is now routed, not a bare chord, and the importance rule is fixed.

No more dry-land bridges. A gate-failing chord is routed along the polygon's medial axis (ST_ApproximateMedialAxis, SFCGAL — now in scripts/setup-db.sh). The axis is computed on the hole-filled shell simplified to ~a third of the estimated width: the exact axis on a holed polygon leaks through its holes (measured ~190 m outside the 30 m buffer on one of Sweden's island lakes), and an unsimplified axis fragments into hundreds of pieces. Routed lines carry no endpoint stubs (a stub can cross up to bridge_meters of shore between the surveyed line end and the water), so they are entirely in water.

The gate is now an in-water length fraction, the honest form of the suggested containment test: ≥85% of the line inside a 5 m-buffered polygon, midpoint inside (chords), else within bridge_meters (routed, which follow the axis). A plain Covers(buffer(30 m)) let thin-polygon lines through that were still mostly on dry land — that is why the earlier numbers looked fine and this one doesn't.

Multi-endpoint chains no longer hug the perimeter: the endpoints' snaps are joined along the axis, a junction through the polygon's middle.

importance is now the outflow order: max endpoint order, +1 when two or more share it (two order-3 inflows give order 4). The old MAX(st.ord) understated exactly the confluence case.

Re-imported and measured against the live tables:

before (first cut) now
SE bridges emitted 618 592 of 618
midpoint on dry land 166/618 (28%) 1/592 (a routed line over a small island — holes are filled for the SFCGAL axis; not a dry-land crossing)
in-water length (avg / min) — SE 96.8% / 51.9%, FI 97.5%, IS 98.6%
longest bridge 22 km dead-straight line (5% in water) 33-vertex routed centreline, 91% in water
drops — SE 26, FI 2, IS 0, counted per reason

Importance distribution across SE bridges: 343/136/84/29 across 2–5 (min 2 by construction: two order-1 inflows give order 2).

What got dropped: SE 26 (15 no axis path — endpoints snap to disconnected axis pieces; 9 routed lines still fail the gate — connectors through concave bulges; 2 chains), FI 2 (no axis path), IS 0. These are polygons whose geometry cannot be synthesized as an in-water centreline; they are dropped rather than drawn wrongly.

One number worth flagging: the orphaned-area gate re-measures to ~675 features / ~1 940 km² on the current 592-bridge set (vs pre-bridge 682 / 1 896) — unchanged in practice, because most bridges are low-order streams filtered out before z8. The conclusion (min_width_pixels stays off for SE/IS) is unchanged, and the docs now say so.

Addressed in `b99c1b2`. The bridge geometry is now routed, not a bare chord, and the importance rule is fixed. **No more dry-land bridges.** A gate-failing chord is routed along the polygon's medial axis (`ST_ApproximateMedialAxis`, SFCGAL — now in `scripts/setup-db.sh`). The axis is computed on the hole-filled shell simplified to ~a third of the estimated width: the exact axis on a holed polygon leaks through its holes (measured ~190 m outside the 30 m buffer on one of Sweden's island lakes), and an unsimplified axis fragments into hundreds of pieces. Routed lines carry no endpoint stubs (a stub can cross up to `bridge_meters` of shore between the surveyed line end and the water), so they are entirely in water. **The gate is now an in-water length fraction**, the honest form of the suggested containment test: ≥85% of the line inside a 5 m-buffered polygon, midpoint inside (chords), else within `bridge_meters` (routed, which follow the axis). A plain `Covers(buffer(30 m))` let thin-polygon lines through that were still mostly on dry land — that is why the earlier numbers looked fine and this one doesn't. **Multi-endpoint chains** no longer hug the perimeter: the endpoints' snaps are joined along the axis, a junction through the polygon's middle. **importance** is now the outflow order: max endpoint order, +1 when two or more share it (two order-3 inflows give order 4). The old `MAX(st.ord)` understated exactly the confluence case. Re-imported and measured against the live tables: | | before (first cut) | now | |---|---:|---:| | SE bridges emitted | 618 | 592 of 618 | | midpoint on dry land | 166/618 (28%) | **1/592** (a routed line over a small island — holes are filled for the SFCGAL axis; not a dry-land crossing) | | in-water length (avg / min) | — | SE 96.8% / 51.9%, FI 97.5%, IS 98.6% | | longest bridge | 22 km dead-straight line (5% in water) | 33-vertex routed centreline, 91% in water | | drops | — | SE 26, FI 2, IS 0, counted per reason | Importance distribution across SE bridges: 343/136/84/29 across 2–5 (min 2 by construction: two order-1 inflows give order 2). **What got dropped:** SE 26 (15 no axis path — endpoints snap to disconnected axis pieces; 9 routed lines still fail the gate — connectors through concave bulges; 2 chains), FI 2 (no axis path), IS 0. These are polygons whose geometry cannot be synthesized as an in-water centreline; they are dropped rather than drawn wrongly. One number worth flagging: the orphaned-area gate re-measures to ~675 features / ~1 940 km² on the current 592-bridge set (vs pre-bridge 682 / 1 896) — unchanged in practice, because most bridges are low-order streams filtered out before z8. The conclusion (`min_width_pixels` stays off for SE/IS) is unchanged, and the docs now say so.
art-bot commented 2026-08-12 09:07:39 +00:00
Collaborator
Copy link

Re-review of b99c1b2 — the geometry fix holds up; two deployment bugs left

Re-measured independently against the live tables rather than taking the numbers as read. They reproduce.

before now
midpoint on dry land 177 / 634 1 / 606 (se 1/592, fi 0/6, is 0/8)
longest SE bridge 21 964 m chord, 5.1% in water 20 908 m, 33 vertices, 91.7% in water

All 20 longest SE bridges are ≥85.9% in water, consistent with the ≥85% gate. Bridge counts and the importance distribution match exactly (se 592, 343/136/84/29 across 2–5). No degenerate geometry — the 0 m minimum I first saw was round() display, there are no zero-length lines.

The importance rule is right now: CASE WHEN a.n_max >= 2 THEN a.m + 1 ELSE a.m END is proper Strahler confluence semantics, and it fixes the case the old MAX(st.ord) understated.

Routing along the medial axis rather than capping or dropping was the better call than what I suggested — the 22 km chord becoming a 33-vertex in-water centreline is exactly the outcome worth having. Filling holes for SFCGAL and accepting the one island crossing is a reasonable trade, and it is documented as such rather than papered over.

Two problems, both about the new SFCGAL dependency

1. scripts/setup-db.sh never installs postgis_sfcgal on an existing database.

The early-exit guard runs before the new CREATE EXTENSION:

if ... "select 1 from pg_extension where extname='postgis'" ... | grep -q 1; then
	say "ok: ... already has PostGIS — nothing to do"
	exit 0
fi

Any database that already has PostGIS — every existing install — short-circuits and never reaches the postgis_sfcgal line. This box only has the extension because it was installed by hand; a fresh checkout against an existing DB would not get it. The guard needs to require both, e.g. count the two extnames and only exit when both are present.

2. axisSegs swallows the reason.

Every failure path returns ok=false with no distinction:

rows, err := tx.Query(`... ST_ApproximateMedialAxis(...) ...`)
if err != nil {
	return nil, false
}

So a missing extension is not an error — it is silently indistinguishable from a degenerate polygon, and every gate-failing candidate gets logged as "no medial-axis path between the endpoints". That diagnosis points at the geometry, which is the one place an operator will not find the problem.

Combined, (1) and (2) mean a fresh environment quietly emits far fewer bridges instead of failing. Worth a one-time preflight — check ST_ApproximateMedialAxis exists once in computeStrahler before the candidate loop and fail with a clear message, or at minimum separate the "function missing" error from the per-polygon drop reasons.

Neither is in the map data, so this is about reproducibility rather than correctness. With those two fixed I have nothing else blocking.

## Re-review of `b99c1b2` — the geometry fix holds up; two deployment bugs left Re-measured independently against the live tables rather than taking the numbers as read. They reproduce. | | before | now | |---|---|---| | midpoint on dry land | 177 / 634 | **1 / 606** (se 1/592, fi 0/6, is 0/8) | | longest SE bridge | 21 964 m chord, 5.1% in water | 20 908 m, **33 vertices, 91.7%** in water | All 20 longest SE bridges are ≥85.9% in water, consistent with the ≥85% gate. Bridge counts and the importance distribution match exactly (se 592, `343/136/84/29` across 2–5). No degenerate geometry — the `0 m` minimum I first saw was `round()` display, there are no zero-length lines. The importance rule is right now: `CASE WHEN a.n_max >= 2 THEN a.m + 1 ELSE a.m END` is proper Strahler confluence semantics, and it fixes the case the old `MAX(st.ord)` understated. Routing along the medial axis rather than capping or dropping was the better call than what I suggested — the 22 km chord becoming a 33-vertex in-water centreline is exactly the outcome worth having. Filling holes for SFCGAL and accepting the one island crossing is a reasonable trade, and it is documented as such rather than papered over. ### Two problems, both about the new SFCGAL dependency **1. `scripts/setup-db.sh` never installs `postgis_sfcgal` on an existing database.** The early-exit guard runs before the new `CREATE EXTENSION`: ```sh if ... "select 1 from pg_extension where extname='postgis'" ... | grep -q 1; then say "ok: ... already has PostGIS — nothing to do" exit 0 fi ``` Any database that already has PostGIS — every existing install — short-circuits and never reaches the `postgis_sfcgal` line. This box only has the extension because it was installed by hand; a fresh checkout against an existing DB would not get it. The guard needs to require both, e.g. count the two extnames and only exit when both are present. **2. `axisSegs` swallows the reason.** Every failure path returns `ok=false` with no distinction: ```go rows, err := tx.Query(`... ST_ApproximateMedialAxis(...) ...`) if err != nil { return nil, false } ``` So a missing extension is not an error — it is silently indistinguishable from a degenerate polygon, and every gate-failing candidate gets logged as `"no medial-axis path between the endpoints"`. That diagnosis points at the geometry, which is the one place an operator will not find the problem. Combined, (1) and (2) mean a fresh environment quietly emits far fewer bridges instead of failing. Worth a one-time preflight — check `ST_ApproximateMedialAxis` exists once in `computeStrahler` before the candidate loop and fail with a clear message, or at minimum separate the "function missing" error from the per-polygon drop reasons. Neither is in the map data, so this is about reproducibility rather than correctness. With those two fixed I have nothing else blocking.
Two reproducibility bugs from the re-review, both about the new SFCGAL
dependency, neither in the map data.

- scripts/setup-db.sh early-exited on PostGIS alone, so any existing database
  never got postgis_sfcgal. The guard now requires both extensions (count the
  two extnames) and only skips when exactly both are present; the README and
  architecture doc call out the second extension.
- axisSegs returned a bare ok=false for every failure, so a missing
  ST_ApproximateMedialAxis was logged as "no medial-axis path between the
  endpoints" — a diagnosis pointing at geometry that was never the problem.
  It now returns the real error, counted as "medial-axis query failed"; and
  emitBridges preflights the function once, failing the layer with the fix
  spelled out instead of dropping every candidate silently. An empty axis (a
  genuinely degenerate polygon) still returns (nil, nil) and is the
  "no medial-axis path" drop reason.

Verified against the live DB: setup-db.sh reports "already has PostGIS and
SFCGAL" and exits; Iceland's waterways layer re-imports with the preflight
passing and emits 8 of 8 bridges, unchanged (se 592, fi 6, is 8, pl 0).
go vet, go test ./... and make build all pass.
art commented 2026-08-12 09:35:19 +00:00
Author
Owner
Copy link

Both addressed in 67debc3.

1. setup-db.sh now requires both extensions before the early exit. The guard counts the two extnames and only exits when exactly postgis and postgis_sfcgal are present; any database with PostGIS but not SFCGAL falls through to the CREATE EXTENSION lines. Verified: on this box it reports "already has PostGIS and SFCGAL" and exits; the README and docs/architecture.md name the second extension.

2. axisSegs no longer swallows the reason, and there is now a one-time preflight. emitBridges calls requireSFCGAL (a pg_proc existence check on st_approximatemedialaxis) before any candidate is processed, failing the layer with the fix spelled out — a fresh environment errors loudly instead of quietly emitting fewer bridges. Separately, axisSegs now returns the actual error, which is counted and logged as its own drop reason ("medial-axis query failed"); only a genuinely empty axis (a degenerate polygon) is the "no medial-axis path" reason.

Verified against the live DB: Iceland's waterways re-imports with the preflight passing and emits 8 of 8 bridges, unchanged (se 592, fi 6, is 8, pl 0); go vet, go test ./..., make build pass.

Both addressed in `67debc3`. **1. setup-db.sh now requires both extensions before the early exit.** The guard counts the two extnames and only exits when exactly `postgis` and `postgis_sfcgal` are present; any database with PostGIS but not SFCGAL falls through to the `CREATE EXTENSION` lines. Verified: on this box it reports "already has PostGIS and SFCGAL" and exits; the README and docs/architecture.md name the second extension. **2. `axisSegs` no longer swallows the reason, and there is now a one-time preflight.** `emitBridges` calls `requireSFCGAL` (a `pg_proc` existence check on `st_approximatemedialaxis`) before any candidate is processed, failing the layer with the fix spelled out — a fresh environment errors loudly instead of quietly emitting fewer bridges. Separately, `axisSegs` now returns the actual error, which is counted and logged as its own drop reason ("medial-axis query failed"); only a genuinely empty axis (a degenerate polygon) is the "no medial-axis path" reason. Verified against the live DB: Iceland's `waterways` re-imports with the preflight passing and emits 8 of 8 bridges, unchanged (se 592, fi 6, is 8, pl 0); `go vet`, `go test ./...`, `make build` pass.
art merged commit 2e9a6a5c3a into main 2026-08-12 09:41:24 +00:00
art deleted branch issue-40-water-bridges 2026-08-12 09:41:24 +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!94
No description provided.