From 474cce51ebbb2fd9bf1bca659b89a7aa60151e65 Mon Sep 17 00:00:00 2001 From: Manjunath Date: Tue, 3 Nov 2026 17:27:32 +0530 Subject: [PATCH] fix(voice): keep the area in follow-ups on OSM places osm_query handed its places to the analyst with no presentation, so "which of those have emergency?" after "hospitals in Kathmandu" was answered as "anywhere in the loaded data". The handoff now carries the scope the places were found in ("inside Kathmandu" with its areaId, or "in view"), as an analyst count over that area would. --- src/data/analystEngine.js | 9 +++++++-- src/data/analystEngine.test.mjs | 26 ++++++++++++++++++++++++++ src/voice/gevActions.js | 7 +++++-- src/voice/osmActions.js | 19 +++++++++++++++---- src/voice/osmActions.test.mjs | 19 ++++++++++++++++++- 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/data/analystEngine.js b/src/data/analystEngine.js index 4093e9ddc..50a17204e 100644 --- a/src/data/analystEngine.js +++ b/src/data/analystEngine.js @@ -1164,11 +1164,16 @@ export function createAnalystEngine(providers) { * so a follow-up filters exactly what the user just heard about. * @param {string} layerKey Queryable layer the rows belong to. * @param {Array} rows Records in that layer's analyst shape. - * @param {{scope?: string}} [options] + * @param {{scope?: string, presentation?: object|null}} [options] + * `presentation` is how the set was scoped ("inside Kathmandu"), so a + * follow-up names the same scope instead of "anywhere". */ - remember(layerKey, rows, { scope = 'handoff' } = {}) { + remember(layerKey, rows, { scope = 'handoff', presentation = null } = {}) { if (!ANALYST_LAYERS[layerKey] || !Array.isArray(rows)) return false; lastResult = { + ...(presentation + ? { presentation: structuredClone(presentation) } + : {}), matched: [{ key: layerKey, rows: rows.slice() }], layerKeys: [layerKey], coverage: { diff --git a/src/data/analystEngine.test.mjs b/src/data/analystEngine.test.mjs index 73e5533d4..5a46c629c 100644 --- a/src/data/analystEngine.test.mjs +++ b/src/data/analystEngine.test.mjs @@ -1176,3 +1176,29 @@ test('analyst: without an area provider the area scopes refuse plainly', async ( assert.equal(r.ok, false); assert.equal(r.code, 'AREA_UNAVAILABLE'); }); + +test('a follow-up on handed-off places keeps the area they were found in', async () => { + const rows = [ + { id: 'node/1', lat: 27.7, lon: 85.3, name: 'Bir Hospital', kind: 'hospital', emergency: 'yes' }, + { id: 'node/2', lat: 27.71, lon: 85.31, name: 'Teaching Hospital', kind: 'hospital', emergency: 'no' }, + ]; + const engine = createAnalystEngine({ + getRecords: (key) => (key === 'osm-places' ? rows : []), + resolveRegionRing: async () => null, + getViewContext: () => ({ lat: 27.7, lon: 85.3, viewRadiusKm: 20 }), + }); + assert.equal( + engine.remember('osm-places', rows, { + presentation: { scopeLabel: 'inside Kathmandu', coverageScope: 'area:area-1', areaId: 'area-1' }, + }), + true, + ); + const result = await engine.query({ + layers: ['osm-places'], + followUp: true, + filters: [{ field: 'emergency', op: 'eq', value: 'yes' }], + }); + assert.equal(result.count, 1); + assert.equal(result.scopeLabel, 'inside Kathmandu'); + assert.equal(result.coverage.scope, 'area:area-1'); +}); diff --git a/src/voice/gevActions.js b/src/voice/gevActions.js index b2c4de965..e2acd8933 100644 --- a/src/voice/gevActions.js +++ b/src/voice/gevActions.js @@ -1226,9 +1226,12 @@ export function createGevActionRunner({ onCredit: () => registerDynamicCredit(viewer, OSM_PLACES_CREDIT), // The listed places become the analyst's "last answer", so // "which of those…" filters them and never an older set. - rememberResults: (rows) => { + rememberResults: (rows, presentation) => { if (!current()) return; - if (rows) ensureAnalystEngine().remember('osm-places', rows); + if (rows) + ensureAnalystEngine().remember('osm-places', rows, { + presentation, + }); else analystEngine?.reset(); }, progress: reportProgress, diff --git a/src/voice/osmActions.js b/src/voice/osmActions.js index 30b382055..9adeb4757 100644 --- a/src/voice/osmActions.js +++ b/src/voice/osmActions.js @@ -67,8 +67,9 @@ const EXAMPLES = Object.values(OSM_PRESETS) * @param {(enabled: boolean) => Promise} deps.enableLayer * @param {Function} deps.search `/api/osm/features` client. * @param {() => void} [deps.onCredit] Registers the OpenStreetMap credit. - * @param {(rows: object[]|null) => void} [deps.rememberResults] Hands the - * listed places to analyst follow-ups; null forgets the previous set. + * @param {(rows: object[]|null, presentation?: object) => void} [deps.rememberResults] + * Hands the listed places, and the scope they were found in, to analyst + * follow-ups; null forgets the previous set. * @param {(step: string, label?: string) => void} [deps.progress] * @param {object} args * @param {{isCurrent?: () => boolean, budgetMs?: number, signal?: AbortSignal}} [options] @@ -258,8 +259,18 @@ export async function osmQuery( } if (wasDisplaced()) return displaced(); // These places are now "the last answer": follow-ups filter them and - // "the second one" is the second row below. - rememberResults(records); + // "the second one" is the second row below. A follow-up keeps naming the + // area they were found in, as an analyst count over that area would. + rememberResults( + records, + area + ? { + scopeLabel: `inside ${areaName}`, + coverageScope: `area:${args.areaId}`, + areaId: args.areaId, + } + : { scopeLabel: 'in view', coverageScope: 'view' }, + ); const unnamed = `unnamed ${singularLabel(preset.label)}`; const referents = [ ...records.filter((r) => r.name), diff --git a/src/voice/osmActions.test.mjs b/src/voice/osmActions.test.mjs index b29e67bc2..25879cf2a 100644 --- a/src/voice/osmActions.test.mjs +++ b/src/voice/osmActions.test.mjs @@ -64,6 +64,7 @@ function deps({ area = kathmanduArea(), answer, enabled = false } = {}) { results: null, credits: 0, memory: [], + presentation: [], }; const layer = { setResults(records, meta) { @@ -111,7 +112,10 @@ function deps({ area = kathmanduArea(), answer, enabled = false } = {}) { onCredit: () => { calls.credits += 1; }, - rememberResults: (records) => calls.memory.push(records), + rememberResults: (records, presentation) => { + calls.memory.push(records); + calls.presentation.push(presentation); + }, }, requestOff({ settle = true } = {}) { const change = { @@ -165,6 +169,15 @@ test('hospitals inside an area: exact inside the outline, duplicates merged, sho limit: 500, }); assert.match(result.display.merged, /1 duplicate/); + assert.deepEqual( + calls.presentation.at(-1), + { + scopeLabel: 'inside Kathmandu', + coverageScope: 'area:osm:r1', + areaId: 'osm:r1', + }, + 'a follow-up keeps naming the area the places were found in', + ); }); test('an area too large to list is counted over its box and says so', async () => { @@ -241,6 +254,10 @@ test('the view is searched when no area is named; countOnly leaves the layer alo assert.equal(calls.results, null); assert.match(result.say, /in view, per OpenStreetMap$/); assert.deepEqual(calls.search[0].bbox, [85.2, 27.6, 85.4, 27.8]); + assert.deepEqual(calls.presentation.at(-1), { + scopeLabel: 'in view', + coverageScope: 'view', + }); }); test('unknown kinds, unknown areas, failures and supersession are refused plainly', async () => {