feat: add agent_with_prompt_builder super-component#440
feat: add agent_with_prompt_builder super-component#440
Conversation
As we've seen ChatPromptBuilder be paired with Agent for every use-case, a good opportunity for simplification is a super-component that encapsulates the two.
Pull Request Test Coverage Report for Build 22105228703Details
💛 - Coveralls |
| # --- ChatPromptBuilder parameters --- # | ||
| template: list[ChatMessage] | str | None = None, | ||
| required_variables: list[str] | Literal["*"] | None = None, | ||
| variables: list[str] | None = None, |
There was a problem hiding this comment.
For simplicity we could consider dropping variables and just not use it. I don't think it's often used since we often rely on the auto detection of the template
There was a problem hiding this comment.
Noted! I took the entire function signatures from the underlying components, but happy to simplify those.
| self, | ||
| *, | ||
| # --- ChatPromptBuilder parameters --- # | ||
| template: list[ChatMessage] | str | None = None, |
There was a problem hiding this comment.
I think a preferred interface for this new component is to just expose a param called user_prompt: str which we feed into the ChatPromptBuilder internally.
There was a problem hiding this comment.
I'm not sure I understand the semantics here, can you enlighten me?
Would the user_prompt contain the jinja2 template? And if so, why would you prefer this nomenclature? What would feeding it internally entail?
|
A few other comments I have:
|
|
|
||
|
|
||
| @super_component | ||
| class AgentWithPromptBuilder: |
There was a problem hiding this comment.
Also naming-wise I think we could still do with some iteration but I think we could start with something like IntegratedAgent or UnifiedAgent
There was a problem hiding this comment.
Happy to oblige, I figured I lacked the creativity, so I'd rather be explicit 😄
| from typing import TYPE_CHECKING, Any, Literal | ||
|
|
||
| from haystack import Pipeline, logging, super_component | ||
| from haystack.components.agents.agent import Agent as HaystackAgent |
There was a problem hiding this comment.
We should use the Agent already in the experimental package as the building block. Since we need the ChatMessageStore integration for this to work in platform if we want to enable chat history fetching.
3 participants
Related Issues
n/a
Proposed Changes:
As we've seen ChatPromptBuilder be paired with Agent for every use-case on the Platform, a good opportunity for simplification would be a (super-)component that encapsulates the two.
This is an attempts at implementation with a super-component indeed, happy for general feedback on that take.
How did you test it?
Added unit tests, mainly to ensure good behavior of the super-component's input inference based on the template provided.
Notes for the reviewer
I've tried to be exhaustive but there is one thing that I still don't quite get, and would appreciate guidance on:
to_dictandfrom_dict. I saw for example on Haystack'sdocument_preprocessorhow they are defined, but not on themulti_file_converterand I'm not sure I understand why on or the other.Also - as it's my first contribution to Haystack (albeit experimental), any pointers as to unfollowed standards, code quality or misunderstandings of the framework are more than welcome!
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.