(phonic) Faster handoffs#1221
Conversation
🦋 Changeset detectedLatest commit: 3d9a6c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
qionghuang6
force-pushed
the
qiong/faster-handoffs
branch
from
8c37282 to
343baa0
Compare
| // this means the content is the same as the previous session | ||
| const capabilities = this.llm.capabilities; | ||
| if (!rtReused && this.realtimeSession?.realtimeModel.provider == 'phonic') { | ||
| if (this.realtimeSession?.realtimeModel.provider == 'phonic') { |
There was a problem hiding this comment.
| responseId, | ||
| }; | ||
|
|
||
| if (this.pendingGenerateReplyFut && !this.pendingGenerateReplyFut.done) { |
There was a problem hiding this comment.
Brought over from the Python SDK, was never implemented here.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| chatCtx?: llm.ChatContext, | ||
| tools?: llm.ToolContext, | ||
| ): Promise<void> { | ||
| if (!this.configSent) { |
There was a problem hiding this comment.
i just made an update in my PR, the changes should handle a fresh session and call update* functions in the framework
There was a problem hiding this comment.
Thanks, just replaced this check with await this.readyToStart.await; instead
| this.readyToStart.resolve(); | ||
| this.closeCurrentGeneration({ interrupted: false }); | ||
| this.rejectPendingGenerateReply(); | ||
| this.inputResampler = undefined; |
There was a problem hiding this comment.
🟡 Missing inputResampler.close() in session close() causes resource leak
In the close() method at line 517, this.inputResampler is set to undefined without first calling .close() on it. The PR itself demonstrates awareness of this requirement: in resampleAudio() at line 868, this.inputResampler.close() is correctly called before nullifying the reference when the sample rate changes. The same cleanup is missing in the session teardown path, leaking the native resampler resource.
| this.inputResampler = undefined; | |
| this.inputResampler?.close(); | |
| this.inputResampler = undefined; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| this.closeCurrentGeneration({ interrupted: false }); | ||
| return this.startNewAssistantTurn({ userInitiated: true }); | ||
| this.pendingGenerateReplyFut = new Future<llm.GenerationCreatedEvent>(); | ||
| this.sendGenerateReply(instructions); |
There was a problem hiding this comment.
🟡 Fire-and-forget sendGenerateReply can leave pendingGenerateReplyFut unresolved and cause unhandled rejection
In generateReply() at line 477, this.sendGenerateReply(instructions) is called without await or .catch(). If this.socket.sendGenerateReply(...) at line 489 throws (e.g., WebSocket in an unexpected state), the error propagates as an unhandled promise rejection, and pendingGenerateReplyFut is never resolved or rejected — causing the caller (agent_activity.ts:2924) to hang indefinitely. The codebase's own convention for fire-and-forget async calls adds .catch() (see this.connectTask at plugins/phonic/src/realtime/realtime_model.ts:268).
| this.sendGenerateReply(instructions); | |
| this.sendGenerateReply(instructions).catch((error: unknown) => { | |
| if (this.pendingGenerateReplyFut && !this.pendingGenerateReplyFut.done) { | |
| this.pendingGenerateReplyFut.reject( | |
| error instanceof Error ? error : new Error(String(error)), | |
| ); | |
| this.pendingGenerateReplyFut = undefined; | |
| } | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Description
Adds logic for handling handoffs in the phonic plugin by implementing
_updateSession, which sends aresetmessage to Phonic.Demo video
https://screen.studio/share/Lg5fpdh0?state=uploading
Changes Made
Pre-Review Checklist
Testing
restaurant_agent.tsandrealtime_agent.tswork properly (for major changes)Additional Notes
Note to reviewers: Please ensure the pre-review checklist is completed before starting your review.