fix: bound jinja scalar range render probes - #1553
Conversation
Performance BenchmarksCompared
|
|
Critical review found additional worker-unavailable false negatives, so this is not merge-ready yet. The eager filter list now covers direct
I prepared and validated a context-sensitive fix that preserves cheap direct Publication is currently blocked because shell GitHub credentials are unavailable and the connector has no patch-based write endpoint for these large files. Do not merge the current remote head. |
|
Critical review complete and the branch is synchronized with current The original one-line filter expansion coupled scalar CPU work to the output-character budget, causing both false positives and false negatives. The revised implementation follows Jinja's actual sandbox invariant ( Adversarial coverage includes direct reductions, lazy/default wrappers, assigned values, function alias chains, tuple and conditional bindings, namespaces, higher-order consumers, exact-boundary benign cases, macro shadowing, and overwritten aliases. Focused QA: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9c55361ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72adba986c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Critical review follow-up published at Addressed all remaining feedback:
QA:
All review threads are resolved. CI is running on the exact published head; moving to the next PR now and will revisit this one later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 941f0968a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed all five outstanding Jinja fallback findings in
Validation after the main refresh: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ad2eb096
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pull request was converted to draft
|
Addressed the three reachability findings in
The Jinja suite passes |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1af248bc5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pull request was converted to draft
|
Addressed the third review wave in
The Jinja suite passes |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb8ea7ffbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pull request was converted to draft
|
Addressed the latest review wave in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8dd006b5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pull request was converted to draft
ec6f993 to
f5d11c2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5d11c21e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review follow-up published at |
|
Critical review and QA are complete on exact head The review found and fixed both false-negative and false-positive classes around branch-only bindings, Focused validation: |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73bf639e0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| active_macros, | ||
| ): | ||
| return True | ||
| condition = self._constant_condition_value(node.test) |
There was a problem hiding this comment.
Pass bindings when folding inline conditions
Earlier {% set %} constants are ignored when choosing an inline-if branch. With worker unavailable, {% set c=false %}{{ range(100001)|min if c else 0 }} renders 0, but the static pass treats c as unknown and scans the oversized branch, causing a false failure. guidance
Useful? React with 👍 / 👎.
| if boolean_value is None: | ||
| return True | ||
| boolean_mode = boolean_value | ||
| known, value = cls._constant_condition_scalar(node.node) |
There was a problem hiding this comment.
Pass bindings into default fallback checks
This evaluates default input without current bindings, so earlier truthy values look unknown. With worker unavailable, {% set v=1 %}{{ (v|default(range,true))(100001)|min }} never uses range, but the fallback treats the default as possible and reports a budget failure. guidance
Useful? React with 👍 / 👎.
| for child in macro.body | ||
| for candidate in child.find_all(jinja2.nodes.Name) |
There was a problem hiding this comment.
Skip nested macros when detecting special macro args
This recursive find_all counts kwargs/varargs loads inside nested macro definitions as if the outer macro consumed them. With worker unavailable, {% macro m() %}{% macro inner() %}{{ kwargs }}{% endmacro %}{{ range(100001)|min }}{% endmacro %}{{ m(foo=1) }} is expanded and flagged, but Jinja rejects the unknown keyword before m runs. guidance
Useful? React with 👍 / 👎.
Summary
rangework exceeds Jinja'sMAX_RANGEand the render worker is unavailablevarargs,kwargs, and bounded literal unpackingdefault/dand.get()fallback analysisCritical review fixes
Validation
73bf639e0da7fb6d0933df0ad01da62e1c9fb91854637b4b959f61346dea50acbaf48a1fcac611a2main:838f25046fb94fe018b18a5a2a84e4aef3ed969e410 passed, 1 skipped(expected optionalggufdependency unavailable)25/25expected outcomesgit diff --check: cleanAll reviewed inline threads were resolved before publication.