fix(places): keep the Google feature id, and give list imports the free address lookup

Two halves of #1954, both about a place arriving through a list coming out
thinner than the same place pasted as a single link.

The feature id was never dropped on purpose. googleFtidFromMapsUrl only looked
at the `?ftid=` query parameter, while a share link that resolveGoogleMapsUrl
has followed to its final hop carries the id in the `data=!…!1s0x…:0x…` path
blob — the same run the coordinates are already parsed out of two lines further
down. So every pasted maps.app.goo.gl link lost it, and the Maps button opened
a name search rather than the entry the link pointed at. The parser now reads
the blob too, but only on a /place/ URL: a /maps/dir/ route carries one !1s per
waypoint and the first is the origin, which would attach the wrong place.

List imports never had a reverse lookup at all. The only pass behind them is
the Google enrichment, which returns immediately without a Maps key, so an
instance without one imported a list and left every address empty forever.
They now get the same free Nominatim lookup the single-link path has always
had: detached after the import answers, on the throttled background lane so it
never queues in front of someone's search, through COALESCE so it can only fill
an empty column, and each row pushed over the websocket so the sidebar fills in
without a reload. Capped at 250 places — past that it stops being a backfill
and starts being bulk geocoding.

Not included: an osm_id for imported places. Single links do not produce one
either, and one derived from a zoom-18 reverse lookup would name the building
or the street rather than the POI.

server/src/nest/maps/maps.helpers.ts

@@ -129,17 +129,29 @@ export function namesOverlap(a: string, b: string): boolean {
 }
 
 const GOOGLE_FTID_RE = /^0x[0-9a-f]+:0x[0-9a-f]+$/i;
+// The same id inside the `data=` pathname blob, where a resolved share link keeps it:
+// /maps/place/<name>/data=!4m6!3m5!1s0x47e66e1f06e2b70f:0x40b82c3688c9460!8m2!3d48.85!4d2.29
+const GOOGLE_FTID_DATA_RE = /!1s(0x[0-9a-f]{1,20}:0x[0-9a-f]{1,20})/i;
 
-// Extracts a Google Maps feature id (ftid, 0x..:0x..) from a URL's ?ftid= param.
+// Extracts a Google Maps feature id (ftid, 0x..:0x..) from a URL.
+// Two shapes carry it. The `?ftid=` query parameter is the old one. A share link that
+// resolveGoogleMapsUrl has followed to its final hop carries it instead in the `data=`
+// pathname blob, in the same `!1s…!3d…!4d…` run the coordinates come from (#1954) — so
+// reading only the query parameter dropped the id for every pasted maps.app.goo.gl link.
 // The Places API (New) googleMapsUri is usually a cid-style URL (https://maps.google.com/?cid=NNN)
-// with no ftid, so this returns null for most API responses — the precise query_place_id link is
-// used instead. It does recover an ftid from a /place/?...&ftid= URL, e.g. a pasted share link
-// resolved by resolveGoogleMapsUrl or a Google MyMaps list import.
+// with neither, so this still returns null for most API responses — the precise
+// query_place_id link is used instead.
 export function googleFtidFromMapsUrl(url?: string | null): string | null {
   if (!url) return null;
   try {
-    const ftid = new URL(url).searchParams.get('ftid')?.trim();
-    return ftid && GOOGLE_FTID_RE.test(ftid) ? ftid.toLowerCase() : null;
+    const parsed = new URL(url);
+    const ftid = parsed.searchParams.get('ftid')?.trim();
+    if (ftid && GOOGLE_FTID_RE.test(ftid)) return ftid.toLowerCase();
+    // Only for a place URL. A /maps/dir/ route carries one !1s per waypoint and the
+    // first one is the origin, not the place the link is about.
+    if (!parsed.pathname.includes('/place/')) return null;
+    const fromPath = GOOGLE_FTID_DATA_RE.exec(parsed.pathname)?.[1];
+    return fromPath ? fromPath.toLowerCase() : null;
   } catch {
     return null;
   }

server/src/nest/maps/maps.service.ts

@@ -1974,6 +1974,7 @@ export class MapsService {
     lat: string,
     lng: string,
     lang?: string,
+    opts?: { lane?: GeoLane; timeoutMs?: number },
   ): Promise<{ name: string | null; address: string | null }> {
     const params = new URLSearchParams({
       lat,
@@ -1983,7 +1984,7 @@ export class MapsService {
       zoom: '18',
       'accept-language': toApiLang(lang),
     });
-    const response = await nominatimFetch('reverse', params);
+    const response = await nominatimFetch('reverse', params, opts);
     if (!response.ok) return { name: null, address: null };
     const data = (await response.json()) as { name?: string; display_name?: string; address?: Record<string, string> };
     const addr = data.address || {};

server/src/nest/places/places.helpers.ts

@@ -254,6 +254,12 @@ export const SEARCH_BIAS_RADIUS_METERS = 2000;
 /** Concurrent enrichment lookups — small, to stay friendly to the Maps quota. */
 export const ENRICH_CONCURRENCY = 3;
 
+// The free address backfill is one Nominatim request per place on the throttled
+// background lane (~1/s), so a normal list costs seconds. Past this it stops being
+// a backfill and starts being bulk geocoding, which Nominatim's usage policy asks
+// people not to do — so it stops rather than queueing for an hour.
+export const ADDRESS_BACKFILL_MAX_PLACES = 250;
+
 
 /**
  * Pick the search result that is the same place as the import: it must be a

server/src/nest/places/places.service.ts

@@ -29,6 +29,7 @@ import { JourneyDomainService } from '../journey/journey-domain.service';
 import {
   COORD_DEDUP_TOLERANCE,
   ENRICH_CONCURRENCY,
+  ADDRESS_BACKFILL_MAX_PLACES,
   escapeLikePattern,
   MAX_LIST_RESPONSE_BYTES,
   googleMapsFeatureIdFromItem,
@@ -983,8 +984,8 @@ export class PlacesService {
       }
     });
 
-    if (opts?.enrich && opts.userId && created.length) {
-      void this.enrichImportedPlaces(tripId, opts.userId, created as EnrichablePlace[], opts.lang);
+    if (created.length) {
+      void this.enrichImportedList(tripId, created as EnrichablePlace[], opts);
     }
 
     return { places: created, listName, skipped };
@@ -1126,8 +1127,8 @@ export class PlacesService {
       }
     });
 
-    if (opts?.enrich && opts.userId && created.length) {
-      void this.enrichImportedPlaces(tripId, opts.userId, created as EnrichablePlace[], opts.lang);
+    if (created.length) {
+      void this.enrichImportedList(tripId, created as EnrichablePlace[], opts);
     }
 
     return { places: created, listName, skipped };
@@ -1226,6 +1227,63 @@ export class PlacesService {
     }
   }
 
+  /**
+   * Everything a just-imported list gets in the background, in one place so the
+   * Google and Naver call sites cannot drift apart.
+   *
+   * The Google pass stays exactly as opt-in and key-gated as before. The address
+   * backfill behind it needs neither — it is the same free Nominatim reverse
+   * lookup a single pasted link has always had, which is what made a list import
+   * come out thinner than the same place added one at a time (#1954).
+   */
+  private async enrichImportedList(tripId: string, places: EnrichablePlace[], opts?: ListImportOptions): Promise<void> {
+    if (opts?.enrich && opts.userId) {
+      await this.enrichImportedPlaces(tripId, opts.userId, places, opts.lang);
+    }
+    await this.backfillMissingAddresses(tripId, places, opts?.lang);
+  }
+
+  /**
+   * Fill in the address of imported places that have none, from Nominatim.
+   *
+   * Runs on the background lane so it never queues in front of a user's own
+   * search, writes through COALESCE so it can only fill an empty column — the
+   * Google pass above may already have written a better one — and pushes each
+   * row over the websocket so the sidebar fills in without a reload. Never
+   * throws: one bad lookup must not take down a detached task.
+   */
+  async backfillMissingAddresses(tripId: string, places: EnrichablePlace[], lang?: string): Promise<void> {
+    try {
+      const pending = places.filter(p => !p.address && p.lat != null && p.lng != null);
+      if (!pending.length) return;
+      if (pending.length > ADDRESS_BACKFILL_MAX_PLACES) {
+        console.warn(`[Places] address backfill skipped for trip ${tripId}: ${pending.length} places exceeds the ${ADDRESS_BACKFILL_MAX_PLACES} cap`);
+        return;
+      }
+      // Serial on purpose: the background lane throttles to roughly one request a
+      // second anyway, so concurrency would only build a queue.
+      for (const place of pending) {
+        try {
+          const { address } = await this.maps.reverseGeocode(String(place.lat), String(place.lng), lang, {
+            lane: 'background',
+            timeoutMs: 10000,
+          });
+          if (!address) continue;
+          this.dbs.run(
+            'UPDATE places SET address = COALESCE(address, ?), updated_at = CURRENT_TIMESTAMP WHERE id = ? AND trip_id = ?',
+            address, place.id, tripId,
+          );
+          const updated = this.dbs.getPlaceWithTags(place.id);
+          if (updated) this.realtime.broadcast(tripId, 'place:updated', { place: updated }, undefined);
+        } catch (err) {
+          console.error(`[Places] address backfill failed for place ${place.id}:`, err instanceof Error ? err.message : err);
+        }
+      }
+    } catch (err) {
+      console.error('[Places] address backfill pass failed:', err instanceof Error ? err.message : err);
+    }
+  }
+
   // -------------------------------------------------------------------------
   // Search place image (Unsplash)
   // -------------------------------------------------------------------------

server/tests/unit/nest/maps.service.test.ts

@@ -2266,6 +2266,46 @@ describe('googleFtidFromMapsUrl', () => {
     expect(googleFtidFromMapsUrl('not a url')).toBeNull();
     expect(googleFtidFromMapsUrl(null)).toBeNull();
   });
+
+  // #1954 — a followed maps.app.goo.gl link keeps the id in the `data=` blob, not
+  // in a query parameter, which is why a pasted single link used to lose it.
+  it('MAPS-FTID-004: extracts the ftid from the data= path blob of a resolved share link', () => {
+    expect(
+      googleFtidFromMapsUrl(
+        'https://www.google.com/maps/place/Notre-Dame/data=!4m6!3m5!1s0x47e671e23a09b3b1:0x40b82c3688b2f60!8m2!3d48.8529!4d2.3499',
+      ),
+    ).toBe('0x47e671e23a09b3b1:0x40b82c3688b2f60');
+  });
+
+  it('MAPS-FTID-005: the query parameter still wins over the path blob', () => {
+    expect(
+      googleFtidFromMapsUrl(
+        'https://www.google.com/maps/place/X/data=!3m5!1s0x1:0x2!8m2?ftid=0x882bf179e806d471:0x8591dde29c821a93',
+      ),
+    ).toBe('0x882bf179e806d471:0x8591dde29c821a93');
+  });
+
+  it('MAPS-FTID-006: ignores the path blob outside a /place/ URL', () => {
+    // A directions link carries one !1s per waypoint and the first is the origin,
+    // so reading it would attach the wrong place.
+    expect(
+      googleFtidFromMapsUrl('https://www.google.com/maps/dir/A/B/data=!4m2!1s0x47e671e23a09b3b1:0x40b82c3688b2f60'),
+    ).toBeNull();
+  });
+
+  it('MAPS-FTID-007: a place URL without either shape is still null, and case is normalised', () => {
+    expect(googleFtidFromMapsUrl('https://www.google.com/maps/place/Notre-Dame/@48.85,2.34,17z')).toBeNull();
+    expect(
+      googleFtidFromMapsUrl('https://www.google.com/maps/place/X/data=!1s0x47E671E23A09B3B1:0x40B82C3688B2F60'),
+    ).toBe('0x47e671e23a09b3b1:0x40b82c3688b2f60');
+  });
+
+  it('MAPS-FTID-008: a long hostile path blob resolves in linear time (ReDoS budget)', () => {
+    const hostile = `https://www.google.com/maps/place/X/data=${'!1s0x'.repeat(20000)}`;
+    const started = Date.now();
+    expect(googleFtidFromMapsUrl(hostile)).toBeNull();
+    expect(Date.now() - started).toBeLessThan(500);
+  });
 });
 
 // ── buildUserAgent (instance-specific UA, #1309) ──────────────────────────────

server/tests/unit/nest/places.service.test.ts

@@ -14,6 +14,7 @@ import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/plac
 import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service';
 import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
 import { TRACK_COLORS } from '@trek/shared';
+import { ADDRESS_BACKFILL_MAX_PLACES } from '../../../src/nest/places/places.helpers';
 
 // ── DB setup ──────────────────────────────────────────────────────────────────
 
@@ -1468,3 +1469,100 @@ describe('importNaverList provider payload', () => {
     expect(result.places[0].name).toBe('Gyeongbokgung');
   });
 });
+
+// ── Free address backfill for list imports (#1954) ───────────────────────────
+//
+// A place pasted as a single Google Maps link has always been reverse geocoded;
+// the same place arriving through a list import was not, so it stayed without an
+// address forever. These cover the backfill that closes that gap.
+
+describe('backfillMissingAddresses', () => {
+  function backfillSvc(reverseGeocode: MapsService['reverseGeocode']) {
+    return makePlacesService({ reverseGeocode } as unknown as MapsService);
+  }
+
+  it('PLACE-SVC-078 — fills the address of a place that has none', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Bar', lat: 48.85, lng: 2.35 }) as any;
+
+    const reverseGeocode = vi.fn(async () => ({ name: null, address: '1 Rue de Rivoli, Paris' }));
+    await backfillSvc(reverseGeocode).backfillMissingAddresses(String(trip.id), [
+      { id: place.id, name: 'Bar', lat: 48.85, lng: 2.35 },
+    ]);
+
+    expect(reverseGeocode).toHaveBeenCalledWith('48.85', '2.35', undefined, { lane: 'background', timeoutMs: 10000 });
+    const row = testDb.prepare('SELECT address FROM places WHERE id = ?').get(place.id) as { address: string };
+    expect(row.address).toBe('1 Rue de Rivoli, Paris');
+  });
+
+  it('PLACE-SVC-079 — never overwrites an address the import or the Google pass already wrote', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Bar', lat: 48.85, lng: 2.35 }) as any;
+    testDb.prepare('UPDATE places SET address = ? WHERE id = ?').run('Imported address', place.id);
+
+    const reverseGeocode = vi.fn(async () => ({ name: null, address: 'Nominatim address' }));
+    await backfillSvc(reverseGeocode).backfillMissingAddresses(String(trip.id), [
+      { id: place.id, name: 'Bar', lat: 48.85, lng: 2.35, address: 'Imported address' },
+    ]);
+
+    expect(reverseGeocode).not.toHaveBeenCalled();
+    const row = testDb.prepare('SELECT address FROM places WHERE id = ?').get(place.id) as { address: string };
+    expect(row.address).toBe('Imported address');
+  });
+
+  it('PLACE-SVC-080 — a lookup that answers with nothing leaves the row untouched', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Bar', lat: 48.85, lng: 2.35 }) as any;
+
+    await backfillSvc(vi.fn(async () => ({ name: null, address: null }))).backfillMissingAddresses(String(trip.id), [
+      { id: place.id, name: 'Bar', lat: 48.85, lng: 2.35 },
+    ]);
+
+    const row = testDb.prepare('SELECT address FROM places WHERE id = ?').get(place.id) as { address: string | null };
+    expect(row.address).toBeNull();
+  });
+
+  it('PLACE-SVC-081 — one failing lookup does not take down the rest of the batch', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const first = createPlace(testDb, trip.id, { name: 'A', lat: 1, lng: 2 }) as any;
+    const second = createPlace(testDb, trip.id, { name: 'B', lat: 3, lng: 4 }) as any;
+
+    const reverseGeocode = vi.fn()
+      .mockRejectedValueOnce(new Error('nominatim down'))
+      .mockResolvedValueOnce({ name: null, address: 'Second address' });
+
+    await expect(
+      backfillSvc(reverseGeocode as unknown as MapsService['reverseGeocode']).backfillMissingAddresses(String(trip.id), [
+        { id: first.id, name: 'A', lat: 1, lng: 2 },
+        { id: second.id, name: 'B', lat: 3, lng: 4 },
+      ]),
+    ).resolves.toBeUndefined();
+
+    const rows = testDb.prepare('SELECT id, address FROM places WHERE trip_id = ? ORDER BY id').all(trip.id) as {
+      id: number; address: string | null;
+    }[];
+    expect(rows[0].address).toBeNull();
+    expect(rows[1].address).toBe('Second address');
+  });
+
+  it('PLACE-SVC-082 — an oversized batch is refused rather than queued for an hour', async () => {
+    const reverseGeocode = vi.fn();
+    const batch = Array.from({ length: ADDRESS_BACKFILL_MAX_PLACES + 1 }, (_, i) => ({
+      id: i + 1, name: `P${i}`, lat: 1, lng: 2,
+    }));
+    await backfillSvc(reverseGeocode as unknown as MapsService['reverseGeocode']).backfillMissingAddresses('1', batch);
+    expect(reverseGeocode).not.toHaveBeenCalled();
+  });
+
+  it('PLACE-SVC-083 — a place without coordinates is skipped', async () => {
+    const reverseGeocode = vi.fn();
+    await backfillSvc(reverseGeocode as unknown as MapsService['reverseGeocode']).backfillMissingAddresses('1', [
+      { id: 1, name: 'A', lat: null as unknown as number, lng: null as unknown as number },
+    ]);
+    expect(reverseGeocode).not.toHaveBeenCalled();
+  });
+});