From c040d21dc6bab1ef3789dff455b91f236498ad38 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Sat, 27 Jun 2026 15:55:15 -0400 Subject: [PATCH 1/4] feat(geocode): near-me address geocoding via D1 cache + Meili _geo backfill Make address-only events discoverable in near-me search by geocoding their addresses to coordinates and backfilling _geo into Meilisearch: - shared address normalization for stable cache keys - config-selected Nominatim/LocationIQ geocoder client - geocode_cache D1 table + HTTP client, with a negative-cache policy and worklist grouping - best-effort cache lookup in the sink so writes preserve _geo - external geocode job that backfills _geo for already-indexed events via a Meili _geo-only partial update (PUT/merge, not POST/replace) Uses standard CLOUDFLARE_* env vars and requires an explicit D1 target; treats a LocationIQ 404 as a no-match rather than a transient error. --- apps/web/package.json | 3 +- apps/web/scripts/geocode-cache.sql | 14 + apps/web/scripts/geocode-events.ts | 263 ++++++++++++++++++ apps/web/src/lib/contrail.config.ts | 11 +- apps/web/src/lib/contrail.ts | 33 +-- apps/web/src/lib/contrail/index.ts | 14 +- .../lib/search/server/address-norm.test.ts | 115 ++++++++ .../web/src/lib/search/server/address-norm.ts | 41 +++ .../web/src/lib/search/server/d1-http.test.ts | 61 ++++ apps/web/src/lib/search/server/d1-http.ts | 43 +++ .../lib/search/server/discoverability.test.ts | 60 ++++ .../src/lib/search/server/discoverability.ts | 35 +++ .../lib/search/server/geocode-cache.test.ts | 128 +++++++++ .../src/lib/search/server/geocode-cache.ts | 80 ++++++ .../src/lib/search/server/geocoder.test.ts | 140 ++++++++++ apps/web/src/lib/search/server/geocoder.ts | 144 ++++++++++ .../server/meili-sink.integration.test.ts | 57 +++- .../src/lib/search/server/meili-sink.test.ts | 208 +++++++++++++- apps/web/src/lib/search/server/meili-sink.ts | 72 +++-- .../src/lib/search/server/normalize.test.ts | 45 ++- apps/web/src/lib/search/server/normalize.ts | 24 +- 21 files changed, 1531 insertions(+), 60 deletions(-) create mode 100644 apps/web/scripts/geocode-cache.sql create mode 100644 apps/web/scripts/geocode-events.ts create mode 100644 apps/web/src/lib/search/server/address-norm.test.ts create mode 100644 apps/web/src/lib/search/server/address-norm.ts create mode 100644 apps/web/src/lib/search/server/d1-http.test.ts create mode 100644 apps/web/src/lib/search/server/d1-http.ts create mode 100644 apps/web/src/lib/search/server/discoverability.test.ts create mode 100644 apps/web/src/lib/search/server/discoverability.ts create mode 100644 apps/web/src/lib/search/server/geocode-cache.test.ts create mode 100644 apps/web/src/lib/search/server/geocode-cache.ts create mode 100644 apps/web/src/lib/search/server/geocoder.test.ts create mode 100644 apps/web/src/lib/search/server/geocoder.ts diff --git a/apps/web/package.json b/apps/web/package.json index ec05b5f..c1355ef 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -25,7 +25,8 @@ "env:setup-dev": "npx tsx src/lib/atproto/scripts/setup-dev.ts", "tunnel": "npx tsx src/lib/atproto/scripts/tunnel.ts", "publish-lexicons": "contrail-lex publish", - "seed:conference": "bun run scripts/publish-test-conference.ts" + "seed:conference": "bun run scripts/publish-test-conference.ts", + "geocode:backfill": "tsx scripts/geocode-events.ts" }, "devDependencies": { "@atcute/atproto": "^3.1.10", diff --git a/apps/web/scripts/geocode-cache.sql b/apps/web/scripts/geocode-cache.sql new file mode 100644 index 0000000..20c5f23 --- /dev/null +++ b/apps/web/scripts/geocode-cache.sql @@ -0,0 +1,14 @@ +-- Derived-coordinate cache for address-only events + the geocode job's worklist +-- and done-marker (Meili can't filter "missing _geo", so we track resolution +-- ourselves). Idempotent / restartable. Applied once to the openmeet-atmo D1; +-- the sink reads it, the external geocode job writes it. +CREATE TABLE IF NOT EXISTS geocode_cache ( + address_norm TEXT PRIMARY KEY, -- normalized address key (address-norm.ts) + lat REAL, -- NULL when unresolved (negative cache) + lng REAL, + precision TEXT, -- provider-reported granularity + source TEXT NOT NULL, -- e.g. 'locationiq' / 'nominatim' + geocoded_at INTEGER NOT NULL, -- epoch ms + fail_count INTEGER NOT NULL DEFAULT 0, + last_error TEXT +); diff --git a/apps/web/scripts/geocode-events.ts b/apps/web/scripts/geocode-events.ts new file mode 100644 index 0000000..b3413f2 --- /dev/null +++ b/apps/web/scripts/geocode-events.ts @@ -0,0 +1,263 @@ +// apps/web/scripts/geocode-events.ts +// External geocode job (runs OFF Cloudflare): finds address-only events in the +// openmeet-atmo D1 that lack coordinates, geocodes each unique address through +// the config-selected Nominatim/LocationIQ client, writes geocode_cache, and +// _geo-updates the affected Meili docs. ONE job: "find work" and "geocode" are +// sequential steps sharing the cache, not two passes. Rate limiting is the +// sleep between calls — the whole reason geocoding stays off Workers. +// +// Run (backfill MUST point the geocoder at LocationIQ, never public Nominatim): +// CLOUDFLARE_API_TOKEN=… MEILI_URL=https://search.testnet.openmeet.net MEILI_KEY=… \ +// GEOCODER_URL=https://us1.locationiq.com/v1/search GEOCODER_KEY=… \ +// pnpm -C apps/web exec tsx scripts/geocode-events.ts --limit 50 +import { createD1Client, type D1Client } from '../src/lib/search/server/d1-http'; +import { + createGeocoder, + addressToQuery, + requireGeocoderForBulk +} from '../src/lib/search/server/geocoder'; +import { + isEligible, + groupEventsByNorm, + addressNeedingGeocode, + type GeocodeCacheRow, + type WorklistEvent +} from '../src/lib/search/server/geocode-cache'; +import { eventToSearchDoc } from '../src/lib/search/server/normalize'; +import { discoverableSql } from '../src/lib/search/server/discoverability'; +import { MeiliEventIndex, EVENT_COLLECTION } from '../src/lib/search/server/meili-sink'; + +const env = process.env; +const argv = process.argv.slice(2); +const flag = (name: string) => argv.includes(name); +const opt = (name: string, def?: string) => { + const i = argv.indexOf(name); + return i >= 0 && i + 1 < argv.length ? argv[i + 1] : def; +}; + +// Strict: 0 = no cap, otherwise a positive integer. Reject negatives/non-integers +// up front — `Number('-1') || 0` is -1, which used to slip past BOTH the bulk +// guard (its old limit===0 check) and the cap (`limit > 0 ? slice : all`), +// silently running an uncapped keyless backfill against public Nominatim. +const parseLimit = (raw: string | undefined): number => { + const n = Number(raw); + if (!Number.isInteger(n) || n < 0) { + throw new Error( + `--limit must be a non-negative integer (0 = no cap); got ${JSON.stringify(raw)}` + ); + } + return n; +}; + +const retryNegative = flag('--retry-negative'); +const dryRun = flag('--dry-run'); +const allowPublicNominatim = flag('--allow-public-nominatim'); +const limit = parseLimit(opt('--limit', '0')); // 0 = no cap +const sleepMs = Number(env.GEOCODE_SLEEP_MS ?? '1100'); + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +// Worklist: discoverable events carrying a .address location. We deliberately +// do NOT exclude coordinate locations in SQL — that filtered by $type presence, +// which diverges from the sink's actual _geo derivation (it ignored fsq, and +// excluded events whose only geo/hthree coords are out of range and so never +// get an in-index _geo). The precise "does this already resolve to coordinates?" +// decision is made in memory by addressNeedingGeocode (recordGeo), the same +// derivation the sink uses. json_each walks locations[]; the "$type" key is +// quoted because it starts with $. +const WORKLIST_SQL = ` +SELECT r.uri AS uri, r.did AS did, r.rkey AS rkey, r.record AS record +FROM records_event AS r +WHERE EXISTS ( + SELECT 1 FROM json_each(r.record, '$.locations') + WHERE json_extract(value, '$."$type"') = 'community.lexicon.location.address') + AND ${discoverableSql('r.record')} +`; + +// Re-read events from records_event by uri, keeping only those still present and +// discoverable. The parsed record drives a full-doc upsert, so it's read fresh +// (not from the job-start worklist) — an event deleted or unlisted while the slow +// geocode loop runs is skipped here instead of being resurrected as a stale doc. +async function fetchLiveDocs( + d1: D1Client, + uris: string[] +): Promise<{ uri: string; did: string; rkey: string; record: Record }[]> { + if (uris.length === 0) return []; + const placeholders = uris.map(() => '?').join(','); + const rows = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( + `SELECT uri, did, rkey, record FROM records_event + WHERE uri IN (${placeholders}) AND ${discoverableSql('record')}`, + uris + ); + const out: { uri: string; did: string; rkey: string; record: Record }[] = []; + for (const r of rows) { + try { + out.push({ uri: r.uri, did: r.did, rkey: r.rkey, record: JSON.parse(r.record) }); + } catch { + // Unparseable record JSON — skip, same as the worklist parse. + } + } + return out; +} + +async function main() { + if (!env.CLOUDFLARE_API_TOKEN) throw new Error('CLOUDFLARE_API_TOKEN is required'); + // No hardcoded account/DB fallbacks: the target D1 must be chosen explicitly so the + // job can never silently write a baked-in database, and so no infra IDs live in source. + if (!env.CLOUDFLARE_ACCOUNT_ID || !env.D1_DATABASE_ID) + throw new Error('CLOUDFLARE_ACCOUNT_ID and D1_DATABASE_ID are required'); + if (!env.MEILI_URL || !env.MEILI_KEY) throw new Error('MEILI_URL and MEILI_KEY are required'); + if (!env.GEOCODER_KEY) { + console.warn( + '[geocode] GEOCODER_KEY is unset → using PUBLIC Nominatim. OK for a small drip; ' + + 'a bulk backfill against public Nominatim risks a silent IP ban. Use LocationIQ for backfill.' + ); + } + // Hard-stop a bulk/uncapped keyless run before it can touch public Nominatim. + requireGeocoderForBulk({ + hasKey: !!env.GEOCODER_KEY, + dryRun, + limit, + allowPublic: allowPublicNominatim + }); + + const d1 = createD1Client({ + accountId: env.CLOUDFLARE_ACCOUNT_ID, + databaseId: env.D1_DATABASE_ID, + apiToken: env.CLOUDFLARE_API_TOKEN + }); + const geocoder = createGeocoder(env); + const meili = new MeiliEventIndex({ + url: env.MEILI_URL, + apiKey: env.MEILI_KEY, + indexUid: env.SEARCH_INDEX ?? 'events' + }); + + // Defensive: the table is created by geocode-cache.sql, but a fresh DB + // shouldn't make the job crash before it can self-heal. + await d1.query( + `CREATE TABLE IF NOT EXISTS geocode_cache ( + address_norm TEXT PRIMARY KEY, lat REAL, lng REAL, precision TEXT, + source TEXT NOT NULL, geocoded_at INTEGER NOT NULL, + fail_count INTEGER NOT NULL DEFAULT 0, last_error TEXT)` + ); + + // Load the whole cache once (small) and decide eligibility in memory. + const cacheRows = await d1.query(`SELECT * FROM geocode_cache`); + const cache = new Map(cacheRows.map((r) => [r.address_norm, r])); + + // Worklist → WorklistEvent[] (parse record JSON; keep only events that need + // geocoding — an address location AND no coordinates the index already + // derives, per addressNeedingGeocode). + const rawEvents = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( + WORKLIST_SQL + ); + const events: WorklistEvent[] = []; + for (const e of rawEvents) { + let record: Record; + try { + record = JSON.parse(e.record); + } catch { + continue; + } + const loc = addressNeedingGeocode(record); + if (loc) events.push({ uri: e.uri, did: e.did, rkey: e.rkey, loc }); + } + + const byNorm = groupEventsByNorm(events); + const now = Date.now(); + const work = [...byNorm.entries()].filter(([norm]) => + isEligible(cache.get(norm), now, retryNegative) + ); + const capped = limit > 0 ? work.slice(0, limit) : work; + + console.log( + `[geocode] worklist events=${events.length} unique-addresses=${byNorm.size} ` + + `eligible=${work.length} processing=${capped.length}${dryRun ? ' (dry-run)' : ''}` + ); + + let resolved = 0; + let negative = 0; + let transient = 0; + let skippedGone = 0; + for (const [norm, group] of capped) { + const query = addressToQuery(group[0].loc); + if (dryRun) { + console.log(`[geocode] would geocode "${query}" -> ${group.length} event(s)`); + continue; + } + try { + const point = await geocoder.geocode(query); + if (point) { + await d1.query( + `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) + VALUES (?, ?, ?, ?, ?, ?, 0, NULL) + ON CONFLICT(address_norm) DO UPDATE SET + lat=excluded.lat, lng=excluded.lng, precision=excluded.precision, + source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=0, last_error=NULL`, + [ + norm, + point.lat, + point.lng, + point.precision ?? null, + env.GEOCODER_KEY ? 'locationiq' : 'nominatim', + now + ] + ); + // Re-read the events fresh and upsert the FULL doc with _geo attached, + // keeping only those still present AND discoverable. A full-doc upsert is + // idempotent and needs no "is it indexed?" snapshot: it merges if the + // event is already indexed, lands a complete doc if not (never a {id,_geo} + // stub), and is identical to what the sink writes — so a concurrent sink + // write converges instead of clobbering. + const live = await fetchLiveDocs( + d1, + group.map((e) => e.uri) + ); + skippedGone += group.length - live.length; + if (live.length) { + await meili.upsert( + live.map((r) => { + const doc = eventToSearchDoc({ + uri: r.uri, + did: r.did, + collection: EVENT_COLLECTION, + rkey: r.rkey, + record: r.record + }); + // Don't overwrite a coordinate _geo the record gained mid-run. + if (!doc._geo) doc._geo = { lat: point.lat, lng: point.lng }; + return doc; + }) + ); + } + resolved++; + } else { + // No-match: write/increment a negative row (backoff handled by isEligible). + await d1.query( + `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) + VALUES (?, NULL, NULL, NULL, ?, ?, 1, 'no match') + ON CONFLICT(address_norm) DO UPDATE SET + source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=geocode_cache.fail_count+1, last_error='no match'`, + [norm, env.GEOCODER_KEY ? 'locationiq' : 'nominatim', now] + ); + negative++; + } + } catch (err) { + // Transient (HTTP/network): DON'T write a negative row — retry next run. + transient++; + console.warn(`[geocode] transient error for "${query}": ${(err as Error).message}`); + } + await sleep(sleepMs); + } + + console.log( + `[geocode] done resolved=${resolved} negative=${negative} transient=${transient} ` + + `skipped-gone=${skippedGone}` + ); +} + +main().catch((e) => { + console.error('[geocode] fatal:', e); + process.exit(1); +}); diff --git a/apps/web/src/lib/contrail.config.ts b/apps/web/src/lib/contrail.config.ts index c54943d..5ef7011 100644 --- a/apps/web/src/lib/contrail.config.ts +++ b/apps/web/src/lib/contrail.config.ts @@ -2,6 +2,7 @@ import type { ContrailConfig } from '@atmo-dev/contrail'; import { SPACE_TYPE } from './spaces/config'; import { MAX_HYDRATION_URIS } from './search/constants'; import { createMeiliSink, meiliSinkBackendFromEnv } from './search/server/meili-sink'; +import { discoverableSql } from './search/server/discoverability'; // The `contrail` CLI (`pnpm backfill` / `contrail refresh`) fires `config.sinks` // on the backfill/refresh paths, so a fresh or re-synced install gets full search @@ -16,11 +17,11 @@ const searchSinks: NonNullable = ? [createMeiliSink(() => meiliSinkBackendFromEnv(process.env))] : []; -// Events hidden from discovery (preferences.showInDiscovery === false) are -// excluded; a missing field defaults to true so pre-existing records without -// `preferences` are included. Shared by every discovery-facing pipelineQuery. -const DISCOVERABLE_CONDITION = `(json_extract(r.record, '$.preferences.showInDiscovery') IS NULL - OR json_extract(r.record, '$.preferences.showInDiscovery') != 0)`; +// Events hidden from discovery (a falsey preferences.showInDiscovery) are +// excluded; a missing field defaults to discoverable so pre-existing records +// without `preferences` are included. The predicate is the single source of +// truth shared with the sink's in-memory filter and the geocode worklist. +const DISCOVERABLE_CONDITION = discoverableSql('r.record'); export const config: ContrailConfig = { namespace: 'rsvp.atmo', diff --git a/apps/web/src/lib/contrail.ts b/apps/web/src/lib/contrail.ts index 2900a88..b7f55af 100644 --- a/apps/web/src/lib/contrail.ts +++ b/apps/web/src/lib/contrail.ts @@ -280,9 +280,9 @@ export async function listEventRecordsFromContrail( /** * Hits the `listDiscoverable` pipelineQuery, which reuses the listRecords - * pipeline but adds a WHERE condition excluding events where - * `preferences.showInDiscovery === false`. Missing field is treated as true. - * Response shape is identical to listRecords. + * pipeline but adds a WHERE condition excluding events with a falsey + * `preferences.showInDiscovery` (false or 0). Missing field is treated as + * discoverable. Response shape is identical to listRecords. */ export async function listDiscoverableEventsFromContrail( client: Client, @@ -351,24 +351,17 @@ export async function listDiscoverableEventsByUrisFromContrail( */ export async function listConferenceTalksFromContrail( client: Client, - { - parentUri, - actor, - limit = 300 - }: { parentUri: string; actor?: ActorIdentifier; limit?: number } + { parentUri, actor, limit = 300 }: { parentUri: string; actor?: ActorIdentifier; limit?: number } ): Promise { - const response = await client.get( - 'rsvp.atmo.event.listTalks' as 'rsvp.atmo.event.listRecords', - { - params: { - ...(actor ? { actor } : {}), - parentUri, - sort: 'startsAt', - order: 'asc', - limit - } as ListEventsParams - } - ); + const response = await client.get('rsvp.atmo.event.listTalks' as 'rsvp.atmo.event.listRecords', { + params: { + ...(actor ? { actor } : {}), + parentUri, + sort: 'startsAt', + order: 'asc', + limit + } as ListEventsParams + }); if (!response.ok) return null; return response.data; diff --git a/apps/web/src/lib/contrail/index.ts b/apps/web/src/lib/contrail/index.ts index 41b44d8..acc5abf 100644 --- a/apps/web/src/lib/contrail/index.ts +++ b/apps/web/src/lib/contrail/index.ts @@ -27,16 +27,28 @@ if (!spacesAvailable()) { // which is fine: reads never ingest, so the sink never fires there. let searchSinkBackend: MeiliSinkBackend | null = null; +// The geocode cache lives in D1; the sink reads it (read-only, best-effort) to +// reproduce the _geo the external geocode job writes, so a live update doesn't +// drop it. Like searchSinkBackend, it's a module-level holder the env-bearing +// ensureInit populates — the sink no-ops on it until then. +let geocodeCacheDb: D1Database | null = null; + export const contrail = new Contrail({ ...config, ...(spaces ? { spaces } : {}), - sinks: [createMeiliSink(() => searchSinkBackend)] + sinks: [ + createMeiliSink( + () => searchSinkBackend, + () => geocodeCacheDb + ) + ] }); let initialized = false; let sinkConfigured = false; export async function ensureInit(db: D1Database, env?: MeiliSinkEnv) { + geocodeCacheDb = db; if (!initialized) { await contrail.init(db); initialized = true; diff --git a/apps/web/src/lib/search/server/address-norm.test.ts b/apps/web/src/lib/search/server/address-norm.test.ts new file mode 100644 index 0000000..fdb512d --- /dev/null +++ b/apps/web/src/lib/search/server/address-norm.test.ts @@ -0,0 +1,115 @@ +// apps/web/src/lib/search/server/address-norm.test.ts +import { describe, it, expect } from 'vitest'; +import { normalizeAddress, addressLocation, ADDRESS_TYPE } from './address-norm'; +import { addressNeedingGeocode } from './geocode-cache'; + +describe('normalizeAddress', () => { + it('joins all present fields in fixed order with | separators', () => { + const key = normalizeAddress({ + country: 'US', + locality: 'Dayton', + street: '905 East 3rd Street', + region: 'Ohio', + postalCode: '45402', + name: 'The Venue' + }); + // fixed order: name, street, locality, region, postalCode, country + expect(key).toBe('the venue|905 east 3rd street|dayton|ohio|45402|us'); + }); + + it('omits absent/empty fields entirely', () => { + expect(normalizeAddress({ locality: 'Dayton', country: 'US' })).toBe('dayton|us'); + }); + + it('lowercases, NFC-normalizes, collapses whitespace, trims edge punctuation', () => { + expect(normalizeAddress({ locality: 'Dayton,', region: ' Ohio State ' })).toBe( + 'dayton|ohio state' + ); + }); + + it('strips the separator char from field content', () => { + expect(normalizeAddress({ name: 'A|B', country: 'US' })).toBe('a b|us'); + }); + + it('returns null when nothing is present', () => { + expect(normalizeAddress({})).toBeNull(); + expect(normalizeAddress({ country: ' ' })).toBeNull(); + }); + + it('produces an identical key regardless of field insertion order', () => { + const a = normalizeAddress({ country: 'US', locality: 'Dayton' }); + const b = normalizeAddress({ locality: 'Dayton', country: 'US' }); + expect(a).toBe(b); + }); +}); + +describe('addressLocation', () => { + it('returns the first .address location', () => { + const loc = addressLocation({ + locations: [ + { $type: 'community.lexicon.location.geo', latitude: '1', longitude: '2' }, + { $type: ADDRESS_TYPE, locality: 'Dayton', country: 'US' } + ] + }); + expect(loc).toMatchObject({ locality: 'Dayton', country: 'US' }); + }); + + it('returns null when no address location is present', () => { + expect( + addressLocation({ locations: [{ $type: 'community.lexicon.location.geo' }] }) + ).toBeNull(); + expect(addressLocation({})).toBeNull(); + }); +}); + +// INV-A: the geocode job (write path) keys geocode_cache by +// normalizeAddress(addressNeedingGeocode(record)); the sink (read path) looks it +// back up by normalizeAddress(addressLocation(record)). If those two keys ever +// diverge, the _geo the job writes is invisible to the sink and near-me silently +// loses coordinates. These lock the round-trip - including across the Unicode/ +// punctuation/whitespace representation drift a PDS re-serialization can introduce. +describe('INV-A job <-> sink cache-key parity', () => { + const jobKey = (record: Record) => { + const loc = addressNeedingGeocode(record); + return loc ? normalizeAddress(loc) : null; + }; + const sinkKey = (record: Record) => { + const loc = addressLocation(record); + return loc ? normalizeAddress(loc) : null; + }; + + it('derives the same non-null key on both paths for one address-only record', () => { + const record = { + locations: [{ $type: ADDRESS_TYPE, name: 'The Venue', locality: 'Dayton', country: 'US' }] + }; + const k = jobKey(record); + expect(k).not.toBeNull(); + expect(sinkKey(record)).toBe(k); + }); + + it('matches across NFD/NFC, case, whitespace and edge-punctuation drift', () => { + // Precomposed (NFC) base strings as the source file stores them. + const venueNFC = 'Café Bar'; + const cityNFC = 'Zürich'; + // Decompose to NFD at runtime so the job side genuinely carries combining + // marks - independent of how this file happens to be encoded - then add the + // messy edges (doubled spaces, trailing comma) the job might first see. + const jobName = ` ${venueNFC.normalize('NFD')} `; + const jobCity = `${cityNFC.normalize('NFD')},`; + const jobRecord = { + locations: [{ $type: ADDRESS_TYPE, name: jobName, locality: jobCity, country: 'CH' }] + }; + // The same venue re-serialized: precomposed (NFC), already trimmed/cased. + const sinkRecord = { + locations: [{ $type: ADDRESS_TYPE, name: venueNFC, locality: cityNFC, country: 'ch' }] + }; + // The raw job/sink inputs differ in codepoints (NFD carries combining marks + // NFC lacks); only NFC folding makes them equal, so this fails if the + // normalize step ever drops it. + expect(jobName).not.toBe(venueNFC); + const expected = `${venueNFC.normalize('NFC').toLowerCase()}|${cityNFC.normalize('NFC').toLowerCase()}|ch`; + const k = jobKey(jobRecord); + expect(k).toBe(expected); + expect(sinkKey(sinkRecord)).toBe(k); + }); +}); diff --git a/apps/web/src/lib/search/server/address-norm.ts b/apps/web/src/lib/search/server/address-norm.ts new file mode 100644 index 0000000..1933eab --- /dev/null +++ b/apps/web/src/lib/search/server/address-norm.ts @@ -0,0 +1,41 @@ +// Deterministic cache key for a community.lexicon.location.address, shared +// byte-for-byte by the search sink (read path) and the external geocode job +// (write path). If these two ever diverge the cache keys stop matching and the +// _geo the job writes is invisible to the sink, so this module is the single +// source of truth for both. + +export const ADDRESS_TYPE = 'community.lexicon.location.address'; + +const SEP = '|'; +// Fixed field order, independent of the order fields appear in the record, so +// identical data always yields the same key. `name` MUST be included: ~750 +// events carry only a venue name + country, and dropping it would collapse +// every venue-only event in a country into one row and cross-contaminate coords. +const FIELDS = ['name', 'street', 'locality', 'region', 'postalCode', 'country']; + +export function normalizeAddress(loc: Record): string | null { + const key = FIELDS.map((f) => (typeof loc[f] === 'string' ? (loc[f] as string) : '')) + .map((v) => + v + .normalize('NFC') + .toLowerCase() + .replace(/\s+/g, ' ') + .replace(/\|/g, ' ') + .replace(/^[\s,.;:/-]+|[\s,.;:/-]+$/g, '') + .trim() + ) + .filter((v) => v !== '') + .join(SEP); + return key === '' ? null : key; +} + +/** First community.lexicon.location.address in a record's locations[], or null. */ +export function addressLocation(record: Record): Record | null { + const locs = Array.isArray(record?.locations) ? record.locations : []; + for (const l of locs) { + if (l && typeof l === 'object' && (l as Record).$type === ADDRESS_TYPE) { + return l as Record; + } + } + return null; +} diff --git a/apps/web/src/lib/search/server/d1-http.test.ts b/apps/web/src/lib/search/server/d1-http.test.ts new file mode 100644 index 0000000..3c6e3df --- /dev/null +++ b/apps/web/src/lib/search/server/d1-http.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, vi } from 'vitest'; +import { createD1Client } from './d1-http'; + +const CFG = { accountId: 'acct', databaseId: 'db', apiToken: 'tok' }; + +function fakeFetch(body: unknown, status = 200) { + const calls: { url: string; method: string; auth: string; body: unknown }[] = []; + const fn = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + calls.push({ + url: String(input), + method: init?.method ?? 'GET', + auth: (init?.headers as Record)?.authorization ?? '', + body: init?.body ? JSON.parse(String(init.body)) : undefined + }); + return new Response(JSON.stringify(body), { status }); + }); + return { fn: fn as unknown as typeof fetch, calls }; +} + +describe('createD1Client.query', () => { + it('POSTs sql+params to the D1 query endpoint and returns the first result set', async () => { + const { fn, calls } = fakeFetch({ + success: true, + result: [{ results: [{ uri: 'at://x' }], success: true }] + }); + const rows = await createD1Client(CFG, fn).query( + 'SELECT uri FROM records_event WHERE did = ?', + ['did:plc:a'] + ); + expect(rows).toEqual([{ uri: 'at://x' }]); + expect(calls[0].url).toBe( + 'https://api.cloudflare.com/client/v4/accounts/acct/d1/database/db/query' + ); + expect(calls[0].method).toBe('POST'); + expect(calls[0].auth).toBe('Bearer tok'); + expect(calls[0].body).toEqual({ + sql: 'SELECT uri FROM records_event WHERE did = ?', + params: ['did:plc:a'] + }); + }); + + it('returns [] when the result set is empty', async () => { + const { fn } = fakeFetch({ success: true, result: [{ results: [], success: true }] }); + expect(await createD1Client(CFG, fn).query('SELECT 1')).toEqual([]); + }); + + it('throws on an HTTP error', async () => { + const { fn } = fakeFetch({}, 401); + await expect(createD1Client(CFG, fn).query('SELECT 1')).rejects.toThrow(/401/); + }); + + it('throws on a D1 error envelope (success:false)', async () => { + const { fn } = fakeFetch({ success: false, errors: [{ message: 'bad sql' }] }); + await expect(createD1Client(CFG, fn).query('SELECT')).rejects.toThrow(/bad sql/); + }); + + it('falls back to "unknown D1 error" when all error objects lack a message', async () => { + const { fn } = fakeFetch({ success: false, errors: [{}, {}] }); + await expect(createD1Client(CFG, fn).query('SELECT 1')).rejects.toThrow(/unknown D1 error/); + }); +}); diff --git a/apps/web/src/lib/search/server/d1-http.ts b/apps/web/src/lib/search/server/d1-http.ts new file mode 100644 index 0000000..195f914 --- /dev/null +++ b/apps/web/src/lib/search/server/d1-http.ts @@ -0,0 +1,43 @@ +// Thin Cloudflare D1 REST client for the external geocode job, which runs off +// Cloudflare (no Worker D1 binding). One parameterized-query method; the job +// does all its reads/writes through it. +export interface D1HttpConfig { + accountId: string; + databaseId: string; + apiToken: string; +} + +export interface D1Client { + query>(sql: string, params?: unknown[]): Promise; +} + +export function createD1Client(cfg: D1HttpConfig, fetchImpl: typeof fetch = fetch): D1Client { + const endpoint = `https://api.cloudflare.com/client/v4/accounts/${cfg.accountId}/d1/database/${cfg.databaseId}/query`; + return { + async query>(sql: string, params: unknown[] = []): Promise { + const res = await fetchImpl(endpoint, { + method: 'POST', + headers: { + authorization: `Bearer ${cfg.apiToken}`, + 'content-type': 'application/json' + }, + body: JSON.stringify({ sql, params }) + }); + if (!res.ok) throw new Error(`D1 query failed: ${res.status}`); + const body = (await res.json()) as { + success: boolean; + result?: Array<{ results: T[] }>; + errors?: Array<{ message?: string }>; + }; + if (!body.success) { + const msg = + body.errors + ?.map((e) => e.message) + .filter(Boolean) + .join('; ') || 'unknown D1 error'; + throw new Error(`D1 query error: ${msg}`); + } + return body.result?.[0]?.results ?? []; + } + }; +} diff --git a/apps/web/src/lib/search/server/discoverability.test.ts b/apps/web/src/lib/search/server/discoverability.test.ts new file mode 100644 index 0000000..5530dca --- /dev/null +++ b/apps/web/src/lib/search/server/discoverability.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest'; +import { discoverableSql, isHiddenFromDiscovery } from './discoverability'; + +// The single rule both the SQL read paths (contrail.config.ts pipelineQueries, +// the geocode worklist) and the in-memory sink filter (meili-sink.ts) must agree +// on. If they drift, the index and the hydrated read path diverge: the sink +// indexes an event D1 hides (or vice-versa), creating a phantom doc. +// +// `sqlVisible` is the ground-truth verdict of the shared SQL predicate +// json_extract(...,'$.preferences.showInDiscovery') IS NULL OR != 0 +// evaluated against REAL SQLite (node:sqlite). Each row was confirmed by running +// the exact predicate over the JSON value; the table encodes those results so the +// parity check stays inside the Worker-typed test env (no node:sqlite/@types/node +// import here). SQLite maps JSON false→0 and true→1 and passes numbers through, so +// `!= 0` hides BOTH boolean false AND numeric 0 (and 0.0); strings, null, true, +// and missing all stay visible. +const PARITY: { label: string; record: Record; sqlVisible: boolean }[] = [ + { label: 'true', record: { preferences: { showInDiscovery: true } }, sqlVisible: true }, + { label: 'false', record: { preferences: { showInDiscovery: false } }, sqlVisible: false }, + { label: 'numeric 0', record: { preferences: { showInDiscovery: 0 } }, sqlVisible: false }, + { label: 'float 0.0', record: { preferences: { showInDiscovery: 0.0 } }, sqlVisible: false }, + { label: 'numeric 1', record: { preferences: { showInDiscovery: 1 } }, sqlVisible: true }, + { label: 'explicit null', record: { preferences: { showInDiscovery: null } }, sqlVisible: true }, + { + label: 'string "false"', + record: { preferences: { showInDiscovery: 'false' } }, + sqlVisible: true + }, + { label: 'string "0"', record: { preferences: { showInDiscovery: '0' } }, sqlVisible: true }, + { label: 'field absent', record: { preferences: {} }, sqlVisible: true }, + { label: 'preferences absent', record: {}, sqlVisible: true } +]; + +describe('isHiddenFromDiscovery', () => { + for (const { label, record, sqlVisible } of PARITY) { + it(`mirrors the SQL verdict for showInDiscovery=${label}`, () => { + expect(isHiddenFromDiscovery(record)).toBe(!sqlVisible); + }); + } + + it('tolerates a null/non-object record without throwing', () => { + expect(isHiddenFromDiscovery(null as unknown as Record)).toBe(false); + expect(isHiddenFromDiscovery({ preferences: null } as unknown as Record)).toBe( + false + ); + }); +}); + +describe('discoverableSql', () => { + it('emits the != 0 predicate over the given record column', () => { + const sql = discoverableSql('r.record'); + expect(sql).toContain("json_extract(r.record, '$.preferences.showInDiscovery')"); + expect(sql).toContain('IS NULL'); + expect(sql).toContain('!= 0'); + }); + + it('parameterizes the column so different table aliases reuse one rule', () => { + expect(discoverableSql('x.record')).toContain('json_extract(x.record'); + }); +}); diff --git a/apps/web/src/lib/search/server/discoverability.ts b/apps/web/src/lib/search/server/discoverability.ts new file mode 100644 index 0000000..6a9cf66 --- /dev/null +++ b/apps/web/src/lib/search/server/discoverability.ts @@ -0,0 +1,35 @@ +// Single source of truth for "is this event discoverable?". Used by BOTH the SQL +// read paths (contrail.config.ts pipelineQueries and the external geocode +// worklist) AND the in-memory sink filter (meili-sink.ts). The two MUST agree on +// semantics or the search index and the hydrated read path drift: an event the +// sink indexes but D1 hides (or the reverse) becomes a phantom, present in Meili +// yet dropped at hydration, or missing from search yet listed by D1. +// +// The rule: preferences.showInDiscovery hides the event when it is FALSEY in the +// JSON sense (boolean false OR numeric 0, since SQLite stores JSON false as 0). A +// missing/null/true value, any nonzero number, or any string stays discoverable, +// so pre-existing records without `preferences` are included by default. The SQL +// form uses `!= 0`; the JS form mirrors it with `=== false || === 0` (JS treats +// 0.0 as 0, matching SQLite). Parity across the full value surface is locked by +// discoverability.test.ts against real-SQLite ground truth. + +const PREF_PATH = '$.preferences.showInDiscovery'; + +/** SQL predicate (true = discoverable) over a record column expression, e.g. + * `discoverableSql('r.record')`. The one definition both the contrail + * pipelineQueries and the geocode worklist import, so the two SQL sites cannot + * drift from each other or from the JS mirror below. */ +export function discoverableSql(recordCol: string): string { + return `(json_extract(${recordCol}, '${PREF_PATH}') IS NULL + OR json_extract(${recordCol}, '${PREF_PATH}') != 0)`; +} + +/** In-memory mirror of discoverableSql for the sink: true when the author hid the + * event from discovery. Hides on a falsey showInDiscovery (boolean false OR + * numeric 0, including 0.0) to match SQLite's `!= 0`; missing/null/true/1/strings + * stay discoverable. Tolerates a null or non-object record/preferences. */ +export function isHiddenFromDiscovery(record: Record): boolean { + const prefs = record?.preferences as { showInDiscovery?: unknown } | null | undefined; + const v = prefs?.showInDiscovery; + return v === false || v === 0; +} diff --git a/apps/web/src/lib/search/server/geocode-cache.test.ts b/apps/web/src/lib/search/server/geocode-cache.test.ts new file mode 100644 index 0000000..feaf271 --- /dev/null +++ b/apps/web/src/lib/search/server/geocode-cache.test.ts @@ -0,0 +1,128 @@ +import { describe, it, expect } from 'vitest'; +import { + backoffMs, + isEligible, + groupEventsByNorm, + addressNeedingGeocode, + MAX_FAIL +} from './geocode-cache'; +import type { GeocodeCacheRow } from './geocode-cache'; +import { ADDRESS_TYPE } from './address-norm'; + +const DAY = 86_400_000; +const NOW = 1_000 * DAY; + +function negative(failCount: number, ageMs: number): GeocodeCacheRow { + return { + address_norm: 'x', + lat: null, + lng: null, + precision: null, + source: 'locationiq', + geocoded_at: NOW - ageMs, + fail_count: failCount, + last_error: 'not found' + }; +} + +describe('backoffMs', () => { + it('escalates 1d / 7d / 30d', () => { + expect(backoffMs(1)).toBe(DAY); + expect(backoffMs(2)).toBe(7 * DAY); + expect(backoffMs(3)).toBe(30 * DAY); + }); +}); + +describe('isEligible', () => { + it('treats an absent row as work', () => { + expect(isEligible(undefined, NOW)).toBe(true); + }); + + it('never re-geocodes a resolved row', () => { + const resolved: GeocodeCacheRow = { + address_norm: 'x', + lat: 50, + lng: 4, + precision: 'locality', + source: 'locationiq', + geocoded_at: NOW - 999 * DAY, + fail_count: 0, + last_error: null + }; + expect(isEligible(resolved, NOW)).toBe(false); + }); + + it('retries a negative row only after its backoff elapses', () => { + expect(isEligible(negative(1, 0.5 * DAY), NOW)).toBe(false); // < 1d + expect(isEligible(negative(1, 1.5 * DAY), NOW)).toBe(true); // >= 1d + expect(isEligible(negative(2, 3 * DAY), NOW)).toBe(false); // < 7d + expect(isEligible(negative(2, 8 * DAY), NOW)).toBe(true); // >= 7d + }); + + it('hard-stops at fail_count >= MAX_FAIL', () => { + expect(isEligible(negative(MAX_FAIL, 999 * DAY), NOW)).toBe(false); + }); + + it('--retry-negative ignores backoff and the hard stop', () => { + expect(isEligible(negative(MAX_FAIL, 0), NOW, true)).toBe(true); + }); +}); + +describe('addressNeedingGeocode', () => { + const addr = { $type: ADDRESS_TYPE, locality: 'Dayton', country: 'US' }; + const geo = (lat: string, lng: string) => ({ + $type: 'community.lexicon.location.geo', + latitude: lat, + longitude: lng + }); + const fsq = (lat: string, lng: string) => ({ + $type: 'community.lexicon.location.fsq', + latitude: lat, + longitude: lng + }); + + it('returns the address location for an address-only event', () => { + expect(addressNeedingGeocode({ locations: [addr] })).toMatchObject({ + locality: 'Dayton', + country: 'US' + }); + }); + + it('returns null when a geo location already resolves to coordinates (no overwrite)', () => { + expect(addressNeedingGeocode({ locations: [geo('40', '-105'), addr] })).toBeNull(); + }); + + it('returns null when an fsq location already resolves to coordinates (F1a)', () => { + expect(addressNeedingGeocode({ locations: [fsq('50.8', '4.3'), addr] })).toBeNull(); + }); + + it('returns the address location when the only coordinate location is out of range (F1b)', () => { + expect(addressNeedingGeocode({ locations: [geo('999', '0'), addr] })).toMatchObject({ + locality: 'Dayton' + }); + }); + + it('returns null when there is no address location to geocode', () => { + expect(addressNeedingGeocode({ locations: [geo('40', '-105')] })).toBeNull(); + expect(addressNeedingGeocode({ locations: [] })).toBeNull(); + }); +}); + +describe('groupEventsByNorm', () => { + it('buckets events by normalized address and skips unkeyable ones', () => { + const ev = (rkey: string, loc: Record) => ({ + uri: `at://did:plc:a/community.lexicon.calendar.event/${rkey}`, + did: 'did:plc:a', + rkey, + loc: { $type: ADDRESS_TYPE, ...loc } + }); + const map = groupEventsByNorm([ + ev('1', { locality: 'Dayton', country: 'US' }), + ev('2', { locality: 'Dayton', country: 'US' }), + ev('3', { locality: 'Berlin', country: 'DE' }), + ev('4', {}) // unkeyable -> skipped + ]); + expect([...map.keys()].sort()).toEqual(['berlin|de', 'dayton|us']); + expect(map.get('dayton|us')!.map((e: { rkey: string }) => e.rkey)).toEqual(['1', '2']); + }); +}); diff --git a/apps/web/src/lib/search/server/geocode-cache.ts b/apps/web/src/lib/search/server/geocode-cache.ts new file mode 100644 index 0000000..c883fe1 --- /dev/null +++ b/apps/web/src/lib/search/server/geocode-cache.ts @@ -0,0 +1,80 @@ +// Pure decision logic for the geocode_cache worklist: the row shape, the +// negative-cache retry/backoff policy, and grouping worklist events by their +// normalized address. No I/O — the job feeds rows in and acts on the verdicts. +import { normalizeAddress, addressLocation } from './address-norm'; +import { recordGeo } from './normalize'; + +export interface GeocodeCacheRow { + address_norm: string; + lat: number | null; + lng: number | null; + precision: string | null; + source: string | null; + geocoded_at: number; // epoch ms + fail_count: number; + last_error: string | null; +} + +export interface WorklistEvent { + uri: string; + did: string; + rkey: string; + loc: Record; +} + +const DAY = 86_400_000; +/** No more automatic attempts once fail_count reaches this (~4 tries / ~5 wks). */ +export const MAX_FAIL = 4; + +/** No-match backoff: eligible again after 1d (fail 1), 7d (2), 30d (3+). */ +export function backoffMs(failCount: number): number { + if (failCount <= 1) return DAY; + if (failCount === 2) return 7 * DAY; + return 30 * DAY; +} + +/** Is this address work this run? Absent → yes. Resolved → no. Negative → + * retryable iff fail_count < MAX_FAIL and the backoff elapsed (or forced). */ +export function isEligible( + row: GeocodeCacheRow | undefined, + now: number, + retryNegative = false +): boolean { + if (!row) return true; + if (row.lat !== null && row.lng !== null) return false; // resolved, done + if (retryNegative) return true; + if (row.fail_count >= MAX_FAIL) return false; + return now - row.geocoded_at >= backoffMs(row.fail_count); +} + +/** The address location to geocode for a record, or null when there's nothing + * to do. Returns null if the record ALREADY resolves to coordinates the index + * derives (recordGeo: geo/fsq/hthree, in-range) — geocoding it would overwrite + * precise coords with approximate ones — and null if it carries no address at + * all. So an event with an address plus an out-of-range geo/hthree location + * (recordGeo undefined) correctly still gets its address geocoded. This is the + * in-memory worklist filter that keeps "needs geocoding" aligned with the + * sink's _geo derivation, since the SQL worklist can only coarsely pre-filter. */ +export function addressNeedingGeocode( + record: Record +): Record | null { + if (recordGeo(record)) return null; + return addressLocation(record); +} + +/** Bucket worklist events by their normalized address; events that don't + * normalize to a key (e.g. empty address) are dropped. */ +export function groupEventsByNorm(events: WorklistEvent[]): Map { + const map = new Map(); + for (const e of events) { + const norm = normalizeAddress(e.loc); + if (!norm) continue; + let arr = map.get(norm); + if (!arr) { + arr = []; + map.set(norm, arr); + } + arr.push(e); + } + return map; +} diff --git a/apps/web/src/lib/search/server/geocoder.test.ts b/apps/web/src/lib/search/server/geocoder.test.ts new file mode 100644 index 0000000..c11ba58 --- /dev/null +++ b/apps/web/src/lib/search/server/geocoder.test.ts @@ -0,0 +1,140 @@ +// apps/web/src/lib/search/server/geocoder.test.ts +import { describe, it, expect, vi } from 'vitest'; +import { + addressToQuery, + derivePrecision, + createGeocoder, + requireGeocoderForBulk +} from './geocoder'; + +describe('addressToQuery', () => { + it('joins present fields in fixed order with commas, preserving original case', () => { + expect( + addressToQuery({ + country: 'België / Belgique / Belgien', + street: 'Cantersteen 41', + locality: 'Bruxelles - Brussel', + region: 'Brussel-Hoofdstad' + }) + ).toBe('Cantersteen 41, Bruxelles - Brussel, Brussel-Hoofdstad, België / Belgique / Belgien'); + }); + + it('skips absent/blank fields', () => { + expect(addressToQuery({ locality: 'Dayton', region: ' ', country: 'US' })).toBe('Dayton, US'); + }); +}); + +describe('derivePrecision', () => { + it('classifies a house/building hit as rooftop', () => { + expect(derivePrecision({ addresstype: 'house', place_rank: 30 })).toBe('rooftop'); + }); + it('classifies a road hit as street', () => { + expect(derivePrecision({ type: 'road', class: 'highway' })).toBe('street'); + }); + it('classifies a city/region hit as locality', () => { + expect(derivePrecision({ addresstype: 'city' })).toBe('locality'); + expect(derivePrecision({ type: 'administrative', class: 'boundary' })).toBe('locality'); + }); + it('falls back to the raw type when unclassifiable', () => { + expect(derivePrecision({ type: 'attraction' })).toBe('attraction'); + expect(derivePrecision({})).toBe('unknown'); + }); +}); + +describe('createGeocoder', () => { + function fakeFetch(body: unknown, status = 200) { + const calls: { url: string; headers: Record }[] = []; + const fn = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + calls.push({ url: String(input), headers: (init?.headers ?? {}) as Record }); + return new Response(JSON.stringify(body), { status }); + }); + return { fn: fn as unknown as typeof fetch, calls }; + } + + it('defaults to public Nominatim with the atmo user-agent', async () => { + const { fn, calls } = fakeFetch([{ lat: '50.84', lon: '4.36', addresstype: 'city' }]); + const geo = createGeocoder({}, fn); + const point = await geo.geocode('Bruxelles, BE'); + expect(point).toEqual({ lat: 50.84, lng: 4.36, precision: 'locality' }); + expect(calls[0].url).toContain('https://nominatim.openstreetmap.org/search'); + expect(calls[0].url).toContain('q=Bruxelles%2C+BE'); + expect(calls[0].headers['user-agent']).toContain('atmo-events'); + }); + + it('switches to a keyed LocationIQ endpoint when GEOCODER_KEY+URL are set', async () => { + const { fn, calls } = fakeFetch([{ lat: '52.5', lon: '13.4', addresstype: 'road' }]); + const geo = createGeocoder( + { GEOCODER_URL: 'https://us1.locationiq.com/v1/search', GEOCODER_KEY: 'tok' }, + fn + ); + await geo.geocode('Berlin'); + expect(calls[0].url).toContain('https://us1.locationiq.com/v1/search'); + expect(calls[0].url).toContain('key=tok'); + }); + + it('returns null on an empty result set (no-match)', async () => { + const { fn } = fakeFetch([]); + expect(await createGeocoder({}, fn).geocode('nowhere')).toBeNull(); + }); + + it('returns null on a 404 (LocationIQ no-match) so it negative-caches, not retries', async () => { + const { fn } = fakeFetch({ error: 'Unable to geocode' }, 404); + expect(await createGeocoder({}, fn).geocode('nowhere')).toBeNull(); + }); + + it('throws on a transient HTTP error (429/5xx) so the caller retries', async () => { + const { fn } = fakeFetch({}, 429); + await expect(createGeocoder({}, fn).geocode('x')).rejects.toThrow(/429/); + }); +}); + +describe('requireGeocoderForBulk', () => { + it('allows any run when a geocoder key is set', () => { + expect(() => + requireGeocoderForBulk({ hasKey: true, dryRun: false, limit: 0, allowPublic: false }) + ).not.toThrow(); + expect(() => + requireGeocoderForBulk({ hasKey: true, dryRun: false, limit: 5000, allowPublic: false }) + ).not.toThrow(); + }); + + it('allows a keyless small drip (1..25)', () => { + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 25, allowPublic: false }) + ).not.toThrow(); + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 1, allowPublic: false }) + ).not.toThrow(); + }); + + it('blocks a keyless run over the drip ceiling', () => { + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 26, allowPublic: false }) + ).toThrow(/LocationIQ|allow-public-nominatim/); + }); + + it('blocks a keyless uncapped run (limit 0 = no cap, the worst case)', () => { + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 0, allowPublic: false }) + ).toThrow(/LocationIQ|allow-public-nominatim/); + }); + + it('blocks a keyless negative limit (uncapped, not a tiny drip)', () => { + // A stray `--limit -1` must not masquerade as a 1-call drip and slip the gate. + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: -1, allowPublic: false }) + ).toThrow(/LocationIQ|allow-public-nominatim/); + }); + + it('lets --allow-public-nominatim override a bulk keyless run', () => { + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 0, allowPublic: true }) + ).not.toThrow(); + }); + + it('never blocks a dry run (it makes no geocoder calls)', () => { + expect(() => + requireGeocoderForBulk({ hasKey: false, dryRun: true, limit: 0, allowPublic: false }) + ).not.toThrow(); + }); +}); diff --git a/apps/web/src/lib/search/server/geocoder.ts b/apps/web/src/lib/search/server/geocoder.ts new file mode 100644 index 0000000..c752e2b --- /dev/null +++ b/apps/web/src/lib/search/server/geocoder.ts @@ -0,0 +1,144 @@ +// apps/web/src/lib/search/server/geocoder.ts +// One Nominatim-compatible geocoding client, config-selected. LocationIQ is +// API-compatible with Nominatim (same /search?q=&format=&limit= request, same +// lat/lon/type/class/display_name response — it IS hosted Nominatim), so this +// is one implementation parameterized by endpoint + optional key, not two +// behind an interface. Default = public Nominatim (parity with atmo today); +// set GEOCODER_KEY (+ GEOCODER_URL) to use LocationIQ. Used only by the +// external geocode job — geocoding never runs on the Worker hot path. + +export interface GeoPoint { + lat: number; + lng: number; + /** Provider-reported granularity tier (rooftop/street/locality/...). */ + precision?: string; +} + +export interface Geocoder { + geocode(q: string): Promise; +} + +export interface GeocoderEnv { + GEOCODER_URL?: string; + GEOCODER_KEY?: string; + GEOCODER_USER_AGENT?: string; +} + +const DEFAULT_URL = 'https://nominatim.openstreetmap.org/search'; +const DEFAULT_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; + +// Same fixed order as the cache key, but a human-readable freeform query (commas, +// original case/diacritics) — both Nominatim and LocationIQ handle UTF-8 / non- +// Latin scripts. Freeform tolerates the messy data (city-in-name, JP street-only, +// trailing punctuation) a structured per-field query would drop. +const QUERY_FIELDS = ['name', 'street', 'locality', 'region', 'postalCode', 'country']; + +/** Largest keyless run we treat as a sanctioned "small drip" against public + * Nominatim. Above this (or uncapped), a backfill must use LocationIQ. */ +export const PUBLIC_NOMINATIM_DRIP_MAX = 25; + +/** Enforce the file's contract that a BULK backfill uses LocationIQ, not public + * Nominatim (whose usage policy a large unkeyed run would breach, risking a + * silent IP ban). Throws for a keyless, non-dry-run, non-overridden run that is + * uncapped (limit <= 0) or over the drip ceiling; a small keyless drip stays + * allowed. The hazard is request VOLUME, so the gate is on size, not on the + * mere use of Nominatim. limit <= 0 (not just === 0) counts as uncapped so a + * stray negative can't masquerade as a tiny drip and slip the gate. */ +export function requireGeocoderForBulk(opts: { + hasKey: boolean; + dryRun: boolean; + limit: number; + allowPublic: boolean; +}): void { + if (opts.hasKey || opts.dryRun || opts.allowPublic) return; + const bulk = opts.limit <= 0 || opts.limit > PUBLIC_NOMINATIM_DRIP_MAX; + if (bulk) { + throw new Error( + `Keyless public Nominatim is only allowed for a small drip (--limit 1..${PUBLIC_NOMINATIM_DRIP_MAX}). ` + + 'Set GEOCODER_KEY (LocationIQ) for a bulk/uncapped backfill, or pass --allow-public-nominatim to override.' + ); + } +} + +export function addressToQuery(loc: Record): string { + return QUERY_FIELDS.map((f) => (typeof loc[f] === 'string' ? (loc[f] as string).trim() : '')) + .filter((v) => v !== '') + .join(', '); +} + +type NominatimHit = { + lat?: string; + lon?: string; + display_name?: string; + type?: string; + class?: string; + place_rank?: number; + addresstype?: string; +}; + +const ROOFTOP = new Set(['house', 'building', 'address', 'house_number']); +const STREET = new Set(['road', 'street', 'residential', 'pedestrian']); +const LOCALITY = new Set([ + 'city', + 'town', + 'village', + 'hamlet', + 'suburb', + 'locality', + 'municipality', + 'administrative', + 'state', + 'region', + 'province', + 'country' +]); + +export function derivePrecision(top: { + type?: string; + class?: string; + place_rank?: number; + addresstype?: string; +}): string { + const t = top.addresstype || top.type || ''; + if (ROOFTOP.has(t) || (top.place_rank ?? 0) >= 30) return 'rooftop'; + if (STREET.has(t) || top.class === 'highway') return 'street'; + if (LOCALITY.has(t) || top.class === 'boundary' || top.class === 'place') return 'locality'; + return t || 'unknown'; +} + +export function createGeocoder(env: GeocoderEnv = {}, fetchImpl: typeof fetch = fetch): Geocoder { + const base = env.GEOCODER_URL || DEFAULT_URL; + const key = env.GEOCODER_KEY; + const userAgent = env.GEOCODER_USER_AGENT || DEFAULT_USER_AGENT; + + return { + async geocode(q: string): Promise { + const url = new URL(base); + url.searchParams.set('q', q); + url.searchParams.set('format', 'json'); + url.searchParams.set('limit', '1'); + if (key) url.searchParams.set('key', key); + + const res = await fetchImpl(url, { + headers: { accept: 'application/json', 'user-agent': userAgent } + }); + // NO-MATCH vs TRANSIENT. Nominatim signals no-match as 200 + []; LocationIQ + // signals it as 404 (e.g. {"error":"Unable to geocode"}). Treat 404 as a + // no-match (return null → negative cache w/ backoff) so an ungeocodable + // address isn't retried every run. Other non-2xx (429 rate-limit, 5xx, + // transport) throw → the job treats them as TRANSIENT and retries next run. + if (res.status === 404) return null; + if (!res.ok) throw new Error(`geocode request failed: ${res.status}`); + + const results = (await res.json()) as NominatimHit[]; + const top = Array.isArray(results) ? results[0] : undefined; + if (!top) return null; + + const lat = Number(top.lat); + const lng = Number(top.lon); + if (!Number.isFinite(lat) || !Number.isFinite(lng)) return null; + + return { lat, lng, precision: derivePrecision(top) }; + } + }; +} diff --git a/apps/web/src/lib/search/server/meili-sink.integration.test.ts b/apps/web/src/lib/search/server/meili-sink.integration.test.ts index 791ce20..97723d6 100644 --- a/apps/web/src/lib/search/server/meili-sink.integration.test.ts +++ b/apps/web/src/lib/search/server/meili-sink.integration.test.ts @@ -8,8 +8,14 @@ // MEILI_TEST_URL=http://localhost:7700 MEILI_TEST_KEY=masterKey \ // pnpm vitest run src/lib/search/server/meili-sink.integration.test.ts import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { createMeiliSink, applyMeiliSettings, type MeiliSinkBackend } from './meili-sink'; +import { + createMeiliSink, + applyMeiliSettings, + MeiliEventIndex, + type MeiliSinkBackend +} from './meili-sink'; import { searchEvents, nearMeEvents, type SearchBackend } from './meili'; +import { eventToSearchDoc } from './normalize'; const URL = process.env.MEILI_TEST_URL; const KEY = process.env.MEILI_TEST_KEY ?? 'masterKey'; @@ -101,6 +107,55 @@ run('MeiliSink ↔ read client, live against real Meilisearch', () => { expect(hit?.distanceMeters).toBeGreaterThanOrEqual(0); }); + it('the geocode job upsert attaches _geo to an address-only event, keeping its other fields', async () => { + const addrUri = 'at://did:plc:alice/community.lexicon.calendar.event/addr-only'; + const addrRecord = { + name: 'Antwerp Atproto Drinks', + description: 'address-only event', + startsAt: FUTURE, + locations: [ + { $type: 'community.lexicon.location.address', locality: 'Antwerp', country: 'BE' } + ] + }; + // An address-only event: indexed by the sink, but with no _geo yet. + await sink.onRecords([created(addrUri, addrRecord)], { phase: 'live' }); + await eventually( + () => searchEvents(readBackend, { q: 'Antwerp Atproto', limit: 10, offset: 0 }), + (r) => r.hits.some((h) => h.uri === addrUri) + ); + + // Attach coordinates the way the external geocode job does: rebuild the FULL + // doc (eventToSearchDoc) and upsert it with _geo — idempotent, stub-free, and + // identical to the doc the sink writes. + const doc = eventToSearchDoc({ + uri: addrUri, + did: 'did:plc:alice', + collection: EVENT, + rkey: addrUri.split('/').pop()!, + record: addrRecord + }); + doc._geo = { lat: 51.2194, lng: 4.4025 }; + await new MeiliEventIndex(backend).upsert([doc]); + + // near-me finds it (so _geo landed) AND text still matches (so name/startsAt + // survived) — both read-path filters pass against the full doc. + const near = await eventually( + () => + nearMeEvents(readBackend, { + lat: 51.2194, + lng: 4.4025, + radiusMeters: 5000, + limit: 10, + offset: 0 + }), + (r) => r.hits.some((h) => h.uri === addrUri) + ); + expect(near.hits.map((h) => h.uri)).toContain(addrUri); + + const text = await searchEvents(readBackend, { q: 'Antwerp Atproto', limit: 10, offset: 0 }); + expect(text.hits.map((h) => h.uri)).toContain(addrUri); // name survived the merge + }); + it('removes a deleted event so the read path no longer finds it', async () => { await sink.onRecords( [{ kind: 'deleted', uri, did: 'did:plc:alice', collection: EVENT, rkey: 'round-trip' }], diff --git a/apps/web/src/lib/search/server/meili-sink.test.ts b/apps/web/src/lib/search/server/meili-sink.test.ts index aa19357..32d1d00 100644 --- a/apps/web/src/lib/search/server/meili-sink.test.ts +++ b/apps/web/src/lib/search/server/meili-sink.test.ts @@ -3,9 +3,11 @@ import { createMeiliSink, meiliSinkBackendFromEnv, applyMeiliSettings, + MeiliEventIndex, type MeiliSinkBackend } from './meili-sink'; import { searchDocId } from './normalize'; +import { normalizeAddress, ADDRESS_TYPE } from './address-norm'; const BACKEND: MeiliSinkBackend = { url: 'http://meili.local', apiKey: 'admin-key' }; @@ -66,7 +68,11 @@ describe('meiliSinkBackendFromEnv', () => { describe('createMeiliSink onRecords', () => { it('upserts a created event as a normalized doc (PUT documents)', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); await sink.onRecords( [ @@ -102,7 +108,11 @@ describe('createMeiliSink onRecords', () => { it('removes a deleted event by derived id (delete-batch)', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); const uri = 'at://did:plc:alice/community.lexicon.calendar.event/2'; await sink.onRecords( @@ -118,7 +128,11 @@ describe('createMeiliSink onRecords', () => { it('removes (does not index) a created event hidden from discovery', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); const uri = 'at://did:plc:alice/community.lexicon.calendar.event/hidden'; await sink.onRecords( @@ -133,9 +147,34 @@ describe('createMeiliSink onRecords', () => { expect(del!.body).toEqual([searchDocId(uri)]); }); + it('removes a created event whose showInDiscovery is a numeric 0 (matches the SQL != 0 filter)', async () => { + const { fn, calls } = fakeFetch(); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); + const uri = 'at://did:plc:alice/community.lexicon.calendar.event/zero'; + + await sink.onRecords( + [created(uri, { name: 'SerializedFalse', preferences: { showInDiscovery: 0 } })], + { phase: 'live' } + ); + + // Some serializers emit booleans as 0/1; the D1 worklist/read filter uses + // `!= 0`, so the sink must hide 0 too or it would index an event D1 drops. + expect(calls.find((c) => c.method === 'PUT')).toBeUndefined(); + const del = calls.find((c) => c.url.endsWith('/documents/delete-batch')); + expect(del!.body).toEqual([searchDocId(uri)]); + }); + it('indexes a created event when showInDiscovery is missing or true', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); await sink.onRecords( [ @@ -155,7 +194,11 @@ describe('createMeiliSink onRecords', () => { it('ignores records from other collections', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); await sink.onRecords( [ @@ -178,7 +221,11 @@ describe('createMeiliSink onRecords', () => { it('no-ops (no fetch) when the backend is unconfigured', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => null, fn); + const sink = createMeiliSink( + () => null, + () => null, + fn + ); await sink.onRecords([created('at://did:plc:alice/community.lexicon.calendar.event/3', {})], { phase: 'backfill' @@ -189,7 +236,7 @@ describe('createMeiliSink onRecords', () => { it('applies index settings once, before the first write (fresh-index safety)', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, fn); + const sink = createMeiliSink(() => BACKEND, () => null, fn); // Two batches on the same sink: a fresh-rollout `pnpm backfill` must not // let PUT /documents auto-create a bare index whose _geo/startsAt searches @@ -231,7 +278,11 @@ describe('fetch is invoked detached (workerd Illegal invocation guard)', () => { }); it('onRecords upsert/delete do not trip Illegal invocation', async () => { - const sink = createMeiliSink(() => BACKEND, strictFetch()); + const sink = createMeiliSink( + () => BACKEND, + () => null, + strictFetch() + ); await expect( sink.onRecords( [created('at://did:plc:alice/community.lexicon.calendar.event/4', { name: 'x' })], @@ -240,3 +291,144 @@ describe('fetch is invoked detached (workerd Illegal invocation guard)', () => { ).resolves.toBeUndefined(); }); }); + +/** A minimal D1 double: prepare().bind().all() returns the seeded resolved + * rows whose address_norm is in the bound args. Throw mode exercises the + * best-effort swallow. */ +function fakeDb( + rows: { address_norm: string; lat: number; lng: number }[], + mode: 'ok' | 'throw' = 'ok' +) { + return { + prepare() { + return { + bind(...args: unknown[]) { + return { + async all() { + if (mode === 'throw') throw new Error('no such table: geocode_cache'); + return { + results: rows.filter((r) => args.includes(r.address_norm)) as unknown as T[] + }; + } + }; + } + }; + } + } as unknown as D1Database; +} + +const ADDR_RECORD = { + name: 'TGIF meetup', + locations: [{ $type: ADDRESS_TYPE, name: 'TGIF meetup', locality: 'Bruxelles', country: 'BE' }] +}; + +describe('createMeiliSink geocode cache lookup', () => { + it('fills _geo from a resolved cache row for an address-only event', async () => { + const norm = normalizeAddress({ name: 'TGIF meetup', locality: 'Bruxelles', country: 'BE' })!; + const { fn, calls } = fakeFetch(); + const sink = createMeiliSink( + () => BACKEND, + () => fakeDb([{ address_norm: norm, lat: 50.84, lng: 4.36 }]), + fn + ); + + await sink.onRecords( + [created('at://did:plc:alice/community.lexicon.calendar.event/addr', ADDR_RECORD)], + { phase: 'live' } + ); + + const put = calls.find((c) => c.method === 'PUT'); + const docs = put!.body as Array>; + expect(docs[0]._geo).toEqual({ lat: 50.84, lng: 4.36 }); + }); + + it('leaves _geo unset on a cache miss but still indexes the doc', async () => { + const { fn, calls } = fakeFetch(); + const sink = createMeiliSink( + () => BACKEND, + () => fakeDb([]), + fn + ); + await sink.onRecords( + [created('at://did:plc:alice/community.lexicon.calendar.event/miss', ADDR_RECORD)], + { phase: 'live' } + ); + const docs = calls.find((c) => c.method === 'PUT')!.body as Array>; + expect(docs[0]._geo).toBeUndefined(); + expect(docs[0].name).toBe('TGIF meetup'); + }); + + it('does not consult the cache when the event already has coordinate _geo', async () => { + const norm = normalizeAddress({ locality: 'Bruxelles', country: 'BE' })!; + const { fn, calls } = fakeFetch(); + const sink = createMeiliSink( + () => BACKEND, + () => fakeDb([{ address_norm: norm, lat: 1, lng: 1 }]), + fn + ); + await sink.onRecords( + [ + created('at://did:plc:alice/community.lexicon.calendar.event/geo', { + locations: [ + { $type: 'community.lexicon.location.geo', latitude: '40.0', longitude: '-105.0' }, + { $type: ADDRESS_TYPE, locality: 'Bruxelles', country: 'BE' } + ] + }) + ], + { phase: 'live' } + ); + const docs = calls.find((c) => c.method === 'PUT')!.body as Array>; + expect(docs[0]._geo).toEqual({ lat: 40, lng: -105 }); // from .geo, not the cache row + }); + + it('swallows a cache query error and indexes without _geo', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { fn, calls } = fakeFetch(); + const sink = createMeiliSink( + () => BACKEND, + () => fakeDb([], 'throw'), + fn + ); + await expect( + sink.onRecords( + [created('at://did:plc:alice/community.lexicon.calendar.event/err', ADDR_RECORD)], + { phase: 'live' } + ) + ).resolves.toBeUndefined(); + const docs = calls.find((c) => c.method === 'PUT')!.body as Array>; + expect(docs[0]._geo).toBeUndefined(); + warn.mockRestore(); + }); +}); + +describe('MeiliEventIndex.upsert', () => { + it('PUTs a full doc with _geo (the write the geocode job makes to attach coords)', async () => { + const { fn, calls } = fakeFetch(); + const doc = { + id: 'abc', + uri: 'at://did:plc:alice/community.lexicon.calendar.event/addr', + did: 'did:plc:alice', + rkey: 'addr', + name: 'Antwerp Drinks', + startsAt: '2099-01-01T10:00:00Z', + locationTypes: ['community.lexicon.location.address'], + _geo: { lat: 51.2194, lng: 4.4025 } + }; + await new MeiliEventIndex(BACKEND, fn).upsert([doc]); + + // PUT /documents is "add or update" — it merges an existing doc and lands a + // COMPLETE doc when absent, so a full-doc payload never leaves a {id,_geo} stub. + const put = calls.find( + (c) => + c.url === 'http://meili.local/indexes/events/documents?primaryKey=id' && c.method === 'PUT' + ); + expect(put).toBeDefined(); + expect(put!.body).toEqual([doc]); + }); + + it('no-ops on an empty doc list', async () => { + const { fn, calls } = fakeFetch(); + await new MeiliEventIndex(BACKEND, fn).upsert([]); + expect(calls).toHaveLength(0); + }); +}); diff --git a/apps/web/src/lib/search/server/meili-sink.ts b/apps/web/src/lib/search/server/meili-sink.ts index 18c1f4a..ca2219b 100644 --- a/apps/web/src/lib/search/server/meili-sink.ts +++ b/apps/web/src/lib/search/server/meili-sink.ts @@ -14,6 +14,8 @@ // so a Meilisearch outage degrades to "search index falls behind", not // "ingest stops". import { eventToSearchDoc, searchDocId, type SearchDoc } from './normalize'; +import { addressLocation, normalizeAddress } from './address-norm'; +import { isHiddenFromDiscovery } from './discoverability'; import type { ContrailConfig } from '@atmo-dev/contrail'; // The umbrella re-exports ContrailConfig (which carries `sinks?: Sink[]`) but @@ -25,14 +27,6 @@ type RecordEvent = Parameters[0][number]; /** The one collection we index for search. */ export const EVENT_COLLECTION = 'community.lexicon.calendar.event'; -/** True when the event author hid it from discovery. Mirrors the D1 filter in - * contrail.config.ts: only `preferences.showInDiscovery === false` hides; - * a missing/null field stays discoverable. */ -function isHiddenFromDiscovery(record: Record): boolean { - const prefs = record?.preferences as { showInDiscovery?: boolean } | undefined; - return prefs?.showInDiscovery === false; -} - export interface MeiliSinkBackend { url: string; apiKey: string; @@ -147,6 +141,34 @@ export async function applyMeiliSettings( await new MeiliEventIndex(backend, fetchFn).applySettings(); } +/** Best-effort fill of doc._geo from the geocode_cache. Read-only; a missing + * table or D1 hiccup just means "no _geo this pass" (never an ingest failure). + * Its real job is to reproduce the _geo the external geocode job wrote, so a + * later live UPDATE (full-doc PUT) of an already-geocoded event doesn't drop it. */ +async function fillGeoFromCache( + db: D1Database | null, + pending: { doc: SearchDoc; norm: string }[] +): Promise { + if (!db || pending.length === 0) return; + try { + const norms = [...new Set(pending.map((p) => p.norm))]; + const placeholders = norms.map(() => '?').join(','); + const { results } = await db + .prepare( + `SELECT address_norm, lat, lng FROM geocode_cache WHERE lat IS NOT NULL AND address_norm IN (${placeholders})` + ) + .bind(...norms) + .all<{ address_norm: string; lat: number; lng: number }>(); + const byNorm = new Map(results.map((r) => [r.address_norm, r])); + for (const { doc, norm } of pending) { + const row = byNorm.get(norm); + if (row) doc._geo = { lat: row.lat, lng: row.lng }; + } + } catch (e) { + console.warn('[search-sink] geocode cache lookup failed; indexing without _geo:', e); + } +} + /** Builds the contrail Sink. The backend is resolved lazily per batch via * `getBackend` because a Cloudflare Worker only has env per invocation, while * `contrail` is constructed once at module load — the cron/xrpc handlers set @@ -154,6 +176,7 @@ export async function applyMeiliSettings( * the backend is null (unconfigured), onRecords is a no-op. */ export function createMeiliSink( getBackend: () => MeiliSinkBackend | null, + getDb: () => D1Database | null = () => null, fetchFn?: typeof fetch ): Sink { // Apply the read-path's filterable/sortable settings once, before the first @@ -169,28 +192,39 @@ export function createMeiliSink( const docs: SearchDoc[] = []; const deletes: string[] = []; + // Docs that have no coordinate _geo but do carry an address — candidates + // for a geocode_cache fill. + const pending: { doc: SearchDoc; norm: string }[] = []; for (const e of events) { if (e.collection !== EVENT_COLLECTION) continue; - // Mirror the D1 discoverable filter (contrail.config.ts): only - // `showInDiscovery === false` hides; missing/null stays discoverable. + // Discoverability is decided by the shared predicate (discoverability.ts) + // so this in-memory filter and the D1 SQL filter never diverge. // Index discoverable creates; for everything else — real deletes AND // events hidden from discovery — remove the doc so the search index // never holds a hidden event's name/description and a discoverable→ // unlisted flip purges the existing entry. if (e.kind === 'created' && !isHiddenFromDiscovery(e.record)) { - docs.push( - eventToSearchDoc({ - uri: e.uri, - did: e.did, - collection: e.collection, - rkey: e.rkey, - record: e.record - }) - ); + const doc = eventToSearchDoc({ + uri: e.uri, + did: e.did, + collection: e.collection, + rkey: e.rkey, + record: e.record + }); + docs.push(doc); + if (!doc._geo) { + const loc = addressLocation(e.record); + const norm = loc ? normalizeAddress(loc) : null; + if (norm) pending.push({ doc, norm }); + } } else { deletes.push(searchDocId(e.uri)); } } + + // Mutates doc._geo in place before the upsert below. + await fillGeoFromCache(getDb(), pending); + if (docs.length === 0 && deletes.length === 0) return; const index = new MeiliEventIndex(backend, fetchFn); diff --git a/apps/web/src/lib/search/server/normalize.test.ts b/apps/web/src/lib/search/server/normalize.test.ts index 82387ff..1926467 100644 --- a/apps/web/src/lib/search/server/normalize.test.ts +++ b/apps/web/src/lib/search/server/normalize.test.ts @@ -1,10 +1,16 @@ import { describe, it, expect } from 'vitest'; -import { eventToSearchDoc, type EventRecordPayload } from './normalize'; +import { eventToSearchDoc, recordGeo, type EventRecordPayload } from './normalize'; const URI = 'at://did:plc:alice/community.lexicon.calendar.event/1'; function payload(record: Record): EventRecordPayload { - return { uri: URI, did: 'did:plc:alice', collection: 'community.lexicon.calendar.event', rkey: '1', record }; + return { + uri: URI, + did: 'did:plc:alice', + collection: 'community.lexicon.calendar.event', + rkey: '1', + record + }; } function geoLoc(latitude: string, longitude: string) { @@ -45,3 +51,38 @@ describe('eventToSearchDoc geo derivation', () => { expect(doc._geo).toBeUndefined(); }); }); + +const FSQ = 'community.lexicon.location.fsq'; +const ADDRESS = 'community.lexicon.location.address'; + +describe('recordGeo', () => { + it('derives coordinates from a geo location', () => { + expect(recordGeo({ locations: [geoLoc('40.0', '-105.0')] })).toEqual({ lat: 40, lng: -105 }); + }); + + it('derives coordinates from an fsq location that carries lat/lng', () => { + expect( + recordGeo({ locations: [{ $type: FSQ, latitude: '50.84', longitude: '4.36' }] }) + ).toEqual({ lat: 50.84, lng: 4.36 }); + }); + + it('returns undefined when the only coordinate location is out of range', () => { + expect(recordGeo({ locations: [geoLoc('999', '-105.0')] })).toBeUndefined(); + }); + + it('returns undefined for an address-only record', () => { + expect( + recordGeo({ locations: [{ $type: ADDRESS, locality: 'Dayton', country: 'US' }] }) + ).toBeUndefined(); + }); + + it('matches the _geo eventToSearchDoc derives (single source of truth)', () => { + const record = { + locations: [ + { $type: FSQ, latitude: '50.84', longitude: '4.36' }, + { $type: ADDRESS, locality: 'x' } + ] + }; + expect(recordGeo(record)).toEqual(eventToSearchDoc(payload(record))._geo); + }); +}); diff --git a/apps/web/src/lib/search/server/normalize.ts b/apps/web/src/lib/search/server/normalize.ts index 1a2a65a..ab9e610 100644 --- a/apps/web/src/lib/search/server/normalize.ts +++ b/apps/web/src/lib/search/server/normalize.ts @@ -97,11 +97,29 @@ function str(v: unknown): string | undefined { return typeof v === 'string' ? v : undefined; } -export function eventToSearchDoc(payload: EventRecordPayload): SearchDoc { - const record = payload.record ?? {}; - const locations = (Array.isArray(record.locations) ? record.locations : []).filter( +/** The record's locations[] narrowed to the location objects deriveGeo and the + * doc builder read. */ +function recordLocations(record: Record): Loc[] { + return (Array.isArray(record?.locations) ? record.locations : []).filter( (l): l is Loc => !!l && typeof l === 'object' ); +} + +/** The single canonical _geo a record resolves to (precedence geo > fsq > + * hthree, in-range only), or undefined. Shared by the search doc AND the + * external geocode worklist so "already has coordinates" means the exact same + * thing in both: the worklist must not geocode an event the index already + * geo-derives (which would overwrite precise fsq/geo coords), and must still + * geocode one whose only coordinate location is out of range (no _geo). */ +export function recordGeo( + record: Record +): { lat: number; lng: number } | undefined { + return deriveGeo(recordLocations(record)); +} + +export function eventToSearchDoc(payload: EventRecordPayload): SearchDoc { + const record = payload.record ?? {}; + const locations = recordLocations(record); const doc: SearchDoc = { id: searchDocId(payload.uri), -- 2.51.2 From a4ffb1196de4a22a2da0e0ad424407eea29ea8c6 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Sat, 27 Jun 2026 15:55:15 -0400 Subject: [PATCH 2/4] =?UTF-8?q?feat(geocode):=20in-Worker=20address?= =?UTF-8?q?=E2=86=92=5Fgeo=20drip=20as=20a=20cron=20module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move geocoding into the Worker as a cron-driven drip: each run geocodes a bounded batch of address-only events and writes _geo back to Meili, giving steady-state near-me coverage without the external script. Adds config/db/process modules with tests and folds in two rounds of code-review fixes (om-16z0, om-xip5). --- apps/web/scripts/geocode-events.ts | 250 +++-------- apps/web/src/app.d.ts | 14 + apps/web/src/lib/geocode/config.ts | 20 + apps/web/src/lib/geocode/db.ts | 40 ++ apps/web/src/lib/geocode/process.test.ts | 160 +++++++ apps/web/src/lib/geocode/process.ts | 89 ++++ apps/web/src/lib/search/server/d1-native.ts | 16 + .../src/lib/search/server/geocode-job.test.ts | 420 ++++++++++++++++++ apps/web/src/lib/search/server/geocode-job.ts | 293 ++++++++++++ .../src/lib/search/server/geocoder.test.ts | 104 ++++- apps/web/src/lib/search/server/geocoder.ts | 63 ++- apps/web/src/lib/search/server/normalize.ts | 6 +- apps/web/src/routes/api/cron/+server.ts | 11 + 13 files changed, 1270 insertions(+), 216 deletions(-) create mode 100644 apps/web/src/lib/geocode/config.ts create mode 100644 apps/web/src/lib/geocode/db.ts create mode 100644 apps/web/src/lib/geocode/process.test.ts create mode 100644 apps/web/src/lib/geocode/process.ts create mode 100644 apps/web/src/lib/search/server/d1-native.ts create mode 100644 apps/web/src/lib/search/server/geocode-job.test.ts create mode 100644 apps/web/src/lib/search/server/geocode-job.ts diff --git a/apps/web/scripts/geocode-events.ts b/apps/web/scripts/geocode-events.ts index b3413f2..cb74fe8 100644 --- a/apps/web/scripts/geocode-events.ts +++ b/apps/web/scripts/geocode-events.ts @@ -1,31 +1,24 @@ // apps/web/scripts/geocode-events.ts -// External geocode job (runs OFF Cloudflare): finds address-only events in the -// openmeet-atmo D1 that lack coordinates, geocodes each unique address through -// the config-selected Nominatim/LocationIQ client, writes geocode_cache, and -// _geo-updates the affected Meili docs. ONE job: "find work" and "geocode" are -// sequential steps sharing the cache, not two passes. Rate limiting is the -// sleep between calls — the whole reason geocoding stays off Workers. +// CLI wrapper around the shared geocode core (lib/search/server/geocode-job.ts): +// runs it OFF Cloudflare against the openmeet-atmo D1 over the REST client, for +// the one-time bulk backfill and ad-hoc manual drips. The in-Worker cron drip +// (lib/geocode/process.ts) calls the same core over the native D1 binding. This +// wrapper owns the CLI surface: env/flag parsing, the public-Nominatim bulk +// guard, and client construction. // // Run (backfill MUST point the geocoder at LocationIQ, never public Nominatim): // CLOUDFLARE_API_TOKEN=… MEILI_URL=https://search.testnet.openmeet.net MEILI_KEY=… \ // GEOCODER_URL=https://us1.locationiq.com/v1/search GEOCODER_KEY=… \ // pnpm -C apps/web exec tsx scripts/geocode-events.ts --limit 50 -import { createD1Client, type D1Client } from '../src/lib/search/server/d1-http'; +import { createD1Client } from '../src/lib/search/server/d1-http'; import { createGeocoder, - addressToQuery, - requireGeocoderForBulk + requireGeocoderForBulk, + isPublicNominatimHost } from '../src/lib/search/server/geocoder'; -import { - isEligible, - groupEventsByNorm, - addressNeedingGeocode, - type GeocodeCacheRow, - type WorklistEvent -} from '../src/lib/search/server/geocode-cache'; -import { eventToSearchDoc } from '../src/lib/search/server/normalize'; -import { discoverableSql } from '../src/lib/search/server/discoverability'; -import { MeiliEventIndex, EVENT_COLLECTION } from '../src/lib/search/server/meili-sink'; +import { MeiliEventIndex } from '../src/lib/search/server/meili-sink'; +import { runGeocodeJob } from '../src/lib/search/server/geocode-job'; +import { DEFAULT_GEOCODE_SLEEP_MS } from '../src/lib/geocode/config'; const env = process.env; const argv = process.argv.slice(2); @@ -49,56 +42,33 @@ const parseLimit = (raw: string | undefined): number => { return n; }; +// Throttle between geocoder calls (ms). Unset → the safe default; set → must be a +// finite, non-negative number. Fail loud on a malformed value here (a human typed +// it): an unvalidated Number() would yield NaN → sleep(NaN) ≈ 0ms and silently +// hammer the provider on a bulk backfill. (The core also clamps defensively.) +const parseSleepMs = (raw: string | undefined): number => { + if (raw === undefined) return DEFAULT_GEOCODE_SLEEP_MS; + const n = Number(raw); + if (!Number.isFinite(n) || n < 0) { + throw new Error( + `GEOCODE_SLEEP_MS must be a non-negative number of milliseconds; got ${JSON.stringify(raw)}` + ); + } + return n; +}; + const retryNegative = flag('--retry-negative'); const dryRun = flag('--dry-run'); const allowPublicNominatim = flag('--allow-public-nominatim'); const limit = parseLimit(opt('--limit', '0')); // 0 = no cap -const sleepMs = Number(env.GEOCODE_SLEEP_MS ?? '1100'); - -const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); - -// Worklist: discoverable events carrying a .address location. We deliberately -// do NOT exclude coordinate locations in SQL — that filtered by $type presence, -// which diverges from the sink's actual _geo derivation (it ignored fsq, and -// excluded events whose only geo/hthree coords are out of range and so never -// get an in-index _geo). The precise "does this already resolve to coordinates?" -// decision is made in memory by addressNeedingGeocode (recordGeo), the same -// derivation the sink uses. json_each walks locations[]; the "$type" key is -// quoted because it starts with $. -const WORKLIST_SQL = ` -SELECT r.uri AS uri, r.did AS did, r.rkey AS rkey, r.record AS record -FROM records_event AS r -WHERE EXISTS ( - SELECT 1 FROM json_each(r.record, '$.locations') - WHERE json_extract(value, '$."$type"') = 'community.lexicon.location.address') - AND ${discoverableSql('r.record')} -`; - -// Re-read events from records_event by uri, keeping only those still present and -// discoverable. The parsed record drives a full-doc upsert, so it's read fresh -// (not from the job-start worklist) — an event deleted or unlisted while the slow -// geocode loop runs is skipped here instead of being resurrected as a stale doc. -async function fetchLiveDocs( - d1: D1Client, - uris: string[] -): Promise<{ uri: string; did: string; rkey: string; record: Record }[]> { - if (uris.length === 0) return []; - const placeholders = uris.map(() => '?').join(','); - const rows = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( - `SELECT uri, did, rkey, record FROM records_event - WHERE uri IN (${placeholders}) AND ${discoverableSql('record')}`, - uris - ); - const out: { uri: string; did: string; rkey: string; record: Record }[] = []; - for (const r of rows) { - try { - out.push({ uri: r.uri, did: r.did, rkey: r.rkey, record: JSON.parse(r.record) }); - } catch { - // Unparseable record JSON — skip, same as the worklist parse. - } - } - return out; -} +const sleepMs = parseSleepMs(env.GEOCODE_SLEEP_MS); +// The effective geocoder is public OSM Nominatim when GEOCODER_URL is unset or +// points at the public host — EVEN if GEOCODER_KEY is set (createGeocoder ignores +// the key against Nominatim, the ?key= is dropped). Gate the bulk guard AND the +// cache provenance tag on the effective host, not key presence, so a +// key-without-URL run can't slip an uncapped backfill onto public Nominatim or +// mis-tag the cache rows as locationiq. +const onPublicNominatim = isPublicNominatimHost(env.GEOCODER_URL); async function main() { if (!env.CLOUDFLARE_API_TOKEN) throw new Error('CLOUDFLARE_API_TOKEN is required'); @@ -107,15 +77,21 @@ async function main() { if (!env.CLOUDFLARE_ACCOUNT_ID || !env.D1_DATABASE_ID) throw new Error('CLOUDFLARE_ACCOUNT_ID and D1_DATABASE_ID are required'); if (!env.MEILI_URL || !env.MEILI_KEY) throw new Error('MEILI_URL and MEILI_KEY are required'); - if (!env.GEOCODER_KEY) { + if (onPublicNominatim) { console.warn( - '[geocode] GEOCODER_KEY is unset → using PUBLIC Nominatim. OK for a small drip; ' + - 'a bulk backfill against public Nominatim risks a silent IP ban. Use LocationIQ for backfill.' + '[geocode] effective geocoder is PUBLIC Nominatim' + + (env.GEOCODER_KEY + ? ' (GEOCODER_KEY is set but GEOCODER_URL is unset/public, so the key is IGNORED)' + : '') + + '. OK for a small drip; a bulk backfill risks a silent IP ban. ' + + 'Set GEOCODER_URL to your LocationIQ endpoint for backfill.' ); } - // Hard-stop a bulk/uncapped keyless run before it can touch public Nominatim. + // Hard-stop a bulk/uncapped run before it can touch public Nominatim — keyed on + // the EFFECTIVE host so a GEOCODER_KEY with no URL (still public Nominatim) can't + // slip the gate the way `!!GEOCODER_KEY` did. requireGeocoderForBulk({ - hasKey: !!env.GEOCODER_KEY, + nonPublicEndpoint: !onPublicNominatim, dryRun, limit, allowPublic: allowPublicNominatim @@ -133,128 +109,18 @@ async function main() { indexUid: env.SEARCH_INDEX ?? 'events' }); - // Defensive: the table is created by geocode-cache.sql, but a fresh DB - // shouldn't make the job crash before it can self-heal. - await d1.query( - `CREATE TABLE IF NOT EXISTS geocode_cache ( - address_norm TEXT PRIMARY KEY, lat REAL, lng REAL, precision TEXT, - source TEXT NOT NULL, geocoded_at INTEGER NOT NULL, - fail_count INTEGER NOT NULL DEFAULT 0, last_error TEXT)` - ); - - // Load the whole cache once (small) and decide eligibility in memory. - const cacheRows = await d1.query(`SELECT * FROM geocode_cache`); - const cache = new Map(cacheRows.map((r) => [r.address_norm, r])); - - // Worklist → WorklistEvent[] (parse record JSON; keep only events that need - // geocoding — an address location AND no coordinates the index already - // derives, per addressNeedingGeocode). - const rawEvents = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( - WORKLIST_SQL - ); - const events: WorklistEvent[] = []; - for (const e of rawEvents) { - let record: Record; - try { - record = JSON.parse(e.record); - } catch { - continue; - } - const loc = addressNeedingGeocode(record); - if (loc) events.push({ uri: e.uri, did: e.did, rkey: e.rkey, loc }); - } - - const byNorm = groupEventsByNorm(events); - const now = Date.now(); - const work = [...byNorm.entries()].filter(([norm]) => - isEligible(cache.get(norm), now, retryNegative) - ); - const capped = limit > 0 ? work.slice(0, limit) : work; - - console.log( - `[geocode] worklist events=${events.length} unique-addresses=${byNorm.size} ` + - `eligible=${work.length} processing=${capped.length}${dryRun ? ' (dry-run)' : ''}` - ); - - let resolved = 0; - let negative = 0; - let transient = 0; - let skippedGone = 0; - for (const [norm, group] of capped) { - const query = addressToQuery(group[0].loc); - if (dryRun) { - console.log(`[geocode] would geocode "${query}" -> ${group.length} event(s)`); - continue; - } - try { - const point = await geocoder.geocode(query); - if (point) { - await d1.query( - `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) - VALUES (?, ?, ?, ?, ?, ?, 0, NULL) - ON CONFLICT(address_norm) DO UPDATE SET - lat=excluded.lat, lng=excluded.lng, precision=excluded.precision, - source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=0, last_error=NULL`, - [ - norm, - point.lat, - point.lng, - point.precision ?? null, - env.GEOCODER_KEY ? 'locationiq' : 'nominatim', - now - ] - ); - // Re-read the events fresh and upsert the FULL doc with _geo attached, - // keeping only those still present AND discoverable. A full-doc upsert is - // idempotent and needs no "is it indexed?" snapshot: it merges if the - // event is already indexed, lands a complete doc if not (never a {id,_geo} - // stub), and is identical to what the sink writes — so a concurrent sink - // write converges instead of clobbering. - const live = await fetchLiveDocs( - d1, - group.map((e) => e.uri) - ); - skippedGone += group.length - live.length; - if (live.length) { - await meili.upsert( - live.map((r) => { - const doc = eventToSearchDoc({ - uri: r.uri, - did: r.did, - collection: EVENT_COLLECTION, - rkey: r.rkey, - record: r.record - }); - // Don't overwrite a coordinate _geo the record gained mid-run. - if (!doc._geo) doc._geo = { lat: point.lat, lng: point.lng }; - return doc; - }) - ); - } - resolved++; - } else { - // No-match: write/increment a negative row (backoff handled by isEligible). - await d1.query( - `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) - VALUES (?, NULL, NULL, NULL, ?, ?, 1, 'no match') - ON CONFLICT(address_norm) DO UPDATE SET - source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=geocode_cache.fail_count+1, last_error='no match'`, - [norm, env.GEOCODER_KEY ? 'locationiq' : 'nominatim', now] - ); - negative++; - } - } catch (err) { - // Transient (HTTP/network): DON'T write a negative row — retry next run. - transient++; - console.warn(`[geocode] transient error for "${query}": ${(err as Error).message}`); - } - await sleep(sleepMs); - } - - console.log( - `[geocode] done resolved=${resolved} negative=${negative} transient=${transient} ` + - `skipped-gone=${skippedGone}` - ); + await runGeocodeJob({ + d1, + geocoder, + meili, + source: onPublicNominatim ? 'nominatim' : 'locationiq', + limit, + sleepMs, + retryNegative, + dryRun, + log: (m) => console.log('[geocode]', m), + warn: (m) => console.warn('[geocode]', m) + }); } main().catch((e) => { diff --git a/apps/web/src/app.d.ts b/apps/web/src/app.d.ts index 827e42e..067b36b 100644 --- a/apps/web/src/app.d.ts +++ b/apps/web/src/app.d.ts @@ -101,6 +101,20 @@ declare global { /** Default Admin API Key for the write path (set via * `wrangler secret put`). Never the instance root key. */ SEARCH_SINK_API_KEY?: string; + /** Forward-geocoder endpoint for the address→_geo drip (the cron job + * that resolves coordinates for newly-ingested address-only events). + * Nominatim-compatible /search; LocationIQ = `https://us1.locationiq.com/v1/search`. + * Reuses the SEARCH_SINK_* Meili write creds + the DB binding, so this + * plus GEOCODER_KEY are the only drip-specific config. */ + GEOCODER_URL?: string; + /** Geocoder API key (LocationIQ), set via `wrangler secret put`. When + * unset, the drip no-ops (it won't fall back to public Nominatim). */ + GEOCODER_KEY?: string; + /** Optional User-Agent for geocoder requests. */ + GEOCODER_USER_AGENT?: string; + /** Min ms between geocoder calls in the drip — the rate limiter. Set to + * the ceiling the geocoder tier allows; defaults to DEFAULT_GEOCODE_SLEEP_MS. */ + GEOCODE_SLEEP_MS?: string; }; /** Cloudflare Worker execution context. Use `ctx.waitUntil(promise)` to * let the worker keep a fire-and-forget task alive after the response diff --git a/apps/web/src/lib/geocode/config.ts b/apps/web/src/lib/geocode/config.ts new file mode 100644 index 0000000..26ebb03 --- /dev/null +++ b/apps/web/src/lib/geocode/config.ts @@ -0,0 +1,20 @@ +// Address→_geo geocode drip — shared constants for the in-cron backfill of +// coordinates for newly-ingested address-only events. Mirrors lib/notify/config. + +/** Cadence gate: the drip body runs at most once per this interval. It rides the + * every-minute cron (routes/api/cron) via a D1 timestamp, so a new address-only + * event waits at most ~this long for its _geo. 30 min keeps geocoder usage and + * D1 reads low while bounding that latency. */ +export const GEOCODE_DRIP_INTERVAL_MS = 30 * 60 * 1000; + +/** Max unique addresses geocoded per drip run. Each costs ~one geocoder call + + * GEOCODE_SLEEP_MS, so this bounds the run's wall time far under the 15-min cron + * cap and bounds per-tier geocoder spend. Leftover eligible work is picked up on + * the next run; steady-state inflow is well under the cap. */ +export const MAX_GEOCODE_PER_TICK = 50; + +/** Default min delay between geocoder calls, in ms — the rate limiter. Operators + * set GEOCODE_SLEEP_MS to the ceiling their geocoder tier allows (e.g. LocationIQ + * paid can go faster; public Nominatim wants >=1000). Sized safe-by-default; a + * drip's low volume makes the exact value latency-irrelevant. */ +export const DEFAULT_GEOCODE_SLEEP_MS = 1100; diff --git a/apps/web/src/lib/geocode/db.ts b/apps/web/src/lib/geocode/db.ts new file mode 100644 index 0000000..b7e548a --- /dev/null +++ b/apps/web/src/lib/geocode/db.ts @@ -0,0 +1,40 @@ +// D1 state for the geocode drip's cadence gate. A tiny single-purpose key/value +// table so geocode_cache stays purely address rows. The job's own geocode_cache +// table is created/self-healed by the shared core (runGeocodeJob). + +/** Row key for the drip's last-run timestamp (epoch ms). */ +const LAST_DRIP_KEY = 'last_drip_at'; + +export async function ensureGeocodeDripSchema(db: D1Database): Promise { + await db + .prepare( + `CREATE TABLE IF NOT EXISTS geocode_drip_state ( + key TEXT PRIMARY KEY, + value INTEGER NOT NULL + )` + ) + .run(); +} + +/** Atomically claim the drip's cadence slot for `now`. A single conditional + * upsert: insert the first-ever timestamp, or advance it ONLY when a full + * `intervalMs` has elapsed since the stored one. Returns true iff THIS call won + * the slot (it wrote a row) — so two cron ticks racing inside the same interval + * can't both proceed, unlike the old read-then-write gate (a window where both + * read the stale timestamp before either wrote). Relies on D1 reporting + * meta.changes (1 = claimed, 0 = gated out by the WHERE on DO UPDATE). */ +export async function claimDripSlot( + db: D1Database, + now: number, + intervalMs: number +): Promise { + const res = await db + .prepare( + `INSERT INTO geocode_drip_state (key, value) VALUES (?, ?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value + WHERE excluded.value - geocode_drip_state.value >= ?` + ) + .bind(LAST_DRIP_KEY, now, intervalMs) + .run(); + return (res.meta?.changes ?? 0) > 0; +} diff --git a/apps/web/src/lib/geocode/process.test.ts b/apps/web/src/lib/geocode/process.test.ts new file mode 100644 index 0000000..6f53daf --- /dev/null +++ b/apps/web/src/lib/geocode/process.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect } from 'vitest'; +import { runGeocodeDrip, geocodeDripConfigured, dripGeocoderPolicy } from './process'; +import { GEOCODE_DRIP_INTERVAL_MS, MAX_GEOCODE_PER_TICK, DEFAULT_GEOCODE_SLEEP_MS } from './config'; +import { + PUBLIC_NOMINATIM_DRIP_MAX, + PUBLIC_NOMINATIM_MIN_SLEEP_MS +} from '../search/server/geocoder'; + +type Env = Parameters[0]; + +/** The drip needs only the sink creds; the geocoder key is optional (keyless runs + * on public Nominatim at safe limits). Tests add GEOCODER_* / GEOCODE_SLEEP_MS. */ +const configuredEnv = (over: Partial = {}): Env => + ({ + SEARCH_SINK_URL: 'http://meili.test', + SEARCH_SINK_API_KEY: 'admin-key', + ...over + }) as Env; + +const locationiqEnv = (over: Partial = {}): Env => + configuredEnv({ + GEOCODER_KEY: 'loc-key', + GEOCODER_URL: 'https://us1.locationiq.com/v1/search', + ...over + }); + +/** Minimal stand-in for the native D1 binding. Implements the atomic cadence + * claim (INSERT ... ON CONFLICT DO UPDATE ... WHERE → meta.changes) against an + * in-memory value, and returns empty results for everything else so the core + * runs but does no network. Records every prepared SQL so tests count core runs. + * `failCore` makes the core's geocode_cache queries throw, to test that the + * cadence slot is claimed BEFORE the work (a failed run still advances it). */ +function fakeNativeDb(opts: { failCore?: boolean } = {}) { + const state = new Map(); + const sqls: string[] = []; + const db = { + prepare(sql: string) { + sqls.push(sql); + let bound: unknown[] = []; + const stmt = { + bind(...args: unknown[]) { + bound = args; + return stmt; + }, + async first() { + return null as T; + }, + async all() { + if (opts.failCore && sql.includes('geocode_cache')) { + throw new Error('core query failed'); + } + return { results: [] as T[] }; + }, + async run() { + if (sql.includes('INSERT INTO geocode_drip_state')) { + const [key, value, interval] = bound as [string, number, number]; + const prev = state.get(key); + const claimed = prev === undefined || value - prev >= interval; + if (claimed) state.set(key, value); + return { success: true, meta: { changes: claimed ? 1 : 0 } }; + } + return { success: true, meta: { changes: 0 } }; + } + }; + return stmt; + } + } as unknown as D1Database; + return { db, sqls, state }; +} + +const coreRuns = (sqls: string[]) => + sqls.filter((s) => s.includes('CREATE TABLE IF NOT EXISTS geocode_cache')).length; + +describe('geocodeDripConfigured', () => { + it('requires only the sink creds — the geocoder key is optional', () => { + expect(geocodeDripConfigured(configuredEnv())).toBe(true); + expect(geocodeDripConfigured(locationiqEnv())).toBe(true); + }); + + it('is false without the sink (nowhere to write _geo)', () => { + expect(geocodeDripConfigured(configuredEnv({ SEARCH_SINK_URL: undefined }))).toBe(false); + expect(geocodeDripConfigured(configuredEnv({ SEARCH_SINK_API_KEY: undefined }))).toBe(false); + }); +}); + +describe('dripGeocoderPolicy', () => { + it('keyless → public Nominatim safe defaults: small cap, ≥1 req/s, source nominatim', () => { + const p = dripGeocoderPolicy(configuredEnv()); + expect(p.source).toBe('nominatim'); + expect(p.limit).toBe(Math.min(MAX_GEOCODE_PER_TICK, PUBLIC_NOMINATIM_DRIP_MAX)); + expect(p.sleepMs).toBe(DEFAULT_GEOCODE_SLEEP_MS); + expect(p.sleepMs).toBeGreaterThanOrEqual(PUBLIC_NOMINATIM_MIN_SLEEP_MS); + }); + + it('floors a too-fast GEOCODE_SLEEP_MS on Nominatim to the policy minimum', () => { + const p = dripGeocoderPolicy(configuredEnv({ GEOCODE_SLEEP_MS: '100' })); + expect(p.sleepMs).toBe(PUBLIC_NOMINATIM_MIN_SLEEP_MS); + }); + + it('a key with no URL still hits Nominatim → safe defaults, key ignored, source nominatim', () => { + const p = dripGeocoderPolicy(configuredEnv({ GEOCODER_KEY: 'k' })); + expect(p.source).toBe('nominatim'); + expect(p.limit).toBe(Math.min(MAX_GEOCODE_PER_TICK, PUBLIC_NOMINATIM_DRIP_MAX)); + expect(p.sleepMs).toBeGreaterThanOrEqual(PUBLIC_NOMINATIM_MIN_SLEEP_MS); + }); + + it('keyed LocationIQ endpoint lifts the cap and honors the operator throttle', () => { + const p = dripGeocoderPolicy(locationiqEnv({ GEOCODE_SLEEP_MS: '300' })); + expect(p.source).toBe('locationiq'); + expect(p.limit).toBe(MAX_GEOCODE_PER_TICK); + expect(p.sleepMs).toBe(300); + }); + + it('falls back to the default throttle on a malformed GEOCODE_SLEEP_MS', () => { + expect(dripGeocoderPolicy(locationiqEnv({ GEOCODE_SLEEP_MS: 'abc' })).sleepMs).toBe( + DEFAULT_GEOCODE_SLEEP_MS + ); + // negative on the LocationIQ path also falls back (no floor there) + expect(dripGeocoderPolicy(locationiqEnv({ GEOCODE_SLEEP_MS: '-5' })).sleepMs).toBe( + DEFAULT_GEOCODE_SLEEP_MS + ); + }); +}); + +describe('runGeocodeDrip cadence gate', () => { + it('no-ops when unconfigured (touches no D1)', async () => { + const { db, sqls } = fakeNativeDb(); + await runGeocodeDrip(configuredEnv({ SEARCH_SINK_URL: undefined }), db, 1_000); + expect(sqls).toHaveLength(0); + }); + + it('runs on first call, skips within the interval, runs again after it', async () => { + const { db, sqls } = fakeNativeDb(); + const t0 = 1_700_000_000_000; + + await runGeocodeDrip(configuredEnv(), db, t0); + expect(coreRuns(sqls)).toBe(1); + + // 1 min later — inside the 30-min interval → gated out, core does not run. + await runGeocodeDrip(configuredEnv(), db, t0 + 60_000); + expect(coreRuns(sqls)).toBe(1); + + // Past the interval → runs again. + await runGeocodeDrip(configuredEnv(), db, t0 + GEOCODE_DRIP_INTERVAL_MS + 1); + expect(coreRuns(sqls)).toBe(2); + }); + + it('claim-before-work: a failed run still advances the slot, so the next tick gates out', async () => { + const { db, sqls } = fakeNativeDb({ failCore: true }); + const t0 = 1_700_000_000_000; + + // The core throws, but the slot was claimed before the work began. + await expect(runGeocodeDrip(configuredEnv(), db, t0)).rejects.toThrow(); + expect(coreRuns(sqls)).toBe(1); + + // Next tick within the interval is gated out — no retry-storm despite the failure. + await runGeocodeDrip(configuredEnv(), db, t0 + 60_000); + expect(coreRuns(sqls)).toBe(1); + }); +}); diff --git a/apps/web/src/lib/geocode/process.ts b/apps/web/src/lib/geocode/process.ts new file mode 100644 index 0000000..9500257 --- /dev/null +++ b/apps/web/src/lib/geocode/process.ts @@ -0,0 +1,89 @@ +import { + createGeocoder, + isPublicNominatimHost, + PUBLIC_NOMINATIM_DRIP_MAX, + PUBLIC_NOMINATIM_MIN_SLEEP_MS +} from '$lib/search/server/geocoder'; +import { MeiliEventIndex, meiliSinkBackendFromEnv } from '$lib/search/server/meili-sink'; +import { nativeD1Client } from '$lib/search/server/d1-native'; +import { runGeocodeJob } from '$lib/search/server/geocode-job'; +import { GEOCODE_DRIP_INTERVAL_MS, MAX_GEOCODE_PER_TICK, DEFAULT_GEOCODE_SLEEP_MS } from './config'; +import { ensureGeocodeDripSchema, claimDripSlot } from './db'; + +type Env = App.Platform['env']; + +/** Configured when the Meili write path (the sink creds, reused) is present — + * that's the only hard requirement, since the drip just needs somewhere to write + * the resolved _geo. The geocoder works keyless against public Nominatim, so a + * key is OPTIONAL: keyless still runs the drip, just at Nominatim-safe limits (a + * smaller per-tick cap and a >=1 req/s throttle floor — see dripGeocoderPolicy); + * set GEOCODER_KEY + a non-public GEOCODER_URL (LocationIQ) to lift them. No sink + * → no-op (dev, or before search is provisioned). */ +export function geocodeDripConfigured(env: Env): boolean { + return meiliSinkBackendFromEnv(env) !== null; +} + +/** Per-tick volume + throttle + cache-source tag for the drip. When the effective + * endpoint is public Nominatim (keyless, OR a key with an unset/public + * GEOCODER_URL — the ?key= is then ignored and we're really on Nominatim), both + * are clamped to Nominatim's usage policy: the per-tick cap drops to the small- + * drip ceiling and the throttle is floored to >=1 req/s. This keeps the keyless + * default safe for public use without refusing to run. A keyed, non-public + * endpoint (LocationIQ) keeps the full cap and honors the operator's throttle. + * Pure + exported so the safe-default policy can be unit-tested directly. */ +export function dripGeocoderPolicy(env: Env): { limit: number; sleepMs: number; source: string } { + const raw = Number(env.GEOCODE_SLEEP_MS); + const requested = Number.isFinite(raw) && raw >= 0 ? raw : DEFAULT_GEOCODE_SLEEP_MS; + if (isPublicNominatimHost(env.GEOCODER_URL)) { + return { + limit: Math.min(MAX_GEOCODE_PER_TICK, PUBLIC_NOMINATIM_DRIP_MAX), + sleepMs: Math.max(requested, PUBLIC_NOMINATIM_MIN_SLEEP_MS), + source: 'nominatim' + }; + } + return { limit: MAX_GEOCODE_PER_TICK, sleepMs: requested, source: 'locationiq' }; +} + +/** Entry point, called from the cron after firehose ingest. Resolves _geo for + * newly-ingested address-only events so they surface in /near-me. No-ops when + * unconfigured, and self-throttles to GEOCODE_DRIP_INTERVAL_MS via an atomic D1 + * claim so it runs ~every 30 min while riding the every-minute cron — and so two + * overlapping ticks can't both start a drip (the claim is one conditional write, + * not a read-then-write). Idempotent: cached addresses are skipped, so a run with + * no new addresses makes ~zero geocoder calls. `now` is injectable for tests. */ +export async function runGeocodeDrip(env: Env, db: D1Database, now = Date.now()): Promise { + if (!geocodeDripConfigured(env)) return; + await ensureGeocodeDripSchema(db); + + // Atomically claim the cadence slot: a single conditional upsert that advances + // the timestamp ONLY if a full interval has elapsed, reporting whether it won. + // This both gates to the interval and closes the overlap window the old + // read-then-write left open. A claimed-but-failed run waits one interval to + // retry, which is fine for an idempotent drip. + if (!(await claimDripSlot(db, now, GEOCODE_DRIP_INTERVAL_MS))) return; + + const backend = meiliSinkBackendFromEnv(env)!; + const { limit, sleepMs, source } = dripGeocoderPolicy(env); + // A key with no (or a public) URL silently hits Nominatim with the key ignored + // — surface that misconfig rather than letting the operator believe they're on + // LocationIQ. We still run (safely), just on Nominatim's reduced limits. + if (env.GEOCODER_KEY && source === 'nominatim') { + console.warn( + '[geocode-drip] GEOCODER_KEY is set but GEOCODER_URL is unset or public Nominatim — the key ' + + 'is ignored and the drip runs on public Nominatim at its reduced cap/throttle. Set ' + + 'GEOCODER_URL to your LocationIQ endpoint to use the key.' + ); + } + + await runGeocodeJob({ + d1: nativeD1Client(db), + geocoder: createGeocoder(env), + meili: new MeiliEventIndex(backend), + source, + limit, + sleepMs, + now, + log: (m) => console.log('[geocode-drip]', m), + warn: (m) => console.warn('[geocode-drip]', m) + }); +} diff --git a/apps/web/src/lib/search/server/d1-native.ts b/apps/web/src/lib/search/server/d1-native.ts new file mode 100644 index 0000000..eb15f69 --- /dev/null +++ b/apps/web/src/lib/search/server/d1-native.ts @@ -0,0 +1,16 @@ +// Adapt the native Worker D1 binding (platform.env.DB) to the D1Client interface +// the geocode job uses. Lets the same core run in-Worker (the cron drip) over the +// native binding — no REST endpoint, no CLOUDFLARE_API_TOKEN — while the off-box +// CLI keeps the HTTP client in d1-http.ts. One parameterized-query method, the +// only surface the job needs. +import type { D1Client } from './d1-http'; + +export function nativeD1Client(db: D1Database): D1Client { + return { + async query>(sql: string, params: unknown[] = []): Promise { + const stmt = params.length ? db.prepare(sql).bind(...params) : db.prepare(sql); + const { results } = await stmt.all(); + return results ?? []; + } + }; +} diff --git a/apps/web/src/lib/search/server/geocode-job.test.ts b/apps/web/src/lib/search/server/geocode-job.test.ts new file mode 100644 index 0000000..5d34f7e --- /dev/null +++ b/apps/web/src/lib/search/server/geocode-job.test.ts @@ -0,0 +1,420 @@ +import { describe, it, expect, vi } from 'vitest'; +import { runGeocodeJob } from './geocode-job'; +import type { D1Client } from './d1-http'; +import type { GeoPoint } from './geocoder'; +import type { GeocodeCacheRow } from './geocode-cache'; +import { ADDRESS_TYPE, normalizeAddress, addressLocation } from './address-norm'; +import type { MeiliEventIndex } from './meili-sink'; + +const NOW = 1_700_000_000_000; + +type RawEvent = { uri: string; did: string; rkey: string; record: string }; + +const addrRecord = (over: Record = {}) => + JSON.stringify({ + name: 'Test Event', + startsAt: '2026-07-01T18:00:00Z', + locations: [ + { $type: ADDRESS_TYPE, locality: 'Louisville', region: 'KY', country: 'US', ...over } + ] + }); + +/** The normalized cache key the job will compute for a given record — so a + * seeded cache row lines up with the worklist's grouping. */ +const normFor = (record: string) => + normalizeAddress(addressLocation(JSON.parse(record)) as Record); + +/** Minimal D1Client that routes by SQL fragment and captures geocode_cache + * writes. `live` (the fresh re-read for fetchLiveDocs) defaults to the worklist + * but can differ — to model an event deleted mid-run, or one that gained coords. */ +function fakeD1(opts: { cache?: GeocodeCacheRow[]; worklist: RawEvent[]; live?: RawEvent[] }) { + const live = opts.live ?? opts.worklist; + const inserts: { sql: string; params: unknown[] }[] = []; + const client: D1Client = { + async query>(sql: string, params: unknown[] = []): Promise { + if (sql.includes('INSERT INTO geocode_cache')) { + inserts.push({ sql, params }); + return [] as T[]; + } + if (sql.includes('CREATE TABLE')) return [] as T[]; + if (sql.includes('SELECT * FROM geocode_cache')) return (opts.cache ?? []) as T[]; + if (sql.includes('WHERE uri IN')) { + // fetchLiveDocs: only rows whose uri is in the bound params + return live.filter((e) => params.includes(e.uri)) as T[]; + } + if (sql.includes('json_each')) return opts.worklist as T[]; // WORKLIST_SQL + return [] as T[]; + } + }; + return { client, inserts }; +} + +const geocoderReturning = (point: GeoPoint | null) => ({ geocode: vi.fn(async () => point) }); + +function fakeMeili() { + const upsert = vi.fn(async () => {}); + const applySettings = vi.fn(async () => {}); + return { + meili: { upsert, applySettings } as unknown as MeiliEventIndex, + upsert, + applySettings + }; +} + +const resolved = (norm: string): GeocodeCacheRow => ({ + address_norm: norm, + lat: 1, + lng: 2, + precision: 'locality', + source: 'locationiq', + geocoded_at: NOW - 1000, + fail_count: 0, + last_error: null +}); + +describe('runGeocodeJob', () => { + it('geocodes a new address: writes cache and _geo-upserts the doc', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76, precision: 'locality' }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(geocoder.geocode).toHaveBeenCalledTimes(1); + expect(res.resolved).toBe(1); + // Resolved insert carries lat/lng at params [1],[2]. + const ins = inserts.find((i) => i.params[1] === 38.25); + expect(ins).toBeTruthy(); + expect(upsert).toHaveBeenCalledTimes(1); + const docs = ( + upsert.mock.calls[0] as unknown as [Array<{ _geo?: { lat: number; lng: number } }>] + )[0]; + expect(docs[0]._geo).toEqual({ lat: 38.25, lng: -85.76 }); + }); + + it('is idempotent: skips an already-resolved address (no geocoder call)', async () => { + const record = addrRecord(); + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record }]; + const { client, inserts } = fakeD1({ worklist, cache: [resolved(normFor(record)!)] }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 1, lng: 2 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(geocoder.geocode).not.toHaveBeenCalled(); + expect(res.processed).toBe(0); + expect(inserts).toHaveLength(0); + expect(upsert).not.toHaveBeenCalled(); + }); + + it('respects the limit cap across unique addresses', async () => { + const worklist = [ + { + uri: 'at://did:x/c/1', + did: 'did:x', + rkey: '1', + record: addrRecord({ locality: 'Berlin' }) + }, + { + uri: 'at://did:x/c/2', + did: 'did:x', + rkey: '2', + record: addrRecord({ locality: 'Brussels' }) + }, + { uri: 'at://did:x/c/3', did: 'did:x', rkey: '3', record: addrRecord({ locality: 'Paris' }) } + ]; + const { client } = fakeD1({ worklist }); + const { meili } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 1, lng: 2 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + limit: 2, + sleepMs: 0, + now: NOW + }); + + expect(res.uniqueAddresses).toBe(3); + expect(res.processed).toBe(2); + expect(geocoder.geocode).toHaveBeenCalledTimes(2); + }); + + it('dry-run computes the worklist but makes no geocoder/cache/Meili writes', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 1, lng: 2 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + dryRun: true, + sleepMs: 0, + now: NOW + }); + + expect(res.processed).toBe(1); + expect(res.resolved).toBe(0); + expect(geocoder.geocode).not.toHaveBeenCalled(); + expect(inserts).toHaveLength(0); + expect(upsert).not.toHaveBeenCalled(); + }); + + it('writes a negative cache row on no-match (and does not upsert)', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning(null); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(res.negative).toBe(1); + expect(res.resolved).toBe(0); + expect(inserts.some((i) => /no match/.test(i.sql))).toBe(true); + expect(upsert).not.toHaveBeenCalled(); + }); + + it('counts a thrown geocoder error as transient and writes no cache row', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const { meili } = fakeMeili(); + const geocoder = { + geocode: vi.fn(async () => { + throw new Error('429 rate limited'); + }) + }; + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(res.transient).toBe(1); + expect(res.resolved).toBe(0); + expect(res.negative).toBe(0); + expect(inserts).toHaveLength(0); + }); + + it('does NOT positive-cache when the Meili upsert fails (address retries, not skips forever)', async () => { + // Geocode succeeds and a live doc exists, but the upsert throws. The positive + // cache row must NOT be written — otherwise the address is cached "resolved" + // yet never indexed, and the idempotent skip locks in a permanent _geo gap. + // It must count transient and stay eligible for the next run. + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const upsert = vi.fn(async () => { + throw new Error('Meilisearch PUT failed: 503'); + }); + const applySettings = vi.fn(async () => {}); + const meili = { upsert, applySettings } as unknown as MeiliEventIndex; + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(res.transient).toBe(1); + expect(res.resolved).toBe(0); + expect(inserts).toHaveLength(0); // no positive (or any) geocode_cache write + }); + + it('does NOT positive-cache when applySettings fails before the first upsert', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client, inserts } = fakeD1({ worklist }); + const upsert = vi.fn(async () => {}); + const applySettings = vi.fn(async () => { + throw new Error('Meilisearch PATCH failed: 500'); + }); + const meili = { upsert, applySettings } as unknown as MeiliEventIndex; + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(res.transient).toBe(1); + expect(res.resolved).toBe(0); + expect(upsert).not.toHaveBeenCalled(); + expect(inserts).toHaveLength(0); + }); + + it('applies index settings exactly once, before the first upsert (S3)', async () => { + const worklist = [ + { + uri: 'at://did:x/c/1', + did: 'did:x', + rkey: '1', + record: addrRecord({ locality: 'Berlin' }) + }, + { uri: 'at://did:x/c/2', did: 'did:x', rkey: '2', record: addrRecord({ locality: 'Paris' }) } + ]; + const { client } = fakeD1({ worklist }); + const { meili, upsert, applySettings } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + // Two distinct addresses → two upserts, but settings applied just once... + expect(applySettings).toHaveBeenCalledTimes(1); + expect(upsert).toHaveBeenCalledTimes(2); + // ...and that one application precedes the first upsert (a bare index 400s). + expect(applySettings.mock.invocationCallOrder[0]).toBeLessThan( + upsert.mock.invocationCallOrder[0] + ); + }); + + it('never touches Meili when nothing resolves (a no-match run skips settings + upsert)', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + const { client } = fakeD1({ worklist }); + const { meili, upsert, applySettings } = fakeMeili(); + const geocoder = geocoderReturning(null); + + await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(applySettings).not.toHaveBeenCalled(); + expect(upsert).not.toHaveBeenCalled(); + }); + + it('caches the address but skips the upsert when the event vanished mid-run (gap 1)', async () => { + const worklist = [{ uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record: addrRecord() }]; + // In the worklist, but gone (deleted/unlisted) by the fresh re-read. + const { client, inserts } = fakeD1({ worklist, live: [] }); + const { meili, upsert, applySettings } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(res.resolved).toBe(1); // the ADDRESS resolved + cached (won't re-geocode) + expect(res.skippedGone).toBe(1); // but its one event is gone + expect(upsert).not.toHaveBeenCalled(); + expect(applySettings).not.toHaveBeenCalled(); + expect(inserts.some((i) => i.params[1] === 38.25)).toBe(true); + }); + + it('geocodes a shared address once and upserts every event carrying it (gap 2)', async () => { + const record = addrRecord(); // identical address on two distinct events + const worklist = [ + { uri: 'at://did:x/c/1', did: 'did:x', rkey: '1', record }, + { uri: 'at://did:y/c/2', did: 'did:y', rkey: '2', record } + ]; + const { client } = fakeD1({ worklist }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + const res = await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + expect(geocoder.geocode).toHaveBeenCalledTimes(1); // one address → one geocoder call + expect(res.uniqueAddresses).toBe(1); + expect(res.resolved).toBe(1); + expect(upsert).toHaveBeenCalledTimes(1); + const docs = ( + upsert.mock.calls[0] as unknown as [Array<{ _geo?: { lat: number; lng: number } }>] + )[0]; + expect(docs).toHaveLength(2); // both events upserted in one batch + expect(docs.every((d) => d._geo?.lat === 38.25)).toBe(true); + }); + + it('does not overwrite a coordinate _geo the record gained mid-run (gap 3)', async () => { + // Worklist sees an address-only record (eligible to geocode), but the fresh + // re-read finds it now also carries an in-range geo location. + const uri = 'at://did:x/c/1'; + const liveRecord = JSON.stringify({ + name: 'Test Event', + startsAt: '2026-07-01T18:00:00Z', + locations: [ + { $type: ADDRESS_TYPE, locality: 'Louisville', region: 'KY', country: 'US' }, + { $type: 'community.lexicon.location.geo', latitude: '40.0', longitude: '-80.0' } + ] + }); + const { client } = fakeD1({ + worklist: [{ uri, did: 'did:x', rkey: '1', record: addrRecord() }], + live: [{ uri, did: 'did:x', rkey: '1', record: liveRecord }] + }); + const { meili, upsert } = fakeMeili(); + const geocoder = geocoderReturning({ lat: 38.25, lng: -85.76 }); + + await runGeocodeJob({ + d1: client, + geocoder, + meili, + source: 'locationiq', + sleepMs: 0, + now: NOW + }); + + const docs = ( + upsert.mock.calls[0] as unknown as [Array<{ _geo?: { lat: number; lng: number } }>] + )[0]; + // The record's own precise coords win — the geocoder point must not clobber them. + expect(docs[0]._geo).toEqual({ lat: 40, lng: -80 }); + }); +}); diff --git a/apps/web/src/lib/search/server/geocode-job.ts b/apps/web/src/lib/search/server/geocode-job.ts new file mode 100644 index 0000000..6f70140 --- /dev/null +++ b/apps/web/src/lib/search/server/geocode-job.ts @@ -0,0 +1,293 @@ +// Shared core of the address→_geo geocode job: find address-only events that +// lack coordinates, geocode each unique address through the injected geocoder, +// write geocode_cache, and _geo-upsert the affected Meili docs. ONE job: "find +// work" and "geocode" are sequential steps sharing the cache, not two passes. +// +// Deliberately I/O-agnostic — it takes an injected D1Client, Geocoder, and +// MeiliEventIndex rather than reading env or constructing clients. Two callers: +// • scripts/geocode-events.ts — off-box CLI, D1 over the REST client, one-shot +// backfill / manual drip (keeps the CLI flags + bulk guard). +// • lib/geocode/process.ts — in-Worker cron drip, D1 over the native binding +// (no CLOUDFLARE_API_TOKEN), self-throttled to a cadence. +// Rate limiting is the caller-supplied sleep between calls — the whole reason +// geocoding stays off the ingest hot path. +import { addressToQuery, type Geocoder } from './geocoder'; +import { + isEligible, + groupEventsByNorm, + addressNeedingGeocode, + type GeocodeCacheRow, + type WorklistEvent +} from './geocode-cache'; +import { eventToSearchDoc } from './normalize'; +import { discoverableSql } from './discoverability'; +import { MeiliEventIndex, EVENT_COLLECTION } from './meili-sink'; +import type { D1Client } from './d1-http'; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +// Worklist: discoverable events carrying a .address location. We deliberately +// do NOT exclude coordinate locations in SQL — that filtered by $type presence, +// which diverges from the sink's actual _geo derivation (it ignored fsq, and +// excluded events whose only geo/hthree coords are out of range and so never +// get an in-index _geo). The precise "does this already resolve to coordinates?" +// decision is made in memory by addressNeedingGeocode (recordGeo), the same +// derivation the sink uses. json_each walks locations[]; the "$type" key is +// quoted because it starts with $. +// +// The CASE WHEN json_valid(r.record) guard is REQUIRED, not belt-and-suspenders: +// json_each / json_extract raise "malformed JSON" and abort the ENTIRE query on a +// single poison record, which would fail the whole drip/backfill — so one bad row +// must not even reach them. We use CASE rather than `json_valid(...) AND ...` +// because SQLite guarantees lazy, per-branch evaluation for CASE, whereas WHERE +// conjuncts are optimizer-reorderable (their left-to-right short-circuit is not a +// contract): the THEN holding json_each/json_extract is evaluated ONLY when +// json_valid is true. (The in-memory JSON.parse skip below is a second layer, but +// it never runs if the SQL itself aborts.) +const WORKLIST_SQL = ` +SELECT r.uri AS uri, r.did AS did, r.rkey AS rkey, r.record AS record +FROM records_event AS r +WHERE CASE WHEN json_valid(r.record) THEN ( + EXISTS ( + SELECT 1 FROM json_each(r.record, '$.locations') + WHERE json_extract(value, '$."$type"') = 'community.lexicon.location.address') + AND ${discoverableSql('r.record')} + ) ELSE 0 END +`; + +// Re-read events from records_event by uri, keeping only those still present and +// discoverable. The parsed record drives a full-doc upsert, so it's read fresh +// (not from the job-start worklist) — an event deleted or unlisted while the slow +// geocode loop runs is skipped here instead of being resurrected as a stale doc. +async function fetchLiveDocs( + d1: D1Client, + uris: string[] +): Promise<{ uri: string; did: string; rkey: string; record: Record }[]> { + if (uris.length === 0) return []; + const placeholders = uris.map(() => '?').join(','); + // CASE-guard discoverableSql for the same reason as WORKLIST_SQL: json_extract + // aborts the query on a poison record. CASE guarantees the THEN (json_extract) + // runs ONLY when json_valid is true — robust against the WHERE-conjunct + // reordering a bare `... AND json_valid(record) AND json_extract(...)` leaves to + // the optimizer. uri IN (...) still narrows the scan to the candidate rows. + const rows = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( + `SELECT uri, did, rkey, record FROM records_event + WHERE uri IN (${placeholders}) + AND CASE WHEN json_valid(record) THEN ${discoverableSql('record')} ELSE 0 END`, + uris + ); + const out: { uri: string; did: string; rkey: string; record: Record }[] = []; + for (const r of rows) { + try { + out.push({ uri: r.uri, did: r.did, rkey: r.rkey, record: JSON.parse(r.record) }); + } catch { + // Unparseable record JSON — skip, same as the worklist parse. + } + } + return out; +} + +export interface GeocodeJobOptions { + d1: D1Client; + geocoder: Geocoder; + meili: MeiliEventIndex; + /** Cache provenance tag written to geocode_cache.source ('locationiq'|'nominatim'). */ + source: string; + /** Max unique addresses to process this run; 0 = no cap. */ + limit?: number; + /** Delay between geocoder calls in ms — the rate limiter. */ + sleepMs?: number; + /** Force-retry negative-cached addresses (ignores backoff). */ + retryNegative?: boolean; + /** Compute the worklist and log it, but make no geocoder/cache/Meili writes. */ + dryRun?: boolean; + /** Injectable clock (eligibility backoff + geocoded_at); defaults to now. */ + now?: number; + /** Progress line sink; callers prefix as they like. Default: no-op. */ + log?: (msg: string) => void; + /** Transient-error sink. Default: no-op. */ + warn?: (msg: string) => void; +} + +export interface GeocodeJobResult { + worklistEvents: number; + uniqueAddresses: number; + eligible: number; + processed: number; + resolved: number; + negative: number; + transient: number; + skippedGone: number; +} + +/** Run one geocode pass over the eligible worklist. Pure orchestration over the + * injected clients; idempotent (cached addresses are skipped via isEligible), so + * a steady-state run with no new addresses makes ~zero geocoder calls. */ +export async function runGeocodeJob(opts: GeocodeJobOptions): Promise { + const { + d1, + geocoder, + meili, + source, + limit = 0, + sleepMs, + retryNegative = false, + dryRun = false, + now = Date.now(), + log = () => {}, + warn = () => {} + } = opts; + + // Defensive throttle: a caller passing NaN/negative/undefined (e.g. a malformed + // GEOCODE_SLEEP_MS that slipped past a caller's own parse) must not collapse the + // rate limiter into a busy loop. undefined → the safe 1100ms default; any + // non-finite or negative value → the same. This is the single clamp both the + // Worker drip and the CLI rely on, so the loop below never sleeps on a bad value. + const throttleMs = + typeof sleepMs === 'number' && Number.isFinite(sleepMs) && sleepMs >= 0 ? sleepMs : 1100; + + // Defensive: the table is created by geocode-cache.sql, but a fresh DB + // shouldn't make the job crash before it can self-heal. + await d1.query( + `CREATE TABLE IF NOT EXISTS geocode_cache ( + address_norm TEXT PRIMARY KEY, lat REAL, lng REAL, precision TEXT, + source TEXT NOT NULL, geocoded_at INTEGER NOT NULL, + fail_count INTEGER NOT NULL DEFAULT 0, last_error TEXT)` + ); + + // Load the whole cache once (small) and decide eligibility in memory. + const cacheRows = await d1.query(`SELECT * FROM geocode_cache`); + const cache = new Map(cacheRows.map((r) => [r.address_norm, r])); + + // Worklist → WorklistEvent[] (parse record JSON; keep only events that need + // geocoding — an address location AND no coordinates the index already + // derives, per addressNeedingGeocode). + const rawEvents = await d1.query<{ uri: string; did: string; rkey: string; record: string }>( + WORKLIST_SQL + ); + const events: WorklistEvent[] = []; + for (const e of rawEvents) { + let record: Record; + try { + record = JSON.parse(e.record); + } catch { + continue; + } + const loc = addressNeedingGeocode(record); + if (loc) events.push({ uri: e.uri, did: e.did, rkey: e.rkey, loc }); + } + + const byNorm = groupEventsByNorm(events); + const work = [...byNorm.entries()].filter(([norm]) => + isEligible(cache.get(norm), now, retryNegative) + ); + const capped = limit > 0 ? work.slice(0, limit) : work; + + log( + `worklist events=${events.length} unique-addresses=${byNorm.size} ` + + `eligible=${work.length} processing=${capped.length}${dryRun ? ' (dry-run)' : ''}` + ); + + let resolved = 0; + let negative = 0; + let transient = 0; + let skippedGone = 0; + // Apply the read-path's filterable/sortable settings once, lazily, before the + // FIRST upsert. A PUT to a bare auto-created index would 400 the read path's + // _geo/startsAt filters; the sink ensures this for live ingest, but a drip can + // be the first writer to a freshly (re)created index. Only fires when there's + // actually a doc to write, so a no-resolution run touches Meili zero times. + let settingsApplied = false; + for (let i = 0; i < capped.length; i++) { + const [norm, group] = capped[i]; + const query = addressToQuery(group[0].loc); + if (dryRun) { + log(`would geocode "${query}" -> ${group.length} event(s)`); + continue; + } + try { + const point = await geocoder.geocode(query); + if (point) { + // Re-read the events fresh and upsert the FULL doc with _geo attached, + // keeping only those still present AND discoverable. A full-doc upsert is + // idempotent and needs no "is it indexed?" snapshot: it merges if the + // event is already indexed, lands a complete doc if not (never a {id,_geo} + // stub), and is identical to what the sink writes — so a concurrent sink + // write converges instead of clobbering. + const live = await fetchLiveDocs( + d1, + group.map((e) => e.uri) + ); + skippedGone += group.length - live.length; + if (live.length) { + if (!settingsApplied) { + await meili.applySettings(); + settingsApplied = true; + } + await meili.upsert( + live.map((r) => { + const doc = eventToSearchDoc({ + uri: r.uri, + did: r.did, + collection: EVENT_COLLECTION, + rkey: r.rkey, + record: r.record + }); + // Don't overwrite a coordinate _geo the record gained mid-run. + if (!doc._geo) doc._geo = { lat: point.lat, lng: point.lng }; + return doc; + }) + ); + } + // Write the positive cache row ONLY after the Meili write succeeds (or + // when there's no live doc to write). If applySettings/upsert throws, the + // catch below counts it transient and we do NOT cache — so the address + // stays eligible and retries next run, instead of being cached "resolved" + // yet never indexed: a permanent _geo gap the idempotent skip would lock + // in. A redundant re-geocode on the rare Meili failure is the cheap price. + await d1.query( + `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) + VALUES (?, ?, ?, ?, ?, ?, 0, NULL) + ON CONFLICT(address_norm) DO UPDATE SET + lat=excluded.lat, lng=excluded.lng, precision=excluded.precision, + source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=0, last_error=NULL`, + [norm, point.lat, point.lng, point.precision ?? null, source, now] + ); + resolved++; + } else { + // No-match: write/increment a negative row (backoff handled by isEligible). + await d1.query( + `INSERT INTO geocode_cache (address_norm, lat, lng, precision, source, geocoded_at, fail_count, last_error) + VALUES (?, NULL, NULL, NULL, ?, ?, 1, 'no match') + ON CONFLICT(address_norm) DO UPDATE SET + source=excluded.source, geocoded_at=excluded.geocoded_at, fail_count=geocode_cache.fail_count+1, last_error='no match'`, + [norm, source, now] + ); + negative++; + } + } catch (err) { + // Transient (HTTP/network): DON'T write a negative row — retry next run. + transient++; + warn(`transient error for "${query}": ${(err as Error).message}`); + } + // Throttle BETWEEN geocoder calls only — skip the trailing sleep after the + // last one, which would just waste a sleepMs at the end of every run. + if (i < capped.length - 1) await sleep(throttleMs); + } + + log( + `done resolved=${resolved} negative=${negative} transient=${transient} ` + + `skipped-gone=${skippedGone}` + ); + + return { + worklistEvents: events.length, + uniqueAddresses: byNorm.size, + eligible: work.length, + processed: capped.length, + resolved, + negative, + transient, + skippedGone + }; +} diff --git a/apps/web/src/lib/search/server/geocoder.test.ts b/apps/web/src/lib/search/server/geocoder.test.ts index c11ba58..6163fd4 100644 --- a/apps/web/src/lib/search/server/geocoder.test.ts +++ b/apps/web/src/lib/search/server/geocoder.test.ts @@ -4,7 +4,8 @@ import { addressToQuery, derivePrecision, createGeocoder, - requireGeocoderForBulk + requireGeocoderForBulk, + isPublicNominatimHost } from './geocoder'; describe('addressToQuery', () => { @@ -82,6 +83,20 @@ describe('createGeocoder', () => { expect(await createGeocoder({}, fn).geocode('nowhere')).toBeNull(); }); + it('returns null on out-of-WGS84-range coords (so a bad hit negative-caches, not poisons Meili)', async () => { + // A finite-but-out-of-range result must not be cached as resolved: Meili + // silently fails the whole _geo batch on such a doc. Reject → null → negative. + const { fn } = fakeFetch([{ lat: '91.5', lon: '-85.76', addresstype: 'city' }]); + expect(await createGeocoder({}, fn).geocode('off the map')).toBeNull(); + const { fn: fn2 } = fakeFetch([{ lat: '38.25', lon: '-200', addresstype: 'city' }]); + expect(await createGeocoder({}, fn2).geocode('off the map')).toBeNull(); + }); + + it('returns null on non-finite coords', async () => { + const { fn } = fakeFetch([{ lat: 'not-a-number', lon: '4.36', addresstype: 'city' }]); + expect(await createGeocoder({}, fn).geocode('garbled')).toBeNull(); + }); + it('throws on a transient HTTP error (429/5xx) so the caller retries', async () => { const { fn } = fakeFetch({}, 429); await expect(createGeocoder({}, fn).geocode('x')).rejects.toThrow(/429/); @@ -89,52 +104,119 @@ describe('createGeocoder', () => { }); describe('requireGeocoderForBulk', () => { - it('allows any run when a geocoder key is set', () => { + it('allows any run against a non-public endpoint (keyed LocationIQ or self-hosted)', () => { expect(() => - requireGeocoderForBulk({ hasKey: true, dryRun: false, limit: 0, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: true, + dryRun: false, + limit: 0, + allowPublic: false + }) ).not.toThrow(); expect(() => - requireGeocoderForBulk({ hasKey: true, dryRun: false, limit: 5000, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: true, + dryRun: false, + limit: 5000, + allowPublic: false + }) ).not.toThrow(); }); it('allows a keyless small drip (1..25)', () => { expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 25, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: 25, + allowPublic: false + }) ).not.toThrow(); expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 1, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: 1, + allowPublic: false + }) ).not.toThrow(); }); it('blocks a keyless run over the drip ceiling', () => { expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 26, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: 26, + allowPublic: false + }) ).toThrow(/LocationIQ|allow-public-nominatim/); }); it('blocks a keyless uncapped run (limit 0 = no cap, the worst case)', () => { expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 0, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: 0, + allowPublic: false + }) ).toThrow(/LocationIQ|allow-public-nominatim/); }); it('blocks a keyless negative limit (uncapped, not a tiny drip)', () => { // A stray `--limit -1` must not masquerade as a 1-call drip and slip the gate. expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: -1, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: -1, + allowPublic: false + }) ).toThrow(/LocationIQ|allow-public-nominatim/); }); it('lets --allow-public-nominatim override a bulk keyless run', () => { expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: false, limit: 0, allowPublic: true }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: false, + limit: 0, + allowPublic: true + }) ).not.toThrow(); }); it('never blocks a dry run (it makes no geocoder calls)', () => { expect(() => - requireGeocoderForBulk({ hasKey: false, dryRun: true, limit: 0, allowPublic: false }) + requireGeocoderForBulk({ + nonPublicEndpoint: false, + dryRun: true, + limit: 0, + allowPublic: false + }) ).not.toThrow(); }); }); + +describe('isPublicNominatimHost', () => { + it('flags the public OSM Nominatim host', () => { + expect(isPublicNominatimHost('https://nominatim.openstreetmap.org/search')).toBe(true); + }); + + it('does not flag a keyed LocationIQ endpoint', () => { + expect(isPublicNominatimHost('https://us1.locationiq.com/v1/search')).toBe(false); + }); + + it('treats unset or malformed URLs as public (fail safe toward Nominatim limits)', () => { + expect(isPublicNominatimHost(undefined)).toBe(true); + expect(isPublicNominatimHost('')).toBe(true); + expect(isPublicNominatimHost('not a url')).toBe(true); + }); + + it('is host-based, not substring-based (a path mentioning the host is not public)', () => { + expect(isPublicNominatimHost('https://geo.example.com/nominatim.openstreetmap.org')).toBe( + false + ); + }); +}); diff --git a/apps/web/src/lib/search/server/geocoder.ts b/apps/web/src/lib/search/server/geocoder.ts index c752e2b..595db97 100644 --- a/apps/web/src/lib/search/server/geocoder.ts +++ b/apps/web/src/lib/search/server/geocoder.ts @@ -6,6 +6,7 @@ // behind an interface. Default = public Nominatim (parity with atmo today); // set GEOCODER_KEY (+ GEOCODER_URL) to use LocationIQ. Used only by the // external geocode job — geocoding never runs on the Worker hot path. +import { inGeoRange } from './normalize'; export interface GeoPoint { lat: number; @@ -26,6 +27,25 @@ export interface GeocoderEnv { const DEFAULT_URL = 'https://nominatim.openstreetmap.org/search'; const DEFAULT_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; +const PUBLIC_NOMINATIM_HOST = 'nominatim.openstreetmap.org'; + +/** True when the effective geocoder endpoint is public OSM Nominatim — i.e. the + * shared, usage-policy-bound host. The in-Worker drip uses this to pick SAFE + * defaults (a smaller per-tick cap + a slower throttle floor) when on Nominatim, + * rather than refusing to run: the drip works keyless out of the box, just + * policy-compliantly. `createGeocoder` falls back to DEFAULT_URL (this host) + * when GEOCODER_URL is unset — even with a key present, in which case the ?key= + * is ignored and we're really on Nominatim — so an unset URL counts as public. + * Malformed URL → treated as public (fail safe toward the slower limits). A + * keyed, non-public GEOCODER_URL (LocationIQ) is the only way to lift them. */ +export function isPublicNominatimHost(url: string | undefined): boolean { + if (!url) return true; + try { + return new URL(url).hostname.toLowerCase() === PUBLIC_NOMINATIM_HOST; + } catch { + return true; + } +} // Same fixed order as the cache key, but a human-readable freeform query (commas, // original case/diacritics) — both Nominatim and LocationIQ handle UTF-8 / non- @@ -34,28 +54,40 @@ const DEFAULT_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; const QUERY_FIELDS = ['name', 'street', 'locality', 'region', 'postalCode', 'country']; /** Largest keyless run we treat as a sanctioned "small drip" against public - * Nominatim. Above this (or uncapped), a backfill must use LocationIQ. */ + * Nominatim. Above this (or uncapped), a backfill must use LocationIQ. The + * in-Worker drip also clamps its per-tick cap to this when on public Nominatim. */ export const PUBLIC_NOMINATIM_DRIP_MAX = 25; -/** Enforce the file's contract that a BULK backfill uses LocationIQ, not public - * Nominatim (whose usage policy a large unkeyed run would breach, risking a - * silent IP ban). Throws for a keyless, non-dry-run, non-overridden run that is - * uncapped (limit <= 0) or over the drip ceiling; a small keyless drip stays - * allowed. The hazard is request VOLUME, so the gate is on size, not on the - * mere use of Nominatim. limit <= 0 (not just === 0) counts as uncapped so a - * stray negative can't masquerade as a tiny drip and slip the gate. */ +/** Throttle floor (ms between calls) for public Nominatim — its usage policy is + * an absolute max of 1 request/second. The drip enforces this as a FLOOR when on + * Nominatim (a faster GEOCODE_SLEEP_MS override can't undercut the policy); a + * keyed LocationIQ endpoint honors the operator's own, possibly faster, value. */ +export const PUBLIC_NOMINATIM_MIN_SLEEP_MS = 1000; + +/** Enforce the file's contract that a BULK backfill runs against a NON-PUBLIC + * endpoint (a keyed LocationIQ URL, or a self-hosted host), not public OSM + * Nominatim — whose usage policy a large run would breach, risking a silent IP + * ban. The deciding input is the EFFECTIVE endpoint, not key presence: a + * GEOCODER_KEY with an unset/public GEOCODER_URL still hits public Nominatim (the + * ?key= is ignored), so callers pass `!isPublicNominatimHost(GEOCODER_URL)` here + * rather than `!!GEOCODER_KEY`. Throws for a public-Nominatim, non-dry-run, + * non-overridden run that is uncapped (limit <= 0) or over the drip ceiling; a + * small public drip stays allowed. The hazard is request VOLUME, so the gate is + * on size. limit <= 0 (not just === 0) counts as uncapped so a stray negative + * can't masquerade as a tiny drip and slip the gate. */ export function requireGeocoderForBulk(opts: { - hasKey: boolean; + nonPublicEndpoint: boolean; dryRun: boolean; limit: number; allowPublic: boolean; }): void { - if (opts.hasKey || opts.dryRun || opts.allowPublic) return; + if (opts.nonPublicEndpoint || opts.dryRun || opts.allowPublic) return; const bulk = opts.limit <= 0 || opts.limit > PUBLIC_NOMINATIM_DRIP_MAX; if (bulk) { throw new Error( `Keyless public Nominatim is only allowed for a small drip (--limit 1..${PUBLIC_NOMINATIM_DRIP_MAX}). ` + - 'Set GEOCODER_KEY (LocationIQ) for a bulk/uncapped backfill, or pass --allow-public-nominatim to override.' + 'Set GEOCODER_URL to your LocationIQ endpoint (with GEOCODER_KEY) for a bulk/uncapped backfill, ' + + 'or pass --allow-public-nominatim to override.' ); } } @@ -136,7 +168,14 @@ export function createGeocoder(env: GeocoderEnv = {}, fetchImpl: typeof fetch = const lat = Number(top.lat); const lng = Number(top.lon); - if (!Number.isFinite(lat) || !Number.isFinite(lng)) return null; + // Reject non-finite OR out-of-WGS84-range coordinates. Meilisearch + // silently fails the whole async index task on a bad _geo (losing the + // batch), and the sink guards its own path identically via the SAME + // inGeoRange (normalize.ts). An out-of-range hit therefore becomes a + // no-match (return null → negative cache w/ backoff) rather than a + // poisoned coordinate that's cached as "resolved" and never retried. + // inGeoRange returns false for NaN, so it subsumes the finite check. + if (!inGeoRange(lat, lng)) return null; return { lat, lng, precision: derivePrecision(top) }; } diff --git a/apps/web/src/lib/search/server/normalize.ts b/apps/web/src/lib/search/server/normalize.ts index ab9e610..0dd0e63 100644 --- a/apps/web/src/lib/search/server/normalize.ts +++ b/apps/web/src/lib/search/server/normalize.ts @@ -54,7 +54,11 @@ type Loc = Record; // finite-but-out-of-range value. Meilisearch rejects such a doc and can fail // the whole async indexing task (losing the rest of the batch), so we drop the // _geo here rather than index it. Bounds are WGS84: lat [-90, 90], lng [-180, 180]. -function inGeoRange(lat: number, lng: number): boolean { +// Exported so the geocoder (geocoder.ts) range-checks its results against the +// EXACT same bounds — a geocoder hit and a record's own coords must not be able +// to disagree on what Meili will accept. NaN fails every comparison, so this also +// rejects non-finite input. +export function inGeoRange(lat: number, lng: number): boolean { return lat >= -90 && lat <= 90 && lng >= -180 && lng <= 180; } diff --git a/apps/web/src/routes/api/cron/+server.ts b/apps/web/src/routes/api/cron/+server.ts index 9e65774..6f19117 100644 --- a/apps/web/src/routes/api/cron/+server.ts +++ b/apps/web/src/routes/api/cron/+server.ts @@ -1,6 +1,7 @@ import { contrail, ensureInit } from '$lib/contrail/index'; import { processBotMentions } from '$lib/bot/process-mentions'; import { runNotifications } from '$lib/notify/process'; +import { runGeocodeDrip } from '$lib/geocode/process'; import type { RequestHandler } from './$types'; export const POST: RequestHandler = async ({ request, platform }) => { @@ -60,5 +61,15 @@ export const POST: RequestHandler = async ({ request, platform }) => { console.error('[cron] runNotifications failed:', e); } + // Address→_geo geocode drip: resolves coordinates for newly-ingested + // address-only events so they appear in /near-me. Self-throttled to ~30 min + // via a D1 gate (rides this every-minute cron); no-ops when the geocoder/sink + // isn't configured. Isolated so a geocoder/Meili hiccup can't 500 the tick. + try { + await runGeocodeDrip(platform!.env, db); + } catch (e) { + console.error('[cron] runGeocodeDrip failed:', e); + } + return new Response('OK'); }; -- 2.51.2 From 61a81771c3015a9a70ed99c51fc85fda7951b32d Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Sat, 27 Jun 2026 19:12:34 -0400 Subject: [PATCH 3/4] docs(geocode): document near-me geocoding for operators; drop redundant DDL file Add a 'Near-me geocoding' section to the README (provider choice, the cron drip, rate-limit caveats, and the geocode:backfill CLI) and the GEOCODER_* / GEOCODE_SLEEP_MS vars to .env.example. Remove apps/web/scripts/geocode-cache.sql: geocode_cache is defined and self-healed in code (geocode-job.ts), matching this repo's all-in-code D1 schema convention (notify/db.ts, bot/db.ts, geocode/db.ts). The standalone DDL was a redundant, convention-breaking outlier; update the stale comment that pointed at it. --- README.md | 10 ++++++++++ apps/web/.env.example | 11 +++++++++++ apps/web/scripts/geocode-cache.sql | 14 -------------- apps/web/src/lib/search/server/geocode-job.ts | 5 +++-- 4 files changed, 24 insertions(+), 16 deletions(-) delete mode 100644 apps/web/scripts/geocode-cache.sql diff --git a/README.md b/README.md index dc94418..cf7a81e 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,16 @@ the read and write keys are kept separate on purpose so the browser-facing read **populating the index.** backfill and refresh now feed the sink, so `pnpm backfill` fills meili as it walks each user's pds. on an existing deployment the event records are usually already in d1, so `pnpm meili:reindex` is faster: it replays the stored `community.lexicon.calendar.event` rows straight from d1 into the index with no network walk and no d1 writes. both paths apply the same discoverable filter as live ingest, and the sink applies the index settings on its first write, so a fresh index gets the right filterable fields and re-running either is idempotent. `pnpm meili:reindex:remote` targets the deployed d1 and needs the same wrangler `env.production` that `pnpm backfill:remote` uses. +### Near-me geocoding (optional) + +Many events carry only a street address, no coordinates — so they never surface in near-me, which filters on the Meilisearch document's `_geo`. This resolves those addresses to coordinates and writes `_geo` back into the same index, making address-only events near-me-visible. It layers on top of the sink above: no extra service — it rides the existing cron and writes the same index. Leave it untouched and it runs keyless against public [Nominatim](https://nominatim.org/) at a safe trickle; until an address resolves, that event simply stays out of near-me. + +**How it runs.** The cron already calls a geocode "drip" every minute; it self-throttles to once per ~30 min via a D1 marker, resolves up to 50 new addresses per run (25 on public Nominatim), and `PATCH`es `_geo` into Meilisearch. Every result is cached — including negative results, so an ungeocodable address isn't retried every run. There is nothing to set up: like the app's other D1 tables, the geocode cache and its cadence marker are defined in code and self-heal on first run. The drip no-ops entirely until the **write sink** above is configured, so enabling search is the only switch. + +**Picking a geocoder.** The default is keyless public OSM Nominatim — fine for the steady-state drip's low volume. Set `GEOCODER_USER_AGENT` to a string identifying your deployment (Nominatim's [usage policy](https://operations.osmfoundation.org/policies/nominatim/) requires a real contact; on the public host the per-run cap is held to 25 and the throttle floored to ≥1 req/s). For real volume — and for the bulk backfill below — use [LocationIQ](https://locationiq.com/) (an API-compatible hosted Nominatim): set `GEOCODER_URL=https://us1.locationiq.com/v1/search` and `GEOCODER_KEY`, which lifts the per-run cap to 50 and honors your `GEOCODE_SLEEP_MS` (minimum ms between calls, default 1100). A key with an unset or public `GEOCODER_URL` is *ignored* — you stay on public Nominatim — so always set the URL too. + +**Backfilling an existing corpus.** The drip only trickles, so to resolve a backlog run the off-Cloudflare CLI against the deployed D1: `pnpm -C apps/web geocode:backfill --limit 50`. It reaches D1 over the REST API, so it needs `CLOUDFLARE_ACCOUNT_ID` / `CLOUDFLARE_API_TOKEN` / `D1_DATABASE_ID` and `MEILI_URL` / `MEILI_KEY` (plus `SEARCH_INDEX` if not `events`), and a LocationIQ `GEOCODER_URL` / `GEOCODER_KEY`. It refuses a bulk or uncapped run against public Nominatim (keyless is capped to `--limit 1..25`). Useful flags: `--limit N` (`0` = no cap), `--dry-run`, `--retry-negative` (re-attempt negatively-cached addresses), and `--allow-public-nominatim` (override the public-host guard). Like search itself, geocoding only helps once the sink is feeding the index, so run this after the rollout steps above. + ## contributing open for contributions by all :) diff --git a/apps/web/.env.example b/apps/web/.env.example index 6d816d2..7e99bf0 100644 --- a/apps/web/.env.example +++ b/apps/web/.env.example @@ -19,3 +19,14 @@ COOKIE_SECRET= # The index is SEARCH_INDEX above (the sink writes the same index it reads). # SEARCH_SINK_URL=http://localhost:7700 # SEARCH_SINK_API_KEY= +# Near-me geocoding (optional; layered on the search sink above). Resolves the +# coordinates of address-only events and writes _geo into the index so they +# appear in near-me. Unset → the in-cron drip runs keyless against public OSM +# Nominatim at a safe trickle. See the "near-me geocoding" section in the README. +# For real volume / a bulk backfill, use LocationIQ (set both URL and KEY): +# GEOCODER_URL=https://us1.locationiq.com/v1/search +# GEOCODER_KEY= +# Identify your deployment to public Nominatim (its usage policy requires it): +# GEOCODER_USER_AGENT=atmo-events (you@example.com) +# Minimum ms between geocoder calls (default 1100; floored to 1000 on public Nominatim): +# GEOCODE_SLEEP_MS=1100 diff --git a/apps/web/scripts/geocode-cache.sql b/apps/web/scripts/geocode-cache.sql deleted file mode 100644 index 20c5f23..0000000 --- a/apps/web/scripts/geocode-cache.sql +++ /dev/null @@ -1,14 +0,0 @@ --- Derived-coordinate cache for address-only events + the geocode job's worklist --- and done-marker (Meili can't filter "missing _geo", so we track resolution --- ourselves). Idempotent / restartable. Applied once to the openmeet-atmo D1; --- the sink reads it, the external geocode job writes it. -CREATE TABLE IF NOT EXISTS geocode_cache ( - address_norm TEXT PRIMARY KEY, -- normalized address key (address-norm.ts) - lat REAL, -- NULL when unresolved (negative cache) - lng REAL, - precision TEXT, -- provider-reported granularity - source TEXT NOT NULL, -- e.g. 'locationiq' / 'nominatim' - geocoded_at INTEGER NOT NULL, -- epoch ms - fail_count INTEGER NOT NULL DEFAULT 0, - last_error TEXT -); diff --git a/apps/web/src/lib/search/server/geocode-job.ts b/apps/web/src/lib/search/server/geocode-job.ts index 6f70140..5d24389 100644 --- a/apps/web/src/lib/search/server/geocode-job.ts +++ b/apps/web/src/lib/search/server/geocode-job.ts @@ -146,8 +146,9 @@ export async function runGeocodeJob(opts: GeocodeJobOptions): Promise= 0 ? sleepMs : 1100; - // Defensive: the table is created by geocode-cache.sql, but a fresh DB - // shouldn't make the job crash before it can self-heal. + // This CREATE is the authoritative definition of geocode_cache: like the + // app's other D1 tables it lives in code and self-heals, so a fresh DB just + // works on first run with nothing to apply by hand. await d1.query( `CREATE TABLE IF NOT EXISTS geocode_cache ( address_norm TEXT PRIMARY KEY, lat REAL, lng REAL, precision TEXT, -- 2.51.2 From ef4b564345ff440c01a44530a12e33fef710b0f9 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Mon, 29 Jun 2026 07:42:20 -0400 Subject: [PATCH 4/4] refactor(geocode): source near-me URL + UA from shared geocoder module (om-slun) geocode.ts hardcoded NOMINATIM_SEARCH_URL + USER_AGENT, duplicating literals that already have a canonical home in geocoder.ts. Converge on one source so they can't drift: - geocoder.ts: export DEFAULT_GEOCODER_URL + DEFAULT_GEOCODER_USER_AGENT and add resolveGeocoderUserAgent(env) (honors GEOCODER_USER_AGENT); createGeocoder now uses the resolver too. - geocode.ts: drop its own literals; import the shared URL + UA helper; accept an optional env so the near-me box honors GEOCODER_USER_AGENT. Stays on public Nominatim deliberately (does NOT adopt GEOCODER_URL: that may be a keyed LocationIQ endpoint and this hot-path forward search sends no ?key=, so it would 401 on the live config). - geocode.remote.ts: thread platform.env into geocodeLocation. - geocode.test.ts: assert against the shared source + cover the UA env override. Lands on the open search PR (#55) before it merges upstream, per om-slun. --- apps/web/src/lib/search/geocode.remote.ts | 5 ++-- .../web/src/lib/search/server/geocode.test.ts | 23 ++++++++++++++++--- apps/web/src/lib/search/server/geocode.ts | 17 +++++++++----- apps/web/src/lib/search/server/geocoder.ts | 22 ++++++++++++++---- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/apps/web/src/lib/search/geocode.remote.ts b/apps/web/src/lib/search/geocode.remote.ts index d5ca806..2016ded 100644 --- a/apps/web/src/lib/search/geocode.remote.ts +++ b/apps/web/src/lib/search/geocode.remote.ts @@ -1,4 +1,4 @@ -import { command } from '$app/server'; +import { command, getRequestEvent } from '$app/server'; import { error } from '@sveltejs/kit'; import { geocodeLocation } from './server/geocode'; import { geocodeInput } from './geocode-input'; @@ -8,8 +8,9 @@ import { geocodeInput } from './geocode-input'; * geocoder sees the Worker, not the user. Returns null when nothing * matches (distinct from upstream failure, which is a 502). */ export const geocodeQuery = command(geocodeInput, async ({ q }) => { + const { platform } = getRequestEvent(); try { - return await geocodeLocation(q); + return await geocodeLocation(q, fetch, platform?.env); } catch { error(502, 'Location lookup is unavailable right now'); } diff --git a/apps/web/src/lib/search/server/geocode.test.ts b/apps/web/src/lib/search/server/geocode.test.ts index d72aeb2..e9d15ea 100644 --- a/apps/web/src/lib/search/server/geocode.test.ts +++ b/apps/web/src/lib/search/server/geocode.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { geocodeLocation } from './geocode'; +import { DEFAULT_GEOCODER_URL, DEFAULT_GEOCODER_USER_AGENT } from './geocoder'; function fakeFetch(status: number, body: unknown) { return vi.fn(async () => new Response(JSON.stringify(body), { status })); @@ -20,13 +21,18 @@ describe('geocodeLocation', () => { expect(fetchImpl).toHaveBeenCalledTimes(1); const [input, init] = fetchImpl.mock.calls[0] as unknown as [URL | string, RequestInit]; const url = new URL(String(input)); + // URL is sourced from the shared geocoder constant, not a local literal. + const shared = new URL(DEFAULT_GEOCODER_URL); + expect(url.hostname).toBe(shared.hostname); + expect(url.pathname).toBe(shared.pathname); expect(url.hostname).toBe('nominatim.openstreetmap.org'); - expect(url.pathname).toBe('/search'); expect(url.searchParams.get('q')).toBe('Louisville, KY'); expect(url.searchParams.get('format')).toBe('jsonv2'); expect(url.searchParams.get('limit')).toBe('1'); - // Nominatim's usage policy requires an identifying User-Agent. - expect(new Headers(init.headers).get('user-agent')).toMatch(/atmo/i); + // Nominatim's usage policy requires an identifying User-Agent; default + // comes from the shared env-driven helper. + expect(new Headers(init.headers).get('user-agent')).toBe(DEFAULT_GEOCODER_USER_AGENT); + expect(DEFAULT_GEOCODER_USER_AGENT).toMatch(/atmo/i); expect(result).toEqual({ lat: 38.2542, @@ -35,6 +41,17 @@ describe('geocodeLocation', () => { }); }); + it('sources the User-Agent from GEOCODER_USER_AGENT when set', async () => { + const fetchImpl = fakeFetch(200, louisville); + + await geocodeLocation('Louisville, KY', fetchImpl, { + GEOCODER_USER_AGENT: 'custom-agent/9.9 (https://example.test)' + }); + + const [, init] = fetchImpl.mock.calls[0] as unknown as [URL | string, RequestInit]; + expect(new Headers(init.headers).get('user-agent')).toBe('custom-agent/9.9 (https://example.test)'); + }); + it('returns null when nothing matches', async () => { expect(await geocodeLocation('zzzz no such place', fakeFetch(200, []))).toBeNull(); }); diff --git a/apps/web/src/lib/search/server/geocode.ts b/apps/web/src/lib/search/server/geocode.ts index 22156cd..f2599f2 100644 --- a/apps/web/src/lib/search/server/geocode.ts +++ b/apps/web/src/lib/search/server/geocode.ts @@ -6,6 +6,13 @@ // requirement is satisfied by the OpenStreetMap credit shown on the near-me // page. Heavier-traffic compliance (app-wide rate limiting, caching) is still // a follow-up before high-volume exposure. +// +// Base URL and User-Agent come from the shared geocoder module (one canonical +// source, no duplicate literals). This path stays on public Nominatim — it does +// NOT honor a configured GEOCODER_URL, because that endpoint may be a keyed +// LocationIQ URL and this hot-path forward search sends no ?key= (it would 401). +import { DEFAULT_GEOCODER_URL, resolveGeocoderUserAgent, type GeocoderEnv } from './geocoder'; + export type GeocodeResult = { lat: number; lng: number; @@ -13,20 +20,18 @@ export type GeocodeResult = { label: string; }; -const NOMINATIM_SEARCH_URL = 'https://nominatim.openstreetmap.org/search'; -const USER_AGENT = 'atmo-events (https://atmo.rsvp)'; - export async function geocodeLocation( q: string, - fetchImpl: typeof fetch = fetch + fetchImpl: typeof fetch = fetch, + env: GeocoderEnv = {} ): Promise { - const url = new URL(NOMINATIM_SEARCH_URL); + const url = new URL(DEFAULT_GEOCODER_URL); url.searchParams.set('q', q); url.searchParams.set('format', 'jsonv2'); url.searchParams.set('limit', '1'); const response = await fetchImpl(url, { - headers: { accept: 'application/json', 'user-agent': USER_AGENT } + headers: { accept: 'application/json', 'user-agent': resolveGeocoderUserAgent(env) } }); if (!response.ok) { throw new Error(`geocode request failed: ${response.status}`); diff --git a/apps/web/src/lib/search/server/geocoder.ts b/apps/web/src/lib/search/server/geocoder.ts index 595db97..6e6cf54 100644 --- a/apps/web/src/lib/search/server/geocoder.ts +++ b/apps/web/src/lib/search/server/geocoder.ts @@ -25,15 +25,27 @@ export interface GeocoderEnv { GEOCODER_USER_AGENT?: string; } -const DEFAULT_URL = 'https://nominatim.openstreetmap.org/search'; -const DEFAULT_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; +/** Public OSM Nominatim /search endpoint — the keyless default and the single + * source of the base geocoder URL. The near-me search box (geocode.ts) imports + * this so the two paths can't drift onto different hardcoded literals. */ +export const DEFAULT_GEOCODER_URL = 'https://nominatim.openstreetmap.org/search'; +/** Default identifying User-Agent. Nominatim's usage policy requires one; an + * operator can override it via GEOCODER_USER_AGENT (see resolveGeocoderUserAgent). */ +export const DEFAULT_GEOCODER_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; const PUBLIC_NOMINATIM_HOST = 'nominatim.openstreetmap.org'; +/** Resolve the geocoder User-Agent: operator override (GEOCODER_USER_AGENT) else + * the atmo default. The single env-driven source both the bulk client and the + * near-me search box use, so the UA can't drift across paths. */ +export function resolveGeocoderUserAgent(env: GeocoderEnv = {}): string { + return env.GEOCODER_USER_AGENT || DEFAULT_GEOCODER_USER_AGENT; +} + /** True when the effective geocoder endpoint is public OSM Nominatim — i.e. the * shared, usage-policy-bound host. The in-Worker drip uses this to pick SAFE * defaults (a smaller per-tick cap + a slower throttle floor) when on Nominatim, * rather than refusing to run: the drip works keyless out of the box, just - * policy-compliantly. `createGeocoder` falls back to DEFAULT_URL (this host) + * policy-compliantly. `createGeocoder` falls back to DEFAULT_GEOCODER_URL (this host) * when GEOCODER_URL is unset — even with a key present, in which case the ?key= * is ignored and we're really on Nominatim — so an unset URL counts as public. * Malformed URL → treated as public (fail safe toward the slower limits). A @@ -139,9 +151,9 @@ export function derivePrecision(top: { } export function createGeocoder(env: GeocoderEnv = {}, fetchImpl: typeof fetch = fetch): Geocoder { - const base = env.GEOCODER_URL || DEFAULT_URL; + const base = env.GEOCODER_URL || DEFAULT_GEOCODER_URL; const key = env.GEOCODER_KEY; - const userAgent = env.GEOCODER_USER_AGENT || DEFAULT_USER_AGENT; + const userAgent = resolveGeocoderUserAgent(env); return { async geocode(q: string): Promise { -- 2.51.2