-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Allow custom type to be streamed and use native response #8778
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
Allow custom type to be streamed and use native response #8778
Conversation
No concerns on a quick scan. If @chenmoneygithub approves, feel free to merge. |
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.
Very clean and cool! Left some comments for discussion.
@chenmoneygithub I've moved the native response type config to adapter, can you take a look again? |
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.
Some minor comments, otherwise LGTM!
def output_type(self) -> type | None: | ||
try: | ||
return self.predict.signature.output_fields[self.signature_field_name].annotation | ||
except Exception: |
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.
when will this throw an exception?
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 does not fail in normal cases, but since predict
or signature_field_name
is not guaranteed to be non-null, it's better to have a safeguard
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.
ic, actually I think predict
and signature_field_name
must be configured for stream listener, otherwise it is not doing anything.
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 know, but typing-wise they are nullable (
dspy/dspy/streaming/streaming_listener.py
Lines 24 to 29 in 28a7f78
def __init__( | |
self, | |
signature_field_name: str, | |
predict: Any = None, | |
predict_name: str | None = None, | |
allow_reuse: bool = False, |
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.
LGTM after #8778 (comment) is resolved.
This PR increases the extensibility of dspy.Type and allows users to define a custom type that can be streamed or use native LM response. This is useful when users want to use provider specific response field such as tool call or citation.
Following methods are introduced into dspy.Type