diff --git a/changelog.d/1616-openai-chat-tool-patterns.md b/changelog.d/1616-openai-chat-tool-patterns.md new file mode 100644 index 000000000..225d3f4a5 --- /dev/null +++ b/changelog.d/1616-openai-chat-tool-patterns.md @@ -0,0 +1,4 @@ +- Typed chat on OpenAI answers again: OpenAI refused every message because + `resolve_area`'s area-id pattern uses Unicode classes Chat Completions does + not accept. Chat leaves those patterns out; area ids are still checked + before the tool runs (samehkhamis, #1616). diff --git a/changelog.d/1620-cctv-opening-camera.md b/changelog.d/1620-cctv-opening-camera.md new file mode 100644 index 000000000..10aa13f19 --- /dev/null +++ b/changelog.d/1620-cctv-opening-camera.md @@ -0,0 +1,3 @@ +- Turning CCTV on opens the camera nearest you instead of the first camera in + the catalog, which showed a Sydney camera in London. A share link opens on + the camera nearest the shared view once its flight lands (samehkhamis, #1620). diff --git a/server/providers/openai/chat.js b/server/providers/openai/chat.js index 413d69ba9..6ca4648d6 100644 --- a/server/providers/openai/chat.js +++ b/server/providers/openai/chat.js @@ -64,6 +64,30 @@ function validMessages(messages) { return !pending.length; } +/** + * A tool schema without the `pattern`s Chat Completions refuses: it answers + * 400 for a whole request whose tool uses Unicode property classes (`\p{L}`), + * as `resolve_area`'s area ids do. Every argument is still checked against + * the action's own contract before the tool runs, so the model loses a hint, + * not a rule. Realtime voice accepts these patterns and keeps them. + */ +export function chatSchema(schema) { + if (Array.isArray(schema)) return schema.map(chatSchema); + if (!object(schema)) return schema; + return Object.fromEntries( + Object.entries(schema) + .filter( + ([key, value]) => + !( + key === 'pattern' && + typeof value === 'string' && + /\\[pP]\{/.test(value) + ), + ) + .map(([key, value]) => [key, chatSchema(value)]), + ); +} + export function createOpenAiChatHandler({ annotationGuidance, tools = GEV_REALTIME_TOOLS, @@ -126,7 +150,12 @@ export function createOpenAiChatHandler({ tools: offeredTools(tools, { overpass }).map( ({ name, description, parameters }) => ({ type: 'function', - function: { name, description, parameters, strict: false }, + function: { + name, + description, + parameters: chatSchema(parameters), + strict: false, + }, }), ), }, diff --git a/src/data/cctv.js b/src/data/cctv.js index ad95f2794..ed3f56654 100644 --- a/src/data/cctv.js +++ b/src/data/cctv.js @@ -67,6 +67,8 @@ export const _extractPickedCameraIdForTest = export const _setCctvCoverageStateForTest = layer._setCctvCoverageStateForTest; export const focusCctvRecord = layer.focusCctvRecord; export const maybeAutoHop = layer.maybeAutoHop; +export const openingCameraId = layer.openingCameraId; +export const reopenNearestCamera = layer.reopenNearestCamera; export const cctvCycleIndex = layer.cctvCycleIndex; export { CCTV_CALIBRATION_STORAGE_KEY_V1, diff --git a/src/data/cctv.test.mjs b/src/data/cctv.test.mjs index f76acdb72..bcd5c48ab 100644 --- a/src/data/cctv.test.mjs +++ b/src/data/cctv.test.mjs @@ -61,6 +61,8 @@ import cctvLayer, { materializeCctvActiveCoverageEntities, materializeCctvVisibleCoverageEntities, maybeAutoHop, + openingCameraId, + reopenNearestCamera, prioritizeActiveCctvGeometryRecord, processCctvGeometryDrainBatch, processCctvGeometryQueueBatch, @@ -1296,6 +1298,120 @@ test('CCTV null-active coverage, auto-hop, cycling, and panel targets stay hones } }); +test('CCTV opens on the camera nearest the viewer, not the first in the catalog', () => { + // QA 2026-11-05: at King's Cross, turning CCTV on showed a Sydney camera, + // the first catalog record, instead of the TfL camera 0.3 km away. + const viewer = makeDeselectViewer(); + viewer.camera.positionCartographic = Cesium.Cartographic.fromDegrees( + -0.1209, + 51.5295, + 187, + ); + viewer.camera.positionWC = Cesium.Cartesian3.fromDegrees( + -0.1209, + 51.5295, + 187, + ); + const sydney = makeDeselectRecord('au-sydney'); + sydney.camera = { ...sydney.camera, lat: -34.03, lon: 151.11 }; + const london = makeDeselectRecord('gb-kings-cross'); + london.camera = { ...london.camera, lat: 51.5318, lon: -0.1209 }; + try { + _setCctvCoverageStateForTest({ + viewer, + records: [sydney, london], + activeCameraId: null, + enabled: false, + }); + assert.equal(openingCameraId(), 'gb-kings-cross'); + _setCctvCoverageStateForTest({ + viewer: null, + records: [sydney, london], + activeCameraId: null, + enabled: false, + }); + assert.equal( + openingCameraId(), + 'au-sydney', + 'with no viewer position the first record is the fallback', + ); + _setCctvCoverageStateForTest({ viewer, records: [], enabled: false }); + assert.equal(openingCameraId(), null); + } finally { + _setCctvCoverageStateForTest({ enabled: false }); + } +}); + +test('the opening camera follows a shared view that lands after CCTV turned on, until someone picks one', () => { + // A share link turns CCTV on before its flight lands, so enable() picks + // from the startup view; the flight's settled move re-picks near London. + const viewer = makeDeselectViewer(); + viewer.camera.positionCartographic = Cesium.Cartographic.fromDegrees( + -0.1209, + 51.5295, + 187, + ); + const sydney = makeDeselectRecord('au-sydney'); + sydney.camera = { ...sydney.camera, lat: -34.03, lon: 151.11 }; + const london = makeDeselectRecord('gb-kings-cross'); + london.camera = { ...london.camera, lat: 51.5318, lon: -0.1209 }; + for (const record of [sydney, london]) + Object.assign(record, { + groundResolved: {}, + groundSamples: {}, + groundPrior: 0, + frustumGeometry: null, + frustumPositions: null, + probeClampRangeM: null, + }); + _setCctvOverlayHostForTest({ + setEntries() {}, + setVisible() {}, + clearSource() {}, + }); + // Activation builds the projection canvas and frame image. + const originalDocument = globalThis.document; + const originalImage = globalThis.Image; + globalThis.document = { + createElement: () => ({ getContext: () => null }), + }; + globalThis.Image = class {}; + try { + _setCctvCoverageStateForTest({ + viewer, + records: [sydney, london], + activeCameraId: 'au-sydney', + enabled: true, + openingCameraProvisional: true, + }); + assert.equal(reopenNearestCamera(), true); + assert.equal(cctvLayer.getUIState().activeCameraId, 'gb-kings-cross'); + assert.equal(reopenNearestCamera(), false, 'already the nearest'); + + // Someone chooses a camera: settled moves leave it alone. + setActiveCamera('au-sydney'); + assert.equal(reopenNearestCamera(), false); + assert.equal(cctvLayer.getUIState().activeCameraId, 'au-sydney'); + + // A deselect is a choice too. + _setCctvCoverageStateForTest({ + viewer, + records: [sydney, london], + activeCameraId: 'au-sydney', + enabled: true, + openingCameraProvisional: true, + }); + deactivateActiveCamera(); + assert.equal(reopenNearestCamera(), false); + assert.equal(cctvLayer.getUIState().activeCameraId, null); + } finally { + globalThis.document = originalDocument; + globalThis.Image = originalImage; + _setCctvOverlayHostForTest(); + _setCctvCoverageStateForTest({ enabled: false }); + } +}); + test('CCTV disable clears and hides its shared-host source through the real layer lifecycle', () => { const calls = []; _setCctvOverlayHostForTest({ diff --git a/src/data/sharedOsmDisplays.test.mjs b/src/data/sharedOsmDisplays.test.mjs index 273a9dc56..46b5d8894 100644 --- a/src/data/sharedOsmDisplays.test.mjs +++ b/src/data/sharedOsmDisplays.test.mjs @@ -85,7 +85,7 @@ test('Warendorf webcam display introduces the shared OSM credit', (t) => { const v = viewer(t), empty = new Proxy({}, { get: () => noop }); const lifecycle = createCctvLifecycle({ state: { _viewer: v, _records: [{ camera: { id: 'warendorf-marktplatz-rathaus', cityId: 'warendorf' } }], _recordById: new Map(), _cctvOverlayHost: overlayHost }, services: { credits: { showOsmCredit, hideOsmCredit }, sprites: empty, activation: empty, picking: empty, focus: empty, render: empty }, - parts: { selection: empty, geometryQueue: empty, rendering: empty, projection: empty, cards: empty, presentation: empty }, source: {} }); + parts: { selection: empty, geometryQueue: empty, rendering: empty, projection: empty, cards: empty, presentation: empty, navigation: empty }, source: {} }); lifecycle.methods.enable(); assertCredit(v); lifecycle.methods.disable(); assert.equal(v.credits.filter(c => c.showOnScreen).length, 0); }); diff --git a/src/layers/cctv/index.js b/src/layers/cctv/index.js index 6f1cc77c7..36e1360b4 100644 --- a/src/layers/cctv/index.js +++ b/src/layers/cctv/index.js @@ -112,6 +112,8 @@ export function createCctvLayer({ services, source }) { _setCctvCoverageStateForTest: parts.testing._setCctvCoverageStateForTest, focusCctvRecord: parts.navigation.focusCctvRecord, maybeAutoHop: parts.navigation.maybeAutoHop, + openingCameraId: parts.navigation.openingCameraId, + reopenNearestCamera: parts.navigation.reopenNearestCamera, cctvCycleIndex: parts.navigation.cctvCycleIndex, }, ); diff --git a/src/layers/cctv/lifecycle.js b/src/layers/cctv/lifecycle.js index 4e4443df8..0a10d0090 100644 --- a/src/layers/cctv/lifecycle.js +++ b/src/layers/cctv/lifecycle.js @@ -151,6 +151,7 @@ export function createLifecycle({ layerState._enabled = false; layerState._activeCameraId = null; layerState._autoHopSuspended = false; + layerState._openingCameraProvisional = false; layerState._lastHopAt = 0; layerState._lastViewContext = ''; layerState._calibrationById = parts.calibration.loadCalibrationStore(); @@ -299,11 +300,8 @@ export function createLifecycle({ } layerState._count = layerState._records.length; - if (layerState._records.length > 0) { - // Projection runtime + first frame fetch are deferred to enable() so - // initializing the catalog stays render-cheap. - layerState._activeCameraId = layerState._records[0].camera.id; - } + // No camera is picked here: enable() opens on the one nearest the + // viewer, unless restored params selected one first. // Task 5: if the prior batch lost init's bounded race, apply it post-hoc // when it lands (pure recomputes — applyLateGroundPriors guards against @@ -361,6 +359,7 @@ export function createLifecycle({ layerState._horizonCullListener = () => { layerState._cameraMoving = false; stopViewMotionTimer(); + parts.navigation.reopenNearestCamera(); settleView(); }; layerState._viewer.camera.moveEnd.addEventListener( @@ -508,8 +507,9 @@ export function createLifecycle({ return Boolean(coverage && layerState._recordById.has(coverage[1])); }); if (!layerState._activeCameraId && layerState._records.length) { - layerState._activeCameraId = layerState._records[0].camera.id; + layerState._activeCameraId = parts.navigation.openingCameraId(); layerState._autoHopSuspended = false; + layerState._openingCameraProvisional = true; } const activeRecord = parts.selection.getActiveRecord(); if (activeRecord) { @@ -629,6 +629,7 @@ export function createLifecycle({ layerState._enabled = false; layerState._activeCameraId = null; layerState._autoHopSuspended = false; + layerState._openingCameraProvisional = false; // Clear existing subscribers rather than replacing the Set — // replacing would silently orphan any unsubscribe() closures layerState._listeners.clear(); diff --git a/src/layers/cctv/navigation.js b/src/layers/cctv/navigation.js index 6c576bb4f..5ee3ed2a4 100644 --- a/src/layers/cctv/navigation.js +++ b/src/layers/cctv/navigation.js @@ -33,6 +33,36 @@ export function createNavigation({ return best?.id || null; } + /** + * The camera the layer opens on when none is selected: the one nearest the + * viewer, or the first record when the viewer has no position yet. + * @returns {string|null} Camera ID, or null when the catalog is empty. + */ + + function openingCameraId() { + return ( + nearestCameraIdToViewer() || layerState._records[0]?.camera?.id || null + ); + } + + /** + * After a settled move, moves the opening camera to the one nearest the + * view, until someone chooses a camera. A shared view's flight lands after + * enable() picked from the startup view. + * @returns {boolean} True when the active camera changed. + */ + + function reopenNearestCamera() { + if (!layerState._openingCameraProvisional || !layerState._enabled) + return false; + const nearest = nearestCameraIdToViewer(); + if (!nearest || nearest === layerState._activeCameraId) return false; + parts.selection.setActiveCamera(nearest); + // setActiveCamera ends the opening pick; this is still that pick. + layerState._openingCameraProvisional = true; + return true; + } + /** * Flies the Cesium viewer camera to frame the specified CCTV camera, * looking along its heading from above. @@ -149,6 +179,8 @@ export function createNavigation({ } return { nearestCameraIdToViewer, + openingCameraId, + reopenNearestCamera, focusCctvRecord, focusCamera, maybeAutoHop, diff --git a/src/layers/cctv/selection.js b/src/layers/cctv/selection.js index c86425fd0..f4c8f639e 100644 --- a/src/layers/cctv/selection.js +++ b/src/layers/cctv/selection.js @@ -83,6 +83,7 @@ export function createSelection({ } layerState._activeCameraId = cameraId; layerState._autoHopSuspended = false; + layerState._openingCameraProvisional = false; // A real activation creates projection work — wake the self-stopping loop. parts.projection.startProjectionLoop(); if (previousActiveRecord && previousActiveRecord !== record) { @@ -169,6 +170,7 @@ export function createSelection({ if (!record) return false; layerState._activeCameraId = null; layerState._autoHopSuspended = true; + layerState._openingCameraProvisional = false; record.activationDone = false; parts.geometry.clearProbeClampOnDeactivation(record, (previous) => { parts.geometry.applyFrustumGeometry( diff --git a/src/layers/cctv/state.js b/src/layers/cctv/state.js index 78c2b8fa9..5e70c814f 100644 --- a/src/layers/cctv/state.js +++ b/src/layers/cctv/state.js @@ -36,6 +36,12 @@ export function createState({ services }) { layerState._autoHop = false; + // True while the active camera is the one enable() opened on and nobody has + // chosen a camera since: each settled move then re-picks the one nearest + // the view, so a share flight that lands after enable still opens nearby. + + layerState._openingCameraProvisional = false; + // An explicit empty-space deselect keeps AUTO HOP configured but prevents its // timer from silently choosing a replacement. A later explicit activation or // AUTO HOP toggle-on releases the hold. diff --git a/src/layers/cctv/testing.js b/src/layers/cctv/testing.js index c4aada9bb..207256f36 100644 --- a/src/layers/cctv/testing.js +++ b/src/layers/cctv/testing.js @@ -91,6 +91,7 @@ export function createTesting({ state: layerState, services, parts, source }) { coverageMode = 'on', showProjection = false, healthById = new Map(), + openingCameraProvisional = false, } = {}) { layerState._viewer = viewer; layerState._records = Array.isArray(records) ? records : []; @@ -104,6 +105,7 @@ export function createTesting({ state: layerState, services, parts, source }) { layerState._billboards = null; layerState._activeCameraId = activeCameraId; layerState._autoHopSuspended = false; + layerState._openingCameraProvisional = !!openingCameraProvisional; layerState._enabled = !!enabled; layerState._coverageMode = parts.model.normalizeCoverageMode( coverageMode, diff --git a/src/textChatServer.test.mjs b/src/textChatServer.test.mjs index 61b09e987..b7137bbdd 100644 --- a/src/textChatServer.test.mjs +++ b/src/textChatServer.test.mjs @@ -3,7 +3,10 @@ import assert from 'node:assert/strict'; import http from 'node:http'; import { once } from 'node:events'; import { createGeminiChatHandler } from '../server/providers/gemini/chat.js'; -import { createOpenAiChatHandler } from '../server/providers/openai/chat.js'; +import { + chatSchema, + createOpenAiChatHandler, +} from '../server/providers/openai/chat.js'; import { GEV_REALTIME_TOOLS } from '../server/providers/openai/tools.js'; import { realtimeDirectives, @@ -534,6 +537,11 @@ for (const [name, provider] of Object.entries(providers)) { assert.equal(offered.includes('osm_query'), overpass); assert.equal(prompt.includes('osm_query'), overpass); assert.ok(offered.includes('resolve_area')); + // Chat Completions answers 400 invalid_function_parameters for a + // pattern with Unicode property classes, as resolve_area's area ids + // use; OpenAI chat leaves those patterns out. + if (name === 'openai') + assert.doesNotMatch(JSON.stringify(body.tools), /\\\\[pP]\{/); } }); test(`${name} throttled replies say they are rate limited`, async (t) => { @@ -697,3 +705,25 @@ test('OpenAI empty tool arrays are removed before history is returned', async (t const body = await (await f.send()).json(); assert.equal(body.content.tool_calls, undefined); }); + +test('OpenAI chat schemas drop only the patterns Chat Completions refuses', () => { + assert.match(JSON.stringify(GEV_REALTIME_TOOLS), /\\\\[pP]\{/); + assert.deepEqual( + chatSchema({ + type: 'object', + properties: { + id: { type: 'string', pattern: '^ne:[\\p{L}]+$', maxLength: 80 }, + code: { type: 'string', pattern: '^[A-Z]{2}$' }, + list: { type: 'array', items: [{ pattern: '\\P{N}' }] }, + }, + }), + { + type: 'object', + properties: { + id: { type: 'string', maxLength: 80 }, + code: { type: 'string', pattern: '^[A-Z]{2}$' }, + list: { type: 'array', items: [{}] }, + }, + }, + ); +});