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

GenAI events - yamlify body definitions and cosmetic examples improvements #1469

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Oct 11, 2024

  • Adds yaml definitions for GenAI events in preparation for Render events with bodies #1464
  • Updates examples to be more concise and readable, fixes small inconsistencies, adds diagrams.

Merge requirement checklist

@lmolkova lmolkova changed the title GenAI event examples - cosmetic improvements GenAI events - yamlify body definitions and cosmetic examples improvements Oct 11, 2024
examples: ["get_weather"]
requirement_level: required
- id: arguments
type: undefined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's clarify if these should be deserialized or provided as string.
The perf cost of deserialization compared to all other things necessary to create this event is small.

Copy link

Choose a reason for hiding this comment

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

A concern: In general the "arguments" string returned by (at least OpenAI -- my experience is limited) may not be valid JSON. In the (likely rare) case that it isn't this means that "arguments" would then, presumably, fallback to be a string. That could be a rare surprise (i.e. possible bug) to whatever system is processing this telemetry.

I suppose that is fine if the type allows both string and object (or perhaps AnyValue would be used) -- then implementations need to cope.


Another possible concern: https://platform.openai.com/docs/api-reference/chat/create#chat-create-response_format includes the (surprising to me) warning:

Important: when using JSON mode, you must also instruct the model to produce JSON yourself via a system or user message. Without this, the model may generate an unending stream of whitespace until the generation reaches the token limit, resulting in a long-running and seemingly "stuck" request.

Not having the raw "arguments" string in the telemetry could make it hard to debug why a parsed JSON response of, say {"foo": "bar"}, took 1024 (or however many) tokens.


What are reasonable use cases for having the parsed arguments in telemetry?

Copy link
Contributor Author

@lmolkova lmolkova Oct 17, 2024

Choose a reason for hiding this comment

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

discussed at GenAI call:

if max token is reached in the middle of the arguments, it's truncated, so let's keep it a string, but we might change it for some systems.

Copy link

Choose a reason for hiding this comment

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

Then we should also update the examples above

@@ -30,6 +70,73 @@ groups:
brief: >
This event describes the assistant message passed to GenAI system or received from it.
extends: gen_ai.common.event.attributes
body:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: add generated md table into the description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Discussions
Development

Successfully merging this pull request may close these issues.

4 participants