Skip to content
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

Input validation #483

Closed
wants to merge 14 commits into from
Closed

Conversation

irgolic
Copy link
Contributor

@irgolic irgolic commented Dec 4, 2023

Implements input validation, as separate iterations in the logs. Still missing async tests, and some tests surrounding error raising are failing (pending discussion below).

A few last discussion points before this is ready:

  • We don't raise exceptions on the 0.3.0 branch, what should be our strategy be for improper calling (e.g., prompt validation with msg_history passed)? Should we allow certain types of exceptions? What about for when input validation fails?
  • In logs, should input validation be a separate Iteration, or should we add fields to merge it with the main one?

EDIT
I've introduced UserFacingException, any exc wrapped in it will be propagated to the user.
I've also added an exception field to Outputs, so we don't just store it stringified

@irgolic irgolic requested a review from CalebCourier December 4, 2023 15:48
Copy link

vercel bot commented Dec 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
guardrails ⬜️ Ignored (Inspect) Visit Preview Dec 7, 2023 6:11pm

@irgolic irgolic marked this pull request as ready for review December 5, 2023 15:46
Copy link
Collaborator

@zsimjee zsimjee left a comment

Choose a reason for hiding this comment

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

It's looking good! Can you please add a notebook to exemplify input validation scenarios?

if prompt_params is None:
prompt_params = {}

if msg_history:
Copy link
Collaborator

Choose a reason for hiding this comment

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

long control switch. Can we split some of the logic up?

For ex:
def start_message_history_action
def validate_msg_history_schema
def start_prompt_action
def validate_prompt_schema
def start_instruction_action
def validate_instruction_schema

@irgolic irgolic mentioned this pull request Dec 7, 2023
@irgolic irgolic closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants