From 7ddee426e7265f4de6dd052a0c3df67f572eaaa8 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 1 Oct 2026 14:39:57 -0700 Subject: [PATCH] fix: count every mapped installation in tool answers The installation source keeps one named site per display cell for the map. Queries now ask for every site and the exact box, and report when the source returned only part of the mapped sites. --- src/data/militaryNames.js | 14 +++++++++++--- src/data/militaryNames.test.mjs | 10 ++++++++++ src/layers/installations/source.js | 22 ++++++++++++++-------- src/tools/queries/environment.js | 14 ++++++++++++-- src/tools/queries/environment.test.mjs | 17 ++++++++++++++++- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/data/militaryNames.js b/src/data/militaryNames.js index 8e9e9a5c2..3f0371bd9 100644 --- a/src/data/militaryNames.js +++ b/src/data/militaryNames.js @@ -93,8 +93,16 @@ export function nameMilitaryFragment(fragment, names) { }; } -/** Area-ranked spatial thinning of visible label points, including dateline views. */ -export function militaryNamesInView(names, box, cap = MILITARY_POINT_CAP) { +/** + * Area-ranked spatial thinning of visible label points, including dateline + * views. With `thinned` false every point in the box is kept, up to `cap`. + */ +export function militaryNamesInView( + names, + box, + cap = MILITARY_POINT_CAP, + thinned = true, +) { const width = box.east >= box.west ? box.east - box.west : 360 + box.east - box.west; const height = box.north - box.south; @@ -107,7 +115,7 @@ export function militaryNamesInView(names, box, cap = MILITARY_POINT_CAP) { continue; count++; const cell = `${Math.min(31, Math.floor((x / width) * 32))}:${Math.min(15, Math.floor(((record.latitude - box.south) / height) * 16))}`; - if (cells.has(cell) || records.length >= cap) continue; + if ((thinned && cells.has(cell)) || records.length >= cap) continue; cells.add(cell); records.push(record); } diff --git a/src/data/militaryNames.test.mjs b/src/data/militaryNames.test.mjs index bdc4449c8..7e52b2c60 100644 --- a/src/data/militaryNames.test.mjs +++ b/src/data/militaryNames.test.mjs @@ -217,3 +217,13 @@ test('a close view keeps the parent name inside its polygon when the usual label hole, ); }); + +test('queries can take every named site in a box instead of one per display cell', async () => { + const names = await loadMilitaryNames(); + const box = { west: -2, south: 50, east: 2, north: 54 }; + const thinned = militaryNamesInView(names, box); + const all = militaryNamesInView(names, box, Infinity, false); + assert.equal(all.count, thinned.count); + assert.equal(all.records.length, all.count); + assert.ok(all.records.length > thinned.records.length); +}); diff --git a/src/layers/installations/source.js b/src/layers/installations/source.js index 7cad3fca0..fed73e87a 100644 --- a/src/layers/installations/source.js +++ b/src/layers/installations/source.js @@ -101,7 +101,7 @@ export function createInstallationSource({ } = {}) { let overpassUnavailable = false; const installationIds = new Map(); - async function getNamedSites(box, signal) { + async function getNamedSites(box, signal, thinned = true) { const names = await waitForSignal(loadNames(), signal).catch((cause) => { signal?.throwIfAborted(); throw Object.assign( @@ -112,7 +112,13 @@ export function createInstallationSource({ ); }); signal?.throwIfAborted(); - const { records, count } = militaryNamesInView(names, box); + // The map keeps one site per display cell; queries can ask for all. + const { records, count } = militaryNamesInView( + names, + box, + thinned ? undefined : Infinity, + thinned, + ); return { records, namedInView: count, @@ -124,9 +130,9 @@ export function createInstallationSource({ source: 'OpenStreetMap', }; } - async function getTileSites(box, signal) { + async function getTileSites(box, signal, thinned) { const zoom = installationTileZoom(box); - if (zoom === null) return getNamedSites(box, signal); + if (zoom === null) return getNamedSites(box, signal, thinned); let names = null; const namesJob = waitForSignal(loadNames(), signal) .then((loaded) => { @@ -181,7 +187,7 @@ export function createInstallationSource({ mapTiles.clear(); installationIds.clear(); }, - async getMappedSites(box, { exact = false, signal } = {}) { + async getMappedSites(box, { exact = false, thinned = true, signal } = {}) { const { south, west, north, east } = box || {}; if ( ![south, west, north, east].every(Number.isFinite) || @@ -197,8 +203,8 @@ export function createInstallationSource({ throw new TypeError('A bounded installation viewport is required'); signal?.throwIfAborted(); if (east < west || north - south > 10 || east - west > 10) - return getNamedSites(box, signal); - if (overpassUnavailable) return getTileSites(box, signal); + return getNamedSites(box, signal, thinned); + if (overpassUnavailable) return getTileSites(box, signal, thinned); const query = new URLSearchParams( Object.entries({ south, west, north, east }).map(([key, value]) => [ key, @@ -213,7 +219,7 @@ export function createInstallationSource({ signal?.throwIfAborted(); if (isUnavailableCapability(body)) { overpassUnavailable = true; - return getTileSites(box, signal); + return getTileSites(box, signal, thinned); } if (!response.ok) throw Object.assign( diff --git a/src/tools/queries/environment.js b/src/tools/queries/environment.js index 8e053f2e4..7def1f87d 100644 --- a/src/tools/queries/environment.js +++ b/src/tools/queries/environment.js @@ -355,8 +355,12 @@ export const findMilitaryInstallations = defineTool({ ); const center = areaCenter(area); const result = await services.installations.getMappedSites(area, { + exact: true, + thinned: false, signal, }); + // A saturated source returned only part of the mapped sites. + const complete = !result.saturated; const rows = (result.records || []) .map((site) => ({ site, @@ -374,8 +378,14 @@ export const findMilitaryInstallations = defineTool({ })) .sort((a, b) => a.distance_km - b.distance_km); return { - summary: `${countNoun(rows.length, 'mapped military installation')} in ${area.label}.`, - data: { ...capRows(rows, args.limit), source: result.source ?? null }, + summary: + `${countNoun(rows.length, 'mapped military installation')} in ${area.label}` + + (complete ? '.' : ' (partial: the source returned only some sites).'), + data: { + ...capRows(rows, args.limit), + complete, + source: result.source ?? null, + }, }; }, }); diff --git a/src/tools/queries/environment.test.mjs b/src/tools/queries/environment.test.mjs index 3635ff207..40fdd4793 100644 --- a/src/tools/queries/environment.test.mjs +++ b/src/tools/queries/environment.test.mjs @@ -263,10 +263,14 @@ test('terrain heights are returned in request order', async () => { test('military installations need a bounded area and are nearest first', async () => { const requested = []; + const options = []; + let saturated = false; const installations = { - async getMappedSites(box) { + async getMappedSites(box, { signal, ...rest } = {}) { requested.push(box); + options.push(rest); return { + saturated, source: 'OpenStreetMap', records: [ { @@ -302,7 +306,18 @@ test('military installations need a bounded area and are nearest first', async ( ['osm:2', 'osm:1'], ); assert.equal(result.data.source, 'OpenStreetMap'); + assert.equal(result.data.complete, true); assert.ok(requested[0].north - requested[0].south < 1); + assert.deepEqual(options[0], { exact: true, thinned: false }); + saturated = true; + const partial = await catalog.call('find_military_installations', { + area: { lat: 32.7, lon: -117.2, radius_km: 20 }, + }); + assert.equal(partial.data.complete, false); + assert.match( + partial.summary, + /\(partial: the source returned only some sites\)\.$/, + ); await assert.rejects( catalog.call('find_military_installations', { area: { bbox: [-130, 20, -100, 50] },