-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Python: Fix: Parse oauth_consent_request events in Azure AI client #4197
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -588,6 +588,68 @@ def _get_current_conversation_id(self, options: Mapping[str, Any], **kwargs: Any | |
| """Get the current conversation ID from chat options or kwargs.""" | ||
| return options.get("conversation_id") or kwargs.get("conversation_id") or self.conversation_id | ||
|
|
||
| @override | ||
| def _parse_response_from_openai( | ||
| self, | ||
| response: Any, | ||
| options: dict[str, Any], | ||
| ) -> ChatResponse: | ||
| """Parse an Azure AI Responses API response, handling Azure-specific output item types.""" | ||
| result = super()._parse_response_from_openai(response, options) | ||
|
|
||
| if result.messages: | ||
| for item in response.output: | ||
| if item.type == "oauth_consent_request": | ||
| consent_link = item.consent_link | ||
| if consent_link and not consent_link.startswith("https://"): | ||
| logger.warning("Received oauth_consent_request with non-HTTPS consent_link: %s", item) | ||
|
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. nit: this is essentially skipping the link. The log message should probably mention it.
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. Same below for parsing chunks |
||
| consent_link = "" | ||
| if consent_link: | ||
| result.messages[0].contents.append( | ||
| Content.from_oauth_consent_request( | ||
| consent_link=consent_link, | ||
| raw_representation=item, | ||
| ) | ||
| ) | ||
| else: | ||
| logger.warning("Received oauth_consent_request output without consent_link: %s", item) | ||
|
|
||
| return result | ||
|
|
||
| @override | ||
| def _parse_chunk_from_openai( | ||
| self, | ||
| event: Any, | ||
| options: dict[str, Any], | ||
| function_call_ids: dict[int, tuple[str, str]], | ||
| ) -> ChatResponseUpdate: | ||
| """Parse an Azure AI streaming event, handling Azure-specific event types.""" | ||
| # Intercept output_item.added events for Azure-specific item types | ||
| if event.type == "response.output_item.added" and event.item.type == "oauth_consent_request": | ||
|
Member
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. and this logic will silently ignore other content types? And that seems inconsistent with the non-streaming flow?
Contributor
Author
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. When |
||
| event_item = event.item | ||
| consent_link = event_item.consent_link | ||
| if consent_link and not consent_link.startswith("https://"): | ||
| logger.warning("Received oauth_consent_request with non-HTTPS consent_link: %s", event_item) | ||
| consent_link = "" | ||
| contents: list[Content] = [] | ||
giles17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if consent_link: | ||
| contents.append( | ||
| Content.from_oauth_consent_request( | ||
| consent_link=consent_link, | ||
| raw_representation=event_item, | ||
| ) | ||
| ) | ||
| else: | ||
| logger.warning("Received oauth_consent_request output without consent_link: %s", event_item) | ||
| return ChatResponseUpdate( | ||
| contents=contents, | ||
| role="assistant", | ||
| model_id=self.model_id, | ||
| raw_representation=event, | ||
| ) | ||
|
|
||
| return super()._parse_chunk_from_openai(event, options, function_call_ids) | ||
|
|
||
| def _prepare_messages_for_azure_ai(self, messages: Sequence[Message]) -> tuple[list[Message], str | None]: | ||
| """Prepare input from messages and convert system/developer messages to instructions.""" | ||
| result: list[Message] = [] | ||
|
|
||
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.