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

wrap generator #3

Merged
merged 1 commit into from
Mar 13, 2024
Merged

wrap generator #3

merged 1 commit into from
Mar 13, 2024

Conversation

biobootloader
Copy link
Member

No description provided.

@@ -18,7 +18,7 @@ class SpiceResponse:
# if stream=False, all attributes will be available immediately
# if stream=True, will error if trying to access attributes before stream is exhausted

def __init__(self, stream_generator, full_text=None, cost=None, usage=None):
def __init__(self, stream_generator=None, full_text=None, cost=None, usage=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider implementing a more robust way to check if a generator is exhausted. Returning True by default may lead to incorrect behavior if the generator still has elements to yield. You could potentially wrap the generator to track its state or explore other methods to check its exhaustion.

Copy link
Contributor

mentatbot bot commented Mar 13, 2024

MENTAT CODE REVIEW IN ACTIVE DEVELOPMENT. Only in use on mentat and internal repos.
Please Reply with feedback.

The changes introduced in this pull request seem to aim at improving the handling of streaming responses from different API providers. The approach of wrapping the stream generator is a good one, but there are a few areas where the implementation could be improved for clarity and robustness, particularly around the handling of generator state and the definition of variables within the scope of functions.

@biobootloader biobootloader merged commit 7f359c0 into main Mar 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant