From 302a1c1995e10393c68df1e18833d88a0940c921 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 1 Oct 2026 15:24:01 -0700 Subject: [PATCH] fix: say when bike share, transit and place limits leave results out Bike share and transit read only the three nearest systems, and place search returns at most five matches within 50 km, while answers read as complete. Answers now name the systems not searched and say when a place search limit may have left places out. --- src/tools/queries/mobility.js | 33 +++++++++++++++++++----- src/tools/queries/mobility.test.mjs | 24 +++++++++++++++++ src/tools/queries/places.js | 40 +++++++++++++++++++++++++---- src/tools/queries/places.test.mjs | 26 +++++++++++++++++++ 4 files changed, 112 insertions(+), 11 deletions(-) diff --git a/src/tools/queries/mobility.js b/src/tools/queries/mobility.js index 055889a7c..b4743e81f 100644 --- a/src/tools/queries/mobility.js +++ b/src/tools/queries/mobility.js @@ -24,18 +24,32 @@ const MAX_SYSTEMS = 3; const round = (value, digits) => Number.isFinite(value) ? Number(value.toFixed(digits)) : null; -/** Systems or feeds whose coverage reaches the area, nearest first. */ +/** + * Systems or feeds whose coverage reaches the area, nearest first: the + * nearest MAX_SYSTEMS are read and the rest are returned as `skipped`. + */ function covering(area, entries, centerOf, radiusOf) { const center = areaCenter(area); const reach = areaRadiusKm(area); - return entries + const reaching = entries .map((entry) => ({ entry, distance: distanceKm(center, centerOf(entry)) })) .filter(({ entry, distance }) => distance <= reach + radiusOf(entry)) .sort((a, b) => a.distance - b.distance) - .slice(0, MAX_SYSTEMS) .map(({ entry }) => entry); + return { + selected: reaching.slice(0, MAX_SYSTEMS), + skipped: reaching.slice(MAX_SYSTEMS), + }; } +/** A note naming systems left out of an answer, or an empty list. */ +const skippedNote = (skipped, nameOf) => + skipped.length + ? [ + `${skipped.length} more not searched: ${skipped.map(nameOf).join(', ')}; use a smaller area`, + ] + : []; + export const getBikeShare = defineTool({ name: 'get_bike_share', title: 'Bike-share stations', @@ -52,7 +66,7 @@ export const getBikeShare = defineTool({ async run(args, { services, signal }) { const area = await resolveArea(args.area, { services, signal }); const { bikeshare } = services; - const systems = covering( + const { selected: systems, skipped } = covering( area, bikeshare.systems, (system) => ({ lat: system.centerLat, lon: system.centerLon }), @@ -110,14 +124,19 @@ export const getBikeShare = defineTool({ const failed = results.filter( (result) => result.status === 'rejected', ).length; + const notes = [ + ...(failed ? [`${countNoun(failed, 'system')} unavailable`] : []), + ...skippedNote(skipped, (system) => system.city), + ]; return { summary: `${countNoun(rows.length, 'bike-share station')} in ${area.label} with ` + `${countNoun(bikes, 'bike')} available` + - (failed ? ` (${countNoun(failed, 'system')} unavailable).` : '.'), + (notes.length ? ` (${notes.join('; ')}).` : '.'), data: { ...capRows(rows, args.limit), systems: systems.map((system) => system.city), + systems_not_searched: skipped.map((system) => system.city), bikes_available: bikes, }, }; @@ -149,7 +168,7 @@ export const getTransitVehicles = defineTool({ requires: ['transit'], async run(args, { services, signal }) { const area = await resolveArea(args.area, { services, signal }); - const feeds = covering( + const { selected: feeds, skipped } = covering( area, await services.transit.getFeeds({ signal }), (feed) => feed.center, @@ -248,6 +267,7 @@ export const getTransitVehicles = defineTool({ `${countNoun(staleFeeds, 'feed')} stale; positions may be out of date`, ] : []), + ...skippedNote(skipped, (feed) => feed.name), ]; const what = args.route ? ` on route ${args.route}` : ''; return { @@ -258,6 +278,7 @@ export const getTransitVehicles = defineTool({ ...capRows(rows, args.limit), stale: staleFeeds > 0, expired_positions_dropped: expired, + feeds_not_searched: skipped.map((feed) => feed.name), feeds: feeds.map((feed, index) => ({ name: feed.name, status: diff --git a/src/tools/queries/mobility.test.mjs b/src/tools/queries/mobility.test.mjs index 11510f01a..7b41e7085 100644 --- a/src/tools/queries/mobility.test.mjs +++ b/src/tools/queries/mobility.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { composeCatalog, coreTools } from '../index.js'; +import { GBFS_CITY_REGISTRY } from '../../layers/bikeshare/registry.js'; const system = (id, city, centerLat, centerLon) => ({ id, @@ -404,3 +405,26 @@ test('transit marks stale feeds and drops expired positions', async () => { '1 transit vehicle in 2 km around 30.267, -97.743 (1 feed stale; positions may be out of date).', ); }); + +test('bike share names the systems it did not search', async () => { + const empty = { data: { stations: [] } }; + const catalog = composeCatalog({ + tools: coreTools, + services: { + bikeshare: { + systems: GBFS_CITY_REGISTRY, + getStations: async () => empty, + }, + }, + }); + // New York, Philadelphia, Washington and Boston. + const result = await catalog.call('get_bike_share', { + area: { bbox: [-77.5, 38.5, -70.5, 42.8] }, + }); + assert.equal(result.data.systems.length, 3); + assert.ok(result.data.systems_not_searched.length >= 1); + assert.match( + result.summary, + /more not searched: .*; use a smaller area\)\.$/, + ); +}); diff --git a/src/tools/queries/places.js b/src/tools/queries/places.js index c6532832b..e6ec3a5e5 100644 --- a/src/tools/queries/places.js +++ b/src/tools/queries/places.js @@ -12,6 +12,12 @@ import { } from '../area.js'; import { LIMIT_SCHEMA, capRows, countNoun, thinEvenly } from '../results.js'; +// Most results the app's place search endpoints return, and the widest +// radius a text search covers. +const SEARCH_RESULTS = 5; +const NEARBY_RESULTS = 20; +const MAX_SEARCH_RADIUS_M = 50_000; + const MODES = { walk: 'foot', drive: 'car', bike: 'bike' }; const MAX_ROUTE_POINTS = 100; const NOT_CONFIGURED = @@ -52,8 +58,9 @@ export const searchPlaces = defineTool({ async run(args, { services, signal }) { const area = await resolveArea(args.area, { services, signal }); const center = areaCenter(area); + const wantedM = areaRadiusKm(area) * 1000; const radiusM = Math.round( - Math.min(50000, Math.max(500, areaRadiusKm(area) * 1000)), + Math.min(MAX_SEARCH_RADIUS_M, Math.max(500, wantedM)), ); const result = await services.placeSearch.search( args.query, @@ -66,9 +73,27 @@ export const searchPlaces = defineTool({ areaContains(area, { lat: place.latitude, lon: place.longitude }), ) .map(placeRow); + // The search returns a few matches within a capped radius; say so when + // either limit may have left places out. + const limited = result.places.length >= SEARCH_RESULTS; + const narrowed = wantedM > MAX_SEARCH_RADIUS_M; + const notes = [ + ...(limited + ? [`the search returns at most ${SEARCH_RESULTS} matches`] + : []), + ...(narrowed + ? [`searched within ${MAX_SEARCH_RADIUS_M / 1000} km of the center`] + : []), + ]; return { - summary: `${countNoun(rows.length, 'place')} matching "${args.query}" in ${area.label}.`, - data: capRows(rows, args.limit), + summary: + `${countNoun(rows.length, 'place')} matching "${args.query}" in ${area.label}` + + (notes.length ? ` (${notes.join('; ')}).` : '.'), + data: { + ...capRows(rows, args.limit), + may_have_more: limited || narrowed, + searched_radius_km: radiusM / 1000, + }, }; }, }); @@ -99,9 +124,14 @@ export const placesNearby = defineTool({ ); if (!result.configured) throw new ToolError('unavailable', NOT_CONFIGURED); const rows = result.places.map(placeRow); + const limited = result.places.length >= NEARBY_RESULTS; return { - summary: `${countNoun(rows.length, 'place')} within ${radiusM} m of ${point.label}.`, - data: capRows(rows, args.limit), + summary: + `${countNoun(rows.length, 'place')} within ${radiusM} m of ${point.label}` + + (limited + ? ` (nearby search returns at most ${NEARBY_RESULTS} places).` + : '.'), + data: { ...capRows(rows, args.limit), may_have_more: limited }, }; }, }); diff --git a/src/tools/queries/places.test.mjs b/src/tools/queries/places.test.mjs index ab4a0a0e2..89cad9ba8 100644 --- a/src/tools/queries/places.test.mjs +++ b/src/tools/queries/places.test.mjs @@ -248,3 +248,29 @@ test('the place and route services read the app routes and translate failures', (error) => error.code === 'retry_later' && error.retryAfterSeconds === 5, ); }); + +test('place answers say when the search limits may have left places out', async () => { + const five = Array.from({ length: 5 }, (_, index) => + place(`p${index}`, 30.27, -97.74), + ); + const catalog = composeCatalog({ + tools: coreTools, + services: { + placeSearch: placeSearch({ configured: true, places: five }), + places, + }, + }); + const capped = await catalog.call('search_places', { + query: 'coffee', + area: { place: 'Austin' }, + limit: 100, + }); + assert.equal(capped.data.may_have_more, true); + assert.match(capped.summary, /the search returns at most 5 matches\)\.$/); + const wide = await catalog.call('search_places', { + query: 'coffee', + area: { lat: 30.27, lon: -97.74, radius_km: 200 }, + }); + assert.equal(wide.data.searched_radius_km, 50); + assert.match(wide.summary, /searched within 50 km of the center/); +});