-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(messages): add ToolReturnPart to ModelResponsePart union
#5723
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?
Changes from all commits
46b8a08
9f283a6
e7e25e3
b301af7
dc2cac8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2066,6 +2066,7 @@ def _model_response_part_discriminator(v: Any) -> str | None: | |
| | Annotated[NativeToolCallPart, pydantic.Tag('builtin-tool-call')] | ||
| | Annotated[NativeToolSearchReturnPart, pydantic.Tag('builtin-tool-search-return')] | ||
| | Annotated[NativeToolReturnPart, pydantic.Tag('builtin-tool-return')] | ||
| | Annotated[ToolReturnPart, pydantic.Tag('tool-return')] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DouweM This is the key design decision in the PR and I think it warrants explicit maintainer sign-off before proceeding. Context: Issue #5721 was auto-generated by a The question: Should Trade-offs:
The right call depends on whether "user-constructed
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. David's AICA here: Decision on the design question (Douwe is away, so I'm making the call): we'll keep the union-widening approach — adding the base Reasoning:
One correction to the framing above: the framework never produces a base |
||
| | Annotated[ThinkingPart, pydantic.Tag('thinking')] | ||
| | Annotated[CompactionPart, pydantic.Tag('compaction')] | ||
| | Annotated[FilePart, pydantic.Tag('file')], | ||
|
|
@@ -2285,7 +2286,7 @@ def new_event_body(): | |
|
|
||
| return result | ||
|
|
||
| def otel_message_parts(self, settings: InstrumentationSettings) -> list[_otel_messages.MessagePart]: | ||
| def otel_message_parts(self, settings: InstrumentationSettings) -> list[_otel_messages.MessagePart]: # noqa: C901 | ||
|
dsfaccini marked this conversation as resolved.
|
||
| parts: list[_otel_messages.MessagePart] = [] | ||
| for part in self.parts: | ||
| if isinstance(part, TextPart): | ||
|
|
@@ -2333,6 +2334,10 @@ def otel_message_parts(self, settings: InstrumentationSettings) -> list[_otel_me | |
| return_part['result'] = serialize_any(part.content) | ||
|
|
||
| parts.append(return_part) | ||
| elif isinstance(part, ToolReturnPart): | ||
| # A user-defined tool return can appear here via user-constructed message history; map it | ||
| # like its request-side counterpart (no `builtin` flag) so it isn't dropped from telemetry. | ||
| parts.extend(part.otel_message_parts(settings)) | ||
| elif isinstance(part, CompactionPart): | ||
| # Compaction parts don't map to standard OTel message part types | ||
| pass | ||
|
|
@@ -2710,7 +2715,16 @@ class PartStartEvent: | |
| """The newly started `ModelResponsePart`.""" | ||
|
|
||
| previous_part_kind: ( | ||
| Literal['text', 'thinking', 'tool-call', 'builtin-tool-call', 'builtin-tool-return', 'compaction', 'file'] | ||
| Literal[ | ||
| 'text', | ||
| 'thinking', | ||
| 'tool-call', | ||
| 'builtin-tool-call', | ||
| 'builtin-tool-return', | ||
| 'tool-return', | ||
| 'compaction', | ||
| 'file', | ||
| ] | ||
|
dsfaccini marked this conversation as resolved.
|
||
| | None | ||
| ) = None | ||
| """The kind of the previous part, if any. | ||
|
|
@@ -2751,7 +2765,16 @@ class PartEndEvent: | |
| """The complete `ModelResponsePart`.""" | ||
|
|
||
| next_part_kind: ( | ||
| Literal['text', 'thinking', 'tool-call', 'builtin-tool-call', 'builtin-tool-return', 'compaction', 'file'] | ||
| Literal[ | ||
| 'text', | ||
| 'thinking', | ||
| 'tool-call', | ||
| 'builtin-tool-call', | ||
| 'builtin-tool-return', | ||
| 'tool-return', | ||
| 'compaction', | ||
| 'file', | ||
| ] | ||
| | None | ||
| ) = None | ||
| """The kind of the next part, if any. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.