fix(desktop): keep PTT escalation available on a new voice session#9748
Open
kodjima33 wants to merge 1 commit into
Open
fix(desktop): keep PTT escalation available on a new voice session#9748kodjima33 wants to merge 1 commit into
kodjima33 wants to merge 1 commit into
Conversation
#9734 gated `ask_higher_model` on every field of the prefetched kernel snapshot being non-empty, including the rendered context, the plan identities, and the semantic guidance. Rendered context is context *material*, not an availability signal. `KernelVoiceContextSnapshot.isResolved` says so explicitly: "A valid new conversation may render no text, but it still has a kernel session and a deterministic freshness identity." So on the first PTT turn of a session — nothing in the journal yet, so `renderedContext` is empty — the guard fell through to `.failed(kernel_context_unavailable)` and the user got a refusal instead of an answer. `RealtimeHubTools.escalationBody` already omits each empty section on its own (`testBodyOmitsContextSectionWhenEmpty`), so that path was unreachable, not unsafe. Gate on the snapshot's own resolution contract instead: a kernel session bound to the current owner scope plus a freshness identity. The `.empty` transport-failure sentinel and an owner-scope mismatch both still fail closed, which is what the guard was for. Verification: xcrun swift build -c debug (clean); swift test --filter HubEscalationTests (4/4, incl. 2 new); --filter "RealtimeHub|RuntimeOwnerIdentity|KernelTurnRecordedProjection|FloatingControlBarState" (164 tests, 0 failures); make preflight (8/8).
tianmind-studio
left a comment
Contributor
There was a problem hiding this comment.
The implementation matches the existing resolution contract: the current owner scope must provide a kernel session plus freshness identity, while empty context material remains valid and is already omitted by escalationBody. The focused Swift tests and Desktop Swift CI are green, and I do not see a blocking code issue in this diff. The remaining Hygiene failure is PR metadata: this path matches INV-CHAT-1 and INV-VOICE-1, so the body must name both under Product invariants affected instead of saying none. After that citation update and a green rerun, this looks ready for maintainer review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
After #9734, asking the floating bar a hard question over PTT on a new voice session fails instead of escalating. The user hears a refusal/error rather than an answer from the higher model.
Root cause
#9734 correctly started feeding the canonical kernel context into
ask_higher_model, but its availability guard requires every field of the prefetched snapshot to be non-empty:Rendered context is context material, not an availability signal.
KernelVoiceContextSnapshot.isResolvedstates the contract directly:On the first PTT turn of a session there is nothing in the journal yet, so
snapshot.renderedContextis empty — a perfectly resolved snapshot. The guard reads that as "kernel context unavailable" and hard-fails the tool.RealtimeHubTools.escalationBodywas already written to tolerate this: it filters empty sections out of the system message and has an explicitelse { cacheBoundary = "" }branch, covered bytestBodyOmitsContextSectionWhenEmpty. The new guard made that branch unreachable — the empty case was handled, not unsafe.Fix
Gate on the snapshot's own resolution contract — a kernel session bound to the current owner scope, plus a freshness identity — mirroring
KernelVoiceContextSnapshot.isResolved:voiceSessionContext(for:)already returns an all-empty context on owner-scope mismatch, and the prefetch only stores fields from a snapshot that passedisResolved. So the two cases the guard exists for — the.emptytransport-failure sentinel and a snapshot bound to a different owner — both still fail closed. Only "resolved, but this conversation has no context material yet" now proceeds, which is whatescalationBodyhandles.Tests
Two regression tests on the production decision function (
RealtimeHubController.VoiceSessionContext.isResolved):testResolvedSnapshotWithoutContextMaterialStillAllowsEscalation— bound session + freshness identity, empty rendered/guidance/plan → escalation allowed (this is the case that broke).testUnresolvedSnapshotBlocksEscalation—.emptysentinel and a rendered-but-sessionless context → still fail closed.Verification
xcrun swift build -c debug --package-path Desktop— cleanswift test --filter HubEscalationTests— 4/4 (2 new)swift test --filter "RealtimeHub|RuntimeOwnerIdentity|KernelTurnRecordedProjection|FloatingControlBarState"— 164 tests, 0 failuresmake preflight— 8/8 checks passNot exercised in a running app against a live model (no PTT hardware loop in this environment) — the escalation-availability decision is covered hermetically instead.
🤖 automated by hourly watchdog; opened for review, not merged.