-
Notifications
You must be signed in to change notification settings - Fork 142
feat(openai): standardize completions to indexed attribute format #2242
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
base: main
Are you sure you want to change the base?
feat(openai): standardize completions to indexed attribute format #2242
Conversation
google-adk haystack and bedrock latest failures are not related to this PR, rather drift on their corresponding latest versions. |
4bb94eb
to
463fd1e
Compare
bedrock ci fix: #2243 |
google-adk fix: #2244 |
haystack fix: #2245 |
Thanks @codefromthecrypt. I think the Looks like |
@axiomofjoy so what I was told in envoy AI gateway is that /completions is a special case and needed for LoRA use cases, so while it is specified semantically, we can clarify that this is only applies to that endpoint which is implemented several places outside openai including most notably vLLM (bloomberg was talking about this specifically) So, where I'm getting at is I think we don't imply that /chat/completions and others map to choices, rather the existing semantics which index on messages (a /chat/completions nouns) isn't re-used for the choices of the raw/legacy completions (which has no such response field "messages" only "choices"). In other words, the choices is contained to the completions use case. If pragmatically we want to map /completions attributes into a synthetic schema to merge with /chat/completions, I'm keen on that, just would like guidance on it. Either way, response arrays have the same size issue as the request ones. Thoughts? |
so concretely in LLM spans (normal chat completions who have a concept of role) we do traverse the choices path and get to the message.role or message.content field and add them as indexed attributes.
In completions there is no structured field inside the choices field, so it doesn't make as much sense to map the same way as chat completions which has structured data, so needs to split out the content vs the role (there is no role)
So, I think what you are saying is that because the /chat/completions has a choices field which the existing output attributes are sourced from, saying "choices" in the attribute name for the completions response, even if technically valid would be confusing. What if instead of "llm.output_choices.0.choice.text" we made a pragmatic change to say "llm.output_completion.0.text" . While less technically accurate, it might reduce the confusion? I'll go ahead and spike this and also update the docs as requested. if you think of something better meanwhile lemme know. |
eek.. just realized how bad embeddings looks in practice: "embedding.embeddings.0.embedding.text" "embedding.embeddings.0.embedding.vector" what if we change both of the special apis embedding and completion to be less repetitive? "completion.input.0.prompt" I'll spike this for completions for feedback |
463fd1e
to
62473a2
Compare
62473a2
to
b3fe44e
Compare
Current Design: "completion.prompt.N" and "completion.text.N" RationaleJSON Path AlignmentThe convention directly mirrors the actual OpenAI Completions API structure: Completions API (simple):
Chat Completions API (structured objects):
Key Difference: Completions deals with simple indexed strings, not structured objects with multiple fields. The attribute format reflects this fundamental difference. Index-at-the-End ConventionFor simple values (strings), the index comes last: "completion.prompt.0"
For structured objects, index comes in the middle: "llm.input_messages.0.message.content"
Alternative Conventions (and why they're worse)"completion.input.N.prompt" + "completion.output.N.text"
"llm.prompts.N" (old convention)
"completion.input_prompts.N.prompt.text"
"llm.input_choices.N.choice.prompt"
|
openai drift #2253 |
beeai drift: #2255 |
@codefromthecrypt After discussing with @mikeldking, here's what we'd like to propose. PromptsWe'd like to keep We'd also like to propose namespacing the "text" field under a "prompt" field. Concretely, keys would look like "llm.prompts.<prompt_index>.prompt.text". There are a few reasons we advocate for this approach:
{
"llm": {
"prompts": [
"prompt": {
"text": "Write a haiku"
}
]
}
}
{
"llm": {
"input_messages": [
{
"message": {
"role": "user",
"content": "Write a haiku"
}
}
]
}
} Choices
{
"llm": {
"prompts": [
{
"prompt": {
"text": "Write a haiku"
}
}
],
"input_messages": [
{
"message": {
"role": "user",
"content": "Write a haiku"
}
}
],
"choices": [
{
"completion": {
"text": "Cherry blossoms bloom\nabove New York’s restless streets\nskyline crowned in pink"
}
},
{
"chat_completion": {
... // leave for another day
}
}
]
}
} NotesWe'd like to avoid adding a top-level "completion" prefix. Currently, the prefixes correspond to the span kind attribute values ("chain", "llm", "embedding", etc.). We still think of legacy completions as LLM spans and don't see a need to introduce a new completion span kind. |
looks beautiful @axiomofjoy @mikeldking thanks for collaborating on this! |
Signed-off-by: Adrian Cole <[email protected]>
b3fe44e
to
0cf1098
Compare
beeai drift refactored here: #2255 |
Signed-off-by: Adrian Cole <[email protected]>
also, it seems a trend that if you hide_inputs you also hide things derived from it. similar for outputs. Is that true? if so, maybe I'll do a follow-up to make it coherent in tests and docs. |
Standardizes /completions spans to use
llm.*
prefix with indexed nested attributes, aligning with /chat/completions format:llm.prompts.{i}.prompt.text
for completion prompts (nested indexed format)llm.choices.{i}.completion.text
for completion outputs (nested indexed format)LLM_CHOICES
constant to semantic conventionsLLM_PROMPTS
constant (un-deprecated for completions use)OPENINFERENCE_HIDE_CHOICES
environment variableOPENINFERENCE_HIDE_PROMPTS
environment variableKey benefits:
llm.*
prefix across all LLM span types (nocompletion.*
top-level prefix)llm.input_messages
andllm.output_messages
patternsBreaking changes:
completion.prompt.{i}
→llm.prompts.{i}.prompt.text
completion.text.{i}
→llm.choices.{i}.completion.text
Note
Migrates OpenAI completions to indexed
llm.prompts.N.prompt.text
andllm.choices.N.completion.text
, addsLLM_CHOICES
andOPENINFERENCE_HIDE_CHOICES
, and updates masking, tests, and docs accordingly.llm.prompts.N.prompt.text
(replaces list underllm.prompts
).llm.choices.N.completion.text
from response choices.SpanAttributes.LLM_CHOICES
; documents indexed/nested format forllm.prompts
andllm.choices
.OPENINFERENCE_HIDE_CHOICES
andTraceConfig.hide_choices
; extends masking to redactllm.prompts.*
andllm.choices.*
based on hide settings.Written by Cursor Bugbot for commit 5a324c1. This will update automatically on new commits. Configure here.