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

[sdk] 1/n Standardize Input Schema - Input Attachment Data #929

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

Ankush-lastmile
Copy link
Member

@Ankush-lastmile Ankush-lastmile commented Jan 15, 2024

[rfc][sdk] 1/n Standardize Input Schema - Input Attachment Data

Starting point of standardizing Input Schema for prompts

This diff introduces the type AttachmentDataWithStringValue to python and typescript sdk which will allow attachments to have structure as well as validation in python (pydantic validation).

testplan

Screenshot 2024-01-15 at 5 52 11 PM

@Ankush-lastmile Ankush-lastmile changed the title [sdk] 1/n Standardize Input Schema [sdk] 1/n Standardize Input Schema - Input Attachment Data Jan 15, 2024
@Ankush-lastmile Ankush-lastmile changed the title [sdk] 1/n Standardize Input Schema - Input Attachment Data [rfc][sdk] 1/n Standardize Input Schema - Input Attachment Data Jan 15, 2024
"""

kind: Literal["file_uri", "base64"]
value: str
Copy link
Member Author

@Ankush-lastmile Ankush-lastmile Jan 15, 2024

Choose a reason for hiding this comment

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

This type and code is pretty much a duplicate of

class OutputDataWithStringValue(BaseModel):
    """
    This represents the output content that is storied as a string, but we use
    both the `kind` field here and the `mime_type` in ExecuteResult to convert
    the string into the output format we want.
    """

    kind: Literal["file_uri", "base64"]
    value: str

Should this be merged into one type and abstracted? My take on that is no because That would reduce the explicitness and readability of this addition, but unsure what's right here.

python/src/aiconfig/schema.py Outdated Show resolved Hide resolved
typescript/types.ts Outdated Show resolved Hide resolved
@rholinshead
Copy link
Contributor

Nice, LGTM, just some comments. Would you be able to hook this up to the editor in a subsequent PR so that we are using the correct types there? I wonder if we can just remove the attachments stuff from PromptInputObjectSchema now with this? There's technically support for attachment metadata there but it could also just be set with the JSON toggle

@Ankush-lastmile
Copy link
Member Author

Ankush-lastmile commented Jan 15, 2024

Would you be able to hook this up to the editor in a subsequent PR so that we are using the correct types there? I wonder if we can just remove the attachments stuff from PromptInputObjectSchema now with this? There's technically support for attachment metadata there but it could also just be set with the JSON toggle

Looking into this, I have a couple of questions; will sync offline

Copy link
Contributor

@saqadri saqadri left a comment

Choose a reason for hiding this comment

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

One change required, otherwise looks good. This doesn't actually update the model parsers it seems to actually use this new type yet, coming on top of this I assume?

@@ -152,7 +161,7 @@ class Attachment(BaseModel):
"""

# The data representing the attachment
data: Any
data: Union[AttachmentDataWithStringValue, Any]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
data: Union[AttachmentDataWithStringValue, Any]
data: Union[AttachmentDataWithStringValue, str, Any]

Technically, an attachment can also be a string, just like ExecuteResult.data: data: Union[OutputDataWithValue, str, Any]

Copy link
Member Author

Choose a reason for hiding this comment

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

discussed offline: updating to include str to be explicit, since we know some models like gemini take in multiple parts or stablediffusion has a 'negative prompt` which is just as much as input. We can iterate as we go about how this looks.

Typescript is defined as JSONValue so no need to add explicit string; Already supported.

Starting point of standardizing Input Schema for prompts

This diff introduces the type `AttachmentDataWithStringValue` to python and typescript sdk which will allow attachments to have structure as well as validation in python (pydantic validation).

## testplan

<img width="1273" alt="Screenshot 2024-01-15 at 5 52 11 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/0e5b1c83-cf94-4b2e-b267-8e45b4296593">
@Ankush-lastmile
Copy link
Member Author

This doesn't actually update the model parsers it seems to actually use this new type yet, coming on top of this I assume?

Yes; those diffs will come on top

@Ankush-lastmile Ankush-lastmile merged commit e101b67 into main Jan 15, 2024
4 checks passed
@Ankush-lastmile Ankush-lastmile changed the title [rfc][sdk] 1/n Standardize Input Schema - Input Attachment Data [sdk] 1/n Standardize Input Schema - Input Attachment Data Jan 15, 2024
@rossdanlm rossdanlm deleted the pr929 branch January 16, 2024 03:56
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
Wasn't exported in the pr it was implemented in #929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
`AttachmentDataWithStringValue` was created in #929

Wasn't exported. This diff exports it
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
`AttachmentDataWithStringValue` was created in #929

Wasn't exported. This diff exports it
Ankush-lastmile added a commit that referenced this pull request Jan 16, 2024
[easy] [ts][sdk] export `AttachmentDataWithStringValue` type


`AttachmentDataWithStringValue` was created in #929

Wasn't exported. This diff exports it
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the ASR Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan


### Dependencies
Sapling removed the dependency pr. Depends on
#929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan


### Dependencies
Sapling removed the dependency pr. Depends on
#929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the ASR Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan
<img width="1312" alt="Screenshot 2024-01-16 at 3 29 02 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/f3574860-7d1b-4768-a536-7c6cd875af3c">

### Dependencies
Sapling removed the dependency pr. Depends on
#929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan


### Dependencies
Sapling removed the dependency pr. Depends on
#929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan
<img width="1167" alt="Screenshot 2024-01-16 at 3 32 52 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/bfb6af75-a924-44d4-9a66-25374814d569">


### Dependencies
Sapling removed the dependency pr. Depends on
#929
Ankush-lastmile pushed a commit that referenced this pull request Jan 16, 2024
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type.

Also made some updates to to make input attachment validation clearer.

## Testplan
<img width="1167" alt="Screenshot 2024-01-16 at 3 32 52 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/bfb6af75-a924-44d4-9a66-25374814d569">


### Dependencies
Sapling removed the dependency pr. Depends on
#929
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.

3 participants