fix(voice): resolve follow-up review edge cases

This commit is contained in:
Sameh Khamis committed 2026-10-07 14:21:46 -07:00
1 parent 1594db9497
commit b5cb75bcf9
13 files changed
+232 -16

No files matched your search

+8 -1
View File
@@ -137,7 +137,14 @@ function resolveFly(args, { pointer, referents }) {
if (target.layerId && target.id && source === 'referent') {
// Contacts move: locate them when the flight starts, not from a list.
return {
args: { ...rest, entity: { layerId: target.layerId, id: target.id } },
args: {
...rest,
entity: {
layerId: target.layerId,
id: target.id,
...(hasPosition(target) ? { lat: target.lat, lon: target.lon } : {}),
},
},
used,
};
}
+13
View File
@@ -1,5 +1,6 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { attachVoiceResult } from './speech.js';
import { resolveDeicticArgs } from './deixis.js';
import { createReferentRegistry, referentsFromResult } from './referents.js';
@@ -167,3 +168,15 @@ test('registry: hidden card rows never resolve and last means displayed item fiv
assert.equal(referents.get(6), null);
assert.equal(referents.get(-1)?.id, 'F5');
});
test('numbered custom-labeled pins retain their resolved coordinates', () => {
const referents = createReferentRegistry();
const result = attachVoiceResult('annotate_map', {
ok: true,
items: [{ ok: true, id: 'pin-1', label: 'Meeting point', latitude: 30.27, longitude: -97.74 }],
});
referents.recordResult('annotate_map', result);
assert.deepEqual(resolveDeicticArgs('fly_to_location', { referent: 1 }, { referents }).args,
{ latitude: 30.27, longitude: -97.74 });
});
+28 -1
View File
@@ -3318,7 +3318,7 @@ async function getEntityContext(
const AIRCRAFT_LAYERS = new Set(['flights', 'military']);
/** Live position of a layer record by id, or null when it is gone. */
function locateLayerEntity(dataManager, { layerId, id }) {
function locateLayerEntity(dataManager, { layerId, id, lat, lon }) {
if (!layerId || !id) return null;
if (dataManager?.isEnabled && !dataManager.isEnabled(layerId)) return null;
let found = null;
@@ -3338,6 +3338,33 @@ function locateLayerEntity(dataManager, { layerId, id }) {
Number.isFinite(record.longitude)
)
return { lat: record.latitude, lon: record.longitude, record };
// Analyst-only layers (e.g. bikeshare) have no pick/context lookup. Read
// their current snapshot, so a removed or moved record is not resurrected.
try {
const matches =
dataManager?.layers
?.get(layerId)
?.module?.getAnalystRecords?.()
.filter(
(item) =>
String(item.icao24 ?? item.mmsi ?? item.noradId ?? item.id) ===
String(id),
) || [];
// Bikeshare uses station names as IDs: retain the coordinate identity
// even if a namesake disappears. Moving records use their fresh fix.
const row =
matches.length === 1 && layerId !== 'bikeshare'
? matches[0]
: matches.find((item) => item.lat === lat && item.lon === lon);
if (Number.isFinite(row?.lat) && Number.isFinite(row?.lon))
return {
lat: row.lat,
lon: row.lon,
found: { ...row, latitude: row.lat, longitude: row.lon },
};
} catch {
// A missing or unavailable snapshot cannot establish a live position.
}
return null;
}
+63
View File
@@ -3822,3 +3822,66 @@ test('point-and-ask: only accepted, current results become the referent list', a
assert.equal(used, 1, 'a current pointer resolution announces the chip');
assert.equal(tracked, 1);
});
test('numbered analyst records navigate using the current layer accessor', async () => {
globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null };
const { viewer, styleManager } = createVoiceNavigationHarness();
const referents = createReferentRegistry();
let enabled = true;
let records = [{ id: 'congress', name: 'Congress station', lat: 30.27, lon: -97.74 }];
const runner = createActionRunner({
viewer, styleManager,
dataManager: {
layers: new Map([['bikeshare', { module: { getAnalystRecords: () => records } }]]),
isEnabled: () => enabled,
getAll: () => [],
},
deixis: { referents },
});
referents.recordResult('analyst_query', { ok: true, items: [{ ...records[0], layerKey: 'bikeshare' }] });
const result = await runner('fly_to_location', { referent: 1 });
assert.equal(result.ok, true);
assert.equal(result.latitude, 30.27);
assert.equal(result.longitude, -97.74);
enabled = false;
assert.equal((await runner('fly_to_location', { referent: 1 })).ok, false, 'disabled records cannot be revisited');
enabled = true;
records = [];
assert.equal((await runner('fly_to_location', { referent: 1 })).ok, false, 'removed records are not resurrected');
});
test('analyst-only lookup distinguishes duplicate names and uses canonical aircraft IDs', async () => {
globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null };
const { viewer, styleManager } = createVoiceNavigationHarness();
const referents = createReferentRegistry();
const stations = [
{ id: 'Central Station', name: 'Central Station', lat: 30, lon: -97 },
{ id: 'Central Station', name: 'Central Station', lat: 40, lon: -74 },
];
const aircraft = [{ id: 'UAL1', icao24: 'abc123', lat: 35, lon: -100 }];
const runner = createActionRunner({
viewer, styleManager,
dataManager: {
layers: new Map([
['bikeshare', { module: { getAnalystRecords: () => stations } }],
['local-adsb', { module: { getAnalystRecords: () => aircraft } }],
]),
isEnabled: () => true, getAll: () => [],
},
deixis: { referents },
});
referents.recordResult('analyst_query', { ok: true, items: stations.map(row => ({ ...row, layerKey: 'bikeshare' })) });
const station = await runner('fly_to_location', { referent: 2 });
assert.equal(station.ok, true);
assert.equal(station.latitude, 40);
assert.equal(station.longitude, -74);
stations.pop();
assert.equal((await runner('fly_to_location', { referent: 2 })).ok, false, 'a removed station must not resolve to its remaining namesake');
referents.recordResult('analyst_query', { ok: true, items: aircraft.map(row => ({ ...row, layerKey: 'local-adsb' })) });
aircraft[0].lat = 36;
const plane = await runner('fly_to_location', { referent: 1 });
assert.equal(plane.ok, true);
assert.equal(plane.latitude, 36, 'canonical ID resolves the current position, not old coordinates');
});
+10
View File
@@ -350,3 +350,13 @@ test('deixis prompt lint: one short rule, and every sentinel it names exists in
const size = instructions.length + JSON.stringify(GEV_REALTIME_TOOLS).length;
assert.ok(size <= 52000, `prompt size ${size}`);
});
test('after-tool speech allows returned items to answer requested lists and rankings', () => {
const rule = realtimeInstructions().split('\n').find(line => line.startsWith('AFTER TOOLS:'));
assert.match(rule, /requested analyst list or ranking/);
assert.match(rule, /returned items.*returned values/);
assert.match(rule, /keep their order/);
assert.doesNotMatch(rule, /never adding to it/);
assert.match(rule, /always preserve them/);
});
+10 -1
View File
@@ -62,11 +62,11 @@ export function createVoiceSession({ createAdapter, runner, signal }) {
!actionSignal.aborted &&
epoch === generation &&
(!options.isCurrent || options.isCurrent());
const callId = options.callId ?? null;
try {
if (!isCurrent()) throw abortError();
// Calls carry the adapter's call id so observers can match progress
// and results to the right call when a tool runs twice at once.
const callId = options.callId ?? null;
emit({ type: 'action-call', name, callId, arguments: args });
if (!isCurrent()) throw abortError();
// Step reports reach observers only while the action is current, so a
@@ -91,6 +91,15 @@ export function createVoiceSession({ createAdapter, runner, signal }) {
if (!isCurrent()) throw abortError();
emit({ type: 'action-result', name, callId, result });
return result;
} catch (error) {
if (isCurrent())
emit({
type: 'action-result',
name,
callId,
result: { ok: false, error: error?.message || String(error) },
});
throw error;
} finally {
actions.delete(action);
}
+60
View File
@@ -1,5 +1,10 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import {
initialVoiceCardState,
reduceVoiceCard,
voiceCardView,
} from './voiceCardPresentation.js';
import { createVoiceSession } from './session.js';
import { realtimeSessionEvent } from './realtimeEvents.js';
import { createVoiceCommands } from './sessionCommands.js';
@@ -373,3 +378,58 @@ test('action events carry the call id; progress after cancellation is dropped',
report({ step: 'outline' });
assert.equal(events.filter((e) => e.type === 'progress').length, 1);
});
test('a rejected current action settles its card and preserves the rejection', async () => {
const failure = new Error('Unknown panel');
const f = fixture(async () => {
throw failure;
});
let card = initialVoiceCardState();
const events = [];
f.session.subscribe((event) => {
events.push(event);
card = reduceVoiceCard(card, event);
});
await f.session.start();
await assert.rejects(
f.hooks.runAction(
'set_panel_open',
{ panelId: 'missing' },
{ callId: 'failed-call' },
),
(error) => error === failure,
);
f.hooks.emit({ type: 'completion', status: 'completed' });
assert.equal(voiceCardView(card).busy, false);
assert.equal(card.steps[0].status, 'failed');
const result = events.find((event) => event.type === 'action-result');
assert.equal(result.callId, 'failed-call');
assert.equal(result.result.ok, false);
f.session.destroy();
});
test('a late rejection cannot settle a newer turn card', async () => {
let reject;
const f = fixture(
() =>
new Promise((_, fail) => {
reject = fail;
}),
);
const events = [];
f.session.subscribe((event) => events.push(event));
await f.session.start();
const pending = f.hooks.runAction(
'set_panel_open',
{},
{ callId: 'old-call' },
);
f.hooks.emit({ type: 'interruption', reason: 'user-speech' });
reject(new Error('late failure'));
await assert.rejects(pending, /late failure/);
assert.equal(
events.some((event) => event.type === 'action-result'),
false,
);
f.session.destroy();
});
+2
View File
@@ -373,6 +373,8 @@ function annotateMap(result) {
n: index + 1,
id: item.id || null,
label: spokenLabel(item.label || item.target || `Mark ${index + 1}`),
latitude: item.latitude,
longitude: item.longitude,
}));
const parts = [];
if (failed.length)