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

Proposal to make input variables to PromptBuilder and ChatPromptBuilder required by default #8903

Open
sjrl opened this issue Feb 21, 2025 · 4 comments
Labels
P2 Medium priority, add to the next sprint if no P1 available

Comments

@sjrl
Copy link
Contributor

sjrl commented Feb 21, 2025

Is your feature request related to a problem? Please describe.
Most of our components require some (or all) inputs during runtime. For our components whose inputs are based on Jinja2 templates (e.g. ConditionalRouter, OutputAdpater, PromptBuilder, and ChatPromptBuilder) we differ on how we treat whether all Jinja2 variables are required or are optional by default. For example, the components ConditionalRouter, and OutputAdpater we require all Jinja2 variables defined in their templates to run. But for the PromptBuilder, and ChatPromptBuilder we set all Jinja2 variables as optional by default.

This optionality has caused "intended" but usually unexpected behavior (from the perspective of the user) when running pipelines with multiple branches where each branch may contain a (Chat)PromptBuilder + (Chat)Generator. Specifically, if no required variables are set in the prompt builder then that component will always trigger even if it's along a branch that has been turned "off" by a previous ConditionalRouter.

This can lead to unexpected responses from a branch of the pipeline that wasn't meant to trigger from the users perspective. Ie you could end up with two answers from two branches in a pipeline even though a user only expected one to occur.

Describe the solution you'd like
I'd like to propose changing the default behavior of the PromptBuilder and ChatPromptBuilder to require all input variables rather than having them be optional by default. This would be a breaking change but one that I think is more intuitive to users and would be inline with how our ConditionalRouter and OutputAdapter work.

Describe alternatives you've considered
Leave as is and just add a warning to PromptBuilder and ChatPromptBuilder. See #8901

Additional context
@ju-gu and I have run into this multiple times when building pipelines for clients.

@anakin87
Copy link
Member

Old related issue: #7441

@mathislucka
Copy link
Member

@sjrl with the new run-logic released in 2.10 the component will not always trigger anymore. It needs at least some input to run. This means:

  • PromptBuilder with documents (pipeline provided) and query (user provided) will trigger even if it never receives documents (but only once)
  • PromptBuilder with only documents (pipeline provided) will not trigger when it never receives documents

From my experience, that addresses at least some of the cases where a running PromptBuilder might have surprised users previously.

@sjrl
Copy link
Contributor Author

sjrl commented Feb 24, 2025

@mathislucka thanks for the additional info. I'll need to talk with @ju-gu again about how exactly his pipeline looks that recently faced this issue to see if its with the scenarios described.

@julian-risch julian-risch added the P2 Medium priority, add to the next sprint if no P1 available label Feb 24, 2025
@ju-gu
Copy link
Member

ju-gu commented Feb 25, 2025

PromptBuilder with documents (pipeline provided) and query (user provided) will trigger even if it never receives documents (but only once)

I think this can still cause problems as it can run before the correct input is created inside the pipeline, if no required params are set, right?

So probably still worth making all variables to be required by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Medium priority, add to the next sprint if no P1 available
Projects
None yet
Development

No branches or pull requests

5 participants