Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Moving all the python content over#1

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

Merged
gabor-openai merged 1 commit intomainfromdev/steven/python_split_import
Oct 2, 2025

Conversation

@steven10a
Copy link
Collaborator

Added all of the Python content from the early access repo

  • Tested a few examples to confirm it works (hello_world, multi_bundle, agents_sdk)
  • Ranmake serve-docs and verified the docs look good

Copy link

CopilotAI left a 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 moves all Python content from the early access repository to provide a complete guardrails SDK implementation. The changes include:

  • Complete Python SDK implementation with async/sync OpenAI client wrappers
  • Type system and runtime execution framework for guardrails
  • Comprehensive test coverage with unit and integration tests
  • Evaluation framework with benchmarking capabilities

Reviewed Changes

Copilot reviewed 125 out of 140 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
vercel.jsonDeployment configuration for docs building
tests/unit/*.pyUnit test suite covering types, specs, runtime, and registry
tests/integration/*.pyIntegration tests and demo configuration
src/guardrails/types.pyCore type definitions and protocols
src/guardrails/spec.pyGuardrail specification and metadata models
src/guardrails/runtime.pyConfiguration loading and execution engine
src/guardrails/registry.pyGuardrail registration and discovery system
src/guardrails/exceptions.pyException hierarchy for error handling
src/guardrails/context.pyContext management using ContextVars
src/guardrails/utils/*.pyUtility modules for schema, parsing, and context validation
src/guardrails/resources/*.pyOpenAI API resource wrappers with guardrails
src/guardrails/evals/*.pyComprehensive evaluation and benchmarking framework

Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.

Comment on lines +83 to +86
def __post_init__(self) -> None:
"""Validate required fields and consistency."""
if "checked_text" not in self.info:
raise ValueError("GuardrailResult.info must contain 'checked_text' field")
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The documentation states that 'checked_text' is required in the info dict, but the validation logic doesn't match the docstring. The docstring says 'Must include' but doesn't explain why this field is mandatory or what it should contain.

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +44
try:
validate_dataset(path)
except ValueError as e:
logger.error("Dataset validation failed: %s", e)
raise ValueError(f"Dataset validation failed: {e}") from e
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Double error handling creates redundant validation error messages. The validate_dataset function already logs errors, so this catch-and-re-raise pattern results in duplicate logging and error wrapping.

Suggested change
try:
validate_dataset(path)
exceptValueErrorase:
logger.error("Dataset validation failed: %s",e)
raiseValueError(f"Dataset validation failed:{e}")frome
validate_dataset(path)

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +38
def __init__(self, output_dir: Path) -> None:
"""Initialize the visualizer.

Args:
output_dir: Directory to save generated charts
"""
self.output_dir = output_dir
self.output_dir.mkdir(parents=True, exist_ok=True)

# Set style and color palette
plt.style.use('default')
self.colors = [
'#1f77b4', '#ff7f0e', '#2ca02c', '#d62728', '#9467bd',
'#8c564b', '#e377c2', '#7f7f7f', '#bcbd22', '#17becf',
]
sns.set_palette(self.colors)
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Setting global matplotlib and seaborn styles in the constructor can affect other parts of the application. Consider using context managers or style parameters in individual plotting methods to avoid global side effects.

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +26
'.c', '.cpp', '.cs', '.css', '.doc', '.docx', '.go', '.html',
'.java', '.js', '.json', '.md', '.pdf', '.php', '.pptx',
'.py', '.rb', '.sh', '.tex', '.ts', '.txt'
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

[nitpick] The supported file types set uses mixed line formatting that makes it harder to maintain. Consider formatting each extension on its own line for better version control diffs and easier maintenance.

Suggested change
'.c','.cpp','.cs','.css','.doc','.docx','.go','.html',
'.java','.js','.json','.md','.pdf','.php','.pptx',
'.py','.rb','.sh','.tex','.ts','.txt'
'.c',
'.cpp',
'.cs',
'.css',
'.doc',
'.docx',
'.go',
'.html',
'.java',
'.js',
'.json',
'.md',
'.pdf',
'.php',
'.pptx',
'.py',
'.rb',
'.sh',
'.tex',
'.ts',
'.txt',

Copilot uses AI. Check for mistakes.
Copy link

CopilotAI left a 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 125 out of 140 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/guardrails/evals/core/types.py:1

  • The supported file types set is defined in the wrong file (types.py) when it should be increate_vector_store.py where it's actually used. This creates unnecessary coupling and makes the types module less focused.
"""

src/guardrails/evals/core/benchmark_calculator.py:1

  • The NaN checknot value != value is confusing and potentially incorrect. The standard way to check for NaN in Python isnot math.isnan(value) ornot pandas.isna(value) for clearer intent and reliability.
"""

Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.

def __post_init__(self) -> None:
"""Validate required fields and consistency."""
if "checked_text" not in self.info:
raise ValueError("GuardrailResult.info must contain 'checked_text' field")
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The__post_init__ method validation will fail for existing code that createsGuardrailResult instances without the required 'checked_text' field. This represents a breaking change that could cause runtime errors in production code that was previously working.

Suggested change
raiseValueError("GuardrailResult.info must contain 'checked_text' field")
# Backward compatibility: insert default and warn
object.__setattr__(self,"info", {**self.info,"checked_text":""})
logger.warning(
"GuardrailResult.info did not contain 'checked_text'; defaulting to empty string. "
"Please update code to include 'checked_text' in info."
)

Copilot uses AI. Check for mistakes.
Comment on lines +112 to +120
from sklearn.metrics import roc_curve
fpr, tpr, _ = roc_curve(y_true, y_scores)
roc_auc = np.trapz(tpr, fpr)
ax.plot(fpr, tpr, label=f'{model_name} (AUC = {roc_auc:.3f})', linewidth=2)
except Exception as e:
logger.error("Failed to calculate ROC curve for model %s: %s", model_name, e)

# Add diagonal line and customize plot
ax.plot([0, 1], [0, 1], 'k--', alpha=0.5, label='Random Classifier')
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The ROC AUC calculation usingnp.trapz(tpr, fpr) is incorrect. The correct calculation should benp.trapz(tpr, fpr) with reversed order or usesklearn.metrics.auc(fpr, tpr) for proper AUC computation.

Copilot uses AI. Check for mistakes.
try:
from openai import AsyncAzureOpenAI
except ImportError:
AsyncAzureOpenAI = None # type: ignore
Copy link

CopilotAIOct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

SettingAsyncAzureOpenAI = None after import failure creates potential runtime errors when the code later checksif not AsyncAzureOpenAI:. A more robust approach would be to use a proper sentinel value or define a stub class.

Suggested change
AsyncAzureOpenAI=None# type: ignore
classAsyncAzureOpenAI:
def__init__(self,*args,**kwargs):
raiseImportError("AsyncAzureOpenAI is not available. Please install the required dependencies.")

Copilot uses AI. Check for mistakes.
@gabor-openaigabor-openai merged commit091a665 intomainOct 2, 2025
@steven10asteven10a deleted the dev/steven/python_split_import branchOctober 6, 2025 14:20
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

@gabor-openaigabor-openaigabor-openai approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@steven10a@gabor-openai

[8]ページ先頭

©2009-2025 Movatter.jp