-
Notifications
You must be signed in to change notification settings - Fork 364
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
feat: Add PromptTemplate type #5787
feat: Add PromptTemplate type #5787
Conversation
@@ -29,7 +31,7 @@ class PromptVersion(Node): | |||
description: str | |||
template_type: PromptTemplateType | |||
template_format: PromptTemplateFormat | |||
template: JSON | |||
template: PromptTemplateVersion |
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.
template_version
?
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.
This is the wrong type
app/schema.graphql
Outdated
@@ -1086,6 +1086,11 @@ type JSONInvocationParameter implements InvocationParameterBase { | |||
defaultValue: JSON | |||
} | |||
|
|||
type JSONPromptMessageGQL { |
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.
Can we remove GQL
from the type names in the GraphQL schema?
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.
Agreed
AI = "ai" # E.g. the assistant. Normalize to AI for consistency. | ||
|
||
|
||
class PromptStringTemplate(BaseModel): |
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.
I see the appeal of keeping the Pydantic models next to their corresponding GraphQL types, but will these models also be needed for the REST API?
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.
Yeah they will - let's de-couple
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.
@axiomofjoy I will say decoupling these into different files might lead to some weird circular import stuff
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.
You can reduce the circular imports by just having the conversion code be functional in its own file and not be attached to classes
return TextPromptMessageGQL.from_model(self) | ||
|
||
|
||
class JSONPromptMessage(BaseModel): |
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.
Do we need both to and from methods?
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.
maybe eventually but we're feeling it out
|
||
class PromptMessagesTemplateV1(BaseModel): | ||
_version: str = "messages-v1" | ||
template: list[Union[TextPromptMessage, JSONPromptMessage]] |
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.
message_templates
or messages
?
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.
ChatTemplateV1
?
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.
I think it's probably a good idea for the different template types to conform to the same interface for now
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.
This should just be a union no?
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.
ah right
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.
wait no, I think it's list[Union[...]] since there can be multiple messages
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.
they key should be messages
that's what's confusing here
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.
ah I see, I changed it to template
so all of our template wrappers have the same interface which I'm kind of partial to (the wrapper for the plain string template just has a template
key)
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.
It shouldn't leak into the graphQL type
app/schema.graphql
Outdated
|
||
type PromptMessagesTemplateV1GQL { | ||
version: String! | ||
template: [TextPromptMessageGQLJSONPromptMessageGQL!]! |
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.
I'm not following why this is a list?
app/schema.graphql
Outdated
type PromptVersion implements Node { | ||
"""The Globally Unique ID of this object""" | ||
id: GlobalID! | ||
user: String | ||
description: String! | ||
templateType: PromptTemplateType! | ||
templateFormat: PromptTemplateFormat! | ||
template: JSON! | ||
template: PromptTemplateVersion! |
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 union type should be here
7a90962
to
1154306
Compare
import strawberry | ||
from strawberry.scalars import JSON | ||
|
||
from phoenix.server.api.helpers.prompthub.models import ( |
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.
from phoenix.server.api.helpers.prompthub.models import ( | |
from phoenix.server.api.helpers.prompts.models import ( |
PromptTemplate = strawberry.union( | ||
"PromptTemplateVersion", (PromptStringTemplate, PromptChatTemplateV1) | ||
) |
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.
Maybe just name PromptTemplateVersion
to match the underlying GraphQL type.
@strawberry.type | ||
class JSONPromptMessage: | ||
role: PromptMessageRole | ||
content: JSON |
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.
Is this for tool call messages, images, etc.?
app/schema.graphql
Outdated
content: String! | ||
} | ||
|
||
union TextPromptMessageJSONPromptMessage = TextPromptMessage | JSONPromptMessage |
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.
Can we name this union type as PromptMessage
?
app/schema.graphql
Outdated
@@ -1377,6 +1382,11 @@ type Prompt implements Node { | |||
promptVersions(first: Int = 50, last: Int, after: String, before: String): PromptVersionConnection! | |||
} | |||
|
|||
type PromptChatTemplateV1 { |
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.
type PromptChatTemplateV1 { | |
type PromptChatTemplate { |
app/schema.graphql
Outdated
@@ -1377,6 +1382,11 @@ type Prompt implements Node { | |||
promptVersions(first: Int = 50, last: Int, after: String, before: String): PromptVersionConnection! | |||
} | |||
|
|||
type PromptChatTemplateV1 { | |||
version: String! |
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.
version: String! | |
__version: String! |
app/schema.graphql
Outdated
@@ -1377,6 +1382,11 @@ type Prompt implements Node { | |||
promptVersions(first: Int = 50, last: Int, after: String, before: String): PromptVersionConnection! | |||
} | |||
|
|||
type PromptChatTemplateV1 { | |||
version: String! | |||
messages: [TextPromptMessageJSONPromptMessage!]! |
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.
messages: [TextPromptMessageJSONPromptMessage!]! | |
messages: [PromptTemplateMessages!]! |
@@ -1395,6 +1405,12 @@ type PromptEdge { | |||
node: Prompt! | |||
} | |||
|
|||
enum PromptMessageRole { | |||
USER | |||
SYSTEM |
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.
add a description that this includes the openAI developer role
@@ -1395,6 +1405,12 @@ type PromptEdge { | |||
node: Prompt! | |||
} | |||
|
|||
enum PromptMessageRole { | |||
USER |
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.
Missing tool
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
* Flesh out PromptVersionTemplate type * Return GraphQL objects with the correct type * Use new types in node query * Decouple pydantic models and gql types * Rebuild gql schema * Rework model names * Update gql schema * Propagate name into schema * Incorporate feedback * Update schema * adjust UI to new schema * cleanup * Remove `hub` naming and clean up type annotations --------- Co-authored-by: Mikyo King <[email protected]>
resolves #5771