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

chore(weave): Wrap up loose ends with Annotations #3050

Merged
merged 8 commits into from
Nov 25, 2024

Conversation

tssweeney
Copy link
Collaborator

@tssweeney tssweeney commented Nov 22, 2024

  1. Rename json_schema to field_schema
  2. Adds backend validation when creating feedback records
  3. Allows user to pass a Pydantic BaseModel or Field to the field_schema and not worry about json.
  4. Fixes maxLength in string field to be correct

Other followup notes for Griffin:

  • String len validation not enforced in ui
  • Save buttons should be disabled if the form is invalid

@tssweeney tssweeney requested review from a team as code owners November 22, 2024 03:40
@circle-job-mirror
Copy link

circle-job-mirror bot commented Nov 22, 2024

Copy link
Member

@gtarpenning gtarpenning left a comment

Choose a reason for hiding this comment

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

Sweet


# Handle Pydantic model
if isinstance(field_schema, type) and issubclass(field_schema, BaseModel):
data["field_schema"] = field_schema.model_json_schema() # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

We currently don't support nested objects, we might want to throw with an actionable error mentioning they will have to create multiple specs. I will add a ticket to move as much of the human feedback UI to the Zod form, which will allow for nested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should allow the creation, but maybe in the UI there is a fallback that allows free-form json input or something

Comment on lines +77 to +85
"""
Validates a payload against this annotation spec's schema.

Args:
payload: The data to validate against the schema

Returns:
bool: True if validation succeeds, False otherwise
"""
Copy link
Member

Choose a reason for hiding this comment

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

I think including the work "feedback" or in some way tying it to the feedback schema might make sense here, perhaps an example

@tssweeney tssweeney enabled auto-merge (squash) November 25, 2024 20:47
@tssweeney tssweeney merged commit 12630a7 into master Nov 25, 2024
113 of 115 checks passed
@tssweeney tssweeney deleted the tim/modify_annotations branch November 25, 2024 21:29
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants