From f610de77dcc07b6ed17468cbca2627f533643b35 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 1 Oct 2026 16:23:30 -0700 Subject: [PATCH] fix: judge camera catalog trimming by region, not by returned cameras Coverage was checked only for packs among the cameras found, so an area whose cameras were all trimmed away answered zero cameras, complete. The catalog now reports where each trimmed pack's cameras are, and the tool marks an answer partial whenever its area overlaps that region. --- server/providers/cctv/catalog.js | 5 +++- server/providers/cctv/normalize.js | 23 ++++++++++++++++- src/data/cctvCatalogCap.test.mjs | 11 +++++--- src/tools/queries/media.js | 21 +++++++++++++++- src/tools/queries/media.test.mjs | 40 ++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 6 deletions(-) diff --git a/server/providers/cctv/catalog.js b/server/providers/cctv/catalog.js index cc258d7f4..7e9be7ac9 100644 --- a/server/providers/cctv/catalog.js +++ b/server/providers/cctv/catalog.js @@ -3,7 +3,7 @@ import path from 'node:path'; import { DEFAULT_CCTV_SOURCE_FILE, CCTV_SOURCE_CACHE_MS } from './constants.js'; import { allocateSourceCap, resolveCatalogCap } from './cap.js'; import { loadGroundHeights, joinGroundHeights } from './groundHeights.js'; -import { normalizeSourceItem } from './normalize.js'; +import { cameraRegion, normalizeSourceItem } from './normalize.js'; import { loadAustinSourcesFromOpenData, loadCaltransSourcesFromOpenData, @@ -217,6 +217,7 @@ export function createCctvCatalog({ sourceRoot = process.cwd() } = {}) { available: Number.isFinite(items.available) ? items.available : items.length, + region: items.region ?? null, }); const packs = [ ...LIVE_PACKS.map((pack, index) => @@ -246,6 +247,8 @@ export function createCctvCatalog({ sourceRoot = process.cwd() } = {}) { pack: pack.name, available: Math.max(pack.available, allocation.packs[index].offered), served: allocation.packs[index].kept, + // Where the pack's cameras are, including those not served. + region: pack.region ?? cameraRegion(pack.sources), })) .filter((pack) => pack.served < pack.available); if (trimmed.length) { diff --git a/server/providers/cctv/normalize.js b/server/providers/cctv/normalize.js index 99cd54712..9e183b23d 100644 --- a/server/providers/cctv/normalize.js +++ b/server/providers/cctv/normalize.js @@ -471,11 +471,32 @@ export function prioritizeSources(cameras, maxCount, anchors) { }); const kept = scored.slice(0, cap).map((entry) => entry.camera); - // How many cameras the pack offered before trimming, for coverage reports. + // How many cameras the pack offered before trimming, and where, for + // coverage reports. Object.defineProperty(kept, 'available', { value: list.length }); + Object.defineProperty(kept, 'region', { value: cameraRegion(list) }); return kept; } +/** The bounding box of cameras with coordinates, or null. */ +export function cameraRegion(cameras) { + let region = null; + for (const camera of cameras || []) { + const lat = Number(camera?.lat); + const lon = Number(camera?.lon); + if (!Number.isFinite(lat) || !Number.isFinite(lon)) continue; + region = region + ? { + west: Math.min(region.west, lon), + south: Math.min(region.south, lat), + east: Math.max(region.east, lon), + north: Math.max(region.north, lat), + } + : { west: lon, south: lat, east: lon, north: lat }; + } + return region; +} + /** * Normalize a raw CCTV source item into a canonical shape with safe defaults. * diff --git a/src/data/cctvCatalogCap.test.mjs b/src/data/cctvCatalogCap.test.mjs index 8e3c08f03..9d93778db 100644 --- a/src/data/cctvCatalogCap.test.mjs +++ b/src/data/cctvCatalogCap.test.mjs @@ -129,9 +129,13 @@ test('the catalog applies the fair cap to configured packs end to end', async () ['env-0', 'env-1', 'env-2'], ); assert.equal(sources[0].pack, 'env'); - assert.deepEqual(getSources.trimmedPacks(), [ - { pack: 'env', available: 12, served: 9 }, - ]); + const [trimmed] = getSources.trimmedPacks(); + assert.deepEqual( + { ...trimmed, region: undefined }, + { pack: 'env', available: 12, served: 9, region: undefined }, + ); + assert.equal(trimmed.region.south, 30); + assert.ok(Math.abs(trimmed.region.north - 30.11) < 1e-9); } finally { for (const key of Object.keys(process.env)) { if (!(key in saved)) delete process.env[key]; @@ -154,4 +158,5 @@ test('a pack trimmed to its nearest cameras reports how many it offered', async ['c0', 'c1'], ); assert.equal(kept.available, 5); + assert.deepEqual(kept.region, { west: 0, south: 0, east: 0, north: 4 }); }); diff --git a/src/tools/queries/media.js b/src/tools/queries/media.js index 45037ae1b..675ef2a27 100644 --- a/src/tools/queries/media.js +++ b/src/tools/queries/media.js @@ -31,6 +31,21 @@ async function readCatalog(services, signal) { }; } +/** Whether an area's box overlaps a plain region box. */ +function boxesOverlap(area, region) { + if (area.south > region.north || area.north < region.south) return false; + const spans = + area.west <= area.east + ? [[area.west, area.east]] + : [ + [area.west, 180], + [-180, area.east], + ]; + return spans.some( + ([west, east]) => west <= region.east && east >= region.west, + ); +} + async function readCameras(services, signal) { return (await readCatalog(services, signal)).cameras; } @@ -55,8 +70,12 @@ export const findCctvCameras = defineTool({ const found = catalog.cameras.filter((camera) => areaContains(area, camera), ); + // A trimmed pack matters wherever its cameras are, served or not, so + // its region decides, not the cameras returned. const packs = new Set(found.map((camera) => camera.pack).filter(Boolean)); - const trimmed = catalog.trimmed.filter((entry) => packs.has(entry.pack)); + const trimmed = catalog.trimmed.filter((entry) => + entry.region ? boxesOverlap(area, entry.region) : packs.has(entry.pack), + ); const rows = found .map((camera) => ({ id: camera.id, diff --git a/src/tools/queries/media.test.mjs b/src/tools/queries/media.test.mjs index 5b5a2e9c4..d7d014df7 100644 --- a/src/tools/queries/media.test.mjs +++ b/src/tools/queries/media.test.mjs @@ -282,3 +282,43 @@ test('cameras from a trimmed catalog pack are reported as partial', async () => /\(the catalog serves only some cameras here: 250 of 900 from tfl\)\.$/, ); }); + +test('an area the catalog trimmed away entirely is not reported complete', async () => { + const catalog = composeCatalog({ + tools: coreTools, + services: { + cctv: { + getCatalog: async () => ({ + trimmedPacks: [ + { + pack: 'tfl', + available: 900, + served: 250, + region: { west: -0.6, south: 51.3, east: 0.3, north: 51.7 }, + }, + ], + // Every served camera is in central London, none in the suburb. + sources: [ + { + id: 'jam-1', + name: 'Strand', + lat: 51.51, + lon: -0.12, + pack: 'tfl', + }, + ], + }), + }, + }, + }); + const suburb = await catalog.call('find_cctv_cameras', { + area: { lat: 51.4, lon: -0.5, radius_km: 3 }, + }); + assert.equal(suburb.data.rows.length, 0); + assert.equal(suburb.data.complete, false); + assert.match(suburb.summary, /250 of 900 from tfl/); + const elsewhere = await catalog.call('find_cctv_cameras', { + area: { lat: 40.7, lon: -74, radius_km: 3 }, + }); + assert.equal(elsewhere.data.complete, true); +});