Skip to content

Mask: reconnect all harbors (2x resolution, center-sampled land, connectivity gate)#8

Merged
DocGerd merged 5 commits into
mainfrom
fix/mask-harbor-connectivity
Jul 15, 2026
Merged

Mask: reconnect all harbors (2x resolution, center-sampled land, connectivity gate)#8
DocGerd merged 5 commits into
mainfrom
fix/mask-harbor-connectivity

Conversation

@DocGerd

@DocGerd DocGerd commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the harbor-connectivity failures reported in #6 (Flensburg's inner harbor showed as unreachable land; 14/33 harbor snaps were disconnected from open water at the default 3.0 m safety depth).

Root causes (confirmed):

  1. all_touched=True land rasterization at ~93 m cells ate a full cell of margin off both banks of every quay-lined basin/narrow channel.
  2. Resampling.max (shallowest-wins) on the ~115 m native EMODnet DTM let shallow surroundings dominate cells straddling a dredged/buoyed channel.
  3. EMODnet voids inside basins fell back to unknown -> 0 (land).

Changes:

  • pipeline/build_mask.py: 2x resolution (2200x2400, ~46 m cells), land rasterization switched to cell-center sampling (all_touched=False), depth reprojection blends Resampling.max and Resampling.bilinear per cell (trust bilinear only where it agrees with max within a 2.0 m tolerance; otherwise fall back to the conservative max value — an unconditional switch to bilinear alone manufactured depth at unrelated shoal/land boundaries across the grid, see "Depth resampling" below), plausibility assertions rescaled for the new grid.
  • pipeline/verify_mask.py: new connectivity gate — 4-connected flood-fill (scipy.ndimage.label) from a fixed open-water seed; every harbor snap must reach it at its gate depth (default 3.0 m, or a documented exception). Two exceptions added, each grounded in a measured reconnection threshold and the harbor's own approachNote text: Augustenborg (2.8 m), Marstal (2.0 m). A KNOWN_DISCONNECTED allowlist (5 harbors, each cited against issue Mask: 5 harbors remain disconnected — Schlei fairway, Dyvig channel, Gråsten bridge (sub-cell features) #9) means the gate reports these as tracked rather than failing the run — and fails the run if one of them ever becomes stale (i.e. turns out connected without the entry being removed).
  • pipeline/requirements.txt: added scipy==1.18.0 for the flood-fill.
  • Regenerated app/public/data/mask.bin (1.3 MB → 5.28 MB) + mask.meta.json.

Before / after connectivity

14/33 disconnected → 5/33 disconnected, all tracked (issue #9). Flensburg (the reported case) is now connected, along with 8 of the other 13 originally-named harbors. verify_mask.py exits 0 against the shipped mask.

Full per-harbor table, diagnosis of the 5 tracked (Arnis, Dyvig, Graasten, Kappeln, Maasholm — none force-connected with unjustified exceptions; each has a specific documented reason it wasn't), and all judgment calls are in .superpowers/sdd/fix-mask-report.md (gitignored working notes, not part of this diff).

Why 5 remain open, tracked under #9 rather than force-connected: investigated with a threshold-scan (binary search over gate depth) and a raw-DTM cross-check, not just left alone:

  • Arnis/Kappeln/Maasholm (Schlei-side): each isolated in its own tiny pocket, not even connected to each other, despite the raw EMODnet DTM showing real channel depth at each named point individually — the maintained fairway is narrower than even the 115 m-native source can resolve as a continuous ribbon. None of their notes document depth below 3.0 m (Kappeln's claims ~5 m), so an exception would misrepresent the source.
  • Dyvig/Graasten: don't reconnect at any depth threshold — the blocker is land (a sub-cell-width buoyed channel, and the Egernsund bascule bridge deck respectively), not depth, so a depth exception can't fix it by construction.

Depth resampling: why a blend, not a straight switch

An unconditional switch from Resampling.max to Resampling.bilinear fixes the channels above but also manufactures depth broadly elsewhere: measured against pure max in the actual final encoded output (post land-mask, post-Schlei-carve, post-floor), unconditional bilinear flips 22,948 cells from LAND to WATER, 1,780 of them ≥ 3.0 m (the app's default safety depth) and 663 ≥ 5.0 m — 97.6% of the ≥3.0 m flips more than 1 km from any harbor snap, i.e. mostly outside the channels this fix needed to help. That violates the project's "never overstate depth" rule.

Fixed by computing both reprojections and blending per cell: bilinear only where |bilinear − max| ≤ 2.0 m (smooth, gently-varying depth), else the conservative max value (a source discontinuity bilinear can't be trusted to interpolate). At that tolerance, land→water flips crossing 3.0 m (and 5.0 m) drop from 1,780 (663) to zero, while the connectivity gate still reports the identical 28 OK / 2 exceptions / 5 known-disconnected table.

Test plan

  • pipeline/build_mask.py completes, all assertions pass.
  • pipeline/verify_mask.py connectivity gate: exit code 0. 28/33 OK, 2 via documented exceptions (Augustenborg 2.8 m, Marstal 2.0 m), 5 known-disconnected and tracked under issue Mask: 5 harbors remain disconnected — Schlei fairway, Dyvig channel, Gråsten bridge (sub-cell features) #9 (Arnis, Dyvig, Graasten, Kappeln, Maasholm) — reported, not failing the run.
  • Flensburg snap (54.798, 9.4335) confirmed reaching the main open-water component.
  • Land→water flips crossing the 3.0 m / 5.0 m default safety depths: 0 / 0 post-blend (down from 1,780 / 663 under an unconditional bilinear switch).
  • npm --prefix app install && npm --prefix app run test -- mask — 8/8 pass (fixture-based, confirms no app-side regression from the mask format/dimension change).

Closes #6

🤖 Generated with Claude Code

Patrick Kuhn and others added 2 commits July 15, 2026 12:54
…ors (Closes #6)

Increase mask resolution 2200x2400 (~46m cells, was ~93m), switch land
rasterization from all_touched to cell-center sampling, and switch depth
reprojection from Resampling.max to Resampling.bilinear as a last resort
for channels the max-resampled DTM was flattening below their documented
depth. Add a 4-connected flood-fill connectivity gate to verify_mask.py
(scipy.ndimage.label) so every harbor snap must reach open water at its
gate depth, with a documented per-harbor exception list for genuinely
shallow approaches (Augustenborg 2.8m, Marstal 2.0m, both cited from their
own approachNote text).

Reduces disconnected harbors from 14/33 to 5/33. The remaining 5 (Arnis,
Dyvig, Graasten, Kappeln, Maasholm) are documented as known concerns in
.superpowers/sdd/fix-mask-report.md rather than force-connected with
unjustified exceptions - their approach notes don't support a depth
exception (Kappeln's fairway is charted at ~5m) or the blocker is land,
not depth (Dyvig's sub-cell-width channel, Graasten's bascule bridge).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cripts

Self-review (pr-review-toolkit code-reviewer + comment-analyzer) on PR #8
found: the resampling-switch comment overstated conservatism guarantees
(bilinear averaging can raise a depth estimate above what max() would have
given, unlike the floor step); the OSM feature-count comment embedded a
transient "this week's export" measurement as if it were a permanent
invariant; the n_land threshold had no derivation comment despite two
competing effects (resolution 4x vs. all_touched flip) acting on it; and
verify_mask.py duplicated the lat/lon->row/col transform in both depth_m()
and rc_of(), plus referenced a gitignored report file that ships to no one
who clones the repo. Fixed all four; no logic changes (verify_mask.py
re-run to confirm identical connectivity output).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DocGerd

DocGerd commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

Self-review

Ran pr-review-toolkit:code-reviewer + pr-review-toolkit:comment-analyzer against the diff before reporting this done. No critical bugs (the flood-fill/exception logic and coordinate transforms were verified correct — both the "snap cell itself too shallow" and "snap cell cut off from the main body" cases are handled). Four accuracy/maintainability findings, all fixed in 62deac9:

  1. build_mask.py — the comment on the Resampling.bilinear switch claimed conservatism was "unchanged," but averaging (unlike max()) can report a cell deeper than its shallowest contributing source pixel. Reworded to state the real tradeoff.
  2. build_mask.py — the OSM feature-count assertion comment embedded this-regen's measured numbers ("117 features... ~95k vertices") as if they were a permanent invariant justifying the threshold. Reworded to state the general policy (existence check only, real coverage enforced elsewhere) instead of citing a transient measurement as the rationale.
  3. build_mask.pyn_land > 200000 had no derivation comment, despite two competing effects acting on it in this same diff (4x resolution vs. all_touched flip reducing fringe cells). Added one.
  4. verify_mask.pydepth_m() and rc_of() duplicated the same lat/lon→row/col transform; deduped. Also removed a reference to fix-mask-report.md, which is gitignored and ships to no one who clones the repo — replaced with a reference to this PR.

🤖 Generated with Claude Code

Patrick Kuhn and others added 3 commits July 15, 2026 13:03
…sions

verify_mask.py exited non-zero on the 5 harbors documented as unresolvable
in the PR #8 report, meaning the shipped verifier was permanently red on
the committed, correct mask.bin - unable to distinguish a known limitation
from a new regression. Add KNOWN_DISCONNECTED (harbor id -> reason, citing
issue #9) alongside CONNECTIVITY_EXCEPTIONS_M: a listed harbor that's still
disconnected is reported as KNOWN and does not fail the run; a harbor NOT
listed that's disconnected still fails (regression); and a listed harbor
that turns out connected also fails, with a "remove the stale entry"
message, so the allowlist can't silently drift out of date as the data
improves.

Verified the stale-entry branch by patching a copy of the script to list
a genuinely-connected harbor and confirming it fails with that exact
message, then confirmed the real script exits 0 against the committed
mask, printing the full 28 OK / 2 exception / 5 known-disconnected table.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Controller review found the unconditional Resampling.max -> bilinear switch
(62deac9) fixed the 4 named channels but also manufactured depth broadly
elsewhere: measured 22,948 land->water flips vs. pure max in the actual
final encoded output, 1,780 of them >= 3.0 m (the app's default safety
depth), 97.6% of those >1 km from any harbor snap - i.e. mostly outside
the channels this fix was meant to help, violating "never overstate depth"
project-wide.

Fix: compute both Resampling.max and Resampling.bilinear reprojections from
the same source and blend per cell - trust bilinear only where it agrees
with max within TOLERANCE_M (smooth, gently-varying depth); otherwise fall
back to the conservative max value (a source discontinuity bilinear can't
be trusted to interpolate correctly).

TOLERANCE_M=2.0 satisfies both tuning constraints on the first value tried:
verify_mask.py reports the identical 28 OK / 2 exceptions / 5 known-
disconnected connectivity table (exit 0), while land->water flips crossing
3.0 m (and 5.0 m) drop from 1,780 (663) to zero.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread pipeline/build_mask.py
Comment thread pipeline/build_mask.py
Comment thread pipeline/verify_mask.py
Comment thread pipeline/verify_mask.py
@DocGerd

DocGerd commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

Self-review cycle complete: implementer-run lenses (code, comments) + controller review + independent reviewer with empirical re-derivation of the resampling numbers. The PR-body staleness finding from the independent review is resolved (body rewritten at 25899b2). All 4 inline threads above are resolved; the blend's 38-cell residual is documented in-code and its permanent fix (settings-UI draft clamp) is Phase E intake. Ready to merge pending CI.

@DocGerd
DocGerd merged commit 053598b into main Jul 15, 2026
1 check passed
@DocGerd
DocGerd deleted the fix/mask-harbor-connectivity branch July 15, 2026 11:35
DocGerd pushed a commit that referenced this pull request Jul 17, 2026
…odeQL)

CodeQL js/incomplete-multi-character-sanitization re-fired on the moved
copyright-holder sanitizer (alert #8): the `.replace(/<[^>]*>/g, '')`
pair-regex is flagged for the removal itself, and the trailing residual
`[<>]` strip does not silence the per-call heuristic.

Replace both bracket regexes with a depth-counting character scanner
(stripAngleBracketed) that runs after the paren strip. The scanner never
emits `<` or `>`, so the result is bracket-free by construction with no
regex removal that could be "incomplete". Ordering the scanner last also
catches the paren-splice case (`<(x)script` -> `<script` -> truncated).

Notices output stays byte-identical for well-formed `Name <email> (url)`
authors; two test expectations change under the new semantics (unclosed
`<script` truncates; the splice falls back), re-derived by hand and pinned.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxREWu8gax6BnxGcW3n4mH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mask: 14 of 44 harbors disconnected from open water at default safety depth (incl. Flensburg)

1 participant