-
Notifications
You must be signed in to change notification settings - Fork 89
fix: address round-2 voice-cross-guardian review feedback #7552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import type { RelayConnection } from './relay-server.js'; | |
| import { registerCallOrchestrator, unregisterCallOrchestrator, fireCallQuestionNotifier, fireCallCompletionNotifier, fireCallTranscriptNotifier } from './call-state.js'; | ||
| import type { PromptSpeakerContext } from './speaker-identification.js'; | ||
| import { addPointerMessage, formatDuration } from './call-pointer-messages.js'; | ||
| import * as conversationStore from '../memory/conversation-store.js'; | ||
| import { dispatchGuardianQuestion } from './guardian-dispatch.js'; | ||
| import type { ServerMessage } from '../daemon/ipc-contract.js'; | ||
|
|
||
|
|
@@ -452,6 +453,13 @@ export class CallOrchestrator { | |
| if (spokenText.length > 0) { | ||
| const session = getCallSession(this.callSessionId); | ||
| if (session) { | ||
| // Persist assistant transcript to the voice conversation so it | ||
| // survives even when no live daemon Session is listening. | ||
| conversationStore.addMessage( | ||
| session.conversationId, | ||
| 'assistant', | ||
| JSON.stringify([{ type: 'text', text: spokenText }]), | ||
| ); | ||
| fireCallTranscriptNotifier(session.conversationId, this.callSessionId, 'assistant', spokenText); | ||
|
Comment on lines
+459
to
463
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This block now writes the assistant transcript directly to Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -454,6 +454,13 @@ export class RelayConnection { | |
|
|
||
| const session = getCallSession(this.callSessionId); | ||
| if (session) { | ||
| // Persist caller transcript to the voice conversation so it survives | ||
| // even when no live daemon Session is listening. | ||
| conversationStore.addMessage( | ||
| session.conversationId, | ||
| 'user', | ||
| JSON.stringify([{ type: 'text', text: msg.voicePrompt }]), | ||
| ); | ||
|
Comment on lines
+459
to
+463
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Duplicate transcript persistence when a live daemon Session is listening on the voice conversation Every caller and assistant transcript is persisted twice to the voice conversation when a daemon Session is actively listening. The new direct Root Cause and ImpactThe new code at conversationStore.addMessage(
session.conversationId, // voice conversation
'user',
JSON.stringify([{ type: 'text', text: msg.voicePrompt }]),
);Then conversationStore.addMessage(
conversationId, // same voice conversation
'assistant',
JSON.stringify([{ type: 'text', text: transcriptText }]),
);The same duplication occurs for assistant transcripts in When no Session is listening the notifier doesn't fire, so only the new direct write happens (correct). But when a Session IS listening, both writes execute, producing two messages per transcript event. The messages also have inconsistent roles: the direct write uses Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| fireCallTranscriptNotifier(session.conversationId, this.callSessionId, 'caller', msg.voicePrompt); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Duplicate assistant transcript persistence when a live daemon Session is listening on the voice conversation
Same duplication issue as the caller transcript path, but for assistant transcripts in the call orchestrator.
Root Cause
The new code at
assistant/src/calls/call-orchestrator.ts:458-462persists the assistant transcript directly:Then on line 463,
fireCallTranscriptNotifiertriggers the notifier inassistant/src/daemon/session-notifiers.ts:125-129which persists a second formatted copy to the same conversation as'assistant'role with"**Live call transcript**\nAssistant: ..."prefix. This results in two assistant messages per LLM turn in the voice conversation when a Session is listening.Was this helpful? React with 👍 or 👎 to provide feedback.