diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 88da1f213..c52fa66cf 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -1075,9 +1075,18 @@ async function withCachedPlaces( } export const mapsApi = { - search: (query: string, lang?: string) => + /** + * `locationBias` is what tells the search which "Hase-dera" is meant, and it + * is the difference between finding the temple the user stands next to and + * one 400km away. The route has always accepted it; nothing passed it. + * + * It also decides whether the index answers at all: a common single word + * without coordinates is refused upstream as too expensive, and the search + * then falls back to Nominatim alone. + */ + search: (query: string, lang?: string, locationBias?: { lat: number; lng: number; radius?: number }) => withCachedPlaces(query, (places) => ({ places, source: 'offline-cache' }), () => - apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => checkInDev(mapsSearchResultSchema, r.data, 'maps.search'))), + apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query, locationBias }).then(r => checkInDev(mapsSearchResultSchema, r.data, 'maps.search'))), autocomplete: (input: string, lang?: string, locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, signal?: AbortSignal, sessionToken?: string) => withCachedPlaces( input, diff --git a/client/src/components/Planner/AddressInput.tsx b/client/src/components/Planner/AddressInput.tsx index 50ed98b32..13ce18ef2 100644 --- a/client/src/components/Planner/AddressInput.tsx +++ b/client/src/components/Planner/AddressInput.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react' import { MapPin } from 'lucide-react' import { mapsApi } from '../../api/client' import { useTranslation } from '../../i18n' +import { useLocationBias } from '../../hooks/useLocationBias' interface Props { value: string @@ -16,6 +17,8 @@ interface Props { // is never lost, and picking a suggestion just replaces the text (#1496). export default function AddressInput({ value, onChange, placeholder, className }: Props) { const { t, locale } = useTranslation() + // Ohne Reisekontext ist der Hinweis leer, und die Suche laeuft wie bisher. + const { point: locationBias } = useLocationBias() const [open, setOpen] = useState(false) const [results, setResults] = useState([]) const [highlight, setHighlight] = useState(-1) @@ -47,7 +50,7 @@ export default function AddressInput({ value, onChange, placeholder, className } const myReq = ++reqIdRef.current setLoading(true) try { - const data = await mapsApi.search(trimmed, locale) + const data = await mapsApi.search(trimmed, locale, locationBias) if (myReq !== reqIdRef.current) return setResults(data.places || []) setHighlight(-1) diff --git a/client/src/components/Planner/LocationSelect.tsx b/client/src/components/Planner/LocationSelect.tsx index 7547ef356..ae7fbc1b4 100644 --- a/client/src/components/Planner/LocationSelect.tsx +++ b/client/src/components/Planner/LocationSelect.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react' import { MapPin, X } from 'lucide-react' import { mapsApi } from '../../api/client' import { useTranslation } from '../../i18n' +import { useLocationBias } from '../../hooks/useLocationBias' export interface LocationPoint { name: string @@ -19,6 +20,8 @@ interface Props { export default function LocationSelect({ value, onChange, placeholder, style }: Props) { const { t, locale } = useTranslation() + // Ohne Reisekontext ist der Hinweis leer, und die Suche laeuft wie bisher. + const { point: locationBias } = useLocationBias() const [query, setQuery] = useState(value?.name || '') const [open, setOpen] = useState(false) const [results, setResults] = useState([]) @@ -49,7 +52,7 @@ export default function LocationSelect({ value, onChange, placeholder, style }: debounceRef.current = setTimeout(async () => { setLoading(true) try { - const data = await mapsApi.search(trimmed, locale) + const data = await mapsApi.search(trimmed, locale, locationBias) setResults(data.places || []) setHighlight(-1) } catch { diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index fe16c7ff8..a83fc3c90 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -17,6 +17,7 @@ import { useTranslation } from '../../i18n' import CustomTimePicker from '../shared/CustomTimePicker' import { DEFAULT_FORM, isGoogleMapsUrl, mergeResult, type PlaceFormData, type ResultField } from './PlaceFormModal.helpers' import { getApiErrorMessage } from '../../utils/apiError' +import { useLocationBias } from '../../hooks/useLocationBias' import { BookingCostsSection } from './BookingCostsSection' import type { BookingExpenseRequest } from './BookingCostsSection.types' import type { Place, Category, Assignment, BudgetItem } from '../../types' @@ -222,32 +223,11 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } }, [isOpen]) - // Derive location bias bounding box from the trip's existing places const places = useTripStore((s) => s.places) - const locationBias = useMemo(() => { - const withCoords = (places || []).filter((p) => p.lat != null && p.lng != null) - if (withCoords.length === 0) return undefined - - let minLat = Infinity, maxLat = -Infinity, minLng = Infinity, maxLng = -Infinity - for (const p of withCoords) { - const lat = Number(p.lat), lng = Number(p.lng) - if (!Number.isFinite(lat) || !Number.isFinite(lng)) continue - if (lat < minLat) minLat = lat - if (lat > maxLat) maxLat = lat - if (lng < minLng) minLng = lng - if (lng > maxLng) maxLng = lng - } - if (!Number.isFinite(minLat)) return undefined - - // Skip bias if the bounding box is too large (~500 km diagonal) - const dlat = maxLat - minLat - const dlng = maxLng - minLng - const avgLatRad = ((minLat + maxLat) / 2) * (Math.PI / 180) - const diagKm = Math.sqrt((dlat * 111) ** 2 + (dlng * 111 * Math.cos(avgLatRad)) ** 2) - if (diagKm > 500) return undefined - - return { low: { lat: minLat, lng: minLng }, high: { lat: maxLat, lng: maxLng } } - }, [places]) + // Where the trip is happening — the hint that tells the search which of a + // thousand identically named places is meant. Autocomplete wants a box, the + // search wants a point; useLocationBias derives both from the same places. + const { box: locationBias, point: locationBiasPoint } = useLocationBias() // Autocomplete fetch — aborts any in-flight request before starting a new one const fetchSuggestions = useCallback(async (query: string) => { @@ -320,7 +300,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { return } } - const result = await mapsApi.search(mapsSearch, language) + const result = await mapsApi.search(mapsSearch, language, locationBiasPoint) searchMetaRef.current = { query: mapsSearch.trim(), source: result.source || 'unknown' } setMapsResults(result.places || []) } catch (err: unknown) { @@ -409,7 +389,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } if (!place) { const query = [suggestion.mainText, suggestion.secondaryText].filter(Boolean).join(', ') - const search = await mapsApi.search(query, language) + const search = await mapsApi.search(query, language, locationBiasPoint) place = search.places?.[0] ?? null } if (place) { diff --git a/client/src/hooks/useLocationBias.test.ts b/client/src/hooks/useLocationBias.test.ts new file mode 100644 index 000000000..de8229357 --- /dev/null +++ b/client/src/hooks/useLocationBias.test.ts @@ -0,0 +1,83 @@ +// FE-HOOK-LOCBIAS-001 to FE-HOOK-LOCBIAS-008 +import { renderHook } from '@testing-library/react'; +import { useTripStore } from '../store/tripStore'; +import { boxFromCoords, pointFromBox, radiusKm, useLocationBias } from './useLocationBias'; + +// Coordinates of real places, because the whole point of the hint is distance +// and a made-up grid would not show when a box stops being useful. +const KYOTO_A = { id: 1, lat: 35.0116, lng: 135.7681 }; // Kyoto Bahnhof +const KYOTO_B = { id: 2, lat: 34.9949, lng: 135.7850 }; // Kiyomizu-dera, ~2 km +const TOKIO = { id: 3, lat: 35.6762, lng: 139.6503 }; // ~370 km entfernt + +function seed(state: Record) { + useTripStore.setState({ places: [], assignments: {}, selectedDayId: null, ...state } as never); +} + +afterEach(() => seed({})); + +describe('boxFromCoords', () => { + it('ignoriert Eintraege ohne brauchbare Koordinaten', () => { + const box = boxFromCoords([KYOTO_A, { lat: null, lng: null }, { lat: 'x', lng: 'y' }]); + expect(box).toEqual({ low: { lat: KYOTO_A.lat, lng: KYOTO_A.lng }, high: { lat: KYOTO_A.lat, lng: KYOTO_A.lng } }); + }); + + it('gibt nichts zurueck, wenn keine Koordinate brauchbar ist', () => { + expect(boxFromCoords([{ lat: null, lng: null }])).toBeUndefined(); + expect(boxFromCoords([])).toBeUndefined(); + }); +}); + +describe('pointFromBox', () => { + it('legt den Punkt in die Mitte und deckt die Box mit dem Radius ab', () => { + const punkt = pointFromBox(boxFromCoords([KYOTO_A, KYOTO_B])); + expect(punkt!.lat).toBeCloseTo((KYOTO_A.lat + KYOTO_B.lat) / 2, 6); + expect(punkt!.lng).toBeCloseTo((KYOTO_A.lng + KYOTO_B.lng) / 2, 6); + // gut 2 km Diagonale, also rund 1 km Radius — angehoben auf die Untergrenze + expect(punkt!.radius).toBe(5000); + }); + + it('haelt eine Untergrenze ein, damit ein einzelner Ort die Nachbarschaft meint', () => { + // Ein Punkt ohne Ausdehnung wuerde sonst mit Radius 0 auf die Tuerschwelle zeigen. + expect(pointFromBox(boxFromCoords([KYOTO_A]))!.radius).toBe(5000); + }); +}); + +describe('useLocationBias', () => { + it('gibt ohne Orte keinen Hinweis', () => { + seed({}); + expect(renderHook(() => useLocationBias()).result.current).toEqual({}); + }); + + it('nimmt die Orte des geoeffneten Tages, nicht die der ganzen Reise', () => { + // Die Reise reicht von Kyoto bis Tokio; der offene Tag spielt in Kyoto. + seed({ + places: [KYOTO_A, KYOTO_B, TOKIO], + assignments: { '7': [{ place_id: 1 }, { place_id: 2 }] }, + selectedDayId: 7, + }); + const { box } = renderHook(() => useLocationBias()).result.current; + expect(box).toBeDefined(); + // Tokio darf nicht in der Box liegen, sonst zeigt der Hinweis ins Leere. + expect(box!.high.lng).toBeLessThan(TOKIO.lng); + expect(radiusKm(box!)).toBeLessThan(5); + }); + + it('faellt auf die Reise zurueck, solange sie in eine Region passt', () => { + seed({ places: [KYOTO_A, KYOTO_B] }); + const { box, point } = renderHook(() => useLocationBias()).result.current; + expect(box).toBeDefined(); + expect(point!.lat).toBeCloseTo((KYOTO_A.lat + KYOTO_B.lat) / 2, 6); + }); + + it('verwirft einen Hinweis, der ueber eine ganze Rundreise spannt', () => { + // Gemessen: eine Box ueber Kyoto UND Tokio kostet mehr Treffer auf Platz 1 + // als sie einbringt. Kein Hinweis ist dann die bessere Antwort. + seed({ places: [KYOTO_A, TOKIO] }); + expect(renderHook(() => useLocationBias()).result.current).toEqual({}); + }); + + it('verwirft die zu weite Reise-Box auch dann, wenn ein Tag gewaehlt aber leer ist', () => { + seed({ places: [KYOTO_A, TOKIO], assignments: { '7': [] }, selectedDayId: 7 }); + expect(renderHook(() => useLocationBias()).result.current).toEqual({}); + }); +}); diff --git a/client/src/hooks/useLocationBias.ts b/client/src/hooks/useLocationBias.ts new file mode 100644 index 000000000..3e1f06fa7 --- /dev/null +++ b/client/src/hooks/useLocationBias.ts @@ -0,0 +1,125 @@ +import { useMemo } from 'react' +import { useTripStore } from '../store/tripStore' + +/** + * Where the user is planning right now, as a hint for the place search. + * + * Without it the search cannot tell which of a thousand identically named + * places is meant: "Hase-dera" returns the temple in Nara rather than the one + * in Kamakura, and a common single word is refused by the index as too + * expensive to answer, which drops the search back to Nominatim alone. + * + * The hint has to be NARROW, and that is the part that is easy to get wrong. + * Measured over 126 places of a real trip through Japan, searched against a + * live instance: + * + * no hint at all 54.8% found, 38.1% at rank 1 + * box around the WHOLE trip 57.1% found, 17.5% at rank 1 + * box around the nearest few places 70.6% found, 34.9% at rank 1 + * + * A box spanning a whole round trip has its centre somewhere between the + * cities, points at nothing, and costs more rank-1 hits than it gains. It is + * worse than no hint. So the day being planned comes first, and a hint that + * ends up wider than a metropolitan area is dropped rather than sent. + */ + +export interface LocationBiasBox { + low: { lat: number; lng: number } + high: { lat: number; lng: number } +} + +export interface LocationBiasPoint { + lat: number + lng: number + radius?: number +} + +/** + * Widest hint still worth sending, as a radius. Roughly a metropolitan area: + * far enough to cover a city and its surroundings, narrow enough that its + * centre still means something. The old value here was 500km of diagonal, + * which is how the whole-trip box slipped through. + */ +const MAX_RADIUS_KM = 60 + +const KM_PER_DEGREE = 111 + +interface Coord { lat?: number | string | null; lng?: number | string | null } + +export function boxFromCoords(coords: Coord[]): LocationBiasBox | undefined { + let minLat = Infinity, maxLat = -Infinity, minLng = Infinity, maxLng = -Infinity + for (const p of coords) { + // Number(null) is 0, and 0/0 is a real coordinate in the Gulf of Guinea. + // A place without coordinates would otherwise stretch the box across half + // the planet and the hint would be dropped for being too wide. + if (p.lat == null || p.lng == null || p.lat === '' || p.lng === '') continue + const lat = Number(p.lat), lng = Number(p.lng) + if (!Number.isFinite(lat) || !Number.isFinite(lng)) continue + if (lat < minLat) minLat = lat + if (lat > maxLat) maxLat = lat + if (lng < minLng) minLng = lng + if (lng > maxLng) maxLng = lng + } + if (!Number.isFinite(minLat)) return undefined + return { low: { lat: minLat, lng: minLng }, high: { lat: maxLat, lng: maxLng } } +} + +/** Half the diagonal of a box, in kilometres — how far the hint reaches. */ +export function radiusKm(box: LocationBiasBox): number { + const lat = (box.low.lat + box.high.lat) / 2 + const dLat = box.high.lat - box.low.lat + const dLng = box.high.lng - box.low.lng + return Math.sqrt( + (dLat * KM_PER_DEGREE) ** 2 + (dLng * KM_PER_DEGREE * Math.cos(lat * (Math.PI / 180))) ** 2, + ) / 2 +} + +/** + * The centre of a box, with the radius that covers it. + * + * Autocomplete takes a box, search takes a point — same hint, two shapes, + * because the two upstream APIs were built years apart. The radius has a floor + * so a day whose places sit on one street still biases toward the + * neighbourhood rather than the doorstep. + */ +export function pointFromBox(box: LocationBiasBox | undefined): LocationBiasPoint | undefined { + if (!box) return undefined + return { + lat: (box.low.lat + box.high.lat) / 2, + lng: (box.low.lng + box.high.lng) / 2, + radius: Math.round(Math.max(radiusKm(box), 5) * 1000), + } +} + +/** + * The hint for the place search, in both shapes the two endpoints want. + * + * Preference order, narrowest first: + * 1. the places of the day currently open — this is the area being planned + * 2. the trip's places, but only if they still fit inside one region + * Anything wider is dropped: no hint beats a hint pointing at open country. + */ +export function useLocationBias(): { box?: LocationBiasBox; point?: LocationBiasPoint } { + const places = useTripStore((s) => s.places) + const assignments = useTripStore((s) => s.assignments) + const selectedDayId = useTripStore((s) => s.selectedDayId) + + return useMemo(() => { + const alle = places || [] + + // The day being planned. Places reach a day through assignments, so the + // ids come from there and the coordinates from the place pool. + let quelle: Coord[] = [] + if (selectedDayId != null) { + const desTages = new Set( + (assignments?.[String(selectedDayId)] || []).map((a) => a.place_id), + ) + if (desTages.size > 0) quelle = alle.filter((p) => desTages.has(Number(p.id))) + } + if (quelle.length === 0) quelle = alle + + const box = boxFromCoords(quelle) + if (!box || radiusKm(box) > MAX_RADIUS_KM) return {} + return { box, point: pointFromBox(box) } + }, [places, assignments, selectedDayId]) +} diff --git a/client/src/mobile/screens/trip/sheets/MPlaceEditSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceEditSheet.tsx index 7d6d3a32f..a9027dbb8 100644 --- a/client/src/mobile/screens/trip/sheets/MPlaceEditSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MPlaceEditSheet.tsx @@ -16,6 +16,7 @@ import PlTimeFields from './PlTimeFields' import PlFileAttach from './PlFileAttach' import type { Place } from '../../../../types' import type { TripPlanner } from '../MTripShell' +import { useLocationBias } from '../../../../hooks/useLocationBias' export interface MPlaceEditSheetProps { planner: TripPlanner @@ -146,25 +147,10 @@ export default function MPlaceEditSheet({ planner, onOpenExpense }: MPlaceEditSh // eslint-disable-next-line react-hooks/exhaustive-deps }, [showPlaceForm, editingPlace, prefillCoords, editingAssignmentId]) - // Trip-centre bias for search/autocomplete, skipped past ~500 km diagonal. - const locationBias = useMemo(() => { - const withCoords = (places || []).filter(p => p.lat != null && p.lng != null) - if (withCoords.length === 0) return undefined - let minLat = Infinity, maxLat = -Infinity, minLng = Infinity, maxLng = -Infinity - for (const p of withCoords) { - const lat = Number(p.lat), lng = Number(p.lng) - if (!Number.isFinite(lat) || !Number.isFinite(lng)) continue - if (lat < minLat) minLat = lat - if (lat > maxLat) maxLat = lat - if (lng < minLng) minLng = lng - if (lng > maxLng) maxLng = lng - } - if (!Number.isFinite(minLat)) return undefined - const avgLatRad = ((minLat + maxLat) / 2) * (Math.PI / 180) - const diagKm = Math.sqrt(((maxLat - minLat) * 111) ** 2 + ((maxLng - minLng) * 111 * Math.cos(avgLatRad)) ** 2) - if (diagKm > 500) return undefined - return { low: { lat: minLat, lng: minLng }, high: { lat: maxLat, lng: maxLng } } - }, [places]) + // The area being planned, as a hint for search and autocomplete. Same helper + // as the desktop dialog: the day currently open first, the whole trip only + // while it still fits inside one region. + const { box: locationBias } = useLocationBias() const handleChange = (field: keyof PlaceFormData, value: string) => { // Typed by hand, so the next pick must leave it alone. diff --git a/client/src/mobile/screens/trip/sheets/PlPlaceSearch.tsx b/client/src/mobile/screens/trip/sheets/PlPlaceSearch.tsx index 72af6d0e3..af52465d9 100644 --- a/client/src/mobile/screens/trip/sheets/PlPlaceSearch.tsx +++ b/client/src/mobile/screens/trip/sheets/PlPlaceSearch.tsx @@ -5,6 +5,7 @@ import { recordPlacePick } from '../../../../api/placeShadow' import { PlacesSession } from '../../../../utils/placesSession' import { isGoogleMapsUrl } from '../../../../components/Planner/PlaceFormModal.helpers' import { getApiErrorMessage } from '../../../../utils/apiError' +import { pointFromBox } from '../../../../hooks/useLocationBias' import { FIELD_CLS } from './PlSheetChrome' import type { TripPlanner } from '../MTripShell' @@ -177,7 +178,9 @@ export default function PlPlaceSearch({ planner, locationBias, onPick, onResolvi return } } - const result = await mapsApi.search(trimmed, language) + // Derselbe Hinweis, den die Vervollstaendigung schon bekommt: die Suche + // braucht ihn genauso, nur als Punkt statt als Kasten. + const result = await mapsApi.search(trimmed, language, pointFromBox(locationBias)) searchMetaRef.current = { query: trimmed, source: result.source || 'unknown' } setResults(result.places || []) } catch (err: unknown) {