Solver: adaptive substeps for real coastline geometry (Closes #20)#22
Merged
Conversation
… navigable (issue #20) Root cause: a full isochrone step (0.5-2 km at 150-600 s dt) is longer than real harbor arms are straight. Out of Flensburg (~350 m wide channel, bend within 1 km) every candidate heading's segment crossed land within one step, so the frontier died at the first expansion and the solver reported 'unreachable' for any route departing the harbor. Synthetic-mask tests never caught this because their channels are wider than one step. Fix, in isochrone.ts: - When a candidate's full-step segment fails segmentNavigable, retry the same heading at dt/2, dt/4, dt/8 and take the largest substep that fits; the child carries the honest shorter clock. Navigability still comes solely from segmentNavigable at query time; maneuver pricing is untouched. - better() prefers earlier tMs (no-op while clocks are synchronized) so same-cell pruning keeps earlier arrivals once substeps desynchronize. - Loop guards (best-ETA cutoff, beyond-horizon) use the earliest frontier clock, and children are horizon-checked individually; both no-ops in the synchronized case. Also corrects a stale cell-size comment in mask.ts (real mask is ~46 m cells, not ~557x321 m - those were the synthetic fixture's dimensions). New regression tests run against the real shipped mask/polars (realmask.repro.test.ts, own tsconfig.test.json for node builtins): Flensburg->Gluecksburg at defaults, a 5-direction wind sweep, and the spec acceptance Flensburg->Marstal - at safetyDepthM 2.3 because Marstal's snap cell only 4-connects to open water at <= 2.3 m in the shipped mask (see CONNECTIVITY_EXCEPTIONS_M in pipeline/verify_mask.py, PR #8); at the 3.0 m default 'unreachable' is the correct answer for this data. Full suite 110/110 green; property suite runtime unchanged (227.8s -> 226.2s). Closes #20 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
onProgress reported the global ring clock, which post-substepping can run up to 7/8 dtS ahead of the earliest live node. Report the new frontier's minimum clock instead (identical to the ring clock when no substeps occurred; empty frontier falls back to the ring). Regression-asserted in the real-mask suite where the first Flensburg frontier is guaranteed to be entirely substepped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DocGerd
commented
Jul 15, 2026
Owner
Author
|
Gate review (strong-model, doubles as this branch's self-review): Ready to merge — Yes. No-op claims verified by induction over the time-synchronized pre-substep frontier; termination structural + sealed-pocket probe (11 ms); heterogeneous-clock domination sound; design principles intact (query-time navigability only, no new passes, maneuver pricing unchanged); 110/110 incl. property suite at unchanged runtime, independently re-run. Residual theoretical gaps tracked in #21. The only post-review delta (55ac84a) is the review-prescribed progress fix, under reviewer confirmation now. |
DocGerd
commented
Jul 15, 2026
DocGerd
commented
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
A full isochrone step is longer than real harbor arms are straight. Out of Flensburg, every one of the 39 first-expansion candidates produced a 507–1165 m straight segment (dtS = 300 s at ~4 nm to go; 2× that at 600 s) inside a channel only ~200–420 m wide (4–9 cells at 46 m/cell) with a bend within ~1 km.
segmentNavigablecorrectly rejected all of them, the frontier went 1 → 0 at t = 0, and the solver reportedunreachablefor any route departing the harbor. Synthetic-mask tests never caught this because their channels are wider than one step.Hypotheses ruled out with instrumentation: heading quantization (headings within ~5° of the channel axis existed and died on segment length), MAX_FRONTIER (max observed 18,848 < 30,000; the death happens at frontier size 1), snap placement (origin moved ~14 m onto an 8.9 m cell).
Marstal premise correction
Issue #20's premise that "the flood-fill gate proves both harbor snaps reach open water at 3.0 m" is wrong for Marstal: its snap cell sits in a 119-cell pocket that 4-connects to open water only at safety depth ≤ 2.3 m (measured with the app's exact predicate; matches the
CONNECTIVITY_EXCEPTIONS_M = {"marstal": 2.0}entry from PR #8). At the 3.0 m default,unreachableis the correct answer for the shipped mask — a data limitation in the issue #9 family, not a solver bug.Fix
Three changes in
isochrone.ts, one uniform mechanism, no new passes:segmentNavigable, retry the same heading at dtS/2, dtS/4, dtS/8 and take the largest substep that fits; the child carries the honest shorter clock. Navigability still comes solely fromsegmentNavigableat query time; maneuver pricing is untouched (the penalty is charged once, and a substep whose effective distance reaches 0 stops the descent).better()prefers earliertMsfirst, so same-cell pruning keeps earlier arrivals once substeps desynchronize node clocks.onProgressreports the new frontier's minimum clock rather than the ring clock (which can drift up to 7/8 dtS ahead under substeps — relevant for the Phase E progress UI).All three are no-ops while the frontier is time-synchronized (the pre-fix invariant: no substeps ⇒ all clocks equal), which is why every synthetic golden/property test passes unmodified.
Acceptance tests (
realmask.repro.test.ts, real shipped mask + polars)DEFAULT_SETTINGSviaplanRoute: ok, both rigs, ~0.6 h for ~4 nm, every leg re-validated against the mask at 3.0 m.planRouteatsafetyDepthM: 2.3: ok, ETA 8.0 h, ~38 nm, legs mask-validated at 2.3 m. Runs at 2.3 m with the data caveat documented in-test; tighten toDEFAULT_SETTINGSif the mask ever resolves the dredged approach at 3.0 m.The real-asset test gets its own
tsconfig.test.json(node builtins) so node globals never leak into the browser app project.Suite evidence
tsc -b --noEmitandeslint srcclean.Remaining theoretical gaps (tracked in #21)
visited/byKeypruning is time-blind across desynchronized clocks (~220×190 m cells keyed by min maneuvers); substepped threading through prune-cell-sized dead ends could in theory starve — not observed in any tested geometry.Diagnosis report:
.superpowers/sdd/issue20-diagnosis.md.Closes #20
🤖 Generated with Claude Code