Follow up voice tilt review: superseded flights, wait, globe range

- A motion waiting on a voice fly-to follows a later fly-to in the
  same reply that cancels it, within one deadline, instead of
  reporting that its flight did not arrive. A stale reply stops
  following.
- The wait is 15 s, down from 30 s: it holds the tool output, and on
  Gemini the later tools in the turn.
- Beyond 1,000 km of range a tilt happens in place instead of
  swinging thousands of km about the crosshair point.
- The tilt-nudge limits read the pivot pitch the motion starts from,
  including for a voice tilt from a tracked view, whose follow camera
  is released before it moves.
- move_camera's argument checks run before the flight wait, so a
  malformed call is refused at once.
- Tests: t.after cleanup in the fly-then-motion loop, the pitchDeg
  pin asserts the field before removing it, and the handoff-order
  pin follows the extracted argument checks.
This commit is contained in:
Manjunath committed 2026-11-03 20:05:34 +05:30
1 parent 2aef67197c
commit 9e64d517ad
7 files changed
+266 -79

No files matched your search

+10 -1
View File
@@ -223,11 +223,20 @@ test('validated voice camera destinations share the UI navigation authority faca
'move camera',
);
ordered(move, [
"if (!['orbit', 'pan', 'tilt', 'rotate', 'rotate_globe'].includes(motion))",
'if (argsError) return argsError;',
'const start = () => {',
/const preserveCameraFlight =\s*motion === 'orbit'/,
'runNavigation(start, { preserveCameraFlight })',
], 'move validation before handoff');
const moveArgs = body(
cameraVerbs,
/export function moveCameraArgsError\(args = \{\}\) \{([\s\S]*?)\n\}/,
'move camera argument checks',
);
assert.ok(
moveArgs.includes("if (!['orbit', 'pan', 'tilt', 'rotate', 'rotate_globe'].includes(motion))"),
'unknown motions are refused before handoff',
);
const route = body(
cameraVerbs,
+32
View File
@@ -368,3 +368,35 @@ test('a zoom out during a far orbit never pulls the camera in', (t) => {
h.tick();
assert.ok(pivotRange(h) >= before - 1, `range fell to ${pivotRange(h)} m`);
});
test('from globe range a tilt happens in place', (t) => {
// A pivot 2,000 km out would swing the camera thousands of km.
const h = harness(t, { height: 2_000_000 });
const before = Cesium.Cartesian3.clone(h.camera.positionWC);
assert.equal(moveCamera({ motion: 'tilt', pitchDeg: -45 }).ok, true);
h.finish();
assert.ok(Math.abs(pitchDeg(h.camera) - -45) < 0.5);
assert.ok(Cesium.Cartesian3.distance(h.camera.positionWC, before) < 1e-3);
});
test('a regional tilt-up nudge at the pivot limit refuses honestly', (t) => {
// 20 km up at -7°, the pivot 186 km out sits at -5.3°: already at the limit.
harness(t, { height: 20000, pitch: -7 });
const result = moveCamera({ motion: 'tilt', direction: 'up' });
assert.equal(result.ok, false);
assert.match(result.error, /upper tilt limit/);
assert.equal(getActiveCameraMotion(), null);
});
test('a voice tilt from a tracked view judges its limit from the pivot it will use', (t) => {
const h = harness(t, { height: 20000, pitch: -7 });
h.viewer.trackedEntity = { id: 'tracked' };
// Voice releases the follow camera before the motion starts.
const release = (start) => {
h.viewer.trackedEntity = undefined;
return start();
};
const result = moveCamera({ motion: 'tilt', direction: 'up' }, release);
assert.equal(result.ok, false);
assert.match(result.error, /upper tilt limit/);
});
+100 -65
View File
@@ -57,6 +57,11 @@ const TILT_TO_DEG_S = 30;
* range × sin|pitch|. It swings no lower than this; the rest tilts in place.
*/
const TILT_MIN_CLEARANCE_M = 150;
/**
* Beyond this range a pivot would swing the camera thousands of km about the
* crosshair point, so the tilt happens in place instead.
*/
const TILT_PIVOT_MAX_RANGE_M = 1_000_000;
/* ── Route dolly: cinematic tuning ──────────────────────────────────────────
* Every knob an owner may want to retune lives in this block. The shaping is
@@ -1322,11 +1327,15 @@ function tiltPivotPitchMax(range) {
/**
* Where a tilt starts. It swings about the ground under the crosshair so the
* place stays centred; the follow camera owns a tracked view's frame.
* place stays centred. It tilts in place with no ground there, from globe
* range, or while an entity is tracked (the follow camera owns that frame;
* voice releases it before moving, so a voice tilt still pivots).
*/
function tiltStartFrame(cam) {
const target = _viewer.trackedEntity ? null : _getTarget?.(_viewer);
const pivot = target ? tiltPivot(cam, target) : null;
function tiltStartFrame(cam, { releasesTracking = false } = {}) {
const tracked = Boolean(_viewer.trackedEntity) && !releasesTracking;
const target = tracked ? null : _getTarget?.(_viewer);
let pivot = target ? tiltPivot(cam, target) : null;
if (pivot && pivot.range > TILT_PIVOT_MAX_RANGE_M) pivot = null;
return { target, pivot, from: pivot ? pivot.pitch : cam.pitch };
}
@@ -1542,6 +1551,83 @@ export function initCameraVerbs(
}
}
/**
* A tilt's angle: the one asked for, the default oblique view when a bare
* "tilt the camera" names neither angle nor direction, or null for a nudge.
*/
function requestedTiltPitch(args, direction) {
// Only a real number or numeric text: Number('') and Number(null) are 0,
// which would swing the camera to the horizon.
const raw = args.pitchDeg;
const requested =
typeof raw === 'number' || (typeof raw === 'string' && raw.trim() !== '')
? Number(raw)
: NaN;
// The camera never looks above the horizon, so a positive angle can only
// mean the same angle below it ("tilt 45 degrees" sent as 45).
if (Number.isFinite(requested))
return Cesium.Math.clamp(
Cesium.Math.toRadians(-Math.abs(requested)),
PITCH_MIN,
PITCH_MAX,
);
return direction === null
? Cesium.Math.toRadians(TILT_DEFAULT_PITCH_DEG)
: null;
}
/**
* Argument errors move_camera refuses whatever the camera is doing, so a
* caller holding a motion back behind a flight can refuse it before waiting.
* @param {Object} args
* @returns {Object|null} The refusal, or null when the arguments are usable.
*/
export function moveCameraArgsError(args = {}) {
const motion = String(args.motion || '').toLowerCase();
const direction = args.direction
? String(args.direction).toLowerCase()
: null;
if (motion === 'stop') return null;
if (!['orbit', 'pan', 'tilt', 'rotate', 'rotate_globe'].includes(motion)) {
return {
ok: false,
action: 'move_camera',
error: `Unknown motion "${args.motion}" — use orbit, pan, tilt, rotate, rotate_globe, or stop.`,
};
}
if (
motion !== 'orbit' &&
motion !== 'rotate_globe' &&
(motion !== 'tilt' || requestedTiltPitch(args, direction) === null) &&
!['left', 'right', 'up', 'down'].includes(direction)
) {
return {
ok: false,
action: 'move_camera',
error: `${motion} needs a direction (left/right${motion !== 'rotate' ? '/up/down' : ''}).`,
};
}
if (motion === 'rotate' && !['left', 'right'].includes(direction)) {
return {
ok: false,
action: 'move_camera',
error: 'rotate needs left or right.',
};
}
if (
motion === 'rotate_globe' &&
direction !== null &&
!['left', 'right'].includes(direction)
) {
return {
ok: false,
action: 'move_camera',
error: 'rotate_globe needs left or right.',
};
}
return null;
}
/**
* move_camera implementation. Returns the house result shape; rejections are
* plain-English `ok:false` errors.
@@ -1574,69 +1660,17 @@ export function moveCamera(args = {}, runNavigation = null) {
};
return typeof runNavigation === 'function' ? runNavigation(stop) : stop();
}
if (!['orbit', 'pan', 'tilt', 'rotate', 'rotate_globe'].includes(motion)) {
return {
ok: false,
action: 'move_camera',
error: `Unknown motion "${args.motion}" — use orbit, pan, tilt, rotate, rotate_globe, or stop.`,
};
}
// Tilt to an angle: the one asked for, or a default oblique view when a
// bare "tilt the camera" names neither angle nor direction.
let tiltToPitch = null;
if (motion === 'tilt') {
// Only a real number or numeric text: Number('') and Number(null) are 0,
// which would swing the camera to the horizon.
const raw = args.pitchDeg;
const requested =
typeof raw === 'number' || (typeof raw === 'string' && raw.trim() !== '')
? Number(raw)
: NaN;
// The camera never looks above the horizon, so a positive angle can only
// mean the same angle below it ("tilt 45 degrees" sent as 45).
if (Number.isFinite(requested))
tiltToPitch = Cesium.Math.clamp(
Cesium.Math.toRadians(-Math.abs(requested)),
PITCH_MIN,
PITCH_MAX,
);
else if (direction === null)
tiltToPitch = Cesium.Math.toRadians(TILT_DEFAULT_PITCH_DEG);
}
if (
motion !== 'orbit' &&
motion !== 'rotate_globe' &&
tiltToPitch === null &&
!['left', 'right', 'up', 'down'].includes(direction)
) {
return {
ok: false,
action: 'move_camera',
error: `${motion} needs a direction (left/right${motion !== 'rotate' ? '/up/down' : ''}).`,
};
}
if (motion === 'rotate' && !['left', 'right'].includes(direction)) {
return {
ok: false,
action: 'move_camera',
error: 'rotate needs left or right.',
};
}
if (
motion === 'rotate_globe' &&
direction !== null &&
!['left', 'right'].includes(direction)
) {
return {
ok: false,
action: 'move_camera',
error: 'rotate_globe needs left or right.',
};
}
const argsError = moveCameraArgsError(args);
if (argsError) return argsError;
const tiltToPitch =
motion === 'tilt' ? requestedTiltPitch(args, direction) : null;
// A managed (voice) navigation releases the follow camera before start(),
// so checks made now must judge from the frame start() will see.
const releasesTracking = typeof runNavigation === 'function';
if (motion === 'tilt' && tiltToPitch !== null) {
// Judge from the pitch the motion will start at: the pivot's, which at
// regional range is a degree or two shallower than the camera's.
const { from } = tiltStartFrame(_viewer.camera);
const { from } = tiltStartFrame(_viewer.camera, { releasesTracking });
if (Math.abs(tiltToPitch - from) < Cesium.Math.toRadians(0.5)) {
return {
ok: true,
@@ -1661,7 +1695,8 @@ export function moveCamera(args = {}, runNavigation = null) {
} else if (motion === 'tilt') {
// Honest limits: at the clamp a tilt is a no-op — say so instead of
// reporting success and letting the model gaslight itself (field test).
const pitch = _viewer.camera.pitch;
// The clamp reads the pitch the motion starts from.
const pitch = tiltStartFrame(_viewer.camera, { releasesTracking }).from;
if (direction === 'up' && pitch >= PITCH_MAX - Cesium.Math.toRadians(0.5)) {
return {
ok: false,
+1
View File
@@ -1018,6 +1018,7 @@ test('the voice TOOL SCHEMA matches the pinned contract — the mission mapping
// the legacy camera fields to keep the pin.
const camera = legacyTools.find((tool) => tool.name === 'move_camera')
.parameters.properties;
assert.equal(camera.pitchDeg.type, 'number');
delete camera.pitchDeg;
camera.direction.description =
'Required for pan/tilt/rotate; orbit/rotate_globe default right.';
+32 -6
View File
@@ -48,6 +48,7 @@ import {
interruptCameraMotion,
adjustOrbitRange,
manualCameraInputCount,
moveCameraArgsError,
} from '../cameraVerbs.js';
import * as defaultFloorServices from '../data/groundFloor.js';
import { isPickedWorldPosition } from '../data/scenePick.js';
@@ -1198,8 +1199,26 @@ export function createGevActionRunner({
voiceFlight &&
!voiceFlight.done
) {
const flight = voiceFlight;
const status = await waitForVoiceFlight(flight, runOptions.signal);
// Bad arguments are refused now, not after waiting on the flight.
const argsError = moveCameraArgsError(args);
if (argsError) return argsError;
const deadline = Date.now() + VOICE_FLIGHT_WAIT_MS;
let flight = voiceFlight;
let status = await waitForVoiceFlight(
flight,
runOptions.signal,
deadline,
);
// A later fly-to in the same reply cancels this one: follow it to
// where the camera lands, within the same deadline.
while (status === 'cancelled' && voiceFlight !== flight && current()) {
flight = voiceFlight;
status = await waitForVoiceFlight(
flight,
runOptions.signal,
deadline,
);
}
if (!current())
return { ok: false, action: 'move_camera', cancelled: true };
if (status !== 'arrived')
@@ -2759,8 +2778,12 @@ function trackVoiceFlight(inputCount = manualCameraInputCount()) {
return flight;
}
/** Longest a motion waits for a flight (geocode plus flight) before giving up. */
const VOICE_FLIGHT_WAIT_MS = 30_000;
/**
* Longest a motion waits for its flight (geocode plus flight, which is about
* 2 s) before giving up. The wait also holds the tool output, and on Gemini
* the later tools in the turn.
*/
const VOICE_FLIGHT_WAIT_MS = 15_000;
/**
* Motions that act on the place the camera ends up at, so one sent beside a
@@ -2775,7 +2798,7 @@ const VOICE_FLIGHT_WAITING_MOTIONS = new Set([
]);
/** Wait for a voice flight to end; 'timeout' or 'aborted' if it does not. */
function waitForVoiceFlight(flight, signal) {
function waitForVoiceFlight(flight, signal, deadline) {
if (signal?.aborted) return Promise.resolve('aborted');
return new Promise((resolve) => {
const finish = (status) => {
@@ -2784,7 +2807,10 @@ function waitForVoiceFlight(flight, signal) {
resolve(status);
};
const onAbort = () => finish('aborted');
const timer = setTimeout(() => finish('timeout'), VOICE_FLIGHT_WAIT_MS);
const timer = setTimeout(
() => finish('timeout'),
Math.max(0, deadline - Date.now()),
);
signal?.addEventListener?.('abort', onAbort, { once: true });
flight.ended.then(finish);
});
+90 -6
View File
@@ -347,7 +347,8 @@ for (const motion of [
{ motion: 'pan', direction: 'left' },
{ motion: 'rotate', direction: 'right' },
]) {
test(`${motion.motion} in the same reply as a fly-to waits for the flight to land`, async () => {
test(`${motion.motion} in the same reply as a fly-to waits for the flight to land`, async (t) => {
t.after(() => interruptCameraMotion('test-cleanup'));
const { viewer, styleManager, order, flights } = tiltAfterFlightHarness();
const runner = createGevActionRunner({
viewer,
@@ -368,10 +369,10 @@ for (const motion of [
const result = await moved;
assert.equal(result.ok, true);
assert.equal(result.motion, motion.motion);
interruptCameraMotion('test-cleanup');
});
test(`${motion.motion} waits through a geocoded fly-to search, then for its flight`, async () => {
test(`${motion.motion} waits through a geocoded fly-to search, then for its flight`, async (t) => {
t.after(() => interruptCameraMotion('test-cleanup'));
const { viewer, styleManager } = tiltAfterFlightHarness();
let finishSearch;
let landFlight;
@@ -399,7 +400,6 @@ for (const motion of [
landFlight();
assert.equal((await moved).ok, true);
assert.equal(getActiveCameraMotion()?.kind, motion.motion);
interruptCameraMotion('test-cleanup');
});
test(`${motion.motion} waiting on a flight that never lands is not run`, async () => {
@@ -470,7 +470,8 @@ test('a drag during the first of two chained fly-tos still drops the queued moti
assert.match(result.error, /moved the camera during the flight/);
});
test('input before a fly-to does not drop the motion queued behind it', async () => {
test('input before a fly-to does not drop the motion queued behind it', async (t) => {
t.after(() => interruptCameraMotion('test-cleanup'));
const { viewer, styleManager, flights } = tiltAfterFlightHarness();
const canvasListeners = new Map();
viewer.scene.canvas.addEventListener = (type, listener) => {
@@ -486,7 +487,90 @@ test('input before a fly-to does not drop the motion queued behind it', async ()
const moved = runner('move_camera', { motion: 'orbit' });
flights[0].complete();
assert.equal((await moved).ok, true);
interruptCameraMotion('test-cleanup');
});
test('a motion follows a later fly-to in the same reply that cancels its flight', async (t) => {
t.after(() => interruptCameraMotion('test-cleanup'));
const { viewer, styleManager, flights } = tiltAfterFlightHarness();
const runner = createGevActionRunner({
viewer,
styleManager,
dataManager: { layers: new Map(), getAll: () => [] },
});
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
const moved = runner('move_camera', { motion: 'orbit' });
const movedDone = settledYet(moved);
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
// The second flight supersedes the first, as Cesium's flyTo does.
flights[0].cancel();
await new Promise((resolve) => setTimeout(resolve, 0));
assert.equal(movedDone(), false, 'the orbit must wait for the second flight');
flights[1].complete();
const result = await moved;
assert.equal(result.ok, true);
assert.equal(result.motion, 'orbit');
});
test('a stale motion does not follow a newer reply\'s fly-to', async () => {
const { viewer, styleManager, flights } = tiltAfterFlightHarness();
const runner = createGevActionRunner({
viewer,
styleManager,
dataManager: { layers: new Map(), getAll: () => [] },
});
let live = true;
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
const moved = runner('move_camera', { motion: 'orbit' }, { isCurrent: () => live });
// A newer reply takes over and flies elsewhere; this reply is stale.
live = false;
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
flights[0].cancel();
// The newer flight never lands here: following it would hold this answer.
const result = await Promise.race([
moved,
new Promise((resolve) => setTimeout(() => resolve('still waiting'), 50)),
]);
assert.notEqual(result, 'still waiting');
assert.equal(result.ok, false);
assert.equal(result.cancelled, true);
});
test('a motion gives up on a flight that has not landed in 15 s', async (t) => {
const { viewer, styleManager } = tiltAfterFlightHarness();
const runner = createGevActionRunner({
viewer,
styleManager,
dataManager: { layers: new Map(), getAll: () => [] },
});
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
t.mock.timers.enable({ apis: ['setTimeout', 'Date'] });
const moved = runner('move_camera', { motion: 'orbit' });
const movedDone = settledYet(moved);
await Promise.resolve();
t.mock.timers.tick(14_999);
await Promise.resolve();
assert.equal(movedDone(), false);
t.mock.timers.tick(1);
const result = await moved;
assert.equal(result.ok, false);
assert.match(result.error, /did not arrive/);
});
test('bad move_camera arguments are refused without waiting for a flight', async () => {
const { viewer, styleManager } = tiltAfterFlightHarness();
const runner = createGevActionRunner({
viewer,
styleManager,
dataManager: { layers: new Map(), getAll: () => [] },
});
assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true);
const moved = runner('move_camera', { motion: 'pan' });
const movedDone = settledYet(moved);
await new Promise((resolve) => setTimeout(resolve, 0));
assert.equal(movedDone(), true, 'the refusal must not wait on the flight');
const result = await moved;
assert.equal(result.ok, false);
assert.match(result.error, /needs a direction/);
});
test('stop does not wait for a voice flight', async () => {