-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: Add FallbackChatGenerator #9859
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
Conversation
Pull Request Test Coverage Report for Build 18592817487Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| - Any other exception | ||
| """ | ||
|
|
||
| def __init__(self, generators: list[ChatGenerator]): |
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.
for consistency with our other components that accept a ChatGenerator let's call this chat_generators
| def __init__(self, generators: list[ChatGenerator]): | |
| def __init__(self, chat_generators: list[ChatGenerator]): |
|
Thank you @sjrl I'll fix these now! |
releasenotes/notes/add-fallback-chat-generator-ffe557ca01fcdaca.yaml
Outdated
Show resolved
Hide resolved
Amnah199
left a comment
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.
Looks very good overall. Some minor comments, plus maybe we need to update pydocs, generators_api.yml to add the new chat generator.
Co-authored-by: Sebastian Husch Lee <[email protected]>
| def run( | ||
| self, | ||
| messages: list[ChatMessage], | ||
| generation_kwargs: Union[dict[str, Any], None] = None, | ||
| tools: Union[list[Tool], Toolset, None] = None, | ||
| streaming_callback: Union[StreamingCallbackT, None] = None, | ||
| ) -> dict[str, Any]: |
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.
Lets explain these parameters and return instance in docstrings.
|
@vblagoje lets just add consistent docstrings for |
Co-authored-by: Amna Mubashar <[email protected]>
Amnah199
left a comment
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.
Looks good. Only docstrings of to_dict and from_dict can be proved. Buts its your call.
Why
Introduces a resilient chat-generator wrapper so production pipelines can survive provider hiccups while keeping latency predictable and responses flowing.
What
FallbackChatGeneratorcomponent that cascades across multiple chat generators with detailed success/failure metadata.chat.__init__and documented behavior with docstrings.FallbackChatGeneratorunit tests covering sync/async execution and error propagation.How can it be used
How did you test it
pytest test/components/generators/chat/test_fallback.pyNotes for the reviewer
Focus on the fallback loop and metadata contract (
failed_generators,successful_generator_*); downstream consumers might rely on those keys for telemetry.