-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: Fix variable extraction in Jinja2 templates returning set variables #10093
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Closing in favor of a new PR to pin jinja2 after an offline discussion |
|
Reopening since we found that pinning jinja2 to |
Pull Request Test Coverage Report for Build 19852408366Details
💛 - Coveralls |
|
@sjrl the only thing that comes to mind here is to extract free standing method: and then use it across builders, adapters, and routers! |
Good point! I'll probably mark it as a private method since it doesn't cover every case and I'd hope this will be properly fixed by the Jinja2 library. |
Related Issues
PromptBuilderincorrectly marks internally set Jinja2 variables as required inputs when using `required_variables='*' #9916Proposed Changes:
While investigating #9916 I found this open issue pallets/jinja#2069 in the Jinja2 library which is the cause of our issue.
I've developed a partial workaround by introducing the utility method
_collect_assigned_variables. The work around is partial because it does not exhaustively identify all types of assignment. It only covers regular assignment, list and tuple assignments.Unfortunately the suggested solution in the issue in the Jinja2 library of pinning the version to
jinja2<3.1.5does not work. I've left a follow-up comment on their issue.How did you test it?
Added new tests.
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.