From 4af15aa6a9907f2a655a08e1b363af4cd159bfff Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 1 Oct 2026 15:17:07 -0700 Subject: [PATCH] fix: keep a named place's own point in situation briefs Brief sections received a named place as its bounding box, so they centered on the box's middle: Tokyo's aircraft were sorted from a point in the Pacific. Sections now receive the place name and resolve it like a direct query, and place lookups are cached briefly so a brief resolves it once. --- scripts/format-scope.json | 3 +- src/tools/places.js | 47 +++++++++++++++++++++++--- src/tools/places.test.mjs | 39 +++++++++++++++++++++ src/tools/queries/brief.js | 42 ++++++++++++++++------- src/tools/queries/environment.test.mjs | 27 +++++++++++++-- 5 files changed, 137 insertions(+), 21 deletions(-) create mode 100644 src/tools/places.test.mjs diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 9fd4c4a4c..580988971 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -887,5 +887,6 @@ "src/tools/queries/infrastructure.test.mjs", "src/tools/queries/events.test.mjs", "src/tools/functions.test.mjs", - "src/tools/surfaces.test.mjs" + "src/tools/surfaces.test.mjs", + "src/tools/places.test.mjs" ] diff --git a/src/tools/places.js b/src/tools/places.js index e7f4f8084..97b4ce8f1 100644 --- a/src/tools/places.js +++ b/src/tools/places.js @@ -20,18 +20,55 @@ const POINT_RADIUS_DEGREES = 0.25; */ export function createGeocodePlaceService({ fetchImpl = (...args) => globalThis.fetch(...args), + cacheSize = 200, + cacheMs = 10 * 60_000, + now = () => Date.now(), } = {}) { + // Recent answers by name, so tools that combine others resolve a place to + // the same point once instead of once per section. + const cache = new Map(); + async function lookup(name) { + const query = new URLSearchParams({ q: name }); + const response = await fetchImpl(`/api/geocode?${query}`); + if (!response.ok) throw new Error(`Geocode HTTP ${response.status}`); + const payload = await response.json(); + return placeFromGeocodeResult(payload?.results?.[0]); + } return { async resolve(name, { signal } = {}) { - const query = new URLSearchParams({ q: name }); - const response = await fetchImpl(`/api/geocode?${query}`, { signal }); - if (!response.ok) throw new Error(`Geocode HTTP ${response.status}`); - const payload = await response.json(); - return placeFromGeocodeResult(payload?.results?.[0]); + signal?.throwIfAborted(); + const key = name.trim().toLowerCase(); + const hit = cache.get(key); + if (hit && now() - hit.at < cacheMs) { + cache.delete(key); + cache.set(key, hit); + return waitFor(hit.place, signal); + } + const place = lookup(name); + const entry = { at: now(), place }; + cache.set(key, entry); + while (cache.size > cacheSize) cache.delete(cache.keys().next().value); + place.catch(() => { + if (cache.get(key) === entry) cache.delete(key); + }); + return waitFor(place, signal); }, }; } +/** Settle with `promise`, or reject when `signal` aborts first. */ +function waitFor(promise, signal) { + if (!signal) return promise; + signal.throwIfAborted(); + return new Promise((resolve, reject) => { + const abort = () => reject(signal.reason); + signal.addEventListener('abort', abort, { once: true }); + promise + .then(resolve, reject) + .finally(() => signal.removeEventListener('abort', abort)); + }); +} + /** Convert one geocode result to a place with bounds, or null. */ export function placeFromGeocodeResult(result) { const location = result?.geometry?.location; diff --git a/src/tools/places.test.mjs b/src/tools/places.test.mjs new file mode 100644 index 000000000..63fdeb28a --- /dev/null +++ b/src/tools/places.test.mjs @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createGeocodePlaceService } from './places.js'; + +const answer = { + results: [ + { + formatted_address: 'Tokyo, Japan', + geometry: { location: { lat: 35.68, lng: 139.76 } }, + }, + ], +}; + +test('place lookups are cached by name for a while, and failures are not', async () => { + let clock = 0; + const requests = []; + let fail = true; + const places = createGeocodePlaceService({ + now: () => clock, + cacheMs: 1000, + fetchImpl: async (url) => { + requests.push(url); + if (fail) return new Response(null, { status: 503 }); + return Response.json(answer); + }, + }); + await assert.rejects(places.resolve('Tokyo'), /Geocode HTTP 503/); + fail = false; + const first = await places.resolve('Tokyo'); + assert.deepEqual(first.point, { lat: 35.68, lon: 139.76 }); + assert.equal(await places.resolve(' tokyo '), first); + assert.equal(requests.length, 2); + clock = 2000; + await places.resolve('Tokyo'); + assert.equal(requests.length, 3); + const aborted = new AbortController(); + aborted.abort(); + await assert.rejects(places.resolve('Tokyo', { signal: aborted.signal })); +}); diff --git a/src/tools/queries/brief.js b/src/tools/queries/brief.js index 750cd1dfb..cf6161693 100644 --- a/src/tools/queries/brief.js +++ b/src/tools/queries/brief.js @@ -21,7 +21,9 @@ const SECTIONS = [ label: 'Weather', tool: 'get_weather', args: (area, center) => ({ - location: { lat: center.lat, lon: center.lon }, + location: area.argument.place + ? { place: area.argument.place } + : { lat: center.lat, lon: center.lon }, }), }, { @@ -93,19 +95,30 @@ function sectionFeedState(section) { : 'nominal'; } -/** The resolved area, plus the argument sections receive in its place. */ -function sectionArea(resolved) { +/** + * The resolved area, plus the argument sections receive. A named place is + * passed by name, so sections resolve it to the same point and label as a + * direct query does (place lookups are cached). + */ +function sectionArea(resolved, original) { return { ...resolved, - argument: resolved.center - ? { - lat: resolved.center.lat, - lon: resolved.center.lon, - radius_km: resolved.center.radiusKm, - } - : { - bbox: [resolved.west, resolved.south, resolved.east, resolved.north], - }, + argument: original?.place + ? { place: original.place } + : resolved.center + ? { + lat: resolved.center.lat, + lon: resolved.center.lon, + radius_km: resolved.center.radiusKm, + } + : { + bbox: [ + resolved.west, + resolved.south, + resolved.east, + resolved.north, + ], + }, }; } @@ -143,7 +156,10 @@ async function runSections(all, area, { tools, signal }) { async function buildBrief(args, { services, signal, tools }) { // Sections receive the resolved box so a place name is looked up once. - const area = sectionArea(await resolveArea(args.area, { services, signal })); + const area = sectionArea( + await resolveArea(args.area, { services, signal }), + args.area, + ); const { center, answers, lines } = await runSections(SECTIONS, area, { tools, signal, diff --git a/src/tools/queries/environment.test.mjs b/src/tools/queries/environment.test.mjs index b79f578d3..2a7a82982 100644 --- a/src/tools/queries/environment.test.mjs +++ b/src/tools/queries/environment.test.mjs @@ -421,8 +421,8 @@ test('the situation brief combines available sections and marks failures', async }); assert.equal( result.summary, - 'Situation in Austin, Texas: Weather at 30.3100, -97.7500: overcast, 26°C, wind 5 km/h. ' + - '1 earthquake of M2.5+ in the last 24 hours in the requested box; strongest M3.1 near Austin.', + 'Situation in Austin, Texas: Weather at Austin, Texas: overcast, 26°C, wind 5 km/h. ' + + '1 earthquake of M2.5+ in the last 24 hours in Austin, Texas; strongest M3.1 near Austin.', ); assert.deepEqual(Object.keys(result.data.sections), [ 'weather', @@ -642,3 +642,26 @@ test('the HUD caption context carries each section feed state', async () => { assert.equal(sent[0].feedProvenance.overall, 'unavailable'); assert.match(sent[0].feedProvenance.note, /Aircraft is STALE/); }); + +test('brief sections use a named place at its own point', async () => { + const tokyo = { + resolve: async () => ({ + name: 'Tokyo, Japan', + point: { lat: 35.6769, lon: 139.7639 }, + bounds: { west: 135, south: 20, east: 155, north: 36 }, + }), + }; + const queries = []; + const aircraft = { + getSnapshot: async (query) => { + queries.push(query); + return { records: [], complete: true, source: 'Test feed' }; + }, + }; + const result = await catalogWith({ weather, places: tokyo, aircraft }).call( + 'situation_brief', + { area: { place: 'Tokyo' } }, + ); + assert.deepEqual(queries[0], { latitude: 35.6769, longitude: 139.7639 }); + assert.match(result.data.sections.aircraft.summary, /in Tokyo, Japan\.$/); +});