diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 785d3a835..8d5a9fcb6 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -3660,6 +3660,25 @@ function runMigrations(db: Database.Database): void { console.warn('[migrations] Non-fatal migration step failed:', err); } }, + + // `place_regions` is a re-derivable Nominatim cache, only ever populated for a place ID + // that isn't already cached — so a wrong row, once written, was permanent. Region + // resolution now resolves a place's lat/lng directly against the bundled admin1 polygons + // (the same ones the client renders) instead of trusting Nominatim's address level, which + // could name a subdivision level the bundle doesn't carry (Barcelona's ES-B province vs + // the bundle's ES-CT autonomous community) and never highlight. That fix only helps + // places re-resolved after it, so clear the cache once and let every place re-resolve on + // the next Atlas load. The country_code stored alongside is cleared too, which also drops + // the old wrong-country rows a US-state-abbreviation address used to produce. + () => { + try { + db.exec('DELETE FROM place_regions'); + } catch (err) { + // place_regions is created by an earlier migration; tolerate its absence on an + // unusual partial DB rather than aborting startup. + if (!(err instanceof Error) || !err.message.includes('no such table')) throw err; + } + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/services/atlasService.ts b/server/src/services/atlasService.ts index 3da5177a0..af66ed048 100644 --- a/server/src/services/atlasService.ts +++ b/server/src/services/atlasService.ts @@ -584,6 +584,38 @@ function pointInGeometry(lng: number, lat: number, geom: CompactGeom): boolean { type CompactGeom = { rings: Float64Array[]; polyRingCounts: number[] }; type Box = [number, number, number, number]; // [minLng, minLat, maxLng, maxLat] +// Flatten a GeoJSON Polygon/MultiPolygon into the same compact form the country index uses, +// plus one bounding box per part (a part-box, like buildCountryIndexes builds, so an +// archipelago-style region — Illes Balears, Canarias, … — gets one tight box per island +// group rather than one box spanning the whole span between them). Used to resolve admin1 +// regions against the bundle, which are parsed per country on demand rather than held whole. +function compactGeomFromGeometry(geometry: { type: string; coordinates: unknown }): { geom: CompactGeom; boxes: Box[] } { + const parts = (geometry.type === 'Polygon' ? [geometry.coordinates] : geometry.coordinates) as number[][][][]; + const rings: Float64Array[] = []; + const polyRingCounts: number[] = []; + const boxes: Box[] = []; + for (const part of parts) { + polyRingCounts.push(part.length); + for (const ring of part) { + const flat = new Float64Array(ring.length * 2); + for (let i = 0; i < ring.length; i++) { + flat[2 * i] = ring[i][0]; + flat[2 * i + 1] = ring[i][1]; + } + rings.push(flat); + } + let minLng = Infinity, minLat = Infinity, maxLng = -Infinity, maxLat = -Infinity; + for (const [lng, lat] of part[0]) { + if (lng < minLng) minLng = lng; + if (lng > maxLng) maxLng = lng; + if (lat < minLat) minLat = lat; + if (lat > maxLat) maxLat = lat; + } + boxes.push([minLng, minLat, maxLng, maxLat]); + } + return { geom: { rings, polyRingCounts }, boxes }; +} + // ISO_A2 → compact admin0 geometry + bounding boxes, derived from the bundled admin0 // borders on first use. The parsed FeatureCollection is dropped after this runs; only the // Float64Array rings and boxes are retained (≈1MB vs the ≈145MB parsed geometry, #1576). @@ -673,6 +705,17 @@ function getCountryBoxIndex(): Map { return countryBoxIndex!; } +// Broad sanity check — is (lat,lng) anywhere within the country's own admin0 bounding +// box(es)? Deliberately looser than the polygon test in getCountryFromCoords: a genuine +// border-simplification miss (a point just outside the exact border) still needs to pass +// this, so it only rejects a country that isn't even in the right part of the globe. Used +// to gate the address-derived fallback in both country and region resolution. +export function isPointInCountryBox(countryCode: string, lat: number, lng: number): boolean { + const boxes = getCountryBoxIndex().get(countryCode.toUpperCase()); + if (!boxes) return false; + return boxes.some(([minLng, minLat, maxLng, maxLat]) => lat >= minLat && lat <= maxLat && lng >= minLng && lng <= maxLng); +} + export function getCountryFromCoords(lat: number, lng: number): string | null { // Cheap prefilter: every country with a part-box containing the point. Keep the // area of the matching part so overlapping candidates can be ranked below. @@ -730,25 +773,46 @@ export function getCountryFromAddress(address: string | null): string | null { return null; } -// ── Resolve a place to a country code (address -> bbox -> geocode) ────────── - +// ── Resolve a place to a country code (bbox -> address -> geocode) ────────── +// +// Coordinates are tried FIRST and, when they resolve, trusted outright — getCountryFromCoords +// is a real point-in-polygon test against the same borders the map renders. Address parsing +// is only a fallback. It used to run first, but its "2-letter uppercase last segment = ISO +// code" heuristic collides with US state abbreviations that are ALSO real ISO country codes +// (DE=Germany, GA=Georgia, IN=India, LA=Laos, MA=Morocco, MO=Macau, PA=Panama, VA=Vatican, +// CA=Canada, ...) — a place stored as "..., San Francisco, CA" resolved to Canada, not the +// United States, whenever address ran first. When coordinates are present but didn't resolve +// to any country, the address result is sanity-gated against that country's own admin0 +// bounding box (isPointInCountryBox) before being trusted — the same guard the region-level +// address fallback uses. A place with no coordinates at all has nothing to gate against, so +// the address is trusted directly there, as before. async function resolveCountryCode(place: Place): Promise { - let code = getCountryFromAddress(place.address); - if (!code && place.lat && place.lng) { - code = getCountryFromCoords(place.lat, place.lng); + const hasCoords = !!(place.lat && place.lng); + if (hasCoords) { + const fromCoords = getCountryFromCoords(place.lat!, place.lng!); + if (fromCoords) return fromCoords; } - if (!code && place.lat && place.lng) { - code = await reverseGeocodeCountry(place.lat, place.lng); + const fromAddress = getCountryFromAddress(place.address); + if (fromAddress && (!hasCoords || isPointInCountryBox(fromAddress, place.lat!, place.lng!))) { + return fromAddress; } - return code; + if (hasCoords) { + return await reverseGeocodeCountry(place.lat!, place.lng!); + } + return null; } function resolveCountryCodeSync(place: Place): string | null { - let code = getCountryFromAddress(place.address); - if (!code && place.lat && place.lng) { - code = getCountryFromCoords(place.lat, place.lng); + const hasCoords = !!(place.lat && place.lng); + if (hasCoords) { + const fromCoords = getCountryFromCoords(place.lat!, place.lng!); + if (fromCoords) return fromCoords; } - return code; + const fromAddress = getCountryFromAddress(place.address); + if (fromAddress && (!hasCoords || isPointInCountryBox(fromAddress, place.lat!, place.lng!))) { + return fromAddress; + } + return null; } // ── Shared query: all trips the user owns or is a member of ───────────────── @@ -815,7 +879,7 @@ function resolvePlaceCountries(places: Place[]): Map { try { for (const place of uncachedForGeocode) { try { - const info = await reverseGeocodeRegion(place.lat!, place.lng!); + 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 */ @@ -1154,7 +1218,7 @@ export function unmarkRegionVisited(userId: number, regionCode: string): void { // ── Sub-national region resolution ──────────────────────────────────────── -interface RegionInfo { +export interface RegionInfo { country_code: string; region_code: string; region_name: string; @@ -1165,10 +1229,83 @@ const geocodingInFlight = new Set(); const regionCache = new Map(); -// A zoom-8 reverse geocode of a GB place only resolves to the constituent country -// (England/Scotland/Wales/Northern Ireland). Natural Earth's admin-1 polygons for GB -// are counties and boroughs, so those four codes match no polygon and never highlight. -const GB_CONSTITUENT_CODES = new Set(['GB-ENG', 'GB-SCT', 'GB-WLS', 'GB-NIR']); +// ── Point-in-polygon over the bundled admin1 regions ──────────────────────── +// +// Nominatim's reverse-geocode address levels (province, autonomous community, borough, …) +// don't line up with whatever granularity geoBoundaries ships per country — e.g. Nominatim +// gives Barcelona the *province* code ES-B while the bundle only has the *autonomous- +// community* level (Catalonia), and Belgium/Italy's bundle only has a handful of top-level +// regions while Nominatim returns provinces. Comparing those codes/names (even accent/dash- +// normalized) can never match because they name different levels of subdivision. Resolving +// the place's own lat/lng directly against the SAME polygons the client renders — like +// getCountryFromCoords does for admin0 (#1331) — sidesteps the whole class of bug: the +// stored region_code/region_name are then guaranteed to equal a bundle feature. +// +// The admin1 bundle is streamed and held as per-country GeoJSON text (never parsed whole, +// #1576), so a country's region features are parsed and flattened to CompactGeom on first +// use and cached — only visited countries ever pay the parse. +type RegionFeature = { code: string; name: string; nameEn: string; geom: CompactGeom; boxes: Box[] }; +const regionFeatureCache = new Map(); + +async function getRegionFeatures(countryCode: string): Promise { + const cc = countryCode.toUpperCase(); + const cached = regionFeatureCache.get(cc); + if (cached) return cached; + const store = await getAdmin1Store(); + const text = store.get(cc); + if (!text) { + regionFeatureCache.set(cc, []); + return []; + } + let features: { properties?: Record; geometry?: { type: string; coordinates: unknown } }[]; + try { + features = JSON.parse(`{"type":"FeatureCollection","features":[${text}]}`).features ?? []; + } catch { + regionFeatureCache.set(cc, []); + return []; + } + const out: RegionFeature[] = []; + for (const f of features) { + const code = f.properties?.iso_3166_2; + if (!code || !f.geometry) continue; + const { geom, boxes } = compactGeomFromGeometry(f.geometry); + out.push({ + code, + name: f.properties?.name || code, + nameEn: f.properties?.name_en || f.properties?.name || code, + geom, + boxes, + }); + } + regionFeatureCache.set(cc, out); + return out; +} + +// Resolve (lat,lng) to a bundled admin1 region within the given country, smallest +// matching-part-first (mirroring getCountryFromCoords' candidate ranking) so a point near +// a shared border prefers the tighter-fitting candidate. Returns null when the country has +// no admin1 coverage in the bundle or the point falls outside every polygon (simplification +// gaps at coastlines, etc.) — callers should fall back to reverse geocoding. +export async function getRegionFromCoords(countryCode: string, lat: number, lng: number): Promise { + const features = await getRegionFeatures(countryCode); + if (features.length === 0) return null; + const candidates: { f: RegionFeature; area: number }[] = []; + for (const f of features) { + for (const [minLng, minLat, maxLng, maxLat] of f.boxes) { + if (lat >= minLat && lat <= maxLat && lng >= minLng && lng <= maxLng) { + candidates.push({ f, area: (maxLng - minLng) * (maxLat - minLat) }); + break; + } + } + } + candidates.sort((a, b) => a.area - b.area); + for (const { f } of candidates) { + if (pointInGeometry(lng, lat, f.geom)) { + return { country_code: countryCode.toUpperCase(), region_code: f.code, region_name: f.nameEn || f.name }; + } + } + return null; +} // Returns the OSM address object, {} for an "ok but empty" response (so it is cached as // a definitive miss), or null for a transient failure (so it is retried next time). @@ -1223,20 +1360,50 @@ function buildRegionInfo(address: Record, preferFinest: boolean) }; } -async function reverseGeocodeRegion(lat: number, lng: number): Promise { +async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise { const key = roundKey(lat, lng); if (regionCache.has(key)) return regionCache.get(key)!; + + // 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 + // coverage or the point lands outside every polygon. + const coordCountry = getCountryFromCoords(lat, lng); + if (coordCountry) { + const fromBundle = await getRegionFromCoords(coordCountry, lat, lng); + if (fromBundle) { + regionCache.set(key, 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 + // Luxembourg side of the Sauer river fell inside Germany's simplified box). Retry against + // the place's own stored address, the same order resolveCountryCode(Sync) uses for country + // resolution — but only as a fallback: trusting it FIRST regressed places whose address + // ends in a US state abbreviation that collides with a real ISO code (e.g. "...CA" parsed + // as Canada instead of California), which coordinates alone already resolve correctly. + // + // Sanity-gate the address country against its own admin0 BOX (not the tighter polygon — + // the Luxembourg case needs a country whose exact border misses this very point) before + // 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; + } + } + + // Only reached when the bundle's own polygons for this country don't cover the point at + // all (coastal/simplification gaps) — a genuinely rare miss. Nominatim's coarse address + // level (state/province) is what the bundle actually carries; the former GB "rescue" to a + // finer county/borough level targeted the old Natural Earth polygons and produced a code + // the current geoBoundaries bundle can never match, so it was removed. const address = await fetchNominatimAddress(lat, lng, 8); if (!address) return null; // transient failure — leave uncached so a later call retries - let info = buildRegionInfo(address, false); - // GB constituent-country codes map to no admin-1 polygon, so re-resolve them at a finer - // zoom where Nominatim exposes the county/borough code (GB-LND, GB-MAN, GB-CON, …) that - // the polygons actually carry. - if (info && info.country_code === 'GB' && GB_CONSTITUENT_CODES.has(info.region_code)) { - const finerAddress = await fetchNominatimAddress(lat, lng, 10); - const finer = finerAddress ? buildRegionInfo(finerAddress, true) : null; - if (finer && !GB_CONSTITUENT_CODES.has(finer.region_code)) info = finer; - } + const info = buildRegionInfo(address, false); regionCache.set(key, info); return info; } @@ -1269,7 +1436,7 @@ export async function getVisitedRegions( try { for (const place of uncached) { try { - const info = await reverseGeocodeRegion(place.lat!, place.lng!); + 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 diff --git a/server/tests/unit/services/atlasService.test.ts b/server/tests/unit/services/atlasService.test.ts index 2ced095f1..2ccf1f95e 100644 --- a/server/tests/unit/services/atlasService.test.ts +++ b/server/tests/unit/services/atlasService.test.ts @@ -31,7 +31,7 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createReservation } from '../../helpers/factories'; -import { getStats, getCached, setCache, getCountryFromCoords, getCountryFromAddress, reverseGeocodeCountry, getRegionGeo, getCountryGeo, getCountryPlaces, getVisitedRegions, markCountryVisited, unmarkCountryVisited } from '../../../src/services/atlasService'; +import { getStats, getCached, setCache, getCountryFromCoords, getCountryFromAddress, isPointInCountryBox, reverseGeocodeCountry, getRegionGeo, getCountryGeo, getCountryPlaces, getVisitedRegions, markCountryVisited, unmarkCountryVisited } from '../../../src/services/atlasService'; function insertReservationEndpoint( db: any, @@ -317,6 +317,27 @@ describe('getCountryFromCoords', () => { }); }); +// ── isPointInCountryBox — sanity gate for the address-derived region fallback ────── + +describe('isPointInCountryBox', () => { + it('ATLAS-SVC-006a: accepts a country whose box genuinely covers the point, even where the exact border excludes it', () => { + // Bollendorf-Pont: on the Luxembourg side of the border, but outside LU's exact + // simplified polygon (see getCountryFromCoords returning DE for this same point in + // atlasService.test.ts's region-resolution tests). The box gate must stay loose + // enough to admit this, or the Luxembourg address-fallback fix would regress. + expect(isPointInCountryBox('LU', 49.8502458, 6.3576404)).toBe(true); + }); + + it('ATLAS-SVC-006b: rejects a country whose box is nowhere near the point', () => { + // Mid-Atlantic, nowhere close to Japan under any simplification. + expect(isPointInCountryBox('JP', 20, -35)).toBe(false); + }); + + it('ATLAS-SVC-006c: returns false for an unknown/garbage country code', () => { + expect(isPointInCountryBox('ZZ', 48.85, 2.35)).toBe(false); + }); +}); + // ── Removing a visited country sticks (#1490) ─────────────────────────────── describe('unmarkCountryVisited — tombstones', () => { @@ -601,6 +622,36 @@ describe('getStats — extended', () => { expect(stats.lastTrip).toBeNull(); }); + + it('ATLAS-UNIT-027: a US place whose address ends in a state abbreviation resolves to US, not the colliding ISO country', async () => { + // getCountryFromAddress()'s "2-letter uppercase last segment = ISO code" heuristic + // parses "..., CA" as Canada (a real ISO code), not California. resolveCountryCodeSync + // used to try the address FIRST, so a place with coordinates that plainly resolve to the + // US via getCountryFromCoords would still get bucketed under Canada. Mirrors the + // region-level fix (ATLAS-UNIT-024) at the country level. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'San Francisco Trip' }); + insertPlaceWithCoords(testDb, trip.id, 'Hotel Pickwick', 37.7830549, -122.4066689, '85 5th St, San Francisco, CA'); + + const stats = await getStats(user.id); + + const codes = stats.countries.map((c: any) => c.code); + expect(codes).toContain('US'); + expect(codes).not.toContain('CA'); + }); + + it('ATLAS-UNIT-028: lastTrip.countryCode resolves via coordinates, not a misparsed state-abbreviation address', async () => { + // lastTrip.countryCode calls resolveCountryCodeSync directly (not through the + // place_regions cache), so this exercises the fix independently of ATLAS-UNIT-027. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Past NY Trip', start_date: '2023-05-01', end_date: '2023-05-10' }); + insertPlaceWithCoords(testDb, trip.id, 'Imperial Court Hotel', 40.7848394, -73.981643, '307 W 79th Street, New York, NY'); + + const stats = await getStats(user.id); + + expect(stats.lastTrip).not.toBeNull(); + expect(stats.lastTrip!.countryCode).toBe('US'); + }); }); // ── getCountryPlaces ───────────────────────────────────────────────────────── @@ -728,31 +779,167 @@ describe('getVisitedRegions', () => { expect(codes).toContain('FR-75'); }); - it('ATLAS-UNIT-021: GB places resolving to a constituent country are re-resolved to the finer admin-1 code', async () => { - vi.useFakeTimers(); - // A zoom-8 lookup only yields the constituent country (GB-ENG); the zoom-10 lookup - // exposes the borough code (GB-MAN) that Natural Earth's polygons actually carry. - vi.stubGlobal('fetch', vi.fn().mockImplementation((url: string) => Promise.resolve({ - ok: true, - json: async () => ({ - address: url.includes('zoom=10') - ? { country_code: 'gb', 'ISO3166-2-lvl8': 'GB-MAN', city: 'Manchester', state: 'England', 'ISO3166-2-lvl4': 'GB-ENG' } - : { country_code: 'gb', 'ISO3166-2-lvl4': 'GB-ENG', state: 'England' }, - }), - }))); + it('ATLAS-UNIT-021: a GB place resolves against the bundled admin1 polygon without calling Nominatim', async () => { + // The shipped geoBoundaries bundle only carries GB's 4 constituent countries + // (England/Scotland/Wales/Northern Ireland) — no county/borough level. Resolving + // Old Trafford's coordinates directly against those polygons lands on GB-ENG, the + // same feature the client highlights, with no reverse-geocode round trip at all. + const mockFetch = vi.fn(); + vi.stubGlobal('fetch', mockFetch); const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'Manchester Trip' }); insertPlaceWithCoords(testDb, trip.id, 'Old Trafford', 53.4631, -2.2913); + await getVisitedRegions(user.id); + // The background geocode is fire-and-forget; give its microtasks a turn to settle + // before reading the now-cached result back. + await new Promise(resolve => setTimeout(resolve, 10)); + const result = await getVisitedRegions(user.id); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(result.regions['GB']).toBeDefined(); + const codes = result.regions['GB'].map((r: any) => r.code); + expect(codes).toContain('GB-ENG'); + }); + + it('ATLAS-UNIT-022: a place whose Nominatim region level is finer than the bundle (Spain province vs autonomous community) still resolves to a bundle-matching feature', async () => { + // Regression for the Barcelona/Madrid bug: Nominatim's ISO3166-2-lvl6 gives the + // *province* (ES-B), but the bundle only has the *autonomous-community* level + // (Catalonia). Resolving by coordinates instead of trusting the geocoder's level + // guarantees a code the client bundle actually carries. + const mockFetch = vi.fn(); + vi.stubGlobal('fetch', mockFetch); + + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Barcelona Trip' }); + insertPlaceWithCoords(testDb, trip.id, 'Sagrada Familia', 41.4036, 2.1744); + + await getVisitedRegions(user.id); + // The background geocode is fire-and-forget; give its microtasks a turn to settle + // before reading the now-cached result back. + await new Promise(resolve => setTimeout(resolve, 10)); + const result = await getVisitedRegions(user.id); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(result.regions['ES']).toBeDefined(); + expect(result.regions['ES'][0].code).not.toBe('ES-B'); + }); + + it('ATLAS-UNIT-023: a place address disambiguates a border point the simplified admin0 polygon puts in the wrong country', async () => { + // A real Airbnb at Bollendorf-Pont sits on the Luxembourg side of the Sauer river, + // but the coordinates alone fall inside Germany's simplified admin0 polygon + // (border-simplification slop) — getCountryFromCoords(lat, lng) returns DE, so a + // coordinate-only region lookup finds nothing in DE. The place's own stored address + // says Luxembourg, so it is retried as a fallback before ever reaching Nominatim. + const mockFetch = vi.fn(); + vi.stubGlobal('fetch', mockFetch); + + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Luxembourg Trip' }); + insertPlaceWithCoords( + testDb, trip.id, 'Airbnb - Welcome Home', 49.8502458, 6.3576404, + '4 Gruusswiss, Bollendorf-Pont, Distrikt Gréiwemaacher 6555, Luxembourg' + ); + + await getVisitedRegions(user.id); + await new Promise(resolve => setTimeout(resolve, 10)); + const result = await getVisitedRegions(user.id); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(result.regions['LU']).toBeDefined(); + expect(result.regions['DE']).toBeUndefined(); + }); + + it('ATLAS-UNIT-024: a US place whose address ends in a state abbreviation still resolves by coordinates, ignoring the address', async () => { + // getCountryFromAddress() treats any 2-letter uppercase last address segment as an + // ISO country code — "...CA" parses as Canada, not California. Trusting the address + // FIRST (as ATLAS-UNIT-023 might suggest) would send a San Francisco hotel's region + // lookup to Canada and fail to find one, costing a needless Nominatim round trip (or + // worse, a wrong match) for every US place whose address ends in a state code. + // Coordinates resolve this correctly on their own, so the address must only be + // consulted when the coordinate-only lookup finds nothing. + const mockFetch = vi.fn(); + vi.stubGlobal('fetch', mockFetch); + + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'San Francisco Trip' }); + insertPlaceWithCoords( + testDb, trip.id, 'Hotel Pickwick', 37.7830549, -122.4066689, + '85 5th St, San Francisco, CA' + ); + + await getVisitedRegions(user.id); + await new Promise(resolve => setTimeout(resolve, 10)); + const result = await getVisitedRegions(user.id); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(result.regions['US']).toBeDefined(); + expect(result.regions['CA']).toBeUndefined(); + }); + + it('ATLAS-UNIT-025: when the bundle-only lookup finds nothing, the Nominatim fallback keeps the coarse GB constituent-country code instead of rescuing to a finer one', async () => { + // Mid-Atlantic open ocean — getCountryFromCoords finds no country and there's no + // address, so this always falls through to the Nominatim path. That path used to re-query at a + // finer zoom for GB and swap in a county/borough code (GB-MAN, GB-LND, …) that targeted + // Natural Earth's old, finer GB polygons — the current geoBoundaries bundle only has the + // 4 constituent countries, so that rescued code could never match anything and the + // region would never highlight. The coarse Nominatim result (GB-ENG) IS a real bundle + // feature and must be kept as-is, with a single geocode call (no zoom=10 re-query). + vi.useFakeTimers(); + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ address: { country_code: 'gb', 'ISO3166-2-lvl4': 'GB-ENG', state: 'England' } }), + }); + vi.stubGlobal('fetch', mockFetch); + + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Middle of the ocean' }); + insertPlaceWithCoords(testDb, trip.id, 'Buoy', 10, -40); + await getVisitedRegions(user.id); await vi.runAllTimersAsync(); const result = await getVisitedRegions(user.id); + expect(mockFetch).toHaveBeenCalledTimes(1); expect(result.regions['GB']).toBeDefined(); const codes = result.regions['GB'].map((r: any) => r.code); - expect(codes).toContain('GB-MAN'); - expect(codes).not.toContain('GB-ENG'); + expect(codes).toContain('GB-ENG'); + + vi.useRealTimers(); + }); + + it('ATLAS-UNIT-026: an address country nowhere near the coordinates is rejected before it can produce a bogus region match', async () => { + // Coordinates in the open mid-Atlantic (no country polygon contains them) paired + // with a stored address ending in "JP" — getCountryFromAddress()'s 2-letter-uppercase + // heuristic returns 'JP' regardless of how implausible that is for these coordinates. + // Without a sanity check, getRegionFromCoords('JP', ...) would only return null because + // no Japanese region polygon happens to reach the mid-Atlantic — but that's incidental, + // not a guarantee, for some other coordinate/bogus-code combination. The admin0 box + // gate rejects JP outright (its bounding box is nowhere near these coordinates) so the + // address is never even tried against JP's regions, and resolution correctly falls + // through to Nominatim instead of risking a wrong match. + vi.useFakeTimers(); + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ address: {} }), // Nominatim finds nothing here either + }); + vi.stubGlobal('fetch', mockFetch); + + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Mid-Atlantic buoy' }); + // Different coordinates than ATLAS-UNIT-025's mid-Atlantic point — reverseGeocodeRegion's + // regionCache is an in-memory Map keyed by rounded lat/lng and persists across tests in + // this file, so reusing the same point would silently hit that cached result instead of + // exercising this test's fetch/gate path. + insertPlaceWithCoords(testDb, trip.id, 'Weather buoy', 20, -35, '123 Nowhere Rd, JP'); + + await getVisitedRegions(user.id); + await vi.runAllTimersAsync(); + const result = await getVisitedRegions(user.id); + + expect(mockFetch).toHaveBeenCalledTimes(1); // fell through to Nominatim, not a fabricated JP match + expect(result.regions['JP']).toBeUndefined(); vi.useRealTimers(); });