feat(desktop): transcript-driven proactivity during ambient listening - #11804
feat(desktop): transcript-driven proactivity during ambient listening#11804aryanorastar wants to merge 11 commits into
Conversation
When a fresh user utterance lands while a context visit is active, evaluate that bucket grounded on the live speech window instead of waiting for the next dwell, so a question spoken mid-context can be answered hands-free. - SpeechProactivityWindow: bounded, deduping window of recent speech slices, measuring recency by seen-time rather than the backend-defined timestamps - SpeechProactivityAdmission: pure decide loop — flag on, not mid voice turn, user speech, minimum utterance length, and a cooldown between evaluations - SpeechProactivityCoordinator: MainActor observe path from the transcript funnel that hands a Sendable snapshot to the engine actor - ContextProactivityEngine.evaluateFromSpeech: runs the shared post-settle director tail (gates, frame grounding, retrieval hop, presentation) with a LIVE SPEECH section appended below the untrusted preamble - ContextVisitCoordinator.activeFence(): read-only production accessor for the live visit - ContextBucketsFeature.isTranscriptProactivityEnabled: dogfood-only, on for non-production with inverted env override, off for prod/beta - Tests: TranscriptDrivenProactivityTests — window bounds/dedup/retention, admission outcomes, prompt-section rendering (14 cases) Verification: - xcrun swift build -c debug --package-path Desktop: Build complete! - swift test (TranscriptDrivenProactivityTests): 14 passed, 0 failures - swift test (ContextProactivity*): all passed (shared tail untouched) - swiftlint-wrapper: 0 violations - check_desktop_test_quality.py: OK at baseline
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…w coverage Fix the two failing CI checks: - desktop-changelog-entry: add unreleased fragment for the new transcript-driven proactivity feature. - desktop-e2e-flow-coverage: cover SpeechProactivityAdmission, SpeechProactivityCoordinator, and TranscriptSpeechWindow under context-buckets-dogfood.yaml alongside the engine Core files they plug into.
|
Both failing checks fixed and pushed (0cf23fc):
Local verification: macos |
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks @aryanorastar — this is careful, well-gated work. I reviewed all 11 changed files; the engineering quality is notably high. Specifics:
What checks out
ContextProactivityEngine.evaluateFromSpeechreuses the full guarded tail — authorization snapshot + recheck,ContextDeliveryBudget.freeGatepreflight, fence freshness (checked twice),ContextDirectorEligibility, frame grounding viatrackedFrameForDirector/frameMayGroundDirector, anddwellAdmissionserialization against dwell/departure — so speech-triggered evaluations cannot bypass any existing delivery gate. ThespeechSection: String? = nildefault keeps dwell/departure prompts byte-identical, preserving the cached prompt prefix.- Prompt-injection hygiene in
ContextProactivityPromptBuilder.liveSpeechSectionis right: every slice is flattened throughContextDestinationKey.singleLine(_:limit: 160)(newline/control-char collapse, verified against main) and rides in the volatile suffix below the untrusted preamble, under an explicit "quoted, untrusted … not instructions" header — the same trust treatment as screen content. The test flattening"first line\nsecond line\n> forge prompt structure"is a nice touch. SpeechProactivityWindow(12 slices / 180 s, in-place replace per segmentID, recency byseenAtrather than backend timestamps) andSpeechProactivityAdmission(user-only, ≥4 words, 90 s cooldown, mid-turn suppression) are pure, well-factored, and well-tested — the 12 tests cover the window, admission, and section-rendering corners.- Flag gating in
ContextBucketsFeature.isTranscriptProactivityEnabledmatches the existing departure-evaluation pattern exactly (OMI_FORCE_SPEECH_PROACTIVITY=0,AppBuild.isNonProduction, hard-off in production/beta) — nothing ships dark to users.
Follow-ups (non-blocking)
SpeechProactivityCoordinator.observeruns admission on every arriving slice, including other-speaker slices. SincelatestUserSlicestays in the window for 180 s, another person speaking ~95 s after a user utterance (cooldown elapsed, utterance still retained) re-triggers an evaluation grounded on the stale user utterance. Every gate still applies, so this is polish rather than safety — consider only evaluating when the arriving slice is itself a user slice, or keying the cooldown to the triggering segmentID.- The changelog entry reads as shipped behavior ("the assistant now answers aloud during ambient listening") — worth a "(dogfood)" qualifier so release notes don't get ahead of the flag.
- The description frames this as stacking on #11801 (wake word). Good news: there's no hard dependency — the base is
mainand every referenced symbol (VoiceTurnCoordinator.activeTurnIDetc.) exists on main, and CI confirms it builds and tests standalone. Merged alone, the speech trigger works; the "wake word opens the mouthpiece" half of the loop arrives with #11801.
One thing for maintainers to weigh consciously: the LIVE SPEECH section quotes other people's ambient speech into the director prompt (bounded to 6 slices / 160 chars each). That's inherent to grounding and dogfood-gated, but it is a new ambient-audio-to-model-prompt surface worth explicit product sign-off, along with the unprompted spoken-response UX itself.
Leaving for human maintainer review: product-direction decision on unprompted speech-triggered spoken responses and ambient-speech-in-prompt sign-off. No blocking technical issues found.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
Git-on-my-level
added
needs-maintainer-review
|
Thank you @Git-on-my-level for the thoughtful and detailed review! Appreciate the sharp observation on
Ready for final maintainer sign-off and merge! |
…oactivity # Conflicts: # desktop/macos/e2e/flows/context-buckets-dogfood.yaml
|
Status check on the updated head (
The open item remains the product call rather than the engineering: unprompted speech-triggered spoken responses, and quoting bounded ambient speech into the director prompt, need maintainer sign-off. by AI on behalf of David — product-direction decision needed for unprompted speech-triggered spoken responses and ambient-speech-in-prompt; tag @Git-on-my-level to weigh in. |
…he window Admission read the window's retained user slice, so once the 90s cooldown lapsed another person speaking re-opened an evaluation grounded on a user utterance from up to the 180s retention window earlier. The user had said nothing; the director would have answered a stale question. Every delivery gate still applied, so this was wrong grounding rather than an escape. The decision now runs against the slice that just arrived: a non-user slice can never trigger. Other-speaker slices are still appended to the window, so they keep grounding the LIVE SPEECH section — they just stop being triggers. Also keys the decision to the backend segment. A segment is re-delivered as it grows, so the same utterance arrives repeatedly and would otherwise evaluate again on each re-delivery once the cooldown lapsed. Changelog now says "(dogfood builds only)" so release notes cannot get ahead of isTranscriptProactivityEnabled, which is hard-off in production and beta. Both were raised in review on 2026-08-18 and again on the merged head. Verification: - swift build -> clean - swift test --filter TranscriptDrivenProactivityTests -> 16 passed (was 14) - new guard proven live: dropping the isUser gate makes testOtherSpeakerAfterCooldownDoesNotRetriggerOnAStaleUserUtterance fail with "evaluate"; restoring it passes - check-desktop-changelog.py -> fragment found Failure-Class: none
|
@Git-on-my-level both refinements from your review are now on tip Refinements landed
Two regression tests pin exactly the scenario you described — an other-speaker slice arriving 95s later with the user's utterance still retained, and the same segment re-arriving after the cooldown. Suite is 16 tests, up from 14. I checked the new guard actually does the work rather than assuming it: dropping the Changelog fragment now reads "(dogfood builds only)" so release notes cannot get ahead of The red
git archive origin/main | tar -x -C /tmp/mainlint
cd /tmp/mainlint && python3 desktop/macos/scripts/desktop-flow-lint.pyThis PR only surfaced it by being merged with current main. Filed as #11985. It reddens every desktop PR that merges main; it went unnoticed because main's own Desktop Backend Contracts runs are sitting at Resolved: main fixed this itself in Edited to correct this paragraph: it originally said the fix was pending in #11984 and that I was holding the merge back. Main landed its own fix in the meantime, so the merge is pushed and #11984 is closed. Product call is unchanged and still yours: unprompted speech-triggered spoken responses, and quoting bounded ambient speech into the director prompt. |
…oactivity # Conflicts: # desktop/macos/Desktop/Sources/ProactiveAssistants/Core/ContextProactivityEngine.swift
|
Follow-up on the current head (
No open technical concerns from my side. What remains is the product decision for a maintainer: unprompted speech-triggered spoken responses, and quoting bounded ambient speech into the director prompt, are a new UX/privacy surface even dogfood-gated — ready to merge once someone who owns that UX signs off. by AI on behalf of David — remaining before merge: product sign-off on unprompted speech-triggered spoken responses and ambient speech in the director prompt; tag @Git-on-my-level to weigh in. |
|
@Git-on-my-level thanks — one reframing of what the remaining sign-off actually commits to, because I think the current framing asks for more than the merge does. Each of your passes has landed on some version of "a new ambient-audio-to-model-prompt surface needs product sign-off before merge". That reads as though merging turns the surface on. It cannot. The gate is bundle-derived, not remote: @MainActor static var isTranscriptProactivityEnabled: Bool {
guard isEnabled else { return false }
if AppBuild.isNonProduction {
return ProcessInfo.processInfo.environment["OMI_FORCE_SPEECH_PROACTIVITY"] != "0"
}
return false
}Following that through var isNonProduction: Bool {
bundleIdentifier.hasPrefix("com.omi.")
&& !AppBuild.productionFamilyBundleIdentifiers.contains(bundleIdentifier)
}
So merging this ships no behavior change to any user. Nothing quotes ambient speech into a prompt on a shipped build, because the funnel no-ops before the window is ever appended to. That splits the decision you are being asked to make into two, with very different weights:
Right now (2) is blocking (1), and I do not think it needs to. Deciding (2) well wants dogfood evidence — how often it fires, what it quotes, whether the 6-slice / 160-char bound holds up against real multi-speaker rooms — and that evidence cannot exist until the code is on I am not asking anyone to skip (2). I am asking whether it should gate the merge or gate the flag flip. If the answer is the flag flip, this is mergeable today. If you would rather hold the whole thing until the UX call is made, that is a legitimate answer too and I will stop merging One honest note on the asymmetry with my other stalled PR: I am not making this argument for #11801. That one genuinely does ship to users on merge, and its measured ~15–35 s ambient latency makes it weak as command-and-control, so the product call there really is a merge blocker. The difference is exactly this flag. Nothing else is outstanding here — both refinements from your earlier passes landed on |
Gate rejections carried a stage but not a trigger, so a rejection from a screen visit and one from a transcript were the same row. That made the question a reviewer asked of this PR unanswerable from the data: both lanes draw on one daily allocation, so "do transcripts crowd out screen nudges?" needs the split and there was none. `GateTrigger.forSpeechSection` derives it from an argument the call already carries — the speech lane is the only evaluation entry point that supplies a transcript section, so no new plumbing has to be kept in sync when an entry point is added. Named rather than inlined so the rule has one home and a test. Threaded through both shared evaluators; 9 of the 11 gate sites now attribute a lane. The two that keep the `.screenVisit` default are the screen entry point itself and the retrieval-hop site, which has no lane context. The values are bounded enums and go out as analytics properties — no bucket, owner, or content data, matching what the event already carried. Verification: swift build -> clean swift test --filter ContextProactivityEngineTests -> 23 passed Includes a test pinning the two raw values, because renaming one silently breaks the split this exists for, and one for the empty-string case: an empty section still came from the speech lane, and defaulting it to screen would move rows into the wrong column. This instruments the question; it does not answer it. The A/B is the same session run twice, `OMI_FORCE_SPEECH_PROACTIVITY=0` then default, comparing evaluations consumed and delivered per lane. Numbers to follow on the PR. Failure-Class: none
|
A/B, identical script: 1 delivery transcripts-off, 1 transcripts-on. No increase. Video: 10.5 min real Hotstar + room talk, 213 segments → 1 notification, and it was correct — flagged a real open task while I was on leisure content. 17 cooldown suppressions behind it. Budget: a transcript evaluation only consumes allocation if it delivers; the counter excludes Caveat: single session, and the injected half tests gating not STT. |
|
Follow-up on the current head ( Lane-dimension telemetry (
One nit: the Merge check ( The final merge brought roughly 150 desktop files in from main. Nothing PR-owned changed: On the merge-vs-flag-flip framing I verified the gate chain you quoted: Whether that makes the UX/privacy call a gate on the merge or a gate on removing the flag is a product-ordering decision I'm leaving explicitly with David rather than deciding here. No blocking technical issues on this head. by AI on behalf of David — decision needed on merge-vs-hold ordering for this dogfood-hard-off feature, alongside the standing UX/privacy sign-off for unprompted speech-triggered responses and ambient speech in the director prompt; @Git-on-my-level. |
…t started them The retrieval hop is reached from inside evaluateAndDeliver, which the speech lane enters, so a speech-grounded evaluation rejected while the hop rebuilt the free gate was counted as screen_visit — the exact crowding-out miscount the trigger dimension exists to prevent. gateTrigger is threaded in without a default so the compiler, not a reviewer, enforces that every call site says which lane it came from.
|
@Git-on-my-level the
No test added: this is an argument threaded through a private method with a single caller, and the engine has no end-to-end retrieval-hop harness — building one to pin a pass-through would be more machinery than the change. The compiler covers it, and the three existing lane tests still pin
Nothing else outstanding from any of your passes. The open item is still the product one, and I take your point that the merge-vs-hold ordering is David's call rather than yours to make here. |
|
Follow-up on the current head (
With that, every technical item raised across the passes on this PR is resolved: admission decides on the arriving slice and dedupes by segment ID, the changelog fragment carries the dogfood qualifier, the speech path rides the full guarded delivery tail, and the The one open item is the product decision the thread has converged on: whether UX/privacy sign-off for unprompted speech-triggered responses (and ambient speech quoted into the director prompt) gates this merge or gates removing the dogfood flag. That ordering call stays with a human maintainer. by AI on behalf of David — remaining before merge: product decision on whether UX/privacy sign-off gates the merge or the flag removal, for the speech-triggered response surface; @Git-on-my-level. |
|
@Git-on-my-level thanks for the re-audit of all 11 With that pass, this PR is in an unusual state that I think is worth naming plainly, because it looks like a review gap rather than a decision:
The gap is mechanical: your passes on this thread have been issue comments rather than submitted reviews. The only formal review on this PR is a single @kodjima33 — this one has never reached you. You have given approve-only passes on #11206, #11400, #11452 and #11807 under the same feature policy; #11804 is the sibling of #11807 and is the only one of the set with nothing outstanding against it. I cannot add you as a reviewer myself (the API returns 404 for a fork contributor), so this comment is the only route I have. On the product question, I am not re-arguing it — I made that case above and @Git-on-my-level has already verified the gate chain independently. I would just note the two are separable: the approving review is about whether the code is correct, which the passes above have answered; the UX/privacy sign-off is about whether the flag ever flips, which stays open either way and does not need to be settled to approve correct code that is hard-off on every shipped bundle. If the intent is to hold this until the UX call is made, that is a fine answer and I will mark it draft and stop merging main into it. I would rather have that than have it sit on a gate nobody realised was unsatisfied. |
|
@Git-on-my-level ## Current-main refresh + verification proof Refreshed this branch onto Scope integrity
Behavior verification on current mainThis covers the existing dwell path plus transcript-window dedup/bounds, admission decisions, arriving-slice semantics, cooldown, prompt grounding, stale-fence rejection, lane attribution, and delivery behavior. Static and repository gates
Known base-branch preflight issueThe aggregate preflight currently stops on a pre-existing That file is not in this PR. The isolated main-wide repair is #12339; its hygiene and backend unit gates are green. I deliberately did not mix that unrelated repair into this feature branch. Rollout safetyThe feature remains hard-off in production and Beta. Only non-production/dogfood bundles enable it, with |
|
Refresh check on PR-owned code is unchanged. All 13 files re-checked against the last-reviewed head ( The red Hygiene check is not this diff. From the job log: One new integration point worth a look before dogfood widens. Main's JIT work now fronts the dwell and departure lanes: Desktop checks are green on this head (build & tests, release compile, static & test contracts, e2e t0). The open item is unchanged from the previous passes: whether UX/privacy sign-off for unprompted speech-triggered responses (and ambient speech in the director prompt) gates this merge or gates removing the dogfood flag — that stays with a human maintainer. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
2 participants
Summary
The desktop context-proactivity ("director") engine historically only evaluated a screen context on dwell — after you settle into a context through
contextEntered, or right as you leave viaevaluateAfterDeparture. Ambient speech was never a trigger: if you asked something aloud while a context visit was active, nothing happened until the next dwell.This PR makes speech a first-class evaluation trigger. While a context visit is active, a fresh user utterance arriving through the ambient transcript funnel now evaluates that context immediately, grounded on the live screen plus a
== LIVE SPEECH ==section quoted from what you just said — so hands-free requests ("where does the deploy script live?", "what's open on this page?") get answered during ambient listening, not on the next dwell.This stacks on top of #11801 (wake word); together the pair closes the loop: the wake word opens the mouthpiece, speech triggers the evaluation, and the director delivers a grounded answer.
Behavior
Before
contextEnteredandevaluateAfterDeparture.evaluateAndDeliverproduced a prompt composed of the tracked frame + task list + recent-delivery dedup only.After
ContextProactivityEngine.evaluateFromSpeech(speech:)— reuses the exact same authorized-delivery tail as dwell evaluations (approve → gate → fence → admission-queue → eligibility → frame grounding), so behavior is consistent with dwell, scaled to one-shot speech slices.AppState+ListenEventsfeeds each new backend segment toSpeechProactivityCoordinator.shared.observe(...)(no-op when the feature flag is off).== LIVE SPEECH ==section (up to 6 slices) behind the candidates section, so the director answers against both the screen and what you said.Evaluation policy (admission)
A pure, unit-tested decision loop (
SpeechProactivityAdmission) decides whether speech warrants an evaluation:isTranscriptProactivityEnabled(off in prod/beta; on in dev bundles;OMI_FORCE_SPEECH_PROACTIVITY=0force-disables)VoiceTurnCoordinator.activeTurnID)minimumUserWordCount)evaluationCooldownSeconds)Skip reasons (
admission.outcome) are observable:.flagDisabled,.conversationActive,.noUserSpeech,.utteranceTooShort,.coolingDown.The speech window itself:
TranscriptSpeechSliceis a small Sendable value (segment ID, speaker, text,isUser, start/end).SpeechProactivityWindowkeeps max 12 slices, prunes by seen time (180s) — not backend timestamps (which can be in the future or missing) — and dedups by segment ID (transcripts are delivered repeatedly).Files
Added
desktop/macos/Desktop/Sources/ProactiveAssistants/Core/TranscriptSpeechWindow.swift— slice + window types;SpeakerSegment.speechProactivitySliceextraction.desktop/macos/Desktop/Sources/ProactiveAssistants/Core/SpeechProactivityAdmission.swift— pure admission policy.desktop/macos/Desktop/Sources/ProactiveAssistants/Core/SpeechProactivityCoordinator.swift—@MainActorobserver that hops to the engine actor.desktop/macos/Desktop/Tests/TranscriptDrivenProactivityTests.swift— 14 hermetic tests.Modified
Core/ContextProactivityEngine.swift—evaluateFromSpeech(speech:)entry;evaluateAndDelivergained defaultedspeechSection:appended after the candidates section. All existing dwell call sites compile unchanged.Core/ContextBucketRollup.swift—ContextProactivityPromptBuilder.liveSpeechSection(_:maximumSliceCount:timeZone:);[You]/[Other speaker N]tags,ContextDestinationKey.singleLine(limit:)flattening,== LIVE SPEECH ==header.Core/ContextBucketsFeature.swift—isTranscriptProactivityEnabled+OMI_FORCE_SPEECH_PROACTIVITYoverride.Core/ContextVisitCoordinator.swift— productionactiveFence()accessor beside the existingactiveFenceForTesting().AppState/AppState+ListenEvents.swift— funnel wiring after the new segment is built.Notes / design choices
dwellAdmission.begin(visitID:)queue as dwell evaluations, so concurrent speech + dwell evaluations can't interleave on the same visit.VoiceTurnCoordinator.shared.activeTurnID != nil(WakeWordService isn't on this base yet; feat(desktop): hands-free wake word to command the assistant during ambient listening #11801 introduces it and this PR is stacked on it, so no behavioral conflict).Testing / Verification
xcrun swift build -c debug --package-path Desktop— clean.TranscriptDrivenProactivityTests— 14/14 pass (window dedup/retention/bounds, every admission outcome, prompt-section tags/flattening/slice cap).ContextProactivity*engine suites — all pass (dwell paths unaffected by the defaulted param).Stack: #11801 → this PR.
Failure-Class: none