feat: Adding watsonx support in Haystack #1949
Conversation
|
|
github-actions
bot
added
the
type:documentation
|
Hey @divyaruhil thanks for working on this integration! Here are some initial comments I have before doing an in-depth review:
|
integrations/watsonx/src/haystack_integrations/components/__init__.py
Outdated
Show resolved
Hide resolved
|
Thankyou @sjrl , for reviewing , sure i'll make the requested changes . |
integrations/watsonx/src/haystack_integrations/components/embedders/watsonx/__init__.py
Show resolved
Hide resolved
|
@divyaruhil thanks for making the changes so far! I'll also go ahead and do a deeper dive review on the actual components themselves now this week. |
|
Hey @divyaruhil I realize there are quite a few comments. I'd also be happy to push the changes myself if you are willing to give me write access to your branch. Let me know! |
|
Hi @sjrl, thank you so much for reviewing! I know it’s quite a large PR, and I really appreciate your time. This is my first big contribution, so I’m still learning a lot as I go — which is probably why there are quite a few mistakes. I’ve already enabled “Allow edits by maintainers,” so feel free to push any changes directly to the branch! |
Yeah for sure! Add a new line for this integration here https://github.com/divyaruhil/haystack-core-integrations/blob/45a180bd2071550b222e35939903d3617b94036f/README.md?plain=1#L62 I'd suggest using the Bedrock (near the top) as an example to follow |
| self, | ||
| model: str = "ibm/slate-30m-english-rtrvr", |
There was a problem hiding this comment.
Repeating this just in case it may be missed.
For all component's let's enforce keyword arguments by making the following change
| self, | |
| model: str = "ibm/slate-30m-english-rtrvr", | |
| self, | |
| *, | |
| model: str = "ibm/slate-30m-english-rtrvr", |
...tions/watsonx/src/haystack_integrations/components/generators/watsonx/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
| deserialize_secrets_inplace(data["init_parameters"], keys=["api_key"]) | ||
| return default_from_dict(cls, data) | ||
|
|
||
| @component.output_types(replies=list[str], meta=list[dict[str, Any]], chunks=list[StreamingChunk]) |
There was a problem hiding this comment.
Ahh we don't typically return the chunks as part of the response. We usually only create them internally and pass them to a streaming_callback function. You can see an example of that in our OpenAIChatGenerator._handle_stream_response where you can see one of it's args is callback: SyncStreamingCallbackT. You can find the function here
| @component | ||
| class WatsonxGenerator: | ||
| """ | ||
| Generates text using IBM's watsonx.ai foundational models. |
There was a problem hiding this comment.
Sorry to have not brought this up earlier, but it would be great if you could actually have WatsonxGenerator inherit from WatsonxChatGenerator and simply overwrite the relevant methods (e.g. run and run_async) to work with our standard Generator.run inputs which are
@component.output_types(replies=List[str], meta=List[Dict[str, Any]])
def run(
self,
prompt: str,
system_prompt: Optional[str] = None,
streaming_callback: Optional[StreamingCallbackT] = None,
generation_kwargs: Optional[Dict[str, Any]] = None,
):This is a pattern we are planning to follow for our other chat generators but haven't had a chance to do yet. But hopefully this should help reduce duplicate code between the two components.
Let me know if you need anymore clarifications!
Co-authored-by: Sebastian Husch Lee <[email protected]>
Co-authored-by: Sebastian Husch Lee <[email protected]>
|
Hi @sjrl, after committing the suggested changes, some tests are now failing. Should I revert those changes? Also, just a quick heads-up—it might take me a little while to thoughtfully work through all the requested updates, but I’m on it! |
Hey @divyaruhil my colleague @anakin87 and I decided to go through the pyproject.toml and the github workflow to make sure it matched with our other integrations. Currently everything is passing which is great! So I think it's safe to leave those files alone now and please go ahead with the other requested updates! |
Related Issues
Proposed Changes:
Add WatsonxGenerator and WatsonxChatGenerator components to wrap IBM watsonx.ai’s text- and chat-generation APIs (supporting streaming, custom models, and generation parameters).
Add WatsonxTextEmbedder and WatsonxDocumentEmbedder components that support embedding use cases for both text and documents.
Ensure all components follow the existing Haystack interfaces and patterns for Generator and Embedder.
Include tests files accordingly.
How did you test it?
--Text generation works with WatsonxGenerator
--Chat generation works with WatsonxChatGenerator
-- Embeddings are correctly generated for both text and documents using the respective embedder components
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.