- Notifications
You must be signed in to change notification settings - Fork17
Remove reduntant client type check#39
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Pull Request Overview
This PR simplifies the client extraction logic in themoderation function by removing redundant type checking. The original code checked if the context'sguardrail_llm attribute was specifically anAsyncOpenAI instance before using it, but this check is unnecessary since the code later handles API incompatibilities gracefully.
- Simplified client extraction from context by removing unnecessary type check
- Reduced lines of code from 7 to 2 while maintaining the same functionality
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/guardrails/checks/text/moderation.py:36
- Missing import for sync
OpenAIclass. Line 190 usesisinstance(client, AsyncOpenAI)to detect async clients, implying sync clients are supported, but the syncOpenAIclass is not imported. This will cause the code to work accidentally (since non-AsyncOpenAI clients will be treated as sync), but type checking will fail and the code is not explicit about its dependencies. AddOpenAIto the import:from openai import AsyncOpenAI, NotFoundError, OpenAI
from openai import AsyncOpenAI, NotFoundError💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| # Try the context client first, fall back if moderation endpoint doesn't exist | ||
| ifclientisnotNone: | ||
| # Determine if client is async or sync | ||
| is_async=isinstance(client,AsyncOpenAI) |
CopilotAINov 3, 2025
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.
The client type detection assumes any non-AsyncOpenAI client is synchronous, which may not be accurate for Azure clients or other variants. Consider using a more explicit check likehasattr(client.moderations.create, '__call__') andnot inspect.iscoroutinefunction(client.moderations.create) similar to the pattern inllm_base.py (line 231), or import and check against specific sync client types (OpenAI,AzureOpenAI).
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.
WAI. Only OpenAI clients are valid for moderation so we will catch the other types and fallback to generating an OAI client.
gabor-openai left a comment
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.
TY
66b40d0 intomainUh oh!
There was an error while loading.Please reload this page.
Removes unneeded check that the client is OpenAI. This is handled by directly trying the moderation endpoint, if it doesn't exist we create a fallback client
isinstancecheck is fragile and if it fails to match will cause us to incorrectly setclient=None