From b3d69aa760468f20bb743f7cdf4ebf57b696a279 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 5 Sep 2026 23:15:51 +0200 Subject: [PATCH] feat(places): record which search result was picked, on this instance only The corpus behind "is our own index good enough to replace Nominatim". When somebody searches and then picks a result, the client posts the query, the rank and the pick. Nothing else: no user, no trip, no session, and coordinates only to three decimals. Deliberately decoupled from the search endpoint. Correlating a response with a later pick would mean handing out a ticket on POST /api/maps/search and threading it through all fifteen call sites; the client already holds everything the row needs at the moment of the pick. Evaluation happens offline afterwards: export the corpus, run it against a candidate index, count how often the pick came back in the top five. That is why collection can start before such an index exists, which is the whole point, because the corpus needs weeks of real use and the index needs a server, and those two waits should not be serial. Off unless an admin switches it on, and it reads === 'true' rather than !== 'false'. The three older places switches fail open because a migration backfilled a row for installs already using them; nothing uses this one, so an instance that upgrades and never visits the admin panel logs nothing. Reading is admin only, retention is 180 days, and the switch state is on the unauthenticated app-config endpoint on purpose: whether an instance records what its users search for is something a visitor may know before logging in. --- client/src/App.tsx | 5 +- client/src/api/client.ts | 2 + client/src/api/placeShadow.test.ts | 98 +++++++ client/src/api/placeShadow.ts | 42 +++ .../src/components/Planner/PlaceFormModal.tsx | 47 +++- .../screens/admin/MAdminSettingsSection.tsx | 23 +- .../screens/trip/sheets/PlPlaceSearch.tsx | 40 ++- client/src/pages/admin/AdminSettingsTab.tsx | 28 +- client/src/pages/admin/useAdmin.ts | 9 +- client/src/store/authStore.ts | 6 + server/src/db/migrations.ts | 35 +++ server/src/nest/addons/addons.service.ts | 10 + server/src/nest/admin/admin.controller.ts | 10 + server/src/nest/app.module.ts | 3 +- server/src/nest/auth/auth.service.ts | 6 + .../place-shadow/place-shadow.controller.ts | 54 ++++ .../src/nest/place-shadow/place-shadow.dto.ts | 9 + .../src/nest/place-shadow/place-shadow.job.ts | 37 +++ .../nest/place-shadow/place-shadow.module.ts | 17 ++ .../nest/place-shadow/place-shadow.service.ts | 196 ++++++++++++++ server/tests/e2e/place-shadow.e2e.test.ts | 169 ++++++++++++ .../unit/nest/place-shadow.controller.test.ts | 64 +++++ .../tests/unit/nest/place-shadow.job.test.ts | 65 +++++ .../unit/nest/place-shadow.service.test.ts | 241 ++++++++++++++++++ shared/src/i18n/ar/admin.ts | 3 + shared/src/i18n/br/admin.ts | 3 + shared/src/i18n/ca/admin.ts | 3 + shared/src/i18n/cs/admin.ts | 3 + shared/src/i18n/de/admin.ts | 3 + shared/src/i18n/en/admin.ts | 3 + shared/src/i18n/es/admin.ts | 3 + shared/src/i18n/fr/admin.ts | 3 + shared/src/i18n/gr/admin.ts | 3 + shared/src/i18n/hu/admin.ts | 3 + shared/src/i18n/id/admin.ts | 3 + shared/src/i18n/it/admin.ts | 3 + shared/src/i18n/ja/admin.ts | 3 + shared/src/i18n/ko/admin.ts | 3 + shared/src/i18n/nl/admin.ts | 3 + shared/src/i18n/pl/admin.ts | 3 + shared/src/i18n/ru/admin.ts | 3 + shared/src/i18n/sv/admin.ts | 3 + shared/src/i18n/tr/admin.ts | 3 + shared/src/i18n/uk/admin.ts | 3 + shared/src/i18n/vi/admin.ts | 3 + shared/src/i18n/zh-TW/admin.ts | 3 + shared/src/i18n/zh/admin.ts | 3 + shared/src/index.ts | 1 + shared/src/maps/place-shadow.schema.ts | 115 +++++++++ 49 files changed, 1385 insertions(+), 16 deletions(-) create mode 100644 client/src/api/placeShadow.test.ts create mode 100644 client/src/api/placeShadow.ts create mode 100644 server/src/nest/place-shadow/place-shadow.controller.ts create mode 100644 server/src/nest/place-shadow/place-shadow.dto.ts create mode 100644 server/src/nest/place-shadow/place-shadow.job.ts create mode 100644 server/src/nest/place-shadow/place-shadow.module.ts create mode 100644 server/src/nest/place-shadow/place-shadow.service.ts create mode 100644 server/tests/e2e/place-shadow.e2e.test.ts create mode 100644 server/tests/unit/nest/place-shadow.controller.test.ts create mode 100644 server/tests/unit/nest/place-shadow.job.test.ts create mode 100644 server/tests/unit/nest/place-shadow.service.test.ts create mode 100644 shared/src/maps/place-shadow.schema.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index d71308729..509a943be 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -283,7 +283,7 @@ function RouteFallback() { } export default function App() { - const { loadUser, isAuthenticated, demoMode, setManaged, setDemoMode, setDevMode, setIsPrerelease, setAppVersion, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled, setPlacesPhotosEnabled, setPlacesAutocompleteEnabled, setPlacesDetailsEnabled, setPlacesEnrichEnabled } = useAuthStore() + const { loadUser, isAuthenticated, demoMode, setManaged, setDemoMode, setDevMode, setIsPrerelease, setAppVersion, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled, setPlacesPhotosEnabled, setPlacesAutocompleteEnabled, setPlacesDetailsEnabled, setPlacesEnrichEnabled, setPlaceShadowEnabled } = useAuthStore() const { loadSettings } = useSettingsStore() const { loadAddons } = useAddonStore() const { loadPlugins } = usePluginStore() @@ -300,7 +300,7 @@ export default function App() { loadUser() } } - authApi.getAppConfig().then(async (config: { managed?: boolean; demo_mode?: boolean; dev_mode?: boolean; is_prerelease?: boolean; has_maps_key?: boolean; version?: string; timezone?: string; require_mfa?: boolean; trip_reminders_enabled?: boolean; places_photos_enabled?: boolean; places_autocomplete_enabled?: boolean; places_details_enabled?: boolean; places_enrich_enabled?: boolean; permissions?: Record }) => { + authApi.getAppConfig().then(async (config: { managed?: boolean; demo_mode?: boolean; dev_mode?: boolean; is_prerelease?: boolean; has_maps_key?: boolean; version?: string; timezone?: string; require_mfa?: boolean; trip_reminders_enabled?: boolean; places_photos_enabled?: boolean; places_autocomplete_enabled?: boolean; places_details_enabled?: boolean; places_enrich_enabled?: boolean; place_shadow_enabled?: boolean; permissions?: Record }) => { setManaged(!!config?.managed) setDemoMode(!!config?.demo_mode) if (config?.dev_mode) setDevMode(true) @@ -314,6 +314,7 @@ export default function App() { if (config?.places_autocomplete_enabled !== undefined) setPlacesAutocompleteEnabled(config.places_autocomplete_enabled) if (config?.places_details_enabled !== undefined) setPlacesDetailsEnabled(config.places_details_enabled) if (config?.places_enrich_enabled !== undefined) setPlacesEnrichEnabled(config.places_enrich_enabled) + if (config?.place_shadow_enabled !== undefined) setPlaceShadowEnabled(config.place_shadow_enabled) if (config?.permissions) usePermissionsStore.getState().setPermissions(config.permissions) // A version is a short release tag and nothing else. It arrives over the diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 11d75ea5b..5e0e10c0c 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -647,6 +647,8 @@ export const adminApi = { updateBagTracking: (enabled: boolean) => apiClient.put('/admin/bag-tracking', { enabled }).then(r => r.data), getPlacesPhotos: () => apiClient.get('/admin/places-photos').then(r => r.data), updatePlacesPhotos: (enabled: boolean) => apiClient.put('/admin/places-photos', { enabled }).then(r => r.data), + getPlaceShadow: () => apiClient.get('/admin/place-shadow').then(r => r.data), + updatePlaceShadow: (enabled: boolean) => apiClient.put('/admin/place-shadow', { enabled }).then(r => r.data), getPlacesAutocomplete: () => apiClient.get('/admin/places-autocomplete').then(r => r.data), updatePlacesAutocomplete: (enabled: boolean) => apiClient.put('/admin/places-autocomplete', { enabled }).then(r => r.data), getPlacesDetails: () => apiClient.get('/admin/places-details').then(r => r.data), diff --git a/client/src/api/placeShadow.test.ts b/client/src/api/placeShadow.test.ts new file mode 100644 index 000000000..ab2b74f3e --- /dev/null +++ b/client/src/api/placeShadow.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { http, HttpResponse } from 'msw' +import { server } from '../../tests/helpers/msw/server' +import { useAuthStore } from '../store/authStore' +import { recordPlacePick } from './placeShadow' +import type { PlaceShadowPickRequest } from '@trek/shared' + +/** + * The point of this module is that it never gets in the way: it must not fire + * when the instance has the log switched off, must not throw when the request + * fails, and must not send coordinates finer than the ranking needs. + */ + +const PICK: PlaceShadowPickRequest = { + query: ' losteria rostock ', + lang: 'de', + biasLat: 54.088712, + biasLng: 12.140493, + source: 'search:nominatim', + liveRank: 0, + liveCount: 8, + pickedName: "L'Osteria", + pickedLat: 54.0891234, + pickedLng: 12.1372987, + pickedPlaceId: 'osm:node/1', +} + +/** Resolves with the body the module posted, or null if it never posted. */ +function capture(status = 200) { + let seen: unknown = null + let resolve: (v: unknown) => void = () => {} + const posted = new Promise(r => { resolve = r }) + server.use( + http.post('/api/place-shadow/pick', async ({ request }) => { + seen = await request.json() + resolve(seen) + return status === 200 + ? HttpResponse.json({ recorded: true }) + : HttpResponse.json({ error: 'nope' }, { status }) + }), + ) + return { posted, seen: () => seen } +} + +beforeEach(() => { + useAuthStore.setState({ placeShadowEnabled: true }) +}) + +describe('recordPlacePick', () => { + it('sends nothing at all when the instance has the log switched off', async () => { + useAuthStore.setState({ placeShadowEnabled: false }) + const cap = capture() + recordPlacePick(PICK) + await new Promise(r => setTimeout(r, 20)) + expect(cap.seen()).toBeNull() + }) + + it('trims the query and rounds every coordinate to three decimals', async () => { + const cap = capture() + recordPlacePick(PICK) + expect(await cap.posted).toMatchObject({ + query: 'losteria rostock', + pickedLat: 54.089, + pickedLng: 12.137, + biasLat: 54.089, + biasLng: 12.14, + }) + }) + + it('leaves an absent bias absent rather than sending a zero', async () => { + const cap = capture() + recordPlacePick({ ...PICK, biasLat: undefined, biasLng: undefined }) + const body = (await cap.posted) as Record + expect(body.biasLat).toBeUndefined() + expect(body.biasLng).toBeUndefined() + }) + + it('drops a pick with no query, no name or an impossible rank before sending', async () => { + const cap = capture() + recordPlacePick({ ...PICK, query: ' ' }) + recordPlacePick({ ...PICK, pickedName: '' }) + recordPlacePick({ ...PICK, liveRank: 8, liveCount: 8 }) + recordPlacePick({ ...PICK, pickedLat: Number.NaN }) + await new Promise(r => setTimeout(r, 20)) + expect(cap.seen()).toBeNull() + }) + + it('swallows a server error instead of surfacing it to the pick that triggered it', async () => { + const cap = capture(500) + const onUnhandled = vi.fn() + process.on('unhandledRejection', onUnhandled) + expect(() => recordPlacePick(PICK)).not.toThrow() + await cap.posted + await new Promise(r => setTimeout(r, 20)) + process.off('unhandledRejection', onUnhandled) + expect(onUnhandled).not.toHaveBeenCalled() + }) +}) diff --git a/client/src/api/placeShadow.ts b/client/src/api/placeShadow.ts new file mode 100644 index 000000000..73e1c7774 --- /dev/null +++ b/client/src/api/placeShadow.ts @@ -0,0 +1,42 @@ +import type { PlaceShadowPickRequest } from '@trek/shared' +import { apiClient } from './client' +import { useAuthStore } from '../store/authStore' + +/** + * Records which search result the user picked, for the local shadow corpus. + * + * Fire and forget in the strongest sense: it never throws, never blocks the + * pick it is describing, and does nothing at all unless the instance switched + * the log on. Adding a place must not get slower or more fragile because + * somebody is collecting a measurement. + * + * The store flag is checked first so a switched-off instance makes no request + * at all; the server checks it again, because a client is not a permission. + */ + +/** About 100 m. Enough to reproduce a ranking, too coarse to place someone. */ +function coarse(value: number): number { + return Math.round(value * 1000) / 1000 +} + +export function recordPlacePick(pick: PlaceShadowPickRequest): void { + if (!useAuthStore.getState().placeShadowEnabled) return + if (!Number.isFinite(pick.pickedLat) || !Number.isFinite(pick.pickedLng)) return + if (pick.liveRank >= pick.liveCount) return + + const body: PlaceShadowPickRequest = { + ...pick, + query: pick.query.trim().slice(0, 200), + pickedName: pick.pickedName.slice(0, 300), + pickedLat: coarse(pick.pickedLat), + pickedLng: coarse(pick.pickedLng), + biasLat: pick.biasLat === undefined ? undefined : coarse(pick.biasLat), + biasLng: pick.biasLng === undefined ? undefined : coarse(pick.biasLng), + } + // The contract requires both; an unnamed result is not worth a row anyway. + if (!body.query || !body.pickedName) return + + void apiClient.post('/place-shadow/pick', body).catch(() => { + // Deliberately silent. The user was adding a place, not running a study. + }) +} diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index d5d9b6f7e..fe16c7ff8 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -3,6 +3,7 @@ import Modal from '../shared/Modal' import CustomSelect from '../shared/CustomSelect' import NoteFormatToolbar from '../shared/NoteFormatToolbar' import { mapsApi } from '../../api/client' +import { recordPlacePick } from '../../api/placeShadow' import { useAuthStore } from '../../store/authStore' import { useCanDo } from '../../store/permissionsStore' import { useTripStore } from '../../store/tripStore' @@ -89,6 +90,14 @@ function usePlaceFormModal(props: PlaceFormModalProps) { const [form, setForm] = useState(DEFAULT_FORM) const [mapsSearch, setMapsSearch] = useState('') const [mapsResults, setMapsResults] = useState([]) + /** + * What produced the list currently on screen, kept for the shadow log: the + * query as typed and the provider the envelope named. A ref rather than + * state because nothing renders from it and a pick must read the value that + * belonged to the list, not a value a re-render replaced. + */ + const searchMetaRef = useRef<{ query: string; source: string } | null>(null) + const acMetaRef = useRef<{ query: string; source: string } | null>(null) const [isSearchingMaps, setIsSearchingMaps] = useState(false) const [newCategoryName, setNewCategoryName] = useState('') const [showNewCategory, setShowNewCategory] = useState(false) @@ -252,6 +261,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { acAbortRef.current = controller try { const result = await mapsApi.autocomplete(query, language, locationBias, controller.signal, placesSessionRef.current.current()) + acMetaRef.current = { query, source: result.source || 'unknown' } setAcSuggestions(result.suggestions || []) setAcHighlight(-1) } catch (err: unknown) { @@ -311,6 +321,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } } const result = await mapsApi.search(mapsSearch, language) + searchMetaRef.current = { query: mapsSearch.trim(), source: result.source || 'unknown' } setMapsResults(result.places || []) } catch (err: unknown) { toast.error(getApiErrorMessage(err, t('places.mapsSearchError'))) @@ -319,7 +330,13 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } } - const handleSelectMapsResult = (result) => { + /** + * `pick` is present only when the click came from a ranked list. The + * collection picker and the autocomplete detour reach this function with a + * place that was never ranked against a query, and a made-up rank would be + * worse than no row at all. + */ + const handleSelectMapsResult = (result, pick?: { mode: 'search' | 'autocomplete'; rank: number; count: number }) => { setForm(prev => mergeResult(prev, result, autoFilledRef.current)) // The one point every pick flows through, so the detail column hangs here. // A new pick drops whatever hero image belonged to the previous place. @@ -336,12 +353,36 @@ function usePlaceFormModal(props: PlaceFormModalProps) { details: result, }) setForm(prev => ({ ...prev, image_url: undefined })) + if (pick) { + const meta = pick.mode === 'search' ? searchMetaRef.current : acMetaRef.current + if (meta) { + recordPlacePick({ + query: meta.query, + lang: language, + // The bias the search actually ran under is a box around the trip's + // existing places; the corpus stores its centre, which is what an + // evaluation needs to bias its own index the same way. + biasLat: locationBias ? (locationBias.low.lat + locationBias.high.lat) / 2 : undefined, + biasLng: locationBias ? (locationBias.low.lng + locationBias.high.lng) / 2 : undefined, + source: `${pick.mode}:${meta.source}`, + liveRank: pick.rank, + liveCount: pick.count, + pickedName: result.name || '', + pickedLat: lat, + pickedLng: lng, + pickedPlaceId: result.google_place_id || result.osm_id || null, + }) + } + } } setMapsResults([]) setMapsSearch('') } const handleSelectSuggestion = async (suggestion: { placeId: string; mainText: string; secondaryText: string }) => { + // Read before the list is cleared: this is the rank the user saw. + const acRank = acSuggestions.findIndex(s => s.placeId === suggestion.placeId) + const acCount = acSuggestions.length setAcSuggestions([]) setAcHighlight(-1) const previousSearch = mapsSearch @@ -372,7 +413,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { place = search.places?.[0] ?? null } if (place) { - handleSelectMapsResult(place) + handleSelectMapsResult(place, acRank >= 0 ? { mode: 'autocomplete', rank: acRank, count: acCount } : undefined) } else { setMapsSearch(previousSearch) toast.error(t('places.mapsSearchError')) @@ -768,7 +809,7 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {