-
Notifications
You must be signed in to change notification settings - Fork 285
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(playground): parse span attribute llm.input_messages
for playground span replay
#4906
feat(playground): parse span attribute llm.input_messages
for playground span replay
#4906
Conversation
182c053
to
f01acd4
Compare
const toolCallSchema = z | ||
.object({ | ||
function: z | ||
.object({ | ||
name: z.string(), | ||
arguments: z.string(), | ||
}) | ||
.partial(), | ||
}) | ||
.partial(); | ||
|
||
/** | ||
* The zod schema for llm message contents | ||
* @see {@link https://github.com/Arize-ai/openinference/blob/main/spec/semantic_conventions.md|Semantic Conventions} | ||
*/ | ||
const messageContentSchema = z.object({ |
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.
these are currently unused, but will be necessary in the future so i didn't want to remove them, can remove for now and add back later.
app/src/pages/playground/utils.ts
Outdated
if (parseError) { | ||
throw new Error("Invalid attributes"); | ||
} | ||
const { data, success } = llmAttributesSchema.safeParse(parsedAttributes); |
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 zod stuff is pretty nice, but i think we'll need to make the schemas purely optional and then check for variants, just input.value, input_messages with contents, prompt templates etc. etc. Given that i'm not sure how this will look but i think it makes sense to stick with 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.
You might be right that it doesn't really provide a ton.
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.
If you need to test variants of even wildly different schemas you can use a zod union. Each entry of the union will be tested against separately and the first one that passes wins.
Meaning you can make a schema for each variant and accept any of them via a zod union, and then further refine the type when you need to pass it somewhere specifically.
The union schema can be required meaning that at least one of the members must match in order to validate
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.
Oh nice, yeah was going to take a look at unions and see what they had to offer
|
||
return <Playground />; | ||
if (!span) { | ||
throw new Error("Invalid span"); |
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 avoid this with @required? If not, maybe throw a more user-friendly message
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's actually not nullable, but above this we return null in the event that the __typename
is not Span, this is because of relay adding other to the type to be safe, will update the error message but should never happen
// This will never be '%other', but we need some
// value in case none of the concrete values match.
readonly __typename: "%other";
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 makes sense
app/src/pages/playground/utils.ts
Outdated
if (parseError) { | ||
throw new Error("Invalid attributes"); | ||
} | ||
const { data, success } = llmAttributesSchema.safeParse(parsedAttributes); |
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 might be right that it doesn't really provide a ton.
1ee44b7
to
458ee2d
Compare
80e35d7
to
4b357be
Compare
expect(getChatRole("assistant")).toEqual("ai"); | ||
expect(getChatRole("bot")).toEqual("ai"); | ||
expect(getChatRole("system")).toEqual("system"); | ||
expect(getChatRole("human:")).toEqual("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.
expect(getChatRole("human:")).toEqual("user"); | |
expect(getChatRole("human")).toEqual("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.
ahh i had this intentionally to check the includes vs. strict matching
|
||
return <Playground />; | ||
if (!span) { | ||
throw new Error("Invalid span"); |
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 makes sense
…round span replay (#4906) * begin building zod schemas for llm attributes * feat(playground): parse span input messages * move to utils, update role * update initial instance id * update id generation, add todo, add test files * add tests * memoize * move jest-canvas-mock to dev dependencies * update span not found error message * parse roles as strings and corece to ChatMessageRole * update ChatRoleMap comment * fix typos * update prop type to be InitialPlaygroundState * update naming * fix naming conflict
…round span replay (#4906) * begin building zod schemas for llm attributes * feat(playground): parse span input messages * move to utils, update role * update initial instance id * update id generation, add todo, add test files * add tests * memoize * move jest-canvas-mock to dev dependencies * update span not found error message * parse roles as strings and corece to ChatMessageRole * update ChatRoleMap comment * fix typos * update prop type to be InitialPlaygroundState * update naming * fix naming conflict
…round span replay (#4906) * begin building zod schemas for llm attributes * feat(playground): parse span input messages * move to utils, update role * update initial instance id * update id generation, add todo, add test files * add tests * memoize * move jest-canvas-mock to dev dependencies * update span not found error message * parse roles as strings and corece to ChatMessageRole * update ChatRoleMap comment * fix typos * update prop type to be InitialPlaygroundState * update naming * fix naming conflict
…round span replay (#4906) * begin building zod schemas for llm attributes * feat(playground): parse span input messages * move to utils, update role * update initial instance id * update id generation, add todo, add test files * add tests * memoize * move jest-canvas-mock to dev dependencies * update span not found error message * parse roles as strings and corece to ChatMessageRole * update ChatRoleMap comment * fix typos * update prop type to be InitialPlaygroundState * update naming * fix naming conflict
resolves #4782
llm.input_messages
and put them into a playground instanceThis does not account for any variance in span attributes. When dealing with the variants, it may turn out that zod doesn't get us as much as it seems here but is still nice for safely parsing. For example, we can use zod to parse and then have a series of fallbacks (try input_messages, then prompt template, then input.value) or we can create increasingly strict zod schemas to parse to determine the final values for the playground instance