Skip to content

[litellm-agent] Staging → litellm_internal_staging (5/22/2026)#28601

Closed
oss-pr-review-agent-shin[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
shin_agent_oss_staging_05_22_2026
Closed

[litellm-agent] Staging → litellm_internal_staging (5/22/2026)#28601
oss-pr-review-agent-shin[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
shin_agent_oss_staging_05_22_2026

Conversation

@oss-pr-review-agent-shin

@oss-pr-review-agent-shin oss-pr-review-agent-shin Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Merged PRs (1)

# Title
#28598 fix(thinking): handle None thinking param in is_thinking_enabled

Auto-updated by litellm-agent on each merge.

)

Squash-merged by litellm-agent from Terrajlz's PR.
@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor Author

@greptile please review

@CLAassistant

CLAassistant commented May 22, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ devauxbr
❌ Terrajlz
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This staging merge bundles a single bug fix (#28598) that prevents an AttributeError in is_thinking_enabled when the thinking parameter is None, plus a Helm chart enhancement that lets users embed Go template expressions inside podAnnotations.

  • transformation.py: Replaces non_default_params.get(\"thinking\", {}) with (non_default_params.get(\"thinking\") or {}) so that None, False, and other falsy values fall back to {} rather than raising AttributeError on the subsequent .get(\"type\") call. Ten parametrised unit tests are added alongside the fix.
  • deployment.yaml: podAnnotations is now rendered through tpl, enabling dynamic values like {{ .Values.image.tag }}; a helm-unittest test covers the new behaviour.

Confidence Score: 5/5

Safe to merge — both changes are narrow, well-tested, and fix real user-facing problems without touching auth or critical request paths.

The Python fix is a one-liner with comprehensive parametrised tests covering every falsy edge case. The Helm change follows standard Helm idioms and is validated by a new helm-unittest test. No logic regressions, no security implications.

No files require special attention, though users of the Helm chart with literal brace characters in their podAnnotations values should be aware of the tpl rendering change.

Important Files Changed

Filename Overview
litellm/llms/base_llm/chat/transformation.py One-line fix: replaces .get("thinking", {}) with (... or {}) to handle None thinking param without AttributeError
tests/test_litellm/test_thinking_enabled.py New unit test file covering 10 parameter combinations for is_thinking_enabled, including all the falsy edge cases addressed by the fix
deploy/charts/litellm-helm/templates/deployment.yaml podAnnotations now rendered via tpl, enabling Go template expressions; any existing user with literal {{ }} braces in annotation values would experience a breaking change
deploy/charts/litellm-helm/tests/deployment_tests.yaml New helm-unittest test validating tpl-based annotation rendering, covering both dynamic and literal annotation values

Reviews (2): Last reviewed commit: "feat(helm): support tpl rendering in pod..." | Re-trigger Greptile

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Squash-merged by litellm-agent from devauxbr's PR.
@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor Author

@greptile please review

@Sameerlite

Copy link
Copy Markdown
Collaborator

covered in #28770

@Sameerlite Sameerlite closed this May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants