Handle cancelled stream #65
Conversation
…or custom cleanup after cancellation
| async def handle_stream_cancelled( | ||
| self, | ||
| thread: ThreadMetadata, | ||
| pending_items: list[ThreadItem], | ||
| context: TContext, | ||
| ): | ||
| """Perform custom cleanup / stop inference when a stream is cancelled. | ||
| Args: | ||
| thread: The thread that was being processed. | ||
| pending_items: Items that were not done streaming at cancellation time. | ||
| By default, already-streamed assistant messages, widgets, and workflows are | ||
| saved to the store during error handling prior to this method being called. | ||
| If you want to remove them from the thread, you can do so here. | ||
| (Updates you make here will not be reflected in the UI until a reload.) | ||
| context: Arbitrary per-request context provided by the caller. | ||
| """ |
There was a problem hiding this comment.
New overridable ChatKitServer method. Not sure if pending_items is the right param name - they include items that were automatically persisted on cancel.
chatkit/server.py
Outdated
| case ThreadItemUpdatedEvent(): | ||
| # Keep the pending assistant message item up to date | ||
| # so that we can persist already-streamed partial content | ||
| # if the stream is cancelled. | ||
| self._update_pending_assistant_message_items( | ||
| pending_items, event | ||
| ) |
There was a problem hiding this comment.
Only applying assistant message items updates to keep the default case simple.
@weedon-openai wdyt:
- Should we apply updates for WorkflowItem and WidgetItem as well?
- Or.. since if we're not applying updates for WorkflowItem and WidgetItem automatically, should we also skip the auto-saving on cancel for these two item types?
There was a problem hiding this comment.
Hmm for the workflow item, we probably have the current workflow in memory and could just save it as is right? Technically the integration doesn't have to use our workflow utils though 🤔
Regardless, workflow deltas are really easy to apply.
Widget deltas are tricky. I kind of think we should apply them, but I'm not sure how difficult that is. Otherwise we should probably delete / revert the widget (widget streaming can happen in a response other than the one that created with widget).
There was a problem hiding this comment.
widget streaming can happen in a response other than the one that created with widget
Ahhh right good point!
Otherwise we should probably delete / revert the widget
Okay I'm leaning towards not auto-saving the widget at all and leaving the integration to handle it themselves in the overridable method.
chatkit/server.py
Outdated
| case ThreadItemUpdatedEvent(): | ||
| # Keep the pending assistant message item up to date | ||
| # so that we can persist already-streamed partial content | ||
| # if the stream is cancelled. | ||
| self._update_pending_assistant_message_items( | ||
| pending_items, event | ||
| ) |
There was a problem hiding this comment.
Hmm for the workflow item, we probably have the current workflow in memory and could just save it as is right? Technically the integration doesn't have to use our workflow utils though 🤔
Regardless, workflow deltas are really easy to apply.
Widget deltas are tricky. I kind of think we should apply them, but I'm not sure how difficult that is. Otherwise we should probably delete / revert the widget (widget streaming can happen in a response other than the one that created with widget).
chatkit/server.py
Outdated
| HiddenContextItem( | ||
| thread_id=thread.id, | ||
| created_at=datetime.now(), | ||
| id=self.store.generate_item_id("hidden_context", thread, context), | ||
| content="SYSTEM: The user cancelled the stream.", | ||
| ), |
There was a problem hiding this comment.
But what if they always use HCI with data shaped content and this breaks their converter.
Maybe HCI needs a new optional field message? Kinda cleaner than the current automatic parser approach that has to sniff the type of content.
There was a problem hiding this comment.
Yeah good point 🤔
I'm leaning towards not adding anything and instead urging the integration to write a hidden context item in their overridable method.
I also considered whether using EndOfTurnItem would make sense here.. but it might break the UI for cases where the integration decides to append some items after the stream is stopped.
There was a problem hiding this comment.
+1 I think we should probably leave it to the integration but update all our examples with a minimal implementation of this method.
jiwon-oai
force-pushed
the
threads-stop
branch
from
1674f22 to
4a4b81b
Compare
jiwon-oai
force-pushed
the
threads-stop
branch
from
12827a6 to
6df9ee6
Compare
Add an overridable
ChatKitServer.handle_stream_cancelledmethod with pending items that is invoked when the stream is cancelled by the user.Add an overridable
ChatKitServer.get_stream_optionsmethod that returnsStreamOptions(allow_cancel=True)StreamOptionsEventwhich is sent at the start of_process_eventsallow_cancel=Truewill be wired up to show a stop button in the composer during the response.Tested against local branch in chatkit studio with the stop UX enabled:
stop-mid-message.mov
stop-mid-workflow.mov
stop-after-progress-update.mov