-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Responses API - support tags in metadata #15867
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on November 25
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| elif litellm_metadata.get("tags", []): | ||
| request_tags = litellm_metadata.get("tags", []) | ||
| else: | ||
| request_tags = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Refactored Code Lacks Type Validation for Tags
The refactored _get_request_tags function no longer validates that tags is actually a list before using it. The old implementation checked isinstance(metadata.get("tags", []), list) to ensure type safety, but the new code directly assigns metadata.get("tags", []) or litellm_metadata.get("tags", []) without validation. If tags is set to a non-list value (e.g., a string or dict), this could cause runtime errors when the code later tries to extend request_tags with user_agent_tags or additional_header_tags (line 4474-4476), since extend() expects an iterable.
| elif litellm_metadata.get("tags", []): | ||
| request_tags = litellm_metadata.get("tags", []) | ||
| else: | ||
| request_tags = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Metadata Tag Priority Violation
The new logic for _get_request_tags violates the stated priority that "metadata should take priority" over "litellm_metadata". When metadata["tags"] exists but is an empty list [], the condition if metadata.get("tags", []): evaluates to False (since empty lists are falsy), causing the code to fall through and potentially use tags from litellm_metadata instead. The correct behavior should be to use metadata's tags (even if empty) when they exist, and only fall back to litellm_metadata when metadata doesn't have a "tags" key at all. This should be changed to check for key existence: if "tags" in metadata: instead of if metadata.get("tags", []):.
Title
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes