fix(routing): close the three #20-review solver gaps (issue #21)

[DocGerd]

Closes the three theoretical solver gaps the #20 fix review identified in app/src/routing/isochrone.ts — all verified real, none previously observed in tests.

Gap 1 — time-blind visited pruning across desynchronized clocks

visited stored min maneuvers only; with mixed per-node clocks after substepping, a later-clock node reaching a prune cell in an earlier ring could stamp it and prune a later-ring-but-earlier-clock substepped thread (realistic worst case: ETA suboptimality). The map now stores a VisitedStamp — componentwise minima of arrival clock and maneuver count — and a candidate is pruned only when the stamp dominates on BOTH axes (visitedDominates), the fix direction prescribed in the issue. Componentwise minima can combine two different stampers into a dominator neither was alone; that residual is documented in code and is strictly less pruning than the maneuvers-only rule it replaces (the new rule adds a conjunct). Termination is preserved: clocks are monotone along paths and per-cell minima stabilize.

Testing honesty: I could not construct an end-to-end synthetic-mask trigger with independently derivable expectations, for structural reasons:

1. the endpoint-capture check runs on child creation before the visited check, so pruning near the destination cannot block an arrival;
2. a pruned child's sibling candidates (headings every ~10°, near-identical clocks) land in different prune cells and continue, so an ETA delta needs the stamper's sparse per-step trail to cover the entire continuation of the pruned thread — only conceivable inside a ≤ 1-prune-cell (~190 m) mandatory corridor;
3. even there, such a trigger is constructible in principle — design-forward geometry of the kind gaps 2/3 use would produce one — but the cost/benefit is poor: which prune cell each lineage lands in depends on continuous entry offsets, so the expectations would be tuned against solver output (the feat(profile): depth-over-time route profile with wind/heading indicators #50 tautology trap). The invariant is pinned by the helper truth-table tests instead.

Per the issue's framing ("none observed in any test", spurious unreachable "would need contrived horizon-edge geometry") the invariant is instead pinned at unit level on the exported helpers (visitedDominates truth table, stampVisited componentwise-minima semantics, hand literals). Known limitation, stated openly: the call-site mutant seen.maneuvers <= child.maneuvers (i.e. pre-change behavior) survives the suite — verified by an explicit mutation check. The stamping call site is now swap-proof by construction: stampVisited takes a single named-field VisitedStamp, so its tMs and maneuvers can no longer be transposed the way two same-typed positional numbers could.

Gap 2 — blocked direct-arrival candidates get the substep retry

The direct-arrival branch consumed its candidate with continue even when the arrival leg was blocked, so the direct heading never got the dtS/2..dtS/8 substep retry every other blocked candidate gets — the destination-pocket mirror of the #20 origin-pocket fix. The continue now sits inside the navigable branch; the blocked case falls through to the normal step with its substep retry. (The full step along the same great circle is always blocked when the direct line is, so the fall-through reaches the substep loop.)

Test: fine 50 m-cell mask, full-height wall 326 m east of the origin, destination 501 m east (0.2707 nm ≤ the 0.3 nm full step, so the direct candidate takes the arrival branch). Wind 12 kn from N → heading 090 = TWA −90 → TEST_POLAR grid point 7.2 kn exactly; first substep 7.2 kn × 75 s = 0.15 nm = 278 m fits with 48 m margin. Heading 090 exists only as the direct candidate (EXTRA_TWAS has ±85/±95; beat 42°, gybe 165° at 12 kn), and the nearest non-direct substep endpoint (heading 085) lies 24.2 m away, so a 0.005 nm tolerance on a segmentNavigable spy uniquely identifies the direct retry. The sealed pocket stays a correct no-route/unreachable.

Gap 3 — the endpoint-capture hop is mask-validated

The final capture hop child→destination was the only solver edge that skipped segmentNavigable. It is now validated like every other edge.

Test: destination 451 m east of the origin, 75 m behind the wall — the ring-1 heading-085 substep child ends √((451.2−276.7)² + 24.2²) = 176 m = 0.0951 nm from it, inside the 0.1 nm capture radius, while every line from that water to the destination crosses the wall. Before the fix, solve returned an ok route whose final hop crossed land; now the sealed pocket is correctly no-route/unreachable.

Mutation checks

Mutant	Result
gap-3 validation removed	killed (gap-3 test, plus gap-2 test via a capture leak)
gap-2 unconditional continue restored	killed (exactly the gap-2 assertion)
gap-1 call site made time-blind again	survives — expected and documented above

Deliberately skipped

The issue's optional pinned "Flensburg→Marstal at 3.0 m stays unreachable" correct-negative test is not added: #53 will deliberately change exactly that behavior (find the shallow route and warn the user), so pinning it now would create churn. The behavior remains documented in realmask.repro.test.ts.

Verification

• npm --prefix app run typecheck / lint — clean.
• New app/src/routing/isochrone.followups.test.ts — 4/4 pass (0.8 s).
• Full suite npm --prefix app run test — 50 files / 478 tests all green, wall 4:19.
• Perf (gap 1 touches the hot loop, measured before/after on the same machine): invariants.property + realmask.repro vitest duration 242.9 s → 284.1 s (+17%); realmask.repro alone 43.3 s → 49.9 s; Flensburg→Marstal 38.8 s → 43.7 s (+12.7%). The cost is the intended semantics — clock-aware pruning prunes strictly less, widening the search front; MAX_FRONTIER still bounds the worst case. Because 43.7 s × the documented 6-10× CI slowdown brushes the Marstal test's old 240 s per-test budget, the second commit raises that explicit timeout to 600 s (loosening only — no timeout was tightened).

Closes #21

🤖 Generated with Claude Code

[DocGerd]

PR #66 review — out-of-diff / non-blocking notes (2 Minors)

Minor 1 — gap-1 core change has no end-to-end regression test (surviving time-blind mutant). Confirmed: reverting the call site app/src/routing/isochrone.ts:312 from visitedDominates(seen, child) to seen.maneuvers <= child.maneuvers (pre-change behavior) survives the whole suite. The helper unit tests exercise visitedDominates/stampVisited directly, not the wiring, and gap-2/gap-3 tests don't depend on gap-1's pruning semantics.

Ruling: acceptable as documented — no pinning test required. I re-derived the monotone-improvement claim independently: stampVisited computes seen.maneuvers as the same componentwise-min-maneuvers value the old rule stored, so the new prune predicate is (seen.maneuvers <= child.maneuvers) && (seen.tMs <= child.tMs) — the old predicate AND an extra conjunct. Therefore new-prune ⊆ old-prune (strictly less pruning). Reverting the call site degrades to the already-green old behavior, not to a bug; best only ever lowers on etaMs < best.etaMs, so wider exploration can only equal or improve ETA; termination is independent of pruning (every child advances the clock ≥ dtS/8, so minTMs rises ≥ ~18.75 s/ring toward horizonMs). The stamping side is additionally type-enforced (old visited.set(k, n.maneuvers) no longer compiles). An honest end-to-end trigger would need a boolean reachability flip through a ≤1-prune-cell corridor with hand-forced cell coincidence — not derivable without tuning against solver internals (the #50 tautology trap), so I do not ask for one. Flagging only so the phase gate knows the PR's central change rests on the proof above rather than a red-to-green test.

Minor 2 — open-water +43 % wall-time is overhead, not search-widening. In synchronized open water the tMs conjunct is near-always true (earlier rings carry smaller clocks), so pruning reduces to the old maneuvers rule and the search shape is unchanged there. The measured +43 % (3.63→5.19 s, +1.6 s abs) is therefore per-op overhead — VisitedStamp object allocation + GC replacing a boxed number, plus some coastal-substep desync near the fjord mouth. Small absolute, design needs two components stored, not worth optimizing; noted for awareness. The Marstal +12.7 % / property +17 % are the intended semantic cost (substep desync ⇒ real widening).

Both are non-actionable; the only fix I'm requesting is the Major timeout widening on the inline thread.