fix(proxy): case-insensitive User-Agent header lookup in _get_user_agent_tags - #41012
fix(proxy): case-insensitive User-Agent header lookup in _get_user_agent_tags#41012ege-arhan wants to merge 3 commits into
Conversation
|
ege-arhan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Greptile SummaryThis PR makes User-Agent extraction case-insensitive so capitalized header names produce the expected request and spend-tracking tags
Confidence Score: 4/5The behavior change appears correct, but the explicit source-comment requirement must be satisfied before merging The case-insensitive lookup and regression assertions are sound; remaining findings are limited to redundant nesting and a repository-rule violation Files Needing Attention: litellm/litellm_core_utils/litellm_logging.py, tests/test_litellm/litellm_core_utils/test_litellm_logging.py
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/litellm_logging.py | Correctly adds case-insensitive User-Agent lookup, but introduces an identical nested null check |
| tests/test_litellm/litellm_core_utils/test_litellm_logging.py | Adds a meaningful regression assertion, along with an unnecessary comment that violates repository guidance |
Reviews (1): Last reviewed commit: "fix(proxy): case-insensitive User-Agent ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| user_agent: Final = next( | ||
| (v for k, v in headers.items() if k.lower() == "user-agent"), None | ||
| ) | ||
| if user_agent is not None: |
There was a problem hiding this comment.
The identical nested check adds needless indentation and makes this touched logic harder to read without changing behavior
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| assert "User-Agent: litellm" in tags | ||
| assert "User-Agent: litellm/0.1.0" in tags | ||
|
|
||
| # Case-insensitive header test |
There was a problem hiding this comment.
This comment merely restates the test, violating the repository directive limiting comments to complex logic, tooling, or TODOs. Remove it before merging
| # Case-insensitive header test |
Context Used: CLAUDE.md (source)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0333b7168
ℹ️ 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".
| user_agent: Final = next( | ||
| (v for k, v in headers.items() if k.lower() == "user-agent"), None | ||
| ) | ||
| if user_agent is not None: |
There was a problem hiding this comment.
Remove this redundant outer guard; its identical inner check adds a nesting level even though CLAUDE.md requires early returns over deep nesting
AGENTS.md reference: AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
| assert "User-Agent: litellm" in tags | ||
| assert "User-Agent: litellm/0.1.0" in tags | ||
|
|
||
| # Case-insensitive header test |
There was a problem hiding this comment.
Remove this explanatory test comment; CLAUDE.md permits comments only for complex logic, tool directives, or justified TODO/FIXME entries
AGENTS.md reference: AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Problem
StandardLoggingPayloadSetup._get_user_agent_tagsperforms a case-sensitive dictionary lookupif "user-agent" in headers:. Per HTTP specifications (RFC 7230 / RFC 9110), HTTP headers are case-insensitive. When callers send capitalized headers such asUser-Agent: curl/7.68.0orUser-Agent: python-requests/2.31.0,_get_user_agent_tagsfails to detect the header and does not recordUser-Agentrequest tags in spend tracking or proxy request tags.Solution
Use a case-insensitive lookup over
headers.items()matchingk.lower() == "user-agent"to extract the header value regardless of capitalization.Impact
Callers sending
User-Agentwith any capitalization will have their client tags correctly tracked in spend logs and analytics.Evidence
Added test case in
tests/test_litellm/litellm_core_utils/test_litellm_logging.pyvalidating thatUser-Agent: curl/7.68.0correctly extractsUser-Agent: curlandUser-Agent: curl/7.68.0.Fixes #40979