Merge pull request #2390 from liketrek/fix/roadtrip-day-routes

fix(roadtrip): a stop on a dead end took the whole day's routing with it

client/src/components/Budget/CostsPanel.test.tsx

@@ -744,7 +744,7 @@ describe('CostsPanel — settlements in the ledger', () => {
 
 type Flow = { from: { user_id: number; username: string }; to: { user_id: number; username: string }; amount: number }
 type Balance = { user_id: number; username: string; avatar_url: string | null; balance: number }
-type Payment = { id: number; from_user_id: number; to_user_id: number; amount: number; currency?: string | null; created_at?: string; settled_at?: string | null }
+type Payment = { id: number; from_user_id: number; to_user_id: number; amount: number; currency?: string | null; exchange_rate?: number; created_at?: string; settled_at?: string | null }
 
 // `members` here is the wire shape the panel reads; `paid` is only set by the server.
 type MemberFixture = { user_id: number; username?: string; amount?: number; paid?: number }

client/src/components/Map/RouteCalculator.test.ts

@@ -975,3 +975,77 @@ describe('calculateAlternatives', () => {
       .rejects.toMatchObject({ name: 'RoutingRefusedError', status: 429 })
   })
 })
+
+// ── turning round at a stop ────────────────────────────────────────────────────
+
+/**
+ * Gouffre de Padirac snaps onto `Route du Puits au Salvage`, which leads nowhere else.
+ * OSRM's car profile forbids a u-turn at an INTERMEDIATE waypoint unless asked, and it
+ * refuses the ENTIRE request when it cannot obey, so a road-trip day lost every leg it
+ * had over one cave car park, while each of its pairs routed perfectly on its own.
+ */
+describe('turning round at a stop', () => {
+  const askedFor = async (
+    call: (points: { lat: number; lng: number }[]) => Promise<unknown>,
+    count: number,
+  ): Promise<string> => {
+    let asked = ''
+    server.use(http.get(`${FOSSGIS.driving}/:coords`, ({ request }) => {
+      asked = request.url
+      return HttpResponse.json(buildOsrmRouteResponse())
+    }))
+    await call(freshWaypoints(count))
+    return asked
+  }
+
+  it('FE-COMP-ROUTECALCULATOR-055: a drive that stops on the way may turn round where it stopped', async () => {
+    expect(await askedFor(p => calculateRoute(p), 3)).toContain('continue_straight=false')
+    expect(await askedFor(p => calculateRouteWithLegs(p), 3)).toContain('continue_straight=false')
+    expect(await askedFor(p => calculateSegments(p), 3)).toContain('continue_straight=false')
+  })
+
+  it('FE-COMP-ROUTECALCULATOR-056: two points have no middle, so the request is the one it always was', async () => {
+    expect(await askedFor(p => calculateRoute(p), 2)).not.toContain('continue_straight')
+    expect(await askedFor(p => calculateRouteWithLegs(p), 2)).not.toContain('continue_straight')
+    expect(await askedFor(p => calculateSegments(p), 2)).not.toContain('continue_straight')
+  })
+
+  it('FE-COMP-ROUTECALCULATOR-057: a router that has never heard of the permission is asked without it, once and then always', async () => {
+    // OSRM refuses an unknown parameter outright instead of ignoring it, and says which
+    // it is refusing: `InvalidQuery` names the query string, `NoRoute` names the road.
+    // Without this a router that predates the parameter would refuse every drive with a
+    // stop in the middle, which is a far worse fault than the one it fixes.
+    useSettingsStore.setState(st => ({ settings: { ...st.settings, routing_base_url: 'https://old-osrm.example.org' } }))
+    const asked: string[] = []
+    server.use(http.get('https://old-osrm.example.org/route/v1/driving/:coords', ({ request }) => {
+      asked.push(request.url)
+      return new URL(request.url).searchParams.has('continue_straight')
+        ? HttpResponse.json({ code: 'InvalidQuery', message: 'Query string malformed' }, { status: 400 })
+        : HttpResponse.json(buildOsrmRouteResponse())
+    }))
+
+    const first = await calculateRoute(freshWaypoints(3))
+    expect(first.coordinates).toHaveLength(2)
+    expect(asked).toHaveLength(2)
+    expect(asked[1]).not.toContain('continue_straight')
+
+    // Remembered per host, so the second drive costs one request rather than two.
+    await calculateRoute(freshWaypoints(3))
+    expect(asked).toHaveLength(3)
+    expect(asked[2]).not.toContain('continue_straight')
+    useSettingsStore.setState(st => ({ settings: { ...st.settings, routing_base_url: '' } }))
+  })
+
+  it('FE-COMP-ROUTECALCULATOR-058: a refusal about the road is still a refusal, not a second request', async () => {
+    useSettingsStore.setState(st => ({ settings: { ...st.settings, routing_base_url: 'https://noroute-osrm.example.org' } }))
+    let calls = 0
+    server.use(http.get('https://noroute-osrm.example.org/route/v1/driving/:coords', () => {
+      calls += 1
+      return HttpResponse.json({ code: 'NoRoute', message: 'No route found between points' }, { status: 400 })
+    }))
+
+    await expect(calculateRoute(freshWaypoints(3))).rejects.toThrow()
+    expect(calls).toBe(1)
+    useSettingsStore.setState(st => ({ settings: { ...st.settings, routing_base_url: '' } }))
+  })
+})

client/src/components/Map/RouteCalculator.ts

@@ -53,6 +53,31 @@ function routeBaseFor(profile: 'driving' | 'walking' | 'cycling'): string {
   return `${withoutTrailingSlashes(configured)}/route/v1/${OSRM_PROFILE_PATH[profile]}`
 }
 
+/**
+ * Permission to turn around at a stop, for requests that have stops to turn around at.
+ *
+ * OSRM's car profile forbids a u-turn at an INTERMEDIATE waypoint unless asked otherwise,
+ * and a refusal is not local: the whole request comes back `400 NoRoute`, every leg of it,
+ * however many were fine. A stop on a dead end is enough. Gouffre de Padirac snaps onto
+ * `Route du Puits au Salvage`, which goes nowhere else, so a day that visits it in the
+ * middle lost all its legs at once while each of its pairs routed perfectly on its own.
+ * That is also why the fault looked like it depended on how the stops were spread across
+ * days: a pair on a day boundary is asked for on its own, and a request with nothing in
+ * the middle has nothing to refuse.
+ *
+ * Correct as well as convenient here. TREK's waypoints are places somebody stops at, not
+ * shape hints, and the way out of a dead end IS the way back in, and the constraint was
+ * buying a detour even where it did not refuse outright.
+ *
+ * Empty for two waypoints, which have no middle, so the many two-point callers
+ * (alternatives, the day connectors, booking geometry) keep the URL they had.
+ */
+const U_TURN_PARAM = '&continue_straight=false'
+
+function uTurnParam(waypoints: readonly Waypoint[]): string {
+  return waypoints.length > 2 ? U_TURN_PARAM : ''
+}
+
 /**
  * `fetch`, with the request counted.
  *
@@ -79,10 +104,16 @@ async function routedFetch(
   let km = 0
   for (let i = 1; i < waypoints.length; i++) km += haversineKm(waypoints[i - 1], waypoints[i])
   const sample = { profile, surface: kind, selfHosted, waypoints: waypoints.length, km }
+  const asked = withoutUTurnIfRefused(url)
   try {
-    const response = await fetch(url, { signal })
+    const response = await fetch(asked, { signal })
     countRoute({ ...sample, failed: !response.ok })
-    return response
+    if (response.ok || !asked.includes(U_TURN_PARAM)) return response
+    const plain = await droppedUTurn(asked, response)
+    if (!plain) return response
+    const second = await fetch(plain, { signal })
+    countRoute({ ...sample, failed: !second.ok })
+    return second
   } catch (err) {
     if (!(err instanceof DOMException && err.name === 'AbortError')) {
       countRoute({ ...sample, failed: true })
@@ -91,6 +122,49 @@ async function routedFetch(
   }
 }
 
+/**
+ * Hosts that answered `InvalidQuery` to `continue_straight`, by origin.
+ *
+ * OSRM refuses an unknown query parameter outright rather than ignoring it, so a router
+ * that has never heard of this one would refuse EVERY request with a stop in the middle,
+ * which is a far worse fault than the one it fixes. `osrm-routed` has understood it
+ * since 5.6, so this is about something else answering under `routing_base_url`, and the
+ * first real request is the probe: there is no separate one.
+ *
+ * By origin rather than per profile, because a router either knows the parameter or does
+ * not; and in a Set rather than a setting, so pointing the instance at a router that does
+ * know it starts asking again without a reload. Same shape and the same reasoning as
+ * `excludeUnsupported` below.
+ */
+const uTurnUnsupported = new Set<string>()
+
+const originOf = (url: string): string => {
+  try { return new URL(url).origin } catch { return url }
+}
+
+/** The url as it should go out, with the permission dropped for a host that refused it. */
+function withoutUTurnIfRefused(url: string): string {
+  if (!url.includes(U_TURN_PARAM)) return url
+  return uTurnUnsupported.has(originOf(url)) ? url.replace(U_TURN_PARAM, '') : url
+}
+
+/**
+ * The same url without the u-turn permission, once it is clear the host objects to the
+ * PARAMETER and not to the route. Null when the refusal was about the route itself.
+ *
+ * OSRM says which it is: `InvalidQuery` names the query string, `NoRoute` names the road.
+ * Reading the body is safe here because this response is about to be thrown away either
+ * way, and it only happens on a 400 for a request that carried the parameter.
+ */
+async function droppedUTurn(url: string, response: Response): Promise<string | null> {
+  if (response.status !== 400) return null
+  let code: unknown
+  try { code = (await response.clone().json())?.code } catch { return null }
+  if (code !== 'InvalidQuery') return null
+  uTurnUnsupported.add(originOf(url))
+  return url.replace(U_TURN_PARAM, '')
+}
+
 /**
  * A routing request the host refused, as opposed to one it answered with "no route".
  *
@@ -153,7 +227,7 @@ export async function calculateRoute(
   }
 
   const coords = waypoints.map((p) => `${p.lng},${p.lat}`).join(';')
-  const url = `${routeBaseFor(profile)}/${coords}?overview=full&geometries=geojson&steps=false`
+  const url = `${routeBaseFor(profile)}/${coords}?overview=full&geometries=geojson&steps=false${uTurnParam(waypoints)}`
 
   const response = await routedFetch(url, signal, 'route', profile, waypoints)
   if (!response.ok) {
@@ -360,7 +434,7 @@ export async function calculateSegments(
   if (!waypoints || waypoints.length < 2) return []
 
   const coords = waypoints.map((p) => `${p.lng},${p.lat}`).join(';')
-  const url = `${routeBaseFor('driving')}/${coords}?overview=false&geometries=geojson&steps=false&annotations=distance,duration`
+  const url = `${routeBaseFor('driving')}/${coords}?overview=false&geometries=geojson&steps=false&annotations=distance,duration${uTurnParam(waypoints)}`
 
   const response = await routedFetch(url, signal, 'segments', 'driving', waypoints)
   if (!response.ok) throw new RoutingRefusedError(response.status, retryAfterMs(response))
@@ -488,7 +562,7 @@ export async function calculateRouteWithLegs(
   // (plugins name their own modes), which no comparison narrows to the three OSRM knows.
   const osrmProfile: 'driving' | 'walking' | 'cycling' =
     profile === 'walking' ? 'walking' : profile === 'cycling' ? 'cycling' : 'driving'
-  const url = `${routeBaseFor(osrmProfile)}/${coords}?overview=full&geometries=geojson&annotations=distance,duration`
+  const url = `${routeBaseFor(osrmProfile)}/${coords}?overview=full&geometries=geojson&annotations=distance,duration${uTurnParam(waypoints)}`
   const response = await routedFetch(url, signal, 'legs', osrmProfile, waypoints)
   if (!response.ok) throw new RoutingRefusedError(response.status, retryAfterMs(response))
 

client/src/components/Planner/PlaceFormModal.test.tsx

@@ -1772,6 +1772,16 @@ describe('PlaceFormModal as a road trip service stop', () => {
     expect(screen.getAllByText('Add as a stop').length).toBeGreaterThan(0);
   });
 
+  it('FE-PLANNER-PLACEFORM-085: it opens on the kind the corridor was looking for, dwell and all', () => {
+    // The panel seeds itself to charging on an electric car, and a form opening on a
+    // fuel pump under a picker that says Charging is the fault this carries the answer to.
+    render(<PlaceFormModal {...defaultProps} serviceStop={serviceStop({ defaultKind: 'charging' })} />);
+
+    expect(screen.getByRole('button', { name: 'Charging' })).toHaveAttribute('aria-pressed', 'true');
+    expect(screen.getByRole('button', { name: /30 min/ })).toHaveAttribute('aria-pressed', 'true');
+    expect(screen.getByRole('button', { name: 'Fuel' })).toHaveAttribute('aria-pressed', 'false');
+  });
+
   it('FE-PLANNER-PLACEFORM-076: picking a kind also picks how long that kind takes', async () => {
     const onSave = vi.fn().mockResolvedValue({ id: 9 });
     render(<PlaceFormModal {...defaultProps} onSave={onSave} serviceStop={serviceStop()} />);

client/src/components/Planner/PlaceFormModal.tsx

@@ -254,10 +254,15 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
       // path exists to avoid. Read out of the closure rather than watched, because the
       // mode is fixed for the life of one opening and a rebuilt drive must not reset a
       // half-filled form.
+      // What the corridor panel was looking for when the button was pressed, because
+      // that is the traveller's own answer to what they are adding; only a panel with
+      // nothing selected falls back to the constant. The dwell follows the kind, the
+      // same way picking one by hand moves it.
+      const kind = serviceStop.defaultKind ?? DEFAULT_SERVICE_KIND
       setForm({
         ...DEFAULT_FORM,
-        stop_type: DEFAULT_SERVICE_KIND,
-        duration_minutes: STOP_KIND_BY_KEY[DEFAULT_SERVICE_KIND].defaultMinutes,
+        stop_type: kind,
+        duration_minutes: STOP_KIND_BY_KEY[kind].defaultMinutes,
       })
     } else {
       setForm(DEFAULT_FORM)

client/src/components/Roadtrip/RoadtripCorridorPanel.test.tsx

@@ -441,6 +441,35 @@ describe('RoadtripCorridorPanel', () => {
     // nothing is added until a place has been found and saved in there.
     fireEvent.click(manual)
     expect(onAddManual).toHaveBeenCalledTimes(1)
+    // The kind, never the MouseEvent: the categories above the button are the answer to
+    // what is being added, and a form that ignored them opened every manual stop on fuel.
+    expect(onAddManual).toHaveBeenCalledWith('fuel')
+  })
+
+  it('FE-ROADTRIP-PANEL-035: the manual add opens on what the panel is looking for', () => {
+    const open = (categories: string[]): unknown => {
+      const onAddManual = vi.fn()
+      const { unmount } = wrap(
+        <RoadtripCorridorPanel
+          corridor={corridor({ categories })}
+          routes={routes([day(1, 1)])}
+          onAddPoi={vi.fn()}
+          onAddManual={onAddManual}
+        />,
+      )
+      fireEvent.click(searchRow()[1])
+      unmount()
+      return onAddManual.mock.calls[0][0]
+    }
+
+    // An electric car seeds the panel to charging, and this is the case the fault was
+    // reported on: the panel said Charging and the form opened on Fuel.
+    expect(open(['charging'])).toBe('charging')
+    expect(open(['restaurant'])).toBe('restaurant')
+    // Multi-select: the first the panel LISTS wins, rather than a kind nobody switched on.
+    expect(open(['sights', 'rest_area'])).toBe('rest_area')
+    // Nothing selected is no answer, and the form falls back on its own.
+    expect(open([])).toBeNull()
   })
 
   it('FE-ROADTRIP-PANEL-026: with no drive on the trip there is nowhere to add one by hand', () => {

client/src/components/Roadtrip/RoadtripCorridorPanel.tsx

@@ -13,9 +13,10 @@ import CustomSelect from '../shared/CustomSelect'
 import RoadtripCategoryPicker from './RoadtripCategoryPicker'
 import { serviceColor } from './roadtripModel'
 import { alongLabel, offRouteLabel } from './corridorSearchModel'
-import { CORRIDOR_CATEGORY_BY_KEY } from './stopKinds'
+import { CORRIDOR_CATEGORY_BY_KEY, manualStopKindFor } from './stopKinds'
 import { FS } from './typeScale'
 import { CORRIDOR_CATEGORY_KEYS, CORRIDOR_SECTION_KM, CORRIDOR_WIDTHS_KM, type RoadtripCorridor } from './useRoadtripCorridor'
+import type { RoadtripStopType } from '@trek/shared'
 import type { CorridorPoi } from './useCorridorPois'
 import type { RoadtripRoutes } from './useRoadtripRoutes'
 
@@ -38,8 +39,12 @@ interface RoadtripCorridorPanelProps {
   /**
    * Opens the place form for a stop the search never found. Given only to somebody who
    * may add places, which is what keeps the button off a reader's panel.
+   *
+   * Carries the kind the panel is looking for, so the form opens on it rather than on a
+   * constant: the categories above the button are the traveller's own answer to "what am
+   * I adding", and ignoring them made every manual stop start life as a fuel stop.
    */
-  onAddManual?: () => void
+  onAddManual?: (kind: RoadtripStopType | null) => void
 }
 
 /**
@@ -476,7 +481,9 @@ export default function RoadtripCorridorPanel({
           {onAddPoi && onAddManual && corridor.day ? (
             <button
               type="button"
-              onClick={onAddManual}
+              // Wrapped rather than handed the prop directly: `onClick` would otherwise
+              // pass the MouseEvent as the kind.
+              onClick={() => onAddManual(manualStopKindFor(corridor.categories))}
               // The full sentence is the accessible name and the tooltip; the face of
               // the button carries the one word that fits beside "Search".
               aria-label={t('roadtrip.poi.addManual')}

client/src/components/Roadtrip/manualStop.ts

@@ -36,6 +36,13 @@ export interface ServiceStopDay {
 
 /** Everything the form needs to ask where a service stop belongs, and nothing else. */
 export interface ServiceStopMode {
+  /**
+   * The kind the form opens on, from what the corridor panel is looking for.
+   *
+   * Absent or null when the panel had nothing selected, and then `DEFAULT_SERVICE_KIND`
+   * stands in. Optional so a caller with no panel behind it stays unchanged.
+   */
+  defaultKind?: RoadtripStopType | null
   days: ServiceStopDay[]
   /** Where a stop goes at the end of the day the panel is on, routed or not. */
   appendDay: { dayId: number; dayNumber: number; position: number } | null
@@ -99,12 +106,13 @@ export interface ServiceStopChoice {
 }
 
 /**
- * The kind the form opens on.
+ * The kind the form opens on when nothing better is known.
  *
- * The panel it is reached from is headed by pumps and chargers, and one click is the
- * whole distance to any of the others. Opening on nothing would be worse than a guess: a
- * stop left without a kind is a numbered destination that counts in every total, which
- * is the very thing this path exists to avoid.
+ * Nothing better means the panel had no category switched on at all; normally the form
+ * opens on what the corridor is looking for (`manualStopKindFor`), because that is the
+ * traveller's own answer rather than a guess. Opening on nothing would be worse than
+ * either: a stop left without a kind is a numbered destination that counts in every
+ * total, which is the very thing this path exists to avoid.
  */
 export const DEFAULT_SERVICE_KIND: RoadtripStopType = 'fuel'
 

client/src/components/Roadtrip/stopKinds.test.ts

@@ -3,7 +3,7 @@ import { roadtripStopTypeSchema } from '@trek/shared'
 import { SERVICE_STOP_TYPES, SERVICE_COLORS } from './roadtripModel'
 import {
   STOP_KINDS, STOP_KIND_BY_KEY, CORRIDOR_CATEGORIES, CORRIDOR_CATEGORY_BY_KEY, CORRIDOR_CATEGORY_KEYS,
-  SERVICE_KIND_KEYS, REFUELLING_STOP_TYPES, isOvernightCategory,
+  SERVICE_KIND_KEYS, REFUELLING_STOP_TYPES, isOvernightCategory, manualStopKindFor,
 } from './stopKinds'
 
 /**
@@ -77,4 +77,19 @@ describe('stop kinds', () => {
       expect(Number.isInteger(kind.defaultMinutes)).toBe(true)
     }
   })
+
+  it('FE-STOPKIND-008: the manual add opens on what the corridor is looking for', () => {
+    // A stop added by hand used to open on fuel whatever the panel above it said, which
+    // on an electric car meant a picker reading Charging over a form reading Fuel.
+    for (const category of CORRIDOR_CATEGORIES) {
+      expect(manualStopKindFor([category.key])).toBe(category.stopKind)
+    }
+    // Multi-select, so the first the panel LISTS wins: falling back would open on a kind
+    // that is not among the ones switched on, which is a worse answer than any of them.
+    const [first, second] = CORRIDOR_CATEGORY_KEYS
+    expect(manualStopKindFor([second, first])).toBe(CORRIDOR_CATEGORY_BY_KEY[first].stopKind)
+    // Nothing selected is no answer at all, and the caller decides what to do with that.
+    expect(manualStopKindFor([])).toBeNull()
+    expect(manualStopKindFor(['not-a-category'])).toBeNull()
+  })
 })

client/src/components/Roadtrip/stopKinds.ts

@@ -86,6 +86,26 @@ export const CORRIDOR_CATEGORY_BY_KEY: Record<string, CorridorCategory> = Object
 /** The keys the corridor search offers, in the order the panel shows them. */
 export const CORRIDOR_CATEGORY_KEYS = CORRIDOR_CATEGORIES.map(c => c.key)
 
+/**
+ * The kind a stop added by hand opens on, read off what the corridor is looking for.
+ *
+ * The categories are what the traveller has just told the search to find, so they are
+ * also the best reading of what they are about to add themselves, and on an electric
+ * car the panel seeds itself to charging, which is precisely the case where a form
+ * opening on a fuel pump read as a fault.
+ *
+ * The picker is multi-select, so the first category the panel LISTS wins. Falling back
+ * instead would open on a kind that is not among the ones switched on, which is a worse
+ * answer than any of them. Null only when nothing is selected at all; the caller decides
+ * what that means, because a form must never open on no kind (see `DEFAULT_SERVICE_KIND`).
+ */
+export function manualStopKindFor(categories: readonly string[]): RoadtripStopType | null {
+  for (const category of CORRIDOR_CATEGORIES) {
+    if (categories.includes(category.key) && category.stopKind) return category.stopKind
+  }
+  return null
+}
+
 /** Categories whose hit becomes a night rather than a pause. */
 export function isOvernightCategory(category: string | null | undefined): boolean {
   // Campsite is both: it is a service stop today and stays one, but a night can be

client/src/components/Roadtrip/useRoadtripRoutes.test.ts

@@ -278,6 +278,42 @@ describe('useRoadtripRoutes', () => {
     expect(result.current.totalDistance).toBe(100000)
   })
 
+  it('FE-ROADTRIP-ROUTES-028: a day the router refuses outright is asked for again one pair at a time', async () => {
+    const days = [day(1, 1)]
+    const stops: StopSpec[] = [{ id: 1, at: HAMBURG }, { id: 2, at: LUENEBURG }, { id: 3, at: BERLIN }]
+    // The Padirac case: one stop in the middle the router will not turn round at, and
+    // what comes back is a refusal of the WHOLE chain rather than of that one leg.
+    calculateRouteWithLegs
+      .mockRejectedValueOnce(new RoutingRefusedError(400, null))
+      .mockResolvedValue(routed(1))
+
+    const { result } = renderHook(() => useRoadtripRoutes(7, days, map(1, stops)))
+    await waitFor(() => expect(result.current.loading).toBe(false), { timeout: 5000 })
+
+    // The run once, refused, then a request per pair, and no retry of the run, because
+    // the same coordinates earn the same refusal.
+    expect(calculateRouteWithLegs).toHaveBeenCalledTimes(3)
+    expect(calculateRouteWithLegs.mock.calls[1][0]).toHaveLength(2)
+    expect(calculateRouteWithLegs.mock.calls[2][0]).toHaveLength(2)
+    expect(result.current.days[0].legs.filter(Boolean)).toHaveLength(2)
+    expect(result.current.totalDistance).toBe(200000)
+  })
+
+  it('FE-ROADTRIP-ROUTES-029: a rate limit is waited out, never split into more requests', async () => {
+    vi.useFakeTimers()
+    const days = [day(1, 1)]
+    const stops: StopSpec[] = [{ id: 1, at: HAMBURG }, { id: 2, at: LUENEBURG }, { id: 3, at: BERLIN }]
+    calculateRouteWithLegs.mockRejectedValue(new RoutingRefusedError(429, null))
+
+    const { result } = renderHook(() => useRoadtripRoutes(7, days, map(1, stops)))
+    await act(async () => { await vi.advanceTimersByTimeAsync(20000) })
+
+    // Three attempts at the run and nothing after them: answering a host that asked for
+    // less traffic with two more requests would be the opposite of backing off.
+    expect(calculateRouteWithLegs).toHaveBeenCalledTimes(3)
+    expect(result.current.days[0].legs).toEqual([undefined, undefined])
+  })
+
   it('FE-ROADTRIP-ROUTES-009: renaming a place does not re-ask the router', async () => {
     const days = [day(1, 1)]
     const stops: StopSpec[] = [{ id: 1, at: HAMBURG }, { id: 2, at: BERLIN }]

client/src/components/Roadtrip/useRoadtripRoutes.ts

@@ -278,6 +278,110 @@ export function useRoadtripRoutes(
     const collectedMisses: Record<number, RouteAvoidClass[]> = {}
     const tasks: (() => Promise<void>)[] = []
 
+    /**
+     * Ask for one run, and when the router refuses the whole of it, ask for its legs
+     * one at a time instead.
+     *
+     * A run travels as a single request because the answer carries a leg per waypoint
+     * pair, which is exactly what the rail wants, and one request is one slot of a
+     * host that allows about one a second. What that thrift costs is that a refusal is
+     * never local: OSRM answers `400 NoRoute` for the entire chain when one stop in the
+     * MIDDLE of it sits where it may not turn around, so a day lost every leg it had
+     * over a single cave car park while each of its pairs routed perfectly alone.
+     * `uTurnParam` in RouteCalculator removes the usual cause; this removes the SHAPE of
+     * the failure, so a stop that really cannot be reached costs its own two legs and
+     * not the day around them.
+     *
+     * The split is enqueued, not awaited, so the pairs queue up behind everything else
+     * and keep the same spacing as any other request: a day falling back must not turn
+     * into a burst at the one host that refused it.
+     */
+    const enqueueRun = (day: PlanDay, dayLegs: Record<string, RoutedLeg>, run: RoadtripStop[], mode: string): void => {
+      /** The same stops, as one request per pair. A pair has nothing left to split. */
+      const splitIntoPairs = (): void => {
+        if (run.length <= 2) return
+        for (let i = 0; i < run.length - 1; i++) enqueueRun(day, dayLegs, [run[i], run[i + 1]], mode)
+      }
+      tasks.push(async () => {
+        // Waypoints are the stops with this day's vias threaded in between them, so the
+        // router draws the road the traveller picked. `stopAt` remembers which waypoint
+        // each stop became, because the answer has a leg per waypoint PAIR and the rail
+        // wants one leg per stop pair.
+        const waypoints: { lat: number; lng: number }[] = []
+        const stopAt: number[] = []
+        run.forEach((stop, i) => {
+          stopAt.push(waypoints.length)
+          waypoints.push({ lat: stop.lat, lng: stop.lng })
+          if (i === run.length - 1) return
+          // A via is filed against the day it was dropped on and the position it sits
+          // after IN THAT DAY. Since a chain can hold stops from an earlier day (see
+          // `nightSpill.ts`), the lookup has to be the stop's own day and its own
+          // index — this used to be the position within the chain, so on any day that
+          // received a night drive every via matched nothing and quietly stopped
+          // shaping the road.
+          ;(viasByDay[stop.ownerDayId ?? day.dayId] ?? [])
+            .filter(v => v.after_order_index === (stop.ownerIndex ?? i))
+            .sort((a, b) => a.sequence - b.sequence)
+            .forEach(v => waypoints.push({ lat: v.lat, lng: v.lng }))
+        })
+
+        for (let attempt = 0; attempt <= RETRY_DELAYS_MS.length; attempt++) {
+          if (controller.signal.aborted) return
+          try {
+            const r = await calculateRouteWithLegs(
+              waypoints,
+              { signal: controller.signal, profile: mode, tripId: tripId ?? null, dayId: day.dayId, avoid },
+            )
+            // Where each stop ended up. stopAt[i] is that stop's waypoint index, so the
+            // vias threaded in between are skipped: a via is a shape handle, not a
+            // destination, and a dashed spur hanging off one reads as a fault.
+            if (r.snapped) {
+              const daySnaps = (collectedSnaps[day.dayId] ??= {})
+              run.forEach((stop, i) => {
+                const s = r.snapped?.[stopAt[i]]
+                if (s) daySnaps[stopKey(stop)] = s
+              })
+            }
+            // What was asked for against what the road turned out to be. Only
+            // the second engine reports it, and only when it answered: an OSRM
+            // fallback leaves `avoidance` absent, which is the honest reading
+            // of "the weighting never happened". Collected here because the
+            // rail plans in the browser and never sees the server's own copy
+            // of this field — without it the "not honoured" badge could not
+            // appear at all, and a motorway-free drive that is not one read as
+            // if the setting had held.
+            if (r.avoidance) {
+              const missed = r.avoidance.asked.filter(cls => !r.avoidance!.achieved.includes(cls))
+              if (missed.length) {
+                collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])]
+              }
+            }
+            Object.assign(dayLegs, foldRouteRun(run, stopAt, r, mode))
+            return
+          } catch (err) {
+            if (controller.signal.aborted) return
+            const refusal = err instanceof RoutingRefusedError ? err : null
+            // Almost always a rate limit on the shared routing host, or a connection
+            // that dropped: back off and try again. A refusal the host MEANT is the
+            // one thing not worth repeating: it objects to these coordinates, and
+            // asking twice more only spends five seconds earning the same 400.
+            const delay = refusal && !refusal.isRateLimit ? undefined : RETRY_DELAYS_MS[attempt]
+            if (delay !== undefined) {
+              // When the host says how long to wait, waiting less is just a second refusal.
+              await sleep(Math.max(delay, (refusal?.isRateLimit ? refusal.retryAfterMs : null) ?? 0), controller.signal)
+              continue
+            }
+            // Nothing left to try. A run of three or more stops asks again one pair at
+            // a time, so a stop the router will not route THROUGH costs its own two
+            // legs instead of every leg of the day around it. Never after a rate limit:
+            // the answer to a host asking for less traffic is not four more requests.
+            if (!refusal?.isRateLimit) splitIntoPairs()
+            return
+          }
+        }
+      })
+    }
+
     for (const day of plan) {
       const dayLegs: Record<string, RoutedLeg> = {}
       collected[day.dayId] = dayLegs
@@ -290,77 +394,7 @@ export function useRoadtripRoutes(
           dfMode,
         ))
 
-
-      for (const { stops: run, mode } of runs) {
-        tasks.push(async () => {
-          // Waypoints are the stops with this day's vias threaded in between them, so the
-          // router draws the road the traveller picked. `stopAt` remembers which waypoint
-          // each stop became, because the answer has a leg per waypoint PAIR and the rail
-          // wants one leg per stop pair.
-          const waypoints: { lat: number; lng: number }[] = []
-          const stopAt: number[] = []
-          run.forEach((stop, i) => {
-            stopAt.push(waypoints.length)
-            waypoints.push({ lat: stop.lat, lng: stop.lng })
-            if (i === run.length - 1) return
-            // A via is filed against the day it was dropped on and the position it sits
-            // after IN THAT DAY. Since a chain can hold stops from an earlier day (see
-            // `nightSpill.ts`), the lookup has to be the stop's own day and its own
-            // index — this used to be the position within the chain, so on any day that
-            // received a night drive every via matched nothing and quietly stopped
-            // shaping the road.
-            ;(viasByDay[stop.ownerDayId ?? day.dayId] ?? [])
-              .filter(v => v.after_order_index === (stop.ownerIndex ?? i))
-              .sort((a, b) => a.sequence - b.sequence)
-              .forEach(v => waypoints.push({ lat: v.lat, lng: v.lng }))
-          })
-
-          for (let attempt = 0; attempt <= RETRY_DELAYS_MS.length; attempt++) {
-            if (controller.signal.aborted) return
-            try {
-              const r = await calculateRouteWithLegs(
-                waypoints,
-                { signal: controller.signal, profile: mode, tripId: tripId ?? null, dayId: day.dayId, avoid },
-              )
-              // Where each stop ended up. stopAt[i] is that stop's waypoint index, so the
-              // vias threaded in between are skipped: a via is a shape handle, not a
-              // destination, and a dashed spur hanging off one reads as a fault.
-              if (r.snapped) {
-                const daySnaps = (collectedSnaps[day.dayId] ??= {})
-                run.forEach((stop, i) => {
-                  const s = r.snapped?.[stopAt[i]]
-                  if (s) daySnaps[stopKey(stop)] = s
-                })
-              }
-              // What was asked for against what the road turned out to be. Only
-              // the second engine reports it, and only when it answered: an OSRM
-              // fallback leaves `avoidance` absent, which is the honest reading
-              // of "the weighting never happened". Collected here because the
-              // rail plans in the browser and never sees the server's own copy
-              // of this field — without it the "not honoured" badge could not
-              // appear at all, and a motorway-free drive that is not one read as
-              // if the setting had held.
-              if (r.avoidance) {
-                const missed = r.avoidance.asked.filter(cls => !r.avoidance!.achieved.includes(cls))
-                if (missed.length) {
-                  collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])]
-                }
-              }
-              Object.assign(dayLegs, foldRouteRun(run, stopAt, r, mode))
-              return
-            } catch (err) {
-              // Almost always a rate limit on the shared routing host. Back off and try
-              // again; a run that still won't route (island hop, dead router) simply stays
-              // blank, and the totals say so by being partial rather than wrong.
-              const delay = RETRY_DELAYS_MS[attempt]
-              if (delay === undefined) return
-              // When the host says how long to wait, waiting less is just a second refusal.
-              const asked = err instanceof RoutingRefusedError && err.isRateLimit ? err.retryAfterMs : null
-              await sleep(Math.max(delay, asked ?? 0), controller.signal)
-            }
-          }
-        })
-      }
+      for (const { stops: run, mode } of runs) enqueueRun(day, dayLegs, run, mode)
     }
 
     // One at a time, spaced out. The day sidebar can afford a small pool because it
@@ -402,7 +436,12 @@ export function useRoadtripRoutes(
   const allLegs = useMemo(() => {
     const out: Record<string, RoutedLeg> = {}
     for (const day of plan) Object.assign(out, legsByDay[day.dayId] ?? {})
-    return { ...out, ...seamLegs }
+    // Seams first, this day's own runs over the top. `legsByDay` is replaced whole on
+    // every routing round while `seamLegs` only ever grows, so a pair that used to sit on
+    // a day boundary and now sits inside one day keeps an answer nobody re-asks for, and
+    // that older answer was shaped by the vias of a stop that has since moved. Where both
+    // exist the day run is the newer of the two, so it is the one to believe.
+    return { ...seamLegs, ...out }
   }, [plan, legsByDay, seamLegs])
 
   // Which date each stop is actually reached on. Nothing is written to make it so; see

client/src/pages/tripPlanner/useTripPlanner.ts

@@ -315,6 +315,14 @@ export function useTripPlanner() {
    * form they have always been.
    */
   const [serviceStopForm, setServiceStopForm] = useState(false)
+  /**
+   * The kind that form opens on, taken from what the corridor panel was looking for.
+   *
+   * Beside the flag rather than inside it, because it is written by the same click and
+   * read by the same memo, and a second piece of state is cheaper to follow than a flag
+   * that is sometimes a boolean and sometimes an object.
+   */
+  const [serviceStopKind, setServiceStopKind] = useState<RoadtripStopType | null>(null)
   /**
    * The corridor hit waiting to become a stop, while the small popup is open.
    *
@@ -1628,7 +1636,7 @@ export function useTripPlanner() {
    * typed-ahead search, which is the whole reason to use it, and where the stop belongs
    * is worked out at the save, from what the save carries.
    */
-  const openManualRoadtripStop = useCallback(() => {
+  const openManualRoadtripStop = useCallback((kind: RoadtripStopType | null = null) => {
     if (!can('place_edit', trip)) return
     setEditingPlace(null)
     setEditingAssignmentId(null)
@@ -1637,6 +1645,9 @@ export function useTripPlanner() {
     // form opens; this one cannot, because nothing has been chosen yet.
     setPlaceFormDayId(null)
     setPlaceFormPosition(null)
+    // Set in the same batch as the flag below, so the kind is already there when the
+    // form's opening effect reads the mode out of its closure.
+    setServiceStopKind(kind)
     setServiceStopForm(true)
     setShowPlaceForm(true)
   }, [can, trip])
@@ -1653,6 +1664,7 @@ export function useTripPlanner() {
     if (!serviceStopForm) return null
     const panelDay = roadtripCorridor.day
     return {
+      defaultKind: serviceStopKind,
       // A day that has not routed has no order to place anything in.
       days: roadtripRoutes.days
         .filter(day => day.geometry.length > 1)
@@ -1680,7 +1692,7 @@ export function useTripPlanner() {
         : null,
       targetFor: manualStopTargetFor,
     }
-  }, [serviceStopForm, roadtripRoutes.days, roadtripCorridor.day, manualStopTargetFor])
+  }, [serviceStopForm, serviceStopKind, roadtripRoutes.days, roadtripCorridor.day, manualStopTargetFor])
 
   const addRoadtripVia = useCallback(async (lat: number, lng: number) => {
     if (!can('day_edit', trip)) return

server/src/nest/roadtrip/roadtrip-router.service.ts

@@ -244,8 +244,16 @@ export class RoadtripRouterService {
         const origin =
           base ||
           `https://routing.openstreetmap.de/routed-${mode === 'walking' ? 'foot' : mode === 'cycling' ? 'bike' : 'car'}`;
+        // `continue_straight=false` because a stop is somewhere the traveller stops, not a
+        // shape hint. OSRM's car profile otherwise forbids a u-turn at an INTERMEDIATE
+        // waypoint, and it refuses the WHOLE request rather than the one leg it cannot
+        // obey on: a single stop on a dead end (Gouffre de Padirac snaps onto a cave
+        // access road that leads nowhere else) turned a whole routed day into a 400. The
+        // browser sends the same thing, see `uTurnParam` in client RouteCalculator.ts.
+        // Only with something in the middle to turn round at.
+        const uTurn = points.length > 2 ? '&continue_straight=false' : '';
         const payload = await this.request(
-          `${origin}/route/v1/${path}/${points.map((p) => `${p.lng},${p.lat}`).join(';')}?overview=full&geometries=geojson&steps=false`,
+          `${origin}/route/v1/${path}/${points.map((p) => `${p.lng},${p.lat}`).join(';')}?overview=full&geometries=geojson&steps=false${uTurn}`,
         );
         const parsed = osrmSchema.parse(payload);
         const route = parsed.routes[0];

server/tests/unit/nest/roadtrip-router.test.ts

@@ -105,6 +105,31 @@ describe('server Roadtrip router', () => {
     expect((await pending).avoidMissed).toEqual(['ferry']);
     expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(2);
   });
+  it('lets a drive turn round at a stop it passes through, and asks nothing extra of a single leg', async () => {
+    // OSRM's car profile forbids a u-turn at an INTERMEDIATE waypoint unless asked, and
+    // refuses the WHOLE request when it cannot obey: one stop on a dead end used to
+    // cost a routed day all of its legs at once.
+    const { router } = setup();
+    await router.route(1, 1, 1, points, 'driving', []);
+    expect(safeFetchAdminConfigured).toHaveBeenCalledWith(
+      expect.stringContaining('continue_straight=false'),
+      expect.anything(),
+    );
+
+    vi.mocked(safeFetchAdminConfigured).mockResolvedValueOnce(
+      new Response(JSON.stringify({
+        code: 'Ok',
+        routes: [{ distance: 1000, duration: 100, geometry: { coordinates: [[10, 48], [11, 49]] }, legs: [{ distance: 1000, duration: 100 }] }],
+      })),
+    );
+    const pair = router.route(2, 1, 1, points.slice(0, 2), 'driving', []);
+    await vi.runAllTimersAsync();
+    await pair;
+    expect(safeFetchAdminConfigured).toHaveBeenLastCalledWith(
+      expect.not.stringContaining('continue_straight'),
+      expect.anything(),
+    );
+  });
   it('does not apply car avoidance to walking routes', async () => {
     const route = await setup().router.route(1, 1, 1, points, 'walking', ['toll']);
     expect(route.avoidMissed).toEqual([]);