From 9234c1b3d2d95b814059d814c36be54bf3acb1a5 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Tue, 10 Nov 2026 15:44:17 -0800 Subject: [PATCH] fix(voice): drop viewport decision codes the caller can never log, name the crop decision eligible (#835) The decision no longer reports not-entity-context or channel-closed, which its only caller rules out first, nor pointer-scale-not-useful, which the crop path already checks and logs. A closed channel returns before any record, as on main. The crop decision is pointer-crop-eligible, since the crop checks after it decide whether the pointer is on ground, and a sender called on a closed channel logs channel-lost. --- docs/CURRENT-STATE.md | 2 +- src/voice/realtimeViewport.js | 35 ++++++++++++----------- src/voice/realtimeViewportPolicy.test.mjs | 26 ++++++----------- 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 5c6e847fe..291d365d2 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -6147,7 +6147,7 @@ and unreachable upstream (502/504) separately from road geometry. - **Reliability**: tool-call dedupe (2.5s window across call/item/args keys); response-create queueing that respects active responses and defers follow-ups when the user starts speaking; per-tool follow-up instructions so the agent confirms only what actually happened (zoom confirms only on `ok=true`). - **Aircraft identity honesty:** “What is this aircraft?” reads callsign, operator, registration, type, and route only from the selected contact context. Missing operator, route, or type enrichment is named explicitly rather than silently omitted or inferred from the callsign. - **Diagnostics**: every client/server event is posted to `/api/realtime/debug-log` and appended to `.gev-logs/realtime-conversations.jsonl` (gitignored) with secret/image redaction; last 30 errors persist in `localStorage` (`gev-realtime-errors`); `window.__gevVoiceCommands.getDiagnostics()` in the console. - - **Visual-context escalation** (OpenAI Realtime): when a response's last tool result is a `get_entity_context` answer, a tool output from that response was sent and the data channel is open, it logs one `viewport_context.decision` with `contextId`, `send` and `reason`, plus `viewScale` (absent only for `channel-closed`) and, when `send` is true, `kind` (`viewport` or `crop`). Decision reasons: `structured-identity-sufficient` (structured context made an image unnecessary), `view-scale-not-local`, `structured-identity-missing` (requests a viewport), `pointer-ground-crop` (a pointer answer with no identity at `local` or `city` scale; requests a crop, and the snapshot checks below confirm the target is ground), `pointer-identity-present`, `pointer-missing`, `pointer-scale-not-useful` and `channel-closed`. A send decision is followed by exactly one outcome with the same `contextId` and `kind`: `viewport_context.result` (`sent`, `data-channel-send-failed`), logged as soon as the send returns, or `viewport_context.skipped`. Pointer-crop skips are `pointer-not-fresh` (no snapshot, or a stale one), `pointer-not-ground`, `pointer-off-screen`, `pointer-already-requested` (the most recently requested snapshot again: already sent, still being captured, or an earlier attempt for it threw), `pointer-expired`, `pointer-pose-unknown` and `pose-changed`; dropped captures are `capture-unavailable`, `session-reset`, `superseded` (a newer request or turn), `channel-lost` and `frame-stale`; a step that throws before an outcome closes the pair with `error`, and the caller then logs `viewport_context.failed`. The `channel-closed` decision and the `channel-closed` and `pointer-scale-not-useful` skip outcomes are defensive and not expected from the current caller; direct calls to the crop or image senders log those skips with a null `contextId`. Decision and outcome are separate records, so the debug-log rate limit can drop one half of a pair. Payloads are static codes only, never image data, tool payloads or user text, and other tool results log nothing here. Gemini Live uses a different gate in `geminiSession.js` (no pointer crop, a 1 s throttle), which is not instrumented. + - **Visual-context escalation** (OpenAI Realtime): when a response's last tool result is a `get_entity_context` answer, a tool output from that response was sent and the data channel is open, it logs one `viewport_context.decision` with `contextId`, `send` and `reason`, plus `viewScale` and, when `send` is true, `kind` (`viewport` or `crop`). Decision reasons: `structured-identity-sufficient` (structured context made an image unnecessary), `view-scale-not-local`, `structured-identity-missing` (requests a viewport), `pointer-crop-eligible` (a pointer answer with no identity; requests a crop, and the crop checks below decide whether one is sent), `pointer-identity-present` and `pointer-missing`. A send decision is followed by exactly one outcome with the same `contextId` and `kind`: `viewport_context.result` (`sent`, `data-channel-send-failed`), logged as soon as the send returns, or `viewport_context.skipped`. Pointer-crop skips are `pointer-not-fresh` (no snapshot, or a stale one), `pointer-not-ground`, `pointer-off-screen`, `pointer-scale-not-useful` (zoomed out past `local` or `city`), `pointer-already-requested` (the most recently requested snapshot again: already sent, still being captured, or an earlier attempt for it threw), `pointer-expired`, `pointer-pose-unknown` and `pose-changed`; dropped captures are `capture-unavailable`, `session-reset`, `superseded` (a newer request or turn), `channel-lost` and `frame-stale`; a step that throws before an outcome closes the pair with `error`, and the caller then logs `viewport_context.failed`. Direct calls to the crop or image senders log their skips with a null `contextId`. Decision and outcome are separate records, so the debug-log rate limit can drop one half of a pair. Payloads are static codes only, never image data, tool payloads or user text, and other tool results log nothing here. Gemini Live uses a different gate in `geminiSession.js` (no pointer crop, a 1 s throttle), which is not instrumented. - **Counting semantics ("near")** — a CONTRACT; new count-bearing tool work inherits it. Three honest numbers exist for one question: the Contacts cohort (250 km around the subject, what the panel shows), `analyst_query`'s count of _currently-loaded_ records for the requested scope, and the layer-wide loaded total in `coverage.layersQueried[].total`. They diverge legitimately — the flights layer loads by viewport, so after a camera dive the loaded set can hold a fraction of the cohort (field case: panel 42, analyst 8). The contract: 1. **Contacts ACTIVE** → "near / nearby / how many aircraft" means the **Contacts window** — the panel's numbers, spoken verbatim. Mechanism: `contactsWindow` (`{centeredOn, radiusKm, flights, military, vessels, complete}`), carried by both `analyst_query` and `get_current_view_state`, derived by `contactsWindowFromSnapshot()` from the same snapshot the panel renders so the two cannot drift. Voice filtering and follow-up memory use that immutable snapshot too. A cohort whose feed cannot answer reports `'unknown'`, never a confident zero; a cohort at the 20,000-row retention bound reports a lower bound. 2. **Contacts OFF** → "nearby" means **in view**; "near \" means a radius around that place. A radius query with Contacts active and no explicit centre is centred on the **active contact**, not the camera. diff --git a/src/voice/realtimeViewport.js b/src/voice/realtimeViewport.js index dc845da31..edfe9ab11 100644 --- a/src/voice/realtimeViewport.js +++ b/src/voice/realtimeViewport.js @@ -73,26 +73,23 @@ const POINTER_CROP_MAX_AGE_MS = 20000; const POINTER_CROP_SCALES = new Set(['local', 'city']); /** - * Reason-coded explanation of the existing visual escalation policy. + * Reason-coded explanation of the existing visual escalation policy, for an + * entity-context answer on an open channel (the caller checks both). * Pointer crops remain a separate path from full-frame screenshots. * These reasons are static diagnostics, never model-generated claims. */ -export function viewportContextDecision(result, { channelOpen = true } = {}) { - if (result?.action !== 'get_entity_context') - return { send: false, reason: 'not-entity-context' }; - if (!channelOpen) return { send: false, reason: 'channel-closed' }; - const viewScale = result.scene?.basemap?.viewScale ?? null; - if (result.scope === 'pointer') { +export function viewportContextDecision(result) { + const viewScale = result?.scene?.basemap?.viewScale ?? null; + if (result?.scope === 'pointer') { if (result.selected) return { send: false, reason: 'pointer-identity-present', viewScale }; if (!result.pointer) return { send: false, reason: 'pointer-missing', viewScale }; - if (!POINTER_CROP_SCALES.has(viewScale)) - return { send: false, reason: 'pointer-scale-not-useful', viewScale }; + // The crop path checks ground, freshness and scale itself, and logs why. return { send: true, kind: 'crop', - reason: 'pointer-ground-crop', + reason: 'pointer-crop-eligible', viewScale, }; } @@ -390,13 +387,17 @@ export class RealtimeViewport { * requests pair up. */ async sendVisualContextIfUseful(result) { - // Runs after every tool output. Only entity context can escalate, so - // other tools log nothing and leave the shared debug-log budget alone. - if (result?.action !== 'get_entity_context') return false; + // Runs after every tool output. Only entity context on an open channel + // can escalate, so anything else logs nothing and leaves the shared + // debug-log budget alone. + if ( + result?.action !== 'get_entity_context' || + !this.dc || + this.dc.readyState !== 'open' + ) + return false; const contextId = ++this.contextSeq; - const decision = viewportContextDecision(result, { - channelOpen: this.dc?.readyState === 'open', - }); + const decision = viewportContextDecision(result); this.debugLog?.('viewport_context.decision', { contextId, ...decision }); if (!decision.send) return false; const diag = diagnosticsRecord(contextId); @@ -481,7 +482,7 @@ export class RealtimeViewport { const report = (event, reason) => reportOutcome(this, diag, event, kind, reason); if (!this.dc || this.dc.readyState !== 'open') { - report('viewport_context.skipped', 'channel-closed'); + report('viewport_context.skipped', 'channel-lost'); return false; } const generation = this.generation; diff --git a/src/voice/realtimeViewportPolicy.test.mjs b/src/voice/realtimeViewportPolicy.test.mjs index 2a90b3795..94f8c9379 100644 --- a/src/voice/realtimeViewportPolicy.test.mjs +++ b/src/voice/realtimeViewportPolicy.test.mjs @@ -13,17 +13,6 @@ const localContext = (overrides = {}) => ({ }); test('viewport context reasons preserve the existing escalation gate', () => { - assert.deepEqual( - viewportContextDecision({ action: 'get_current_view_state' }), - { - send: false, - reason: 'not-entity-context', - }, - ); - assert.deepEqual( - viewportContextDecision(localContext(), { channelOpen: false }), - { send: false, reason: 'channel-closed' }, - ); assert.deepEqual( viewportContextDecision({ action: 'get_entity_context', @@ -87,7 +76,7 @@ test('pointer ground selects the #837 crop path, not a full viewport', () => { assert.deepEqual(viewportContextDecision(pointerContext), { send: true, kind: 'crop', - reason: 'pointer-ground-crop', + reason: 'pointer-crop-eligible', viewScale: 'city', }); assert.deepEqual( @@ -96,8 +85,9 @@ test('pointer ground selects the #837 crop path, not a full viewport', () => { scene: { basemap: { viewScale: 'global' } }, }), { - send: false, - reason: 'pointer-scale-not-useful', + send: true, + kind: 'crop', + reason: 'pointer-crop-eligible', viewScale: 'global', }, ); @@ -173,7 +163,7 @@ test('runtime dispatch preserves pointer crop and full image separation', async calls.some( (call) => call.event === 'viewport_context.decision' && - call.detail.reason === 'pointer-ground-crop', + call.detail.reason === 'pointer-crop-eligible', ), ); }); @@ -333,7 +323,7 @@ test('a crop decision that cannot send logs why, under the same id', async () => assert.equal(events.length, 2, reason); const [decision, outcome] = events; assert.equal(decision.event, 'viewport_context.decision'); - assert.equal(decision.detail.reason, 'pointer-ground-crop'); + assert.equal(decision.detail.reason, 'pointer-crop-eligible'); assert.deepEqual(outcome, { event: 'viewport_context.skipped', detail: { contextId: decision.detail.contextId, kind: 'crop', reason }, @@ -573,8 +563,8 @@ test('direct calls log their defensive skips without a decision id', async () => [...open.events, ...closed.events].map((item) => item.detail), [ { contextId: null, kind: 'crop', reason: 'pointer-scale-not-useful' }, - { contextId: null, kind: 'crop', reason: 'channel-closed' }, - { contextId: null, kind: 'viewport', reason: 'channel-closed' }, + { contextId: null, kind: 'crop', reason: 'channel-lost' }, + { contextId: null, kind: 'viewport', reason: 'channel-lost' }, ], ); });