fix(atlas): a place that moves no longer keeps its old country (#2527)

place_regions caches the country and region Atlas resolved for a place,
and a row that was already there was never re-derived. Correcting a
place's location in the trip left the old row in charge, so Atlas, the
dashboard stats and the journey stats kept counting the country the place
had just left, with the trip's dates on it.

A trigger on places now drops the cached row whenever lat, lng or address
actually change, so every writer is covered: the place editor, the MCP
update tools, the plugin RPC and the import enrichment and address
backfill. An edit that writes the same location back keeps the row.

The background lookup that fills the cache also only writes while the
place still sits where it was resolved from. A place moved while its
lookup was running would otherwise get the old answer written back.

Places corrected before this fix would have kept their old country until
they were moved again, because the trigger only sees new edits.
PlaceRegionsRepairJob goes over place_regions once, on the first start
after the update, and remembers that in app_settings. It re-derives each
row from the bundled borders only, never Nominatim, and replaces a row
whose country or region no longer matches where its place is now. A row
the bundle cannot answer for is left alone unless both the coordinates
and the address put the place in another country; then it is dropped and
the next Atlas load looks the place up again. A place without
coordinates keeps no row. Its writes are guarded the same way as the
background lookup, so a place edited during the pass is not overwritten.

The in-memory region cache is now keyed by the country the address names
as well as the coordinates, so correcting only the address of a border
point no longer returns the old answer until a restart.

On dev the trigger takes schema version 244, the slot it has on main, so a
4.3.3 install upgrading to 4.4.0 does not skip it. Web Push moves up to
245 and is idempotent, so a 4.3.3 install gets its table there. An
instance that already ran Web Push at 244 never runs 244 again, so 246
creates the trigger once more; everywhere else that is a no-op.

server/src/db/migrations.ts

@@ -70,6 +70,22 @@ export function trimUserWhitespace(db: Database.Database): boolean {
   return hadCollision;
 }
 
+/**
+ * Keeps place_regions in step with the place it was resolved from (#2527): a
+ * change to lat, lng or address drops the cached row. Migrations 244 and 246
+ * both create it, see there.
+ */
+function createPlaceRegionsFollowPlaceTrigger(db: Database.Database): void {
+  db.exec(`
+    CREATE TRIGGER IF NOT EXISTS trg_place_regions_follow_place
+    AFTER UPDATE OF lat, lng, address ON places
+    WHEN OLD.lat IS NOT NEW.lat OR OLD.lng IS NOT NEW.lng OR OLD.address IS NOT NEW.address
+    BEGIN
+      DELETE FROM place_regions WHERE place_id = NEW.id;
+    END
+  `);
+}
+
 function runMigrations(db: Database.Database): void {
   db.exec('CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL)');
   const versionRow = db.prepare('SELECT version FROM schema_version').get() as { version: number } | undefined;
@@ -5258,6 +5274,27 @@ function runMigrations(db: Database.Database): void {
       }
     },
 
+    /*
+     * A place that moves takes its Atlas country with it (#2527).
+     *
+     * place_regions caches the country and region Atlas resolved from a
+     * place's coordinates and address, and nothing re-derives a row that is
+     * already there. Correcting a place's location left the old row in charge,
+     * so Atlas, the dashboard stats and the journey stats kept counting the
+     * country the place had just left.
+     *
+     * A trigger rather than a delete in the places code, because the row is
+     * derived from the place and every writer of lat, lng or address has to
+     * let go of it: the place editor, update_place, the plugin RPC, the import
+     * enrichment and its address backfill, and whatever comes next. The WHEN
+     * clause matters because every place edit writes lat, lng and address
+     * back whether they changed or not, and renaming a place must not throw
+     * away a good row. The next Atlas load resolves the place where it is now.
+     *
+     * This is 244 on main (4.3.3) as well, so it sits here, ahead of Web Push.
+     */
+    () => createPlaceRegionsFollowPlaceTrigger(db),
+
     /*
      * Web Push (#894): one row per browser a user switched push on in.
      *
@@ -5269,6 +5306,10 @@ function runMigrations(db: Database.Database): void {
      * up, rather than as a push service refusing every message. failure_count
      * and last_success_at are bookkeeping for the sender; the rows go with the
      * user.
+     *
+     * Web Push was 244 before the #2527 trigger took that slot to match main.
+     * A 4.3.3 install is already at 244 with the trigger and gets the table
+     * here; an instance that ran this at 244 replays it as a no-op.
      */
     () => {
       db.exec(`
@@ -5287,6 +5328,14 @@ function runMigrations(db: Database.Database): void {
         CREATE INDEX IF NOT EXISTS idx_push_subscriptions_user ON push_subscriptions(user_id);
       `);
     },
+
+    /*
+     * The #2527 trigger once more, for an instance that ran Web Push at 244
+     * before the trigger took that slot. Such an instance never runs 244 again,
+     * so it gets the trigger here. Everywhere else it already exists and this
+     * is a no-op.
+     */
+    () => createPlaceRegionsFollowPlaceTrigger(db),
   ];
 
   if (currentVersion < migrations.length) {

server/src/nest/atlas/atlas-geo.ts

@@ -781,7 +781,7 @@ async function resolveCountryCode(place: Place): Promise<string | null> {
   return null;
 }
 
-export function resolveCountryCodeSync(place: Place): string | null {
+export function resolveCountryCodeSync(place: Pick<Place, 'lat' | 'lng' | 'address'>): string | null {
   const hasCoords = !!(place.lat && place.lng);
   if (hasCoords) {
     const fromCoords = getCountryFromCoords(place.lat!, place.lng!);
@@ -937,10 +937,13 @@ function buildRegionInfo(address: Record<string, string>, preferFinest: boolean)
   };
 }
 
-export async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
-  const key = cacheKeyFor(lat, lng, 'region');
-  if (regionCache.has(key)) return regionCache.get(key)!;
-
+/**
+ * The offline half of reverseGeocodeRegion: the bundled admin1 polygons, then the
+ * box-gated address fallback. Null when the bundle has no answer for the point, which
+ * is where reverseGeocodeRegion goes on to ask Nominatim. Deterministic and uncached,
+ * so it is also what the one time place_regions repair re-derives rows with (#2527).
+ */
+export async function resolveRegionFromBundle(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
   // Prefer resolving directly against the bundled polygons: offline, deterministic, and —
   // unlike Nominatim's address levels — guaranteed to match a feature the client can
   // actually highlight. Falls through to reverse geocoding when the country has no admin1
@@ -948,10 +951,7 @@ export async function reverseGeocodeRegion(lat: number, lng: number, placeAddres
   const coordCountry = getCountryFromCoords(lat, lng);
   if (coordCountry) {
     const fromBundle = await getRegionFromCoords(coordCountry, lat, lng);
-    if (fromBundle) {
-      regionCache.set(key, fromBundle);
-      return fromBundle;
-    }
+    if (fromBundle) return fromBundle;
   }
   // The coordinate-only lookup found no matching region — either no country polygon contains
   // the point, or a simplified admin0 border put it in the WRONG country (a place on the
@@ -966,11 +966,22 @@ export async function reverseGeocodeRegion(lat: number, lng: number, placeAddres
   // trusting a region match in it.
   const addressCountry = getCountryFromAddress(placeAddress ?? null);
   if (addressCountry && addressCountry !== coordCountry && isPointInCountryBox(addressCountry, lat, lng)) {
-    const fromAddress = await getRegionFromCoords(addressCountry, lat, lng);
-    if (fromAddress) {
-      regionCache.set(key, fromAddress);
-      return fromAddress;
-    }
+    return getRegionFromCoords(addressCountry, lat, lng);
+  }
+  return null;
+}
+
+export async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
+  // The address takes part in the answer only through the country it names, so that
+  // country is part of the key. Keyed by coordinates alone, a place whose address was
+  // corrected on a border point got the old answer back from memory (#2527).
+  const key = `${cacheKeyFor(lat, lng, 'region')}|${getCountryFromAddress(placeAddress ?? null) ?? ''}`;
+  if (regionCache.has(key)) return regionCache.get(key)!;
+
+  const fromBundle = await resolveRegionFromBundle(lat, lng, placeAddress);
+  if (fromBundle) {
+    regionCache.set(key, fromBundle);
+    return fromBundle;
   }
 
   // Only reached when the bundle's own polygons for this country don't cover the point at

server/src/nest/atlas/atlas.module.ts

@@ -11,6 +11,8 @@ import { PublicStatsController } from './public-stats.controller';
 import { ApiTokenGuard } from '../public-api/api-token.guard';
 import { TokensModule } from '../tokens/tokens.module';
 import { RateLimitModule } from '../common/rate-limit.module';
+import { SchedulingModule } from '../scheduling/scheduling.module';
+import { PlaceRegionsRepairJob } from './place-regions-repair.job';
 
 /**
  * Atlas addon domain (L7 leaf module). Registered in AppModule. Exports
@@ -29,11 +31,13 @@ import { RateLimitModule } from '../common/rate-limit.module';
  * than pulled in with PublicApiModule: @UseGuards instantiates a guard in the
  * declaring controller's module, so exporting it from over there would still leave
  * its TokenService unresolvable here. TokensModule is a leaf, so the edge is free.
+ *
+ * PlaceRegionsRepairJob puts right, once, the place_regions rows cached before #2527.
  */
 @Module({
-  imports: [AuthModule, PluginGuardsModule, AddonsModule, TokensModule, RateLimitModule],
+  imports: [AuthModule, PluginGuardsModule, AddonsModule, TokensModule, RateLimitModule, SchedulingModule],
   controllers: [AtlasController, TravelStatsController, PublicStatsController],
-  providers: [AtlasService, AtlasMcp, AtlasRpc, ApiTokenGuard],
+  providers: [AtlasService, AtlasMcp, AtlasRpc, ApiTokenGuard, PlaceRegionsRepairJob],
   exports: [AtlasService],
 })
 export class AtlasModule {}

server/src/nest/atlas/atlas.service.ts

@@ -4,14 +4,17 @@ import type { AtlasLocateResponse } from '@trek/shared';
 import { Trip, Place } from '../../types';
 import { DatabaseService } from '../database/database.service';
 import {
+  getCountryFromAddress,
   getCountryFromCoords,
   getCountryGeoGz,
   getRegionFromCoords,
   getRegionGeo,
   geocodingInFlight,
   resolveCountryCodeSync,
+  resolveRegionFromBundle,
   reverseGeocodeRegion,
 } from './atlas-geo';
+import type { RegionInfo } from './atlas-geo';
 import { KNOWN_COUNTRIES } from './known-countries';
 import { cityFromAddress } from './city-from-address';
 import { transferEndpointIds } from './transfer-endpoints';
@@ -19,6 +22,15 @@ import type { FlightEndpointRow } from './transfer-endpoints';
 import { countryVisitDates } from './visit-dates';
 import { haversineKm } from '../common/geo';
 
+/** The part of a place that its cached region is derived from. */
+type LocatedPlace = Pick<Place, 'id' | 'lat' | 'lng' | 'address'>;
+
+/** A place_regions row next to the location of its place. */
+type CachedRegionRow = LocatedPlace & { country_code: string; region_code: string };
+
+/** How many cached rows the #2527 repair checks before it lets other work run. */
+const REPAIR_YIELD_EVERY = 200;
+
 /**
  * A reservation endpoint plus the two booking columns that decide whether it may
  * take part in layover pairing at all (see transfer-endpoints.ts).
@@ -175,32 +187,131 @@ export class AtlasService {
       }
     }
 
-    if (uncachedForGeocode.length > 0) {
-      const insertStmt = this.db.prepare(
-        'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)',
-      );
-      for (const p of uncachedForGeocode) geocodingInFlight.add(p.id);
-      void (async () => {
-        try {
-          for (const place of uncachedForGeocode) {
-            try {
-              const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
-              if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name);
-            } catch {
-              /* continue */
-            } finally {
-              geocodingInFlight.delete(place.id);
-            }
-          }
-        } catch {
-          for (const p of uncachedForGeocode) geocodingInFlight.delete(p.id);
-        }
-      })();
-    }
+    this.cacheRegionsInBackground(uncachedForGeocode);
 
     return out;
   }
 
+  /**
+   * Resolve each place's region in the background and cache it in place_regions.
+   */
+  private cacheRegionsInBackground(places: Place[]): void {
+    if (places.length === 0) return;
+    for (const p of places) geocodingInFlight.add(p.id);
+    void (async () => {
+      try {
+        for (const place of places) {
+          try {
+            const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
+            if (info) this.cacheRegionWhileUnmoved(place, info);
+          } catch {
+            // individual failure, continue with the remaining places
+          } finally {
+            geocodingInFlight.delete(place.id);
+          }
+        }
+      } catch {
+        for (const p of places) geocodingInFlight.delete(p.id);
+      }
+    })();
+  }
+
+  /**
+   * Cache a resolved region, but only while the place still sits where it was resolved
+   * from. A place moved while its lookup was running has already had its row dropped by
+   * the place_regions trigger (#2527), and writing the old answer back would pin it to
+   * the country it just left. A place deleted in the meantime is skipped the same way.
+   */
+  private cacheRegionWhileUnmoved(place: LocatedPlace, info: RegionInfo): boolean {
+    const written = this.db.run(
+      `INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name)
+       SELECT id, ?, ?, ? FROM places WHERE id = ? AND lat = ? AND lng = ? AND address IS ?`,
+      info.country_code,
+      info.region_code,
+      info.region_name,
+      place.id,
+      place.lat,
+      place.lng,
+      place.address ?? null,
+    );
+    return written.changes > 0;
+  }
+
+  // ── One time repair of the rows cached before #2527 ───────────────────────
+
+  /**
+   * Before #2527 nothing dropped a place_regions row when its place moved, so an
+   * install that upgrades still holds the country every corrected place left. This
+   * re-derives each row with the bundled resolver alone, never Nominatim, and puts
+   * right the ones that no longer match where their place is now.
+   *
+   * Where the bundle answers for the place's current location with another country
+   * or region, the row gets that answer, which is what a fresh lookup would cache. A
+   * good row matches it, the address fallback case included.
+   *
+   * Where the bundle has no answer (a coastal point outside the simplified polygons,
+   * a country the bundle has no regions for), the row came from Nominatim and only
+   * Nominatim could judge it, so it stays, unless the coordinates put the place in
+   * another country and the address does not name the cached one either. Then it is
+   * dropped and the next Atlas load looks the place up again.
+   *
+   * A place without coordinates keeps no row, the same as the trigger does.
+   *
+   * Every write only lands while the place still holds the location it was read with,
+   * so a place edited while this runs keeps what the trigger and the next lookup give it.
+   */
+  async repairStaleRegionCache(): Promise<{ replaced: number; dropped: number }> {
+    const rows = this.db.all<CachedRegionRow>(`
+      SELECT pr.place_id AS id, pr.country_code, pr.region_code, p.lat, p.lng, p.address
+      FROM place_regions pr
+      JOIN places p ON p.id = pr.place_id
+      ORDER BY pr.place_id
+    `);
+    let replaced = 0;
+    let dropped = 0;
+    for (const [i, row] of rows.entries()) {
+      // A big install holds many rows, so requests get a turn every so often.
+      if (i > 0 && i % REPAIR_YIELD_EVERY === 0) await new Promise((resolve) => setImmediate(resolve));
+      const fix = await this.staleRegionFix(row);
+      if (fix === 'drop') {
+        if (this.dropRegionWhileUnmoved(row)) dropped++;
+      } else if (fix && this.cacheRegionWhileUnmoved(row, fix)) {
+        replaced++;
+      }
+    }
+    return { replaced, dropped };
+  }
+
+  /** What a cached row needs: a new region, 'drop', or null when it is right. */
+  private async staleRegionFix(row: CachedRegionRow): Promise<RegionInfo | 'drop' | null> {
+    if (row.lat == null || row.lng == null) return 'drop';
+    const cachedCountry = row.country_code.toUpperCase();
+    const fresh = await resolveRegionFromBundle(row.lat, row.lng, row.address);
+    if (fresh) {
+      const same = fresh.country_code.toUpperCase() === cachedCountry && fresh.region_code === row.region_code;
+      return same ? null : fresh;
+    }
+    const countryNow = resolveCountryCodeSync(row);
+    const contradicted = !!countryNow && countryNow !== cachedCountry && getCountryFromAddress(row.address) !== cachedCountry;
+    return contradicted ? 'drop' : null;
+  }
+
+  private dropRegionWhileUnmoved(row: CachedRegionRow): boolean {
+    const removed = this.db.run(
+      `DELETE FROM place_regions
+       WHERE place_id = ? AND country_code = ? AND region_code = ?
+         AND EXISTS (SELECT 1 FROM places WHERE id = ? AND lat IS ? AND lng IS ? AND address IS ?)`,
+      row.id,
+      row.country_code,
+      row.region_code,
+      row.id,
+      row.lat ?? null,
+      row.lng ?? null,
+      row.address ?? null,
+    );
+    return removed.changes > 0;
+  }
+
   // ── Stats ─────────────────────────────────────────────────────────────────
 
   async stats(userId: number) {
@@ -731,29 +842,9 @@ export class AtlasService {
     const cachedMap = new Map(cached.map((c) => [c.place_id, c]));
 
     // Kick off background geocoding for uncached places; return cached data immediately.
-    const uncached = places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id));
-    if (uncached.length > 0) {
-      const insertStmt = this.db.prepare(
-        'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)',
-      );
-      for (const p of uncached) geocodingInFlight.add(p.id);
-      void (async () => {
-        try {
-          for (const place of uncached) {
-            try {
-              const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
-              if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name);
-            } catch {
-              // individual failure — continue with remaining places
-            } finally {
-              geocodingInFlight.delete(place.id);
-            }
-          }
-        } catch {
-          for (const p of uncached) geocodingInFlight.delete(p.id);
-        }
-      })();
-    }
+    this.cacheRegionsInBackground(
+      places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id)),
+    );
 
     // Group by country → regions with place counts
     const regionMap: Record<

server/src/nest/atlas/place-regions-repair.job.ts

@@ -0,0 +1,53 @@
+import { logInfo, logError } from '../audit/audit-log.logger';
+import { DatabaseService } from '../database/database.service';
+import { CronRegistrarService } from '../scheduling/cron-registrar.service';
+import { AtlasService } from './atlas.service';
+import { Injectable, type OnApplicationBootstrap } from '@nestjs/common';
+
+/** The app_settings row that says the pass below has been through every cached region. */
+export const PLACE_REGIONS_REPAIR_DONE_KEY = 'place_regions_repair_2527';
+
+/**
+ * One pass over place_regions on the first start after #2527 (see
+ * AtlasService.repairStaleRegionCache).
+ *
+ * Not a cron: from this version on the trigger on places drops a row as soon as its
+ * place moves, so once the rows cached before it are put right there is nothing left
+ * to repair. app_settings remembers that the pass ran. A pass cut short by a restart
+ * or an error simply runs again on the next start; a second run changes nothing the
+ * first one wrote.
+ *
+ * In the background, like the other boot sweeps: the server does not wait for it.
+ */
+@Injectable()
+export class PlaceRegionsRepairJob implements OnApplicationBootstrap {
+  constructor(
+    private readonly atlas: AtlasService,
+    private readonly registrar: CronRegistrarService,
+    private readonly db: DatabaseService,
+  ) {}
+
+  onApplicationBootstrap(): void {
+    if (!this.registrar.isEnabled()) return;
+    void this.runOnce();
+  }
+
+  async runOnce(): Promise<void> {
+    try {
+      if (this.db.get('SELECT 1 FROM app_settings WHERE key = ?', PLACE_REGIONS_REPAIR_DONE_KEY)) return;
+      const { replaced, dropped } = await this.atlas.repairStaleRegionCache();
+      this.db.run(
+        'INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)',
+        PLACE_REGIONS_REPAIR_DONE_KEY,
+        new Date().toISOString(),
+      );
+      if (replaced + dropped > 0) {
+        logInfo(
+          `Atlas region repair: ${replaced} place region(s) re-derived where the place is now, ${dropped} left for the next Atlas load`,
+        );
+      }
+    } catch (err: unknown) {
+      logError(`Atlas region repair: ${err instanceof Error ? err.message : err}`);
+    }
+  }
+}